Using VlanId String None instead of -1

Change-Id: I2597ac37285cc3f40ad1304d668564a56a5b862f

@@ -79,7 +79,7 @@ public final class HostId extends ElementId {
         checkArgument(string.length() >= MIN_ID_LENGTH,
                       "Host ID must be at least %s characters", MIN_ID_LENGTH);
         MacAddress mac = MacAddress.valueOf(string.substring(0, MAC_LENGTH));
-        VlanId vlanId = VlanId.vlanId(Short.parseShort(string.substring(MAC_LENGTH + 1)));
+        VlanId vlanId = VlanId.vlanId(string.substring(MAC_LENGTH + 1));
         return new HostId(mac, vlanId);
     }
 

@@ -83,7 +83,7 @@ public class ConnectPointTest {
         String cp = "16:3A:BD:6E:31:E4/-1/1";
 
         ConnectPoint connectPoint = ConnectPoint.hostConnectPoint(cp);
-        assertEquals("16:3A:BD:6E:31:E4/-1", connectPoint.hostId().toString());
+        assertEquals("16:3A:BD:6E:31:E4/None", connectPoint.hostId().toString());
         assertEquals("1", connectPoint.port().toString());
 
         expectHostParseException("");

@@ -49,8 +49,8 @@ import static org.onosproject.net.intent.LinksHaveEntryWithSourceDestinationPair
 public class HostToHostIntentCompilerTest extends AbstractIntentTest {
     private static final String HOST_ONE_MAC = "00:00:00:00:00:01";
     private static final String HOST_TWO_MAC = "00:00:00:00:00:02";
-    private static final String HOST_ONE_VLAN = "-1";
-    private static final String HOST_TWO_VLAN = "-1";
+    private static final String HOST_ONE_VLAN = "None";
+    private static final String HOST_TWO_VLAN = "None";
     private static final String HOST_ONE = HOST_ONE_MAC + "/" + HOST_ONE_VLAN;
     private static final String HOST_TWO = HOST_TWO_MAC + "/" + HOST_TWO_VLAN;
 

@@ -18,9 +18,9 @@ package org.onlab.packet;
 import org.onlab.util.Identifier;
 
 /**
- * Representation of a VLAN ID.
+ * Representation of a VLAN identifier.
  */
-public class VlanId extends Identifier<Short> {
+public final class VlanId extends Identifier<Short> {
     // Based on convention used elsewhere? Check and change if needed
     public static final short UNTAGGED = (short) 0xffff;
 
@@ -32,30 +32,45 @@ public class VlanId extends Identifier<Short> {
     public static final VlanId NONE = VlanId.vlanId(UNTAGGED);
     public static final VlanId ANY = VlanId.vlanId(ANY_VALUE);
 
+    private static final String STRING_NONE = "None";
+    private static final String STRING_NUMERIC_NONE = "-1";
+    private static final String STRING_ANY = "Any";
+
     // A VLAN ID is actually 12 bits of a VLAN tag.
     public static final short MAX_VLAN = 4095;
 
-    protected VlanId() {
+    // Constructor for serialization.
+    private VlanId() {
         super(UNTAGGED);
     }
 
-    protected VlanId(short value) {
+    // Creates a VLAN identifier for the specified VLAN number.
+    private VlanId(short value) {
         super(value);
     }
 
+    /**
+     * Creates a VLAN identifier for untagged VLAN.
+     *
+     * @return VLAN identifier
+     */
     public static VlanId vlanId() {
         return new VlanId(UNTAGGED);
     }
 
+    /**
+     * Creates a VLAN identifier using the supplied VLAN identifier.
+     *
+     * @param value VLAN identifier expressed as short
+     * @return VLAN identifier
+     */
     public static VlanId vlanId(short value) {
         if (value == UNTAGGED) {
             return new VlanId();
         }
-
         if (value == ANY_VALUE) {
             return new VlanId(ANY_VALUE);
         }
-
         if (value > MAX_VLAN) {
             throw new IllegalArgumentException(
                     "value exceeds allowed maximum VLAN ID value (4095)");
@@ -63,6 +78,31 @@ public class VlanId extends Identifier<Short> {
         return new VlanId(value);
     }
 
+    /**
+     * Creates a VLAN identifier Object using the supplied VLAN identifier.
+     *
+     * @param value VLAN identifier expressed as string
+     * @return VLAN identifier
+     */
+    public static VlanId vlanId(String value) {
+        if (value.equals(STRING_NONE) || value.equals(STRING_NUMERIC_NONE)) {
+            return new VlanId();
+        }
+        if (value.equals(STRING_ANY)) {
+            return new VlanId(ANY_VALUE);
+        }
+        try {
+            return new VlanId(Short.parseShort(value));
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException(e);
+        }
+    }
+
+    /**
+     * Returns the backing VLAN number.
+     *
+     * @return VLAN number
+     */
     public short toShort() {
         return this.identifier;
     }
@@ -72,6 +112,9 @@ public class VlanId extends Identifier<Short> {
         if (this.identifier == ANY_VALUE) {
             return "Any";
         }
+        if (this.identifier == UNTAGGED) {
+            return "None";
+        }
         return String.valueOf(this.identifier);
     }
 }

@@ -26,12 +26,14 @@ public class VlanIdTest {
     @Test
     public void testEquality() {
 
-        VlanId vlan1 = VlanId.vlanId((short) -1);
-        VlanId vlan2 = VlanId.vlanId((short) 100);
+        VlanId vlan1 = VlanId.vlanId("None");
+        VlanId vlan2 = VlanId.vlanId((short) -1);
         VlanId vlan3 = VlanId.vlanId((short) 100);
+        VlanId vlan4 = VlanId.vlanId((short) 100);
 
         new EqualsTester().addEqualityGroup(VlanId.vlanId(), vlan1)
-        .addEqualityGroup(vlan2, vlan3)
+        .addEqualityGroup(vlan1, vlan2)
+        .addEqualityGroup(vlan3, vlan4)
         .addEqualityGroup(VlanId.vlanId((short) 10));
 
     }
@@ -41,9 +43,12 @@ public class VlanIdTest {
         // purposefully create UNTAGGED VLAN
         VlanId vlan1 = VlanId.vlanId((short) 10);
         VlanId vlan2 = VlanId.vlanId((short) -1);
+        VlanId vlan3 = VlanId.vlanId("None");
 
         assertEquals("incorrect VLAN value", 10, vlan1.toShort());
-        assertEquals("invalid untagged value", VlanId.UNTAGGED, vlan2.toShort());
+        assertEquals("invalid untagged value", VlanId.UNTAGGED,
+                     vlan2.toShort(), vlan3.toShort());
+
     }
 
     @Test(expected = IllegalArgumentException.class)

@@ -390,7 +390,7 @@ public class HostResourceTest extends ResourceTest {
                 .post(Entity.json(jsonStream));
         assertThat(response.getStatus(), is(HttpURLConnection.HTTP_CREATED));
         String location = response.getLocation().getPath();
-        assertThat(location, Matchers.startsWith("/hosts/11:22:33:44:55:66/-1"));
+        assertThat(location, Matchers.startsWith("/hosts/11:22:33:44:55:66/None"));
     }
 }