From 1cb3068737d2516a614382c1e80def36c60cd194 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 6 Oct 2015 16:02:17 -0400 Subject: [PATCH] Make root_cause of field conflicts more obvious Does so by improving the error message passed to MapperParsingException. The error messages for mapping conflicts now look like: ``` { "error" : { "root_cause" : [ { "type" : "mapper_parsing_exception", "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]" } ], "type" : "mapper_parsing_exception", "reason" : "Failed to parse mapping [type_one]: Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]", "caused_by" : { "type" : "illegal_argument_exception", "reason" : "Mapper for [text] conflicts with existing mapping in other types:\n[mapper [text] has different [analyzer], mapper [text] is used by multiple types. Set update_all_types to true to update [search_analyzer] across all types., mapper [text] is used by multiple types. Set update_all_types to true to update [search_quote_analyzer] across all types.]" } }, "status" : 400 } ``` Closes #12839 Change implementation Rather than make a new exception this improves the error message of the old exception. --- .../metadata/MetaDataCreateIndexService.java | 4 +- .../index/mapper/MapperException.java | 4 ++ .../index/mapper/MapperParsingException.java | 4 ++ .../admin/indices/create/CreateIndexIT.java | 46 +++++++++++++------ .../date/DateBackwardsCompatibilityTests.java | 4 +- 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 629e3c07427..c949a293010 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -324,7 +324,7 @@ public class MetaDataCreateIndexService extends AbstractComponent { mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedXContent(XContentFactory.jsonBuilder().map(mappings.get(MapperService.DEFAULT_MAPPING)).string()), false, request.updateAllTypes()); } catch (Exception e) { removalReason = "failed on parsing default mapping on index creation"; - throw new MapperParsingException("mapping [" + MapperService.DEFAULT_MAPPING + "]", e); + throw new MapperParsingException("Failed to parse mapping [{}]: {}", e, MapperService.DEFAULT_MAPPING, e.getMessage()); } } for (Map.Entry> entry : mappings.entrySet()) { @@ -336,7 +336,7 @@ public class MetaDataCreateIndexService extends AbstractComponent { mapperService.merge(entry.getKey(), new CompressedXContent(XContentFactory.jsonBuilder().map(entry.getValue()).string()), true, request.updateAllTypes()); } catch (Exception e) { removalReason = "failed on parsing mappings on index creation"; - throw new MapperParsingException("mapping [" + entry.getKey() + "]", e); + throw new MapperParsingException("Failed to parse mapping [{}]: {}", e, entry.getKey(), e.getMessage()); } } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperException.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperException.java index c4964a00a2b..0241f1c8e45 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperException.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperException.java @@ -39,4 +39,8 @@ public class MapperException extends ElasticsearchException { public MapperException(String message, Throwable cause) { super(message, cause); } + + public MapperException(String message, Throwable cause, Object... args) { + super(message, cause, args); + } } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java index 8fb999e778a..df886c5ce9d 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperParsingException.java @@ -41,6 +41,10 @@ public class MapperParsingException extends MapperException { super(message, cause); } + public MapperParsingException(String message, Throwable cause, Object... args) { + super(message, cause, args); + } + @Override public RestStatus status() { return RestStatus.BAD_REQUEST; diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index cbdb8644b5d..08714913014 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -19,11 +19,9 @@ package org.elasticsearch.action.admin.indices.create; -import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.action.admin.indices.delete.DeleteIndexResponse; -import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; @@ -32,11 +30,11 @@ import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase.ClusterScope; import org.elasticsearch.test.ESIntegTestCase.Scope; -import org.junit.Test; import java.util.HashMap; import java.util.concurrent.CountDownLatch; @@ -45,13 +43,15 @@ import java.util.concurrent.atomic.AtomicInteger; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.core.IsNull.notNullValue; @ClusterScope(scope = Scope.TEST) public class CreateIndexIT extends ESIntegTestCase { - - @Test public void testCreationDate_Given() { prepareCreate("test").setSettings(Settings.builder().put(IndexMetaData.SETTING_CREATION_DATE, 4l)).get(); ClusterStateResponse response = client().admin().cluster().prepareState().get(); @@ -67,7 +67,6 @@ public class CreateIndexIT extends ESIntegTestCase { assertThat(index.getCreationDate(), equalTo(4l)); } - @Test public void testCreationDate_Generated() { long timeBeforeRequest = System.currentTimeMillis(); prepareCreate("test").get(); @@ -85,7 +84,6 @@ public class CreateIndexIT extends ESIntegTestCase { assertThat(index.getCreationDate(), allOf(lessThanOrEqualTo(timeAfterRequest), greaterThanOrEqualTo(timeBeforeRequest))); } - @Test public void testDoubleAddMapping() throws Exception { try { prepareCreate("test") @@ -113,7 +111,6 @@ public class CreateIndexIT extends ESIntegTestCase { } } - @Test public void testInvalidShardCountSettings() throws Exception { try { prepareCreate("test").setSettings(Settings.builder() @@ -152,7 +149,6 @@ public class CreateIndexIT extends ESIntegTestCase { } } - @Test public void testCreateIndexWithBlocks() { try { setClusterReadOnly(true); @@ -162,14 +158,12 @@ public class CreateIndexIT extends ESIntegTestCase { } } - @Test public void testCreateIndexWithMetadataBlocks() { assertAcked(prepareCreate("test").setSettings(Settings.builder().put(IndexMetaData.SETTING_BLOCKS_METADATA, true))); assertBlocked(client().admin().indices().prepareGetSettings("test"), IndexMetaData.INDEX_METADATA_BLOCK); disableIndexBlock("test", IndexMetaData.SETTING_BLOCKS_METADATA); } - @Test public void testInvalidShardCountSettingsWithoutPrefix() throws Exception { try { prepareCreate("test").setSettings(Settings.builder() @@ -222,7 +216,8 @@ public class CreateIndexIT extends ESIntegTestCase { @Override public void onResponse(DeleteIndexResponse deleteIndexResponse) { Thread thread = new Thread() { - public void run() { + @Override + public void run() { try { client().prepareIndex("test", "test").setSource("index_version", indexVersion.get()).get(); // recreate that index synchronized (indexVersionLock) { @@ -265,4 +260,29 @@ public class CreateIndexIT extends ESIntegTestCase { 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", "string") + .field("analyzer", "standard") + .field("search_analyzer", "whitespace") + .endObject().endObject().endObject()); + b.addMapping("type2", jsonBuilder().humanReadable(true).startObject().startObject("properties") + .startObject("text") + .field("type", "string") + .endObject().endObject().endObject()); + try { + b.get(); + } catch (MapperParsingException e) { + StringBuilder messages = new StringBuilder(); + for (Exception rootCause: e.guessRootCauses()) { + messages.append(rootCause.getMessage()); + } + assertThat(messages.toString(), containsString("mapper [text] is used by multiple types")); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/date/DateBackwardsCompatibilityTests.java b/core/src/test/java/org/elasticsearch/index/mapper/date/DateBackwardsCompatibilityTests.java index 4e906ae82fa..8ddfc3a2ae7 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/date/DateBackwardsCompatibilityTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/date/DateBackwardsCompatibilityTests.java @@ -40,6 +40,7 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.VersionUtils.randomVersionBetween; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; /** @@ -174,7 +175,8 @@ public class DateBackwardsCompatibilityTests extends ESSingleNodeTestCase { createIndex(Version.CURRENT, mapping); fail("Expected a MapperParsingException, but did not happen"); } catch (MapperParsingException e) { - assertThat(e.getMessage(), is("mapping [" + type + "]")); + assertThat(e.getMessage(), containsString("Failed to parse mapping [" + type + "]")); + assertThat(e.getMessage(), containsString("Epoch [epoch_seconds] is not supported as dynamic date format")); } }