Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions config/schema/artifacts/datastore_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Team
_routing:
required: true
_size:
Expand Down Expand Up @@ -1312,6 +1315,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: WidgetCurrency
_routing:
required: true
_size:
Expand Down Expand Up @@ -1475,6 +1481,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Widget
_routing:
required: true
_size:
Expand Down Expand Up @@ -1523,6 +1532,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Address
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1586,6 +1598,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Component
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1644,6 +1659,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: ElectricalPart
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1677,6 +1695,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Manufacturer
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1706,6 +1727,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: MechanicalPart
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1759,6 +1783,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: PhysicalStore
_size:
enabled: true
settings:
Expand All @@ -1781,6 +1808,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Sponsor
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1810,6 +1840,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: WidgetWorkspace
_size:
enabled: true
settings:
Expand Down
33 changes: 33 additions & 0 deletions config/schema/artifacts_with_apollo/datastore_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Team
_routing:
required: true
_size:
Expand Down Expand Up @@ -1312,6 +1315,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: WidgetCurrency
_routing:
required: true
_size:
Expand Down Expand Up @@ -1475,6 +1481,9 @@ index_templates:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Widget
_routing:
required: true
_size:
Expand Down Expand Up @@ -1523,6 +1532,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Address
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1586,6 +1598,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Component
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1644,6 +1659,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: ElectricalPart
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1677,6 +1695,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Manufacturer
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1706,6 +1727,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: MechanicalPart
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1759,6 +1783,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: PhysicalStore
_size:
enabled: true
settings:
Expand All @@ -1781,6 +1808,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: Sponsor
_size:
enabled: true
settings:
Expand Down Expand Up @@ -1810,6 +1840,9 @@ indices:
__versions:
type: object
dynamic: 'false'
__typename:
type: constant_keyword
value: WidgetWorkspace
_size:
enabled: true
settings:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def simulate_presence_of_extra_setting(admin, index_definition_name, name, value
let(:output_io) { StringIO.new }
let(:clock) { class_double(::Time, now: ::Time.utc(2024, 3, 20, 12, 0, 0)) }
let(:mapping_removal_note_snippet) { "extra fields listed here will not actually get removed" }
let(:index_meta_fields) { ["__sources", "__versions"] }
let(:index_meta_fields) { ["__sources", "__typename", "__versions"] }

it "idempotently creates an index or index template, avoiding unneeded datastore write calls" do
expect {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ class QueryAdapter
# `BrokerWholesaler` documents from `distribution_channels` would appear in results.
# So we inject:
#
# __typename: { equal_to_any_of: [nil, "OnlineStore", "PhysicalStore"] }
# __typename: { equal_to_any_of: ["OnlineStore", "PhysicalStore"] }
#
# `nil` is included because `PhysicalStore` has a dedicated index where documents lack
# `__typename` — the index itself identifies the type. We only include `nil` when at least
# one of the queried indexes stores only a single type (and thus lacks `__typename`).
# `PhysicalStore` has a dedicated index with `__typename` stored as a `constant_keyword`,
# so it matches the `equal_to_any_of` filter just like documents in the shared index.
class AbstractTypeFilter
def initialize(schema_element_names)
@equal_to_any_of = schema_element_names.equal_to_any_of
Expand All @@ -48,15 +47,8 @@ def call(field:, query:, args:, lookahead:, context:)

return query unless doc_type.shares_index_with_non_subtypes?

schema = context.fetch(:elastic_graph_schema)
subtypes = doc_type.subtypes # Note: subtypes returns all concrete subtypes at any depth
typename_values = subtypes.map(&:name)
# Only include nil when at least one queried index stores only a single type — those
# documents lack __typename (the index itself identifies the type), so nil is needed to
# allow them through.
if doc_type.search_index_definitions.any? { |idx| schema.document_types_stored_in(idx.name).size == 1 }
typename_values += [nil]
end
query.merge_with(internal_filters: [{
"__typename" => {@equal_to_any_of => typename_values}
}])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,16 @@ class QueryAdapter
end

context "when querying an abstract type that shares a search index with a non-subtype" do
it "applies a __typename filter scoped to the queried type's concrete subtypes, including nil for subtypes with dedicated indexes" do
it "applies a __typename filter scoped to the queried type's concrete subtypes" do
query = datastore_query_for(:Query, :retailers, "query { retailers { edges { node { id } } } }")

expect(typename_filter_from(query)).to contain_exactly(nil, "OnlineStore", "PhysicalStore")
expect(typename_filter_from(query)).to contain_exactly("OnlineStore", "PhysicalStore")
end

it "applies a __typename filter on aggregations of this kind of abstract type" do
query = datastore_query_for(:Query, :retail_aggregations, "query { retail_aggregations { nodes { count } } }")

expect(typename_filter_from(query)).to contain_exactly(nil, "OnlineStore", "PhysicalStore")
end

it "omits nil from the __typename filter when all queried indexes store multiple types" do
query = datastore_query_for(:Query, :wholesalers, "query { wholesalers { edges { node { id } } } }")

expect(typename_filter_from(query)).to contain_exactly("DirectWholesaler", "BrokerWholesaler")
expect(typename_filter_from(query)).to contain_exactly("OnlineStore", "PhysicalStore")
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,16 @@ def prepare_for_index(type_name, value, mapping_properties)
# what the concrete subtype is. `__typename` is required on abstract types and indicates that.
eg_meta_by_field_name = @eg_meta_by_field_name_by_concrete_type.fetch(value["__typename"] || type_name)

# Determine whether __typename belongs at this position by checking the index mapping.
typename_in_mapping = mapping_properties&.key?("__typename")
# We only want to consider __typename if it's in the per-record mapping in order to determine
# whether __typename is required on records. When it's a constant_keyword it exists at the index
# level and therefore should be ignored for this purpose.
typename_type = mapping_properties&.dig("__typename", "type")
typename_in_record_mapping = typename_type && typename_type != "constant_keyword"

prepared_fields = value.filter_map do |field_name, field_value|
if field_name == "__typename"
# Only include __typename if the index mapping has it at this position.
[field_name, field_value] if typename_in_mapping
[field_name, field_value] if typename_in_record_mapping
elsif (eg_meta = eg_meta_by_field_name[field_name])
name_in_index = eg_meta.fetch("nameInIndex")
nested_mapping_properties = mapping_properties&.dig(name_in_index, "properties")
Expand All @@ -134,7 +137,7 @@ def prepare_for_index(type_name, value, mapping_properties)

# Inject __typename if the mapping requires it but it's absent from the record
# (e.g. for a concrete type indexed in a mixed-type index).
if typename_in_mapping && !value.key?("__typename")
if typename_in_record_mapping && !value.key?("__typename")
prepared_fields["__typename"] = type_name
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def mappings
.except("type") # `type` is invalid at the mapping root because it always has to be an object.
.then { |mapping| ListCountsMapping.merged_into(mapping, for_type: indexed_type) }
.then do |fm|
Support::HashUtil.deep_merge(fm, {"properties" => {
internal_fields = {
Comment thread
myronmarston marked this conversation as resolved.
"__sources" => {"type" => "keyword"},
"__versions" => {
"type" => "object",
Expand All @@ -317,7 +317,17 @@ def mappings
# a boolean.
"dynamic" => "false"
}
}})
}

# We add __typename for concrete types so they can be matched by __typename filters, which are
# applied when querying abstract types that span multiple indices. Since every document in a
# concrete type's index has the same value, we use constant_keyword here. It stores __typename
# once at the index level with zero per-document overhead.
unless indexed_type.abstract?
internal_fields["__typename"] = {"type" => "constant_keyword", "value" => indexed_type.name}
end
Comment thread
myronmarston marked this conversation as resolved.

Support::HashUtil.deep_merge(fm, {"properties" => internal_fields})
end

{"dynamic" => "strict"}.merge(field_mappings).tap do |hash|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ def expect_out_of_date_artifacts_with_details(example_diff, test_color: false)
expect(error.message.lines.first(8).join).to eq(<<~EOS)
6 schema artifact(s) are out of date. Run `bundle exec rake schema_artifacts:dump` to update the following artifact(s):

1. config/schema/artifacts/datastore_config.yaml (see [1] below for the diff)
1. config/schema/artifacts/datastore_config.yaml (see [1] below for the first 50 lines of the diff)
2. config/schema/artifacts/json_schemas.yaml (see [2] below for the first 50 lines of the diff)
3. config/schema/artifacts/json_schemas_by_version/v1.yaml (see [3] below for the diff)
4. config/schema/artifacts/json_schemas_by_version/v2.yaml (file does not exist)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,20 @@ module SchemaDefinition
end
end

context "for a concrete type with its own index" do
Comment thread
marcdaniels-toast marked this conversation as resolved.
it "includes __typename as a constant_keyword with the type name as the value" do
mapping = index_mapping_for "widgets" do |s|
s.object_type "Widget" do |t|
t.field "id", "ID!"
t.field "name", "String"
t.index "widgets"
end
end

expect(mapping.dig("properties", "__typename")).to eq({"type" => "constant_keyword", "value" => "Widget"})
end
end

context "on a type union" do
include_examples "a type with subtypes", :union_type do
def link_subtype_to_supertype(object_type, supertype_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ module SchemaDefinition
mapping = generate_mapping.call(graphql_only: true)

# Verify that it does not have a property for `size` or `options.size`
expect(mapping.fetch("properties").keys).to contain_exactly("id", "options", "__sources", "__versions")
expect(mapping.fetch("properties").keys).to contain_exactly("id", "options", "__sources", "__versions", "__typename")
expect(mapping.fetch("properties")).to include({
"id" => {"type" => "keyword"},
"options" => {
Expand Down
Loading