From 8406ca8f81c8279529d0bde67fd94537ad610b41 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Wed, 16 Oct 2019 16:27:56 +0200 Subject: [PATCH 1/4] Do not prefix already prefixed format with 8 It happens when a locale is specified on a mapping that a new JavaDateFormatter is created with a new mapping and already prefixed with 8 format is prefixed again. This should change avoids this behavior. --- .../common/time/JavaDateFormatter.java | 2 +- .../admin/indices/create/CreateIndexIT.java | 44 +++++++++++++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java b/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java index af9552b19aa87..6ae251e091b7c 100644 --- a/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java +++ b/server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java @@ -76,7 +76,7 @@ class JavaDateFormatter implements DateFormatter { throw new IllegalArgumentException("formatters must have the same locale"); } this.printer = printer; - this.format = "8" + format; + this.format = format.startsWith("8") ? format : "8" + format; if (parsers.length == 0) { this.parsers = Collections.singletonList(printer); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index 575577c25097c..272a8fdcb04c0 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -42,6 +42,7 @@ import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; +import org.hamcrest.Matchers; import java.util.HashMap; import java.util.List; @@ -60,6 +61,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.core.IsNull.notNullValue; +import static org.mockito.Matchers.contains; @ClusterScope(scope = Scope.TEST) public class CreateIndexIT extends ESIntegTestCase { @@ -385,21 +387,35 @@ public void testIndexNameInResponse() { } public void testCreateIndexWithJava8Date() throws Exception { - String jodaIncompatibleFormat = "8yyyy-MM-dd HH:mm:ssXX"; - XContentBuilder builder = jsonBuilder().startObject().startObject("properties") - .startObject("time") - .field("type", "date") - .field("format", jodaIncompatibleFormat) - .endObject().endObject().endObject(); - - CreateIndexRequestBuilder requestBuilder = client().admin().indices().prepareCreate("test"); - assertAcked(requestBuilder.addMapping("doc", builder).get()); + BiFunction> createIndex = (xContentBuilder, propertyToBeReturned) -> { + String indexName = "test" + xContentBuilder.hashCode(); + CreateIndexRequestBuilder requestBuilder = client().admin().indices().prepareCreate(indexName); + assertAcked(requestBuilder.addMapping("doc", xContentBuilder).get()); + + GetMappingsResponse response = client().admin().indices().prepareGetMappings(indexName).get(); + List formats = + XContentMapValues.extractRawValues(propertyToBeReturned, response.getMappings().get(indexName).get("doc").getSourceAsMap()); + return formats; + }; - GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").get(); - List formats = - XContentMapValues.extractRawValues("properties.time.format", response.getMappings().get("test").get("doc").getSourceAsMap()); - assertThat(formats, hasSize(1)); - assertThat(formats.get(0), is(jodaIncompatibleFormat)); + XContentBuilder builder = jsonBuilder().startObject().startObject("properties") + .startObject("time") + .field("type", "date") + .field("format", "8yyyy-MM-dd-MM-dd HH:mm:ssXX") + .endObject() + .endObject().endObject(); + + assertThat(createIndex.apply(builder, "properties.time.format"), Matchers.contains("8yyyy-MM-dd-MM-dd HH:mm:ssXX")); + + + builder = jsonBuilder().startObject().startObject("properties") + .startObject("time") + .field("type", "date") + .field("format", "8yyyy-MM-dd-MM-dd HH:mm:ssXX") + .field("locale", "de") + .endObject() + .endObject().endObject(); + assertThat(createIndex.apply(builder, "properties.time.locale"), Matchers.contains("de")); } } From c7bd009803ad0e55c3282a6e0e501739375c40c5 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Wed, 16 Oct 2019 16:40:01 +0200 Subject: [PATCH 2/4] codestyle --- .../action/admin/indices/create/CreateIndexIT.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index 272a8fdcb04c0..a87a23cfca64e 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -57,11 +57,8 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.core.IsNull.notNullValue; -import static org.mockito.Matchers.contains; @ClusterScope(scope = Scope.TEST) public class CreateIndexIT extends ESIntegTestCase { @@ -393,8 +390,11 @@ public void testCreateIndexWithJava8Date() throws Exception { assertAcked(requestBuilder.addMapping("doc", xContentBuilder).get()); GetMappingsResponse response = client().admin().indices().prepareGetMappings(indexName).get(); - List formats = - XContentMapValues.extractRawValues(propertyToBeReturned, response.getMappings().get(indexName).get("doc").getSourceAsMap()); + List formats = XContentMapValues.extractRawValues(propertyToBeReturned, response + .getMappings() + .get(indexName) + .get("doc") + .getSourceAsMap()); return formats; }; From 3ab203753293fcca3858342e13460e2fce8e0b0b Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Thu, 17 Oct 2019 10:50:39 +0200 Subject: [PATCH 3/4] added a testcase for deserializing --- .../index/mapper/DateFieldMapperTests.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index d9443bf076482..afd55f0d6502a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -270,6 +270,25 @@ public void testFloatEpochFormat() throws IOException { assertEquals(epochMillis, pointField.numericValue().longValue()); } + public void testChangeLocaleWith8Prefix() throws IOException { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("field").field("type", "date") + .field("format", "8E, d MMM uuuu HH:mm:ss Z") + .field("locale", "de") + .endObject().endObject().endObject().endObject()); + + DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); + + assertEquals(mapping, mapper.mappingSource().toString()); + + mapper.parse(SourceToParse.source("test", "type", "1", BytesReference + .bytes(XContentFactory.jsonBuilder() + .startObject() + .field("field", "Mi., 06 Dez. 2000 02:55:00 -0800") + .endObject()), + XContentType.JSON)); + } + public void testChangeLocale() throws IOException { String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("field").field("type", "date").field("locale", "fr").endObject().endObject() From 816f76c96e8910606f05f4d4e65485e10f063260 Mon Sep 17 00:00:00 2001 From: Przemyslaw Gomulka Date: Thu, 17 Oct 2019 11:37:19 +0200 Subject: [PATCH 4/4] remove jdk dependend assetion --- server/build.gradle | 2 ++ .../elasticsearch/index/mapper/DateFieldMapperTests.java | 7 ------- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/server/build.gradle b/server/build.gradle index d6c78ccd4b553..4daa3c2c4f051 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -150,6 +150,8 @@ check.dependsOn(testScriptDocValuesMissingV7Behaviour) unitTest { // these are tested explicitly in separate test tasks exclude '**/*ScriptDocValuesMissingV7BehaviourTests.class' + jvmArg '-Djava.locale.providers=COMPAT' + } forbiddenPatterns { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java index afd55f0d6502a..06e39b4fc35d5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java @@ -280,13 +280,6 @@ public void testChangeLocaleWith8Prefix() throws IOException { DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping)); assertEquals(mapping, mapper.mappingSource().toString()); - - mapper.parse(SourceToParse.source("test", "type", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "Mi., 06 Dez. 2000 02:55:00 -0800") - .endObject()), - XContentType.JSON)); } public void testChangeLocale() throws IOException {