From c2269774aae61675344c797b5cd3b168c6c7c3f0 Mon Sep 17 00:00:00 2001 From: Colin Dixon Date: Sun, 29 Jun 2014 15:19:44 -0500 Subject: [PATCH] Partial fix for Bug 1256 Patch 0e12a849d33ae3ca3d1a8e2e281e6eac2cc13373 introduced a test for data normalization that contained comparators for two kinds of nodes (LegacyNodeData) and (Node) that provided only a partial ordering of nodes and also failed to obey the semantics of comparators when it comes to equality. The result is that several tests were not fully specified and while the appear to run correctly on Windows and Linux, on recent MacOS versions the ambiguity caused the tests to fail. This patch fixes the problem as long as only SimpleNodes and CompositeNodes are used. Change-Id: If5175025fb97e7b8ddb799b27ad7ae9b5026172d Signed-off-by: Colin Dixon --- .../impl/util/compat/DataNormalizerTest.java | 100 ++++++++++++++---- 1 file changed, 82 insertions(+), 18 deletions(-) diff --git a/opendaylight/md-sal/sal-common-impl/src/test/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizerTest.java b/opendaylight/md-sal/sal-common-impl/src/test/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizerTest.java index ddf089c68f..ce861f7e7a 100644 --- a/opendaylight/md-sal/sal-common-impl/src/test/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizerTest.java +++ b/opendaylight/md-sal/sal-common-impl/src/test/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizerTest.java @@ -18,6 +18,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import java.util.AbstractMap; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; @@ -390,15 +391,44 @@ public class DataNormalizerTest { Collections.sort(unorderdChildData, new Comparator() { @Override public int compare(LegacyNodeData arg1, LegacyNodeData arg2) { - String str1 = arg1.nodeKey.getLocalName(); - if (!(arg1.nodeData instanceof List)) - str1 += arg1.nodeData; // add simple node value - - String str2 = arg2.nodeKey.getLocalName(); - if (!(arg2.nodeData instanceof List)) - str2 += arg2.nodeData; // add simple node value - - return str1.compareTo(str2); + if (!(arg1.nodeData instanceof List) && !(arg2.nodeData instanceof List)) { + // if neither is a list, just compare them + String str1 = arg1.nodeKey.getLocalName() + arg1.nodeData; + String str2 = arg2.nodeKey.getLocalName() + arg2.nodeData; + return str1.compareTo(str2); + } else if (arg1.nodeData instanceof List && arg2.nodeData instanceof List) { + // if both are lists, first check their local name + String str1 = arg1.nodeKey.getLocalName(); + String str2 = arg2.nodeKey.getLocalName(); + if (!str1.equals(str2)) { + return str1.compareTo(str2); + } else { + // if local names are the same, then look at the list contents + List l1 = (List) arg1.nodeData; + List l2 = (List) arg2.nodeData; + + if (l1.size() != l2.size()) { + // if the sizes are different, use that + return l2.size() - l1.size(); + } else { + // lastly sort and recursively check the list contents + Collections.sort(l1, this); + Collections.sort(l2, this); + + for (int i = 0 ; i < l1.size() ; i++) { + int diff = this.compare(l1.get(i), l2.get(i)); + if (diff != 0) { + return diff; + } + } + return 0; + } + } + } else if( arg1.nodeData instanceof List ) { + return -1; + } else{ + return 1; + } } }); @@ -417,15 +447,49 @@ public class DataNormalizerTest { Collections.sort(unorderedChildNodes, new Comparator>() { @Override public int compare(Node n1, Node n2) { - String str1 = n1.getKey().getLocalName(); - if (n1 instanceof SimpleNode) - str1 += ((SimpleNode) n1).getValue(); - - String str2 = n2.getKey().getLocalName(); - if (n2 instanceof SimpleNode) - str2 += ((SimpleNode) n2).getValue(); - - return str1.compareTo(str2); + if (n1 instanceof SimpleNode && n2 instanceof SimpleNode) { + // if they're SimpleNodes just compare their strings + String str1 = n1.getKey().getLocalName() + ((SimpleNode)n1).getValue(); + String str2 = n2.getKey().getLocalName() + ((SimpleNode)n2).getValue(); + return str1.compareTo(str2); + } else if (n1 instanceof CompositeNode && n2 instanceof CompositeNode) { + // if they're CompositeNodes, things are more interesting + String str1 = n1.getKey().getLocalName(); + String str2 = n2.getKey().getLocalName(); + if (!str1.equals(str2)) { + // if their local names differ, return that difference + return str1.compareTo(str2); + } else { + // otherwise, we need to look at their contents + ArrayList> l1 = new ArrayList>( ((CompositeNode)n1).getValue() ); + ArrayList> l2 = new ArrayList>( ((CompositeNode)n2).getValue() ); + + if (l1.size() != l2.size()) { + // if they have different numbers of things in them return that + return l2.size() - l1.size(); + } else { + // otherwise, compare the individual elements, first sort them + Collections.sort(l1, this); + Collections.sort(l2, this); + + // then compare them individually + for(int i = 0 ; i < l2.size() ; i++) { + int diff = this.compare(l1.get(i), l2.get(i)); + if(diff != 0){ + return diff; + } + } + return 0; + } + } + } else if (n1 instanceof CompositeNode && n2 instanceof SimpleNode) { + return -1; + } else if (n2 instanceof CompositeNode && n1 instanceof SimpleNode) { + return 1; + } else { + assertTrue("Expected either SimpleNodes CompositeNodes", false); + return 0; + } } }); -- 2.36.6