From 3f633c06ab0169959cbe9d45f80b2189f0d8cb5a Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Sun, 8 Mar 2026 16:53:12 -0400 Subject: [PATCH 1/7] Add index inheritance infrastructure for concrete types Allow concrete types to inherit index definitions from parent abstract types (unions/interfaces). Types can now be indexed without defining their own index by inheriting from an indexed parent, enabling mixed-type indices where multiple types share a single datastore index. Key changes: - Add own_or_inherited_index_def() to resolve inherited indices from parent types - Add inherits_index?() predicate for types in mixed-type indices - Add recursively_resolve_supertypes() to traverse type hierarchy - Add requires_typename_for_mixed_index runtime metadata field - Inject __typename into data_params for mixed-type indices - Add comprehensive test coverage including transitive inheritance Co-Authored-By: Claude Opus 4.5 --- .../elastic_graph/graphql/schema/type_spec.rb | 32 ++- .../runtime_metadata/object_type.rb | 15 +- .../runtime_metadata/object_type_spec.rb | 15 +- .../runtime_metadata/schema_spec.rb | 3 +- .../schema_definition/mixins/has_indices.rb | 54 ++++- .../mixins/implements_interfaces.rb | 12 ++ .../schema_elements/input_type.rb | 3 +- .../schema_elements/interface_type.rb | 8 + .../schema_elements/object_type.rb | 9 + .../schema_elements/union_type.rb | 13 ++ .../elastic_graph/schema_definition/state.rb | 2 + .../schema_definition/mixins/has_indices.rbs | 2 + .../schema_elements/interface_type.rbs | 1 + .../schema_elements/object_type.rbs | 4 + .../index_definition_names_spec.rb | 22 +++ .../index_inheritance_spec.rb | 184 ++++++++++++++++++ .../update_targets_spec.rb | 77 ++++++++ .../spec_support/runtime_metadata_support.rb | 6 +- 18 files changed, 446 insertions(+), 16 deletions(-) create mode 100644 elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb index ef326adaf..0a46b07b2 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb @@ -53,12 +53,22 @@ class Schema t.field "name", "String" end + schema.object_type "Temperature" do |t| + t.field "id", "ID" + t.field "value", "Float" + end + + schema.object_type "Pressure" do |t| + t.field "id", "ID" + t.field "amount", "Int" + end + schema.union_type "Attribute" do |t| t.subtypes "Color", "Velocity" end schema.union_type "IndexedAttribute" do |t| - t.subtypes "Color", "Velocity" + t.subtypes "Temperature", "Pressure" t.index "attributes" end @@ -266,6 +276,26 @@ class Schema expect(type.unwrap_non_null).to be type end + it "can model a type that inherits an index from a union" do + # Temperature doesn't have its own index, but inherits from IndexedAttribute union + type = schema.type_named("Temperature") + + expect(type.name).to eq "Temperature" + expect(type).to only_satisfy_predicates(:nullable?, :object?, :indexed_document?) + expect(type.unwrap_fully).to be schema.type_named("Temperature") + expect(type.unwrap_non_null).to be type + end + + it "can model a type that inherits an index from an interface" do + # Velocity doesn't have its own index, but inherits from DirectlyIndexedInterface + type = schema.type_named("Velocity") + + expect(type.name).to eq "Velocity" + expect(type).to only_satisfy_predicates(:nullable?, :object?, :indexed_document?) + expect(type.unwrap_fully).to be schema.type_named("Velocity") + expect(type.unwrap_non_null).to be type + end + it "can model an indexed aggregation type" do type = type_for("indexed_aggregation") diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb index f5ddcf1a8..e00ed5780 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb @@ -25,7 +25,10 @@ class ObjectType < ::Data.define( # imply that this type was user-defined; we have recently introduced this metadata and are not yet setting # it for all generated GraphQL types. For now, we are only setting it for specific cases where we need it. :source_type, - :graphql_only_return_type + :graphql_only_return_type, + # Indicates if this type requires __typename to be added during indexing for query-time type resolution. + # True for types in mixed-type indices (types that inherit an index from a parent union/interface). + :requires_typename_for_mixed_index ) UPDATE_TARGETS = "update_targets" INDEX_DEFINITION_NAMES = "index_definition_names" @@ -33,8 +36,9 @@ class ObjectType < ::Data.define( ELASTICGRAPH_CATEGORY = "elasticgraph_category" SOURCE_TYPE = "source_type" GRAPHQL_ONLY_RETURN_TYPE = "graphql_only_return_type" + REQUIRES_TYPENAME_FOR_MIXED_INDEX = "requires_typename_for_mixed_index" - def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:) + def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:, requires_typename_for_mixed_index:) graphql_fields_by_name = graphql_fields_by_name.select { |name, field| field.needed?(name) } super( @@ -43,7 +47,8 @@ def initialize(update_targets:, index_definition_names:, graphql_fields_by_name: graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type + graphql_only_return_type: graphql_only_return_type, + requires_typename_for_mixed_index: requires_typename_for_mixed_index ) end @@ -63,7 +68,8 @@ def self.from_hash(hash) graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: hash[ELASTICGRAPH_CATEGORY]&.to_sym || nil, source_type: hash[SOURCE_TYPE] || nil, - graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE] + graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE], + requires_typename_for_mixed_index: !!hash[REQUIRES_TYPENAME_FOR_MIXED_INDEX] ) end @@ -80,6 +86,7 @@ def to_dumpable_hash GRAPHQL_FIELDS_BY_NAME => dumped_graphql_fields_by_name, GRAPHQL_ONLY_RETURN_TYPE => graphql_only_return_type ? true : nil, INDEX_DEFINITION_NAMES => index_definition_names, + REQUIRES_TYPENAME_FOR_MIXED_INDEX => requires_typename_for_mixed_index ? true : nil, SOURCE_TYPE => source_type, UPDATE_TARGETS => update_targets.map(&:to_dumpable_hash) } diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb index d2c845018..82f1d52ed 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb @@ -24,7 +24,8 @@ module RuntimeMetadata graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + requires_typename_for_mixed_index: false ) end @@ -47,6 +48,18 @@ module RuntimeMetadata expect(type.to_dumpable_hash).to include("graphql_only_return_type" => true) end + it "models `requires_typename_for_mixed_index` as `true` or `nil` so that our runtime metadata pruning can omit nils" do + type = ObjectType.from_hash({}) + + expect(type.requires_typename_for_mixed_index).to eq false + expect(type.to_dumpable_hash).to include("requires_typename_for_mixed_index" => nil) + + type = ObjectType.from_hash({"requires_typename_for_mixed_index" => true}) + + expect(type.requires_typename_for_mixed_index).to eq true + expect(type.to_dumpable_hash).to include("requires_typename_for_mixed_index" => true) + end + it "omits `name_in_index` from dumped GraphQL fields when it matches the GraphQL field name" do relation = relation_with(foreign_key: "other_id") type = object_type_with( diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb index 1b8041cbe..0482b14f4 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb @@ -86,7 +86,8 @@ module RuntimeMetadata }, elasticgraph_category: :some_category, source_type: "SomeType", - graphql_only_return_type: true + graphql_only_return_type: true, + requires_typename_for_mixed_index: false ) }, scalar_types_by_name: { diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb index 0b4c51ec5..eaf0c34c3 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb @@ -100,12 +100,18 @@ def has_own_index_def? !@own_index_def.nil? end - # @return [Boolean] true if this type is a root document type that lives at the document root in the datastore. - # For types with `own_index_def`, returns true. For abstract types with indexed subtypes, overridden in {HasSubtypes}. + # @return [Boolean] true if this type is queryable at the root level of the GraphQL schema (i.e., has direct `Query` fields). def root_document_type? has_own_index_def? end + # @return [Boolean] true if this type inherits an index from a parent abstract type (union/interface). + # When true, the type may share an index with other types (a "mixed-type index"). + # @private + def inherits_index? + own_index_def.nil? && !own_or_inherited_index_def.nil? + end + # Abstract types are rare, so return false. This can be overridden in the host class. # # @private @@ -191,11 +197,12 @@ def override_runtime_metadata(**overrides) def runtime_metadata(extra_update_targets) SchemaArtifacts::RuntimeMetadata::ObjectType.new( update_targets: derived_indexed_types.map(&:runtime_metadata_for_source_type) + [self_update_target].compact + extra_update_targets, - index_definition_names: [own_index_def&.name].compact, + index_definition_names: [own_or_inherited_index_def&.name].compact, graphql_fields_by_name: runtime_metadata_graphql_fields_by_name, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: graphql_only? + graphql_only_return_type: graphql_only?, + requires_typename_for_mixed_index: inherits_index? ).with(**runtime_metadata_overrides) end @@ -262,6 +269,33 @@ def fields_with_sources indexing_fields_by_name_in_index.values.reject { |f| f.source.nil? } end + # Returns the index definition that this type resolves to. This will be one of: + # - This type's own_index_def (if it directly defines an index) + # - An inherited index from a parent abstract type (union/interface) that defines an index + # + # A type can be a subtype of multiple abstract types (e.g., implement multiple interfaces), + # but at most one of those parent types may define an index. If multiple parent types define + # indices, this method raises an error to prevent ambiguity about which index to inherit. + # + # @return [Indexing::Index, nil] the index definition, or nil if this type has no index + # @raise [Errors::SchemaError] if this type is a subtype of multiple indexed abstract types + def own_or_inherited_index_def + return own_index_def if own_index_def + + indexed_parents = recursively_resolve_supertypes.select do |supertype| + supertype.own_index_def + end + + if indexed_parents.size > 1 + parent_names = indexed_parents.map { |p| p.own_index_def.name }.join(", ") + raise Errors::SchemaError, + "The `#{name}` type is a subtype of multiple indexed abstract types (#{parent_names}). " \ + "If a concrete type does not define an index, it may not be a member of multiple indexed abstract types." + end + + indexed_parents.first&.own_index_def + end + private def initialize_has_indices @@ -273,7 +307,9 @@ def initialize_has_indices end def self_update_target - return nil if abstract? || !root_document_type? + # Only concrete types that are indexed in the datastore need an update target. + index_def = own_or_inherited_index_def + return nil if abstract? || index_def.nil? # We exclude `id` from `data_params` because `Indexer::Operator::Update` automatically includes # `params.id` so we don't want it duplicated at `params.data.id` alongside other data params. @@ -284,7 +320,13 @@ def self_update_target [field, SchemaArtifacts::RuntimeMetadata::DynamicParam.new(source_path: field, cardinality: :one)] end - index_runtime_metadata = own_index_def.runtime_metadata + # Add __typename to data_params for types in mixed-type indices. + # RecordPreparer will add __typename during indexing and we need to include it in data_params so it gets indexed. + if inherits_index? + data_params["__typename"] = SchemaArtifacts::RuntimeMetadata::DynamicParam.new(source_path: "__typename", cardinality: :one) + end + + index_runtime_metadata = index_def.runtime_metadata Indexing::UpdateTargetFactory.new_normal_indexing_update_target( type: name, diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb index 9e352da03..e87b5b74d 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb @@ -117,6 +117,18 @@ def to_sdl(&field_arg_selector) generate_sdl(name_section: name_section, &field_arg_selector) end + + private + + # Returns all interface types that this type implements, including ancestor interfaces. + # + # @return [Array] list of interface types this type implements + def resolve_interface_supertypes + implemented_interfaces.flat_map do |interface_ref| + interface = schema_def_state.types_by_name[interface_ref.name] + [interface] + interface.recursively_resolve_supertypes + end + end end end end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb index 286541f66..dbb65b25d 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb @@ -51,7 +51,8 @@ def runtime_metadata(extra_update_targets) graphql_fields_by_name: graphql_input_fields_by_name, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + requires_typename_for_mixed_index: false ) end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb index 7e314979d..47129b2b6 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb @@ -61,6 +61,14 @@ def interface_fields_by_name __getobj__.graphql_fields_by_name end + # Returns all interface types that this interface implements, including ancestor interfaces. + # + # @return [Array] list of interface types this interface implements + # @private + def recursively_resolve_supertypes + resolve_interface_supertypes + end + private def resolve_subtypes diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb index c920ab6bc..a9ae64d3a 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb @@ -38,6 +38,15 @@ class ObjectType < DelegateClass(TypeWithSubfields) include Mixins::ImplementsInterfaces include Mixins::HasReadableToSAndInspect.new { |t| t.name } + # Returns all supertypes of this object type, including union memberships + # and interface ancestors. + # + # @return [Array] list of supertypes + # @private + def recursively_resolve_supertypes + schema_def_state.union_types_by_member_ref[type_ref].to_a + resolve_interface_supertypes + end + # @private def initialize(schema_def_state, name) field_factory = schema_def_state.factory.method(:new_field) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb index b36458af5..6ca3c74e2 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb @@ -94,6 +94,10 @@ def subtype(name) end subtype_refs << type_ref + + # Register reverse lookup so we can efficiently find which unions contain this type + union_types = schema_def_state.union_types_by_member_ref[type_ref] # : ::Set[UnionType] + union_types << self end # Defines multiple subtypes of this union type. @@ -128,6 +132,15 @@ def to_sdl "#{formatted_documentation}union #{name} #{directives_sdl(suffix_with: " ")}= #{subtype_refs.map(&:name).to_a.join(" | ")}" end + # Union types cannot themselves be members of other unions or implement interfaces, + # so they have no supertypes. + # + # @return [Array] empty array + # @private + def recursively_resolve_supertypes + [] + end + # @private def verify_graphql_correctness! # Nothing to verify. `verify_graphql_correctness!` will be called on each subtype automatically. diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb index 9be0171db..b8c73eedc 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb @@ -32,6 +32,7 @@ class State < Struct.new( :scalar_types_by_name, :enum_types_by_name, :implementations_by_interface_ref, + :union_types_by_member_ref, :sdl_parts, :paginated_collection_element_types, :user_defined_fields, @@ -79,6 +80,7 @@ def self.with( scalar_types_by_name: {}, enum_types_by_name: {}, implementations_by_interface_ref: ::Hash.new { |h, k| h[k] = ::Set.new }, + union_types_by_member_ref: ::Hash.new { |h, k| h[k] = ::Set.new }, sdl_parts: [], paginated_collection_element_types: ::Set.new, user_defined_fields: ::Set.new, diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs index e879f397e..4ff0654cd 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs @@ -4,6 +4,8 @@ module ElasticGraph module HasIndices def own_index_def: () -> Indexing::Index? def has_own_index_def?: () -> bool + def own_or_inherited_index_def: () -> Indexing::Index? + def inherits_index?: () -> bool def root_document_type?: () -> bool attr_reader runtime_metadata_overrides: ::Hash[::Symbol, untyped] attr_reader default_graphql_resolver: SchemaArtifacts::RuntimeMetadata::ConfiguredGraphQLResolver? diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs index bccfbaa16..1987c3fd5 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs @@ -18,6 +18,7 @@ module ElasticGraph private def resolve_subtypes: () -> ::Set[SchemaElements::TypeWithSubfields] + def recursively_resolve_supertypes: () -> ::Array[InterfaceType] end end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs index 3982f7d5a..28f9109e6 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs @@ -15,6 +15,10 @@ module ElasticGraph include _IndexableType def initialize: (State, ::String) ?{ (ObjectType) -> void } -> void + + private + + def recursively_resolve_supertypes: () -> ::Array[UnionType | InterfaceType] end end end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb index 98de85ffa..c43858a81 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb @@ -155,6 +155,28 @@ module SchemaDefinition end }.to raise_error(ElasticGraph::Errors::SchemaError, a_string_including("Cannot define an index on `Thing` after initialization is complete")) end + + it "raises an error when a concrete type without an index is a subtype of multiple indexed abstract types" do + expect { + object_type_metadata_for "Widget" do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + link_subtype_to_supertype(t, "ThingA") + link_subtype_to_supertype(t, "ThingB") + end + + s.public_send type_def_method, "ThingA" do |t| + link_supertype_to_subtypes(t, "Widget") + t.index "things_a" + end + + s.public_send type_def_method, "ThingB" do |t| + link_supertype_to_subtypes(t, "Widget") + t.index "things_b" + end + end + }.to raise_error(ElasticGraph::Errors::SchemaError, a_string_including("The `Widget` type is a subtype of multiple indexed abstract types", "things_a, things_b")) + end end end end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb new file mode 100644 index 000000000..42905c0b6 --- /dev/null +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb @@ -0,0 +1,184 @@ +# Copyright 2024 - 2026 Block, Inc. +# +# Use of this source code is governed by an MIT-style +# license that can be found in the LICENSE file or at +# https://opensource.org/licenses/MIT. +# +# frozen_string_literal: true + +require_relative "object_type_metadata_support" + +module ElasticGraph + module SchemaDefinition + RSpec.describe "RuntimeMetadata #object_types_by_name index inheritance" do + include_context "object type metadata support" + + on_a_type_union_or_interface_type do |type_def_method| + context "with a comprehensive index inheritance example (Store types)" do + attr_reader :physical_store_metadata, :mobile_store_metadata, :online_store_metadata, :store_metadata + + before(:context) do + # Build the schema once and reuse it for all tests in this context. + # This tests both union and interface inheritance depending on type_def_method. + # + # Schema structure: + # - Store: abstract type (union or interface) with `index "stores"` + # - PhysicalStore: concrete type with its own `index "physical_stores"` (does not inherit) + # - MobileStore: concrete type that inherits index from Store + # - OnlineStore: concrete type that inherits index from Store + @physical_store_metadata, @mobile_store_metadata, @online_store_metadata, @store_metadata = object_type_metadata_for("PhysicalStore", "MobileStore", "OnlineStore", "Store") do |s| + # PhysicalStore has its own direct index (does not inherit) + s.object_type "PhysicalStore" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.field "address", "String" + link_subtype_to_supertype(t, "Store") + t.index "physical_stores" + end + + # MobileStore and OnlineStore inherit index from Store abstract type + s.object_type "MobileStore" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.field "app_url", "String" + link_subtype_to_supertype(t, "Store") + end + + s.object_type "OnlineStore" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.field "website", "String" + link_subtype_to_supertype(t, "Store") + end + + # Store abstract type (union or interface) with shared index + s.public_send type_def_method, "Store" do |t| + # Interfaces need fields defined + if type_def_method == :interface_type + t.field "id", "ID!" + t.field "name", "String" + end + link_supertype_to_subtypes(t, "PhysicalStore", "MobileStore", "OnlineStore") + t.index "stores" + end + end + end + + describe "index_definition_names" do + it "allows concrete types to inherit the index from their parent abstract type" do + expect(mobile_store_metadata.index_definition_names).to eq ["stores"] + expect(online_store_metadata.index_definition_names).to eq ["stores"] + end + + it "allows concrete types with their own index to not inherit from the parent" do + expect(physical_store_metadata.index_definition_names).to eq ["physical_stores"] + end + + it "gives the parent abstract type its own index" do + expect(store_metadata.index_definition_names).to eq ["stores"] + end + end + + describe "requires_typename_for_mixed_index" do + it "is true for types that inherit an index (sharing a mixed-type index)" do + expect(mobile_store_metadata.requires_typename_for_mixed_index).to eq true + expect(online_store_metadata.requires_typename_for_mixed_index).to eq true + end + + it "is false for types with their own direct index (single-type index)" do + expect(physical_store_metadata.requires_typename_for_mixed_index).to eq false + end + + it "is false for the parent abstract type with its own direct index" do + expect(store_metadata.requires_typename_for_mixed_index).to eq false + end + end + + describe "update_targets data_params" do + it "includes __typename for types that inherit an index (needed for type resolution in mixed-type indices)" do + mobile_store_target = mobile_store_metadata.update_targets.find { |t| t.type == "MobileStore" } + expect(mobile_store_target.data_params.keys).to include("__typename") + expect(mobile_store_target.data_params["__typename"].source_path).to eq "__typename" + + online_store_target = online_store_metadata.update_targets.find { |t| t.type == "OnlineStore" } + expect(online_store_target.data_params.keys).to include("__typename") + expect(online_store_target.data_params["__typename"].source_path).to eq "__typename" + end + + it "does not include __typename for types with their own direct index (single-type index doesn't need type resolution)" do + physical_store_target = physical_store_metadata.update_targets.find { |t| t.type == "PhysicalStore" } + expect(physical_store_target.data_params.keys).not_to include("__typename") + end + + it "includes normal fields in data_params for types that inherit an index" do + mobile_store_target = mobile_store_metadata.update_targets.find { |t| t.type == "MobileStore" } + expect(mobile_store_target.data_params.keys).to include("name", "app_url") + + online_store_target = online_store_metadata.update_targets.find { |t| t.type == "OnlineStore" } + expect(online_store_target.data_params.keys).to include("name", "website") + end + end + + describe "validation" do + it "allows a concrete type to be a subtype of multiple abstract types as long as only one has an index" do + widget_metadata = object_type_metadata_for("Widget") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + link_subtype_to_supertype(t, "Named") + link_subtype_to_supertype(t, "Thing") + end + + # Named has no index + s.public_send type_def_method, "Named" do |t| + t.field "name", "String" if type_def_method == :interface_type + link_supertype_to_subtypes(t, "Widget") + end + + # Thing has an index + s.public_send type_def_method, "Thing" do |t| + t.field "id", "ID!" if type_def_method == :interface_type + link_supertype_to_subtypes(t, "Widget") + t.index "things" + end + end + + expect(widget_metadata.index_definition_names).to eq ["things"] + expect(widget_metadata.requires_typename_for_mixed_index).to eq true + end + end + end + end + + describe "transitive interface inheritance" do + it "allows a concrete type to inherit an index from a grandparent interface" do + gadget_metadata = object_type_metadata_for("Gadget") do |s| + # Gadget implements InterfaceA + s.object_type "Gadget" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.field "category", "String" + t.implements "InterfaceA" + end + + # InterfaceA implements InterfaceB (with the index) + s.interface_type "InterfaceA" do |t| + t.field "name", "String" + t.field "category", "String" + t.implements "InterfaceB" + end + + # InterfaceB has the index + s.interface_type "InterfaceB" do |t| + t.field "name", "String" + t.index "indexed_interfaces" + end + end + + expect(gadget_metadata.index_definition_names).to eq ["indexed_interfaces"] + expect(gadget_metadata.requires_typename_for_mixed_index).to eq true + end + end + end + end +end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb index 018dde3d5..17b222870 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb @@ -1559,6 +1559,83 @@ def update_targets_for( expect(metadata.update_targets.first.data_params).to eq({"id" => dynamic_param_with(source_path: "id", cardinality: :many)}) expect(metadata.update_targets.first.metadata_params).to eq({}) end + + it "inherits routing configuration from the parent abstract type's index when the concrete type has no direct index" do + widget_metadata, component_metadata = object_type_metadata_for("Widget", "Component") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "workspace_id", "ID!" + link_subtype_to_supertype(t, "Thing") + end + + s.object_type "Component" do |t| + t.field "id", "ID!" + t.field "workspace_id", "ID!" + link_subtype_to_supertype(t, "Thing") + end + + s.public_send type_def_method, "Thing" do |t| + link_supertype_to_subtypes(t, "Widget", "Component") + t.index "things" do |i| + i.route_with "workspace_id" + end + end + end + + widget_target = widget_metadata.update_targets.find { |t| t.type == "Widget" } + expect(widget_target.routing_value_source).to eq("workspace_id") + + component_target = component_metadata.update_targets.find { |t| t.type == "Component" } + expect(component_target.routing_value_source).to eq("workspace_id") + end + + it "inherits rollover configuration from the parent abstract type's index when the concrete type has no direct index" do + widget_metadata, component_metadata = object_type_metadata_for("Widget", "Component") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "created_at", "DateTime!" + link_subtype_to_supertype(t, "Thing") + end + + s.object_type "Component" do |t| + t.field "id", "ID!" + t.field "created_at", "DateTime!" + link_subtype_to_supertype(t, "Thing") + end + + s.public_send type_def_method, "Thing" do |t| + link_supertype_to_subtypes(t, "Widget", "Component") + t.index "things" do |i| + i.rollover :monthly, "created_at" + end + end + end + + widget_target = widget_metadata.update_targets.find { |t| t.type == "Widget" } + expect(widget_target.rollover_timestamp_value_source).to eq("created_at") + + component_target = component_metadata.update_targets.find { |t| t.type == "Component" } + expect(component_target.rollover_timestamp_value_source).to eq("created_at") + end + + it "defaults to standard routing when the parent abstract type's index has no custom routing or rollover configuration" do + widget_metadata = object_type_metadata_for("Widget") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + link_subtype_to_supertype(t, "Thing") + end + + s.public_send type_def_method, "Thing" do |t| + link_supertype_to_subtypes(t, "Widget") + t.index "things" + end + end + + widget_target = widget_metadata.update_targets.find { |t| t.type == "Widget" } + expect(widget_target.routing_value_source).to eq("id") # defaults to id when no custom routing + expect(widget_target.rollover_timestamp_value_source).to be_nil # no default for rollover + end end def standard_metadata_params(relationship:) diff --git a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb index 29316874b..020148fa4 100644 --- a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb +++ b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb @@ -42,7 +42,8 @@ def object_type_with( graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false + graphql_only_return_type: false, + requires_typename_for_mixed_index: false ) ObjectType.new( index_definition_names: index_definition_names, @@ -50,7 +51,8 @@ def object_type_with( graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type + graphql_only_return_type: graphql_only_return_type, + requires_typename_for_mixed_index: requires_typename_for_mixed_index ) end From 40ed8d11ca668e261da2348893cb96b099a6dda1 Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Sun, 8 Mar 2026 20:34:18 -0400 Subject: [PATCH 2/7] Fix JSON schema type enum to include transitively indexed types When an abstract type (union/interface) has an index, concrete subtypes inherit that index. The event envelope type enum should include these concrete types (which will actually be indexed) rather than just types with their own direct index definition. Changes: - Use own_or_inherited_index_def instead of root_document_type? to determine which types belong in the JSON schema type enum - Rename variable from root_document_type_names to indexed_type_names for clarity - Add test coverage for event envelope type enum with transitive indexing, including a mixed scenario where some subtypes inherit an index and others have their own index --- .../schema_definition/results.rb | 6 +-- .../schema_definition/json_schema_spec.rb | 37 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb index 238d0b44f..f026d8199 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -335,8 +335,8 @@ def build_public_json_schema raise Errors::SchemaError, "`json_schema_version` must be specified in the schema. To resolve, add `schema.json_schema_version 1` in a schema definition block." end - root_document_type_names = state.object_types_by_name.values - .select { |type| type.root_document_type? && !type.abstract? } + indexed_type_names = state.object_types_by_name.values + .select { |type| !type.own_or_inherited_index_def.nil? && !type.abstract? } .reject { |type| derived_indexing_type_names.include?(type.name) } .map(&:name) @@ -348,7 +348,7 @@ def build_public_json_schema "$schema" => JSON_META_SCHEMA, JSON_SCHEMA_VERSION_KEY => json_schema_version, "$defs" => { - "ElasticGraphEventEnvelope" => Indexing::EventEnvelope.json_schema(root_document_type_names, json_schema_version) + "ElasticGraphEventEnvelope" => Indexing::EventEnvelope.json_schema(indexed_type_names, json_schema_version) }.merge(definitions_by_name) } end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb index 57b07433b..92bdbf4ea 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb @@ -2524,6 +2524,43 @@ def have_json_schema_like(type_name, *args, **kwargs) nil ) end + + it "includes concrete subtypes (not the abstract supertype) in the event envelope type enum" do + json_schema = dump_schema do |s| + # PhysicalStore has its own index + s.object_type "PhysicalStore" do |t| + t.field "id", "ID!" + t.field "name", "String!" + link_subtype_to_supertype(t, "Store") + t.index "physical_stores" + end + + # OnlineStore and MobileStore inherit index from Store + s.object_type "OnlineStore" do |t| + t.field "id", "ID!" + t.field "name", "String!" + link_subtype_to_supertype(t, "Store") + end + + s.object_type "MobileStore" do |t| + t.field "id", "ID!" + t.field "name", "String!" + link_subtype_to_supertype(t, "Store") + end + + s.public_send type_def_method, "Store" do |t| + link_supertype_to_subtypes(t, "PhysicalStore", "OnlineStore", "MobileStore") + t.index "stores" + end + end + + # All concrete types should be in the enum: + # - PhysicalStore (has its own "physical_stores" index) + # - OnlineStore and MobileStore (inherit "stores" index from Store) + # The abstract Store type should NOT be in the enum. + type_definitions = json_schema.fetch("$defs") + expect(envelope_type_enum_values(type_definitions)).to contain_exactly("PhysicalStore", "OnlineStore", "MobileStore") + end end context "that is an embedded type" do From 525bd0d66bdb7ad322b300c5b2ede2deb4760528 Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Wed, 11 Mar 2026 23:43:16 -0400 Subject: [PATCH 3/7] Address PR review feedback and refine index inheritance implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses all review feedback on PR #1067 and makes several improvements to the index inheritance infrastructure: Semantic method separation: - root_document_type? now means "is this type indexed?" (own or inherited) - directly_queryable? now means "does this type have Query fields?" - Updated all callers to use the appropriate method - Query field generation uses directly_queryable? - Apollo @key directive uses root_document_type? (entities need indexing, not Query fields) Method renaming for clarity: - own_or_inherited_index_def → index_def - inherits_index? → needs_typename? (later inlined) - Removed unnecessary aliases Set-based supertypes: - recursively_resolve_supertypes now returns Set instead of Array - Automatic deduplication, semantically correct - Union memberships tracked as Sets, no need for .to_a conversion Consolidated recursively_resolve_supertypes: - Single implementation in ImplementsInterfaces mixin - Handles both union memberships and interface ancestors - Removed overrides from ObjectType and InterfaceType - UnionType override remains (returns Set[], has no supertypes) __typename handling improvements: - Added indexing_fields_by_name_in_index override in ObjectType - __typename automatically included for types in mixed-type indices - Flows naturally into data_params without manual addition - Removed requires_typename_for_mixed_index runtime metadata field - Inlined needs_typename? logic (only one caller) Code cleanup: - Removed dead root_document_type? methods from enum/scalar/type_with_subfields - Removed Velocity from Attribute union in test schema (mixed indexed/non-indexed) - Simplified EntityTypeExtension overrides All changes maintain 100% test coverage. Co-Authored-By: Claude Opus 4.5 --- .../apollo/schema_definition/api_extension.rb | 7 ++ .../entity_type_extension.rb | 4 + .../schema_definition/factory_extension.rb | 5 -- .../elastic_graph/graphql/schema/type_spec.rb | 2 +- .../runtime_metadata/object_type.rb | 15 +--- .../runtime_metadata/object_type_spec.rb | 15 +--- .../runtime_metadata/schema_spec.rb | 3 +- .../schema_definition/mixins/has_indices.rb | 77 ++++++++----------- .../schema_definition/mixins/has_subtypes.rb | 7 ++ .../mixins/implements_interfaces.rb | 20 ++--- .../schema_definition/results.rb | 10 +-- .../schema_elements/enum_type.rb | 5 -- .../schema_elements/input_type.rb | 3 +- .../schema_elements/interface_type.rb | 8 -- .../schema_elements/object_type.rb | 16 ++-- .../schema_elements/scalar_type.rb | 5 -- .../schema_elements/type_with_subfields.rb | 5 -- .../schema_elements/union_type.rb | 7 +- .../schema_definition/mixins/has_indices.rbs | 4 +- .../mixins/implements_interfaces.rbs | 1 + .../schema_elements/interface_type.rbs | 1 - .../schema_elements/object_type.rbs | 4 - .../index_inheritance_spec.rb | 17 ---- .../spec_support/runtime_metadata_support.rb | 6 +- 24 files changed, 93 insertions(+), 154 deletions(-) diff --git a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb index 4ac69fbd5..77f7b969d 100644 --- a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb +++ b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/api_extension.rb @@ -336,6 +336,13 @@ def define_apollo_schema_elements end end + # Add @key directives to all root document types with an id field. + # This happens after schema definition is complete so root_document_type? sees the complete type hierarchy. + state.object_types_by_name.values + .grep(ElasticGraph::SchemaDefinition::SchemaElements::ObjectType) + .select { |object_type| object_type.root_document_type? && object_type.graphql_fields_by_name.key?("id") } + .each { |object_type| object_type.apollo_key fields: "id" } + entity_types = state.object_types_by_name.values.select do |object_type| object_type.directives.any? do |directive| directive.name == "key" && directive.arguments.fetch(:resolvable, true) diff --git a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb index a6bb4a292..7f22a74f3 100644 --- a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb +++ b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb @@ -26,6 +26,10 @@ def graphql_fields_by_name def root_document_type? false end + + def directly_queryable? + false + end end end end diff --git a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/factory_extension.rb b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/factory_extension.rb index 1d6e1e3ad..c51405584 100644 --- a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/factory_extension.rb +++ b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/factory_extension.rb @@ -66,17 +66,12 @@ def new_interface_type(name) end end - # Here we override `object_type` in order to automatically add the apollo `@key` directive to indexed types. def new_object_type(name) super(name) do |raw_type| raw_type.extend ObjectTypeExtension type = raw_type # : ElasticGraph::SchemaDefinition::SchemaElements::ObjectType & ObjectTypeExtension yield type if block_given? - - if type.root_document_type? && type.graphql_fields_by_name.key?("id") - type.apollo_key fields: "id" - end end end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb index 0a46b07b2..7c474f07a 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/schema/type_spec.rb @@ -64,7 +64,7 @@ class Schema end schema.union_type "Attribute" do |t| - t.subtypes "Color", "Velocity" + t.subtypes "Color" end schema.union_type "IndexedAttribute" do |t| diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb index e00ed5780..f5ddcf1a8 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/object_type.rb @@ -25,10 +25,7 @@ class ObjectType < ::Data.define( # imply that this type was user-defined; we have recently introduced this metadata and are not yet setting # it for all generated GraphQL types. For now, we are only setting it for specific cases where we need it. :source_type, - :graphql_only_return_type, - # Indicates if this type requires __typename to be added during indexing for query-time type resolution. - # True for types in mixed-type indices (types that inherit an index from a parent union/interface). - :requires_typename_for_mixed_index + :graphql_only_return_type ) UPDATE_TARGETS = "update_targets" INDEX_DEFINITION_NAMES = "index_definition_names" @@ -36,9 +33,8 @@ class ObjectType < ::Data.define( ELASTICGRAPH_CATEGORY = "elasticgraph_category" SOURCE_TYPE = "source_type" GRAPHQL_ONLY_RETURN_TYPE = "graphql_only_return_type" - REQUIRES_TYPENAME_FOR_MIXED_INDEX = "requires_typename_for_mixed_index" - def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:, requires_typename_for_mixed_index:) + def initialize(update_targets:, index_definition_names:, graphql_fields_by_name:, elasticgraph_category:, source_type:, graphql_only_return_type:) graphql_fields_by_name = graphql_fields_by_name.select { |name, field| field.needed?(name) } super( @@ -47,8 +43,7 @@ def initialize(update_targets:, index_definition_names:, graphql_fields_by_name: graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type, - requires_typename_for_mixed_index: requires_typename_for_mixed_index + graphql_only_return_type: graphql_only_return_type ) end @@ -68,8 +63,7 @@ def self.from_hash(hash) graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: hash[ELASTICGRAPH_CATEGORY]&.to_sym || nil, source_type: hash[SOURCE_TYPE] || nil, - graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE], - requires_typename_for_mixed_index: !!hash[REQUIRES_TYPENAME_FOR_MIXED_INDEX] + graphql_only_return_type: !!hash[GRAPHQL_ONLY_RETURN_TYPE] ) end @@ -86,7 +80,6 @@ def to_dumpable_hash GRAPHQL_FIELDS_BY_NAME => dumped_graphql_fields_by_name, GRAPHQL_ONLY_RETURN_TYPE => graphql_only_return_type ? true : nil, INDEX_DEFINITION_NAMES => index_definition_names, - REQUIRES_TYPENAME_FOR_MIXED_INDEX => requires_typename_for_mixed_index ? true : nil, SOURCE_TYPE => source_type, UPDATE_TARGETS => update_targets.map(&:to_dumpable_hash) } diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb index 82f1d52ed..d2c845018 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/object_type_spec.rb @@ -24,8 +24,7 @@ module RuntimeMetadata graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false, - requires_typename_for_mixed_index: false + graphql_only_return_type: false ) end @@ -48,18 +47,6 @@ module RuntimeMetadata expect(type.to_dumpable_hash).to include("graphql_only_return_type" => true) end - it "models `requires_typename_for_mixed_index` as `true` or `nil` so that our runtime metadata pruning can omit nils" do - type = ObjectType.from_hash({}) - - expect(type.requires_typename_for_mixed_index).to eq false - expect(type.to_dumpable_hash).to include("requires_typename_for_mixed_index" => nil) - - type = ObjectType.from_hash({"requires_typename_for_mixed_index" => true}) - - expect(type.requires_typename_for_mixed_index).to eq true - expect(type.to_dumpable_hash).to include("requires_typename_for_mixed_index" => true) - end - it "omits `name_in_index` from dumped GraphQL fields when it matches the GraphQL field name" do relation = relation_with(foreign_key: "other_id") type = object_type_with( diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb index 0482b14f4..1b8041cbe 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/schema_spec.rb @@ -86,8 +86,7 @@ module RuntimeMetadata }, elasticgraph_category: :some_category, source_type: "SomeType", - graphql_only_return_type: true, - requires_typename_for_mixed_index: false + graphql_only_return_type: true ) }, scalar_types_by_name: { diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb index eaf0c34c3..e249a2e2a 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb @@ -100,16 +100,40 @@ def has_own_index_def? !@own_index_def.nil? end - # @return [Boolean] true if this type is queryable at the root level of the GraphQL schema (i.e., has direct `Query` fields). + # Resolves this type's index definition. This will be one of: + # - This type's own_index_def (if it directly defines an index) + # - An inherited index from a parent abstract type (union/interface) that defines an index + # + # A type can be a subtype of multiple abstract types (e.g., implement multiple interfaces), + # but at most one of those parent types may define an index. If multiple parent types are + # indexed, this method raises an error to prevent ambiguity about which index to inherit. + # + # @return [Indexing::Index, nil] the index definition, or nil if this type has no index + # @raise [Errors::SchemaError] if this type is a subtype of multiple indexed abstract types + def index_def + return own_index_def if has_own_index_def? + + indexed_parents = recursively_resolve_supertypes.select(&:has_own_index_def?) + + if indexed_parents.size > 1 + parent_names = indexed_parents.map { |p| p.own_index_def.name }.join(", ") + raise Errors::SchemaError, + "The `#{name}` type is a subtype of multiple indexed abstract types (#{parent_names}). " \ + "If a concrete type does not define an index, it may not be a member of multiple indexed abstract types." + end + + indexed_parents.first&.own_index_def + end + + # @return [Boolean] true if this type is a root document type that lives at a document root in the datastore (is indexed). + # This returns true for types with their own index definition or types that inherit an index from a parent abstract type. def root_document_type? - has_own_index_def? + !index_def.nil? end - # @return [Boolean] true if this type inherits an index from a parent abstract type (union/interface). - # When true, the type may share an index with other types (a "mixed-type index"). - # @private - def inherits_index? - own_index_def.nil? && !own_or_inherited_index_def.nil? + # @return [Boolean] true if this type is directly queryable from the root `Query` type. + def directly_queryable? + has_own_index_def? end # Abstract types are rare, so return false. This can be overridden in the host class. @@ -197,12 +221,11 @@ def override_runtime_metadata(**overrides) def runtime_metadata(extra_update_targets) SchemaArtifacts::RuntimeMetadata::ObjectType.new( update_targets: derived_indexed_types.map(&:runtime_metadata_for_source_type) + [self_update_target].compact + extra_update_targets, - index_definition_names: [own_or_inherited_index_def&.name].compact, + index_definition_names: [index_def&.name].compact, graphql_fields_by_name: runtime_metadata_graphql_fields_by_name, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: graphql_only?, - requires_typename_for_mixed_index: inherits_index? + graphql_only_return_type: graphql_only? ).with(**runtime_metadata_overrides) end @@ -269,33 +292,6 @@ def fields_with_sources indexing_fields_by_name_in_index.values.reject { |f| f.source.nil? } end - # Returns the index definition that this type resolves to. This will be one of: - # - This type's own_index_def (if it directly defines an index) - # - An inherited index from a parent abstract type (union/interface) that defines an index - # - # A type can be a subtype of multiple abstract types (e.g., implement multiple interfaces), - # but at most one of those parent types may define an index. If multiple parent types define - # indices, this method raises an error to prevent ambiguity about which index to inherit. - # - # @return [Indexing::Index, nil] the index definition, or nil if this type has no index - # @raise [Errors::SchemaError] if this type is a subtype of multiple indexed abstract types - def own_or_inherited_index_def - return own_index_def if own_index_def - - indexed_parents = recursively_resolve_supertypes.select do |supertype| - supertype.own_index_def - end - - if indexed_parents.size > 1 - parent_names = indexed_parents.map { |p| p.own_index_def.name }.join(", ") - raise Errors::SchemaError, - "The `#{name}` type is a subtype of multiple indexed abstract types (#{parent_names}). " \ - "If a concrete type does not define an index, it may not be a member of multiple indexed abstract types." - end - - indexed_parents.first&.own_index_def - end - private def initialize_has_indices @@ -308,7 +304,6 @@ def initialize_has_indices def self_update_target # Only concrete types that are indexed in the datastore need an update target. - index_def = own_or_inherited_index_def return nil if abstract? || index_def.nil? # We exclude `id` from `data_params` because `Indexer::Operator::Update` automatically includes @@ -320,12 +315,6 @@ def self_update_target [field, SchemaArtifacts::RuntimeMetadata::DynamicParam.new(source_path: field, cardinality: :one)] end - # Add __typename to data_params for types in mixed-type indices. - # RecordPreparer will add __typename during indexing and we need to include it in data_params so it gets indexed. - if inherits_index? - data_params["__typename"] = SchemaArtifacts::RuntimeMetadata::DynamicParam.new(source_path: "__typename", cardinality: :one) - end - index_runtime_metadata = index_def.runtime_metadata Indexing::UpdateTargetFactory.new_normal_indexing_update_target( diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb index b2502fdb4..099579257 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb @@ -38,6 +38,13 @@ def root_document_type? super || subtypes_are_root_document_types? end + # A parent type is queryable if all of its subtypes are root document types (via a direct or inherited index) + # even if those subtypes aren't themselves directly queryable. This is why this doesn't delegate to a + # subtypes_are_directly_queryable helper. + def directly_queryable? + super || subtypes_are_root_document_types? + end + def recursively_resolve_subtypes resolve_subtypes.flat_map do |type| type.is_a?(HasSubtypes) ? (_ = type).recursively_resolve_subtypes : [type] diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb index e87b5b74d..4143ca879 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb @@ -118,16 +118,18 @@ def to_sdl(&field_arg_selector) generate_sdl(name_section: name_section, &field_arg_selector) end - private - - # Returns all interface types that this type implements, including ancestor interfaces. + # Returns all supertypes of this type, including union memberships and interface ancestors. # - # @return [Array] list of interface types this type implements - def resolve_interface_supertypes - implemented_interfaces.flat_map do |interface_ref| - interface = schema_def_state.types_by_name[interface_ref.name] - [interface] + interface.recursively_resolve_supertypes - end + # @return [Set] set of supertypes + # @private + def recursively_resolve_supertypes + union_memberships = schema_def_state.union_types_by_member_ref[type_ref] + + interface_supertypes = implemented_interfaces.flat_map do |interface_ref| + [interface_ref.resolved] + interface.recursively_resolve_supertypes.to_a + end.to_set + + union_memberships | interface_supertypes end end end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb index f026d8199..0bd9ad604 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/results.rb @@ -119,7 +119,7 @@ def define_root_graphql_type query_type.documentation "The query entry point for the entire schema." query_type.resolve_fields_with nil - state.types_by_name.values.select(&:root_document_type?).sort_by(&:name).each do |type| + state.object_types_by_name.values.select(&:directly_queryable?).sort_by(&:name).each do |type| # @type var root_doc_type: Mixins::HasIndices & _Type root_doc_type = _ = type @@ -212,7 +212,7 @@ def build_runtime_metadata enum_generator = state.factory.new_enums_for_root_document_types sort_order_enum_types_by_name = state.object_types_by_name.values - .select(&:root_document_type?) + .select(&:directly_queryable?) .filter_map { |type| enum_generator.sort_order_enum_for(_ = type) } .to_h { |enum_type| [(_ = enum_type).name, (_ = enum_type).runtime_metadata] } @@ -335,8 +335,8 @@ def build_public_json_schema raise Errors::SchemaError, "`json_schema_version` must be specified in the schema. To resolve, add `schema.json_schema_version 1` in a schema definition block." end - indexed_type_names = state.object_types_by_name.values - .select { |type| !type.own_or_inherited_index_def.nil? && !type.abstract? } + root_document_type_names = state.object_types_by_name.values + .select { |type| type.root_document_type? && !type.abstract? } .reject { |type| derived_indexing_type_names.include?(type.name) } .map(&:name) @@ -348,7 +348,7 @@ def build_public_json_schema "$schema" => JSON_META_SCHEMA, JSON_SCHEMA_VERSION_KEY => json_schema_version, "$defs" => { - "ElasticGraphEventEnvelope" => Indexing::EventEnvelope.json_schema(indexed_type_names, json_schema_version) + "ElasticGraphEventEnvelope" => Indexing::EventEnvelope.json_schema(root_document_type_names, json_schema_version) }.merge(definitions_by_name) } end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/enum_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/enum_type.rb index e413318ce..0bdb27b60 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/enum_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/enum_type.rb @@ -161,11 +161,6 @@ def to_indexing_field_type Indexing::FieldType::Enum.new(values_by_name.keys) end - # @return [false] enum types are never root document types - def root_document_type? - false - end - # @return [EnumType] converts the enum type to its input form for when different naming is used for input vs output enums. def as_input input_name = type_ref diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb index dbb65b25d..286541f66 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/input_type.rb @@ -51,8 +51,7 @@ def runtime_metadata(extra_update_targets) graphql_fields_by_name: graphql_input_fields_by_name, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false, - requires_typename_for_mixed_index: false + graphql_only_return_type: false ) end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb index 47129b2b6..7e314979d 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/interface_type.rb @@ -61,14 +61,6 @@ def interface_fields_by_name __getobj__.graphql_fields_by_name end - # Returns all interface types that this interface implements, including ancestor interfaces. - # - # @return [Array] list of interface types this interface implements - # @private - def recursively_resolve_supertypes - resolve_interface_supertypes - end - private def resolve_subtypes diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb index a9ae64d3a..fb3a40d3d 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb @@ -38,13 +38,17 @@ class ObjectType < DelegateClass(TypeWithSubfields) include Mixins::ImplementsInterfaces include Mixins::HasReadableToSAndInspect.new { |t| t.name } - # Returns all supertypes of this object type, including union memberships - # and interface ancestors. - # - # @return [Array] list of supertypes + # @return [Hash] fields that will be indexed, including __typename for mixed-type indices # @private - def recursively_resolve_supertypes - schema_def_state.union_types_by_member_ref[type_ref].to_a + resolve_interface_supertypes + def indexing_fields_by_name_in_index + fields = super + + # Add __typename for types in mixed-type indices (types that inherit an index from a parent union/interface) + if !has_own_index_def? && !index_def.nil? + fields.merge("__typename" => schema_def_state.factory.new_field(name: "__typename", type: "String", parent_type: self)) + else + fields + end end # @private diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb index 2d63b30eb..8e2703cd2 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb @@ -236,11 +236,6 @@ def derived_graphql_types end end - # @private - def root_document_type? - false - end - private EQUAL_TO_ANY_OF_DOC = <<~EOS diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_with_subfields.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_with_subfields.rb index 3449a0f9c..eb45743ac 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_with_subfields.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_with_subfields.rb @@ -470,11 +470,6 @@ def aggregated_values_type schema_def_state.type_ref("NonNumeric").as_aggregated_values end - # @private - def root_document_type? - false - end - # @private def to_indexing_field_type Indexing::FieldType::Object.new( diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb index 6ca3c74e2..b8c4b4d1b 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/union_type.rb @@ -96,8 +96,7 @@ def subtype(name) subtype_refs << type_ref # Register reverse lookup so we can efficiently find which unions contain this type - union_types = schema_def_state.union_types_by_member_ref[type_ref] # : ::Set[UnionType] - union_types << self + schema_def_state.union_types_by_member_ref[type_ref] << self end # Defines multiple subtypes of this union type. @@ -135,10 +134,10 @@ def to_sdl # Union types cannot themselves be members of other unions or implement interfaces, # so they have no supertypes. # - # @return [Array] empty array + # @return [Set] empty set # @private def recursively_resolve_supertypes - [] + Set[] end # @private diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs index 4ff0654cd..5502ede0e 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/has_indices.rbs @@ -4,9 +4,9 @@ module ElasticGraph module HasIndices def own_index_def: () -> Indexing::Index? def has_own_index_def?: () -> bool - def own_or_inherited_index_def: () -> Indexing::Index? - def inherits_index?: () -> bool + def index_def: () -> Indexing::Index? def root_document_type?: () -> bool + def directly_queryable?: () -> bool attr_reader runtime_metadata_overrides: ::Hash[::Symbol, untyped] attr_reader default_graphql_resolver: SchemaArtifacts::RuntimeMetadata::ConfiguredGraphQLResolver? def index: (::String, ::Hash[::Symbol, ::String | ::Integer]) ?{ (Indexing::Index) -> void } -> Indexing::Index diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/implements_interfaces.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/implements_interfaces.rbs index b733b437b..2245d1356 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/implements_interfaces.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/mixins/implements_interfaces.rbs @@ -5,6 +5,7 @@ module ElasticGraph def implements: (*::String) -> void attr_reader implemented_interfaces: ::Array[SchemaElements::TypeReference] def verify_graphql_correctness!: () -> void + def recursively_resolve_supertypes: () -> ::Set[SchemaElements::UnionType | SchemaElements::InterfaceType] end end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs index 1987c3fd5..bccfbaa16 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/interface_type.rbs @@ -18,7 +18,6 @@ module ElasticGraph private def resolve_subtypes: () -> ::Set[SchemaElements::TypeWithSubfields] - def recursively_resolve_supertypes: () -> ::Array[InterfaceType] end end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs index 28f9109e6..3982f7d5a 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/object_type.rbs @@ -15,10 +15,6 @@ module ElasticGraph include _IndexableType def initialize: (State, ::String) ?{ (ObjectType) -> void } -> void - - private - - def recursively_resolve_supertypes: () -> ::Array[UnionType | InterfaceType] end end end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb index 42905c0b6..870dcb7ad 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb @@ -79,21 +79,6 @@ module SchemaDefinition end end - describe "requires_typename_for_mixed_index" do - it "is true for types that inherit an index (sharing a mixed-type index)" do - expect(mobile_store_metadata.requires_typename_for_mixed_index).to eq true - expect(online_store_metadata.requires_typename_for_mixed_index).to eq true - end - - it "is false for types with their own direct index (single-type index)" do - expect(physical_store_metadata.requires_typename_for_mixed_index).to eq false - end - - it "is false for the parent abstract type with its own direct index" do - expect(store_metadata.requires_typename_for_mixed_index).to eq false - end - end - describe "update_targets data_params" do it "includes __typename for types that inherit an index (needed for type resolution in mixed-type indices)" do mobile_store_target = mobile_store_metadata.update_targets.find { |t| t.type == "MobileStore" } @@ -144,7 +129,6 @@ module SchemaDefinition end expect(widget_metadata.index_definition_names).to eq ["things"] - expect(widget_metadata.requires_typename_for_mixed_index).to eq true end end end @@ -176,7 +160,6 @@ module SchemaDefinition end expect(gadget_metadata.index_definition_names).to eq ["indexed_interfaces"] - expect(gadget_metadata.requires_typename_for_mixed_index).to eq true end end end diff --git a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb index 020148fa4..29316874b 100644 --- a/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb +++ b/spec_support/lib/elastic_graph/spec_support/runtime_metadata_support.rb @@ -42,8 +42,7 @@ def object_type_with( graphql_fields_by_name: {}, elasticgraph_category: nil, source_type: nil, - graphql_only_return_type: false, - requires_typename_for_mixed_index: false + graphql_only_return_type: false ) ObjectType.new( index_definition_names: index_definition_names, @@ -51,8 +50,7 @@ def object_type_with( graphql_fields_by_name: graphql_fields_by_name, elasticgraph_category: elasticgraph_category, source_type: source_type, - graphql_only_return_type: graphql_only_return_type, - requires_typename_for_mixed_index: requires_typename_for_mixed_index + graphql_only_return_type: graphql_only_return_type ) end From eb855d9600e81fa1ccaa794762adad4dd5845d9b Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Fri, 13 Mar 2026 10:57:18 -0400 Subject: [PATCH 4/7] Consolidate index inheritance tests into existing spec files Consolidated tests from index_inheritance_spec.rb into appropriate existing spec files for better test organization: - Moved concrete type inheritance tests to index_definition_names_spec.rb - Moved transitive interface inheritance test to index_definition_names_spec.rb - Moved __typename data_params tests to update_targets_spec.rb - Removed redundant test from json_schema_spec.rb - Deleted index_inheritance_spec.rb Tests are now co-located with the features they test at the right abstraction layers. Maintained 100% line and branch coverage. Also fixed Set union operator in implements_interfaces.rb to use idiomatic | instead of +. --- .../mixins/implements_interfaces.rb | 3 +- .../schema_definition/json_schema_spec.rb | 37 ---- .../index_definition_names_spec.rb | 50 ++++++ .../index_inheritance_spec.rb | 167 ------------------ .../update_targets_spec.rb | 42 +++++ 5 files changed, 94 insertions(+), 205 deletions(-) delete mode 100644 elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb index 4143ca879..2edcf32c2 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/implements_interfaces.rb @@ -126,7 +126,8 @@ def recursively_resolve_supertypes union_memberships = schema_def_state.union_types_by_member_ref[type_ref] interface_supertypes = implemented_interfaces.flat_map do |interface_ref| - [interface_ref.resolved] + interface.recursively_resolve_supertypes.to_a + interface = interface_ref.resolved + [interface] + interface.recursively_resolve_supertypes.to_a end.to_set union_memberships | interface_supertypes diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb index 92bdbf4ea..57b07433b 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/json_schema_spec.rb @@ -2524,43 +2524,6 @@ def have_json_schema_like(type_name, *args, **kwargs) nil ) end - - it "includes concrete subtypes (not the abstract supertype) in the event envelope type enum" do - json_schema = dump_schema do |s| - # PhysicalStore has its own index - s.object_type "PhysicalStore" do |t| - t.field "id", "ID!" - t.field "name", "String!" - link_subtype_to_supertype(t, "Store") - t.index "physical_stores" - end - - # OnlineStore and MobileStore inherit index from Store - s.object_type "OnlineStore" do |t| - t.field "id", "ID!" - t.field "name", "String!" - link_subtype_to_supertype(t, "Store") - end - - s.object_type "MobileStore" do |t| - t.field "id", "ID!" - t.field "name", "String!" - link_subtype_to_supertype(t, "Store") - end - - s.public_send type_def_method, "Store" do |t| - link_supertype_to_subtypes(t, "PhysicalStore", "OnlineStore", "MobileStore") - t.index "stores" - end - end - - # All concrete types should be in the enum: - # - PhysicalStore (has its own "physical_stores" index) - # - OnlineStore and MobileStore (inherit "stores" index from Store) - # The abstract Store type should NOT be in the enum. - type_definitions = json_schema.fetch("$defs") - expect(envelope_type_enum_values(type_definitions)).to contain_exactly("PhysicalStore", "OnlineStore", "MobileStore") - end end context "that is an embedded type" do diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb index c43858a81..792c2d369 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb @@ -107,6 +107,30 @@ module SchemaDefinition expect(metadata.index_definition_names).to eq ["things"] end + it "allows concrete subtypes to inherit the index from the supertype" do + widget_metadata, component_metadata = object_type_metadata_for("Widget", "Component") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + link_subtype_to_supertype(t, "Thing") + end + + s.object_type "Component" do |t| + t.field "id", "ID!" + t.field "size", "Int" + link_subtype_to_supertype(t, "Thing") + end + + s.public_send type_def_method, "Thing" do |t| + link_supertype_to_subtypes(t, "Widget", "Component") + t.index "things" + end + end + + expect(widget_metadata.index_definition_names).to eq ["things"] + expect(component_metadata.index_definition_names).to eq ["things"] + end + it "does not dump any when no direct index is defined on it (even if the subtypes have indices)" do metadata = object_type_metadata_for "Thing" do |s| s.object_type "Widget" do |t| @@ -178,6 +202,32 @@ module SchemaDefinition }.to raise_error(ElasticGraph::Errors::SchemaError, a_string_including("The `Widget` type is a subtype of multiple indexed abstract types", "things_a, things_b")) end end + + context "with transitive interface inheritance" do + it "allows a concrete type to inherit an index from a grandparent interface" do + widget_metadata = object_type_metadata_for("Widget") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.field "category", "String" + t.implements "InterfaceA" + end + + s.interface_type "InterfaceA" do |t| + t.field "name", "String" + t.field "category", "String" + t.implements "InterfaceB" + end + + s.interface_type "InterfaceB" do |t| + t.field "name", "String" + t.index "indexed_interfaces" + end + end + + expect(widget_metadata.index_definition_names).to eq ["indexed_interfaces"] + end + end end end end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb deleted file mode 100644 index 870dcb7ad..000000000 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_inheritance_spec.rb +++ /dev/null @@ -1,167 +0,0 @@ -# Copyright 2024 - 2026 Block, Inc. -# -# Use of this source code is governed by an MIT-style -# license that can be found in the LICENSE file or at -# https://opensource.org/licenses/MIT. -# -# frozen_string_literal: true - -require_relative "object_type_metadata_support" - -module ElasticGraph - module SchemaDefinition - RSpec.describe "RuntimeMetadata #object_types_by_name index inheritance" do - include_context "object type metadata support" - - on_a_type_union_or_interface_type do |type_def_method| - context "with a comprehensive index inheritance example (Store types)" do - attr_reader :physical_store_metadata, :mobile_store_metadata, :online_store_metadata, :store_metadata - - before(:context) do - # Build the schema once and reuse it for all tests in this context. - # This tests both union and interface inheritance depending on type_def_method. - # - # Schema structure: - # - Store: abstract type (union or interface) with `index "stores"` - # - PhysicalStore: concrete type with its own `index "physical_stores"` (does not inherit) - # - MobileStore: concrete type that inherits index from Store - # - OnlineStore: concrete type that inherits index from Store - @physical_store_metadata, @mobile_store_metadata, @online_store_metadata, @store_metadata = object_type_metadata_for("PhysicalStore", "MobileStore", "OnlineStore", "Store") do |s| - # PhysicalStore has its own direct index (does not inherit) - s.object_type "PhysicalStore" do |t| - t.field "id", "ID!" - t.field "name", "String" - t.field "address", "String" - link_subtype_to_supertype(t, "Store") - t.index "physical_stores" - end - - # MobileStore and OnlineStore inherit index from Store abstract type - s.object_type "MobileStore" do |t| - t.field "id", "ID!" - t.field "name", "String" - t.field "app_url", "String" - link_subtype_to_supertype(t, "Store") - end - - s.object_type "OnlineStore" do |t| - t.field "id", "ID!" - t.field "name", "String" - t.field "website", "String" - link_subtype_to_supertype(t, "Store") - end - - # Store abstract type (union or interface) with shared index - s.public_send type_def_method, "Store" do |t| - # Interfaces need fields defined - if type_def_method == :interface_type - t.field "id", "ID!" - t.field "name", "String" - end - link_supertype_to_subtypes(t, "PhysicalStore", "MobileStore", "OnlineStore") - t.index "stores" - end - end - end - - describe "index_definition_names" do - it "allows concrete types to inherit the index from their parent abstract type" do - expect(mobile_store_metadata.index_definition_names).to eq ["stores"] - expect(online_store_metadata.index_definition_names).to eq ["stores"] - end - - it "allows concrete types with their own index to not inherit from the parent" do - expect(physical_store_metadata.index_definition_names).to eq ["physical_stores"] - end - - it "gives the parent abstract type its own index" do - expect(store_metadata.index_definition_names).to eq ["stores"] - end - end - - describe "update_targets data_params" do - it "includes __typename for types that inherit an index (needed for type resolution in mixed-type indices)" do - mobile_store_target = mobile_store_metadata.update_targets.find { |t| t.type == "MobileStore" } - expect(mobile_store_target.data_params.keys).to include("__typename") - expect(mobile_store_target.data_params["__typename"].source_path).to eq "__typename" - - online_store_target = online_store_metadata.update_targets.find { |t| t.type == "OnlineStore" } - expect(online_store_target.data_params.keys).to include("__typename") - expect(online_store_target.data_params["__typename"].source_path).to eq "__typename" - end - - it "does not include __typename for types with their own direct index (single-type index doesn't need type resolution)" do - physical_store_target = physical_store_metadata.update_targets.find { |t| t.type == "PhysicalStore" } - expect(physical_store_target.data_params.keys).not_to include("__typename") - end - - it "includes normal fields in data_params for types that inherit an index" do - mobile_store_target = mobile_store_metadata.update_targets.find { |t| t.type == "MobileStore" } - expect(mobile_store_target.data_params.keys).to include("name", "app_url") - - online_store_target = online_store_metadata.update_targets.find { |t| t.type == "OnlineStore" } - expect(online_store_target.data_params.keys).to include("name", "website") - end - end - - describe "validation" do - it "allows a concrete type to be a subtype of multiple abstract types as long as only one has an index" do - widget_metadata = object_type_metadata_for("Widget") do |s| - s.object_type "Widget" do |t| - t.field "id", "ID!" - t.field "name", "String" - link_subtype_to_supertype(t, "Named") - link_subtype_to_supertype(t, "Thing") - end - - # Named has no index - s.public_send type_def_method, "Named" do |t| - t.field "name", "String" if type_def_method == :interface_type - link_supertype_to_subtypes(t, "Widget") - end - - # Thing has an index - s.public_send type_def_method, "Thing" do |t| - t.field "id", "ID!" if type_def_method == :interface_type - link_supertype_to_subtypes(t, "Widget") - t.index "things" - end - end - - expect(widget_metadata.index_definition_names).to eq ["things"] - end - end - end - end - - describe "transitive interface inheritance" do - it "allows a concrete type to inherit an index from a grandparent interface" do - gadget_metadata = object_type_metadata_for("Gadget") do |s| - # Gadget implements InterfaceA - s.object_type "Gadget" do |t| - t.field "id", "ID!" - t.field "name", "String" - t.field "category", "String" - t.implements "InterfaceA" - end - - # InterfaceA implements InterfaceB (with the index) - s.interface_type "InterfaceA" do |t| - t.field "name", "String" - t.field "category", "String" - t.implements "InterfaceB" - end - - # InterfaceB has the index - s.interface_type "InterfaceB" do |t| - t.field "name", "String" - t.index "indexed_interfaces" - end - end - - expect(gadget_metadata.index_definition_names).to eq ["indexed_interfaces"] - end - end - end - end -end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb index 17b222870..6bdd0ff79 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb @@ -1474,6 +1474,19 @@ def update_targets_for( widget_target = metadata.update_targets.find { |t| t.type == "Widget" } expect(widget_target.rollover_timestamp_value_source).to eq("created_at") end + + it "does not include __typename in data_params for types with their own index (single-type index)" do + metadata = object_type_metadata_for "Widget" do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.index "widgets" + end + end + + widget_target = metadata.update_targets.find { |t| t.type == "Widget" } + expect(widget_target.data_params.keys).not_to include("__typename") + end end context "on an embedded object type" do @@ -1636,6 +1649,35 @@ def update_targets_for( expect(widget_target.routing_value_source).to eq("id") # defaults to id when no custom routing expect(widget_target.rollover_timestamp_value_source).to be_nil # no default for rollover end + + it "includes __typename in data_params for types that inherit an index (needed for field extraction during indexing)" do + widget_metadata, component_metadata = object_type_metadata_for("Widget", "Component") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + link_subtype_to_supertype(t, "Thing") + end + + s.object_type "Component" do |t| + t.field "id", "ID!" + t.field "size", "Int" + link_subtype_to_supertype(t, "Thing") + end + + s.public_send type_def_method, "Thing" do |t| + link_supertype_to_subtypes(t, "Widget", "Component") + t.index "things" + end + end + + widget_target = widget_metadata.update_targets.find { |t| t.type == "Widget" } + expect(widget_target.data_params.keys).to include("__typename") + expect(widget_target.data_params["__typename"].source_path).to eq "__typename" + + component_target = component_metadata.update_targets.find { |t| t.type == "Component" } + expect(component_target.data_params.keys).to include("__typename") + expect(component_target.data_params["__typename"].source_path).to eq "__typename" + end end def standard_metadata_params(relationship:) From 6a4879a692dd4b24b28f3953ca69cb9be3b99d41 Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Sat, 14 Mar 2026 08:32:13 -0400 Subject: [PATCH 5/7] Test index inheritance with subtypes defined before supertype Modified existing apollo test to put the index on NamedEntity interface (previously concrete types had their own indices). IndexedType1 and IndexedType2 now inherit the index from NamedEntity. Intentionally define subtypes before their indexed supertype to verify type references resolve correctly regardless of definition order. This would fail if @key directives were applied during type initialization (when supertypes may not be defined yet). --- .../unit/elastic_graph/apollo/schema_definition_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb b/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb index 865f52da4..dbd4f1ee6 100644 --- a/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb +++ b/elasticgraph-apollo/spec/unit/elastic_graph/apollo/schema_definition_spec.rb @@ -291,24 +291,27 @@ def self.with_both_casing_forms(&block) it "avoids including indexed interfaces in the `_Entity` union (and does not add `@key` to it) since unions can't include interfaces" do schema_string = graphql_schema_string do |schema| + # Define subtypes before their indexed supertype to verify type references + # resolve correctly regardless of definition order. schema.object_type "IndexedType1" do |t| t.implements "NamedEntity" t.field "graphql", "String", name_in_index: "index" t.field "id", "ID!" t.field "name", "String" - t.index "index1" + # Inherits index from NamedEntity end schema.object_type "IndexedType2" do |t| t.implements "NamedEntity" t.field "id", "ID!" t.field "name", "String" - t.index "index1" + # Inherits index from NamedEntity end schema.interface_type "NamedEntity" do |t| t.field "id", "ID!" t.field "name", "String" + t.index "named_entities" end end From e65afe6ef447442b4f606e047d3c890b3c9c2358 Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Sat, 14 Mar 2026 08:56:49 -0400 Subject: [PATCH 6/7] Use clearer Animal/Dog/Poodle examples for transitive interface tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renamed InterfaceA/InterfaceB to Animal/Dog/Poodle for clearer conceptual hierarchy. Added test validating that multiple indexed interfaces in an inheritance chain (Poodle → Dog [indexed] → Animal [indexed]) correctly raises a validation error. --- .../index_definition_names_spec.rb | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb index 792c2d369..5bcfda7ac 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/index_definition_names_spec.rb @@ -205,27 +205,50 @@ module SchemaDefinition context "with transitive interface inheritance" do it "allows a concrete type to inherit an index from a grandparent interface" do - widget_metadata = object_type_metadata_for("Widget") do |s| - s.object_type "Widget" do |t| + poodle_metadata = object_type_metadata_for("Poodle") do |s| + s.object_type "Poodle" do |t| t.field "id", "ID!" t.field "name", "String" - t.field "category", "String" - t.implements "InterfaceA" + t.field "breed", "String" + t.implements "Dog" end - s.interface_type "InterfaceA" do |t| + s.interface_type "Dog" do |t| t.field "name", "String" - t.field "category", "String" - t.implements "InterfaceB" + t.field "breed", "String" + t.implements "Animal" end - s.interface_type "InterfaceB" do |t| + s.interface_type "Animal" do |t| t.field "name", "String" - t.index "indexed_interfaces" + t.index "animals" end end - expect(widget_metadata.index_definition_names).to eq ["indexed_interfaces"] + expect(poodle_metadata.index_definition_names).to eq ["animals"] + end + + it "raises an error when a concrete type's interface chain includes multiple indexed interfaces" do + expect { + object_type_metadata_for("Poodle") do |s| + s.object_type "Poodle" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.implements "Dog" + end + + s.interface_type "Dog" do |t| + t.field "name", "String" + t.implements "Animal" + t.index "dogs" + end + + s.interface_type "Animal" do |t| + t.field "name", "String" + t.index "animals" + end + end + }.to raise_error(ElasticGraph::Errors::SchemaError, a_string_including("The `Poodle` type is a subtype of multiple indexed abstract types", "dogs, animals")) end end end From 2503c6c819094dda546d707e5321173f6b86b88e Mon Sep 17 00:00:00 2001 From: Marc Daniels Date: Sat, 14 Mar 2026 09:34:52 -0400 Subject: [PATCH 7/7] Polish documentation and use consistent supertype/subtype terminology - Standardized on supertype/subtype instead of parent/child throughout - Renamed indexed_parents to indexed_supertypes in has_indices.rb - Simplified object_type.rb indexing_fields_by_name_in_index with early returns - Improved EntityTypeExtension documentation explaining _Entity overrides - Updated directly_queryable? doc to match existing pattern: "directly queryable via a type-specific field on the root Query type" - Simplified self_update_target to use root_document_type? (clearer semantics) - Removed duplicate negative test in update_targets_spec (covered elsewhere) --- .../entity_type_extension.rb | 16 +++++++------ .../schema_definition/mixins/has_indices.rb | 23 +++++++++---------- .../schema_definition/mixins/has_subtypes.rb | 2 +- .../schema_elements/object_type.rb | 13 ++++------- .../update_targets_spec.rb | 13 ----------- 5 files changed, 26 insertions(+), 41 deletions(-) diff --git a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb index 7f22a74f3..1e4d2aa12 100644 --- a/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb +++ b/elasticgraph-apollo/lib/elastic_graph/apollo/schema_definition/entity_type_extension.rb @@ -9,24 +9,26 @@ module ElasticGraph module Apollo module SchemaDefinition - # The Apollo `_Entity` type is a type union of _all_ entity subtypes in an ElasticGraph schema. - # However, unlike a normal union type: - # - # - `_Entity` is never a root document type, and should not be treated as one (even though its subtypes are all root document types, - # which would usually cause it to be treated as a root document type!). - # - A merged set of `graphql_fields_by_name` cannot be safely computed. That method raises errors if a field with the same name - # has conflicting definitions on different subtypes, but we must allow that on `_Entity` subtypes. + # The Apollo `_Entity` type is a union of all entity types in an ElasticGraph schema. These overrides + # prevent ElasticGraph from treating `_Entity` like a normal indexed union type, which would trigger + # unwanted derived schema generation and validation. # # @private module EntityTypeExtension + # A merged set of `graphql_fields_by_name` cannot be safely computed. That method raises errors if a field with + # the same name has conflicting definitions on different subtypes, but we must allow that on `_Entity` subtypes. def graphql_fields_by_name {} end + # `_Entity` is never a root document type, and should not be treated as one (even though its subtypes are all + # root document types, which would usually cause it to be treated as a root document type!). def root_document_type? false end + # `_Entity` is never directly queryable from the root `Query` type. It's queried via the apollo + # `_entities(representations: ...)` field instead. def directly_queryable? false end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb index e249a2e2a..e47d59257 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_indices.rb @@ -102,36 +102,36 @@ def has_own_index_def? # Resolves this type's index definition. This will be one of: # - This type's own_index_def (if it directly defines an index) - # - An inherited index from a parent abstract type (union/interface) that defines an index + # - An inherited index from an abstract supertype (union/interface) that has an index # - # A type can be a subtype of multiple abstract types (e.g., implement multiple interfaces), - # but at most one of those parent types may define an index. If multiple parent types are - # indexed, this method raises an error to prevent ambiguity about which index to inherit. + # This type can be a subtype of multiple abstract types (e.g., implements multiple interfaces), but unless it + # defines its own index, at most one of its supertypes may have an index. If multiple parent types are indexed, + # this method raises an error to prevent ambiguity about which index to inherit. # # @return [Indexing::Index, nil] the index definition, or nil if this type has no index # @raise [Errors::SchemaError] if this type is a subtype of multiple indexed abstract types def index_def return own_index_def if has_own_index_def? - indexed_parents = recursively_resolve_supertypes.select(&:has_own_index_def?) + indexed_supertypes = recursively_resolve_supertypes.select(&:has_own_index_def?) - if indexed_parents.size > 1 - parent_names = indexed_parents.map { |p| p.own_index_def.name }.join(", ") + if indexed_supertypes.size > 1 + parent_names = indexed_supertypes.map { |p| p.own_index_def.name }.join(", ") raise Errors::SchemaError, "The `#{name}` type is a subtype of multiple indexed abstract types (#{parent_names}). " \ "If a concrete type does not define an index, it may not be a member of multiple indexed abstract types." end - indexed_parents.first&.own_index_def + indexed_supertypes.first&.own_index_def end # @return [Boolean] true if this type is a root document type that lives at a document root in the datastore (is indexed). - # This returns true for types with their own index definition or types that inherit an index from a parent abstract type. + # This returns true for types with their own index definition or types that inherit an index from a supertype. def root_document_type? !index_def.nil? end - # @return [Boolean] true if this type is directly queryable from the root `Query` type. + # @return [Boolean] true if this type is directly queryable via a type-specific field on the root `Query` type. def directly_queryable? has_own_index_def? end @@ -303,8 +303,7 @@ def initialize_has_indices end def self_update_target - # Only concrete types that are indexed in the datastore need an update target. - return nil if abstract? || index_def.nil? + return nil if abstract? || !root_document_type? # We exclude `id` from `data_params` because `Indexer::Operator::Update` automatically includes # `params.id` so we don't want it duplicated at `params.data.id` alongside other data params. diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb index 099579257..b5db57997 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_subtypes.rb @@ -38,7 +38,7 @@ def root_document_type? super || subtypes_are_root_document_types? end - # A parent type is queryable if all of its subtypes are root document types (via a direct or inherited index) + # An abstract type is queryable if all of its subtypes are root document types (via a direct or inherited index) # even if those subtypes aren't themselves directly queryable. This is why this doesn't delegate to a # subtypes_are_directly_queryable helper. def directly_queryable? diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb index fb3a40d3d..348260053 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/object_type.rb @@ -38,17 +38,14 @@ class ObjectType < DelegateClass(TypeWithSubfields) include Mixins::ImplementsInterfaces include Mixins::HasReadableToSAndInspect.new { |t| t.name } - # @return [Hash] fields that will be indexed, including __typename for mixed-type indices + # @return [Hash] fields that will be indexed, including __typename for mixed-type indices (types + # that inherit an index from an abstract supertype) # @private def indexing_fields_by_name_in_index - fields = super + return super if has_own_index_def? + return super unless root_document_type? - # Add __typename for types in mixed-type indices (types that inherit an index from a parent union/interface) - if !has_own_index_def? && !index_def.nil? - fields.merge("__typename" => schema_def_state.factory.new_field(name: "__typename", type: "String", parent_type: self)) - else - fields - end + super.merge("__typename" => schema_def_state.factory.new_field(name: "__typename", type: "String", parent_type: self)) end # @private diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb index 6bdd0ff79..f79d67ea2 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/update_targets_spec.rb @@ -1474,19 +1474,6 @@ def update_targets_for( widget_target = metadata.update_targets.find { |t| t.type == "Widget" } expect(widget_target.rollover_timestamp_value_source).to eq("created_at") end - - it "does not include __typename in data_params for types with their own index (single-type index)" do - metadata = object_type_metadata_for "Widget" do |s| - s.object_type "Widget" do |t| - t.field "id", "ID!" - t.field "name", "String" - t.index "widgets" - end - end - - widget_target = metadata.update_targets.find { |t| t.type == "Widget" } - expect(widget_target.data_params.keys).not_to include("__typename") - end end context "on an embedded object type" do