From 88171160a29956620652261f01a01cc40f45678b Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 23 Nov 2022 16:24:12 -0800 Subject: [PATCH 1/6] Added default constructor for JSON parsing using jackson Signed-off-by: Owais Kazi --- .../aggregations/AbstractAggregationBuilder.java | 4 ++++ .../search/aggregations/AggregationBuilder.java | 14 +++++++++++++- .../metrics/SumAggregationBuilder.java | 4 ++++ .../support/ValuesSourceAggregationBuilder.java | 8 ++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java index 5863bf66f59f8..a08cfa95a42d8 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java @@ -77,6 +77,10 @@ protected AbstractAggregationBuilder(StreamInput in) throws IOException { metadata = in.readMap(); } + protected AbstractAggregationBuilder() { + super(); + } + @Override public final void writeTo(StreamOutput out) throws IOException { out.writeString(name); diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java index f5f11834b4484..700c9f01a6497 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java @@ -31,6 +31,10 @@ package org.opensearch.search.aggregations; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonTypeInfo; import org.opensearch.common.ParseField; import org.opensearch.common.Strings; import org.opensearch.common.io.stream.NamedWriteable; @@ -51,6 +55,8 @@ * * @opensearch.internal */ +@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) +@JsonIgnoreProperties(ignoreUnknown = true) public abstract class AggregationBuilder implements NamedWriteable, @@ -58,9 +64,13 @@ public abstract class AggregationBuilder BaseAggregationBuilder, Rewriteable { - protected final String name; + protected String name; protected AggregatorFactories.Builder factoriesBuilder = AggregatorFactories.builder(); + @JsonCreator + AggregationBuilder() { + } + /** * Constructs a new aggregation builder. * @@ -100,11 +110,13 @@ public String getName() { public abstract AggregationBuilder subAggregation(PipelineAggregationBuilder aggregation); /** Return the configured set of subaggregations **/ + @JsonProperty("subAggregations") public Collection getSubAggregations() { return factoriesBuilder.getAggregatorFactories(); } /** Return the configured set of pipeline aggregations **/ + @JsonProperty("pipelineAggregations") public Collection getPipelineAggregations() { return factoriesBuilder.getPipelineAggregatorFactories(); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java index d6d33eae90c96..52b3f3b7e5b0c 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java @@ -75,6 +75,10 @@ public SumAggregationBuilder(String name) { super(name); } + private SumAggregationBuilder() { + super(); + } + protected SumAggregationBuilder( SumAggregationBuilder clone, AggregatorFactories.Builder factoriesBuilder, diff --git a/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java index b492d9cadb975..0ef898a751e41 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java @@ -59,6 +59,10 @@ */ public abstract class ValuesSourceAggregationBuilder> extends AbstractAggregationBuilder { + private ValuesSourceAggregationBuilder() { + super(); + } + public static void declareFields( AbstractObjectParser, T> objectParser, boolean scriptable, @@ -166,6 +170,10 @@ protected LeafOnly(StreamInput in) throws IOException { super(in); } + public LeafOnly() { + super(); + } + @Override public final AB subAggregations(Builder subFactories) { throw new AggregationInitializationException( From 420f8f92dd398a0d9546b3d31855e2ca727382ca Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 23 Nov 2022 16:30:03 -0800 Subject: [PATCH 2/6] Updated changelog Signed-off-by: Owais Kazi --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e580801590c86..883a56640d2aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Enforce type safety for RegisterTransportActionsRequest([#4796](https://github.com/opensearch-project/OpenSearch/pull/4796)) - Modified local node request to return Discovery Node ([#4862](https://github.com/opensearch-project/OpenSearch/pull/4862)) - Enforce type safety for NamedWriteableRegistryParseRequest ([#4923](https://github.com/opensearch-project/OpenSearch/pull/4923)) + - Added default constructor for JSON parsing using Jackson ([#5369](https://github.com/opensearch-project/OpenSearch/pull/5369)) ## [Unreleased 2.x] ### Added From 63bd5b79053843b431b6b180c407e8e558c622ac Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Wed, 23 Nov 2022 16:45:03 -0800 Subject: [PATCH 3/6] Formatted code using spotless Signed-off-by: Owais Kazi --- .../org/opensearch/search/aggregations/AggregationBuilder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java index 700c9f01a6497..a1ff6ee7205ad 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java @@ -68,8 +68,7 @@ public abstract class AggregationBuilder protected AggregatorFactories.Builder factoriesBuilder = AggregatorFactories.builder(); @JsonCreator - AggregationBuilder() { - } + AggregationBuilder() {} /** * Constructs a new aggregation builder. From b577b57222d3d989782cf483029eb1bd2802f9bf Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 28 Nov 2022 12:10:28 -0800 Subject: [PATCH 4/6] Addressed PR comments Signed-off-by: Owais Kazi --- .../search/aggregations/AbstractAggregationBuilder.java | 3 +++ .../opensearch/search/aggregations/AggregationBuilder.java | 5 ----- .../search/aggregations/metrics/SumAggregationBuilder.java | 3 +++ .../support/ValuesSourceAggregationBuilder.java | 6 ++++++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java index a08cfa95a42d8..535b19b5c5dd8 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AbstractAggregationBuilder.java @@ -77,6 +77,9 @@ protected AbstractAggregationBuilder(StreamInput in) throws IOException { metadata = in.readMap(); } + /** + * Constructor required for Jackson parsing + */ protected AbstractAggregationBuilder() { super(); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java index a1ff6ee7205ad..b6b6da3e7f82c 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java @@ -55,8 +55,6 @@ * * @opensearch.internal */ -@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) -@JsonIgnoreProperties(ignoreUnknown = true) public abstract class AggregationBuilder implements NamedWriteable, @@ -67,7 +65,6 @@ public abstract class AggregationBuilder protected String name; protected AggregatorFactories.Builder factoriesBuilder = AggregatorFactories.builder(); - @JsonCreator AggregationBuilder() {} /** @@ -109,13 +106,11 @@ public String getName() { public abstract AggregationBuilder subAggregation(PipelineAggregationBuilder aggregation); /** Return the configured set of subaggregations **/ - @JsonProperty("subAggregations") public Collection getSubAggregations() { return factoriesBuilder.getAggregatorFactories(); } /** Return the configured set of pipeline aggregations **/ - @JsonProperty("pipelineAggregations") public Collection getPipelineAggregations() { return factoriesBuilder.getPipelineAggregatorFactories(); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java index 52b3f3b7e5b0c..1d90988e0676b 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/metrics/SumAggregationBuilder.java @@ -75,6 +75,9 @@ public SumAggregationBuilder(String name) { super(name); } + /** + * Constructor required for Jackson parsing + */ private SumAggregationBuilder() { super(); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java index 0ef898a751e41..496711ae4173e 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/support/ValuesSourceAggregationBuilder.java @@ -59,6 +59,9 @@ */ public abstract class ValuesSourceAggregationBuilder> extends AbstractAggregationBuilder { + /** + * Constructor required for Jackson parsing + */ private ValuesSourceAggregationBuilder() { super(); } @@ -170,6 +173,9 @@ protected LeafOnly(StreamInput in) throws IOException { super(in); } + /** + * Constructor required for Jackson parsing + */ public LeafOnly() { super(); } From e202a93491e8efba0736f73352bfaeb32ef221ee Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 28 Nov 2022 12:16:27 -0800 Subject: [PATCH 5/6] Added comment for default construtor Signed-off-by: Owais Kazi --- .../org/opensearch/search/aggregations/AggregationBuilder.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java index b6b6da3e7f82c..d1efb5ee91dc6 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java @@ -65,6 +65,9 @@ public abstract class AggregationBuilder protected String name; protected AggregatorFactories.Builder factoriesBuilder = AggregatorFactories.builder(); + /** + * Constructor required for Jackson parsing + */ AggregationBuilder() {} /** From b17cd4721fb042daf9471cc5441434ade178cff2 Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Mon, 28 Nov 2022 18:00:39 -0800 Subject: [PATCH 6/6] Spotless Signed-off-by: Owais Kazi --- .../opensearch/search/aggregations/AggregationBuilder.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java index d1efb5ee91dc6..7e1b889915f67 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java +++ b/server/src/main/java/org/opensearch/search/aggregations/AggregationBuilder.java @@ -31,10 +31,6 @@ package org.opensearch.search.aggregations; -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonTypeInfo; import org.opensearch.common.ParseField; import org.opensearch.common.Strings; import org.opensearch.common.io.stream.NamedWriteable;