Skip to content

Commit 228e2f8

Browse files
committed
Preserve highlighting for returnable: false fields
1 parent ce9d5c3 commit 228e2f8

5 files changed

Lines changed: 269 additions & 29 deletions

File tree

config/schema/artifacts/datastore_config.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,10 +1469,6 @@ index_templates:
14691469
required: true
14701470
_size:
14711471
enabled: true
1472-
_source:
1473-
excludes:
1474-
- internal_name
1475-
- internal_details.*
14761472
settings:
14771473
index.mapping.ignore_malformed: false
14781474
index.mapping.coerce: false

config/schema/artifacts_with_apollo/datastore_config.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,10 +1469,6 @@ index_templates:
14691469
required: true
14701470
_size:
14711471
enabled: true
1472-
_source:
1473-
excludes:
1474-
- internal_name
1475-
- internal_details.*
14761472
settings:
14771473
index.mapping.ignore_malformed: false
14781474
index.mapping.coerce: false
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
# Copyright 2024 - 2026 Block, Inc.
2+
#
3+
# Use of this source code is governed by an MIT-style
4+
# license that can be found in the LICENSE file or at
5+
# https://opensource.org/licenses/MIT.
6+
#
7+
# frozen_string_literal: true
8+
9+
require_relative "elasticgraph_graphql_acceptance_support"
10+
11+
module ElasticGraph
12+
RSpec.describe "ElasticGraph::GraphQL--returnable fields" do
13+
include_context "ElasticGraph GraphQL acceptance support"
14+
15+
with_both_casing_forms do
16+
let(:aggregated_values) { case_correctly("aggregated_values") }
17+
let(:approximate_distinct_value_count) { case_correctly("approximate_distinct_value_count") }
18+
let(:grouped_by) { case_correctly("grouped_by") }
19+
let(:internal_details) { case_correctly("internal_details") }
20+
let(:internal_name) { case_correctly("internal_name") }
21+
let(:widget_aggregations) { case_correctly("widget_aggregations") }
22+
let(:widgets) { case_correctly("widgets") }
23+
24+
it "supports filtering, sorting, grouping, aggregating, and highlighting a hidden leaf field" do
25+
index_records(
26+
build(:widget, id: "w1", internal_name: "alpha secret"),
27+
build(:widget, id: "w2", internal_name: "beta secret"),
28+
build(:widget, id: "w3", internal_name: "gamma hidden")
29+
)
30+
31+
response = call_graphql_query(<<~QUERY)
32+
query {
33+
widgets(
34+
filter: {
35+
internal_name: {
36+
contains: {
37+
any_substring_of: ["secret"]
38+
}
39+
}
40+
}
41+
order_by: [#{internal_name}_ASC]
42+
) {
43+
edges {
44+
node {
45+
id
46+
}
47+
48+
highlights {
49+
internal_name
50+
}
51+
}
52+
}
53+
54+
widget_aggregations {
55+
nodes {
56+
grouped_by {
57+
internal_name
58+
}
59+
60+
count
61+
62+
aggregated_values {
63+
internal_name {
64+
approximate_distinct_value_count
65+
}
66+
}
67+
}
68+
}
69+
}
70+
QUERY
71+
72+
expect(response.dig("data", widgets, "edges").map { |edge| edge.dig("node", "id") }).to eq(%w[w1 w2])
73+
expect(response.dig("data", widgets, "edges").to_h { |edge| [edge.dig("node", "id"), edge.dig("highlights", internal_name)] }).to eq({
74+
"w1" => ["<em>alpha secret</em>"],
75+
"w2" => ["<em>beta secret</em>"]
76+
})
77+
78+
aggregation_nodes = response
79+
.dig("data", widget_aggregations, "nodes")
80+
.sort_by { |node| node.dig(grouped_by, internal_name) }
81+
82+
expect(aggregation_nodes).to eq([
83+
{
84+
grouped_by => {internal_name => "alpha secret"},
85+
"count" => 1,
86+
aggregated_values => {internal_name => {approximate_distinct_value_count => 1}}
87+
},
88+
{
89+
grouped_by => {internal_name => "beta secret"},
90+
"count" => 1,
91+
aggregated_values => {internal_name => {approximate_distinct_value_count => 1}}
92+
},
93+
{
94+
grouped_by => {internal_name => "gamma hidden"},
95+
"count" => 1,
96+
aggregated_values => {internal_name => {approximate_distinct_value_count => 1}}
97+
}
98+
])
99+
100+
expect {
101+
hidden_field_response = call_graphql_query(<<~QUERY, allow_errors: true)
102+
query {
103+
widgets {
104+
edges {
105+
node {
106+
id
107+
internal_name
108+
}
109+
}
110+
}
111+
}
112+
QUERY
113+
114+
expect_error_related_to(hidden_field_response, "Widget", internal_name, "doesn't exist on type")
115+
}.to log(a_string_including("Widget", internal_name, "doesn't exist on type"))
116+
end
117+
118+
it "supports filtering, grouping, aggregating, and highlighting a hidden object field" do
119+
index_records(
120+
build(:widget, id: "w1", internal_details: build(:widget_internal_details, name: "alpha vault")),
121+
build(:widget, id: "w2", internal_details: build(:widget_internal_details, name: "beta vault")),
122+
build(:widget, id: "w3", internal_details: build(:widget_internal_details, name: "gamma archive"))
123+
)
124+
125+
response = call_graphql_query(<<~QUERY)
126+
query {
127+
widgets(
128+
filter: {
129+
internal_details: {
130+
name: {
131+
contains: {
132+
any_substring_of: ["vault"]
133+
}
134+
}
135+
}
136+
}
137+
) {
138+
edges {
139+
node {
140+
id
141+
}
142+
143+
highlights {
144+
internal_details {
145+
name
146+
}
147+
}
148+
}
149+
}
150+
151+
widget_aggregations {
152+
nodes {
153+
grouped_by {
154+
internal_details {
155+
name
156+
}
157+
}
158+
159+
count
160+
161+
aggregated_values {
162+
internal_details {
163+
name {
164+
approximate_distinct_value_count
165+
}
166+
}
167+
}
168+
}
169+
}
170+
}
171+
QUERY
172+
173+
expect(response.dig("data", widgets, "edges").map { |edge| edge.dig("node", "id") }).to match_array(%w[w1 w2])
174+
expect(response.dig("data", widgets, "edges").to_h { |edge| [edge.dig("node", "id"), edge.dig("highlights", internal_details, "name")] }).to eq({
175+
"w1" => ["<em>alpha vault</em>"],
176+
"w2" => ["<em>beta vault</em>"]
177+
})
178+
179+
aggregation_nodes = response
180+
.dig("data", widget_aggregations, "nodes")
181+
.sort_by { |node| node.dig(grouped_by, internal_details, "name") }
182+
183+
expect(aggregation_nodes).to eq([
184+
{
185+
grouped_by => {internal_details => {"name" => "alpha vault"}},
186+
"count" => 1,
187+
aggregated_values => {internal_details => {"name" => {approximate_distinct_value_count => 1}}}
188+
},
189+
{
190+
grouped_by => {internal_details => {"name" => "beta vault"}},
191+
"count" => 1,
192+
aggregated_values => {internal_details => {"name" => {approximate_distinct_value_count => 1}}}
193+
},
194+
{
195+
grouped_by => {internal_details => {"name" => "gamma archive"}},
196+
"count" => 1,
197+
aggregated_values => {internal_details => {"name" => {approximate_distinct_value_count => 1}}}
198+
}
199+
])
200+
201+
expect {
202+
hidden_field_response = call_graphql_query(<<~QUERY, allow_errors: true)
203+
query {
204+
widgets {
205+
edges {
206+
node {
207+
id
208+
internal_details {
209+
name
210+
}
211+
}
212+
}
213+
}
214+
}
215+
QUERY
216+
217+
expect_error_related_to(hidden_field_response, "Widget", internal_details, "doesn't exist on type")
218+
}.to log(a_string_including("Widget", internal_details, "doesn't exist on type"))
219+
end
220+
end
221+
end
222+
end

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,22 +292,33 @@ def fields_with_sources
292292
indexing_fields_by_name_in_index.values.reject { |f| f.source.nil? }
293293
end
294294

295-
# Returns the list of `_source.excludes` paths for non-returnable fields.
295+
# Returns the list of `_source.excludes` paths for non-returnable, non-highlightable fields.
296+
#
297+
# Hidden highlightable fields must remain in `_source` so the datastore can still
298+
# produce search highlight snippets for them.
296299
#
297300
# Uses `indexing_fields_by_name_in_index` for traversal (same as
298301
# `index_field_runtime_metadata_tuples`) to avoid infinite recursion
299302
# through interface/union subtype cycles.
300303
#
301304
# @private
302-
def source_excludes_paths(path_prefix: "")
305+
def source_excludes_paths(path_prefix: "", under_non_returnable_parent: false)
303306
indexing_fields_by_name_in_index.flat_map do |name, field|
304307
path = path_prefix + name
305308
object_type = field.type.fully_unwrapped.as_object_type
309+
non_returnable = under_non_returnable_parent || !field.returnable?
306310

307-
if !field.returnable?
308-
[object_type ? "#{path}.*" : path]
309-
elsif object_type
310-
object_type.source_excludes_paths(path_prefix: "#{path}.")
311+
if object_type
312+
if non_returnable && !field.highlightable?
313+
["#{path}.*"]
314+
else
315+
object_type.source_excludes_paths(
316+
path_prefix: "#{path}.",
317+
under_non_returnable_parent: non_returnable
318+
)
319+
end
320+
elsif non_returnable && !field.highlightable?
321+
[path]
311322
else
312323
[]
313324
end

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

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,38 @@ module SchemaDefinition
117117
})
118118
end
119119

120-
it "uses `name_in_index` in `_source.excludes` for `returnable: false` fields" do
120+
it "uses `name_in_index` in `_source.excludes` for non-highlightable `returnable: false` fields" do
121121
mapping = index_mapping_for "my_type" do |s|
122122
s.object_type "MyType" do |t|
123123
t.field "id", "ID"
124-
t.field "name", "String"
125-
t.field "internal_code_gql", "String", name_in_index: "internal_code", returnable: false
124+
t.field "internal_count_gql", "Int", name_in_index: "internal_count", returnable: false
126125
t.index "my_type"
127126
end
128127
end
129128

130-
expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_code")
129+
expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_count")
131130
# The field should still appear in properties (it's indexed, just not in _source)
131+
expect(mapping.dig("properties", "internal_count")).to eq({"type" => "integer"})
132+
end
133+
134+
it "keeps highlightable `returnable: false` fields in `_source` so they can still be highlighted" do
135+
mapping = index_mapping_for "my_type" do |s|
136+
s.object_type "MyType" do |t|
137+
t.field "id", "ID"
138+
t.field "internal_code", "String", returnable: false
139+
t.index "my_type"
140+
end
141+
end
142+
143+
expect(mapping).not_to have_key("_source")
132144
expect(mapping.dig("properties", "internal_code")).to eq({"type" => "keyword"})
133145
end
134146

135-
it "includes `.*` in `_source.excludes` for `returnable: false` object fields" do
147+
it "only excludes non-highlightable descendants for highlightable `returnable: false` object fields" do
136148
mapping = index_mapping_for "my_type" do |s|
137149
s.object_type "InternalMetadata" do |t|
138150
t.field "internal_code", "String"
151+
t.field "internal_count", "Int"
139152
end
140153

141154
s.object_type "MyType" do |t|
@@ -145,33 +158,35 @@ module SchemaDefinition
145158
end
146159
end
147160

148-
expect(mapping).to include("_source" => {"excludes" => ["internal_metadata.*"]})
161+
expect(mapping).to include("_source" => {"excludes" => ["internal_metadata.internal_count"]})
149162
expect(mapping.dig("properties", "internal_metadata", "properties", "internal_code")).to eq({"type" => "keyword"})
163+
expect(mapping.dig("properties", "internal_metadata", "properties", "internal_count")).to eq({"type" => "integer"})
150164
end
151165

152-
it "excludes `returnable: false` indexing-only fields but not `graphql_only` fields" do
166+
it "excludes non-highlightable `returnable: false` indexing-only fields but not `graphql_only` fields" do
153167
mapping = index_mapping_for "my_type" do |s|
154168
s.object_type "MyType" do |t|
155169
t.field "id", "ID"
156170
t.field "name", "String"
157-
t.field "legacy_name", "String", graphql_only: true, name_in_index: "name", returnable: false
158-
t.field "internal_code", "String", indexing_only: true, returnable: false
171+
t.field "legacy_count", "Int", graphql_only: true, name_in_index: "legacy_count_index", returnable: false
172+
t.field "internal_count", "Int", indexing_only: true, returnable: false
159173
t.index "my_type"
160174
end
161175
end
162176

163-
expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_code")
177+
expect(mapping.dig("_source", "excludes")).to contain_exactly("internal_count")
164178
expect(mapping.fetch("properties")).to include(
165179
"name" => {"type" => "keyword"},
166-
"internal_code" => {"type" => "keyword"}
180+
"internal_count" => {"type" => "integer"}
167181
)
168-
expect(mapping.fetch("properties")).not_to include("legacy_name")
182+
expect(mapping.fetch("properties")).not_to include("legacy_count")
183+
expect(mapping.fetch("properties")).not_to include("legacy_count_index")
169184
end
170185

171186
it "uses full indexed paths in `_source.excludes` for `returnable: false` fields under nested mappings" do
172187
mapping = index_mapping_for "my_type" do |s|
173188
s.object_type "Parent" do |t|
174-
t.field "child", "String", name_in_index: "child_in_index", returnable: false
189+
t.field "child_count_gql", "Int", name_in_index: "child_count", returnable: false
175190
end
176191

177192
s.object_type "Grandparent" do |t|
@@ -187,13 +202,13 @@ module SchemaDefinition
187202
end
188203
end
189204

190-
expect(mapping.dig("_source", "excludes")).to contain_exactly("grandparents_in_index.parent_in_index.child_in_index")
205+
expect(mapping.dig("_source", "excludes")).to contain_exactly("grandparents_in_index.parent_in_index.child_count")
191206
expect(mapping.dig("properties", "grandparents_in_index")).to include(
192207
"type" => "nested",
193208
"properties" => {
194209
"parent_in_index" => {
195210
"properties" => {
196-
"child_in_index" => {"type" => "keyword"}
211+
"child_count" => {"type" => "integer"}
197212
}
198213
}
199214
}

0 commit comments

Comments
 (0)