You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by ni...@apache.org on 2021/07/24 19:13:36 UTC

[lucenenet] 01/02: BREAKING: Lucene.Net.QueryParser.Flexible.Core.Nodes.IQueryNode: Added RemoveChildren() method from Lucene 8.8.1 to fix broken RemoveFromParent() method behavior (applies patch LUCENE-5805).

This is an automated email from the ASF dual-hosted git repository.

nightowl888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git

commit 68e8e48dd75161d90481316c231780a6c42941c1
Author: Shad Storhaug <sh...@shadstorhaug.com>
AuthorDate: Sat Jul 24 05:17:24 2021 +0700

    BREAKING: Lucene.Net.QueryParser.Flexible.Core.Nodes.IQueryNode: Added RemoveChildren() method from Lucene 8.8.1 to fix broken RemoveFromParent() method behavior (applies patch LUCENE-5805).
---
 .../Flexible/Core/Nodes/QueryNode.cs               |  7 +++++
 .../Flexible/Core/Nodes/QueryNodeImpl.cs           | 35 +++++++++++-----------
 .../Flexible/Core/Nodes/TestQueryNode.cs           | 17 +++++++++++
 3 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNode.cs b/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNode.cs
index a42b93c..4ea4544 100644
--- a/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNode.cs
+++ b/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNode.cs
@@ -102,5 +102,12 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
         /// Removes this query node from its parent.
         /// </summary>
         void RemoveFromParent();
+
+        // LUCENENET: From Lucene 8.8.1, patch to broken RemoveFromParent() behavior
+        /// <summary>
+        /// Remove a child node.
+        /// </summary>
+        /// <param name="childNode">Which child to remove.</param>
+        void RemoveChildren(IQueryNode childNode);
     }
 }
diff --git a/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNodeImpl.cs b/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNodeImpl.cs
index be6599d..3be4de4 100644
--- a/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNodeImpl.cs
+++ b/src/Lucene.Net.QueryParser/Flexible/Core/Nodes/QueryNodeImpl.cs
@@ -4,7 +4,7 @@ using Lucene.Net.QueryParsers.Flexible.Core.Util;
 using System;
 using System.Collections.Generic;
 using System.Globalization;
-using System.Resources;
+using JCG = J2N.Collections.Generic;
 
 namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
 {
@@ -41,13 +41,13 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
 
         private Dictionary<string, object> tags = new Dictionary<string, object>();
 
-        private List<IQueryNode> clauses = null;
+        private JCG.List<IQueryNode> clauses = null;
 
         protected virtual void Allocate()
         {
             if (this.clauses == null)
             {
-                this.clauses = new List<IQueryNode>();
+                this.clauses = new JCG.List<IQueryNode>();
             }
             else
             {
@@ -129,7 +129,7 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
             // copy children
             if (this.clauses != null)
             {
-                List<IQueryNode> localClauses = new List<IQueryNode>();
+                JCG.List<IQueryNode> localClauses = new JCG.List<IQueryNode>();
                 foreach (IQueryNode clause in this.clauses)
                 {
                     localClauses.Add(clause.CloneTree());
@@ -155,7 +155,7 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
             {
                 return null;
             }
-            return new List<IQueryNode>(this.clauses);
+            return new JCG.List<IQueryNode>(this.clauses);
         }
 
         public virtual void SetTag(string tagName, object value)
@@ -240,23 +240,24 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
         /// </summary>
         public virtual IDictionary<string, object> TagMap => new Dictionary<string, object>(this.tags);
 
+        // LUCENENET NOTE: There was a bug in 4.8.1 here because parent.GetChildren() returns a copy of the
+        // children, so removing items from it is pointless. We therefore diverge to the version 8.8.1 source of Lucene
+        // from this point. Not sure when this patch was applied.
+
+        public virtual void RemoveChildren(IQueryNode childNode)
+        {
+            // LUCENENET: Use RemoveAll() method for optimal performance.
+            clauses.RemoveAll((value) => value == childNode);
+            childNode.RemoveFromParent();
+        }
+
         public virtual void RemoveFromParent()
         {
             if (this.parent != null)
             {
-                IList<IQueryNode> parentChildren = this.parent.GetChildren();
-
-                // LUCENENET NOTE: Loop in reverse so we can remove items
-                // without screwing up our iterator.
-                for (int i = parentChildren.Count - 1; i >= 0; i--)
-                {
-                    if (parentChildren[i] == this)
-                    {
-                        parentChildren.RemoveAt(i);
-                    }
-                }
-
+                IQueryNode parent = this.parent;
                 this.parent = null;
+                parent.RemoveChildren(this);
             }
         }
 
diff --git a/src/Lucene.Net.Tests.QueryParser/Flexible/Core/Nodes/TestQueryNode.cs b/src/Lucene.Net.Tests.QueryParser/Flexible/Core/Nodes/TestQueryNode.cs
index 980aad7..346150a 100644
--- a/src/Lucene.Net.Tests.QueryParser/Flexible/Core/Nodes/TestQueryNode.cs
+++ b/src/Lucene.Net.Tests.QueryParser/Flexible/Core/Nodes/TestQueryNode.cs
@@ -62,6 +62,8 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
 
             fieldNode.RemoveFromParent();
             assertNull(fieldNode.Parent);
+            /* LUCENE-5805 - QueryNodeImpl.removeFromParent does a lot of work without any effect */
+            assertFalse(booleanNode.GetChildren().Contains(fieldNode));
 
             booleanNode.Add(fieldNode);
             assertNotNull(fieldNode.Parent);
@@ -69,5 +71,20 @@ namespace Lucene.Net.QueryParsers.Flexible.Core.Nodes
             booleanNode.Set(Collections.EmptyList<IQueryNode>());
             assertNull(fieldNode.Parent);
         }
+
+        // LUCENENET: Added this patch in Lucene.NET 4.8.0 from Lucene 5.3.0
+        [Test]
+        public void TestRemoveChildren()
+        {
+            BooleanQueryNode booleanNode = new BooleanQueryNode(Collections.EmptyList<IQueryNode>());
+            FieldQueryNode fieldNode = new FieldQueryNode("foo", "A", 0, 1);
+
+            booleanNode.Add(fieldNode);
+            assertTrue(booleanNode.GetChildren().Count == 1);
+
+            booleanNode.RemoveChildren(fieldNode);
+            assertTrue(booleanNode.GetChildren().Count == 0);
+            assertNull(fieldNode.Parent);
+        }
     }
 }