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

Introducing a IsoLocal.ROOT constant which should be used instead of java.util.Locale.ROOT in ES when dealing with dates. IsoLocal.ROOT  customises start of the week to be Monday instead of Sunday.

closes #42588 an issue with investigation details
relates #41670 bug raised (this won't fix it on its own. joda.parseInto has to be reimplemented
closes #43275 an issue raised by community member

Author: pgomulka

libs/core/src/main/java/org/elasticsearch/common/time/IsoLocale.java

@@ -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();
+}

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/310_date_agg_per_day_of_week.yml

@@ -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" }

server/src/main/java/org/elasticsearch/common/time/DateFormatters.java

@@ -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),

server/src/main/java/org/elasticsearch/common/time/EpochTime.java

@@ -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;
@@ -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

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

@@ -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;
@@ -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 {
@@ -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);

server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -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;
@@ -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) {
@@ -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()) {

server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java

@@ -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;
@@ -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(){

server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java

@@ -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));
     }

server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java

@@ -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;
 
@@ -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()
@@ -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()
@@ -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;