From c18adb94172f2ea2c2b2df7afc737dd523b35dd2 Mon Sep 17 00:00:00 2001 From: jvan1997 <32347479+jvan1997@users.noreply.github.com> Date: Wed, 7 Feb 2024 06:40:19 -0800 Subject: [PATCH] Removed required index for Hit.java (#833) For InnerHits, since the index and id is assumed to be the same as parent, they were not included in the InnerHit object, but when converting it into a Hit object in the java sdk, it would throw an exception, saying index is required. Removed it, and also added unit tests. Signed-off-by: jvan1997 <32347479+jvan1997@users.noreply.github.com> --- CHANGELOG.md | 5 +- .../client/opensearch/core/search/Hit.java | 14 +-- .../core/search/InnerHitsResultTest.java | 86 +++++++++++++++++++ 3 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 java-client/src/test/java/org/opensearch/client/opensearch/core/search/InnerHitsResultTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b936050dc4..0108e75715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,8 @@ This section is for maintaining a changelog for all breaking changes for the cli - Fix version and build ([#254](https://github.com/opensearch-project/opensearch-java/pull/254)) - Fix PutTemplateRequest field deserialization ([#723](https://github.com/opensearch-project/opensearch-java/pull/723)) - Fix PutIndexTemplateRequest field deserialization ([#765](https://github.com/opensearch-project/opensearch-java/pull/765)) -- Fix InnerHits storedFields deserialization/serialization ([#781](https://github.com/opensearch-project/opensearch-java/pull/781) +- Fix InnerHits storedFields deserialization/serialization ([#781](https://github.com/opensearch-project/opensearch-java/pull/781)) +- Fix InnerHits to no longer enforce the nullable Index field when converting to Hit. ([#825](https://github.com/opensearch-project/opensearch-java/issues/825)) ### Security @@ -265,4 +266,4 @@ This section is for maintaining a changelog for all breaking changes for the cli [2.5.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.4.0...v2.5.0 [2.4.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.3.0...v2.4.0 [2.3.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.2.0...v2.3.0 -[2.2.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.1.0...v2.2.0 \ No newline at end of file +[2.2.0]: https://github.com/opensearch-project/opensearch-java/compare/v2.1.0...v2.2.0 diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java b/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java index dab34ab94a..e39edf8bbc 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/core/search/Hit.java @@ -107,7 +107,7 @@ public class Hit implements JsonpSerializable { private Hit(Builder builder) { - this.index = ApiTypeHelper.requireNonNull(builder.index, this, "index"); + this.index = builder.index; this.id = builder.id; this.score = builder.score; this.explanation = builder.explanation; @@ -134,7 +134,7 @@ public static Hit of(Function, ObjectB } /** - * Required - API name: {@code _index} + * API name: {@code _index} */ public final String index() { return this.index; @@ -281,8 +281,10 @@ public void serialize(JsonGenerator generator, JsonpMapper mapper) { protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { - generator.writeKey("_index"); - generator.write(this.index); + if (this.index != null) { + generator.writeKey("_index"); + generator.write(this.index); + } if (this.id != null) { generator.writeKey("_id"); @@ -419,6 +421,8 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) { */ public static class Builder extends ObjectBuilderBase implements ObjectBuilder> { + + @Nullable private String index; @Nullable @@ -478,7 +482,7 @@ public static class Builder extends ObjectBuilderBase implements Obje /** * Required - API name: {@code _index} */ - public final Builder index(String value) { + public final Builder index(@Nullable String value) { this.index = value; return this; } diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/core/search/InnerHitsResultTest.java b/java-client/src/test/java/org/opensearch/client/opensearch/core/search/InnerHitsResultTest.java new file mode 100644 index 0000000000..902cbdc5fd --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/opensearch/core/search/InnerHitsResultTest.java @@ -0,0 +1,86 @@ +package org.opensearch.client.opensearch.core.search; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import java.io.StringReader; +import org.junit.Test; +import org.opensearch.client.json.JsonData; +import org.opensearch.client.json.JsonpMapper; +import org.opensearch.client.json.jsonb.JsonbJsonpMapper; + +public class InnerHitsResultTest { + private final JsonpMapper mapper = new JsonbJsonpMapper(); + private final String storedSalary = "details.salary"; + private final String storedJobId = "details.jobId"; + + /** + * test if the InnerHitsResult will build the Hit + */ + @Test + public void testInnerHits() { + + String classString = String.valueOf(hitResultWithIdIndex.innerHits().get("test_child").getClass()); + assertEquals(classString, InnerHitsResult.class.toString()); + // take hitResult and get the InnerHit + InnerHitsResult innerHitsResult = hitResultWithIdIndex.innerHits().get("test_child"); + Hit innerHitResult = innerHitsResult.hits().hits().get(0); + assertNotNull(innerHitResult.index()); + assertEquals(innerHitResult.index(), "_index"); + assertNotNull(innerHitResult.id()); + assertEquals(innerHitResult.id(), "child_id"); + } + + /** + * test if the InnerHitsResult will still build the Hit even if id and index is not specified + */ + @Test + public void testInnerHitWithoutIdIndex() { + + String classString = String.valueOf(hitResultNoIdIndex.innerHits().get("test_child").getClass()); + assertEquals(classString, InnerHitsResult.class.toString()); + // take hitResult and get the InnerHit + InnerHitsResult innerHitsResult = hitResultNoIdIndex.innerHits().get("test_child"); + Hit innerHitResult = innerHitsResult.hits().hits().get(0); + // Id and index are now nullable. + assertNull(innerHitResult.index()); + assertNull(innerHitResult.id()); + } + + private final String innerHitJsonWithNoIdOrIndex = "{\"key\":\"value\"}"; + private final String innerHitJsonWithIdOrIndex = "{\"id\":\"value\",\"index\":\"value\"}"; + + private final Hit hitResultNoIdIndex = Hit.of( + it -> it.id("test_parent") + .index("_index") + .innerHits( + "test_child", + innerHitsResultBuilder -> innerHitsResultBuilder.hits( + innerHitsMetadataBuilder -> innerHitsMetadataBuilder.total(total -> total.value(1).relation(TotalHitsRelation.Eq)) + .hits( + innerHitsListMemberBuilder -> innerHitsListMemberBuilder.source( + JsonData.from(mapper.jsonProvider().createParser(new StringReader(innerHitJsonWithNoIdOrIndex)), mapper) + ) + ) + ) + ) + ); + private final Hit hitResultWithIdIndex = Hit.of( + it -> it.id("test_parent") + .index("_index") + .innerHits( + "test_child", + innerHitsResultBuilder -> innerHitsResultBuilder.hits( + innerHitsMetadataBuilder -> innerHitsMetadataBuilder.total(total -> total.value(1).relation(TotalHitsRelation.Eq)) + .hits( + innerHitsListMemberBuilder -> innerHitsListMemberBuilder.id("child_id") + .index("_index") + .source( + JsonData.from(mapper.jsonProvider().createParser(new StringReader(innerHitJsonWithIdOrIndex)), mapper) + ) + ) + ) + ) + ); +}