tests for flow service

Change-Id: I7138c7642f266c0691ead13ff1f230d063eb6b71
diff --git a/core/api/src/main/java/org/onlab/onos/cluster/MastershipListener.java b/core/api/src/main/java/org/onlab/onos/cluster/MastershipListener.java
index 22daed3..8a49c31 100644
--- a/core/api/src/main/java/org/onlab/onos/cluster/MastershipListener.java
+++ b/core/api/src/main/java/org/onlab/onos/cluster/MastershipListener.java
@@ -5,6 +5,6 @@
 /**
  * Entity capable of receiving device mastership-related events.
  */
-public interface MastershipListener extends EventListener<MastershipEvent>{
+public interface MastershipListener extends EventListener<MastershipEvent> {
 
 }
diff --git a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
index de59896..d3745b4 100644
--- a/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
+++ b/core/api/src/main/java/org/onlab/onos/net/flow/DefaultFlowRule.java
@@ -168,6 +168,7 @@
                 .add("selector", selector)
                 .add("treatment", treatment)
                 .add("created", created)
+                .add("state", state)
                 .toString();
     }
 
diff --git a/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java b/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
index 1cee01a..51ea328 100644
--- a/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/flow/impl/FlowRuleManager.java
@@ -84,8 +84,9 @@
 
     @Override
     public void removeFlowRules(FlowRule... flowRules) {
+        FlowRule f;
         for (int i = 0; i < flowRules.length; i++) {
-            FlowRule f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
+            f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
             final Device device = deviceService.getDevice(f.deviceId());
             final FlowRuleProvider frp = getProvider(device.providerId());
             store.deleteFlowRule(f);
@@ -134,7 +135,7 @@
         public void flowMissing(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
-            log.info("Flow {} has not been installed.");
+            log.info("Flow {} has not been installed.", flowRule);
 
         }
 
@@ -142,7 +143,7 @@
         public void extraneousFlow(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
-            log.info("Flow {} is on switch but not in store.");
+            log.info("Flow {} is on switch but not in store.", flowRule);
         }
 
         @Override
@@ -170,8 +171,8 @@
         @Override
         public void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowEntries) {
             List<FlowRule> storedRules = Lists.newLinkedList(store.getFlowEntries(deviceId));
-            //List<FlowRule> switchRules = Lists.newLinkedList(flowEntries);
-            Iterator<FlowRule> switchRulesIterator = flowEntries.iterator(); //switchRules.iterator();
+
+            Iterator<FlowRule> switchRulesIterator = flowEntries.iterator();
 
             while (switchRulesIterator.hasNext()) {
                 FlowRule rule = switchRulesIterator.next();
diff --git a/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
index 564bea2..8d82320 100644
--- a/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/flow/impl/FlowRuleManagerTest.java
@@ -26,6 +26,7 @@
 import org.onlab.onos.net.device.DeviceService;
 import org.onlab.onos.net.flow.DefaultFlowRule;
 import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRule.FlowRuleState;
 import org.onlab.onos.net.flow.FlowRuleEvent;
 import org.onlab.onos.net.flow.FlowRuleListener;
 import org.onlab.onos.net.flow.FlowRuleProvider;
@@ -57,7 +58,7 @@
 
     protected FlowRuleService service;
     protected FlowRuleProviderRegistry registry;
-    protected FlowRuleProviderService providerSerivce;
+    protected FlowRuleProviderService providerService;
     protected TestProvider provider;
     protected TestListener listener = new TestListener();
 
@@ -73,7 +74,7 @@
         mgr.activate();
         mgr.addListener(listener);
         provider = new TestProvider(PID);
-        providerSerivce = registry.register(provider);
+        providerService = registry.register(provider);
         assertTrue("provider should be registered",
                 registry.getProviders().contains(provider.id()));
     }
@@ -95,10 +96,15 @@
         return new DefaultFlowRule(DID, ts, tr, 0);
     }
 
-    private void addFlowRule(int hval) {
+    private FlowRule flowRule(FlowRule rule, FlowRuleState state) {
+        return new DefaultFlowRule(rule, state);
+    }
+
+    private FlowRule addFlowRule(int hval) {
         FlowRule rule = flowRule(hval, hval);
-        providerSerivce.flowAdded(rule);
+        providerService.flowAdded(rule);
         assertNotNull("rule should be found", service.getFlowEntries(DID));
+        return rule;
     }
 
     private void validateEvents(FlowRuleEvent.Type ... events) {
@@ -135,40 +141,53 @@
         validateEvents(RULE_UPDATED);
     }
 
+
+    //backing store is sensitive to the order of additions/removals
+    private boolean validateState(FlowRuleState... state) {
+        Iterable<FlowRule> rules = service.getFlowEntries(DID);
+        int i = 0;
+        for (FlowRule f : rules) {
+            if (f.state() != state[i]) {
+                return false;
+            }
+            i++;
+        }
+        return true;
+    }
+
     @Test
     public void applyFlowRules() {
-        TestSelector ts = new TestSelector(1);
+
         FlowRule r1 = flowRule(1, 1);
         FlowRule r2 = flowRule(1, 2);
         FlowRule r3 = flowRule(1, 3);
 
-        //current FlowRules always return 0. FlowEntries inherit the value
-        FlowRule e1 = new DefaultFlowRule(DID, ts, r1.treatment(), 0);
-        FlowRule e2 = new DefaultFlowRule(DID, ts, r2.treatment(), 0);
-        FlowRule e3 = new DefaultFlowRule(DID, ts, r3.treatment(), 0);
-        List<FlowRule> fel = Lists.newArrayList(e1, e2, e3);
-
         assertTrue("store should be empty",
                 Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
         mgr.applyFlowRules(r1, r2, r3);
         assertEquals("3 rules should exist", 3, flowCount());
-        assertTrue("3 entries should result", fel.containsAll(Lists.newArrayList(r1, r2, r3)));
+        assertTrue("Entries should be pending add.",
+                validateState(FlowRuleState.PENDING_ADD, FlowRuleState.PENDING_ADD,
+                        FlowRuleState.PENDING_ADD));
     }
 
     @Test
     public void removeFlowRules() {
-        addFlowRule(1);
-        addFlowRule(2);
+        FlowRule f1 = addFlowRule(1);
+        FlowRule f2 = addFlowRule(2);
         addFlowRule(3);
         assertEquals("3 rules should exist", 3, flowCount());
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_ADDED);
 
-        FlowRule rem1 = flowRule(1, 1);
-        FlowRule rem2 = flowRule(2, 2);
+        FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+        FlowRule rem2 = flowRule(f2, FlowRuleState.REMOVED);
         mgr.removeFlowRules(rem1, rem2);
         //removing from north, so no events generated
         validateEvents();
         assertEquals("3 rule should exist", 3, flowCount());
+        assertTrue("Entries should be pending remove.",
+                validateState(FlowRuleState.CREATED, FlowRuleState.PENDING_REMOVE,
+                        FlowRuleState.PENDING_REMOVE));
 
         mgr.removeFlowRules(rem1);
         assertEquals("3 rule should still exist", 3, flowCount());
@@ -176,16 +195,34 @@
 
     @Test
     public void flowRemoved() {
-        addFlowRule(1);
+        FlowRule f1 = addFlowRule(1);
         addFlowRule(2);
-        FlowRule rem1 = flowRule(1, 1);
-        providerSerivce.flowRemoved(rem1);
+        FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+        providerService.flowRemoved(rem1);
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
 
-        providerSerivce.flowRemoved(rem1);
+        providerService.flowRemoved(rem1);
         validateEvents();
     }
 
+    @Test
+    public void flowMetrics() {
+        FlowRule f1 = flowRule(1, 1);
+        FlowRule f2 = flowRule(2, 2);
+        FlowRule f3 = flowRule(3, 3);
+
+        FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
+        FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
+        mgr.applyFlowRules(f1, f2, f3);
+
+        providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
+
+        assertTrue("Entries should be added.",
+                validateState(FlowRuleState.PENDING_ADD, FlowRuleState.ADDED,
+                        FlowRuleState.ADDED));
+        //TODO: add tests for flowmissing and extraneous flows
+    }
+
     private static class TestListener implements FlowRuleListener {
         final List<FlowRuleEvent> events = new ArrayList<>();
 
diff --git a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
index 2f0de26..fbfd0ee 100644
--- a/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
+++ b/core/trivial/src/main/java/org/onlab/onos/net/trivial/impl/SimpleFlowRuleStore.java
@@ -60,6 +60,7 @@
          *  find the rule and mark it for deletion.
          *  Ultimately a flow removed will come remove it.
          */
+
         if (flowEntries.containsEntry(did, rule)) {
             synchronized (flowEntries) {