From 5ab5cc69b8165e5349cc6384842c217223f5e478 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 3 Aug 2016 14:51:50 +0100 Subject: [PATCH 1/3] 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 From c14155e4a850d1e3e39d22d68fa33ad37efbd25b Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 3 Aug 2016 16:05:52 +0100 Subject: [PATCH 2/3] Remove TimeZoneRounding abstraction Because the Rounding class now only deals with date based rounding of values we can remove the TimeZoneRounding abstraction to simplify the code. --- .../common/rounding/Rounding.java | 285 +++++++++++++++- .../common/rounding/TimeZoneRounding.java | 309 ------------------ .../histogram/DateHistogramAggregator.java | 6 +- .../DateHistogramAggregatorFactory.java | 9 +- .../common/rounding/OffsetRoundingTests.java | 4 +- .../rounding/TimeZoneRoundingTests.java | 109 +++--- 6 files changed, 346 insertions(+), 376 deletions(-) delete mode 100644 core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java 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 4ddba09cfab..59acf468b86 100644 --- a/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java +++ b/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java @@ -22,6 +22,10 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; +import org.elasticsearch.common.unit.TimeValue; +import org.joda.time.DateTimeField; +import org.joda.time.DateTimeZone; +import org.joda.time.IllegalInstantException; import java.io.IOException; import java.util.Objects; @@ -53,6 +57,279 @@ public abstract class Rounding implements Streamable { @Override public abstract int hashCode(); + public static Builder builder(DateTimeUnit unit) { + return new Builder(unit); + } + + public static Builder builder(TimeValue interval) { + return new Builder(interval); + } + + public static class Builder { + + private final DateTimeUnit unit; + private final long interval; + + private DateTimeZone timeZone = DateTimeZone.UTC; + + private long offset; + + public Builder(DateTimeUnit unit) { + this.unit = unit; + this.interval = -1; + } + + public Builder(TimeValue interval) { + this.unit = null; + if (interval.millis() < 1) + throw new IllegalArgumentException("Zero or negative time interval not supported"); + this.interval = interval.millis(); + } + + public Builder timeZone(DateTimeZone timeZone) { + if (timeZone == null) { + throw new IllegalArgumentException("Setting null as timezone is not supported"); + } + this.timeZone = timeZone; + return this; + } + + public Builder offset(long offset) { + this.offset = offset; + return this; + } + + public Rounding build() { + Rounding timeZoneRounding; + if (unit != null) { + timeZoneRounding = new TimeUnitRounding(unit, timeZone); + } else { + timeZoneRounding = new TimeIntervalRounding(interval, timeZone); + } + if (offset != 0) { + timeZoneRounding = new OffsetRounding(timeZoneRounding, offset); + } + return timeZoneRounding; + } + } + + static class TimeUnitRounding extends Rounding { + + static final byte ID = 1; + + private DateTimeUnit unit; + private DateTimeField field; + private DateTimeZone timeZone; + + TimeUnitRounding() { // for serialization + } + + TimeUnitRounding(DateTimeUnit unit, DateTimeZone timeZone) { + this.unit = unit; + this.field = unit.field(timeZone); + this.timeZone = timeZone; + } + + @Override + public byte id() { + return ID; + } + + @Override + public long round(long utcMillis) { + long rounded = field.roundFloor(utcMillis); + if (timeZone.isFixed() == false && timeZone.getOffset(utcMillis) != timeZone.getOffset(rounded)) { + // in this case, we crossed a time zone transition. In some edge + // cases this will + // result in a value that is not a rounded value itself. We need + // to round again + // to make sure. This will have no affect in cases where + // 'rounded' was already a proper + // rounded value + rounded = field.roundFloor(rounded); + } + assert rounded == field.roundFloor(rounded); + return rounded; + } + + @Override + public long nextRoundingValue(long utcMillis) { + long floor = round(utcMillis); + // add one unit and round to get to next rounded value + long next = round(field.add(floor, 1)); + if (next == floor) { + // in rare case we need to add more than one unit + next = round(field.add(floor, 2)); + } + return next; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + unit = DateTimeUnit.resolve(in.readByte()); + timeZone = DateTimeZone.forID(in.readString()); + field = unit.field(timeZone); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeByte(unit.id()); + out.writeString(timeZone.getID()); + } + + @Override + public int hashCode() { + return Objects.hash(unit, timeZone); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + TimeUnitRounding other = (TimeUnitRounding) obj; + return Objects.equals(unit, other.unit) && Objects.equals(timeZone, other.timeZone); + } + + @Override + public String toString() { + return "[" + timeZone + "][" + unit + "]"; + } + } + + static class TimeIntervalRounding extends Rounding { + + static final byte ID = 2; + + private long interval; + private DateTimeZone timeZone; + + TimeIntervalRounding() { // for serialization + } + + TimeIntervalRounding(long interval, DateTimeZone timeZone) { + if (interval < 1) + throw new IllegalArgumentException("Zero or negative time interval not supported"); + this.interval = interval; + this.timeZone = timeZone; + } + + @Override + public byte id() { + return ID; + } + + @Override + public long round(long utcMillis) { + long timeLocal = timeZone.convertUTCToLocal(utcMillis); + long rounded = roundKey(timeLocal, interval) * interval; + long roundedUTC; + if (isInDSTGap(rounded) == false) { + roundedUTC = timeZone.convertLocalToUTC(rounded, true, utcMillis); + // check if we crossed DST transition, in this case we want the + // last rounded value before the transition + long transition = timeZone.previousTransition(utcMillis); + if (transition != utcMillis && transition > roundedUTC) { + roundedUTC = round(transition - 1); + } + } else { + /* + * Edge case where the rounded local time is illegal and landed + * in a DST gap. In this case, we choose 1ms tick after the + * transition date. We don't want the transition date itself + * because those dates, when rounded themselves, fall into the + * previous interval. This would violate the invariant that the + * rounding operation should be idempotent. + */ + roundedUTC = timeZone.previousTransition(utcMillis) + 1; + } + 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 + * {@link DateTimeZone#convertLocalToUTC(long, boolean)} for the + * `strict` mode case, but instead of throwing an + * {@link IllegalInstantException}, which is costly, we want to return a + * flag indicating that the value is illegal in that time zone. + */ + private boolean isInDSTGap(long instantLocal) { + if (timeZone.isFixed()) { + return false; + } + // get the offset at instantLocal (first estimate) + int offsetLocal = timeZone.getOffset(instantLocal); + // adjust instantLocal using the estimate and recalc the offset + int offset = timeZone.getOffset(instantLocal - offsetLocal); + // if the offsets differ, we must be near a DST boundary + if (offsetLocal != offset) { + // determine if we are in the DST gap + long nextLocal = timeZone.nextTransition(instantLocal - offsetLocal); + if (nextLocal == (instantLocal - offsetLocal)) { + nextLocal = Long.MAX_VALUE; + } + long nextAdjusted = timeZone.nextTransition(instantLocal - offset); + if (nextAdjusted == (instantLocal - offset)) { + nextAdjusted = Long.MAX_VALUE; + } + if (nextLocal != nextAdjusted) { + // we are in the DST gap + return true; + } + } + return false; + } + + @Override + public long nextRoundingValue(long time) { + long timeLocal = time; + timeLocal = timeZone.convertUTCToLocal(time); + long next = timeLocal + interval; + return timeZone.convertLocalToUTC(next, false); + } + + @Override + public void readFrom(StreamInput in) throws IOException { + interval = in.readVLong(); + timeZone = DateTimeZone.forID(in.readString()); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeVLong(interval); + out.writeString(timeZone.getID()); + } + + @Override + public int hashCode() { + return Objects.hash(interval, timeZone); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + TimeIntervalRounding other = (TimeIntervalRounding) obj; + return Objects.equals(interval, other.interval) && Objects.equals(timeZone, other.timeZone); + } + } + public static class OffsetRounding extends Rounding { static final byte ID = 8; @@ -95,12 +372,12 @@ public abstract class Rounding implements Streamable { Rounding.Streams.write(rounding, out); out.writeLong(offset); } - + @Override public int hashCode() { return Objects.hash(rounding, offset); } - + @Override public boolean equals(Object obj) { if (obj == null) { @@ -126,8 +403,8 @@ public abstract class Rounding implements Streamable { Rounding rounding = null; byte id = in.readByte(); switch (id) { - case TimeZoneRounding.TimeUnitRounding.ID: rounding = new TimeZoneRounding.TimeUnitRounding(); break; - case TimeZoneRounding.TimeIntervalRounding.ID: rounding = new TimeZoneRounding.TimeIntervalRounding(); break; + case TimeUnitRounding.ID: rounding = new TimeUnitRounding(); break; + case TimeIntervalRounding.ID: rounding = new TimeIntervalRounding(); 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 deleted file mode 100644 index 5287203df69..00000000000 --- a/core/src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java +++ /dev/null @@ -1,309 +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.common.rounding; - -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.unit.TimeValue; -import org.joda.time.DateTimeField; -import org.joda.time.DateTimeZone; -import org.joda.time.IllegalInstantException; - -import java.io.IOException; -import java.util.Objects; - -/** - * A rounding strategy for dates. It is typically used to group together dates - * that are part of the same hour/day/month, taking into account time zones and - * daylight saving times. - */ -public abstract class TimeZoneRounding extends Rounding { - - public static Builder builder(DateTimeUnit unit) { - return new Builder(unit); - } - - public static Builder builder(TimeValue interval) { - return new Builder(interval); - } - - public static class Builder { - - private final DateTimeUnit unit; - private final long interval; - - private DateTimeZone timeZone = DateTimeZone.UTC; - - private long offset; - - public Builder(DateTimeUnit unit) { - this.unit = unit; - this.interval = -1; - } - - public Builder(TimeValue interval) { - this.unit = null; - if (interval.millis() < 1) - throw new IllegalArgumentException("Zero or negative time interval not supported"); - this.interval = interval.millis(); - } - - public Builder timeZone(DateTimeZone timeZone) { - if (timeZone == null) { - throw new IllegalArgumentException("Setting null as timezone is not supported"); - } - this.timeZone = timeZone; - return this; - } - - public Builder offset(long offset) { - this.offset = offset; - return this; - } - - public Rounding build() { - Rounding timeZoneRounding; - if (unit != null) { - timeZoneRounding = new TimeUnitRounding(unit, timeZone); - } else { - timeZoneRounding = new TimeIntervalRounding(interval, timeZone); - } - if (offset != 0) { - timeZoneRounding = new OffsetRounding(timeZoneRounding, offset); - } - return timeZoneRounding; - } - } - - static class TimeUnitRounding extends TimeZoneRounding { - - static final byte ID = 1; - - private DateTimeUnit unit; - private DateTimeField field; - private DateTimeZone timeZone; - - TimeUnitRounding() { // for serialization - } - - TimeUnitRounding(DateTimeUnit unit, DateTimeZone timeZone) { - this.unit = unit; - this.field = unit.field(timeZone); - this.timeZone = timeZone; - } - - @Override - public byte id() { - return ID; - } - - @Override - public long round(long utcMillis) { - long rounded = field.roundFloor(utcMillis); - if (timeZone.isFixed() == false && timeZone.getOffset(utcMillis) != timeZone.getOffset(rounded)) { - // in this case, we crossed a time zone transition. In some edge cases this will - // result in a value that is not a rounded value itself. We need to round again - // to make sure. This will have no affect in cases where 'rounded' was already a proper - // rounded value - rounded = field.roundFloor(rounded); - } - assert rounded == field.roundFloor(rounded); - return rounded; - } - - @Override - public long nextRoundingValue(long utcMillis) { - long floor = round(utcMillis); - // add one unit and round to get to next rounded value - long next = round(field.add(floor, 1)); - if (next == floor) { - // in rare case we need to add more than one unit - next = round(field.add(floor, 2)); - } - return next; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - unit = DateTimeUnit.resolve(in.readByte()); - timeZone = DateTimeZone.forID(in.readString()); - field = unit.field(timeZone); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeByte(unit.id()); - out.writeString(timeZone.getID()); - } - - @Override - public int hashCode() { - return Objects.hash(unit, timeZone); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - TimeUnitRounding other = (TimeUnitRounding) obj; - return Objects.equals(unit, other.unit) - && Objects.equals(timeZone, other.timeZone); - } - - @Override - public String toString() { - return "[" + timeZone + "][" + unit +"]"; - } - } - - static class TimeIntervalRounding extends TimeZoneRounding { - - static final byte ID = 2; - - private long interval; - private DateTimeZone timeZone; - - TimeIntervalRounding() { // for serialization - } - - TimeIntervalRounding(long interval, DateTimeZone timeZone) { - if (interval < 1) - throw new IllegalArgumentException("Zero or negative time interval not supported"); - this.interval = interval; - this.timeZone = timeZone; - } - - @Override - public byte id() { - return ID; - } - - @Override - public long round(long utcMillis) { - long timeLocal = timeZone.convertUTCToLocal(utcMillis); - long rounded = roundKey(timeLocal, interval) * interval; - long roundedUTC; - if (isInDSTGap(rounded) == false) { - roundedUTC = timeZone.convertLocalToUTC(rounded, true, utcMillis); - // check if we crossed DST transition, in this case we want the last rounded value before the transition - long transition = timeZone.previousTransition(utcMillis); - if (transition != utcMillis && transition > roundedUTC) { - roundedUTC = round(transition - 1); - } - } else { - /* - * Edge case where the rounded local time is illegal and landed - * in a DST gap. In this case, we choose 1ms tick after the - * transition date. We don't want the transition date itself - * because those dates, when rounded themselves, fall into the - * previous interval. This would violate the invariant that the - * rounding operation should be idempotent. - */ - roundedUTC = timeZone.previousTransition(utcMillis) + 1; - } - 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 - * {@link DateTimeZone#convertLocalToUTC(long, boolean)} for the - * `strict` mode case, but instead of throwing an - * {@link IllegalInstantException}, which is costly, we want to return a - * flag indicating that the value is illegal in that time zone. - */ - private boolean isInDSTGap(long instantLocal) { - if (timeZone.isFixed()) { - return false; - } - // get the offset at instantLocal (first estimate) - int offsetLocal = timeZone.getOffset(instantLocal); - // adjust instantLocal using the estimate and recalc the offset - int offset = timeZone.getOffset(instantLocal - offsetLocal); - // if the offsets differ, we must be near a DST boundary - if (offsetLocal != offset) { - // determine if we are in the DST gap - long nextLocal = timeZone.nextTransition(instantLocal - offsetLocal); - if (nextLocal == (instantLocal - offsetLocal)) { - nextLocal = Long.MAX_VALUE; - } - long nextAdjusted = timeZone.nextTransition(instantLocal - offset); - if (nextAdjusted == (instantLocal - offset)) { - nextAdjusted = Long.MAX_VALUE; - } - if (nextLocal != nextAdjusted) { - // we are in the DST gap - return true; - } - } - return false; - } - - @Override - public long nextRoundingValue(long time) { - long timeLocal = time; - timeLocal = timeZone.convertUTCToLocal(time); - long next = timeLocal + interval; - return timeZone.convertLocalToUTC(next, false); - } - - @Override - public void readFrom(StreamInput in) throws IOException { - interval = in.readVLong(); - timeZone = DateTimeZone.forID(in.readString()); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeVLong(interval); - out.writeString(timeZone.getID()); - } - - @Override - public int hashCode() { - return Objects.hash(interval, timeZone); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - TimeIntervalRounding other = (TimeIntervalRounding) obj; - return Objects.equals(interval, other.interval) - && Objects.equals(timeZone, other.timeZone); - } - } -} diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java index cf8325683e2..8785d53e01e 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java @@ -24,7 +24,6 @@ import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.common.inject.internal.Nullable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.rounding.Rounding; -import org.elasticsearch.common.rounding.TimeZoneRounding; import org.elasticsearch.common.util.LongHash; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.Aggregator; @@ -45,8 +44,9 @@ import java.util.Map; /** * An aggregator for date values. Every date is rounded down using a configured - * {@link TimeZoneRounding}. - * @see TimeZoneRounding + * {@link Rounding}. + * + * @see Rounding */ class DateHistogramAggregator extends BucketsAggregator { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index 17c6d82a9c3..2743989a4b5 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.aggregations.bucket.histogram; import org.elasticsearch.common.rounding.DateTimeUnit; import org.elasticsearch.common.rounding.Rounding; -import org.elasticsearch.common.rounding.TimeZoneRounding; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; @@ -95,19 +94,19 @@ public final class DateHistogramAggregatorFactory } private Rounding createRounding() { - TimeZoneRounding.Builder tzRoundingBuilder; + Rounding.Builder tzRoundingBuilder; if (dateHistogramInterval != null) { DateTimeUnit dateTimeUnit = DATE_FIELD_UNITS.get(dateHistogramInterval.toString()); if (dateTimeUnit != null) { - tzRoundingBuilder = TimeZoneRounding.builder(dateTimeUnit); + tzRoundingBuilder = Rounding.builder(dateTimeUnit); } else { // the interval is a time value? - tzRoundingBuilder = TimeZoneRounding.builder( + tzRoundingBuilder = Rounding.builder( TimeValue.parseTimeValue(dateHistogramInterval.toString(), null, getClass().getSimpleName() + ".interval")); } } else { // the interval is an integer time value in millis? - tzRoundingBuilder = TimeZoneRounding.builder(TimeValue.timeValueMillis(interval)); + tzRoundingBuilder = Rounding.builder(TimeValue.timeValueMillis(interval)); } if (timeZone() != null) { tzRoundingBuilder.timeZone(timeZone()); diff --git a/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java b/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java index a601bd140e8..86e4e3b6cd6 100644 --- a/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java +++ b/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java @@ -37,7 +37,7 @@ public class OffsetRoundingTests extends ESTestCase { final long interval = 10; final long offset = 7; Rounding.OffsetRounding rounding = new Rounding.OffsetRounding( - new TimeZoneRounding.TimeIntervalRounding(interval, DateTimeZone.UTC), offset); + new Rounding.TimeIntervalRounding(interval, DateTimeZone.UTC), offset); assertEquals(-3, rounding.round(6)); assertEquals(7, rounding.nextRoundingValue(-3)); assertEquals(7, rounding.round(7)); @@ -53,7 +53,7 @@ public class OffsetRoundingTests extends ESTestCase { public void testOffsetRoundingRandom() { for (int i = 0; i < 1000; ++i) { final long interval = randomIntBetween(1, 100); - Rounding internalRounding = new TimeZoneRounding.TimeIntervalRounding(interval, DateTimeZone.UTC); + Rounding internalRounding = new Rounding.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 diff --git a/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java b/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java index d4920e9afe8..41f60556d9d 100644 --- a/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java +++ b/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java @@ -20,8 +20,8 @@ package org.elasticsearch.common.rounding; import org.elasticsearch.common.collect.Tuple; -import org.elasticsearch.common.rounding.TimeZoneRounding.TimeIntervalRounding; -import org.elasticsearch.common.rounding.TimeZoneRounding.TimeUnitRounding; +import org.elasticsearch.common.rounding.Rounding.TimeIntervalRounding; +import org.elasticsearch.common.rounding.Rounding.TimeUnitRounding; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Description; @@ -47,29 +47,29 @@ import static org.hamcrest.Matchers.lessThanOrEqualTo; public class TimeZoneRoundingTests extends ESTestCase { public void testUTCTimeUnitRounding() { - Rounding tzRounding = TimeZoneRounding.builder(DateTimeUnit.MONTH_OF_YEAR).build(); + Rounding tzRounding = Rounding.builder(DateTimeUnit.MONTH_OF_YEAR).build(); DateTimeZone tz = DateTimeZone.UTC; assertThat(tzRounding.round(time("2009-02-03T01:01:01")), isDate(time("2009-02-01T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-01T00:00:00.000Z")), isDate(time("2009-03-01T00:00:00.000Z"), tz)); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).build(); + tzRounding = Rounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).build(); assertThat(tzRounding.round(time("2012-01-10T01:01:01")), isDate(time("2012-01-09T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2012-01-09T00:00:00.000Z")), isDate(time("2012-01-16T00:00:00.000Z"), tz)); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).offset(-TimeValue.timeValueHours(24).millis()).build(); + tzRounding = Rounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).offset(-TimeValue.timeValueHours(24).millis()).build(); assertThat(tzRounding.round(time("2012-01-10T01:01:01")), isDate(time("2012-01-08T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2012-01-08T00:00:00.000Z")), isDate(time("2012-01-15T00:00:00.000Z"), tz)); } public void testUTCIntervalRounding() { - Rounding tzRounding = TimeZoneRounding.builder(TimeValue.timeValueHours(12)).build(); + Rounding tzRounding = Rounding.builder(TimeValue.timeValueHours(12)).build(); DateTimeZone tz = DateTimeZone.UTC; assertThat(tzRounding.round(time("2009-02-03T01:01:01")), isDate(time("2009-02-03T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-03T00:00:00.000Z")), isDate(time("2009-02-03T12:00:00.000Z"), tz)); assertThat(tzRounding.round(time("2009-02-03T13:01:01")), isDate(time("2009-02-03T12:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-03T12:00:00.000Z")), isDate(time("2009-02-04T00:00:00.000Z"), tz)); - tzRounding = TimeZoneRounding.builder(TimeValue.timeValueHours(48)).build(); + tzRounding = Rounding.builder(TimeValue.timeValueHours(48)).build(); assertThat(tzRounding.round(time("2009-02-03T01:01:01")), isDate(time("2009-02-03T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-03T00:00:00.000Z")), isDate(time("2009-02-05T00:00:00.000Z"), tz)); assertThat(tzRounding.round(time("2009-02-05T13:01:01")), isDate(time("2009-02-05T00:00:00.000Z"), tz)); @@ -77,11 +77,11 @@ public class TimeZoneRoundingTests extends ESTestCase { } /** - * test TimeIntervalTimeZoneRounding, (interval < 12h) with time zone shift + * test TimeIntervalRounding, (interval < 12h) with time zone shift */ - public void testTimeIntervalTimeZoneRounding() { + public void testTimeIntervalRounding() { DateTimeZone tz = DateTimeZone.forOffsetHours(-1); - Rounding tzRounding = TimeZoneRounding.builder(TimeValue.timeValueHours(6)).timeZone(tz).build(); + Rounding tzRounding = Rounding.builder(TimeValue.timeValueHours(6)).timeZone(tz).build(); assertThat(tzRounding.round(time("2009-02-03T00:01:01")), isDate(time("2009-02-02T19:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-02T19:00:00.000Z")), isDate(time("2009-02-03T01:00:00.000Z"), tz)); @@ -90,11 +90,11 @@ public class TimeZoneRoundingTests extends ESTestCase { } /** - * test DayIntervalTimeZoneRounding, (interval >= 12h) with time zone shift + * test DayIntervalRounding, (interval >= 12h) with time zone shift */ - public void testDayIntervalTimeZoneRounding() { + public void testDayIntervalRounding() { DateTimeZone tz = DateTimeZone.forOffsetHours(-8); - Rounding tzRounding = TimeZoneRounding.builder(TimeValue.timeValueHours(12)).timeZone(tz).build(); + Rounding tzRounding = Rounding.builder(TimeValue.timeValueHours(12)).timeZone(tz).build(); assertThat(tzRounding.round(time("2009-02-03T00:01:01")), isDate(time("2009-02-02T20:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-02T20:00:00.000Z")), isDate(time("2009-02-03T08:00:00.000Z"), tz)); @@ -102,37 +102,37 @@ public class TimeZoneRoundingTests extends ESTestCase { assertThat(tzRounding.nextRoundingValue(time("2009-02-03T08:00:00.000Z")), isDate(time("2009-02-03T20:00:00.000Z"), tz)); } - public void testDayTimeZoneRounding() { + public void testDayRounding() { int timezoneOffset = -2; - Rounding tzRounding = TimeZoneRounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.forOffsetHours(timezoneOffset)) + Rounding tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.forOffsetHours(timezoneOffset)) .build(); assertThat(tzRounding.round(0), equalTo(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis())); assertThat(tzRounding.nextRoundingValue(0L - TimeValue.timeValueHours(24 + timezoneOffset).millis()), equalTo(0L - TimeValue .timeValueHours(timezoneOffset).millis())); DateTimeZone tz = DateTimeZone.forID("-08:00"); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); assertThat(tzRounding.round(time("2012-04-01T04:15:30Z")), isDate(time("2012-03-31T08:00:00Z"), tz)); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(tz).build(); assertThat(tzRounding.round(time("2012-04-01T04:15:30Z")), equalTo(time("2012-03-01T08:00:00Z"))); // date in Feb-3rd, but still in Feb-2nd in -02:00 timezone tz = DateTimeZone.forID("-02:00"); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); assertThat(tzRounding.round(time("2009-02-03T01:01:01")), isDate(time("2009-02-02T02:00:00"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-02T02:00:00")), isDate(time("2009-02-03T02:00:00"), tz)); // date in Feb-3rd, also in -02:00 timezone - tzRounding = TimeZoneRounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); assertThat(tzRounding.round(time("2009-02-03T02:01:01")), isDate(time("2009-02-03T02:00:00"), tz)); assertThat(tzRounding.nextRoundingValue(time("2009-02-03T02:00:00")), isDate(time("2009-02-04T02:00:00"), tz)); } - public void testTimeTimeZoneRounding() { + public void testTimeRounding() { // hour unit DateTimeZone tz = DateTimeZone.forOffsetHours(-2); - Rounding tzRounding = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(tz).build(); + Rounding tzRounding = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(tz).build(); assertThat(tzRounding.round(0), equalTo(0L)); assertThat(tzRounding.nextRoundingValue(0L), equalTo(TimeValue.timeValueHours(1L).getMillis())); @@ -144,23 +144,23 @@ public class TimeZoneRoundingTests extends ESTestCase { Rounding tzRounding; // testing savings to non savings switch DateTimeZone cet = DateTimeZone.forID("CET"); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(cet).build(); + tzRounding = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(cet).build(); assertThat(tzRounding.round(time("2014-10-26T01:01:01", cet)), isDate(time("2014-10-26T01:00:00+02:00"), cet)); assertThat(tzRounding.nextRoundingValue(time("2014-10-26T01:00:00", cet)),isDate(time("2014-10-26T02:00:00+02:00"), cet)); assertThat(tzRounding.nextRoundingValue(time("2014-10-26T02:00:00", cet)), isDate(time("2014-10-26T02:00:00+01:00"), cet)); // testing non savings to savings switch - tzRounding = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(cet).build(); + tzRounding = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(cet).build(); assertThat(tzRounding.round(time("2014-03-30T01:01:01", cet)), isDate(time("2014-03-30T01:00:00+01:00"), cet)); assertThat(tzRounding.nextRoundingValue(time("2014-03-30T01:00:00", cet)), isDate(time("2014-03-30T03:00:00", cet), cet)); assertThat(tzRounding.nextRoundingValue(time("2014-03-30T03:00:00", cet)), isDate(time("2014-03-30T04:00:00", cet), cet)); // testing non savings to savings switch (America/Chicago) DateTimeZone chg = DateTimeZone.forID("America/Chicago"); - Rounding tzRounding_utc = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(DateTimeZone.UTC).build(); + Rounding tzRounding_utc = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(DateTimeZone.UTC).build(); assertThat(tzRounding.round(time("2014-03-09T03:01:01", chg)), isDate(time("2014-03-09T03:00:00", chg), chg)); - Rounding tzRounding_chg = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(chg).build(); + Rounding tzRounding_chg = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(chg).build(); assertThat(tzRounding_chg.round(time("2014-03-09T03:01:01", chg)), isDate(time("2014-03-09T03:00:00", chg), chg)); // testing savings to non savings switch 2013 (America/Chicago) @@ -173,18 +173,21 @@ public class TimeZoneRoundingTests extends ESTestCase { } /** - * Randomized test on TimeUnitRounding. - * Test uses random {@link DateTimeUnit} and {@link DateTimeZone} and often (50% of the time) chooses - * test dates that are exactly on or close to offset changes (e.g. DST) in the chosen time zone. + * Randomized test on TimeUnitRounding. Test uses random + * {@link DateTimeUnit} and {@link DateTimeZone} and often (50% of the time) + * chooses test dates that are exactly on or close to offset changes (e.g. + * DST) in the chosen time zone. * - * It rounds the test date down and up and performs various checks on the rounding unit interval that is - * defined by this. Assumptions tested are described in {@link #assertInterval(long, long, long, TimeZoneRounding, DateTimeZone)} + * It rounds the test date down and up and performs various checks on the + * rounding unit interval that is defined by this. Assumptions tested are + * described in + * {@link #assertInterval(long, long, long, Rounding, DateTimeZone)} */ - public void testTimeZoneRoundingRandom() { + public void testRoundingRandom() { for (int i = 0; i < 1000; ++i) { DateTimeUnit timeUnit = randomTimeUnit(); DateTimeZone tz = randomDateTimeZone(); - TimeZoneRounding rounding = new TimeZoneRounding.TimeUnitRounding(timeUnit, tz); + Rounding rounding = new Rounding.TimeUnitRounding(timeUnit, tz); long date = Math.abs(randomLong() % (2 * (long) 10e11)); // 1970-01-01T00:00:00Z - 2033-05-18T05:33:20.000+02:00 long unitMillis = timeUnit.field(tz).getDurationField().getUnitMillis(); if (randomBoolean()) { @@ -226,7 +229,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testTimeIntervalCET_DST_End() { long interval = TimeUnit.MINUTES.toMillis(20); DateTimeZone tz = DateTimeZone.forID("CET"); - TimeZoneRounding rounding = new TimeIntervalRounding(interval, tz); + Rounding rounding = new TimeIntervalRounding(interval, tz); assertThat(rounding.round(time("2015-10-25T01:55:00+02:00")), isDate(time("2015-10-25T01:40:00+02:00"), tz)); assertThat(rounding.round(time("2015-10-25T02:15:00+02:00")), isDate(time("2015-10-25T02:00:00+02:00"), tz)); @@ -246,7 +249,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testTimeIntervalCET_DST_Start() { long interval = TimeUnit.MINUTES.toMillis(20); DateTimeZone tz = DateTimeZone.forID("CET"); - TimeZoneRounding rounding = new TimeIntervalRounding(interval, tz); + Rounding rounding = new TimeIntervalRounding(interval, tz); // test DST start assertThat(rounding.round(time("2016-03-27T01:55:00+01:00")), isDate(time("2016-03-27T01:40:00+01:00"), tz)); assertThat(rounding.round(time("2016-03-27T02:00:00+01:00")), isDate(time("2016-03-27T03:00:00+02:00"), tz)); @@ -263,7 +266,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testTimeInterval_Kathmandu_DST_Start() { long interval = TimeUnit.MINUTES.toMillis(20); DateTimeZone tz = DateTimeZone.forID("Asia/Kathmandu"); - TimeZoneRounding rounding = new TimeIntervalRounding(interval, tz); + Rounding rounding = new TimeIntervalRounding(interval, tz); assertThat(rounding.round(time("1985-12-31T23:55:00+05:30")), isDate(time("1985-12-31T23:40:00+05:30"), tz)); assertThat(rounding.round(time("1986-01-01T00:16:00+05:45")), isDate(time("1986-01-01T00:15:00+05:45"), tz)); assertThat(time("1986-01-01T00:15:00+05:45") - time("1985-12-31T23:40:00+05:30"), equalTo(TimeUnit.MINUTES.toMillis(20))); @@ -281,7 +284,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testIntervalRounding_NotDivisibleInteval() { DateTimeZone tz = DateTimeZone.forID("CET"); long interval = TimeUnit.MINUTES.toMillis(14); - TimeZoneRounding rounding = new TimeZoneRounding.TimeIntervalRounding(interval, tz); + Rounding rounding = new Rounding.TimeIntervalRounding(interval, tz); assertThat(rounding.round(time("2016-03-27T01:41:00+01:00")), isDate(time("2016-03-27T01:30:00+01:00"), tz)); assertThat(rounding.round(time("2016-03-27T01:51:00+01:00")), isDate(time("2016-03-27T01:44:00+01:00"), tz)); @@ -298,7 +301,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testIntervalRounding_HalfDay_DST() { DateTimeZone tz = DateTimeZone.forID("CET"); long interval = TimeUnit.HOURS.toMillis(12); - TimeZoneRounding rounding = new TimeZoneRounding.TimeIntervalRounding(interval, tz); + Rounding rounding = new Rounding.TimeIntervalRounding(interval, tz); assertThat(rounding.round(time("2016-03-26T01:00:00+01:00")), isDate(time("2016-03-26T00:00:00+01:00"), tz)); assertThat(rounding.round(time("2016-03-26T13:00:00+01:00")), isDate(time("2016-03-26T12:00:00+01:00"), tz)); @@ -316,7 +319,7 @@ public class TimeZoneRoundingTests extends ESTestCase { TimeUnit unit = randomFrom(new TimeUnit[] {TimeUnit.MINUTES, TimeUnit.HOURS, TimeUnit.DAYS}); long interval = unit.toMillis(randomIntBetween(1, 365)); DateTimeZone tz = randomDateTimeZone(); - TimeZoneRounding rounding = new TimeZoneRounding.TimeIntervalRounding(interval, tz); + Rounding rounding = new Rounding.TimeIntervalRounding(interval, tz); long mainDate = Math.abs(randomLong() % (2 * (long) 10e11)); // 1970-01-01T00:00:00Z - 2033-05-18T05:33:20.000+02:00 if (randomBoolean()) { mainDate = nastyDate(mainDate, tz, interval); @@ -356,8 +359,8 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testIntervalRoundingMonotonic_CET() { long interval = TimeUnit.MINUTES.toMillis(45); DateTimeZone tz = DateTimeZone.forID("CET"); - TimeZoneRounding rounding = new TimeZoneRounding.TimeIntervalRounding(interval, tz); - List> expectedDates = new ArrayList>(); + Rounding rounding = new Rounding.TimeIntervalRounding(interval, tz); + List> expectedDates = new ArrayList<>(); // first date is the date to be rounded, second the expected result expectedDates.add(new Tuple<>("2011-10-30T01:40:00.000+02:00", "2011-10-30T01:30:00.000+02:00")); expectedDates.add(new Tuple<>("2011-10-30T02:02:30.000+02:00", "2011-10-30T01:30:00.000+02:00")); @@ -387,7 +390,7 @@ public class TimeZoneRoundingTests extends ESTestCase { public void testAmbiguousHoursAfterDSTSwitch() { Rounding tzRounding; final DateTimeZone tz = DateTimeZone.forID("Asia/Jerusalem"); - tzRounding = TimeZoneRounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(tz).build(); assertThat(tzRounding.round(time("2014-10-26T00:30:00+03:00")), isDate(time("2014-10-26T00:00:00+03:00"), tz)); assertThat(tzRounding.round(time("2014-10-26T01:30:00+03:00")), isDate(time("2014-10-26T01:00:00+03:00"), tz)); // the utc date for "2014-10-25T03:00:00+03:00" and "2014-10-25T03:00:00+02:00" is the same, local time turns back 1h here @@ -396,7 +399,7 @@ public class TimeZoneRoundingTests extends ESTestCase { assertThat(tzRounding.round(time("2014-10-26T02:30:00+02:00")), isDate(time("2014-10-26T02:00:00+02:00"), tz)); // Day interval - tzRounding = TimeZoneRounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(tz).build(); assertThat(tzRounding.round(time("2014-11-11T17:00:00", tz)), isDate(time("2014-11-11T00:00:00", tz), tz)); // DST on assertThat(tzRounding.round(time("2014-08-11T17:00:00", tz)), isDate(time("2014-08-11T00:00:00", tz), tz)); @@ -406,17 +409,17 @@ public class TimeZoneRoundingTests extends ESTestCase { assertThat(tzRounding.round(time("2015-03-27T17:00:00", tz)), isDate(time("2015-03-27T00:00:00", tz), tz)); // Month interval - tzRounding = TimeZoneRounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(tz).build(); assertThat(tzRounding.round(time("2014-11-11T17:00:00", tz)), isDate(time("2014-11-01T00:00:00", tz), tz)); // DST on assertThat(tzRounding.round(time("2014-10-10T17:00:00", tz)), isDate(time("2014-10-01T00:00:00", tz), tz)); // Year interval - tzRounding = TimeZoneRounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(tz).build(); assertThat(tzRounding.round(time("2014-11-11T17:00:00", tz)), isDate(time("2014-01-01T00:00:00", tz), tz)); // Two timestamps in same year and different timezone offset ("Double buckets" issue - #9491) - tzRounding = TimeZoneRounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(tz).build(); + tzRounding = Rounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(tz).build(); assertThat(tzRounding.round(time("2014-11-11T17:00:00", tz)), isDate(tzRounding.round(time("2014-08-11T17:00:00", tz)), tz)); } @@ -429,8 +432,8 @@ public class TimeZoneRoundingTests extends ESTestCase { DateTimeZone tz = DateTimeZone.forID("America/Sao_Paulo"); long start = time("2014-10-18T20:50:00.000", tz); long end = time("2014-10-19T01:00:00.000", tz); - Rounding tzRounding = new TimeZoneRounding.TimeUnitRounding(DateTimeUnit.MINUTES_OF_HOUR, tz); - Rounding dayTzRounding = new TimeZoneRounding.TimeIntervalRounding(60000, tz); + Rounding tzRounding = new Rounding.TimeUnitRounding(DateTimeUnit.MINUTES_OF_HOUR, tz); + Rounding dayTzRounding = new Rounding.TimeIntervalRounding(60000, tz); for (long time = start; time < end; time = time + 60000) { assertThat(tzRounding.nextRoundingValue(time), greaterThan(time)); assertThat(dayTzRounding.nextRoundingValue(time), greaterThan(time)); @@ -442,7 +445,7 @@ public class TimeZoneRoundingTests extends ESTestCase { // standard +/-1 hour DST transition, CET DateTimeUnit timeUnit = DateTimeUnit.HOUR_OF_DAY; DateTimeZone tz = DateTimeZone.forID("CET"); - TimeZoneRounding rounding = new TimeZoneRounding.TimeUnitRounding(timeUnit, tz); + Rounding rounding = new Rounding.TimeUnitRounding(timeUnit, tz); // 29 Mar 2015 - Daylight Saving Time Started // at 02:00:00 clocks were turned forward 1 hour to 03:00:00 @@ -466,7 +469,7 @@ public class TimeZoneRoundingTests extends ESTestCase { // which is not a round value for hourly rounding DateTimeUnit timeUnit = DateTimeUnit.HOUR_OF_DAY; DateTimeZone tz = DateTimeZone.forID("Asia/Kathmandu"); - TimeZoneRounding rounding = new TimeZoneRounding.TimeUnitRounding(timeUnit, tz); + Rounding rounding = new Rounding.TimeUnitRounding(timeUnit, tz); assertInterval(time("1985-12-31T22:00:00.000+05:30"), time("1985-12-31T23:00:00.000+05:30"), rounding, 60, tz); assertInterval(time("1985-12-31T23:00:00.000+05:30"), time("1986-01-01T01:00:00.000+05:45"), rounding, 105, tz); @@ -479,7 +482,7 @@ public class TimeZoneRoundingTests extends ESTestCase { // at 02:00:00 clocks were turned backward 0:30 hours to Sunday, 3 March 1991, 01:30:00 DateTimeUnit timeUnit = DateTimeUnit.HOUR_OF_DAY; DateTimeZone tz = DateTimeZone.forID("Australia/Lord_Howe"); - TimeZoneRounding rounding = new TimeZoneRounding.TimeUnitRounding(timeUnit, tz); + Rounding rounding = new Rounding.TimeUnitRounding(timeUnit, tz); assertInterval(time("1991-03-03T00:00:00.000+11:00"), time("1991-03-03T01:00:00.000+11:00"), rounding, 60, tz); assertInterval(time("1991-03-03T01:00:00.000+11:00"), time("1991-03-03T02:00:00.000+10:30"), rounding, 90, tz); @@ -499,7 +502,7 @@ public class TimeZoneRoundingTests extends ESTestCase { // at 03:45:00 clocks were turned backward 1 hour to 02:45:00 DateTimeUnit timeUnit = DateTimeUnit.HOUR_OF_DAY; DateTimeZone tz = DateTimeZone.forID("Pacific/Chatham"); - TimeZoneRounding rounding = new TimeZoneRounding.TimeUnitRounding(timeUnit, tz); + Rounding rounding = new Rounding.TimeUnitRounding(timeUnit, tz); assertInterval(time("2015-04-05T02:00:00.000+13:45"), time("2015-04-05T03:00:00.000+13:45"), rounding, 60, tz); assertInterval(time("2015-04-05T03:00:00.000+13:45"), time("2015-04-05T03:00:00.000+12:45"), rounding, 60, tz); @@ -514,7 +517,7 @@ public class TimeZoneRoundingTests extends ESTestCase { } } - private static void assertInterval(long rounded, long nextRoundingValue, TimeZoneRounding rounding, int minutes, + private static void assertInterval(long rounded, long nextRoundingValue, Rounding rounding, int minutes, DateTimeZone tz) { assertInterval(rounded, dateBetween(rounded, nextRoundingValue), nextRoundingValue, rounding, tz); assertEquals(DateTimeConstants.MILLIS_PER_MINUTE * minutes, nextRoundingValue - rounded); @@ -527,7 +530,7 @@ public class TimeZoneRoundingTests extends ESTestCase { * @param nextRoundingValue the expected upper end of the rounding interval * @param rounding the rounding instance */ - private static void assertInterval(long rounded, long unrounded, long nextRoundingValue, TimeZoneRounding rounding, + private static void assertInterval(long rounded, long unrounded, long nextRoundingValue, Rounding rounding, DateTimeZone tz) { assert rounded <= unrounded && unrounded <= nextRoundingValue; assertThat("rounding should be idempotent ", rounding.round(rounded), isDate(rounded, tz)); From b6ef99195d4ac1d2b503e380ed147f8fef5940e3 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 4 Aug 2016 11:36:26 +0100 Subject: [PATCH 3/3] Remove offset rounding This is in favour of doing the offset calculations in the date histogram --- .../common/rounding/Rounding.java | 73 ------------------- .../histogram/DateHistogramAggregator.java | 13 ++-- .../DateHistogramAggregatorFactory.java | 4 +- .../histogram/InternalDateHistogram.java | 23 +++--- .../common/rounding/OffsetRoundingTests.java | 69 ------------------ .../rounding/TimeZoneRoundingTests.java | 4 - 6 files changed, 24 insertions(+), 162 deletions(-) delete mode 100644 core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java 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 59acf468b86..ad9f926e881 100644 --- a/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java +++ b/core/src/main/java/org/elasticsearch/common/rounding/Rounding.java @@ -72,8 +72,6 @@ public abstract class Rounding implements Streamable { private DateTimeZone timeZone = DateTimeZone.UTC; - private long offset; - public Builder(DateTimeUnit unit) { this.unit = unit; this.interval = -1; @@ -94,11 +92,6 @@ public abstract class Rounding implements Streamable { return this; } - public Builder offset(long offset) { - this.offset = offset; - return this; - } - public Rounding build() { Rounding timeZoneRounding; if (unit != null) { @@ -106,9 +99,6 @@ public abstract class Rounding implements Streamable { } else { timeZoneRounding = new TimeIntervalRounding(interval, timeZone); } - if (offset != 0) { - timeZoneRounding = new OffsetRounding(timeZoneRounding, offset); - } return timeZoneRounding; } } @@ -330,68 +320,6 @@ public abstract class Rounding implements Streamable { } } - public static class OffsetRounding extends Rounding { - - static final byte ID = 8; - - private Rounding rounding; - - private long offset; - - OffsetRounding() { // for serialization - } - - public OffsetRounding(Rounding intervalRounding, long offset) { - this.rounding = intervalRounding; - this.offset = offset; - } - - @Override - public byte id() { - return ID; - } - - @Override - public long round(long value) { - return rounding.round(value - offset) + offset; - } - - @Override - public long nextRoundingValue(long value) { - return rounding.nextRoundingValue(value - offset) + offset; - } - - @Override - public void readFrom(StreamInput in) throws IOException { - rounding = Rounding.Streams.read(in); - offset = in.readLong(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - Rounding.Streams.write(rounding, out); - out.writeLong(offset); - } - - @Override - public int hashCode() { - return Objects.hash(rounding, offset); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { - return false; - } - OffsetRounding other = (OffsetRounding) obj; - return Objects.equals(rounding, other.rounding) - && Objects.equals(offset, other.offset); - } - } - public static class Streams { public static void write(Rounding rounding, StreamOutput out) throws IOException { @@ -405,7 +333,6 @@ public abstract class Rounding implements Streamable { switch (id) { case TimeUnitRounding.ID: rounding = new TimeUnitRounding(); break; case TimeIntervalRounding.ID: rounding = new TimeIntervalRounding(); break; - case OffsetRounding.ID: rounding = new OffsetRounding(); break; default: throw new ElasticsearchException("unknown rounding id [" + id + "]"); } rounding.readFrom(in); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java index 8785d53e01e..0ea2fba719b 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java @@ -45,7 +45,7 @@ import java.util.Map; /** * An aggregator for date values. Every date is rounded down using a configured * {@link Rounding}. - * + * * @see Rounding */ class DateHistogramAggregator extends BucketsAggregator { @@ -60,14 +60,17 @@ class DateHistogramAggregator extends BucketsAggregator { private final ExtendedBounds extendedBounds; private final LongHash bucketOrds; + private long offset; - public DateHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, InternalOrder order, boolean keyed, + public DateHistogramAggregator(String name, AggregatorFactories factories, Rounding rounding, long offset, InternalOrder order, + boolean keyed, long minDocCount, @Nullable ExtendedBounds extendedBounds, @Nullable ValuesSource.Numeric valuesSource, DocValueFormat formatter, AggregationContext aggregationContext, Aggregator parent, List pipelineAggregators, Map metaData) throws IOException { super(name, factories, aggregationContext, parent, pipelineAggregators, metaData); this.rounding = rounding; + this.offset = offset; this.order = order; this.keyed = keyed; this.minDocCount = minDocCount; @@ -100,7 +103,7 @@ class DateHistogramAggregator extends BucketsAggregator { long previousRounded = Long.MIN_VALUE; for (int i = 0; i < valuesCount; ++i) { long value = values.valueAt(i); - long rounded = rounding.round(value); + long rounded = rounding.round(value - offset) + offset; assert rounded >= previousRounded; if (rounded == previousRounded) { continue; @@ -133,7 +136,7 @@ class DateHistogramAggregator extends BucketsAggregator { InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, buckets, order, minDocCount, emptyBucketInfo, formatter, keyed, + return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, formatter, keyed, pipelineAggregators(), metaData()); } @@ -142,7 +145,7 @@ class DateHistogramAggregator extends BucketsAggregator { InternalDateHistogram.EmptyBucketInfo emptyBucketInfo = minDocCount == 0 ? new InternalDateHistogram.EmptyBucketInfo(rounding, buildEmptySubAggregations(), extendedBounds) : null; - return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, emptyBucketInfo, formatter, keyed, + return new InternalDateHistogram(name, Collections.emptyList(), order, minDocCount, offset, emptyBucketInfo, formatter, keyed, pipelineAggregators(), metaData()); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index 2743989a4b5..79f81e28374 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -111,7 +111,7 @@ public final class DateHistogramAggregatorFactory if (timeZone() != null) { tzRoundingBuilder.timeZone(timeZone()); } - Rounding rounding = tzRoundingBuilder.offset(offset).build(); + Rounding rounding = tzRoundingBuilder.build(); return rounding; } @@ -137,7 +137,7 @@ public final class DateHistogramAggregatorFactory // parse any string bounds to longs and round them roundedBounds = extendedBounds.parseAndValidate(name, context.searchContext(), config.format()).round(rounding); } - return new DateHistogramAggregator(name, factories, rounding, order, keyed, minDocCount, roundedBounds, valuesSource, + return new DateHistogramAggregator(name, factories, rounding, offset, order, keyed, minDocCount, roundedBounds, valuesSource, config.format(), context, parent, pipelineAggregators, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java index 4d46c2c1850..56d3792e0c6 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java @@ -178,14 +178,17 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< private final DocValueFormat format; private final boolean keyed; private final long minDocCount; + private final long offset; private final EmptyBucketInfo emptyBucketInfo; - InternalDateHistogram(String name, List buckets, InternalOrder order, long minDocCount, EmptyBucketInfo emptyBucketInfo, + InternalDateHistogram(String name, List buckets, InternalOrder order, long minDocCount, long offset, + EmptyBucketInfo emptyBucketInfo, DocValueFormat formatter, boolean keyed, List pipelineAggregators, Map metaData) { super(name, pipelineAggregators, metaData); this.buckets = buckets; this.order = order; + this.offset = offset; assert (minDocCount == 0) == (emptyBucketInfo != null); this.minDocCount = minDocCount; this.emptyBucketInfo = emptyBucketInfo; @@ -205,6 +208,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< } else { emptyBucketInfo = null; } + offset = in.readLong(); format = in.readNamedWriteable(DocValueFormat.class); keyed = in.readBoolean(); buckets = in.readList(stream -> new Bucket(stream, keyed, format)); @@ -217,6 +221,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< if (minDocCount == 0) { emptyBucketInfo.writeTo(out); } + out.writeLong(offset); out.writeNamedWriteable(format); out.writeBoolean(keyed); out.writeList(buckets); @@ -234,7 +239,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< @Override public InternalDateHistogram create(List buckets) { - return new InternalDateHistogram(name, buckets, order, minDocCount, emptyBucketInfo, format, + return new InternalDateHistogram(name, buckets, order, minDocCount, offset, emptyBucketInfo, format, keyed, pipelineAggregators(), metaData); } @@ -328,7 +333,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< long max = bounds.getMax(); while (key <= max) { iter.add(new InternalDateHistogram.Bucket(key, 0, keyed, format, reducedEmptySubAggs)); - key = emptyBucketInfo.rounding.nextRoundingValue(key); + key = nextKey(key).longValue(); } } } else { @@ -337,7 +342,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< if (key < firstBucket.key) { while (key < firstBucket.key) { iter.add(new InternalDateHistogram.Bucket(key, 0, keyed, format, reducedEmptySubAggs)); - key = emptyBucketInfo.rounding.nextRoundingValue(key); + key = nextKey(key).longValue(); } } } @@ -349,10 +354,10 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< while (iter.hasNext()) { Bucket nextBucket = list.get(iter.nextIndex()); if (lastBucket != null) { - long key = emptyBucketInfo.rounding.nextRoundingValue(lastBucket.key); + long key = nextKey(lastBucket.key).longValue(); while (key < nextBucket.key) { iter.add(new InternalDateHistogram.Bucket(key, 0, keyed, format, reducedEmptySubAggs)); - key = emptyBucketInfo.rounding.nextRoundingValue(key); + key = nextKey(key).longValue(); } assert key == nextBucket.key; } @@ -393,7 +398,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< CollectionUtil.introSort(reducedBuckets, order.comparator()); } - return new InternalDateHistogram(getName(), reducedBuckets, order, minDocCount, emptyBucketInfo, + return new InternalDateHistogram(getName(), reducedBuckets, order, minDocCount, offset, emptyBucketInfo, format, keyed, pipelineAggregators(), getMetaData()); } @@ -424,7 +429,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< @Override public Number nextKey(Number key) { - return emptyBucketInfo.rounding.nextRoundingValue(key.longValue()); + return emptyBucketInfo.rounding.nextRoundingValue(key.longValue() - offset) + offset; } @Override @@ -435,7 +440,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< buckets2.add((Bucket) b); } buckets2 = Collections.unmodifiableList(buckets2); - return new InternalDateHistogram(name, buckets2, order, minDocCount, emptyBucketInfo, format, + return new InternalDateHistogram(name, buckets2, order, minDocCount, offset, emptyBucketInfo, format, keyed, pipelineAggregators(), getMetaData()); } diff --git a/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java b/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java deleted file mode 100644 index 86e4e3b6cd6..00000000000 --- a/core/src/test/java/org/elasticsearch/common/rounding/OffsetRoundingTests.java +++ /dev/null @@ -1,69 +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.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 OffsetRoundingTests extends ESTestCase { - - /** - * Simple test case to illustrate how Rounding.Offset works on readable input. - * offset shifts input value back before rounding (so here 6 - 7 -> -1) - * then shifts rounded Value back (here -10 -> -3) - */ - public void testOffsetRounding() { - final long interval = 10; - final long offset = 7; - Rounding.OffsetRounding rounding = new Rounding.OffsetRounding( - new Rounding.TimeIntervalRounding(interval, DateTimeZone.UTC), offset); - assertEquals(-3, rounding.round(6)); - assertEquals(7, rounding.nextRoundingValue(-3)); - assertEquals(7, rounding.round(7)); - assertEquals(17, rounding.nextRoundingValue(7)); - assertEquals(7, rounding.round(16)); - assertEquals(17, rounding.round(17)); - assertEquals(27, rounding.nextRoundingValue(17)); - } - - /** - * test OffsetRounding with an internal interval rounding on random inputs - */ - public void testOffsetRoundingRandom() { - for (int i = 0; i < 1000; ++i) { - final long interval = randomIntBetween(1, 100); - Rounding internalRounding = new Rounding.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 - long value = Math.max(randomLong() - safetyMargin, Long.MIN_VALUE + safetyMargin); - final long r_value = rounding.round(value); - final long nextRoundingValue = rounding.nextRoundingValue(r_value); - assertThat("Rounding should be idempotent", r_value, equalTo(rounding.round(r_value))); - assertThat("Rounded value smaller than unrounded, regardless of offset", r_value - offset, lessThanOrEqualTo(value - offset)); - assertThat("Rounded value <= value < next interval start", r_value + interval, greaterThan(value)); - assertThat("NextRounding value should be interval from rounded value", r_value + interval, equalTo(nextRoundingValue)); - } - } -} diff --git a/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java b/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java index 41f60556d9d..ff83ddfa57d 100644 --- a/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java +++ b/core/src/test/java/org/elasticsearch/common/rounding/TimeZoneRoundingTests.java @@ -55,10 +55,6 @@ public class TimeZoneRoundingTests extends ESTestCase { tzRounding = Rounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).build(); assertThat(tzRounding.round(time("2012-01-10T01:01:01")), isDate(time("2012-01-09T00:00:00.000Z"), tz)); assertThat(tzRounding.nextRoundingValue(time("2012-01-09T00:00:00.000Z")), isDate(time("2012-01-16T00:00:00.000Z"), tz)); - - tzRounding = Rounding.builder(DateTimeUnit.WEEK_OF_WEEKYEAR).offset(-TimeValue.timeValueHours(24).millis()).build(); - assertThat(tzRounding.round(time("2012-01-10T01:01:01")), isDate(time("2012-01-08T00:00:00.000Z"), tz)); - assertThat(tzRounding.nextRoundingValue(time("2012-01-08T00:00:00.000Z")), isDate(time("2012-01-15T00:00:00.000Z"), tz)); } public void testUTCIntervalRounding() {