Suppress unnecessary PathIntent operations when rerouting

Change-Id: If06fc6937d5a69f4f5ff829dbff5103b0fa1736d
diff --git a/src/main/java/net/onrc/onos/core/intent/PathIntent.java b/src/main/java/net/onrc/onos/core/intent/PathIntent.java
index 4312075..3a668bb 100644
--- a/src/main/java/net/onrc/onos/core/intent/PathIntent.java
+++ b/src/main/java/net/onrc/onos/core/intent/PathIntent.java
@@ -1,5 +1,6 @@
 package net.onrc.onos.core.intent;
 
+import java.util.Objects;
 
 /**
  * @author Toshio Koide (t-koide@onlab.us)
@@ -50,6 +51,26 @@
         return parentIntent;
     }
 
+    /**
+     * Checks the specified PathIntent have the same fields of
+     * path, bandwidth and parentIntent's id with this PathIntent.
+     *
+     * @param target target PathIntent instance
+     * @return true if the specified intent has the same fields, otherwise false
+     */
+    public boolean hasSameFields(PathIntent target) {
+        if (target == null) {
+            return false;
+        }
+        if (!Objects.equals(getPath(), target.getPath())) {
+            return false;
+        }
+        if (getBandwidth() != target.getBandwidth()) {
+            return false;
+        }
+        return Objects.equals(getParentIntent(), target.getParentIntent());
+    }
+
     @Override
     public int hashCode() {
         // TODO: Is this the intended behavior?
diff --git a/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntime.java b/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntime.java
index c900698..837e210 100644
--- a/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntime.java
+++ b/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntime.java
@@ -105,16 +105,32 @@
                         newPathIntentId = PathIntent.createFirstId(spIntent.getId());
                     } else {
                         newPathIntentId = PathIntent.createNextId(oldPathIntentId);
-
-                        // Request removal of low-level intent if it exists.
-                        pathIntentOpList.add(Operator.REMOVE, new Intent(oldPathIntentId));
                     }
 
                     // create new path-intent
-                    PathIntent pathIntent = new PathIntent(newPathIntentId, path, bandwidth, spIntent);
-                    pathIntent.setState(IntentState.INST_REQ);
-                    spIntent.setPathIntent(pathIntent);
-                    pathIntentOpList.add(Operator.ADD, pathIntent);
+                    PathIntent newPathIntent = new PathIntent(newPathIntentId, path, bandwidth, spIntent);
+                    newPathIntent.setState(IntentState.INST_REQ);
+
+                    // create and add operation(s)
+                    if (oldPathIntentId == null) {
+                        // operation for new path-intent
+                        spIntent.setPathIntent(newPathIntent);
+                        pathIntentOpList.add(Operator.ADD, newPathIntent);
+                        log.debug("new intent:{}", newPathIntent);
+                    } else {
+                        PathIntent oldPathIntent = (PathIntent) pathIntents.getIntent(oldPathIntentId);
+                        if (newPathIntent.hasSameFields(oldPathIntent)) {
+                            // skip the same operation (reroute)
+                            spIntent.setState(IntentState.INST_ACK);
+                            log.debug("skip intent:{}", newPathIntent);
+                        } else {
+                            // update existing path-intent (reroute)
+                            spIntent.setPathIntent(newPathIntent);
+                            pathIntentOpList.add(Operator.REMOVE, oldPathIntent);
+                            pathIntentOpList.add(Operator.ADD, newPathIntent);
+                            log.debug("update intent:{} -> {}", oldPathIntent, newPathIntent);
+                        }
+                    }
 
                     break;
                 case REMOVE:
diff --git a/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModule.java b/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModule.java
index 74f89f4..5dc232e 100644
--- a/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModule.java
+++ b/src/main/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModule.java
@@ -238,16 +238,6 @@
             // update the map of path intents and publish the path operations
             pathIntents.executeOperations(pathIntentOperations);
 
-            // XXX Demo special: add a complete path to remove operation
-            for (IntentOperation op : pathIntentOperations) {
-                if (op.operator.equals(Operator.REMOVE)) {
-                    op.intent = pathIntents.getIntent(op.intent.getId());
-                }
-                if (op.intent instanceof PathIntent) {
-                    log.debug("operation: {}, intent:{}", op.operator, op.intent);
-                }
-            }
-
             // send notification
             // XXX: Send notifications using the same key every time
             // and receive them by entryAdded() and entryUpdated()
diff --git a/src/test/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModuleTest.java b/src/test/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModuleTest.java
index c9e2115..8b374ee 100644
--- a/src/test/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModuleTest.java
+++ b/src/test/java/net/onrc/onos/core/intent/runtime/PathCalcRuntimeModuleTest.java
@@ -449,7 +449,7 @@
         final IntentOperationList pathIntentOpListAfterReroute =
                 runtime.executeIntentOperations(opListForReroute);
         assertThat(pathIntentOpListAfterReroute, notNullValue());
-        assertThat(pathIntentOpListAfterReroute, hasSize(2));
+        assertThat(pathIntentOpListAfterReroute, hasSize(0));
 
         //  Check the high level intents.
         final IntentMap highLevelIntentsAfterReroute = runtime.getHighLevelIntents();
@@ -467,7 +467,7 @@
         //  Check the states of the high level intents
         //  Check that switch 1 was correctly processed
         assertThat(highLevelIntents,
-                hasIntentWithIdAndState("1", IntentState.REROUTE_REQ));
+                hasIntentWithIdAndState("1", IntentState.INST_ACK));
 
         //  Check that switch 2 was correctly processed
         assertThat(highLevelIntents,
diff --git a/src/test/java/net/onrc/onos/core/intent/runtime/UseCaseTest.java b/src/test/java/net/onrc/onos/core/intent/runtime/UseCaseTest.java
index 30e2845..bd42884 100644
--- a/src/test/java/net/onrc/onos/core/intent/runtime/UseCaseTest.java
+++ b/src/test/java/net/onrc/onos/core/intent/runtime/UseCaseTest.java
@@ -15,7 +15,6 @@
 import net.floodlightcontroller.core.module.FloodlightModuleContext;
 import net.floodlightcontroller.core.module.FloodlightModuleException;
 import net.floodlightcontroller.restserver.IRestApiService;
-
 import net.onrc.onos.core.datagrid.IDatagridService;
 import net.onrc.onos.core.datagrid.IEventChannel;
 import net.onrc.onos.core.datagrid.IEventChannelListener;
@@ -274,10 +273,51 @@
                 removedLinkEvents,
                 addedDeviceEvents,
                 removedDeviceEvents);
-        System.out.println("Link goes down.");
+        System.out.println("*** Link goes down. ***");
+
+        // send notification
+        IntentStateList isl = new IntentStateList();
+        isl.put("1___0", IntentState.DEL_ACK);
+        isl.put("1___1", IntentState.INST_ACK);
+        isl.domainSwitchDpids.add(1L);
+        isl.domainSwitchDpids.add(2L);
+        isl.domainSwitchDpids.add(4L);
+        runtime1.entryUpdated(isl);
 
         // show results step2
         showResult((PathIntentMap) runtime1.getPathIntents());
+
+        // link up
+        ((MockTopology) topology).addBidirectionalLinks(1L, 12L, 2L, 21L);
+        linkEvent1 = new LinkEvent(1L, 12L, 2L, 21L);
+        linkEvent2 = new LinkEvent(2L, 21L, 1L, 12L);
+        removedLinkEvents.clear();
+        addedLinkEvents.clear();
+        addedLinkEvents.add(linkEvent1);
+        addedLinkEvents.add(linkEvent2);
+        runtime1.topologyEvents(
+                addedSwitchEvents,
+                removedSwitchEvents,
+                addedPortEvents,
+                removedPortEvents,
+                addedLinkEvents,
+                removedLinkEvents,
+                addedDeviceEvents,
+                removedDeviceEvents);
+        System.out.println("*** Link goes up. ***");
+
+        // send notification
+        isl = new IntentStateList();
+        isl.put("1___1", IntentState.DEL_ACK);
+        isl.put("1___2", IntentState.INST_ACK);
+        isl.domainSwitchDpids.add(1L);
+        isl.domainSwitchDpids.add(2L);
+        isl.domainSwitchDpids.add(4L);
+        runtime1.entryUpdated(isl);
+
+        // show results step3
+        showResult((PathIntentMap) runtime1.getPathIntents());
+
         // TODO: show results of plan computation
     }