Preserve highlights for returnable: false fields

Author: jwils

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb

@@ -292,22 +292,30 @@ def fields_with_sources
           indexing_fields_by_name_in_index.values.reject { |f| f.source.nil? }
         end
 
-        # Returns the list of `_source.excludes` paths for non-returnable fields.
+        # Returns the list of `_source.excludes` paths for non-returnable, non-highlightable fields.
+        #
+        # Hidden highlightable fields must remain in `_source` only when the GraphQL
+        # highlights API can still reach them.
         #
         # Uses `indexing_fields_by_name_in_index` for traversal (same as
         # `index_field_runtime_metadata_tuples`) to avoid infinite recursion
         # through interface/union subtype cycles.
         #
         # @private
-        def source_excludes_paths(path_prefix: "")
+        def source_excludes_paths(path_prefix = "", under_non_returnable_parent = false)
           indexing_fields_by_name_in_index.flat_map do |name, field|
             path = path_prefix + name
             object_type = field.type.fully_unwrapped.as_object_type
+            non_returnable = under_non_returnable_parent || !field.returnable?
 
-            if !field.returnable?
-              [object_type ? "#{path}.*" : path]
-            elsif object_type
-              object_type.source_excludes_paths(path_prefix: "#{path}.")
+            if object_type
+              if non_returnable && !field.highlightable?
+                ["#{path}.*"]
+              else
+                object_type.source_excludes_paths("#{path}.", non_returnable)
+              end
+            elsif non_returnable && !field.highlightable?
+              [path]
             else
               []
             end

elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/datastore_config/index_mappings/miscellaneous_spec.rb

@@ -117,25 +117,38 @@ module SchemaDefinition
         })
       end
 
-      it "uses `name_in_index` in `_source.excludes` for `returnable: false` fields" do
+      it "uses `name_in_index` in `_source.excludes` for non-highlightable `returnable: false` fields" do
         mapping = index_mapping_for "my_type" do |s|
           s.object_type "MyType" do |t|
             t.field "id", "ID"
-            t.field "name", "String"
-            t.field "internal_code_gql", "String", name_in_index: "internal_code", returnable: false
+            t.field "internal_count_gql", "Int", name_in_index: "internal_count", returnable: false
             t.index "my_type"
           end
         end
 
-        expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_code")
+        expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_count")
         # The field should still appear in properties (it's indexed, just not in _source)
+        expect(mapping.dig("properties", "internal_count")).to eq({"type" => "integer"})
+      end
+
+      it "keeps highlightable `returnable: false` fields in `_source` so they can still be highlighted" do
+        mapping = index_mapping_for "my_type" do |s|
+          s.object_type "MyType" do |t|
+            t.field "id", "ID"
+            t.field "internal_code", "String", returnable: false
+            t.index "my_type"
+          end
+        end
+
+        expect(mapping).not_to have_key("_source")
         expect(mapping.dig("properties", "internal_code")).to eq({"type" => "keyword"})
       end
 
-      it "includes `.*` in `_source.excludes` for `returnable: false` object fields" do
+      it "only excludes non-highlightable descendants for implicitly highlightable `returnable: false` object fields" do
         mapping = index_mapping_for "my_type" do |s|
           s.object_type "InternalMetadata" do |t|
             t.field "internal_code", "String"
+            t.field "internal_count", "Int"
           end
 
           s.object_type "MyType" do |t|
@@ -145,33 +158,55 @@ module SchemaDefinition
           end
         end
 
+        expect(mapping).to include("_source" => {"excludes" => ["internal_metadata.internal_count"]})
+        expect(mapping.dig("properties", "internal_metadata", "properties", "internal_code")).to eq({"type" => "keyword"})
+        expect(mapping.dig("properties", "internal_metadata", "properties", "internal_count")).to eq({"type" => "integer"})
+      end
+
+      it "excludes all descendants for explicitly non-highlightable `returnable: false` object fields even when a child field is highlightable" do
+        mapping = index_mapping_for "my_type" do |s|
+          s.object_type "InternalMetadata" do |t|
+            t.field "internal_code", "String", highlightable: true
+            t.field "internal_count", "Int"
+          end
+
+          s.object_type "MyType" do |t|
+            t.field "id", "ID"
+            t.field "internal_metadata", "InternalMetadata", returnable: false, highlightable: false
+            t.index "my_type"
+          end
+        end
+
         expect(mapping).to include("_source" => {"excludes" => ["internal_metadata.*"]})
         expect(mapping.dig("properties", "internal_metadata", "properties", "internal_code")).to eq({"type" => "keyword"})
+        expect(mapping.dig("properties", "internal_metadata", "properties", "internal_count")).to eq({"type" => "integer"})
       end
 
-      it "excludes `returnable: false` indexing-only fields but not `graphql_only` fields" do
+      it "excludes non-highlightable `returnable: false` indexing-only fields but not `graphql_only` fields" do
         mapping = index_mapping_for "my_type" do |s|
           s.object_type "MyType" do |t|
             t.field "id", "ID"
             t.field "name", "String"
-            t.field "legacy_name", "String", graphql_only: true, name_in_index: "name", returnable: false
-            t.field "internal_code", "String", indexing_only: true, returnable: false
+            t.field "count", "Int"
+            t.field "legacy_count", "Int", graphql_only: true, name_in_index: "count", returnable: false
+            t.field "internal_count", "Int", indexing_only: true, returnable: false
             t.index "my_type"
           end
         end
 
-        expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_code")
+        expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_count")
         expect(mapping.fetch("properties")).to include(
           "name" => {"type" => "keyword"},
-          "internal_code" => {"type" => "keyword"}
+          "count" => {"type" => "integer"},
+          "internal_count" => {"type" => "integer"}
         )
-        expect(mapping.fetch("properties")).not_to include("legacy_name")
+        expect(mapping.fetch("properties")).not_to include("legacy_count")
       end
 
       it "uses full indexed paths in `_source.excludes` for `returnable: false` fields under nested mappings" do
         mapping = index_mapping_for "my_type" do |s|
           s.object_type "Parent" do |t|
-            t.field "child", "String", name_in_index: "child_in_index", returnable: false
+            t.field "child_count_gql", "Int", name_in_index: "child_count", returnable: false
           end
 
           s.object_type "Grandparent" do |t|
@@ -187,19 +222,35 @@ module SchemaDefinition
           end
         end
 
-        expect(mapping.dig("_source", "excludes")).to contain_exactly("grandparents_in_index.parent_in_index.child_in_index")
+        expect(mapping.dig("_source", "excludes")).to contain_exactly("grandparents_in_index.parent_in_index.child_count")
         expect(mapping.dig("properties", "grandparents_in_index")).to include(
           "type" => "nested",
           "properties" => {
             "parent_in_index" => {
               "properties" => {
-                "child_in_index" => {"type" => "keyword"}
+                "child_count" => {"type" => "integer"}
               }
             }
           }
         )
       end
 
+      it "excludes all descendants for non-highlightable `returnable: false` object fields" do
+        mapping = index_mapping_for "my_type" do |s|
+          s.object_type "InternalMetrics" do |t|
+            t.field "count", "Int"
+          end
+
+          s.object_type "MyType" do |t|
+            t.field "id", "ID"
+            t.field "internal_metrics", "InternalMetrics", returnable: false
+            t.index "my_type"
+          end
+        end
+
+        expect(mapping).to include("_source" => {"excludes" => ["internal_metrics.*"]})
+        expect(mapping.dig("properties", "internal_metrics", "properties", "count")).to eq({"type" => "integer"})
+      end
       it "does not include `_source` config when all fields are returnable" do
         mapping = index_mapping_for "my_type" do |s|
           s.object_type "MyType" do |t|