Remove deprecated instructions() method in the traffic treatment class

Change-Id: I739b35bdcbf9867c639c7b6ca4006f3eeafbb055

@@ -123,7 +123,7 @@ public class FlowsListCommand extends AbstractShellCommand {
         }
 
         ArrayNode instr = mapper.createArrayNode();
-        for (Instruction i : flow.treatment().instructions()) {
+        for (Instruction i : flow.treatment().allInstructions()) {
             instr.add(i.toString());
         }
 

@@ -354,8 +354,8 @@ public class IntentsListCommand extends AbstractShellCommand {
             if (!ci.selector().criteria().isEmpty()) {
                 print("    selector=%s", ci.selector().criteria());
             }
-            if (!ci.treatment().instructions().isEmpty()) {
-                print("    treatment=%s", ci.treatment().instructions());
+            if (!ci.treatment().allInstructions().isEmpty()) {
+                print("    treatment=%s", ci.treatment().allInstructions());
             }
             if (ci.constraints() != null && !ci.constraints().isEmpty()) {
                 print("    constraints=%s", ci.constraints());
@@ -423,8 +423,8 @@ public class IntentsListCommand extends AbstractShellCommand {
             if (!ci.selector().criteria().isEmpty()) {
                 result.put("selector", ci.selector().criteria().toString());
             }
-            if (!ci.treatment().instructions().isEmpty()) {
-                result.put("treatment", ci.treatment().instructions().toString());
+            if (!ci.treatment().allInstructions().isEmpty()) {
+                result.put("treatment", ci.treatment().allInstructions().toString());
             }
         }
 

@@ -32,6 +32,8 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Default traffic treatment implementation.
  */
@@ -52,7 +54,7 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
      * @param instructions treatment instructions
      */
     private DefaultTrafficTreatment(List<Instruction> instructions) {
-        this.immediate = ImmutableList.copyOf(instructions);
+        this.immediate = ImmutableList.copyOf(checkNotNull(instructions));
         this.deferred = ImmutableList.of();
         this.hasClear = false;
         this.table = null;
@@ -62,19 +64,14 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
                                    List<Instruction> immediate,
                                    Instructions.TableTypeTransition table,
                                    boolean clear) {
-        this.immediate = ImmutableList.copyOf(immediate);
-        this.deferred = ImmutableList.copyOf(deferred);
+        this.immediate = ImmutableList.copyOf(checkNotNull(immediate));
+        this.deferred = ImmutableList.copyOf(checkNotNull(deferred));
         this.table = table;
         this.hasClear = clear;
 
     }
 
     @Override
-    public List<Instruction> instructions() {
-        return immediate;
-    }
-
-    @Override
     public List<Instruction> deferred() {
         return deferred;
     }
@@ -184,7 +181,7 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
         // Creates a new builder based off an existing treatment
         //FIXME only works for immediate instruction sets.
         private Builder(TrafficTreatment treatment) {
-            for (Instruction instruction : treatment.instructions()) {
+            for (Instruction instruction : treatment.immediate()) {
                 add(instruction);
             }
         }

@@ -32,14 +32,6 @@ import java.util.List;
 public interface TrafficTreatment {
 
     /**
-     * Returns list of instructions on how to treat traffic.
-     *
-     * @return list of treatment instructions
-     */
-    @Deprecated
-    List<Instruction> instructions();
-
-    /**
      * Returns the list of treatment instructions that will be applied
      * further down the pipeline.
      * @return list of treatment instructions

@@ -102,7 +102,7 @@ public class DefaultFlowRuleExt
                 .add("deviceId", deviceId())
                 .add("priority", priority())
                 .add("selector", selector().criteria())
-                .add("treatment", treatment() == null ? "N/A" : treatment().instructions())
+                .add("treatment", treatment() == null ? "N/A" : treatment().allInstructions())
                         //.add("created", created)
                 .add("flowEntryExtension", flowEntryExtension)
                 .toString();

@@ -15,15 +15,17 @@
  */
 package org.onosproject.net.group;
 
-import static com.google.common.base.MoreObjects.toStringHelper;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import java.util.Objects;
-
 import org.onosproject.core.GroupId;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.flow.instructions.Instruction;
+
+import java.util.List;
+import java.util.Objects;
+
+import static com.google.common.base.MoreObjects.toStringHelper;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * Group bucket implementation. A group bucket is collection of
@@ -206,9 +208,12 @@ public final class DefaultGroupBucket implements GroupBucket {
         }
         if (obj instanceof DefaultGroupBucket) {
             DefaultGroupBucket that = (DefaultGroupBucket) obj;
+            List<Instruction> myInstructions = this.treatment.allInstructions();
+            List<Instruction> theirInstructions = that.treatment.allInstructions();
+
             return Objects.equals(type, that.type) &&
-                   this.treatment.instructions().containsAll(that.treatment.instructions()) &&
-                   that.treatment.instructions().containsAll(this.treatment.instructions());
+                   myInstructions.containsAll(theirInstructions) &&
+                   theirInstructions.containsAll(myInstructions);
         }
         return false;
     }

@@ -75,14 +75,24 @@ public class DefaultTrafficTreatmentTest {
 
         final TrafficTreatment treatment1 = builder1.build();
 
-        final List<Instruction> instructions1 = treatment1.instructions();
+        final List<Instruction> instructions1 = treatment1.immediate();
         assertThat(instructions1, hasSize(9));
 
         builder1.drop();
         builder1.add(instruction1);
 
-        final List<Instruction> instructions2 = builder1.build().instructions();
+        final List<Instruction> instructions2 = builder1.build().immediate();
         assertThat(instructions2, hasSize(11));
+
+        builder1.deferred()
+                .popVlan()
+                .pushVlan()
+                .setVlanId(VlanId.vlanId((short) 5));
+
+        final List<Instruction> instructions3 = builder1.build().immediate();
+        assertThat(instructions3, hasSize(11));
+        final List<Instruction> instructions4 = builder1.build().deferred();
+        assertThat(instructions4, hasSize(3));
     }
 
     /**

@@ -93,18 +93,13 @@ public class IntentTestsMocks {
      */
     public static class MockTreatment implements TrafficTreatment {
         @Override
-        public List<Instruction> instructions() {
-            return new ArrayList<>();
-        }
-
-        @Override
         public List<Instruction> deferred() {
             return null;
         }
 
         @Override
         public List<Instruction> immediate() {
-            return null;
+            return new ArrayList<>();
         }
 
         @Override

@@ -36,12 +36,17 @@ public final class TrafficTreatmentCodec extends JsonCodec<TrafficTreatment> {
         final ObjectNode result = context.mapper().createObjectNode();
         final ArrayNode jsonInstructions = result.putArray("instructions");
 
-        if (treatment.instructions() != null) {
-            final JsonCodec<Instruction> instructionCodec =
-                    context.codec(Instruction.class);
-            for (final Instruction instruction : treatment.instructions()) {
-                jsonInstructions.add(instructionCodec.encode(instruction, context));
-            }
+        final JsonCodec<Instruction> instructionCodec =
+                context.codec(Instruction.class);
+
+        for (final Instruction instruction : treatment.immediate()) {
+            jsonInstructions.add(instructionCodec.encode(instruction, context));
+        }
+
+        final ArrayNode jsonDeferred = result.putArray("deferred");
+
+        for (final Instruction instruction : treatment.deferred()) {
+            jsonDeferred.add(instructionCodec.encode(instruction, context));
         }
 
         return result;

@@ -370,7 +370,7 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode>
         // check treatment
         final JsonNode jsonTreatment = jsonIntent.get("treatment");
         final TrafficTreatment treatment = connectivityIntent.treatment();
-        final List<Instruction> instructions = treatment.instructions();
+        final List<Instruction> instructions = treatment.immediate();
         final JsonNode jsonInstructions = jsonTreatment.get("instructions");
         if (jsonInstructions.size() != instructions.size()) {
             description.appendText("size of instructions array is "

@@ -557,11 +557,6 @@ public class FlowRuleManagerTest {
         }
 
         @Override
-        public List<Instruction> instructions() {
-            return null;
-        }
-
-        @Override
         public List<Instruction> deferred() {
             return null;
         }

@@ -156,8 +156,8 @@ public class HostMonitorTest {
         OutboundPacket packet = packetService.packets.get(0);
 
         // Check the output port is correct
-        assertEquals(1, packet.treatment().instructions().size());
-        Instruction instruction = packet.treatment().instructions().get(0);
+        assertEquals(1, packet.treatment().immediate().size());
+        Instruction instruction = packet.treatment().immediate().get(0);
         assertTrue(instruction instanceof OutputInstruction);
         OutputInstruction oi = (OutputInstruction) instruction;
         assertEquals(portNum, oi.port());
@@ -225,8 +225,8 @@ public class HostMonitorTest {
         OutboundPacket packet = packetService.packets.get(0);
 
         // Check the output port is correct
-        assertEquals(1, packet.treatment().instructions().size());
-        Instruction instruction = packet.treatment().instructions().get(0);
+        assertEquals(1, packet.treatment().immediate().size());
+        Instruction instruction = packet.treatment().immediate().get(0);
         assertTrue(instruction instanceof OutputInstruction);
         OutputInstruction oi = (OutputInstruction) instruction;
         assertEquals(portNum, oi.port());

@@ -503,9 +503,9 @@ public class ProxyArpManagerTest {
     private void verifyPacketOut(Ethernet expected, ConnectPoint outPort,
             OutboundPacket actual) {
         assertArrayEquals(expected.serialize(), actual.data().array());
-        assertEquals(1, actual.treatment().instructions().size());
+        assertEquals(1, actual.treatment().immediate().size());
         assertEquals(outPort.deviceId(), actual.sendThrough());
-        Instruction instruction = actual.treatment().instructions().get(0);
+        Instruction instruction = actual.treatment().immediate().get(0);
         assertTrue(instruction instanceof OutputInstruction);
         assertEquals(outPort.port(), ((OutputInstruction) instruction).port());
     }

@@ -155,15 +155,7 @@ public class SimpleStatisticStore implements StatisticStore {
     private ConnectPoint buildConnectPoint(FlowRule rule) {
         PortNumber port = getOutput(rule);
 
-        boolean hasGoto = rule.treatment().instructions()
-                .stream()
-                .anyMatch(i -> (i instanceof Instructions.GroupInstruction)
-                        || (i instanceof Instructions.TableTypeTransition));
-
         if (port == null) {
-            if (!hasGoto) {
-                log.debug("Rule {} has no output.", rule);
-            }
             return null;
         }
         ConnectPoint cp = new ConnectPoint(rule.deviceId(), port);
@@ -171,7 +163,7 @@ public class SimpleStatisticStore implements StatisticStore {
     }
 
     private PortNumber getOutput(FlowRule rule) {
-        for (Instruction i : rule.treatment().instructions()) {
+        for (Instruction i : rule.treatment().immediate()) {
             if (i.type() == Instruction.Type.OUTPUT) {
                 Instructions.OutputInstruction out = (Instructions.OutputInstruction) i;
                 return out.port();

@@ -138,7 +138,7 @@ public class FlowModBuilderVer10 extends FlowModBuilder {
         if (treatment == null) {
             return acts;
         }
-        for (Instruction i : treatment.instructions()) {
+        for (Instruction i : treatment.immediate()) {
             switch (i.type()) {
             case DROP:
                 log.warn("Saw drop action; assigning drop action");

@@ -186,24 +186,6 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
         return fm;
     }
 
-
-    private List<OFInstruction> buildInstructions() {
-        List<OFInstruction> instructions = new LinkedList<>();
-        if (treatment == null) {
-            return instructions;
-        }
-        for (Instruction i : treatment.instructions()) {
-            switch (i.type()) {
-                case TABLE:
-                    instructions.add(buildTableGoto(((Instructions.TableTypeTransition) i)));
-                    break;
-                default:
-                    break;
-            }
-        }
-        return instructions;
-    }
-
     private List<OFAction> buildActions(List<Instruction> treatments) {
         List<OFAction> actions = new LinkedList<>();
         boolean tableFound = false;

@@ -184,7 +184,7 @@ public final class GroupModBuilder {
             return actions;
         }
 
-        for (Instruction i : treatment.instructions()) {
+        for (Instruction i : treatment.allInstructions()) {
             switch (i.type()) {
                 case DROP:
                     log.warn("Saw drop action; assigning drop action");

@@ -55,7 +55,7 @@ public class OpenFlowCorePacketContext extends DefaultPacketContext {
     }
 
     private void sendPacket(Ethernet eth) {
-        List<Instruction> ins = treatmentBuilder().build().instructions();
+        List<Instruction> ins = treatmentBuilder().build().allInstructions();
         OFPort p = null;
         //TODO: support arbitrary list of treatments must be supported in ofPacketContext
         for (Instruction i : ins) {

@@ -113,7 +113,7 @@ public class OpenFlowPacketProvider extends AbstractProvider implements PacketPr
         //Ethernet eth = new Ethernet();
         //eth.deserialize(packet.data().array(), 0, packet.data().array().length);
         OFPortDesc p = null;
-        for (Instruction inst : packet.treatment().instructions()) {
+        for (Instruction inst : packet.treatment().allInstructions()) {
             if (inst.type().equals(Instruction.Type.OUTPUT)) {
                 p = portDesc(((OutputInstruction) inst).port());
                 OFPacketOut po = packetOut(sw, packet.data().array(), p.getPortNo());

@@ -298,12 +298,12 @@ public class FlowsResourceTest extends ResourceTest {
             if (flow.treatment() != null) {
                 final JsonObject jsonTreatment = jsonFlow.get("treatment").asObject();
                 final JsonArray jsonInstructions = jsonTreatment.get("instructions").asArray();
-                if (flow.treatment().instructions().size() != jsonInstructions.size()) {
+                if (flow.treatment().immediate().size() != jsonInstructions.size()) {
                     reason = "instructions array size of " +
-                            Integer.toString(flow.treatment().instructions().size());
+                            Integer.toString(flow.treatment().immediate().size());
                     return false;
                 }
-                for (final Instruction instruction : flow.treatment().instructions()) {
+                for (final Instruction instruction : flow.treatment().immediate()) {
                     boolean instructionFound = false;
                     for (int instructionIndex = 0; instructionIndex < jsonInstructions.size(); instructionIndex++) {
                         final String jsonType =

@@ -534,7 +534,7 @@ public abstract class TopologyViewMessageHandlerBase extends UiMessageHandler {
         PortNumber out = link.src().port();
         for (FlowEntry entry : entries) {
             TrafficTreatment treatment = entry.treatment();
-            for (Instruction instruction : treatment.instructions()) {
+            for (Instruction instruction : treatment.allInstructions()) {
                 if (instruction.type() == Instruction.Type.OUTPUT &&
                         ((OutputInstruction) instruction).port().equals(out)) {
                     count++;

@@ -528,7 +528,7 @@ public abstract class TopologyViewMessages {
         PortNumber out = link.src().port();
         for (FlowEntry entry : entries) {
             TrafficTreatment treatment = entry.treatment();
-            for (Instruction instruction : treatment.instructions()) {
+            for (Instruction instruction : treatment.allInstructions()) {
                 if (instruction.type() == Instruction.Type.OUTPUT &&
                         ((OutputInstruction) instruction).port().equals(out)) {
                     count++;