From 3ed1aaade6e89de43b54a31dc9a489a4a43d1304 Mon Sep 17 00:00:00 2001 From: "wangdongyu.danny" Date: Mon, 27 May 2024 15:35:09 +0800 Subject: [PATCH] fix visitor pattern by calling visit instead of accept Signed-off-by: wangdongyu.danny --- .../org/opensearch/index/query/BoostingQueryBuilder.java | 4 ++-- .../opensearch/index/query/ConstantScoreQueryBuilder.java | 2 +- .../org/opensearch/index/query/DisMaxQueryBuilder.java | 2 +- .../index/query/FieldMaskingSpanQueryBuilder.java | 2 +- .../opensearch/index/query/SpanContainingQueryBuilder.java | 4 ++-- .../org/opensearch/index/query/SpanFirstQueryBuilder.java | 2 +- .../opensearch/index/query/SpanMultiTermQueryBuilder.java | 2 +- .../org/opensearch/index/query/SpanNearQueryBuilder.java | 2 +- .../org/opensearch/index/query/SpanNotQueryBuilder.java | 4 ++-- .../org/opensearch/index/query/SpanOrQueryBuilder.java | 2 +- .../org/opensearch/index/query/SpanWithinQueryBuilder.java | 4 ++-- .../opensearch/index/query/BoostingQueryBuilderTests.java | 6 ++++-- .../index/query/ConstantScoreQueryBuilderTests.java | 7 +++++-- .../opensearch/index/query/DisMaxQueryBuilderTests.java | 5 ++++- 14 files changed, 28 insertions(+), 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 index 1b52ae2f03605..80194cd914fd8 100644 --- 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 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 index b2764d29da80a..9b76d84ad7b94 100644 --- 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 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 index bd8ec62f6c43e..8a05b135234a4 100644 --- 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 index 4e73d87b07b7a..79a0f26030520 100644 --- 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 index 32a19ea3e9b50..f23f1c18d6943 100644 --- 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 index bcbc64ddf386d..1a4da17530188 100644 --- 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 index 96d03c91964e3..09af488e6ee2c 100644 --- 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 index 30a1c29c29126..cf7716015388e 100644 --- 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 index 59ec5b9d77fc8..02d12b68ea8ab 100644 --- 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 index fae1e318c66bd..ddd52db59527f 100644 --- 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 index 4d5a6dde61a70..fc1ccd09ab21b 100644 --- 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 index 66a02a02d4e5b..1c24f6a0b17d8 100644 --- 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 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 index 527413d2513d0..602d7c8846010 100644 --- 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 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 index cb0df38de5c02..cc680605c5a68 100644 --- 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 visitedQueries = new ArrayList<>(); dismax.visit(createTestVisitor(visitedQueries)); - assertEquals(2, visitedQueries.size()); + assertEquals(5, visitedQueries.size()); } }