commit 65d551b50e782b0c1ea76c1a9ed1c5a801a5a7e4
author Jonathan Hart <jono@onlab.us> Tue Sep 22 09:49:09 2015 -0700
committer Gerrit Code Review <gerrit@onlab.us> Tue Sep 22 18:21:05 2015 +0000
tree 2a283e2a289d8377eeb68a319331cb81b34b67b1
parent d0e932a328aaefbe4d78b9a4e9cb0d8d05002789

Compute the flow ID at build time rather than when fromApp() is called.

Depending on the order that the application calls the builder methods,
there may not be any selector, priority, etc. added at the time fromApp() is
called. This means there's no entropy to the hash function so all flows end
up with the same flow ID.

Change-Id: I9badedc343fdf6f3ee7f73547955cd84302d9226

core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java

diff --git a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
index 44a4d36..5492fa9 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
@@ -291,6 +291,7 @@
     public static final class Builder implements FlowRule.Builder {
 
         private FlowId flowId;
+        private ApplicationId appId;
         private Integer priority;
         private DeviceId deviceId;
         private Integer tableId = 0;
@@ -307,7 +308,7 @@
 
         @Override
         public FlowRule.Builder fromApp(ApplicationId appId) {
-            this.flowId = computeFlowId(appId);
+            this.appId = appId;
             return this;
         }
 
@@ -357,16 +358,22 @@
 
         @Override
         public FlowRule build() {
-            checkNotNull(flowId != null, "Either an application" +
+            checkArgument(flowId != null || appId != null, "Either an application" +
                     " id or a cookie must be supplied");
-            checkNotNull(selector != null, "Traffic selector cannot be null");
-            checkNotNull(timeout != null || permanent != null, "Must either have " +
+            checkNotNull(selector, "Traffic selector cannot be null");
+            checkArgument(timeout != null || permanent != null, "Must either have " +
                     "a timeout or be permanent");
-            checkNotNull(deviceId != null, "Must refer to a device");
-            checkNotNull(priority != null, "Priority cannot be null");
+            checkNotNull(deviceId, "Must refer to a device");
+            checkNotNull(priority, "Priority cannot be null");
             checkArgument(priority >= MIN_PRIORITY, "Priority cannot be less than " +
                     MIN_PRIORITY);
 
+            // Computing a flow ID based on appId takes precedence over setting
+            // the flow ID directly
+            if (appId != null) {
+                flowId = computeFlowId(appId);
+            }
+
             return new DefaultFlowRule(deviceId, selector, treatment, priority,
                                        flowId, permanent, timeout, tableId);
         }