From a75cfb42de2a68dcafd07f7c1631a50971b095b4 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 18 May 2015 17:05:02 +0200 Subject: [PATCH] Mappings: Make DocumentMapper.refreshSource() private. This method should not be public, we should refresh the source automatically when we change mappings. --- .../index/mapper/DocumentMapper.java | 4 ++-- .../index/mapper/DocumentMapperParser.java | 5 +---- .../camelcase/CamelCaseFieldNameTests.java | 4 ---- .../index/mapper/multifield/MultiFieldTests.java | 1 - .../mapper/source/DefaultSourceMappingTests.java | 1 - .../mapper/timestamp/TimestampMappingTests.java | 4 ---- .../index/mapper/ttl/TTLMappingTests.java | 16 ++++++++-------- .../index/mapper/update/UpdateMappingTests.java | 4 ---- 8 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index a1a8180f8dc..42e64ca975b 100644 --- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -491,7 +491,7 @@ public class DocumentMapper implements ToXContent { return mergeResult; } - public CompressedString refreshSource() throws ElasticsearchGenerationException { + private void refreshSource() throws ElasticsearchGenerationException { try { BytesStreamOutput bStream = new BytesStreamOutput(); XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON, CompressorFactory.defaultCompressor().streamOutput(bStream)); @@ -499,7 +499,7 @@ public class DocumentMapper implements ToXContent { toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); builder.close(); - return mappingSource = new CompressedString(bStream.bytes()); + mappingSource = new CompressedString(bStream.bytes()); } catch (Exception e) { throw new ElasticsearchGenerationException("failed to serialize source for type [" + type + "]", e); } diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java index c955d6aeb6a..9084e17d60b 100644 --- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java +++ b/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java @@ -271,10 +271,7 @@ public class DocumentMapperParser extends AbstractIndexComponent { checkNoRemainingFields(mapping, parserContext.indexVersionCreated(), "Root mapping definition has unsupported parameters: "); - DocumentMapper documentMapper = docBuilder.build(mapperService, this); - // update the source with the generated one - documentMapper.refreshSource(); - return documentMapper; + return docBuilder.build(mapperService, this); } public static void checkNoRemainingFields(String fieldName, Map fieldNodeMap, Version indexVersionCreated) { diff --git a/src/test/java/org/elasticsearch/index/mapper/camelcase/CamelCaseFieldNameTests.java b/src/test/java/org/elasticsearch/index/mapper/camelcase/CamelCaseFieldNameTests.java index 622c4e567ac..ae4298c063d 100644 --- a/src/test/java/org/elasticsearch/index/mapper/camelcase/CamelCaseFieldNameTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/camelcase/CamelCaseFieldNameTests.java @@ -26,9 +26,6 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.test.ElasticsearchSingleNodeTest; import org.junit.Test; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; - /** * */ @@ -53,7 +50,6 @@ public class CamelCaseFieldNameTests extends ElasticsearchSingleNodeTest { assertNotNull(documentMapper.mappers().getMapper("thisIsCamelCase")); assertNull(documentMapper.mappers().getMapper("this_is_camel_case")); - documentMapper.refreshSource(); documentMapper = index.mapperService().documentMapperParser().parse(documentMapper.mappingSource().string()); assertNotNull(documentMapper.mappers().getMapper("thisIsCamelCase")); diff --git a/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java b/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java index fd528770d15..e7df72c3dcd 100644 --- a/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java @@ -148,7 +148,6 @@ public class MultiFieldTests extends ElasticsearchSingleNodeTest { .addMultiField(stringField("indexed").index(true).tokenized(true)) .addMultiField(stringField("not_indexed").index(false).store(true)) )).build(indexService.mapperService(), mapperParser); - builderDocMapper.refreshSource(); String builtMapping = builderDocMapper.mappingSource().string(); // System.out.println(builtMapping); diff --git a/src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java index 25ab2a5c6c0..f41a94ab132 100644 --- a/src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java @@ -224,7 +224,6 @@ public class DefaultSourceMappingTests extends ElasticsearchSingleNodeTest { void assertConflicts(String mapping1, String mapping2, DocumentMapperParser parser, String... conflicts) throws IOException { DocumentMapper docMapper = parser.parse(mapping1); - docMapper.refreshSource(); docMapper = parser.parse(docMapper.mappingSource().string()); MergeResult mergeResult = docMapper.merge(parser.parse(mapping2).mapping(), true); diff --git a/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java index f4d449ed3d0..471563af05d 100644 --- a/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java @@ -534,7 +534,6 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest { DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); DocumentMapper docMapper = parser.parse(mapping); - docMapper.refreshSource(); docMapper = parser.parse(docMapper.mappingSource().string()); assertThat(docMapper.mappingSource().string(), equalTo(mapping)); } @@ -557,7 +556,6 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest { DocumentMapper docMapper = parser.parse(mapping); boolean tokenized = docMapper.timestampFieldMapper().fieldType().tokenized(); - docMapper.refreshSource(); docMapper = parser.parse(docMapper.mappingSource().string()); assertThat(tokenized, equalTo(docMapper.timestampFieldMapper().fieldType().tokenized())); } @@ -686,7 +684,6 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest { void assertConflict(String mapping1, String mapping2, DocumentMapperParser parser, String conflict) throws IOException { DocumentMapper docMapper = parser.parse(mapping1); - docMapper.refreshSource(); docMapper = parser.parse(docMapper.mappingSource().string()); MergeResult mergeResult = docMapper.merge(parser.parse(mapping2).mapping(), true); assertThat(mergeResult.buildConflicts().length, equalTo(conflict == null ? 0 : 1)); @@ -744,7 +741,6 @@ public class TimestampMappingTests extends ElasticsearchSingleNodeTest { DocumentMapperParser parser = createIndex("test_doc_values").mapperService().documentMapperParser(); DocumentMapper docMapper = parser.parse(mapping); boolean docValues= docMapper.timestampFieldMapper().hasDocValues(); - docMapper.refreshSource(); docMapper = parser.parse(docMapper.mappingSource().string()); assertThat(docMapper.timestampFieldMapper().hasDocValues(), equalTo(docValues)); assertAcked(client().admin().indices().prepareDelete("test_doc_values")); diff --git a/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java index cbd2003bbd7..c858e9437eb 100644 --- a/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/ttl/TTLMappingTests.java @@ -216,7 +216,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { XContentBuilder mappingWithOnlyDefaultSet = getMappingWithOnlyTtlDefaultSet("6m"); MergeResult mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithOnlyDefaultSet.string()), true).mapping(), false); assertFalse(mergeResult.hasConflicts()); - CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(new CompressedString("{\"type\":{\"_ttl\":{\"enabled\":true,\"default\":360000},\"properties\":{\"field\":{\"type\":\"string\"}}}}"))); } @@ -224,12 +224,12 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { public void testMergeWithOnlyDefaultSetTtlDisabled() throws Exception { XContentBuilder mappingWithTtlEnabled = getMappingWithTtlDisabled("7d"); IndexService indexService = createIndex("testindex", ImmutableSettings.settingsBuilder().build(), "type", mappingWithTtlEnabled); - CompressedString mappingAfterCreation = indexService.mapperService().documentMapper("type").refreshSource(); + CompressedString mappingAfterCreation = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterCreation, equalTo(new CompressedString("{\"type\":{\"_ttl\":{\"enabled\":false},\"properties\":{\"field\":{\"type\":\"string\"}}}}"))); XContentBuilder mappingWithOnlyDefaultSet = getMappingWithOnlyTtlDefaultSet("6m"); MergeResult mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithOnlyDefaultSet.string()), true).mapping(), false); assertFalse(mergeResult.hasConflicts()); - CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(new CompressedString("{\"type\":{\"_ttl\":{\"enabled\":false},\"properties\":{\"field\":{\"type\":\"string\"}}}}"))); } @@ -244,7 +244,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { MergeResult mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithTtlDifferentDefault.string()), true).mapping(), true); assertFalse(mergeResult.hasConflicts()); // make sure simulate flag actually worked - no mappings applied - CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + CompressedString mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(mappingBeforeMerge)); client().admin().indices().prepareDelete("testindex").get(); @@ -256,7 +256,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithTtlEnabled.string()), true).mapping(), true); assertFalse(mergeResult.hasConflicts()); // make sure simulate flag actually worked - no mappings applied - mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(mappingBeforeMerge)); client().admin().indices().prepareDelete("testindex").get(); @@ -268,7 +268,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithTtlEnabled.string()), true).mapping(), true); assertFalse(mergeResult.hasConflicts()); // make sure simulate flag actually worked - no mappings applied - mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(mappingBeforeMerge)); client().admin().indices().prepareDelete("testindex").get(); @@ -279,7 +279,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithTtlEnabled.string()), true).mapping(), false); assertFalse(mergeResult.hasConflicts()); // make sure simulate flag actually worked - mappings applied - mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(new CompressedString("{\"type\":{\"_ttl\":{\"enabled\":true,\"default\":604800000},\"properties\":{\"field\":{\"type\":\"string\"}}}}"))); client().admin().indices().prepareDelete("testindex").get(); @@ -289,7 +289,7 @@ public class TTLMappingTests extends ElasticsearchSingleNodeTest { mergeResult = indexService.mapperService().documentMapper("type").merge(indexService.mapperService().parse("type", new CompressedString(mappingWithTtlEnabled.string()), true).mapping(), false); assertFalse(mergeResult.hasConflicts()); // make sure simulate flag actually worked - mappings applied - mappingAfterMerge = indexService.mapperService().documentMapper("type").refreshSource(); + mappingAfterMerge = indexService.mapperService().documentMapper("type").mappingSource(); assertThat(mappingAfterMerge, equalTo(new CompressedString("{\"type\":{\"_ttl\":{\"enabled\":true,\"default\":604800000},\"properties\":{\"field\":{\"type\":\"string\"}}}}"))); } diff --git a/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java b/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java index 7c12cd14c36..9dc5900187c 100644 --- a/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java @@ -126,7 +126,6 @@ public class UpdateMappingTests extends ElasticsearchSingleNodeTest { .endObject(); DocumentMapper documentMapper = indexService.mapperService().parse("type", new CompressedString(indexMapping.string()), true); assertThat(documentMapper.indexMapper().enabled(), equalTo(enabled)); - documentMapper.refreshSource(); documentMapper = indexService.mapperService().parse("type", new CompressedString(documentMapper.mappingSource().string()), true); assertThat(documentMapper.indexMapper().enabled(), equalTo(enabled)); } @@ -151,7 +150,6 @@ public class UpdateMappingTests extends ElasticsearchSingleNodeTest { assertThat(documentMapper.timestampFieldMapper().enabled(), equalTo(enabled)); assertTrue(documentMapper.timestampFieldMapper().fieldType().stored()); assertTrue(documentMapper.timestampFieldMapper().hasDocValues()); - documentMapper.refreshSource(); documentMapper = indexService.mapperService().parse("type", new CompressedString(documentMapper.mappingSource().string()), true); assertThat(documentMapper.timestampFieldMapper().enabled(), equalTo(enabled)); assertTrue(documentMapper.timestampFieldMapper().hasDocValues()); @@ -173,7 +171,6 @@ public class UpdateMappingTests extends ElasticsearchSingleNodeTest { DocumentMapper documentMapper = indexService.mapperService().parse("type", new CompressedString(indexMapping.string()), true); assertThat(documentMapper.sizeFieldMapper().enabled(), equalTo(enabled)); assertTrue(documentMapper.sizeFieldMapper().fieldType().stored()); - documentMapper.refreshSource(); documentMapper = indexService.mapperService().parse("type", new CompressedString(documentMapper.mappingSource().string()), true); assertThat(documentMapper.sizeFieldMapper().enabled(), equalTo(enabled)); } @@ -184,7 +181,6 @@ public class UpdateMappingTests extends ElasticsearchSingleNodeTest { String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/default_mapping_with_disabled_root_types.json"); DocumentMapper documentMapper = indexService.mapperService().parse("type", new CompressedString(mapping), true); assertThat(documentMapper.mappingSource().string(), equalTo(mapping)); - documentMapper.refreshSource(); documentMapper = indexService.mapperService().parse("type", new CompressedString(documentMapper.mappingSource().string()), true); assertThat(documentMapper.mappingSource().string(), equalTo(mapping)); }