give each ElementSelector a chance to see all nodes first

closes #197
xmlunit · Sep 27, 2020 · f48146a · f48146a
1 parent fe167c7
commit f48146a
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 18 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -21,6 +21,11 @@
   all modules (it has already been Java 7 for the AssertJ module
   before).
 
+* DefaultNodeMatcher with multiple ElementSelectors could fail to find
+  the best matches as the order of ElementSelectors should select
+  them.
+  [#197](https://github.com/xmlunit/xmlunit/issues/197)
+
 ## XMLUnit for Java 2.7.0 - /Released 2020-05-12/
 
 This version contains a backwards incompatible change to the

diff --git a/xmlunit-core/src/main/java/org/xmlunit/diff/DefaultNodeMatcher.java b/xmlunit-core/src/main/java/org/xmlunit/diff/DefaultNodeMatcher.java
@@ -98,47 +98,45 @@ public Iterable<Map.Entry<Node, Node>> match(Iterable<Node> controlNodes,
             unmatchedTestIndexes.add(Integer.valueOf(i));
         }
         final int controlSize = controlList.size();
+        Set<Integer> unmatchedControlIndexes = new HashSet<Integer>();
+        for (int i = 0; i < controlSize; i++) {
+            unmatchedControlIndexes.add(Integer.valueOf(i));
+        }
+
+        for (ElementSelector e : elementSelectors) {
         Match lastMatch = new Match(null, -1);
         for (int i = 0; i < controlSize; i++) {
+                if (!unmatchedControlIndexes.contains(Integer.valueOf(i))) {
+                    continue;
+                }
             Node control = controlList.get(i);
             Match testMatch = findMatchingNode(control, testList,
                                                lastMatch.index,
-                                               unmatchedTestIndexes);
+                                               unmatchedTestIndexes, e);
             if (testMatch != null) {
+                unmatchedControlIndexes.remove(Integer.valueOf(i));
                 unmatchedTestIndexes.remove(testMatch.index);
                 matches.put(control, testMatch.node);
             }
         }
+        }
         return matches.entrySet();
     }
 
     private Match findMatchingNode(final Node searchFor,
                                    final List<Node> searchIn,
                                    final int indexOfLastMatch,
-                                   final Set<Integer> availableIndexes) {
+                                   final Set<Integer> availableIndexes,
+                                   final ElementSelector e) {
         final int searchSize = searchIn.size();
         Match m = searchIn(searchFor, searchIn,
                            availableIndexes,
-                           indexOfLastMatch + 1, searchSize);
+                           indexOfLastMatch + 1, searchSize, e);
         return m != null ? m : searchIn(searchFor, searchIn,
                                         availableIndexes,
-                                        0, indexOfLastMatch);
+                                        0, indexOfLastMatch, e);
     }
 
-    private Match searchIn(final Node searchFor,
-                           final List<Node> searchIn,
-                           final Set<Integer> availableIndexes,
-                           final int fromInclusive, final int toExclusive) {
-        for (ElementSelector e : elementSelectors) {
-            Match m = searchIn(searchFor, searchIn, availableIndexes, fromInclusive, toExclusive, e);
-            if (m != null) {
-                return m;
-            }
-        }
-        return null;
-    }
-
-
     private Match searchIn(final Node searchFor,
                            final List<Node> searchIn,
                            final Set<Integer> availableIndexes,

diff --git a/xmlunit-core/src/test/java/org/xmlunit/diff/DefaultNodeMatcherTest.java b/xmlunit-core/src/test/java/org/xmlunit/diff/DefaultNodeMatcherTest.java
@@ -95,4 +95,33 @@ public void elementSelectorsAreQueriedInSequenceWithConditionalSelector() {
         assertSame(control2, result.get(1).getKey());
         assertSame(test1, result.get(1).getValue());
     }
+
+    @Test
+    public void elementSelectorsAreQueriedInSequenceWithControlNodesSwapped() {
+        Element control1 = doc.createElement("a");
+        control1.appendChild(doc.createTextNode("bar"));
+        Element control2 = doc.createElement("a");
+        control2.appendChild(doc.createTextNode("foo"));
+
+        Element test1 = doc.createElement("a");
+        test1.appendChild(doc.createTextNode("foo"));
+        Element test2 = doc.createElement("a");
+        test2.appendChild(doc.createTextNode("baz"));
+
+        DefaultNodeMatcher m =
+            new DefaultNodeMatcher(ElementSelectors.byNameAndText, ElementSelectors.byName);
+        List<Map.Entry<Node, Node>> result =
+            Linqy.asList(m.match(Arrays.<Node>asList(control1, control2),
+                Arrays.<Node>asList(test1, test2)));
+        assertEquals(2, result.size());
+
+        // byNameAndText
+        assertSame(control2, result.get(0).getKey());
+        assertSame(test1, result.get(0).getValue());
+
+        // byName
+        assertSame(control1, result.get(1).getKey());
+        assertSame(test2, result.get(1).getValue());
+    }
+
 }