From 534b9829f4f6e9571487a73c0cbbeb6cbc27bf8f Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 24 May 2018 12:35:06 -0700 Subject: [PATCH 1/5] Make sure all instance variables are final. --- .../bucket/range/InternalDateRange.java | 4 ---- .../bucket/range/InternalGeoDistance.java | 4 ---- .../bucket/range/InternalRange.java | 24 ++++++++----------- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java index 89d419cc14330..408c1325b85c9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalDateRange.java @@ -36,10 +36,6 @@ public class InternalDateRange extends InternalRange aggregations, boolean keyed, DocValueFormat formatter) { super(key, from, to, docCount, new InternalAggregations(aggregations), keyed, formatter); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java index 2394f57b0b821..08d949d5bef83 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalGeoDistance.java @@ -35,10 +35,6 @@ public class InternalGeoDistance extends InternalRange aggregations, boolean keyed) { this(key, from, to, docCount, new InternalAggregations(aggregations), keyed); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index cde90727e49fc..7042ee4a17d97 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -44,20 +44,16 @@ public static class Bucket extends InternalMultiBucketAggregation.InternalBucket protected final transient boolean keyed; protected final transient DocValueFormat format; - protected double from; - protected double to; - private long docCount; - InternalAggregations aggregations; - private String key; - - public Bucket(boolean keyed, DocValueFormat formatter) { - this.keyed = keyed; - this.format = formatter; - } + protected final double from; + protected final double to; + private final long docCount; + final InternalAggregations aggregations; + private final String key; public Bucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed, DocValueFormat formatter) { - this(keyed, formatter); + this.keyed = keyed; + this.format = formatter; this.key = key != null ? key : generateKey(from, to, formatter); this.from = from; this.to = to; @@ -230,9 +226,9 @@ public B createBucket(InternalAggregations aggregations, B prototype) { } } - private List ranges; - protected DocValueFormat format; - protected boolean keyed; + private final List ranges; + protected final DocValueFormat format; + protected final boolean keyed; public InternalRange(String name, List ranges, DocValueFormat format, boolean keyed, List pipelineAggregators, From 8cbb93174f7e679fff8c6913d007e67f9eee65af Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 24 May 2018 13:58:33 -0700 Subject: [PATCH 2/5] Make generateKey a private static method, instead of protected. This flexibility isn't currently needed, and it's best not to call overridable methods in constructors. --- .../aggregations/bucket/range/InternalRange.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index 7042ee4a17d97..9f029959b6689 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -158,12 +158,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - protected String generateKey(double from, double to, DocValueFormat formatter) { - StringBuilder sb = new StringBuilder(); - sb.append(Double.isInfinite(from) ? "*" : formatter.format(from)); - sb.append("-"); - sb.append(Double.isInfinite(to) ? "*" : formatter.format(to)); - return sb.toString(); + private static String generateKey(double from, double to, DocValueFormat formatter) { + StringBuilder builder = new StringBuilder() + .append(Double.isInfinite(from) ? "*" : formatter.format(from)) + .append("-") + .append(Double.isInfinite(to) ? "*" : formatter.format(to)); + return builder.toString(); } @Override From 2fdeb3132797e989eb37672efccc6fae3570835d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 24 May 2018 14:06:30 -0700 Subject: [PATCH 3/5] Rename formatter -> format for consistency. --- .../bucket/range/InternalRange.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index 9f029959b6689..4cc98a87113e0 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -51,10 +51,10 @@ public static class Bucket extends InternalMultiBucketAggregation.InternalBucket private final String key; public Bucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed, - DocValueFormat formatter) { + DocValueFormat format) { this.keyed = keyed; - this.format = formatter; - this.key = key != null ? key : generateKey(from, to, formatter); + this.format = format; + this.key = key != null ? key : generateKey(from, to, format); this.from = from; this.to = to; this.docCount = docCount; @@ -158,11 +158,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - private static String generateKey(double from, double to, DocValueFormat formatter) { + private static String generateKey(double from, double to, DocValueFormat format) { StringBuilder builder = new StringBuilder() - .append(Double.isInfinite(from) ? "*" : formatter.format(from)) + .append(Double.isInfinite(from) ? "*" : format.format(from)) .append("-") - .append(Double.isInfinite(to) ? "*" : formatter.format(to)); + .append(Double.isInfinite(to) ? "*" : format.format(to)); return builder.toString(); } @@ -202,15 +202,15 @@ public ValueType getValueType() { } @SuppressWarnings("unchecked") - public R create(String name, List ranges, DocValueFormat formatter, boolean keyed, List pipelineAggregators, + public R create(String name, List ranges, DocValueFormat format, boolean keyed, List pipelineAggregators, Map metaData) { - return (R) new InternalRange(name, ranges, formatter, keyed, pipelineAggregators, metaData); + return (R) new InternalRange(name, ranges, format, keyed, pipelineAggregators, metaData); } @SuppressWarnings("unchecked") public B createBucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed, - DocValueFormat formatter) { - return (B) new Bucket(key, from, to, docCount, aggregations, keyed, formatter); + DocValueFormat format) { + return (B) new Bucket(key, from, to, docCount, aggregations, keyed, format); } @SuppressWarnings("unchecked") From b5af95b3e26e78168805f3ffc568974863075c2a Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Fri, 25 May 2018 10:18:38 -0700 Subject: [PATCH 4/5] Pull the stream serialization logic for buckets into the Bucket class. --- .../aggregations/bucket/range/InternalRange.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index 4cc98a87113e0..2a07c68c07034 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -47,7 +47,7 @@ public static class Bucket extends InternalMultiBucketAggregation.InternalBucket protected final double from; protected final double to; private final long docCount; - final InternalAggregations aggregations; + private final InternalAggregations aggregations; private final String key; public Bucket(String key, double from, double to, long docCount, InternalAggregations aggregations, boolean keyed, @@ -168,6 +168,11 @@ private static String generateKey(double from, double to, DocValueFormat format) @Override public void writeTo(StreamOutput out) throws IOException { + out.writeOptionalString(key); + out.writeDouble(from); + out.writeDouble(to); + out.writeVLong(docCount); + aggregations.writeTo(out); } @Override @@ -262,11 +267,7 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeBoolean(keyed); out.writeVInt(ranges.size()); for (B bucket : ranges) { - out.writeOptionalString(((Bucket) bucket).key); - out.writeDouble(bucket.from); - out.writeDouble(bucket.to); - out.writeVLong(((Bucket) bucket).docCount); - bucket.aggregations.writeTo(out); + bucket.writeTo(out); } } From 7b58a8ad42b77a87b3e4baf0ded5a9c4505e351f Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Thu, 24 May 2018 22:06:27 -0700 Subject: [PATCH 5/5] Serialize bucket keys as strings as opposed to optional strings. --- .../aggregations/bucket/range/InternalRange.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java index 2a07c68c07034..2bc63b377a1bf 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.search.aggregations.bucket.range; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -168,7 +169,11 @@ private static String generateKey(double from, double to, DocValueFormat format) @Override public void writeTo(StreamOutput out) throws IOException { - out.writeOptionalString(key); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeString(key); + } else { + out.writeOptionalString(key); + } out.writeDouble(from); out.writeDouble(to); out.writeVLong(docCount); @@ -254,7 +259,9 @@ public InternalRange(StreamInput in) throws IOException { int size = in.readVInt(); List ranges = new ArrayList<>(size); for (int i = 0; i < size; i++) { - String key = in.readOptionalString(); + String key = in.getVersion().onOrAfter(Version.V_7_0_0_alpha1) + ? in.readString() + : in.readOptionalString(); ranges.add(getFactory().createBucket(key, in.readDouble(), in.readDouble(), in.readVLong(), InternalAggregations.readAggregations(in), keyed, format)); }