From 23baad98ca568550d376c5f7e8691d991cb502aa Mon Sep 17 00:00:00 2001 From: Dan Hart Date: Wed, 25 Jan 2023 11:55:08 -0500 Subject: [PATCH 1/3] Fix issue with completion suggester being parsed as term suggester. This commit fixes the issue where a completion suggester search request was being parsed as a term suggester and failing due to "Missing required property 'TermSuggestOption.score'" This solution was originally proposed by Github user @apatrida Signed-off-by: Dan Hart --- CHANGELOG.md | 1 + .../client/json/UnionDeserializer.java | 29 ++++++--- .../integTest/AbstractRequestIT.java | 60 +++++++++++++++++++ 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f47744ec..95aec9801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Update Gradle to 7.6 ([#309](https://github.com/opensearch-project/opensearch-java/pull/309)) - Prevent SPI calls at runtime ([#293](https://github.com/opensearch-project/opensearch-java/pull/293)) - Add support for OpenSearch Serverless ([#339](https://github.com/opensearch-project/opensearch-java/pull/339)) +- Fix issue where completion suggestions were failing, due to being parsed as term suggestions([#107](https://github.com/opensearch-project/opensearch-java/issues/107)) ### Deprecated diff --git a/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java b/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java index dc54bf43e..568d32083 100644 --- a/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java +++ b/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java @@ -48,6 +48,8 @@ import java.util.Map; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.stream.Collectors; public class UnionDeserializer implements JsonpDeserializer { @@ -169,14 +171,15 @@ public Builder addMember(Kind tag, JsonpDeserializer unwrapped = DelegatingDeserializer.unwrap(deserializer); if (unwrapped instanceof ObjectDeserializer) { ObjectDeserializer od = (ObjectDeserializer) unwrapped; - Set allFields = od.fieldNames(); - Set fields = new HashSet<>(allFields); // copy to update - for (UnionDeserializer.SingleMemberHandler member: objectMembers) { - // Remove respective fields on both sides to keep specific ones - fields.removeAll(member.fields); - member.fields.removeAll(allFields); - } - UnionDeserializer.SingleMemberHandler member = new SingleMemberHandler<>(tag, deserializer, fields); +// Set allFields = od.fieldNames(); +// Set fields = new HashSet<>(allFields); // copy to update +// for (UnionDeserializer.SingleMemberHandler member: objectMembers) { +// // Remove respective fields on both sides to keep specific ones +// fields.removeAll(member.fields); +// member.fields.removeAll(allFields); +// } + UnionDeserializer.SingleMemberHandler member = + new SingleMemberHandler<>(tag, deserializer, new HashSet<>(od.fieldNames())); objectMembers.add(member); if (od.shortcutProperty() != null) { // also add it as a string @@ -194,6 +197,16 @@ public Builder addMember(Kind tag, JsonpDeserializer build() { + Map fieldFrequencies = objectMembers.stream().flatMap(m -> m.fields.stream()) + .collect( Collectors.groupingBy(Function.identity(), Collectors.counting())); + Set duplicateFields = fieldFrequencies.entrySet().stream() + .filter(entry -> entry.getValue() > 1) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); + for (UnionDeserializer.SingleMemberHandler member: objectMembers) { + member.fields.removeAll(duplicateFields); + } + // Check that no object member had all its fields removed for (UnionDeserializer.SingleMemberHandler member: objectMembers) { if (member.fields.isEmpty()) { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java index 374b3d1fc..08ec63b00 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java @@ -55,6 +55,10 @@ import org.opensearch.client.opensearch.core.SearchRequest; import org.opensearch.client.opensearch.core.bulk.OperationType; import org.opensearch.client.opensearch.core.msearch.RequestItem; +import org.opensearch.client.opensearch.core.search.CompletionSuggester; +import org.opensearch.client.opensearch.core.search.FieldSuggester; +import org.opensearch.client.opensearch.core.search.FieldSuggesterBuilders; +import org.opensearch.client.opensearch.core.search.Suggester; import org.opensearch.client.opensearch.indices.CreateIndexResponse; import org.opensearch.client.opensearch.indices.GetIndexResponse; import org.opensearch.client.opensearch.indices.GetIndicesSettingsResponse; @@ -433,6 +437,62 @@ public void testDefaultIndexSettings() throws IOException { assertNull(settings.get(index).defaults()); } + @Test + public void testCompletionSuggester() throws IOException { + + String index = "test-completion-suggester"; + + Property intValueProp = new Property.Builder() + .long_(v -> v) + .build(); + Property msgCompletionProp = new Property.Builder() + .completion(c -> c) + .build(); + javaClient().indices().create(c -> c + .index(index) + .mappings(m -> m + .properties("intValue", intValueProp) + .properties("msg", msgCompletionProp))); + + AppData appData = new AppData(); + appData.setIntValue(1337); + appData.setMsg("foo"); + + javaClient().index(b -> b + .index(index) + .id("1") + .document(appData) + .refresh(Refresh.True)); + + appData.setIntValue(1338); + appData.setMsg("bar"); + + javaClient().index(b -> b + .index(index) + .id("2") + .document(appData) + .refresh(Refresh.True)); + + CompletionSuggester completionSuggester = FieldSuggesterBuilders.completion() + .field("msg") + .size(1) + .build(); + FieldSuggester fieldSuggester = new FieldSuggester.Builder() + .completion(completionSuggester) + .build(); + Suggester suggester = new Suggester.Builder() + .suggesters(Collections.singletonMap("msgSuggester", fieldSuggester)) + .text("foo") + .build(); + SearchRequest searchRequest = new SearchRequest.Builder() + .index(index) + .suggest(suggester) + .build(); + + SearchResponse response = javaClient().search(searchRequest, AppData.class); + assertTrue(response.suggest().size() > 0); + } + // @Test // public void testValueBodyResponse() throws Exception { // DiskUsageResponse resp = highLevelClient().indices().diskUsage(b -> b From 6a692f361c7d6e88c619cbc26d339701840680c2 Mon Sep 17 00:00:00 2001 From: Dan Hart Date: Wed, 25 Jan 2023 12:00:03 -0500 Subject: [PATCH 2/3] Remove commented code Signed-off-by: Dan Hart --- .../java/org/opensearch/client/json/UnionDeserializer.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java b/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java index 568d32083..3905139a8 100644 --- a/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java +++ b/java-client/src/main/java/org/opensearch/client/json/UnionDeserializer.java @@ -171,13 +171,6 @@ public Builder addMember(Kind tag, JsonpDeserializer unwrapped = DelegatingDeserializer.unwrap(deserializer); if (unwrapped instanceof ObjectDeserializer) { ObjectDeserializer od = (ObjectDeserializer) unwrapped; -// Set allFields = od.fieldNames(); -// Set fields = new HashSet<>(allFields); // copy to update -// for (UnionDeserializer.SingleMemberHandler member: objectMembers) { -// // Remove respective fields on both sides to keep specific ones -// fields.removeAll(member.fields); -// member.fields.removeAll(allFields); -// } UnionDeserializer.SingleMemberHandler member = new SingleMemberHandler<>(tag, deserializer, new HashSet<>(od.fieldNames())); objectMembers.add(member); From 33f2bf440feb57cefd2141553a1a426621bd8418 Mon Sep 17 00:00:00 2001 From: Dan Hart Date: Wed, 25 Jan 2023 15:36:54 -0500 Subject: [PATCH 3/3] Fix format of changelog item Signed-off-by: Dan Hart --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95aec9801..0744fde7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Update Gradle to 7.6 ([#309](https://github.com/opensearch-project/opensearch-java/pull/309)) - Prevent SPI calls at runtime ([#293](https://github.com/opensearch-project/opensearch-java/pull/293)) - Add support for OpenSearch Serverless ([#339](https://github.com/opensearch-project/opensearch-java/pull/339)) -- Fix issue where completion suggestions were failing, due to being parsed as term suggestions([#107](https://github.com/opensearch-project/opensearch-java/issues/107)) +- Fix issue where completion suggestions were failing, due to being parsed as term suggestions ([#107](https://github.com/opensearch-project/opensearch-java/issues/107)) ### Deprecated