Merge "Fix ClassCastException in logging bridge stop"
authorGiovanni Meo <gmeo@cisco.com>
Wed, 22 Jan 2014 21:16:16 +0000 (21:16 +0000)
committerGerrit Code Review <gerrit@opendaylight.org>
Wed, 22 Jan 2014 21:16:16 +0000 (21:16 +0000)
opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java
opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java
opendaylight/topologymanager/implementation/src/main/java/org/opendaylight/controller/topologymanager/internal/TopologyManagerImpl.java

index 2c3cfb8303736229e6c86b618642f6bd72b12aaa..910695c1e9cbc0ebcfb121b621fb7187305aff37 100644 (file)
@@ -20,6 +20,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
@@ -444,13 +445,16 @@ public class Match implements Cloneable, Serializable {
         if (this.fields == null) {
             result = prime * result;
         } else {
-            int sum = 0;
-            for (MatchType field : this.fields.keySet()) {
-                MatchField f = this.fields.get(field);
-                sum = sum + ((field==null ? 0 : field.calculateConsistentHashCode()) ^
-                             (f==null ? 0 : f.hashCode()));
+            // use a tree map as the order of hashMap is not guaranteed.
+            // 2 Match objects with fields in different order are still equal.
+            // Hence the hashCode should be the same too.
+            TreeMap<MatchType, MatchField> tm = new TreeMap<MatchType, MatchField>(this.fields);
+            for (MatchType field : tm.keySet()) {
+                MatchField f = tm.get(field);
+                int fieldHashCode = (field==null ? 0 : field.calculateConsistentHashCode()) ^
+                             (f==null ? 0 : f.hashCode());
+                result = prime * result + fieldHashCode;
             }
-            result = prime * result + sum;
         }
         result = prime * result + matches;
         return result;
index 30d49cfd9dcc5b5abb90bafd659ac314e855f99a..b88ae034d9aa713c96f4ce3ef633f977d5a223b3 100644 (file)
@@ -337,6 +337,21 @@ public class MatchTest {
         Assert.assertTrue(match1.equals(match2));
     }
 
+    @Test
+    public void testHashCodeWithReverseMatch() throws Exception {
+        InetAddress srcIP1 = InetAddress.getByName("1.1.1.1");
+        InetAddress ipMask1 = InetAddress.getByName("255.255.255.255");
+        InetAddress srcIP2 = InetAddress.getByName("2.2.2.2");
+        InetAddress ipMask2 = InetAddress.getByName("255.255.255.255");
+        MatchField field1 = new MatchField(MatchType.NW_SRC, srcIP1, ipMask1);
+        MatchField field2 = new MatchField(MatchType.NW_DST, srcIP2, ipMask2);
+        Match match1 = new Match();
+        match1.setField(field1);
+        match1.setField(field2);
+        Match match2 = match1.reverse();
+        Assert.assertFalse(match1.hashCode() == match2.hashCode());
+    }
+
     @Test
     public void testHashCode() throws Exception {
         byte srcMac1[] = { (byte) 0x12, (byte) 0x34, (byte) 0x56, (byte) 0x78, (byte) 0x9a, (byte) 0xbc };
index 45a8a5880b70982d83748a3e3d2392dd3e93de1f..b0570fb7f9b6eed29384ac4c0280202df1fd4a5f 100644 (file)
@@ -574,36 +574,38 @@ public class TopologyManagerImpl implements
     private TopoEdgeUpdate edgeUpdate(Edge e, UpdateType type, Set<Property> props) {
         switch (type) {
         case ADDED:
-            // Avoid redundant update as notifications trigger expensive tasks
-            if (edgesDB.containsKey(e)) {
-                log.trace("Skipping redundant edge addition: {}", e);
-                return null;
-            }
-
-            // Ensure that head node connector exists
-            if (!headNodeConnectorExist(e)) {
-                log.warn("Ignore edge that contains invalid node connector: {}", e);
-                return null;
-            }
-
-            // Check if nodeConnectors of the edge were correctly categorized
-            // by OF plugin
-            crossCheckNodeConnectors(e);
 
-            // Make sure the props are non-null
+            // Make sure the props are non-null or create a copy
             if (props == null) {
                 props = new HashSet<Property>();
             } else {
                 props = new HashSet<Property>(props);
             }
 
-            //in case of node switch-over to a different cluster controller,
-            //let's retain edge props
             Set<Property> currentProps = this.edgesDB.get(e);
-            if (currentProps != null){
+            if (currentProps != null) {
+
+                if (currentProps.equals(props)) {
+                    // Avoid redundant updates as notifications trigger expensive tasks
+                    log.trace("Skipping redundant edge addition: {}", e);
+                    return null;
+                }
+
+                // In case of node switch-over to a different cluster controller,
+                // let's retain edge props (e.g. creation time)
                 props.addAll(currentProps);
             }
 
+            // Ensure that head node connector exists
+            if (!headNodeConnectorExist(e)) {
+                log.warn("Ignore edge that contains invalid node connector: {}", e);
+                return null;
+            }
+
+            // Check if nodeConnectors of the edge were correctly categorized
+            // by protocol plugin
+            crossCheckNodeConnectors(e);
+
             // Now make sure there is the creation timestamp for the
             // edge, if not there, stamp with the first update
             boolean found_create = false;