Skip to content

Commit

Permalink
Minor clean-up in InternalRange. (elastic#30886)
Browse files Browse the repository at this point in the history
* Make sure all instance variables are final.
* Make generateKey a private static method, instead of protected.
* Rename formatter -> format for consistency.
* Serialize bucket keys as strings as opposed to optional strings.
* Pull the stream serialization logic for buckets into the Bucket class.
  • Loading branch information
jtibshirani authored May 30, 2018
1 parent 116d083 commit a79c5bd
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ public class InternalDateRange extends InternalRange<InternalDateRange.Bucket, I

public static class Bucket extends InternalRange.Bucket {

public Bucket(boolean keyed, DocValueFormat formatter) {
super(keyed, formatter);
}

public Bucket(String key, double from, double to, long docCount, List<InternalAggregation> aggregations, boolean keyed,
DocValueFormat formatter) {
super(key, from, to, docCount, new InternalAggregations(aggregations), keyed, formatter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ public class InternalGeoDistance extends InternalRange<InternalGeoDistance.Bucke

static class Bucket extends InternalRange.Bucket {

Bucket(boolean keyed) {
super(keyed, DocValueFormat.RAW);
}

Bucket(String key, double from, double to, long docCount, List<InternalAggregation> aggregations, boolean keyed) {
this(key, from, to, docCount, new InternalAggregations(aggregations), keyed);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,21 +45,17 @@ 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;
private 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.key = key != null ? key : generateKey(from, to, formatter);
DocValueFormat format) {
this.keyed = keyed;
this.format = format;
this.key = key != null ? key : generateKey(from, to, format);
this.from = from;
this.to = to;
this.docCount = docCount;
Expand Down Expand Up @@ -162,16 +159,25 @@ 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 format) {
StringBuilder builder = new StringBuilder()
.append(Double.isInfinite(from) ? "*" : format.format(from))
.append("-")
.append(Double.isInfinite(to) ? "*" : format.format(to));
return builder.toString();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
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);
aggregations.writeTo(out);
}

@Override
Expand Down Expand Up @@ -206,15 +212,15 @@ public ValueType getValueType() {
}

@SuppressWarnings("unchecked")
public R create(String name, List<B> ranges, DocValueFormat formatter, boolean keyed, List<PipelineAggregator> pipelineAggregators,
public R create(String name, List<B> ranges, DocValueFormat format, boolean keyed, List<PipelineAggregator> pipelineAggregators,
Map<String, Object> metaData) {
return (R) new InternalRange<B, R>(name, ranges, formatter, keyed, pipelineAggregators, metaData);
return (R) new InternalRange<B, R>(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")
Expand All @@ -230,9 +236,9 @@ public B createBucket(InternalAggregations aggregations, B prototype) {
}
}

private List<B> ranges;
protected DocValueFormat format;
protected boolean keyed;
private final List<B> ranges;
protected final DocValueFormat format;
protected final boolean keyed;

public InternalRange(String name, List<B> ranges, DocValueFormat format, boolean keyed,
List<PipelineAggregator> pipelineAggregators,
Expand All @@ -253,7 +259,9 @@ public InternalRange(StreamInput in) throws IOException {
int size = in.readVInt();
List<B> 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));
}
Expand All @@ -266,11 +274,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);
}
}

Expand Down

0 comments on commit a79c5bd

Please sign in to comment.