fix tests

jimczi · Sep 17, 2024 · 4754f47 · 4754f47
1 parent 0d49121
commit 4754f47
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 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
@@ -108,7 +108,7 @@ public void write(LeafStoredFieldLoader storedFields, int docId, XContentBuilder
 
         @Override
         public Set<String> 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
@@ -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<Object, Leaf> 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
@@ -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
@@ -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<String, Object> source) throws IOException {
+    public static void assertSourcePatch(MapperService mapperService, Map<String, Object> 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);