[AETHER-514] Support MODIFY on SIMPLE next

Add additional debug prints and removes the generation
of the hashed and next_vlan flows

Change-Id: Iecee9e86dd8443ae0b67c5fad193311f929503a2
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
index eac2116..79045f1 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FabricPipeliner.java
@@ -50,6 +50,7 @@
 import java.util.stream.Collectors;
 
 import static java.lang.String.format;
+import static org.onosproject.net.flowobjective.NextObjective.Type.SIMPLE;
 import static org.onosproject.pipelines.fabric.impl.behaviour.FabricUtils.outputPort;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -128,9 +129,12 @@
             return;
         }
 
-        if (obj.op() == Objective.Operation.MODIFY) {
-            // TODO: support MODIFY operation
-            log.warn("MODIFY operation not yet supported for NextObjective, will return failure :(");
+        if (obj.op() == Objective.Operation.MODIFY && obj.type() != SIMPLE) {
+            log.warn("MODIFY operation not yet supported for NextObjective {}, will return failure :(",
+                    obj.type());
+            if (log.isTraceEnabled()) {
+                log.trace("Objective {}", obj);
+            }
             fail(obj, ObjectiveError.UNSUPPORTED);
             return;
         }
@@ -182,10 +186,16 @@
         if (flowRules.isEmpty()) {
             return;
         }
+
+        if (log.isTraceEnabled()) {
+            log.trace("Objective {} -> Flows {}", objective, flowRules);
+        }
+
         final FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
         switch (objective.op()) {
             case ADD:
             case ADD_TO_EXISTING:
+            case MODIFY:
                 flowRules.forEach(ops::add);
                 break;
             case REMOVE:
@@ -193,7 +203,7 @@
                 flowRules.forEach(ops::remove);
                 break;
             default:
-                log.warn("Unsupported Objective operation '{}'", objective.op());
+                log.warn("Unsupported Objective operation {}", objective.op());
                 return;
         }
         flowRuleService.apply(ops.build());
@@ -203,6 +213,11 @@
         if (groups.isEmpty()) {
             return;
         }
+
+        if (log.isTraceEnabled()) {
+            log.trace("Objective {} -> Groups {}", objective, groups);
+        }
+
         switch (objective.op()) {
             case ADD:
                 groups.forEach(groupService::addGroup);
@@ -223,6 +238,14 @@
                         group.appCookie(), group.appId())
                 );
                 break;
+            case MODIFY:
+                // Modify is only supported for simple next objective
+                // Replace group bucket directly
+                groups.forEach(group -> groupService.setBucketsForGroup(
+                        deviceId, group.appCookie(), group.buckets(),
+                        group.appCookie(), group.appId())
+                );
+                break;
             default:
                 log.warn("Unsupported Objective operation {}", objective.op());
         }
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
index 4e3304d..fb08d07 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/NextObjectiveTranslator.java
@@ -199,7 +199,7 @@
                 obj.nextTreatments(), true);
 
         if (forceSimple && treatments.size() > 1) {
-            log.warn("Forcing SIMPLE behavior for NextObjective with {} treatments []",
+            log.warn("Forcing SIMPLE behavior for NextObjective with {} treatments {}",
                      treatments.size(), obj);
         }
 
@@ -494,10 +494,12 @@
     }
 
     private boolean isGroupModifyOp(NextObjective obj) {
-        // If operation is ADD_TO_EXIST or REMOVE_FROM_EXIST, it means we modify
+        // If operation is ADD_TO_EXIST, REMOVE_FROM_EXIST or MODIFY, it means we modify
         // group buckets only, no changes for flow rules.
+        // FIXME Please note that for MODIFY op this could not apply in future if we extend the scope of MODIFY
         return obj.op() == Objective.Operation.ADD_TO_EXISTING ||
-                obj.op() == Objective.Operation.REMOVE_FROM_EXISTING;
+                obj.op() == Objective.Operation.REMOVE_FROM_EXISTING ||
+                obj.op() == Objective.Operation.MODIFY;
     }
 
     private boolean isXconnect(NextObjective obj) {