Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set start of the week to Monday for root locale #43652

Merged
merged 15 commits into from
Aug 9, 2019
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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.time;

import java.util.Locale;

/**
* Locale constants to be used across elasticsearch code base.
* java.util.Locale.ROOT should not be used as it defaults start of the week incorrectly to Sunday.
*/
public final class IsoLocale {
private IsoLocale() {
throw new UnsupportedOperationException();
}

/**
* We want to use Locale.ROOT but with a start of the week as defined in ISO8601 to be compatible with the behaviour in joda-time
* https://github.com/elastic/elasticsearch/issues/42588
* @see java.time.temporal.WeekFields#of(Locale)
*/
public static final Locale ROOT = new Locale.Builder()
.setLocale(Locale.ROOT)
.setUnicodeLocaleKeyword("fw", "mon").build();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
---
setup:
- skip:
version: " - 7.99.99" #TODO change this after backport
reason: "not backported yet"

- do:
indices.create:
index: test
body:
mappings:
properties:
date:
type: date

- do:
index:
index: test
id: 1
body: { "date": "2009-11-15T14:12:12" }

- do:
indices.refresh:
index: [test]

---
# The inserted document has a field date=2009-11-15T14:12:12 which is Sunday.
# When aggregating per day of the week this should be considered as last day of the week (7)
# and this value should be used in 'key_as_string'
"Date aggregartion per day of week":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
aggregations:
test:
"date_histogram": {
"field": "date",
"calendar_interval": "day",
"format": "e",
"offset": 0
}

- match: {hits.total: 1}
- length: { aggregations.test.buckets: 1 }
- match: { aggregations.test.buckets.0.key_as_string: "7" }
409 changes: 216 additions & 193 deletions server/src/main/java/org/elasticsearch/common/time/DateFormatters.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -125,27 +125,27 @@ public long getFrom(TemporalAccessor temporal) {
.optionalStart() // optional is used so isSupported will be called when printing
.appendFraction(NANOS_OF_SECOND, 0, 9, true)
.optionalEnd()
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

// this supports seconds ending in dot
private static final DateTimeFormatter SECONDS_FORMATTER2 = new DateTimeFormatterBuilder()
.appendValue(SECONDS, 1, 19, SignStyle.NORMAL)
.appendLiteral('.')
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

// this supports milliseconds without any fraction
private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder()
.appendValue(MILLIS, 1, 19, SignStyle.NORMAL)
.optionalStart()
.appendFraction(NANOS_OF_MILLI, 0, 6, true)
.optionalEnd()
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

// this supports milliseconds ending in dot
private static final DateTimeFormatter MILLISECONDS_FORMATTER2 = new DateTimeFormatterBuilder()
.append(MILLISECONDS_FORMATTER1)
.appendLiteral('.')
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER1,
builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.common.time.DateFormatters;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.time.DateUtils;
import org.elasticsearch.common.time.IsoLocale;
import org.elasticsearch.common.util.LocaleUtils;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
Expand Down Expand Up @@ -135,7 +136,7 @@ public static class Builder extends FieldMapper.Builder<Builder, DateFieldMapper
public Builder(String name) {
super(name, new DateFieldType(), new DateFieldType());
builder = this;
locale = Locale.ROOT;
locale = IsoLocale.ROOT;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.common.collect.CopyOnWriteHashMap;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.time.IsoLocale;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
Expand All @@ -41,7 +42,6 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public class ObjectMapper extends Mapper implements Cloneable {
Expand Down Expand Up @@ -530,7 +530,7 @@ public void toXContent(XContentBuilder builder, Params params, ToXContent custom
builder.field("type", CONTENT_TYPE);
}
if (dynamic != null) {
builder.field("dynamic", dynamic.name().toLowerCase(Locale.ROOT));
builder.field("dynamic", dynamic.name().toLowerCase(IsoLocale.ROOT));
}
if (enabled != Defaults.ENABLED) {
builder.field("enabled", enabled);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.time.IsoLocale;
import org.elasticsearch.common.util.LocaleUtils;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand Down Expand Up @@ -94,7 +95,7 @@ public static class Defaults {

public static class Builder extends FieldMapper.Builder<Builder, RangeFieldMapper> {
private Boolean coerce;
private Locale locale = Locale.ROOT;
private Locale locale = IsoLocale.ROOT;
private String pattern;

public Builder(String name, RangeType type) {
Expand Down Expand Up @@ -413,7 +414,7 @@ && fieldType().dateTimeFormatter().pattern().equals(DateFieldMapper.DEFAULT_DATE
}
if (fieldType().rangeType == RangeType.DATE
&& (includeDefaults || (fieldType().dateTimeFormatter() != null
&& fieldType().dateTimeFormatter().locale() != Locale.ROOT))) {
&& fieldType().dateTimeFormatter().locale() != IsoLocale.ROOT))) {
builder.field("locale", fieldType().dateTimeFormatter().locale());
}
if (includeDefaults || coerce.explicit()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.joda.time.DateTimeZone;
import org.joda.time.format.ISODateTimeFormat;

import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
Expand All @@ -35,8 +36,31 @@

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.IsEqual.equalTo;

public class JavaJodaTimeDuellingTests extends ESTestCase {
@Override
protected boolean enableWarningsCheck() {
return false;
}

public void testDayOfWeek() {
//7 (ok joda) vs 1 (java by default) but 7 with customized org.elasticsearch.common.time.IsoLocale.ISO8601
ZonedDateTime now = LocalDateTime.of(2009,11,15,1,32,8,328402)
.atZone(ZoneOffset.UTC); //Sunday
DateFormatter jodaFormatter = Joda.forPattern("e").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC);
DateFormatter javaFormatter = DateFormatter.forPattern("8e").withZone(ZoneOffset.UTC);
assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now)));
}

public void testStartOfWeek() {
//2019-21 (ok joda) vs 2019-22 (java by default) but 2019-21 with customized org.elasticsearch.common.time.IsoLocale.ISO8601
ZonedDateTime now = LocalDateTime.of(2019,5,26,1,32,8,328402)
.atZone(ZoneOffset.UTC);
DateFormatter jodaFormatter = Joda.forPattern("xxxx-ww").withLocale(Locale.ROOT).withZone(ZoneOffset.UTC);
DateFormatter javaFormatter = DateFormatter.forPattern("8YYYY-ww").withZone(ZoneOffset.UTC);
assertThat(jodaFormatter.format(now), equalTo(javaFormatter.format(now)));
}

//these parsers should allow both ',' and '.' as a decimal point
public void testDecimalPointParsing(){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void testParsersWithMultipleInternalFormats() throws Exception {
}

public void testLocales() {
assertThat(DateFormatters.forPattern("strict_date_optional_time").locale(), is(Locale.ROOT));
assertThat(DateFormatters.forPattern("strict_date_optional_time").locale(), is(IsoLocale.ROOT));
Locale locale = randomLocale(random());
assertThat(DateFormatters.forPattern("strict_date_optional_time").withLocale(locale).locale(), is(locale));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@

package org.elasticsearch.xpack.sql.proto;

import org.elasticsearch.common.time.IsoLocale;

import java.sql.Timestamp;
import java.time.Duration;
import java.time.OffsetTime;
import java.time.Period;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.util.Locale;
import java.util.Objects;
import java.util.concurrent.TimeUnit;

Expand All @@ -24,7 +25,6 @@
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;

public final class StringUtils {

public static final String EMPTY = "";

public static final DateTimeFormatter ISO_DATE_WITH_MILLIS = new DateTimeFormatterBuilder()
Expand All @@ -38,7 +38,7 @@ public final class StringUtils {
.appendValue(SECOND_OF_MINUTE, 2)
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.appendOffsetId()
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

public static final DateTimeFormatter ISO_TIME_WITH_MILLIS = new DateTimeFormatterBuilder()
.parseCaseInsensitive()
Expand All @@ -49,7 +49,7 @@ public final class StringUtils {
.appendValue(SECOND_OF_MINUTE, 2)
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.appendOffsetId()
.toFormatter(Locale.ROOT);
.toFormatter(IsoLocale.ROOT);

private static final int SECONDS_PER_MINUTE = 60;
private static final int SECONDS_PER_HOUR = SECONDS_PER_MINUTE * 60;
Expand Down