Merge branch 'master' into byte-size-value-setting-limit-error-message

* master:
  Add awaits fix for a query analyzer test
  Check presence of multi-types before validating new mapping (elastic#29316)

Author: jasontedor

modules/percolator/src/test/java/org/elasticsearch/percolator/QueryAnalyzerTests.java

@@ -384,6 +384,7 @@ public void testExtractQueryMetadata_booleanQueryWithMustNot() {
         assertThat(terms.get(1).bytes(), equalTo(phraseQuery.getTerms()[1].bytes()));
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/29363")
     public void testExactMatch_booleanQuery() {
         BooleanQuery.Builder builder = new BooleanQuery.Builder();
         TermQuery termQuery1 = new TermQuery(new Term("_field", "_term1"));

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

@@ -385,6 +385,16 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
             results.put(DEFAULT_MAPPING, defaultMapper);
         }
 
+        if (indexSettings.isSingleType()) {
+            Set<String> actualTypes = new HashSet<>(mappers.keySet());
+            documentMappers.forEach(mapper -> actualTypes.add(mapper.type()));
+            actualTypes.remove(DEFAULT_MAPPING);
+            if (actualTypes.size() > 1) {
+                throw new IllegalArgumentException(
+                    "Rejecting mapping update to [" + index().getName() + "] as the final mapping would have more than 1 type: " + actualTypes);
+            }
+        }
+
         for (DocumentMapper mapper : documentMappers) {
             // check naming
             validateTypeName(mapper.type());
@@ -478,15 +488,6 @@ private synchronized Map<String, DocumentMapper> internalMerge(@Nullable Documen
             }
         }
 
-        if (indexSettings.isSingleType()) {
-            Set<String> actualTypes = new HashSet<>(mappers.keySet());
-            actualTypes.remove(DEFAULT_MAPPING);
-            if (actualTypes.size() > 1) {
-                throw new IllegalArgumentException(
-                        "Rejecting mapping update to [" + index().getName() + "] as the final mapping would have more than 1 type: " + actualTypes);
-            }
-        }
-
         // make structures immutable
         mappers = Collections.unmodifiableMap(mappers);
         results = Collections.unmodifiableMap(results);

server/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java

@@ -264,25 +264,6 @@ public void onFailure(Exception e) {
         logger.info("total: {}", expected.getHits().getTotalHits());
     }
 
-    /**
-     * Asserts that the root cause of mapping conflicts is readable.
-     */
-    public void testMappingConflictRootCause() throws Exception {
-        CreateIndexRequestBuilder b = prepareCreate("test");
-        b.addMapping("type1", jsonBuilder().startObject().startObject("properties")
-                .startObject("text")
-                    .field("type", "text")
-                    .field("analyzer", "standard")
-                    .field("search_analyzer", "whitespace")
-                .endObject().endObject().endObject());
-        b.addMapping("type2", jsonBuilder().humanReadable(true).startObject().startObject("properties")
-                .startObject("text")
-                    .field("type", "text")
-                .endObject().endObject().endObject());
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> b.get());
-        assertThat(e.getMessage(), containsString("Mapper for [text] conflicts with existing mapping:"));
-    }
-
     public void testRestartIndexCreationAfterFullClusterRestart() throws Exception {
         client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("cluster.routing.allocation.enable",
             "none")).get();

server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java

@@ -164,7 +164,7 @@ public void testMappingDepthExceedsLimit() throws Throwable {
         indexService2.mapperService().merge("type", objectMapping, MergeReason.MAPPING_UPDATE);
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-                () -> indexService1.mapperService().merge("type2", objectMapping, MergeReason.MAPPING_UPDATE));
+                () -> indexService1.mapperService().merge("type", objectMapping, MergeReason.MAPPING_UPDATE));
         assertThat(e.getMessage(), containsString("Limit of mapping depth [1] in index [test1] has been exceeded"));
     }
 
@@ -255,7 +255,6 @@ public void testPartitionedConstraints() {
         // partitioned index cannot have parent/child relationships
         IllegalArgumentException parentException = expectThrows(IllegalArgumentException.class, () -> {
             client().admin().indices().prepareCreate("test-index")
-                    .addMapping("parent", "{\"parent\":{\"_routing\":{\"required\":true}}}", XContentType.JSON)
                     .addMapping("child", "{\"child\": {\"_routing\":{\"required\":true}, \"_parent\": {\"type\": \"parent\"}}}",
                         XContentType.JSON)
                     .setSettings(Settings.builder()
@@ -307,6 +306,23 @@ public void testForbidMultipleTypes() throws IOException {
         assertThat(e.getMessage(), Matchers.startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: "));
     }
 
+    /**
+     * This test checks that the multi-type validation is done before we do any other kind of validation on the mapping that's added,
+     * see https://github.com/elastic/elasticsearch/issues/29313
+     */
+    public void testForbidMultipleTypesWithConflictingMappings() throws IOException {
+        String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
+            .startObject("properties").startObject("field1").field("type", "integer_range").endObject().endObject().endObject().endObject());
+        MapperService mapperService = createIndex("test").mapperService();
+        mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE);
+
+        String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type2")
+            .startObject("properties").startObject("field1").field("type", "integer").endObject().endObject().endObject().endObject());
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> mapperService.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getMessage(), Matchers.startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: "));
+    }
+
     public void testDefaultMappingIsRejectedOn7() throws IOException {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_default_").endObject().endObject());
         MapperService mapperService = createIndex("test").mapperService();

server/src/test/java/org/elasticsearch/index/mapper/UpdateMappingTests.java

@@ -117,34 +117,25 @@ public void testConflictSameType() throws Exception {
     }
 
     public void testConflictNewType() throws Exception {
-        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
                 .startObject("properties").startObject("foo").field("type", "long").endObject()
                 .endObject().endObject().endObject();
-        MapperService mapperService = createIndex("test", Settings.builder().build(), "type1", mapping).mapperService();
+        MapperService mapperService = createIndex("test", Settings.builder().build(), "type", mapping).mapperService();
 
-        XContentBuilder update = XContentFactory.jsonBuilder().startObject().startObject("type2")
+        XContentBuilder update = XContentFactory.jsonBuilder().startObject().startObject("type")
                 .startObject("properties").startObject("foo").field("type", "double").endObject()
                 .endObject().endObject().endObject();
 
         try {
-            mapperService.merge("type2", new CompressedXContent(Strings.toString(update)), MapperService.MergeReason.MAPPING_UPDATE);
-            fail();
-        } catch (IllegalArgumentException e) {
-            // expected
-            assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]"));
-        }
-
-        try {
-            mapperService.merge("type2", new CompressedXContent(Strings.toString(update)), MapperService.MergeReason.MAPPING_UPDATE);
+            mapperService.merge("type", new CompressedXContent(Strings.toString(update)), MapperService.MergeReason.MAPPING_UPDATE);
             fail();
         } catch (IllegalArgumentException e) {
             // expected
             assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]"));
         }
 
-        assertThat(((FieldMapper) mapperService.documentMapper("type1").mapping().root().getMapper("foo")).fieldType().typeName(),
+        assertThat(((FieldMapper) mapperService.documentMapper("type").mapping().root().getMapper("foo")).fieldType().typeName(),
                 equalTo("long"));
-        assertNull(mapperService.documentMapper("type2"));
     }
 
     // same as the testConflictNewType except that the mapping update is on an existing type
@@ -208,15 +199,15 @@ public void testReuseMetaField() throws IOException {
 
     public void testRejectFieldDefinedTwice() throws IOException {
         String mapping1 = Strings.toString(XContentFactory.jsonBuilder().startObject()
-                .startObject("type1")
+                .startObject("type")
                     .startObject("properties")
                         .startObject("foo")
                             .field("type", "object")
                         .endObject()
                     .endObject()
                 .endObject().endObject());
         String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject()
-                .startObject("type2")
+                .startObject("type")
                     .startObject("properties")
                         .startObject("foo")
                             .field("type", "long")
@@ -225,17 +216,15 @@ public void testRejectFieldDefinedTwice() throws IOException {
                 .endObject().endObject());
 
         MapperService mapperService1 = createIndex("test1").mapperService();
-        mapperService1.merge("type1", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE);
+        mapperService1.merge("type", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE);
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
-                () -> mapperService1.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE));
-        assertThat(e.getMessage(), equalTo("[foo] is defined as a field in mapping [type2"
-                + "] but this name is already used for an object in other types"));
+                () -> mapperService1.merge("type", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getMessage(), equalTo("Can't merge a non object mapping [foo] with an object mapping [foo]"));
 
         MapperService mapperService2 = createIndex("test2").mapperService();
-        mapperService2.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE);
+        mapperService2.merge("type", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE);
         e = expectThrows(IllegalArgumentException.class,
-                () -> mapperService2.merge("type1", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE));
-        assertThat(e.getMessage(), equalTo("[foo] is defined as an object in mapping [type1"
-                + "] but this name is already used for a field in other types"));
+                () -> mapperService2.merge("type", new CompressedXContent(mapping1), MergeReason.MAPPING_UPDATE));
+        assertThat(e.getMessage(), equalTo("mapper [foo] of different type, current_type [long], merged_type [ObjectMapper]"));
     }
 }