give each ElementSelector a chance to see all nodes first

xmlunit · Sep 27, 2020 · de08f4d · de08f4d
1 parent 7549fb2
commit de08f4d
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 26 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -9,6 +9,11 @@
   implementations of your own.
   [#33](https://github.com/xmlunit/xmlunit.net/pull/33).
 
+* `DefaultNodeMatcher` with multiple `ElementSelector`s could fail to
+  find the best matches as the order of `ElementSelector`s should
+  select them.
+  Issue similar to [xmlunit/#197](https://github.com/xmlunit/xmlunit/issues/197)
+
 ## XMLUnit.NET 2.8.0 - /Released 2020-05-12/
 
 This version contains a backwards incompatible change to the

diff --git a/src/main/net-core/Diff/DefaultNodeMatcher.cs b/src/main/net-core/Diff/DefaultNodeMatcher.cs
@@ -99,16 +99,25 @@ public IEnumerable<KeyValuePair<XmlNode, XmlNode>>
                 unmatchedTestIndexes.Add(i);
             }
             int controlSize = controlList.Count;
-            MatchInfo lastMatch = new MatchInfo(null, -1);
+            ICollection<int> unmatchedControlIndexes = new HashSet<int>();
             for (int i = 0; i < controlSize; i++) {
-                XmlNode control = controlList[i];
-                MatchInfo testMatch = FindMatchingNode(control, testList,
-                                                       lastMatch.Index,
-                                                       unmatchedTestIndexes);
-                if (testMatch != null) {
-                    unmatchedTestIndexes.Remove(testMatch.Index);
-                    matches.AddLast(new KeyValuePair<XmlNode,
-                                    XmlNode>(control, testMatch.Node));
+                unmatchedControlIndexes.Add(i);
+            }
+            foreach (ElementSelector e in elementSelectors) {
+                MatchInfo lastMatch = new MatchInfo(null, -1);
+                for (int i = 0; i < controlSize; i++) {
+                    if (!unmatchedControlIndexes.Contains(i)) {
+                        continue;
+                    }
+                    XmlNode control = controlList[i];
+                    MatchInfo testMatch = FindMatchingNode(control, testList,
+                        lastMatch.Index, unmatchedTestIndexes, e);
+                    if (testMatch != null) {
+                        unmatchedControlIndexes.Remove(i);
+                        unmatchedTestIndexes.Remove(testMatch.Index);
+                        matches.AddLast(new KeyValuePair<XmlNode,
+                                        XmlNode>(control, testMatch.Node));
+                    }
                 }
             }
             return matches;
@@ -117,26 +126,14 @@ public IEnumerable<KeyValuePair<XmlNode, XmlNode>>
         private MatchInfo FindMatchingNode(XmlNode searchFor,
                                            IList<XmlNode> searchIn,
                                            int indexOfLastMatch,
-                                           ICollection<int> availableIndexes) {
+                                           ICollection<int> availableIndexes,
+                                           ElementSelector e) {
             MatchInfo m = SearchIn(searchFor, searchIn,
                                    availableIndexes,
-                                   indexOfLastMatch + 1, searchIn.Count);
+                                   indexOfLastMatch + 1, searchIn.Count, e);
             return m ?? SearchIn(searchFor, searchIn,
                                  availableIndexes,
-                                 0, indexOfLastMatch);
-        }
-
-        private MatchInfo SearchIn(XmlNode searchFor,
-                                   IList<XmlNode> searchIn,
-                                   ICollection<int> availableIndexes,
-                                   int fromInclusive, int toExclusive) {
-            foreach (ElementSelector e in elementSelectors) {
-                MatchInfo m = SearchIn(searchFor, searchIn, availableIndexes, fromInclusive, toExclusive, e);
-                if (m != null) {
-                    return m;
-                }
-            }
-            return null;
+                                 0, indexOfLastMatch, e);
         }
 
         private MatchInfo SearchIn(XmlNode searchFor,
@@ -195,4 +192,4 @@ public static bool DefaultNodeTypeMatcher(XmlNodeType controlType,
                     && testType == XmlNodeType.CDATA);
         }
     }
-}
+}
diff --git a/src/tests/net-core/Diff/DefaultNodeMatcherTest.cs b/src/tests/net-core/Diff/DefaultNodeMatcherTest.cs
@@ -0,0 +1,120 @@
+/*
+  This file is licensed to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Xml;
+using NUnit.Framework;
+
+namespace Org.XmlUnit.Diff {
+
+    [TestFixture]
+    public class DefaultNodeMatcherTest {
+
+        private XmlDocument doc;
+
+        [SetUp]
+        public void CreateDoc() {
+            doc = new XmlDocument();
+        }
+
+        [Test]
+        public void ElementSelectorsAreQueriedInSequence() {
+            XmlElement control1 = doc.CreateElement("a");
+            control1.AppendChild(doc.CreateTextNode("foo"));
+            XmlElement control2 = doc.CreateElement("a");
+            control2.AppendChild(doc.CreateTextNode("bar"));
+
+            XmlElement test1 = doc.CreateElement("a");
+            test1.AppendChild(doc.CreateTextNode("baz"));
+            XmlElement test2 = doc.CreateElement("a");
+            test2.AppendChild(doc.CreateTextNode("foo"));
+
+            DefaultNodeMatcher m =
+                new DefaultNodeMatcher(ElementSelectors.ByNameAndText,
+                    ElementSelectors.ByName);
+            List<KeyValuePair<XmlNode, XmlNode>> result =
+                m.Match(new XmlNode[] { control1, control2},
+                        new XmlNode[] { test1, test2 }).ToList();
+            Assert.AreEqual(result.Count, 2);
+
+            // ByNameAndText
+            Assert.AreSame(result[0].Key, control1);
+            Assert.AreSame(result[0].Value, test2);
+
+            // ByName
+            Assert.AreSame(result[1].Key, control2);
+            Assert.AreSame(result[1].Value, test1);
+        }
+
+        [Test]
+        // https://github.com/xmlunit/xmlunit/issues/197
+        public void ElementSelectorsAreQueriedInSequenceWithConditionalSelector() {
+            XmlElement control1 = doc.CreateElement("a");
+            control1.AppendChild(doc.CreateTextNode("foo"));
+            XmlElement control2 = doc.CreateElement("a");
+            control2.AppendChild(doc.CreateTextNode("bar"));
+
+            XmlElement test1 = doc.CreateElement("a");
+            test1.AppendChild(doc.CreateTextNode("baz"));
+            XmlElement test2 = doc.CreateElement("a");
+            test2.AppendChild(doc.CreateTextNode("foo"));
+
+            DefaultNodeMatcher m =
+                new DefaultNodeMatcher(ElementSelectors.SelectorForElementNamed("a", ElementSelectors.ByNameAndText),
+                    ElementSelectors.ByName);
+            List<KeyValuePair<XmlNode, XmlNode>> result =
+                m.Match(new XmlNode[] { control1, control2},
+                        new XmlNode[] { test1, test2 }).ToList();
+            Assert.AreEqual(result.Count, 2);
+
+            // ByNameAndText
+            Assert.AreSame(result[0].Key, control1);
+            Assert.AreSame(result[0].Value, test2);
+
+            // ByName
+            Assert.AreSame(result[1].Key, control2);
+            Assert.AreSame(result[1].Value, test1);
+        }
+
+        [Test]
+        public void ElementSelectorsAreQueriedInSequenceWithControlNodesSwapped() {
+            XmlElement control1 = doc.CreateElement("a");
+            control1.AppendChild(doc.CreateTextNode("bar"));
+            XmlElement control2 = doc.CreateElement("a");
+            control2.AppendChild(doc.CreateTextNode("foo"));
+
+            XmlElement test1 = doc.CreateElement("a");
+            test1.AppendChild(doc.CreateTextNode("foo"));
+            XmlElement test2 = doc.CreateElement("a");
+            test2.AppendChild(doc.CreateTextNode("baz"));
+
+            DefaultNodeMatcher m =
+                new DefaultNodeMatcher(ElementSelectors.ByNameAndText,
+                    ElementSelectors.ByName);
+            List<KeyValuePair<XmlNode, XmlNode>> result =
+                m.Match(new XmlNode[] { control1, control2},
+                        new XmlNode[] { test1, test2 }).ToList();
+            Assert.AreEqual(result.Count, 2);
+
+            // ByNameAndText
+            Assert.AreSame(result[0].Key, control2);
+            Assert.AreSame(result[0].Value, test1);
+
+            // ByName
+            Assert.AreSame(result[1].Key, control1);
+            Assert.AreSame(result[1].Value, test2);
+        }
+    }
+}
diff --git a/src/tests/net-core/NetFramework/XMLUnit.Core.Tests.NetFramework.csproj b/src/tests/net-core/NetFramework/XMLUnit.Core.Tests.NetFramework.csproj
@@ -74,6 +74,7 @@
     <Compile Include="..\Diff\ByNameAndTextRecSelectorTest.cs" />
     <Compile Include="..\Diff\ChildNodeXPathContextProviderTest.cs" />
     <Compile Include="..\Diff\ComparisonControllersTest.cs" />
+    <Compile Include="..\Diff\DefaultNodeMatcherTest.cs" />
     <Compile Include="..\Diff\DefaultComparisonFormatterTest.cs" />
     <Compile Include="..\Diff\DifferenceEvaluatorsTest.cs" />
     <Compile Include="..\Diff\DifferenceTest.cs" />