From 5ab5cc69b8165e5349cc6384842c217223f5e478 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 3 Aug 2016 14:51:50 +0100 Subject: [PATCH] Remove unused rounding code Factor rounding and Interval rounding (the non-date based rounding) was no longer used so it has been removed. Offset rounding has been retained for no since both date based rounding classes rely on it --- .../common/rounding/Rounding.java | 150 ------------------ .../common/rounding/TimeZoneRounding.java | 23 ++- .../bucket/histogram/DateHistogramParser.java | 17 +- ...ingTests.java => OffsetRoundingTests.java} | 33 +--- 4 files changed, 22 insertions(+), 201 deletions(-) rename core/src/test/java/org/elasticsearch/common/rounding/{RoundingTests.java => OffsetRoundingTests.java} (70%) diff --git a/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java b/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java index 5633cf9f213..4ddba09cfab 100644 --- a/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java +++ b/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.rounding; import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; @@ -54,157 +53,10 @@ public abstract class Rounding implements Streamable { @Override public abstract int hashCode(); - /** - * Rounding strategy which is based on an interval - * - * {@code rounded = value - (value % interval) } - */ - public static class Interval extends Rounding { - - static final byte ID = 0; - - public static final ParseField INTERVAL_FIELD = new ParseField("interval"); - - private long interval; - - public Interval() { // for serialization - } - - /** - * Creates a new interval rounding. - * - * @param interval The interval - */ - public Interval(long interval) { - this.interval = interval; - } - - @Override - public byte id() { - return ID; - } - - public static long roundKey(long value, long interval) { - if (value < 0) { - return (value - interval + 1) / interval; - } else { - return value / interval; - } - } - - public static long roundValue(long key, long interval) { - return key * interval; - } - - @Override - public long round(long value) { - return roundKey(value, interval) * interval; - } - - @Override - public long nextRoundingValue(long value) { - assert value == round(value); - return value + interval; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - interval = in.readVLong(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVLong(interval); - } - - @Override - public int hashCode() { - return Objects.hash(interval); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - Interval other = (Interval) obj; - return Objects.equals(interval, other.interval); - } - } - - public static class FactorRounding extends Rounding { - - static final byte ID = 7; - - public static final ParseField FACTOR_FIELD = new ParseField("factor"); - - private Rounding rounding; - - private float factor; - - FactorRounding() { // for serialization - } - - FactorRounding(Rounding rounding, float factor) { - this.rounding = rounding; - this.factor = factor; - } - - @Override - public byte id() { - return ID; - } - - @Override - public long round(long utcMillis) { - return rounding.round((long) (factor * utcMillis)); - } - - @Override - public long nextRoundingValue(long value) { - return rounding.nextRoundingValue(value); - } - - @Override - public void readFrom(StreamInput in) throws IOException { - rounding = Rounding.Streams.read(in); - factor = in.readFloat(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - Rounding.Streams.write(rounding, out); - out.writeFloat(factor); - } - - @Override - public int hashCode() { - return Objects.hash(rounding, factor); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - FactorRounding other = (FactorRounding) obj; - return Objects.equals(rounding, other.rounding) - && Objects.equals(factor, other.factor); - } - } - public static class OffsetRounding extends Rounding { static final byte ID = 8; - public static final ParseField OFFSET_FIELD = new ParseField("offset"); - private Rounding rounding; private long offset; @@ -274,10 +126,8 @@ public abstract class Rounding implements Streamable { Rounding rounding = null; byte id = in.readByte(); switch (id) { - case Interval.ID: rounding = new Interval(); break; case TimeZoneRounding.TimeUnitRounding.ID: rounding = new TimeZoneRounding.TimeUnitRounding(); break; case TimeZoneRounding.TimeIntervalRounding.ID: rounding = new TimeZoneRounding.TimeIntervalRounding(); break; - case TimeZoneRounding.FactorRounding.ID: rounding = new FactorRounding(); break; case OffsetRounding.ID: rounding = new OffsetRounding(); break; default: throw new ElasticsearchException("unknown rounding id [" + id + "]"); } diff --git a/core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java b/core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java index 932afa15b56..5287203df69 100644 --- a/core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java +++ b/core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.rounding; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.unit.TimeValue; @@ -36,8 +35,6 @@ import java.util.Objects; * daylight saving times. */ public abstract class TimeZoneRounding extends Rounding { - public static final ParseField INTERVAL_FIELD = new ParseField("interval"); - public static final ParseField TIME_ZONE_FIELD = new ParseField("time_zone"); public static Builder builder(DateTimeUnit unit) { return new Builder(unit); @@ -54,8 +51,6 @@ public abstract class TimeZoneRounding extends Rounding { private DateTimeZone timeZone = DateTimeZone.UTC; - private float factor = 1.0f; - private long offset; public Builder(DateTimeUnit unit) { @@ -83,11 +78,6 @@ public abstract class TimeZoneRounding extends Rounding { return this; } - public Builder factor(float factor) { - this.factor = factor; - return this; - } - public Rounding build() { Rounding timeZoneRounding; if (unit != null) { @@ -98,9 +88,6 @@ public abstract class TimeZoneRounding extends Rounding { if (offset != 0) { timeZoneRounding = new OffsetRounding(timeZoneRounding, offset); } - if (factor != 1.0f) { - timeZoneRounding = new FactorRounding(timeZoneRounding, factor); - } return timeZoneRounding; } } @@ -215,7 +202,7 @@ public abstract class TimeZoneRounding extends Rounding { @Override public long round(long utcMillis) { long timeLocal = timeZone.convertUTCToLocal(utcMillis); - long rounded = Rounding.Interval.roundValue(Rounding.Interval.roundKey(timeLocal, interval), interval); + long rounded = roundKey(timeLocal, interval) * interval; long roundedUTC; if (isInDSTGap(rounded) == false) { roundedUTC = timeZone.convertLocalToUTC(rounded, true, utcMillis); @@ -238,6 +225,14 @@ public abstract class TimeZoneRounding extends Rounding { return roundedUTC; } + private static long roundKey(long value, long interval) { + if (value < 0) { + return (value - interval + 1) / interval; + } else { + return value / interval; + } + } + /** * Determine whether the local instant is a valid instant in the given * time zone. The logic for this is taken from diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java index f139ad18bb0..e3a3ea75762 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.aggregations.bucket.histogram; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.rounding.Rounding; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.search.aggregations.support.AbstractValuesSourceParser.NumericValuesSourceParser; @@ -45,7 +44,7 @@ public class DateHistogramParser extends NumericValuesSourceParser { protected DateHistogramAggregationBuilder createFactory(String aggregationName, ValuesSourceType valuesSourceType, ValueType targetValueType, Map otherOptions) { DateHistogramAggregationBuilder factory = new DateHistogramAggregationBuilder(aggregationName); - Object interval = otherOptions.get(Rounding.Interval.INTERVAL_FIELD); + Object interval = otherOptions.get(Histogram.INTERVAL_FIELD); if (interval == null) { throw new ParsingException(null, "Missing required field [interval] for histogram aggregation [" + aggregationName + "]"); } else if (interval instanceof Long) { @@ -55,7 +54,7 @@ public class DateHistogramParser extends NumericValuesSourceParser { } else { throw new IllegalStateException("Unexpected interval class: " + interval.getClass()); } - Long offset = (Long) otherOptions.get(Rounding.OffsetRounding.OFFSET_FIELD); + Long offset = (Long) otherOptions.get(Histogram.OFFSET_FIELD); if (offset != null) { factory.offset(offset); } @@ -83,12 +82,12 @@ public class DateHistogramParser extends NumericValuesSourceParser { protected boolean token(String aggregationName, String currentFieldName, Token token, XContentParser parser, ParseFieldMatcher parseFieldMatcher, Map otherOptions) throws IOException { if (token.isValue()) { - if (parseFieldMatcher.match(currentFieldName, Rounding.Interval.INTERVAL_FIELD)) { + if (parseFieldMatcher.match(currentFieldName, Histogram.INTERVAL_FIELD)) { if (token == XContentParser.Token.VALUE_STRING) { - otherOptions.put(Rounding.Interval.INTERVAL_FIELD, new DateHistogramInterval(parser.text())); + otherOptions.put(Histogram.INTERVAL_FIELD, new DateHistogramInterval(parser.text())); return true; } else { - otherOptions.put(Rounding.Interval.INTERVAL_FIELD, parser.longValue()); + otherOptions.put(Histogram.INTERVAL_FIELD, parser.longValue()); return true; } } else if (parseFieldMatcher.match(currentFieldName, Histogram.MIN_DOC_COUNT_FIELD)) { @@ -97,13 +96,13 @@ public class DateHistogramParser extends NumericValuesSourceParser { } else if (parseFieldMatcher.match(currentFieldName, Histogram.KEYED_FIELD)) { otherOptions.put(Histogram.KEYED_FIELD, parser.booleanValue()); return true; - } else if (parseFieldMatcher.match(currentFieldName, Rounding.OffsetRounding.OFFSET_FIELD)) { + } else if (parseFieldMatcher.match(currentFieldName, Histogram.OFFSET_FIELD)) { if (token == XContentParser.Token.VALUE_STRING) { - otherOptions.put(Rounding.OffsetRounding.OFFSET_FIELD, + otherOptions.put(Histogram.OFFSET_FIELD, DateHistogramAggregationBuilder.parseStringOffset(parser.text())); return true; } else { - otherOptions.put(Rounding.OffsetRounding.OFFSET_FIELD, parser.longValue()); + otherOptions.put(Histogram.OFFSET_FIELD, parser.longValue()); return true; } } else { diff --git a/core/src/test/java/org/elasticsearch/common/rounding/RoundingTests.java b/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java similarity index 70% rename from core/src/test/java/org/elasticsearch/common/rounding/RoundingTests.java rename to core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java index a71cc77ffc1..a601bd140e8 100644 --- a/core/src/test/java/org/elasticsearch/common/rounding/RoundingTests.java +++ b/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java @@ -20,37 +20,13 @@ package org.elasticsearch.common.rounding; import org.elasticsearch.test.ESTestCase; +import org.joda.time.DateTimeZone; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.lessThanOrEqualTo; -public class RoundingTests extends ESTestCase { - /** - * simple test case to illustrate how Rounding.Interval works on readable input - */ - public void testInterval() { - int interval = 10; - Rounding.Interval rounding = new Rounding.Interval(interval); - int value = 24; - final long r = rounding.round(24); - String message = "round(" + value + ", interval=" + interval + ") = " + r; - assertEquals(value/interval * interval, r); - assertEquals(message, 0, r % interval); - } - - public void testIntervalRandom() { - final long interval = randomIntBetween(1, 100); - Rounding.Interval rounding = new Rounding.Interval(interval); - for (int i = 0; i < 1000; ++i) { - long l = Math.max(randomLong(), Long.MIN_VALUE + interval); - final long r = rounding.round(l); - String message = "round(" + l + ", interval=" + interval + ") = " + r; - assertEquals(message, 0, r % interval); - assertThat(message, r, lessThanOrEqualTo(l)); - assertThat(message, r + interval, greaterThan(l)); - } - } +public class OffsetRoundingTests extends ESTestCase { /** * Simple test case to illustrate how Rounding.Offset works on readable input. @@ -60,7 +36,8 @@ public class RoundingTests extends ESTestCase { public void testOffsetRounding() { final long interval = 10; final long offset = 7; - Rounding.OffsetRounding rounding = new Rounding.OffsetRounding(new Rounding.Interval(interval), offset); + Rounding.OffsetRounding rounding = new Rounding.OffsetRounding( + new TimeZoneRounding.TimeIntervalRounding(interval, DateTimeZone.UTC), offset); assertEquals(-3, rounding.round(6)); assertEquals(7, rounding.nextRoundingValue(-3)); assertEquals(7, rounding.round(7)); @@ -76,7 +53,7 @@ public class RoundingTests extends ESTestCase { public void testOffsetRoundingRandom() { for (int i = 0; i < 1000; ++i) { final long interval = randomIntBetween(1, 100); - Rounding.Interval internalRounding = new Rounding.Interval(interval); + Rounding internalRounding = new TimeZoneRounding.TimeIntervalRounding(interval, DateTimeZone.UTC); final long offset = randomIntBetween(-100, 100); Rounding.OffsetRounding rounding = new Rounding.OffsetRounding(internalRounding, offset); long safetyMargin = Math.abs(interval) + Math.abs(offset); // to prevent range overflow