From 476d57150a3d669af21ee3974605094359200d2b Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Wed, 20 Apr 2016 13:32:12 -0400 Subject: [PATCH] Remove readFrom from org.elasticsearch.search Replace with a constructor that takes StreamInput or a static method. In one case (ValuesSourceType) we no longer need to serialize the data at all! Relates to #17085 --- .../resources/checkstyle_suppressions.xml | 1 - .../search/SearchShardTarget.java | 5 - .../search/aggregations/Aggregator.java | 3 +- .../DateHistogramAggregatorBuilder.java | 10 +- .../histogram/DateHistogramInterval.java | 27 +++-- .../bucket/terms/TermsAggregatorBuilder.java | 2 +- .../PercentileRanksAggregatorBuilder.java | 2 +- .../PercentilesAggregatorBuilder.java | 2 +- .../percentiles/PercentilesMethod.java | 3 +- .../support/ValuesSourceType.java | 27 +---- .../search/profile/CollectorResult.java | 30 +++-- .../profile/InternalProfileShardResults.java | 5 - .../search/profile/ProfileResult.java | 43 ++++--- .../search/profile/ProfileShardResult.java | 30 +++-- .../SubAggCollectionModeTests.java | 6 +- .../percentiles/PercentilesMethodTests.java | 6 +- .../support/ValuesSourceTypeTests.java | 109 ------------------ 17 files changed, 76 insertions(+), 235 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceTypeTests.java diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index b25e2a69e66..2f64153a051 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -776,7 +776,6 @@ - diff --git a/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java b/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java index 34ad29a7054..6c2e876f211 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java +++ b/core/src/main/java/org/elasticsearch/search/SearchShardTarget.java @@ -100,11 +100,6 @@ public class SearchShardTarget implements Writeable, Comparab return i; } - @Override - public SearchShardTarget readFrom(StreamInput in) throws IOException { - return new SearchShardTarget(in); - } - @Override public void writeTo(StreamOutput out) throws IOException { if (nodeId == null) { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java index a38a1cd3161..02641207325 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java @@ -139,8 +139,7 @@ public abstract class Aggregator extends BucketCollector implements Releasable { throw new ElasticsearchParseException("no [{}] found for value [{}]", KEY.getPreferredName(), value); } - @Override - public SubAggCollectionMode readFrom(StreamInput in) throws IOException { + public static SubAggCollectionMode readFromStream(StreamInput in) throws IOException { int ordinal = in.readVInt(); if (ordinal < 0 || ordinal >= values().length) { throw new IOException("Unknown SubAggCollectionMode ordinal [" + ordinal + "]"); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorBuilder.java index bf8db6449fb..2442e9f5aab 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorBuilder.java @@ -49,19 +49,13 @@ public class DateHistogramAggregatorBuilder extends AbstractHistogramBuilder { public static final DateHistogramInterval QUARTER = new DateHistogramInterval("1q"); public static final DateHistogramInterval YEAR = new DateHistogramInterval("1y"); - public static final DateHistogramInterval readFromStream(StreamInput in) throws IOException { - return SECOND.readFrom(in); - } - public static DateHistogramInterval seconds(int sec) { return new DateHistogramInterval(sec + "s"); } @@ -70,6 +66,19 @@ public class DateHistogramInterval implements Writeable { this.expression = expression; } + /** + * Read from a stream. + */ + public DateHistogramInterval(StreamInput in) throws IOException { + expression = in.readString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(expression); + } + + @Override public String toString() { return expression; @@ -91,14 +100,4 @@ public class DateHistogramInterval implements Writeable { DateHistogramInterval other = (DateHistogramInterval) obj; return Objects.equals(expression, other.expression); } - - @Override - public DateHistogramInterval readFrom(StreamInput in) throws IOException { - return new DateHistogramInterval(in.readString()); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(expression); - } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorBuilder.java index a905c4f5dac..53887d8b20c 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorBuilder.java @@ -71,7 +71,7 @@ public class TermsAggregatorBuilder extends ValuesSourceAggregatorBuilder { return name; } - @Override - public PercentilesMethod readFrom(StreamInput in) throws IOException { + public static PercentilesMethod readFromStream(StreamInput in) throws IOException { int ordinal = in.readVInt(); if (ordinal < 0 || ordinal >= values().length) { throw new IOException("Unknown PercentilesMethod ordinal [" + ordinal + "]"); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java index 46b56989b28..a6b252d6903 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java @@ -19,34 +19,9 @@ package org.elasticsearch.search.aggregations.support; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; - -import java.io.IOException; - -/* - * The ordinal values for this class are tested in ValuesSourceTypeTests to - * ensure that the ordinal for each value does not change and break bwc - */ -public enum ValuesSourceType implements Writeable { - +public enum ValuesSourceType { ANY, NUMERIC, BYTES, GEOPOINT; - - @Override - public ValuesSourceType readFrom(StreamInput in) throws IOException { - int ordinal = in.readVInt(); - if (ordinal < 0 || ordinal >= values().length) { - throw new IOException("Unknown ValuesSourceType ordinal [" + ordinal + "]"); - } - return values()[ordinal]; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(ordinal()); - } } diff --git a/core/src/main/java/org/elasticsearch/search/profile/CollectorResult.java b/core/src/main/java/org/elasticsearch/search/profile/CollectorResult.java index 8da14d23d96..32b9ccfa942 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/CollectorResult.java +++ b/core/src/main/java/org/elasticsearch/search/profile/CollectorResult.java @@ -80,6 +80,9 @@ public class CollectorResult implements ToXContent, Writeable { this.children = children; } + /** + * Read from a stream. + */ public CollectorResult(StreamInput in) throws IOException { this.collectorName = in.readString(); this.reason = in.readString(); @@ -92,6 +95,17 @@ public class CollectorResult implements ToXContent, Writeable { } } + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(collectorName); + out.writeString(reason); + out.writeLong(time); + out.writeVInt(children.size()); + for (CollectorResult child : children) { + child.writeTo(out); + } + } + /** * @return the profiled time for this collector (inclusive of children) */ @@ -137,20 +151,4 @@ public class CollectorResult implements ToXContent, Writeable { builder = builder.endObject(); return builder; } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(collectorName); - out.writeString(reason); - out.writeLong(time); - out.writeVInt(children.size()); - for (CollectorResult child : children) { - child.writeTo(out); - } - } - - @Override - public Object readFrom(StreamInput in) throws IOException { - return new CollectorResult(in); - } } diff --git a/core/src/main/java/org/elasticsearch/search/profile/InternalProfileShardResults.java b/core/src/main/java/org/elasticsearch/search/profile/InternalProfileShardResults.java index fe94aea1a0a..20471c2fb62 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/InternalProfileShardResults.java +++ b/core/src/main/java/org/elasticsearch/search/profile/InternalProfileShardResults.java @@ -75,11 +75,6 @@ public final class InternalProfileShardResults implements Writeable, ToXContent { private final long nodeTime; private final List children; - public ProfileResult(String queryType, String luceneDescription, Map timings, List children, long nodeTime) { + public ProfileResult(String queryType, String luceneDescription, Map timings, List children, + long nodeTime) { this.queryType = queryType; this.luceneDescription = luceneDescription; this.timings = timings; @@ -65,6 +66,9 @@ final class ProfileResult implements Writeable, ToXContent { this.nodeTime = nodeTime; } + /** + * Read from a stream. + */ public ProfileResult(StreamInput in) throws IOException{ this.queryType = in.readString(); this.luceneDescription = in.readString(); @@ -84,6 +88,22 @@ final class ProfileResult implements Writeable, ToXContent { } } + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(queryType); + out.writeString(luceneDescription); + out.writeLong(nodeTime); // not Vlong because can be negative + out.writeVInt(timings.size()); + for (Map.Entry entry : timings.entrySet()) { + out.writeString(entry.getKey()); + out.writeLong(entry.getValue()); + } + out.writeVInt(children.size()); + for (ProfileResult child : children) { + child.writeTo(out); + } + } + /** * Retrieve the lucene description of this query (e.g. the "explain" text) */ @@ -121,27 +141,6 @@ final class ProfileResult implements Writeable, ToXContent { return Collections.unmodifiableList(children); } - @Override - public ProfileResult readFrom(StreamInput in) throws IOException { - return new ProfileResult(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(queryType); - out.writeString(luceneDescription); - out.writeLong(nodeTime); // not Vlong because can be negative - out.writeVInt(timings.size()); - for (Map.Entry entry : timings.entrySet()) { - out.writeString(entry.getKey()); - out.writeLong(entry.getValue()); - } - out.writeVInt(children.size()); - for (ProfileResult child : children) { - child.writeTo(out); - } - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder = builder.startObject() diff --git a/core/src/main/java/org/elasticsearch/search/profile/ProfileShardResult.java b/core/src/main/java/org/elasticsearch/search/profile/ProfileShardResult.java index c472aa791cf..06fe2164a1d 100644 --- a/core/src/main/java/org/elasticsearch/search/profile/ProfileShardResult.java +++ b/core/src/main/java/org/elasticsearch/search/profile/ProfileShardResult.java @@ -50,6 +50,9 @@ public final class ProfileShardResult implements Writeable, this.rewriteTime = rewriteTime; } + /** + * Read from a stream. + */ public ProfileShardResult(StreamInput in) throws IOException { int profileSize = in.readVInt(); profileResults = new ArrayList<>(profileSize); @@ -61,6 +64,17 @@ public final class ProfileShardResult implements Writeable, rewriteTime = in.readLong(); } + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVInt(profileResults.size()); + for (ProfileResult p : profileResults) { + p.writeTo(out); + } + profileCollector.writeTo(out); + out.writeLong(rewriteTime); + } + + public List getQueryResults() { return Collections.unmodifiableList(profileResults); } @@ -86,20 +100,4 @@ public final class ProfileShardResult implements Writeable, builder.endArray(); return builder; } - - @Override - public ProfileShardResult readFrom(StreamInput in) throws IOException { - return new ProfileShardResult(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVInt(profileResults.size()); - for (ProfileResult p : profileResults) { - p.writeTo(out); - } - profileCollector.writeTo(out); - out.writeLong(rewriteTime); - } - } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/SubAggCollectionModeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/SubAggCollectionModeTests.java index 131144dc5dd..5ed3fe39970 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/SubAggCollectionModeTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/SubAggCollectionModeTests.java @@ -56,13 +56,13 @@ public class SubAggCollectionModeTests extends ESTestCase { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(0); try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(SubAggCollectionMode.BREADTH_FIRST.readFrom(in), equalTo(SubAggCollectionMode.DEPTH_FIRST)); + assertThat(SubAggCollectionMode.readFromStream(in), equalTo(SubAggCollectionMode.DEPTH_FIRST)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(1); try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(SubAggCollectionMode.BREADTH_FIRST.readFrom(in), equalTo(SubAggCollectionMode.BREADTH_FIRST)); + assertThat(SubAggCollectionMode.readFromStream(in), equalTo(SubAggCollectionMode.BREADTH_FIRST)); } } } @@ -71,7 +71,7 @@ public class SubAggCollectionModeTests extends ESTestCase { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(randomIntBetween(2, Integer.MAX_VALUE)); try (StreamInput in = StreamInput.wrap(out.bytes())) { - SubAggCollectionMode.BREADTH_FIRST.readFrom(in); + SubAggCollectionMode.readFromStream(in); fail("Expected IOException"); } catch(IOException e) { assertThat(e.getMessage(), containsString("Unknown SubAggCollectionMode ordinal [")); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentilesMethodTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentilesMethodTests.java index eb08e6f85a2..36c4caae12d 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentilesMethodTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentilesMethodTests.java @@ -55,13 +55,13 @@ public class PercentilesMethodTests extends ESTestCase { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(0); try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(PercentilesMethod.TDIGEST.readFrom(in), equalTo(PercentilesMethod.TDIGEST)); + assertThat(PercentilesMethod.readFromStream(in), equalTo(PercentilesMethod.TDIGEST)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(1); try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(PercentilesMethod.TDIGEST.readFrom(in), equalTo(PercentilesMethod.HDR)); + assertThat(PercentilesMethod.readFromStream(in), equalTo(PercentilesMethod.HDR)); } } } @@ -70,7 +70,7 @@ public class PercentilesMethodTests extends ESTestCase { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(randomIntBetween(2, Integer.MAX_VALUE)); try (StreamInput in = StreamInput.wrap(out.bytes())) { - PercentilesMethod.TDIGEST.readFrom(in); + PercentilesMethod.readFromStream(in); fail("Expected IOException"); } catch(IOException e) { assertThat(e.getMessage(), containsString("Unknown PercentilesMethod ordinal [")); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceTypeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceTypeTests.java deleted file mode 100644 index a297181799e..00000000000 --- a/core/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceTypeTests.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.search.aggregations.support; - -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.test.ESTestCase; - -import java.io.IOException; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; - -public class ValuesSourceTypeTests extends ESTestCase { - - public void testValidOrdinals() { - assertThat(ValuesSourceType.ANY.ordinal(), equalTo(0)); - assertThat(ValuesSourceType.NUMERIC.ordinal(), equalTo(1)); - assertThat(ValuesSourceType.BYTES.ordinal(), equalTo(2)); - assertThat(ValuesSourceType.GEOPOINT.ordinal(), equalTo(3)); - } - - public void testwriteTo() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - ValuesSourceType.ANY.writeTo(out); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(in.readVInt(), equalTo(0)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - ValuesSourceType.NUMERIC.writeTo(out); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(in.readVInt(), equalTo(1)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - ValuesSourceType.BYTES.writeTo(out); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(in.readVInt(), equalTo(2)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - ValuesSourceType.GEOPOINT.writeTo(out); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(in.readVInt(), equalTo(3)); - } - } - } - - public void testReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(0); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(ValuesSourceType.ANY.readFrom(in), equalTo(ValuesSourceType.ANY)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(1); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(ValuesSourceType.ANY.readFrom(in), equalTo(ValuesSourceType.NUMERIC)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(2); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(ValuesSourceType.ANY.readFrom(in), equalTo(ValuesSourceType.BYTES)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(3); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - assertThat(ValuesSourceType.ANY.readFrom(in), equalTo(ValuesSourceType.GEOPOINT)); - } - } - } - - public void testInvalidReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(randomIntBetween(4, Integer.MAX_VALUE)); - try (StreamInput in = StreamInput.wrap(out.bytes())) { - ValuesSourceType.ANY.readFrom(in); - fail("Expected IOException"); - } catch(IOException e) { - assertThat(e.getMessage(), containsString("Unknown ValuesSourceType ordinal [")); - } - - } - } -}