fix visitor pattern by calling visit instead of accept

Signed-off-by: wangdongyu.danny <[email protected]>
wdongyu · Aug 22, 2024 · 3ed1aaa · 3ed1aaa
1 parent 0ca4ed0
commit 3ed1aaa
Show file tree

Hide file tree

Showing 14 changed files with 28 additions and 20 deletions.
diff --git a/server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/BoostingQueryBuilder.java
@@ -258,10 +258,10 @@ protected void extractInnerHitBuilders(Map<String, InnerHitContextBuilder> inner
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
         if (positiveQuery != null) {
-            visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(positiveQuery);
+            positiveQuery.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
         }
         if (negativeQuery != null) {
-            visitor.getChildVisitor(BooleanClause.Occur.SHOULD).accept(negativeQuery);
+            negativeQuery.visit(visitor.getChildVisitor(BooleanClause.Occur.SHOULD));
         }
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/ConstantScoreQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/ConstantScoreQueryBuilder.java
@@ -188,7 +188,7 @@ protected void extractInnerHitBuilders(Map<String, InnerHitContextBuilder> inner
     @Override
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
-        visitor.getChildVisitor(BooleanClause.Occur.FILTER).accept(filterBuilder);
+        filterBuilder.visit(visitor.getChildVisitor(BooleanClause.Occur.FILTER));
     }
 
 }
diff --git a/server/src/main/java/org/opensearch/index/query/DisMaxQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/DisMaxQueryBuilder.java
@@ -254,7 +254,7 @@ public void visit(QueryBuilderVisitor visitor) {
         if (queries.isEmpty() == false) {
             QueryBuilderVisitor subVisitor = visitor.getChildVisitor(BooleanClause.Occur.SHOULD);
             for (QueryBuilder subQb : queries) {
-                subVisitor.accept(subQb);
+                subQb.visit(subVisitor);
             }
         }
     }

diff --git a/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/FieldMaskingSpanQueryBuilder.java
@@ -214,6 +214,6 @@ public String getWriteableName() {
     @Override
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(queryBuilder);
+        queryBuilder.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/SpanContainingQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanContainingQueryBuilder.java
@@ -193,7 +193,7 @@ public String getWriteableName() {
     @Override
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(big);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(little);
+        big.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
+        little.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/SpanFirstQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanFirstQueryBuilder.java
@@ -191,6 +191,6 @@ public String getWriteableName() {
     @Override
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(matchBuilder);
+        matchBuilder.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/SpanMultiTermQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanMultiTermQueryBuilder.java
@@ -219,7 +219,7 @@ public String getWriteableName() {
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
         if (multiTermQueryBuilder != null) {
-            visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(multiTermQueryBuilder);
+            multiTermQueryBuilder.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
         }
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanNearQueryBuilder.java
@@ -306,7 +306,7 @@ public void visit(QueryBuilderVisitor visitor) {
         if (this.clauses.isEmpty() == false) {
             QueryBuilderVisitor subVisitor = visitor.getChildVisitor(BooleanClause.Occur.MUST);
             for (QueryBuilder subQb : this.clauses) {
-                subVisitor.accept(subQb);
+                subQb.visit(subVisitor);
             }
         }
     }

diff --git a/server/src/main/java/org/opensearch/index/query/SpanNotQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanNotQueryBuilder.java
@@ -290,11 +290,11 @@ public String getWriteableName() {
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
         if (include != null) {
-            visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(include);
+            include.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
         }
 
         if (exclude != null) {
-            visitor.getChildVisitor(BooleanClause.Occur.MUST_NOT).accept(exclude);
+            exclude.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST_NOT));
         }
     }
 }
diff --git a/server/src/main/java/org/opensearch/index/query/SpanOrQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanOrQueryBuilder.java
@@ -196,7 +196,7 @@ public void visit(QueryBuilderVisitor visitor) {
         if (clauses.isEmpty() == false) {
             QueryBuilderVisitor subVisitor = visitor.getChildVisitor(BooleanClause.Occur.SHOULD);
             for (QueryBuilder subQb : this.clauses) {
-                subVisitor.accept(subQb);
+                subQb.visit(subVisitor);
             }
         }
     }

diff --git a/server/src/main/java/org/opensearch/index/query/SpanWithinQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/SpanWithinQueryBuilder.java
@@ -202,7 +202,7 @@ public String getWriteableName() {
     @Override
     public void visit(QueryBuilderVisitor visitor) {
         visitor.accept(this);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(big);
-        visitor.getChildVisitor(BooleanClause.Occur.MUST).accept(little);
+        big.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
+        little.visit(visitor.getChildVisitor(BooleanClause.Occur.MUST));
     }
 }
diff --git a/server/src/test/java/org/opensearch/index/query/BoostingQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/BoostingQueryBuilderTests.java
@@ -158,13 +158,15 @@ public void testMustRewrite() throws IOException {
 
     public void testVisit() {
         BoostingQueryBuilder builder = new BoostingQueryBuilder(
-            new TermQueryBuilder("unmapped_field", "value"),
+            new BoolQueryBuilder()
+                .must(new TermQueryBuilder("unmapped_field1", "value1"))
+                .filter(new TermQueryBuilder("unmapped_field2", "value2")),
             new TermQueryBuilder(KEYWORD_FIELD_NAME, "other_value")
         );
 
         List<QueryBuilder> visitedQueries = new ArrayList<>();
         builder.visit(createTestVisitor(visitedQueries));
 
-        assertEquals(3, visitedQueries.size());
+        assertEquals(5, visitedQueries.size());
     }
 }
diff --git a/server/src/test/java/org/opensearch/index/query/ConstantScoreQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/ConstantScoreQueryBuilderTests.java
@@ -137,10 +137,13 @@ public void testMustRewrite() throws IOException {
     }
 
     public void testVisit() {
-        ConstantScoreQueryBuilder queryBuilder = new ConstantScoreQueryBuilder(new TermQueryBuilder("unmapped_field", "foo"));
+        ConstantScoreQueryBuilder queryBuilder = new ConstantScoreQueryBuilder(
+            new BoolQueryBuilder()
+                .must(new TermQueryBuilder("unmapped_field1", "value1"))
+                .filter(new TermQueryBuilder("unmapped_field2", "value2")));
         List<QueryBuilder> visitorQueries = new ArrayList<>();
         queryBuilder.visit(createTestVisitor(visitorQueries));
 
-        assertEquals(2, visitorQueries.size());
+        assertEquals(4, visitorQueries.size());
     }
 }
diff --git a/server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java
@@ -161,10 +161,13 @@ public void testRewriteMultipleTimes() throws IOException {
     public void testVisit() {
         DisMaxQueryBuilder dismax = new DisMaxQueryBuilder();
         dismax.add(new WrapperQueryBuilder(new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()).toString()));
+        dismax.add(new BoolQueryBuilder()
+            .must(new TermQueryBuilder("unmapped_field1", "value1"))
+            .filter(new TermQueryBuilder("unmapped_field2", "value2")));
 
         List<QueryBuilder> visitedQueries = new ArrayList<>();
         dismax.visit(createTestVisitor(visitedQueries));
 
-        assertEquals(2, visitedQueries.size());
+        assertEquals(5, visitedQueries.size());
     }
 }