Add unit test for Instructions class and improve Criteria toString() test

Change-Id: Ie1ffb4ca0c0bcd168625213fecbdb3818a61704e
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
index 86ea44a..abe19e3 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/instructions/L2ModificationInstruction.java
@@ -119,7 +119,7 @@
      */
     public static final class ModVlanIdInstruction extends L2ModificationInstruction {
 
-        public final VlanId vlanId;
+        private final VlanId vlanId;
 
         public ModVlanIdInstruction(VlanId vlanId) {
             this.vlanId = vlanId;
@@ -168,7 +168,7 @@
      */
     public static final class ModVlanPcpInstruction extends L2ModificationInstruction {
 
-        public final Byte vlanPcp;
+        private final Byte vlanPcp;
 
         public ModVlanPcpInstruction(Byte vlanPcp) {
             this.vlanPcp = vlanPcp;
diff --git a/core/api/src/test/java/org/onlab/onos/net/flow/criteria/CriteriaTest.java b/core/api/src/test/java/org/onlab/onos/net/flow/criteria/CriteriaTest.java
index 6a23878..6b113a2 100644
--- a/core/api/src/test/java/org/onlab/onos/net/flow/criteria/CriteriaTest.java
+++ b/core/api/src/test/java/org/onlab/onos/net/flow/criteria/CriteriaTest.java
@@ -116,7 +116,7 @@
     private <T> T checkAndConvert(Criterion criterion, Criterion.Type type, Class clazz) {
         assertThat(criterion, is(notNullValue()));
         assertThat(criterion.type(), is(equalTo(type)));
-        assertThat(criterion, is(instanceOf(clazz)));
+        assertThat(criterion, instanceOf(clazz));
         return (T) criterion;
     }
 
@@ -131,16 +131,19 @@
      */
     private <T extends Criterion> void checkEqualsAndToString(T c1, T c1match,
                                                               T c2, Class clazz) {
-        assertThat(c1, is(instanceOf(clazz)));
-        assertThat(c1match, is(instanceOf(clazz)));
-        assertThat(c2, is(instanceOf(clazz)));
+        assertThat(c1, instanceOf(clazz));
+        assertThat(c1match, instanceOf(clazz));
+        assertThat(c2, instanceOf(clazz));
 
         assertThat(c1, is(equalTo(c1match)));
         assertThat(c1, is(not(equalTo(c2))));
         assertThat(c1, is(not(equalTo(new Object()))));
 
-        // Make sure the enumerated type appears in the toString() output.
+        // Make sure the enumerated type appears in the toString() output and
+        // the toString output is unique.
         assertThat(c1.toString(), containsString(c1.type().toString()));
+        assertThat(c1.toString(), equalTo(c1match.toString()));
+        assertThat(c1.toString(), not(equalTo(c2.toString())));
     }
 
 
diff --git a/core/api/src/test/java/org/onlab/onos/net/flow/instructions/InstructionsTest.java b/core/api/src/test/java/org/onlab/onos/net/flow/instructions/InstructionsTest.java
new file mode 100644
index 0000000..b43393b
--- /dev/null
+++ b/core/api/src/test/java/org/onlab/onos/net/flow/instructions/InstructionsTest.java
@@ -0,0 +1,455 @@
+/*
+ * Copyright 2014 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onlab.onos.net.flow.instructions;
+
+import org.junit.Test;
+import org.onlab.onos.net.PortNumber;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.IpPrefix;
+import org.onlab.packet.MacAddress;
+import org.onlab.packet.VlanId;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
+import static org.onlab.junit.UtilityClassChecker.assertThatClassIsUtility;
+import static org.onlab.onos.net.PortNumber.portNumber;
+
+/**
+ * Unit tests for the Instructions class.
+ */
+public class InstructionsTest {
+
+    /**
+     * Checks that a Criterion object has the proper type, and then converts
+     * it to the proper type.
+     *
+     * @param instruction Instruction object to convert
+     * @param type Enumerated type value for the Criterion class
+     * @param clazz Desired Criterion class
+     * @param <T> The type the caller wants returned
+     * @return converted object
+     */
+    @SuppressWarnings("unchecked")
+    private <T> T checkAndConvert(Instruction instruction, Instruction.Type type, Class clazz) {
+        assertThat(instruction, is(notNullValue()));
+        assertThat(instruction.type(), is(equalTo(type)));
+        assertThat(instruction, instanceOf(clazz));
+        return (T) instruction;
+    }
+
+    /**
+     * Checks the equals() and toString() methods of a Criterion class.
+     *
+     * @param c1 first object to compare
+     * @param c1match object that should be equal to the first
+     * @param c2 object that should be not equal to the first
+     * @param clazz Class object for the Criterion subclass
+     * @param <T> type of the arguments
+     */
+    private <T extends Instruction> void checkEqualsAndToString(T c1, T c1match,
+                                                                T c2, Class clazz) {
+        assertThat(c1, instanceOf(clazz));
+        assertThat(c1match, instanceOf(clazz));
+        assertThat(c2, instanceOf(clazz));
+
+        assertThat(c1, is(equalTo(c1match)));
+        assertThat(c1, is(not(equalTo(c2))));
+        assertThat(c1, is(not(equalTo(new Object()))));
+
+        // Make sure the toString() output is unique and correct.
+        assertThat(c1.toString(), containsString("{"));
+        assertThat(c1.toString(), equalTo(c1match.toString()));
+        assertThat(c1.toString(), not(equalTo(c2.toString())));
+    }
+
+    /**
+     * Checks that Instructions is a proper utility class.
+     */
+    @Test
+    public void testInstructionsUtilityClass() {
+        assertThatClassIsUtility(Instructions.class);
+    }
+
+    /**
+     * Checks that the Instruction class implementations are immutable.
+     */
+    @Test
+    public void testImmutabilityOfInstructions() {
+        assertThatClassIsImmutable(Instructions.DropInstruction.class);
+        assertThatClassIsImmutable(Instructions.OutputInstruction.class);
+        assertThatClassIsImmutable(L0ModificationInstruction.ModLambdaInstruction.class);
+        assertThatClassIsImmutable(L2ModificationInstruction.ModEtherInstruction.class);
+        assertThatClassIsImmutable(L2ModificationInstruction.ModVlanIdInstruction.class);
+        assertThatClassIsImmutable(L2ModificationInstruction.ModVlanPcpInstruction.class);
+        assertThatClassIsImmutable(L3ModificationInstruction.ModIPInstruction.class);
+    }
+
+    //  DropInstruction
+
+    private final Instructions.DropInstruction drop1 = Instructions.createDrop();
+    private final Instructions.DropInstruction drop2 = Instructions.createDrop();
+
+    /**
+     * Test the createDrop method.
+     */
+    @Test
+    public void testCreateDropMethod() {
+        Instructions.DropInstruction instruction = Instructions.createDrop();
+        checkAndConvert(instruction,
+                        Instruction.Type.DROP,
+                        Instructions.DropInstruction.class);
+    }
+
+    /**
+     * Test the equals() method of the DropInstruction class.
+     */
+
+    @Test
+    public void testDropInstructionEquals() throws Exception {
+        assertThat(drop1, is(equalTo(drop2)));
+    }
+
+    /**
+     * Test the hashCode() method of the DropInstruction class.
+     */
+
+    @Test
+    public void testDropInstructionHashCode() {
+        assertThat(drop1.hashCode(), is(equalTo(drop2.hashCode())));
+    }
+
+    //  OutputInstruction
+
+    private final PortNumber port1 = portNumber(1);
+    private final PortNumber port2 = portNumber(2);
+    private final Instructions.OutputInstruction output1 = Instructions.createOutput(port1);
+    private final Instructions.OutputInstruction sameAsOutput1 = Instructions.createOutput(port1);
+    private final Instructions.OutputInstruction output2 = Instructions.createOutput(port2);
+
+    /**
+     * Test the createOutput method.
+     */
+    @Test
+    public void testCreateOutputMethod() {
+        final Instruction instruction = Instructions.createOutput(port2);
+        final Instructions.OutputInstruction outputInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.OUTPUT,
+                        Instructions.OutputInstruction.class);
+        assertThat(outputInstruction.port(), is(equalTo(port2)));
+    }
+
+
+    /**
+     * Test the equals() method of the OutputInstruction class.
+     */
+
+    @Test
+    public void testOutputInstructionEquals() throws Exception {
+        checkEqualsAndToString(output1, sameAsOutput1, output2,
+                Instructions.OutputInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the OutputInstruction class.
+     */
+
+    @Test
+    public void testOutputInstructionHashCode() {
+        assertThat(output1.hashCode(), is(equalTo(sameAsOutput1.hashCode())));
+        assertThat(output1.hashCode(), is(not(equalTo(output2.hashCode()))));
+    }
+
+    //  ModLambdaInstruction
+
+    private final short lambda1 = 1;
+    private final short lambda2 = 2;
+    private final Instruction lambdaInstruction1 = Instructions.modL0Lambda(lambda1);
+    private final Instruction sameAsLambdaInstruction1 = Instructions.modL0Lambda(lambda1);
+    private final Instruction lambdaInstruction2 = Instructions.modL0Lambda(lambda2);
+
+    /**
+     * Test the modL0Lambda method.
+     */
+    @Test
+    public void testCreateLambdaMethod() {
+        final Instruction instruction = Instructions.modL0Lambda(lambda1);
+        final L0ModificationInstruction.ModLambdaInstruction lambdaInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L0MODIFICATION,
+                        L0ModificationInstruction.ModLambdaInstruction.class);
+        assertThat(lambdaInstruction.lambda(), is(equalTo(lambda1)));
+    }
+
+
+    /**
+     * Test the equals() method of the ModLambdaInstruction class.
+     */
+
+    @Test
+    public void testModLambdaInstructionEquals() throws Exception {
+        checkEqualsAndToString(lambdaInstruction1,
+                               sameAsLambdaInstruction1,
+                               lambdaInstruction2,
+                               L0ModificationInstruction.ModLambdaInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the ModLambdaInstruction class.
+     */
+
+    @Test
+    public void testModLambdaInstructionHashCode() {
+        assertThat(lambdaInstruction1.hashCode(),
+                   is(equalTo(sameAsLambdaInstruction1.hashCode())));
+        assertThat(lambdaInstruction1.hashCode(),
+                   is(not(equalTo(lambdaInstruction2.hashCode()))));
+    }
+
+    //  ModEtherInstruction
+
+    private static final String MAC1 = "00:00:00:00:00:01";
+    private static final String MAC2 = "00:00:00:00:00:02";
+    private final MacAddress mac1 = MacAddress.valueOf(MAC1);
+    private final MacAddress mac2 = MacAddress.valueOf(MAC2);
+    private final Instruction modEtherInstruction1 = Instructions.modL2Src(mac1);
+    private final Instruction sameAsModEtherInstruction1 = Instructions.modL2Src(mac1);
+    private final Instruction modEtherInstruction2 = Instructions.modL2Src(mac2);
+
+    /**
+     * Test the modL2Src method.
+     */
+    @Test
+    public void testModL2SrcMethod() {
+        final Instruction instruction = Instructions.modL2Src(mac1);
+        final L2ModificationInstruction.ModEtherInstruction modEtherInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L2MODIFICATION,
+                        L2ModificationInstruction.ModEtherInstruction.class);
+        assertThat(modEtherInstruction.mac(), is(equalTo(mac1)));
+        assertThat(modEtherInstruction.subtype(),
+                is(equalTo(L2ModificationInstruction.L2SubType.ETH_SRC)));
+    }
+
+    /**
+     * Test the modL2Dst method.
+     */
+    @Test
+    public void testModL2DstMethod() {
+        final Instruction instruction = Instructions.modL2Dst(mac1);
+        final L2ModificationInstruction.ModEtherInstruction modEtherInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L2MODIFICATION,
+                        L2ModificationInstruction.ModEtherInstruction.class);
+        assertThat(modEtherInstruction.mac(), is(equalTo(mac1)));
+        assertThat(modEtherInstruction.subtype(),
+                is(equalTo(L2ModificationInstruction.L2SubType.ETH_DST)));
+    }
+
+    /**
+     * Test the equals() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModEtherInstructionEquals() throws Exception {
+        checkEqualsAndToString(modEtherInstruction1,
+                               sameAsModEtherInstruction1,
+                               modEtherInstruction2,
+                               L2ModificationInstruction.ModEtherInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModEtherInstructionHashCode() {
+        assertThat(modEtherInstruction1.hashCode(),
+                   is(equalTo(sameAsModEtherInstruction1.hashCode())));
+        assertThat(modEtherInstruction1.hashCode(),
+                   is(not(equalTo(modEtherInstruction2.hashCode()))));
+    }
+
+
+    //  ModVlanIdInstruction
+
+    private final short vlan1 = 1;
+    private final short vlan2 = 2;
+    private final VlanId vlanId1 = VlanId.vlanId(vlan1);
+    private final VlanId vlanId2 = VlanId.vlanId(vlan2);
+    private final Instruction modVlanId1 = Instructions.modVlanId(vlanId1);
+    private final Instruction sameAsModVlanId1 = Instructions.modVlanId(vlanId1);
+    private final Instruction modVlanId2 = Instructions.modVlanId(vlanId2);
+
+    /**
+     * Test the modVlanId method.
+     */
+    @Test
+    public void testModVlanIdMethod() {
+        final Instruction instruction = Instructions.modVlanId(vlanId1);
+        final L2ModificationInstruction.ModVlanIdInstruction modEtherInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L2MODIFICATION,
+                        L2ModificationInstruction.ModVlanIdInstruction.class);
+        assertThat(modEtherInstruction.vlanId(), is(equalTo(vlanId1)));
+        assertThat(modEtherInstruction.subtype(),
+                is(equalTo(L2ModificationInstruction.L2SubType.VLAN_ID)));
+    }
+
+    /**
+     * Test the equals() method of the ModVlanIdInstruction class.
+     */
+
+    @Test
+    public void testModVlanIdInstructionEquals() throws Exception {
+        checkEqualsAndToString(modVlanId1,
+                               sameAsModVlanId1,
+                               modVlanId2,
+                               L2ModificationInstruction.ModVlanIdInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModVlanIdInstructionHashCode() {
+        assertThat(modVlanId1.hashCode(),
+                is(equalTo(sameAsModVlanId1.hashCode())));
+        assertThat(modVlanId1.hashCode(),
+                is(not(equalTo(modVlanId2.hashCode()))));
+    }
+
+
+    //  ModVlanPcpInstruction
+
+    private final byte vlanPcp1 = 1;
+    private final byte vlanPcp2 = 2;
+    private final Instruction modVlanPcp1 = Instructions.modVlanPcp(vlanPcp1);
+    private final Instruction sameAsModVlanPcp1 = Instructions.modVlanPcp(vlanPcp1);
+    private final Instruction modVlanPcp2 = Instructions.modVlanPcp(vlanPcp2);
+
+    /**
+     * Test the modVlanPcp method.
+     */
+    @Test
+    public void testModVlanPcpMethod() {
+        final Instruction instruction = Instructions.modVlanPcp(vlanPcp1);
+        final L2ModificationInstruction.ModVlanPcpInstruction modEtherInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L2MODIFICATION,
+                        L2ModificationInstruction.ModVlanPcpInstruction.class);
+        assertThat(modEtherInstruction.vlanPcp(), is(equalTo(vlanPcp1)));
+        assertThat(modEtherInstruction.subtype(),
+                is(equalTo(L2ModificationInstruction.L2SubType.VLAN_PCP)));
+    }
+
+    /**
+     * Test the equals() method of the ModVlanPcpInstruction class.
+     */
+
+    @Test
+    public void testModVlanPcpInstructionEquals() throws Exception {
+        checkEqualsAndToString(modVlanPcp1,
+                               sameAsModVlanPcp1,
+                               modVlanPcp2,
+                               L2ModificationInstruction.ModVlanPcpInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModVlanPcpInstructionHashCode() {
+        assertThat(modVlanPcp1.hashCode(),
+                is(equalTo(sameAsModVlanPcp1.hashCode())));
+        assertThat(modVlanPcp1.hashCode(),
+                is(not(equalTo(modVlanPcp2.hashCode()))));
+    }
+
+    //  ModIPerInstruction
+
+    private static final String IP1 = "1.2.3.4/24";
+    private static final String IP2 = "5.6.7.8/24";
+    private IpAddress ip1 = IpPrefix.valueOf(IP1).address();
+    private IpAddress ip2 = IpPrefix.valueOf(IP2).address();
+    private final Instruction modIPInstruction1 = Instructions.modL3Src(ip1);
+    private final Instruction sameAsModIPInstruction1 = Instructions.modL3Src(ip1);
+    private final Instruction modIPInstruction2 = Instructions.modL3Src(ip2);
+
+    /**
+     * Test the modL3Src method.
+     */
+    @Test
+    public void testModL3SrcMethod() {
+        final Instruction instruction = Instructions.modL3Src(ip1);
+        final L3ModificationInstruction.ModIPInstruction modIPInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L3MODIFICATION,
+                        L3ModificationInstruction.ModIPInstruction.class);
+        assertThat(modIPInstruction.ip(), is(equalTo(ip1)));
+        assertThat(modIPInstruction.subtype(),
+                is(equalTo(L3ModificationInstruction.L3SubType.IP_SRC)));
+    }
+
+    /**
+     * Test the modL3Dst method.
+     */
+    @Test
+    public void testModL3DstMethod() {
+        final Instruction instruction = Instructions.modL3Dst(ip1);
+        final L3ModificationInstruction.ModIPInstruction modIPInstruction =
+                checkAndConvert(instruction,
+                        Instruction.Type.L3MODIFICATION,
+                        L3ModificationInstruction.ModIPInstruction.class);
+        assertThat(modIPInstruction.ip(), is(equalTo(ip1)));
+        assertThat(modIPInstruction.subtype(),
+                is(equalTo(L3ModificationInstruction.L3SubType.IP_DST)));
+    }
+
+    /**
+     * Test the equals() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModIPInstructionEquals() throws Exception {
+        checkEqualsAndToString(modIPInstruction1,
+                               sameAsModIPInstruction1,
+                               modIPInstruction2,
+                               L3ModificationInstruction.ModIPInstruction.class);
+    }
+
+    /**
+     * Test the hashCode() method of the ModEtherInstruction class.
+     */
+
+    @Test
+    public void testModIPInstructionHashCode() {
+        assertThat(modIPInstruction1.hashCode(),
+                   is(equalTo(sameAsModIPInstruction1.hashCode())));
+        assertThat(modIPInstruction1.hashCode(),
+                   is(not(equalTo(modIPInstruction2.hashCode()))));
+    }
+
+
+}
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
index c731415..416dc0b 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer10.java
@@ -192,7 +192,7 @@
             return factory().actions().setDlSrc(MacAddress.of(eth.mac().toLong()));
         case VLAN_ID:
             ModVlanIdInstruction vlanId = (ModVlanIdInstruction) l2m;
-            return factory().actions().setVlanVid(VlanVid.ofVlan(vlanId.vlanId.toShort()));
+            return factory().actions().setVlanVid(VlanVid.ofVlan(vlanId.vlanId().toShort()));
         case VLAN_PCP:
             ModVlanPcpInstruction vlanPcp = (ModVlanPcpInstruction) l2m;
             return factory().actions().setVlanPcp(VlanPcp.of(vlanPcp.vlanPcp()));
diff --git a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
index fa8035e..9aff8b7 100644
--- a/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
+++ b/providers/openflow/flow/src/main/java/org/onlab/onos/provider/of/flow/impl/FlowModBuilderVer13.java
@@ -210,11 +210,11 @@
             break;
         case VLAN_ID:
             ModVlanIdInstruction vlanId = (ModVlanIdInstruction) l2m;
-            oxm = factory().oxms().vlanVid(OFVlanVidMatch.ofVlan(vlanId.vlanId.toShort()));
+            oxm = factory().oxms().vlanVid(OFVlanVidMatch.ofVlan(vlanId.vlanId().toShort()));
             break;
         case VLAN_PCP:
             ModVlanPcpInstruction vlanPcp = (ModVlanPcpInstruction) l2m;
-            oxm = factory().oxms().vlanPcp(VlanPcp.of(vlanPcp.vlanPcp));
+            oxm = factory().oxms().vlanPcp(VlanPcp.of(vlanPcp.vlanPcp()));
             break;
         default:
             log.warn("Unimplemented action type {}.", l2m.subtype());