Skip to content

Commit

Permalink
Fix issue with completion suggester being parsed as term suggester. (o…
Browse files Browse the repository at this point in the history
…pensearch-project#347)

* 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 <git@danielhart.me>

* Remove commented code

Signed-off-by: Dan Hart <git@danielhart.me>

* Fix format of changelog item

Signed-off-by: Dan Hart <git@danielhart.me>

Signed-off-by: Dan Hart <git@danielhart.me>
  • Loading branch information
iamdanhart authored Jan 25, 2023
1 parent 41beeda commit 013d8d7
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand Down Expand Up @@ -169,14 +171,8 @@ 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);
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
Expand All @@ -194,6 +190,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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 013d8d7

Please sign in to comment.