From 4754f47182f140482344a2084780993f660019b6 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 17 Sep 2024 21:56:44 +0100 Subject: [PATCH] fix tests --- .../index/mapper/SourceLoader.java | 2 +- .../ConcurrentSegmentSourceProvider.java | 6 +- .../search/lookup/SourceProvider.java | 2 +- .../index/mapper/PatchSourceMapperTests.java | 63 +++++++++++++++---- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java index a70dd0e1df13a..a724184ea3675 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java @@ -108,7 +108,7 @@ public void write(LeafStoredFieldLoader storedFields, int docId, XContentBuilder @Override public Set requiredStoredFields() { - return Set.of(SourceFieldMapper.NAME); + return Set.of(); } }; diff --git a/server/src/main/java/org/elasticsearch/search/lookup/ConcurrentSegmentSourceProvider.java b/server/src/main/java/org/elasticsearch/search/lookup/ConcurrentSegmentSourceProvider.java index 612721b342f63..053167dde899a 100644 --- a/server/src/main/java/org/elasticsearch/search/lookup/ConcurrentSegmentSourceProvider.java +++ b/server/src/main/java/org/elasticsearch/search/lookup/ConcurrentSegmentSourceProvider.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.index.fieldvisitor.LeafStoredFieldLoader; import org.elasticsearch.index.fieldvisitor.StoredFieldLoader; -import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.SourceLoader; import java.io.IOException; @@ -31,11 +30,10 @@ class ConcurrentSegmentSourceProvider implements SourceProvider { private final StoredFieldLoader storedFieldLoader; private final Map leaves = ConcurrentCollections.newConcurrentMap(); - ConcurrentSegmentSourceProvider(SourceLoader loader) { + ConcurrentSegmentSourceProvider(SourceLoader loader, boolean loadSource) { this.sourceLoader = loader; var fields = sourceLoader.requiredStoredFields(); - boolean loadSource = fields.contains(SourceFieldMapper.NAME); - this.storedFieldLoader = fields.isEmpty() ? StoredFieldLoader.empty() : StoredFieldLoader.create(loadSource, fields); + this.storedFieldLoader = StoredFieldLoader.create(loadSource, fields); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/lookup/SourceProvider.java b/server/src/main/java/org/elasticsearch/search/lookup/SourceProvider.java index ca6114094be0b..bdb916670ce98 100644 --- a/server/src/main/java/org/elasticsearch/search/lookup/SourceProvider.java +++ b/server/src/main/java/org/elasticsearch/search/lookup/SourceProvider.java @@ -34,6 +34,6 @@ public interface SourceProvider { * multiple threads. */ static SourceProvider fromLookup(MappingLookup lookup, SourceFieldMetrics metrics) { - return new ConcurrentSegmentSourceProvider(lookup.newSourceLoader(metrics)); + return new ConcurrentSegmentSourceProvider(lookup.newSourceLoader(metrics), lookup.isSourceSynthetic() == false); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/PatchSourceMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/PatchSourceMapperTests.java index eca1f5ad6557f..f6b8f2cb552e7 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/PatchSourceMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/PatchSourceMapperTests.java @@ -50,11 +50,12 @@ public void testPatchSourceFlat() throws IOException { })); assertSourcePatch( mapperService, - Map.of("field", Map.of("obj", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)) + Map.of("field", Map.of("obj", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)), + true ); } - public void testPatchSourceFlatWithCopyTo() throws IOException { + public void testPatchSourceFlatToCopyTo() throws IOException { var mapperService = createMapperService(mapping(b -> { // field b.startObject("field"); @@ -72,7 +73,28 @@ public void testPatchSourceFlatWithCopyTo() throws IOException { b.field("type", "keyword"); b.endObject(); })); - assertSourcePatch(mapperService, Map.of("field", "key1")); + assertSourcePatch(mapperService, Map.of("field", "key1"), true); + } + + public void testPatchSourceFlatFromCopyTo() throws IOException { + var mapperService = createMapperService(mapping(b -> { + // field + b.startObject("field"); + b.field("type", "patch"); + b.endObject(); + + // another_field + b.startObject("another_field"); + b.field("type", "keyword"); + b.field("copy_to", new String[] { "field" }); + b.endObject(); + + // another_field + b.startObject("extra_field"); + b.field("type", "keyword"); + b.endObject(); + })); + assertSourcePatch(mapperService, Map.of("another_field", "value1", "extra_field", "value2"), false); } public void testPatchSourceFlatMulti() throws IOException { @@ -96,7 +118,8 @@ public void testPatchSourceFlatMulti() throws IOException { List.of(Map.of("obj", Map.of("key1", "value1")), Map.of("another", "one")), "another_field", randomAlphaOfLengthBetween(5, 10) - ) + ), + true ) ); assertThat(exc.status(), equalTo(RestStatus.BAD_REQUEST)); @@ -129,7 +152,8 @@ public void testPatchSourceObject() throws IOException { })); assertSourcePatch( mapperService, - Map.of("obj", Map.of("field", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)) + Map.of("obj", Map.of("field", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)), + true ); } @@ -157,7 +181,11 @@ public void testPatchSourceObjectFlat() throws IOException { b.field("type", "keyword"); b.endObject(); })); - assertSourcePatch(mapperService, Map.of("obj.field", Map.of("key1", "value1"), "another_field", randomAlphaOfLengthBetween(5, 10))); + assertSourcePatch( + mapperService, + Map.of("obj.field", Map.of("key1", "value1"), "another_field", randomAlphaOfLengthBetween(5, 10)), + true + ); } public void testPatchSourceNestedObject() throws IOException { @@ -187,7 +215,8 @@ public void testPatchSourceNestedObject() throws IOException { })); assertSourcePatch( mapperService, - Map.of("nested", Map.of("field", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)) + Map.of("nested", Map.of("field", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10)), + true ); } @@ -228,7 +257,8 @@ public void testPatchSourceNestedArray() throws IOException { ), "another_field", randomAlphaOfLengthBetween(5, 10) - ) + ), + true ); } @@ -290,7 +320,8 @@ public void testPatchSourceMulti() throws IOException { Map.of("field", Map.of("key1", "value1")), "another_field", randomAlphaOfLengthBetween(5, 10) - ) + ), + true ); } @@ -347,18 +378,24 @@ public void testPatchSourceMultiFlat() throws IOException { Map.of("key1", "value1"), "another_field", randomAlphaOfLengthBetween(5, 10) - ) + ), + true ); } - public static void assertSourcePatch(MapperService mapperService, Map source) throws IOException { + public static void assertSourcePatch(MapperService mapperService, Map source, boolean needsPatching) + throws IOException { XContentBuilder builder = JsonXContent.contentBuilder(); builder.value(source); SourceToParse origSource = new SourceToParse("0", BytesReference.bytes(builder), builder.contentType()); ParsedDocument doc = mapperService.documentMapper().parse(origSource); var storedSource = doc.rootDoc().getField(SourceFieldMapper.NAME).binaryValue(); - assertThat(storedSource.length, lessThan(origSource.source().length())); - assertFalse(storedSource.utf8ToString().equals(origSource.source().utf8ToString())); + if (needsPatching) { + assertThat(storedSource.length, lessThan(origSource.source().length())); + assertFalse(storedSource.utf8ToString().equals(origSource.source().utf8ToString())); + } else { + assertThat(storedSource.utf8ToString(), equalTo(origSource.source().utf8ToString())); + } withLuceneIndex(mapperService, iw -> iw.addDocuments(doc.docs()), ir -> { Source actual = SourceProvider.fromLookup(mapperService.mappingLookup(), mapperService.getMapperMetrics().sourceFieldMetrics()) .getSource(ir.leaves().get(0), doc.docs().size() - 1);