From e1b0f82764a6c522dffe11a404735df23c363fe2 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 18 Jun 2021 16:49:21 +0200 Subject: [PATCH 1/2] [TEST] rework tests for #74234 Two IT tests added as part of #74234 have turned out to be flaky in that they rely on dynamic mappings updated to be available after indexing documents. This commit reworks them to instead use search and check what gets returned instead of checking what get mappings returns and doing a string comparison. --- .../index/mapper/DynamicMappingIT.java | 86 +++++++++++-------- .../script/AbstractFieldScript.java | 2 + 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java index ed2c9979d4e3b..afd57eed9dcd0 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java @@ -20,15 +20,14 @@ import org.elasticsearch.cluster.metadata.MappingMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Randomness; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.query.GeoBoundingBoxQueryBuilder; +import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESIntegTestCase; @@ -48,8 +47,6 @@ import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -338,8 +335,9 @@ public void testDynamicRuntimeNoConflicts() { assertAcked(client().admin().indices().prepareCreate("test").setMapping("{\"_doc\":{\"dynamic\":\"runtime\"}}").get()); List docs = new ArrayList<>(); + //the root is mapped dynamic:runtime hence there are no type conflicts docs.add(new IndexRequest("test").source("one.two.three", new int[]{1, 2, 3})); - docs.add(new IndexRequest("test").source("one.two", 1.2)); + docs.add(new IndexRequest("test").source("one.two", 3.5)); docs.add(new IndexRequest("test").source("one", "one")); docs.add(new IndexRequest("test").source("{\"one\":{\"two\": { \"three\": \"three\"}}}", XContentType.JSON)); Collections.shuffle(docs, random()); @@ -347,16 +345,22 @@ public void testDynamicRuntimeNoConflicts() { for (IndexRequest doc : docs) { bulkRequest.add(doc); } + bulkRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); BulkResponse bulkItemResponses = client().bulk(bulkRequest).actionGet(); assertFalse(bulkItemResponses.buildFailureMessage(), bulkItemResponses.hasFailures()); - GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get(); - Map sourceAsMap = getMappingsResponse.getMappings().get("test").sourceAsMap(); - assertFalse(sourceAsMap.containsKey("properties")); - @SuppressWarnings("unchecked") - Map runtime = (Map)sourceAsMap.get("runtime"); - //depending on the order of the documents field types may differ, but there are no mapping conflicts - assertThat(runtime.keySet(), containsInAnyOrder("one", "one.two", "one.two.three")); + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("one", "one")).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("one.two", 3.5)).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("one.two.three", "1")).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } } public void testDynamicRuntimeObjectFields() { @@ -365,46 +369,60 @@ public void testDynamicRuntimeObjectFields() { List docs = new ArrayList<>(); docs.add(new IndexRequest("test").source("obj.one", 1)); - docs.add(new IndexRequest("test").source("anything", 1)); + docs.add(new IndexRequest("test").source("anything", "anything")); + //obj.runtime is mapped dynamic:runtime hence there are no type conflicts docs.add(new IndexRequest("test").source("obj.runtime.one.two", "test")); docs.add(new IndexRequest("test").source("obj.runtime.one", "one")); - docs.add(new IndexRequest("test").source("{\"obj\":{\"runtime\":{\"one\":{\"two\": \"test\"}}}}", XContentType.JSON)); + docs.add(new IndexRequest("test").source("{\"obj\":{\"runtime\":{\"one\":{\"two\": 1}}}}", XContentType.JSON)); Collections.shuffle(docs, random()); BulkRequest bulkRequest = new BulkRequest(); for (IndexRequest doc : docs) { bulkRequest.add(doc); } + bulkRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); BulkResponse bulkItemResponses = client().bulk(bulkRequest).actionGet(); assertFalse(bulkItemResponses.buildFailureMessage(), bulkItemResponses.hasFailures()); + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("obj.one", 1)).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("anything", "anything")).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + { + SearchResponse searchResponse = client().prepareSearch("test").setQuery(new MatchQueryBuilder("obj.runtime.one", "one")).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + { + SearchResponse searchResponse = client().prepareSearch("test") + .setQuery(new MatchQueryBuilder("obj.runtime.one.two", "1")).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } + MapperParsingException exception = expectThrows(MapperParsingException.class, () -> client().prepareIndex("test").setSource("obj.runtime", "value").get()); assertEquals("object mapping for [obj.runtime] tried to parse field [obj.runtime] as object, but found a concrete value", exception.getMessage()); - assertEquals("{\"test\":{\"mappings\":" + - "{\"runtime\":{\"obj.runtime.one\":{\"type\":\"keyword\"},\"obj.runtime.one.two\":{\"type\":\"keyword\"}}," + - "\"properties\":{\"anything\":{\"type\":\"long\"}," + - "\"obj\":{\"properties\":{\"one\":{\"type\":\"long\"}," + - "\"runtime\":{\"type\":\"object\",\"dynamic\":\"runtime\"}}}}}}}", - Strings.toString(client().admin().indices().prepareGetMappings("test").get())); - assertAcked(client().admin().indices().preparePutMapping("test").setSource("{\"_doc\":{\"properties\":{\"obj\":{\"properties\":" + "{\"runtime\":{\"properties\":{\"dynamic\":{\"type\":\"object\", \"dynamic\":true}}}}}}}}", XContentType.JSON)); - assertEquals(RestStatus.CREATED, client().prepareIndex("test").setSource("obj.runtime.dynamic.leaf", 1).get().status()); - GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get(); - Map sourceAsMap = getMappingsResponse.getMappings().get("test").sourceAsMap(); - assertThat( - XContentMapValues.extractRawValues("properties.obj.properties.runtime.properties.dynamic.properties.leaf.type", sourceAsMap), - contains("long")); - } + //the parent object has been mapped dynamic:true, hence the field gets indexed + assertEquals(RestStatus.CREATED, client().prepareIndex("test").setSource("obj.runtime.dynamic.number", 1) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get().status()); + + { + SearchResponse searchResponse = client().prepareSearch("test") + .setQuery(new MatchQueryBuilder("obj.runtime.dynamic.number", 1)).get(); + assertEquals(1, searchResponse.getHits().getTotalHits().value); + } - private static Map getMappedField(Map sourceAsMap, String name) { - @SuppressWarnings("unchecked") - Map properties = (Map)sourceAsMap.get("properties"); - @SuppressWarnings("unchecked") - Map mappedField = (Map)properties.get(name); - return mappedField; + //a doc with the same field but a different type causes a conflict + MapperParsingException e = expectThrows(MapperParsingException.class, + () -> client().prepareIndex("test").setId("id").setSource("obj.runtime.dynamic.number", "string").get()); + assertEquals("failed to parse field [obj.runtime.dynamic.number] of type [long] in document with id 'id'. " + + "Preview of field's value: 'string'", e.getMessage()); } } diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index 6f5ff4cafb888..7f9f000f3a623 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -109,7 +109,9 @@ protected List extractFromSource(String path) { protected abstract void emitFromObject(Object v); protected final void emitFromSource() { + System.out.println("emitFromSource"); for (Object v : extractFromSource(fieldName)) { + System.out.println("value " + v + " - field: " + fieldName); emitFromObject(v); } } From 23e0470709f30ccfc6c0751bb0d4e35b3ca6c309 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 18 Jun 2021 17:09:18 +0200 Subject: [PATCH 2/2] remove system out leftover --- .../main/java/org/elasticsearch/script/AbstractFieldScript.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index 7f9f000f3a623..6f5ff4cafb888 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -109,9 +109,7 @@ protected List extractFromSource(String path) { protected abstract void emitFromObject(Object v); protected final void emitFromSource() { - System.out.println("emitFromSource"); for (Object v : extractFromSource(fieldName)) { - System.out.println("value " + v + " - field: " + fieldName); emitFromObject(v); } }