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 <[email protected]>
opensearch-project · Jan 25, 2023 · 23baad9 · 23baad9
1 parent 41beeda
commit 23baad9
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 8 deletions.
diff --git 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
@@ -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<Union, Kind, Member> implements JsonpDeserializer<Union> {
 
@@ -169,14 +171,15 @@ public Builder<Union, Kind, Member> addMember(Kind tag, JsonpDeserializer<? exte
             JsonpDeserializer<?> unwrapped = DelegatingDeserializer.unwrap(deserializer);
             if (unwrapped instanceof ObjectDeserializer) {
                 ObjectDeserializer<?> od = (ObjectDeserializer<?>) unwrapped;
-                Set<String> allFields = od.fieldNames();
-                Set<String> fields = new HashSet<>(allFields); // copy to update
-                for (UnionDeserializer.SingleMemberHandler<Union, Kind, Member> member: objectMembers) {
-                    // Remove respective fields on both sides to keep specific ones
-                    fields.removeAll(member.fields);
-                    member.fields.removeAll(allFields);
-                }
-                UnionDeserializer.SingleMemberHandler<Union, Kind, Member> member = new SingleMemberHandler<>(tag, deserializer, fields);
+//                Set<String> allFields = od.fieldNames();
+//                Set<String> fields = new HashSet<>(allFields); // copy to update
+//                for (UnionDeserializer.SingleMemberHandler<Union, Kind, Member> member: objectMembers) {
+//                    // Remove respective fields on both sides to keep specific ones
+//                    fields.removeAll(member.fields);
+//                    member.fields.removeAll(allFields);
+//                }
+                UnionDeserializer.SingleMemberHandler<Union, Kind, Member> 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<Union, Kind, Member> addMember(Kind tag, JsonpDeserializer<? exte
 
         @Override
         public JsonpDeserializer<Union> build() {
+            Map<String, Long> fieldFrequencies = objectMembers.stream().flatMap(m -> m.fields.stream())
+                    .collect( Collectors.groupingBy(Function.identity(), Collectors.counting()));
+            Set<String> duplicateFields = fieldFrequencies.entrySet().stream()
+                    .filter(entry -> entry.getValue() > 1)
+                    .map(Map.Entry::getKey)
+                    .collect(Collectors.toSet());
+            for (UnionDeserializer.SingleMemberHandler<Union, Kind, Member> member: objectMembers) {
+                member.fields.removeAll(duplicateFields);
+            }
+
             // Check that no object member had all its fields removed
             for (UnionDeserializer.SingleMemberHandler<Union, Kind, Member> 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
@@ -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<AppData> response = javaClient().search(searchRequest, AppData.class);
+        assertTrue(response.suggest().size() > 0);
+    }
+
 //    @Test
 //    public void testValueBodyResponse() throws Exception {
 //        DiskUsageResponse resp = highLevelClient().indices().diskUsage(b -> b