Synthetic _source: fix extra fields in GET (#89778)

The fields loaded to support synthetic `_source` were all coming back in
the `fields` response of `GET` which was confusing. This removes them
from the results unless they are explicitly asked for.

docs/changelog/89778.yaml

@@ -0,0 +1,5 @@
+pr: 89778
+summary: "Synthetic _source: fix extra fields in GET"
+area: TSDB
+type: bug
+issues: []

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/get/100_synthetic_source.yml

@@ -33,7 +33,7 @@ keyword:
   - match:
       _source:
         kwd: foo
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
 ---
 fetch without refresh also produces synthetic source:
@@ -64,7 +64,7 @@ fetch without refresh also produces synthetic source:
         refresh: false # no refreshing!
         body:
           obj.kwd: foo
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -78,7 +78,7 @@ fetch without refresh also produces synthetic source:
       _source: # synthetic source will convert the dotted field names into an object, even when loading from the translog
         obj:
           kwd: foo
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
 ---
 force_synthetic_source_ok:
@@ -106,7 +106,7 @@ force_synthetic_source_ok:
         refresh: true
         body:
           obj.kwd: foo
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   # When _source is used in the fetch the original _source is perfect
   - do:
@@ -127,7 +127,7 @@ force_synthetic_source_ok:
       _source:
         obj:
           kwd: foo
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
 ---
 force_synthetic_source_bad_mapping:
@@ -190,7 +190,7 @@ stored text:
               text:
                 type: text
                 store: true
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       index:
@@ -199,7 +199,7 @@ stored text:
         refresh: true
         body:
           text: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -212,7 +212,39 @@ stored text:
   - match:
       _source:
         text: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
+
+  - do:
+      get:
+        index: test
+        id:    1
+        stored_fields: text
+        _source: true
+  - match: {_index: "test"}
+  - match: {_id: "1"}
+  - match: {_version: 1}
+  - match: {found: true}
+  - match:
+      _source:
+        text: the quick brown fox
+  - match:
+      fields:
+        text: [the quick brown fox]
+
+  - do:
+      get:
+        index: test
+        id:    1
+        stored_fields: garbage
+        _source: true
+  - match: {_index: "test"}
+  - match: {_id: "1"}
+  - match: {_version: 1}
+  - match: {found: true}
+  - match:
+      _source:
+        text: the quick brown fox
+  - is_false: fields
 
 ---
 stored keyword:
@@ -231,7 +263,7 @@ stored keyword:
               kwd:
                 type: keyword
                 store: true
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       index:
@@ -240,7 +272,7 @@ stored keyword:
         refresh: true
         body:
           kwd: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -253,7 +285,7 @@ stored keyword:
   - match:
       _source:
         kwd: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
 ---
 doc values keyword with ignore_above:
@@ -308,7 +340,7 @@ doc values keyword with ignore_above:
   - match:
       _source:
         kwd: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -321,7 +353,7 @@ doc values keyword with ignore_above:
   - match:
       _source:
         kwd: short
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -336,7 +368,7 @@ doc values keyword with ignore_above:
         kwd:
           - short
           - jumped over the lazy dog # fields saved by ignore_above are returned after doc values fields
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
 ---
 stored keyword with ignore_above:
@@ -393,7 +425,7 @@ stored keyword with ignore_above:
   - match:
       _source:
         kwd: the quick brown fox
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -406,7 +438,7 @@ stored keyword with ignore_above:
   - match:
       _source:
         kwd: short
-#  - is_false: fields  TODO fix me
+  - is_false: fields
 
   - do:
       get:
@@ -421,4 +453,4 @@ stored keyword with ignore_above:
         kwd:
           - short
           - jumped over the lazy dog # fields saved by ignore_above are returned after doc values fields
-#  - is_false: fields  TODO fix me
+  - is_false: fields

server/src/main/java/org/elasticsearch/index/get/ShardGetService.java

@@ -256,9 +256,17 @@ public final class ShardGetService extends AbstractIndexShardComponent {
 
         // put stored fields into result objects
         if (leafStoredFieldLoader.storedFields().isEmpty() == false) {
+            Set<String> needed = new HashSet<>();
+            if (storedFields != null) {
+                Collections.addAll(needed, storedFields);
+            }
+            needed.add(RoutingFieldMapper.NAME); // we always return _routing if we see it, even if you don't ask for it.....
             documentFields = new HashMap<>();
             metadataFields = new HashMap<>();
             for (Map.Entry<String, List<Object>> entry : leafStoredFieldLoader.storedFields().entrySet()) {
+                if (false == needed.contains(entry.getKey())) {
+                    continue;
+                }
                 List<Object> values = FetchPhase.processStoredField(mapperService::fieldType, entry.getKey(), entry.getValue());
                 if (mapperService.isMetadataField(entry.getKey())) {
                     metadataFields.put(entry.getKey(), new DocumentField(entry.getKey(), values));

server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java

@@ -151,6 +151,8 @@ public class ShardGetServiceTests extends IndexShardTestCase {
         assertTrue(primary.getEngine().refreshNeeded());
         GetResult testGet = primary.getService().getForUpdate("0", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM);
         assertFalse(testGet.getFields().containsKey(RoutingFieldMapper.NAME));
+        assertFalse(testGet.getFields().containsKey("foo"));
+        assertFalse(testGet.getFields().containsKey("bar"));
         assertThat(new String(testGet.source() == null ? new byte[0] : testGet.source(), StandardCharsets.UTF_8), equalTo(expectedResult));
         try (Engine.Searcher searcher = primary.getEngine().acquireSearcher("test", Engine.SearcherScope.INTERNAL)) {
             assertEquals(searcher.getIndexReader().maxDoc(), 1); // we refreshed
@@ -161,6 +163,8 @@ public class ShardGetServiceTests extends IndexShardTestCase {
         GetResult testGet1 = primary.getService().getForUpdate("1", UNASSIGNED_SEQ_NO, UNASSIGNED_PRIMARY_TERM);
         assertEquals(new String(testGet1.source() == null ? new byte[0] : testGet1.source(), StandardCharsets.UTF_8), expectedResult);
         assertTrue(testGet1.getFields().containsKey(RoutingFieldMapper.NAME));
+        assertFalse(testGet.getFields().containsKey("foo"));
+        assertFalse(testGet.getFields().containsKey("bar"));
         assertEquals("foobar", testGet1.getFields().get(RoutingFieldMapper.NAME).getValue());
         if (sourceOnlyFetchCreatesInMemoryReader) {
             translogInMemorySegmentCountExpected++;