From 51f01702f238b3dc267d9b46022993ff9c9d4c23 Mon Sep 17 00:00:00 2001 From: ellisandrews-toast Date: Mon, 8 Jun 2026 14:22:04 -0400 Subject: [PATCH 1/3] Populate sourced_from_nested_paths_by_relationship on the root index --- .../schema_definition/indexing/index.rb | 31 ++- .../sourced_from_update_targets_resolver.rb | 43 ++++- .../schema_definition/indexing/index.rbs | 2 + .../sourced_from_update_targets_resolver.rbs | 1 + .../index_definitions_by_name_spec.rb | 178 ++++++++++++++++++ .../update_targets_spec.rb | 16 +- 6 files changed, 256 insertions(+), 15 deletions(-) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb index 436bc6e82..cc8b036cd 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb @@ -38,7 +38,11 @@ module Indexing # @return [RolloverConfig, nil] rollover configuration for the index # @!attribute [r] has_had_multiple_sources_flag # @return [Boolean] whether this index has ever had multiple sources - class Index < Struct.new(:name, :default_sort_pairs, :settings, :schema_def_state, :indexed_type, :routing_field_path, :rollover_config, :has_had_multiple_sources_flag) + # @!attribute [r] sourced_from_nested_paths_by_relationship + # @return [Hash>] + # map from a (leaf) relationship name to the path segments the painless script uses to navigate from this + # root index's documents down to the nested elements that receive `sourced_from` data + class Index < Struct.new(:name, :default_sort_pairs, :settings, :schema_def_state, :indexed_type, :routing_field_path, :rollover_config, :has_had_multiple_sources_flag, :sourced_from_nested_paths_by_relationship) include Mixins::HasReadableToSAndInspect.new { |i| i.name } # @param name [String] name of the index @@ -55,7 +59,7 @@ def initialize(name, settings, schema_def_state, indexed_type) settings = DEFAULT_SETTINGS.merge(Support::HashUtil.flatten_and_stringify_keys(settings, prefix: "index")) - super(name, [], settings, schema_def_state, indexed_type, nil, nil, false) + super(name, [], settings, schema_def_state, indexed_type, nil, nil, false, {}) schema_def_state.after_user_definition_complete do # `id` is the field Elasticsearch/OpenSearch use for routing by default: @@ -265,10 +269,31 @@ def runtime_metadata ) end, has_had_multiple_sources: has_had_multiple_sources_flag, - sourced_from_nested_paths_by_relationship: {} # TODO: Populate with real data once paths are registered on the index + sourced_from_nested_paths_by_relationship: sourced_from_nested_paths_by_relationship ) end + # Registers the runtime-metadata path segments for a nested `sourced_from` relationship, keyed by the + # (leaf) relationship name. The painless script uses these to navigate from this root index's documents + # down to the nested elements that receive the sourced data. + # + # @param relationship_name [String] name of the relationship whose `sourced_from` fields drive these paths + # @param path_segments [Array] + # ordered root-to-leaf path segments + # @return [void] + # @api private + def register_sourced_from_nested_paths(relationship_name, path_segments) + if sourced_from_nested_paths_by_relationship.key?(relationship_name) + raise Errors::SchemaError, "Index `#{name}` (for type `#{indexed_type.name}`) already has nested " \ + "`sourced_from` paths registered for relationship `#{relationship_name}`. Each document tracks " \ + "sourced data by relationship name (e.g. in its `__versions` and `__sources` fields), so two " \ + "`parent_relationship` chains terminating at the same index cannot share a relationship name. " \ + "Rename one of the colliding relationships." + end + + sourced_from_nested_paths_by_relationship[relationship_name] = path_segments + end + private # A regex that requires at least one non-whitespace character. diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb index ae7eaa318..57ac76f30 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb @@ -7,6 +7,7 @@ # frozen_string_literal: true require "elastic_graph/errors" +require "elastic_graph/schema_artifacts/runtime_metadata/sourced_from_nested_path_segment" require "elastic_graph/schema_definition/indexing/relationship_chain_resolver" require "elastic_graph/schema_definition/indexing/relationship_resolver" require "elastic_graph/schema_definition/indexing/update_target_resolver" @@ -67,8 +68,8 @@ def resolve_for_type(object_type, &error_reporter) end end - # Resolve any `parent_relationship` chains on this type. For now this only surfaces - # configuration errors; later PRs will use the resolved chains to build nested update targets. + # Resolve any `parent_relationship` chains on this type, surfacing configuration errors and + # registering the resolved path segments on the root index. resolve_relationship_chains(object_type, &error_reporter) results @@ -102,12 +103,46 @@ def resolve_relationship_chains(object_type) relationships_with_parent_ref = object_type.relationships_by_name.each_value.select(&:parent_ref) return if relationships_with_parent_ref.empty? + # The set of relationship names this type's `sourced_from` fields draw their data through. We use + # `fields_with_sources` (rather than `sourced_fields_by_relationship_name`) because it works for + # non-indexed (embedded) types — which is exactly where nested `sourced_from` fields live. + relationship_names_with_sourced_fields = object_type.fields_with_sources.map { |f| (_ = f.source).relationship_name }.to_set + chain_resolver = RelationshipChainResolver.new(schema_def_state: @schema_def_state) relationships_with_parent_ref.each do |relationship| - # TODO: use resolved_chain to build nested update targets once that logic is implemented. - _resolved_chain, chain_errors = chain_resolver.resolve(relationship) + # Every `parent_relationship` chain is resolved so its configuration errors are surfaced, even for + # relationships that don't themselves back any `sourced_from` field. + resolved_chain, chain_errors = chain_resolver.resolve(relationship) chain_errors.each { |error| yield :sourced_field, error } + next unless resolved_chain + + # Only register paths for relationships that back `sourced_from` fields. A pure link in a longer + # chain has no nested data of its own; its navigation lives in the leaf relationship's chain. + next unless relationship_names_with_sourced_fields.include?(relationship.name) + + # Register the path segments on the root index, keyed by the leaf relationship name. This is a + # resolved chain's only effect today; the rest of the nested `sourced_from` machinery comes later. + root_index = resolved_chain.root_relationship.parent_type.index_def # : Index + root_index.register_sourced_from_nested_paths(relationship.name, build_sourced_from_nested_paths(resolved_chain)) + end + end + + # Converts a resolved chain's path segments into the runtime-metadata segment types the index dumps. + # A segment with a `source_field_name` navigates into a list (and needs it to match an element); + # one without navigates into an object. + def build_sourced_from_nested_paths(resolved_chain) + resolved_chain.path_segments.map do |segment| + if (source_field = segment.source_field_name) + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new( + field: segment.field.name_in_index, + source_field: source_field + ) + else + SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new( + field: segment.field.name_in_index + ) + end end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs index 0dc296381..ba1584d57 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs @@ -8,11 +8,13 @@ module ElasticGraph attr_reader rollover_config: RolloverConfig? attr_reader has_had_multiple_sources_flag: bool attr_reader indexed_type: indexableType + attr_reader sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment]] def uses_custom_routing?: () -> bool def to_index_config: () -> ::Hash[::String, untyped] def to_index_template_config: () -> ::Hash[::String, untyped] def runtime_metadata: () -> SchemaArtifacts::RuntimeMetadata::IndexDefinition + def register_sourced_from_nested_paths: (::String, ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment]) -> void private diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs index c9c2ca72b..1c54b7e2f 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs @@ -12,6 +12,7 @@ module ElasticGraph def resolve_for_type: (indexableType) { (::Symbol, ::String) -> void } -> ::Array[[::String, SchemaArtifacts::RuntimeMetadata::UpdateTarget]] def resolve_update_target: (indexableType, ResolvedRelationship, ::Array[SchemaElements::Field]) { (::Symbol, ::String) -> void } -> [::String, SchemaArtifacts::RuntimeMetadata::UpdateTarget]? def resolve_relationship_chains: (indexableType) { (::Symbol, ::String) -> void } -> void + def build_sourced_from_nested_paths: (ResolvedRelationshipChain) -> ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment] def sourced_fields_by_relationship_name: (indexableType) -> ::Hash[::String, ::Array[SchemaElements::Field]] def raise_if_errors: (::Array[::String], ::Array[::String]) -> void end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb index 1835088ba..63dfa4e30 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb @@ -912,6 +912,184 @@ def build_count_paths_from_mapping(mapping) end end + describe "#sourced_from_nested_paths_by_relationship" do + it "registers a `ListPathSegment` for a list embedding field, keyed by the leaf relationship name" do + paths = sourced_from_nested_paths + + expect(paths).to eq( + "statLine" => [ + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players", source_field: "playerId") + ] + ) + end + + it "registers an `ObjectPathSegment` for a non-list embedding field" do + paths = sourced_from_nested_paths(players_field: "Player!") + + expect(paths).to eq( + "statLine" => [ + SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new(field: "players") + ] + ) + end + + it "registers segments for every level of a chain spanning more than two levels" do + paths = sourced_from_nested_paths_for_index("leagues") do |s| + s.object_type "League" do |t| + t.field "id", "ID!" + t.field "teams", "[Team!]!" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "leagueId", dir: :in, indexing_only: true + t.index "leagues" + end + + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "players", "[Player!]!" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| + r.parent_relationship "League", "statLines" + end + end + + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "leagueId", "ID" + t.field "teamId", "ID" + t.field "playerId", "ID" + t.field "goals", "Int" + t.index "stat_lines" + end + end + + # `Team.statLines` is only a link (no `sourced_from` field of its own), so only `Player.statLine` registers a path. + expect(paths).to eq( + "statLine" => [ + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "teams", source_field: "teamId"), + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players", source_field: "playerId") + ] + ) + end + + it "is empty for an index with no `parent_relationship` chains" do + paths = sourced_from_nested_paths_for_index("widgets") do |s| + s.object_type "Widget" do |t| + t.field "id", "ID!" + t.field "name", "String" + t.index "widgets" + end + end + + expect(paths).to eq({}) + end + + it "raises a clear error when two chains terminating at the same index use the same relationship name" do + expect { + # `Player` and `Coach` are both embedded in `Team` and each declares a `statLine` relationship that + # chains up to the `teams` index. The two leaves share the relationship name `statLine`, so they would + # both register under it — a collision that would otherwise silently clobber one set of paths. + sourced_from_nested_paths_for_index("teams") do |s| + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "players", "[Player!]!" do |f| + f.mapping type: "object" + end + t.field "coaches", "[Coach!]!" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true + t.index("teams") { |i| i.has_had_multiple_sources! } + end + + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "Coach" do |t| + t.field "id", "ID!" + t.field "wins", "Int" do |f| + f.sourced_from "statLine", "wins" + end + t.relates_to_one "statLine", "StatLine", via: "coachId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "teamId", "ID" + t.field "playerId", "ID" + t.field "coachId", "ID" + t.field "goals", "Int" + t.field "wins", "Int" + t.index "stat_lines" + end + end + }.to raise_error Errors::SchemaError, a_string_including( + "Index `teams` (for type `Team`) already has nested `sourced_from` paths registered for relationship `statLine`" + ) + end + + # Defines a schema and returns the `sourced_from_nested_paths_by_relationship` registered on the named index. + def sourced_from_nested_paths_for_index(index_name, &schema) + define_schema(&schema) + .runtime_metadata.index_definitions_by_name + .fetch(index_name).sourced_from_nested_paths_by_relationship + end + + # Builds the standard `Team`/`Player`/`StatLine` nested `sourced_from` chain (`Player.statLine` declares a + # `parent_relationship` to `Team.statLines`, with `Player` embedded in `Team.players`) and returns the + # `sourced_from_nested_paths_by_relationship` registered on the `teams` index. + def sourced_from_nested_paths(players_field: "[Player!]!") + sourced_from_nested_paths_for_index("teams") do |s| + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "players", players_field do |f| + f.mapping type: "object" if players_field.start_with?("[") + end + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true + t.index("teams") { |i| i.has_had_multiple_sources! } + end + + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "teamId", "ID" + t.field "playerId", "ID" + t.field "goals", "Int" + t.index "stat_lines" + end + end + end + end + def index_definition_metadata_for(name, type_definition_order: ["NestedFields", "MyType"], on_my_type: nil, **options, &block) type_defs = { "NestedFields" => ->(t) do 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 7cf9c3a43..e605049cc 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 @@ -1401,7 +1401,7 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "teams", "[Team!]!" do |f| f.mapping type: "object" end - t.relates_to_many "statLines", "StatLine", via: "leagueId", dir: :in, indexing_only: true + t.relates_to_many "leagueStatLines", "StatLine", via: "leagueId", dir: :in, indexing_only: true t.index "leagues" end @@ -1410,8 +1410,8 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "players", "[Player!]!" do |f| f.mapping type: "object" end - t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| - r.parent_relationship "League", "statLines" + t.relates_to_many "teamStatLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| + r.parent_relationship "League", "leagueStatLines" end end @@ -1420,18 +1420,18 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "gameAppearances", "[GameAppearance!]!" do |f| f.mapping type: "object" end - t.relates_to_many "statLines", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Team", "statLines" + t.relates_to_many "playerStatLines", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "teamStatLines" end end s.object_type "GameAppearance" do |t| t.field "id", "ID!" t.field "goals", "Int" do |f| - f.sourced_from "statLine", "goals" + f.sourced_from "gameAppearanceStatLine", "goals" end - t.relates_to_one "statLine", "StatLine", via: "gameAppearanceId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Player", "statLines" + t.relates_to_one "gameAppearanceStatLine", "StatLine", via: "gameAppearanceId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Player", "playerStatLines" end end From 6fe416dca980220e46b4f41cc83f8d96bb303820 Mon Sep 17 00:00:00 2001 From: ellisandrews-toast Date: Tue, 9 Jun 2026 15:19:51 -0400 Subject: [PATCH 2/3] Key nested sourced paths by qualified relationship --- .../index_definition/index_spec.rb | 2 +- .../rollover_index_template_spec.rb | 2 +- .../runtime_metadata/index_definition.rb | 12 +- .../runtime_metadata/index_definition.rbs | 10 +- .../runtime_metadata/index_definition_spec.rb | 2 +- .../runtime_metadata/schema_spec.rb | 8 +- .../schema_definition/indexing/index.rb | 37 ++--- .../indexing/relationship_chain_resolver.rb | 13 +- .../sourced_from_update_targets_resolver.rb | 5 +- .../schema_definition/indexing/index.rbs | 2 +- .../indexing/relationship_chain_resolver.rbs | 8 +- .../index_definitions_by_name_spec.rb | 156 ++++++++++-------- .../update_targets_spec.rb | 16 +- .../spec_support/runtime_metadata_support.rb | 4 +- 14 files changed, 152 insertions(+), 125 deletions(-) diff --git a/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/index_spec.rb b/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/index_spec.rb index fc3b126c7..c80f07c9a 100644 --- a/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/index_spec.rb +++ b/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/index_spec.rb @@ -89,7 +89,7 @@ def index_def_named(name, rollover: nil) current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: false, - sourced_from_nested_paths_by_relationship: {} + sourced_from_nested_paths_by_qualified_relationship: {} ) DatastoreCore::IndexDefinition.with( diff --git a/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/rollover_index_template_spec.rb b/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/rollover_index_template_spec.rb index 7026e8a2c..806440f80 100644 --- a/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/rollover_index_template_spec.rb +++ b/elasticgraph-datastore_core/spec/integration/elastic_graph/datastore_core/index_definition/rollover_index_template_spec.rb @@ -558,7 +558,7 @@ def index_def_named(name, rollover: nil) current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: false, - sourced_from_nested_paths_by_relationship: {} + sourced_from_nested_paths_by_qualified_relationship: {} ) DatastoreCore::IndexDefinition.with( diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rb index 960ce9bb1..df542e18d 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rb @@ -17,16 +17,16 @@ module RuntimeMetadata # Runtime metadata related to a datastore index definition. # # @private - class IndexDefinition < ::Data.define(:route_with, :rollover, :default_sort_fields, :current_sources, :fields_by_path, :has_had_multiple_sources, :sourced_from_nested_paths_by_relationship) + class IndexDefinition < ::Data.define(:route_with, :rollover, :default_sort_fields, :current_sources, :fields_by_path, :has_had_multiple_sources, :sourced_from_nested_paths_by_qualified_relationship) ROUTE_WITH = "route_with" ROLLOVER = "rollover" DEFAULT_SORT_FIELDS = "default_sort_fields" CURRENT_SOURCES = "current_sources" FIELDS_BY_PATH = "fields_by_path" HAS_HAD_MULTIPLE_SOURCES = "has_had_multiple_sources" - SOURCED_FROM_NESTED_PATHS_BY_RELATIONSHIP = "sourced_from_nested_paths_by_relationship" + SOURCED_FROM_NESTED_PATHS_BY_QUALIFIED_RELATIONSHIP = "sourced_from_nested_paths_by_qualified_relationship" - def initialize(route_with:, rollover:, default_sort_fields:, current_sources:, fields_by_path:, has_had_multiple_sources:, sourced_from_nested_paths_by_relationship:) + def initialize(route_with:, rollover:, default_sort_fields:, current_sources:, fields_by_path:, has_had_multiple_sources:, sourced_from_nested_paths_by_qualified_relationship:) super( route_with: route_with, rollover: rollover, @@ -34,7 +34,7 @@ def initialize(route_with:, rollover:, default_sort_fields:, current_sources:, f current_sources: current_sources.to_set, fields_by_path: fields_by_path, has_had_multiple_sources: has_had_multiple_sources, - sourced_from_nested_paths_by_relationship: sourced_from_nested_paths_by_relationship + sourced_from_nested_paths_by_qualified_relationship: sourced_from_nested_paths_by_qualified_relationship ) end @@ -46,7 +46,7 @@ def self.from_hash(hash) current_sources: hash[CURRENT_SOURCES] || [], fields_by_path: (hash[FIELDS_BY_PATH] || {}).transform_values { |h| IndexField.from_hash(h) }, has_had_multiple_sources: hash[HAS_HAD_MULTIPLE_SOURCES] || false, - sourced_from_nested_paths_by_relationship: (hash[SOURCED_FROM_NESTED_PATHS_BY_RELATIONSHIP] || {}).transform_values { |segments| segments.map { |h| SourcedFromNestedPathSegment.from_hash(h) } } + sourced_from_nested_paths_by_qualified_relationship: (hash[SOURCED_FROM_NESTED_PATHS_BY_QUALIFIED_RELATIONSHIP] || {}).transform_values { |segments| segments.map { |h| SourcedFromNestedPathSegment.from_hash(h) } } ) end @@ -59,7 +59,7 @@ def to_dumpable_hash HAS_HAD_MULTIPLE_SOURCES => (true if has_had_multiple_sources), ROLLOVER => rollover&.to_dumpable_hash, ROUTE_WITH => route_with, - SOURCED_FROM_NESTED_PATHS_BY_RELATIONSHIP => sourced_from_nested_paths_by_relationship.transform_values { |segments| segments.map(&:to_dumpable_hash) } + SOURCED_FROM_NESTED_PATHS_BY_QUALIFIED_RELATIONSHIP => sourced_from_nested_paths_by_qualified_relationship.transform_values { |segments| segments.map(&:to_dumpable_hash) } } end diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rbs index 6677e24f4..645309b9d 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/index_definition.rbs @@ -8,7 +8,7 @@ module ElasticGraph attr_reader current_sources: ::Set[::String] attr_reader fields_by_path: ::Hash[::String, IndexField] attr_reader has_had_multiple_sources: bool - attr_reader sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] + attr_reader sourced_from_nested_paths_by_qualified_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] def initialize: ( route_with: ::String, @@ -17,7 +17,7 @@ module ElasticGraph current_sources: ::Set[::String], fields_by_path: ::Hash[::String, IndexField], has_had_multiple_sources: bool, - sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] + sourced_from_nested_paths_by_qualified_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] ) -> void def with: ( @@ -27,7 +27,7 @@ module ElasticGraph ?current_sources: ::Enumerable[::String], ?fields_by_path: ::Hash[::String, IndexField], ?has_had_multiple_sources: bool, - ?sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] + ?sourced_from_nested_paths_by_qualified_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] ) -> IndexDefinition end @@ -38,7 +38,7 @@ module ElasticGraph CURRENT_SOURCES: "current_sources" FIELDS_BY_PATH: "fields_by_path" HAS_HAD_MULTIPLE_SOURCES: "has_had_multiple_sources" - SOURCED_FROM_NESTED_PATHS_BY_RELATIONSHIP: "sourced_from_nested_paths_by_relationship" + SOURCED_FROM_NESTED_PATHS_BY_QUALIFIED_RELATIONSHIP: "sourced_from_nested_paths_by_qualified_relationship" def initialize: ( route_with: ::String, @@ -47,7 +47,7 @@ module ElasticGraph current_sources: ::Enumerable[::String], fields_by_path: ::Hash[::String, IndexField], has_had_multiple_sources: bool, - sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] + sourced_from_nested_paths_by_qualified_relationship: ::Hash[::String, ::Array[sourcedFromNestedPathSegment]] ) -> void def self.from_hash: (::Hash[::String, untyped]) -> IndexDefinition diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/index_definition_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/index_definition_spec.rb index 09ae358e7..0939b9fb3 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/index_definition_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/index_definition_spec.rb @@ -25,7 +25,7 @@ module RuntimeMetadata current_sources: Set.new, fields_by_path: {}, has_had_multiple_sources: false, - sourced_from_nested_paths_by_relationship: {} + sourced_from_nested_paths_by_qualified_relationship: {} ) end 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 c754ba9e6..c22a2aab7 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 @@ -128,7 +128,7 @@ module RuntimeMetadata "foo.bar" => IndexField.new(source: "other") }, has_had_multiple_sources: false, - sourced_from_nested_paths_by_relationship: { + sourced_from_nested_paths_by_qualified_relationship: { "currency" => [ ListPathSegment.new(field: "costs", source_field: "cost_id"), ObjectPathSegment.new(field: "details") @@ -142,7 +142,7 @@ module RuntimeMetadata current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: false, - sourced_from_nested_paths_by_relationship: {} + sourced_from_nested_paths_by_qualified_relationship: {} ), "components" => IndexDefinition.new( route_with: "group_id", @@ -151,7 +151,7 @@ module RuntimeMetadata current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: true, - sourced_from_nested_paths_by_relationship: {} + sourced_from_nested_paths_by_qualified_relationship: {} ) }, schema_element_names: SchemaElementNames.new( @@ -279,7 +279,7 @@ module RuntimeMetadata "source" => "other" } }, - "sourced_from_nested_paths_by_relationship" => { + "sourced_from_nested_paths_by_qualified_relationship" => { "currency" => [ {"field" => "costs", "source_field" => "cost_id"}, {"field" => "details"} diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb index cc8b036cd..3c91958c4 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb @@ -38,11 +38,21 @@ module Indexing # @return [RolloverConfig, nil] rollover configuration for the index # @!attribute [r] has_had_multiple_sources_flag # @return [Boolean] whether this index has ever had multiple sources - # @!attribute [r] sourced_from_nested_paths_by_relationship + # @!attribute [r] sourced_from_nested_paths_by_qualified_relationship # @return [Hash>] # map from a (leaf) relationship name to the path segments the painless script uses to navigate from this # root index's documents down to the nested elements that receive `sourced_from` data - class Index < Struct.new(:name, :default_sort_pairs, :settings, :schema_def_state, :indexed_type, :routing_field_path, :rollover_config, :has_had_multiple_sources_flag, :sourced_from_nested_paths_by_relationship) + class Index < Struct.new( + :name, + :default_sort_pairs, + :settings, + :schema_def_state, + :indexed_type, + :routing_field_path, + :rollover_config, + :has_had_multiple_sources_flag, + :sourced_from_nested_paths_by_qualified_relationship + ) include Mixins::HasReadableToSAndInspect.new { |i| i.name } # @param name [String] name of the index @@ -269,29 +279,14 @@ def runtime_metadata ) end, has_had_multiple_sources: has_had_multiple_sources_flag, - sourced_from_nested_paths_by_relationship: sourced_from_nested_paths_by_relationship + sourced_from_nested_paths_by_qualified_relationship: sourced_from_nested_paths_by_qualified_relationship ) end - # Registers the runtime-metadata path segments for a nested `sourced_from` relationship, keyed by the - # (leaf) relationship name. The painless script uses these to navigate from this root index's documents - # down to the nested elements that receive the sourced data. - # - # @param relationship_name [String] name of the relationship whose `sourced_from` fields drive these paths - # @param path_segments [Array] - # ordered root-to-leaf path segments - # @return [void] + # Registers a nested `sourced_from` chain's path segments under its qualified relationship. # @api private - def register_sourced_from_nested_paths(relationship_name, path_segments) - if sourced_from_nested_paths_by_relationship.key?(relationship_name) - raise Errors::SchemaError, "Index `#{name}` (for type `#{indexed_type.name}`) already has nested " \ - "`sourced_from` paths registered for relationship `#{relationship_name}`. Each document tracks " \ - "sourced data by relationship name (e.g. in its `__versions` and `__sources` fields), so two " \ - "`parent_relationship` chains terminating at the same index cannot share a relationship name. " \ - "Rename one of the colliding relationships." - end - - sourced_from_nested_paths_by_relationship[relationship_name] = path_segments + def register_sourced_from_nested_paths(qualified_relationship, path_segments) + sourced_from_nested_paths_by_qualified_relationship[qualified_relationship] = path_segments end private diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb index 5a8a66e8e..6f1e20849 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb @@ -14,10 +14,16 @@ module Indexing # The result of resolving a relationship chain. # # @private - ResolvedRelationshipChain = ::Data.define( - :root_relationship, # Relationship - the root relationship (no parent_relationship) - :path_segments # Array - ordered root-to-leaf + class ResolvedRelationshipChain < ::Data.define( + :root_relationship, # Relationship the chain terminated at on the root indexed type + :leaf_relationship, # Relationship the chain was resolved from — backs `sourced_from` field(s) + :path_segments # Array - the embedding fields to descend, ordered root-to-leaf ) + # The leaf relationship name qualified by its embedding-field path (hence unique per resolved chain) + def qualified_relationship + (path_segments.map { |segment| segment.field.name_in_index } + [leaf_relationship.name]).join(".") + end + end # Describes how to navigate from a parent type into a nested child element. # For list fields, `source_field_name` identifies which element to update: the element @@ -75,6 +81,7 @@ def resolve(starting_relationship) resolved_chain = ResolvedRelationshipChain.new( root_relationship: root_relationship, + leaf_relationship: starting_relationship, path_segments: path_segments.reverse # reverse so root-to-leaf order ) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb index 57ac76f30..1dfe56915 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb @@ -121,10 +121,9 @@ def resolve_relationship_chains(object_type) # chain has no nested data of its own; its navigation lives in the leaf relationship's chain. next unless relationship_names_with_sourced_fields.include?(relationship.name) - # Register the path segments on the root index, keyed by the leaf relationship name. This is a - # resolved chain's only effect today; the rest of the nested `sourced_from` machinery comes later. + # Register the path segments on the root index, keyed by the chain's qualified relationship. root_index = resolved_chain.root_relationship.parent_type.index_def # : Index - root_index.register_sourced_from_nested_paths(relationship.name, build_sourced_from_nested_paths(resolved_chain)) + root_index.register_sourced_from_nested_paths(resolved_chain.qualified_relationship, build_sourced_from_nested_paths(resolved_chain)) end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs index ba1584d57..1f3eef1da 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs @@ -8,7 +8,7 @@ module ElasticGraph attr_reader rollover_config: RolloverConfig? attr_reader has_had_multiple_sources_flag: bool attr_reader indexed_type: indexableType - attr_reader sourced_from_nested_paths_by_relationship: ::Hash[::String, ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment]] + attr_reader sourced_from_nested_paths_by_qualified_relationship: ::Hash[::String, ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment]] def uses_custom_routing?: () -> bool def to_index_config: () -> ::Hash[::String, untyped] diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs index 6c161e20a..7856465d3 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs @@ -1,16 +1,22 @@ module ElasticGraph module SchemaDefinition module Indexing - class ResolvedRelationshipChain + class ResolvedRelationshipChainSuperType attr_reader root_relationship: SchemaElements::Relationship + attr_reader leaf_relationship: SchemaElements::Relationship attr_reader path_segments: ::Array[PathSegment] def initialize: ( root_relationship: SchemaElements::Relationship, + leaf_relationship: SchemaElements::Relationship, path_segments: ::Array[PathSegment] ) -> void end + class ResolvedRelationshipChain < ResolvedRelationshipChainSuperType + def qualified_relationship: () -> ::String + end + class PathSegment attr_reader field: SchemaElements::Field attr_reader source_field_name: ::String? diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb index 63dfa4e30..41bea9393 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb @@ -912,12 +912,12 @@ def build_count_paths_from_mapping(mapping) end end - describe "#sourced_from_nested_paths_by_relationship" do - it "registers a `ListPathSegment` for a list embedding field, keyed by the leaf relationship name" do + describe "#sourced_from_nested_paths_by_qualified_relationship" do + it "registers a `ListPathSegment` for a list embedding field, keyed by the qualified relationship" do paths = sourced_from_nested_paths expect(paths).to eq( - "statLine" => [ + "players.statLine" => [ SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players", source_field: "playerId") ] ) @@ -927,13 +927,15 @@ def build_count_paths_from_mapping(mapping) paths = sourced_from_nested_paths(players_field: "Player!") expect(paths).to eq( - "statLine" => [ + "players.statLine" => [ SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new(field: "players") ] ) end - it "registers segments for every level of a chain spanning more than two levels" do + it "registers interleaved list and object segments for a chain spanning several levels" do + # League -> Team -> Roster -> Player, where `Team` embeds `roster` as an object (not a list). + # The resulting path interleaves `ListPathSegment` and `ObjectPathSegment` across its levels. paths = sourced_from_nested_paths_for_index("leagues") do |s| s.object_type "League" do |t| t.field "id", "ID!" @@ -946,7 +948,7 @@ def build_count_paths_from_mapping(mapping) s.object_type "Team" do |t| t.field "id", "ID!" - t.field "players", "[Player!]!" do |f| + t.field "roster", "Roster!" do |f| f.mapping type: "object" end t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| @@ -954,13 +956,23 @@ def build_count_paths_from_mapping(mapping) end end + s.object_type "Roster" do |t| + t.field "id", "ID!" + t.field "players", "[Player!]!" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "rosterId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + s.object_type "Player" do |t| t.field "id", "ID!" t.field "goals", "Int" do |f| f.sourced_from "statLine", "goals" end t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Team", "statLines" + r.parent_relationship "Roster", "statLines" end end @@ -968,16 +980,19 @@ def build_count_paths_from_mapping(mapping) t.field "id", "ID!" t.field "leagueId", "ID" t.field "teamId", "ID" + t.field "rosterId", "ID" t.field "playerId", "ID" t.field "goals", "Int" t.index "stat_lines" end end - # `Team.statLines` is only a link (no `sourced_from` field of its own), so only `Player.statLine` registers a path. + # Only `Player.statLine` backs a `sourced_from` field; the intermediate relationships are just links. + # Its path spans all three embedding fields: a list (`teams`), an object (`roster`), then a list (`players`). expect(paths).to eq( - "statLine" => [ + "teams.roster.players.statLine" => [ SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "teams", source_field: "teamId"), + SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new(field: "roster"), SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players", source_field: "playerId") ] ) @@ -995,75 +1010,73 @@ def build_count_paths_from_mapping(mapping) expect(paths).to eq({}) end - it "raises a clear error when two chains terminating at the same index use the same relationship name" do - expect { - # `Player` and `Coach` are both embedded in `Team` and each declares a `statLine` relationship that - # chains up to the `teams` index. The two leaves share the relationship name `statLine`, so they would - # both register under it — a collision that would otherwise silently clobber one set of paths. - sourced_from_nested_paths_for_index("teams") do |s| - s.object_type "Team" do |t| - t.field "id", "ID!" - t.field "players", "[Player!]!" do |f| - f.mapping type: "object" - end - t.field "coaches", "[Coach!]!" do |f| - f.mapping type: "object" - end - t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true - t.index("teams") { |i| i.has_had_multiple_sources! } - end - - s.object_type "Player" do |t| - t.field "id", "ID!" - t.field "goals", "Int" do |f| - f.sourced_from "statLine", "goals" - end - t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Team", "statLines" - end - end - - s.object_type "Coach" do |t| - t.field "id", "ID!" - t.field "wins", "Int" do |f| - f.sourced_from "statLine", "wins" - end - t.relates_to_one "statLine", "StatLine", via: "coachId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Team", "statLines" - end - end - - s.object_type "StatLine" do |t| - t.field "id", "ID!" - t.field "teamId", "ID" - t.field "playerId", "ID" - t.field "coachId", "ID" - t.field "goals", "Int" - t.field "wins", "Int" - t.index "stat_lines" - end - end - }.to raise_error Errors::SchemaError, a_string_including( - "Index `teams` (for type `Team`) already has nested `sourced_from` paths registered for relationship `statLine`" - ) - end + it "distinguishes same-named leaf relationships by their embedding path" do + # `Player` and `Coach` are both embedded in `Team` and each declares a `statLine` relationship that + # chains up to the `teams` index. The two leaves share the relationship name `statLine`, but they're + # embedded via different fields (`players` vs `coaches`), so the embedding path keeps their keys distinct. + paths = sourced_from_nested_paths_for_index("teams") do |s| + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "players", "[Player!]!" do |f| + f.mapping type: "object" + end + t.field "coaches", "[Coach!]!" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true + t.index("teams") { |i| i.has_had_multiple_sources! } + end - # Defines a schema and returns the `sourced_from_nested_paths_by_relationship` registered on the named index. - def sourced_from_nested_paths_for_index(index_name, &schema) - define_schema(&schema) - .runtime_metadata.index_definitions_by_name - .fetch(index_name).sourced_from_nested_paths_by_relationship + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "Coach" do |t| + t.field "id", "ID!" + t.field "wins", "Int" do |f| + f.sourced_from "statLine", "wins" + end + t.relates_to_one "statLine", "StatLine", via: "coachId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "teamId", "ID" + t.field "playerId", "ID" + t.field "coachId", "ID" + t.field "goals", "Int" + t.field "wins", "Int" + t.index "stat_lines" + end + end + + expect(paths).to eq( + "players.statLine" => [ + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players", source_field: "playerId") + ], + "coaches.statLine" => [ + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "coaches", source_field: "coachId") + ] + ) end # Builds the standard `Team`/`Player`/`StatLine` nested `sourced_from` chain (`Player.statLine` declares a # `parent_relationship` to `Team.statLines`, with `Player` embedded in `Team.players`) and returns the - # `sourced_from_nested_paths_by_relationship` registered on the `teams` index. + # `sourced_from_nested_paths_by_qualified_relationship` registered on the `teams` index. def sourced_from_nested_paths(players_field: "[Player!]!") sourced_from_nested_paths_for_index("teams") do |s| s.object_type "Team" do |t| t.field "id", "ID!" t.field "players", players_field do |f| - f.mapping type: "object" if players_field.start_with?("[") + f.mapping type: "object" if f.type.list? end t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true t.index("teams") { |i| i.has_had_multiple_sources! } @@ -1088,6 +1101,13 @@ def sourced_from_nested_paths(players_field: "[Player!]!") end end end + + # Defines a schema and returns the `sourced_from_nested_paths_by_qualified_relationship` registered on the named index. + def sourced_from_nested_paths_for_index(index_name, &schema) + define_schema(&schema) + .runtime_metadata.index_definitions_by_name + .fetch(index_name).sourced_from_nested_paths_by_qualified_relationship + end end def index_definition_metadata_for(name, type_definition_order: ["NestedFields", "MyType"], on_my_type: nil, **options, &block) 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 e605049cc..7cf9c3a43 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 @@ -1401,7 +1401,7 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "teams", "[Team!]!" do |f| f.mapping type: "object" end - t.relates_to_many "leagueStatLines", "StatLine", via: "leagueId", dir: :in, indexing_only: true + t.relates_to_many "statLines", "StatLine", via: "leagueId", dir: :in, indexing_only: true t.index "leagues" end @@ -1410,8 +1410,8 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "players", "[Player!]!" do |f| f.mapping type: "object" end - t.relates_to_many "teamStatLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| - r.parent_relationship "League", "leagueStatLines" + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true do |r| + r.parent_relationship "League", "statLines" end end @@ -1420,18 +1420,18 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) t.field "gameAppearances", "[GameAppearance!]!" do |f| f.mapping type: "object" end - t.relates_to_many "playerStatLines", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Team", "teamStatLines" + t.relates_to_many "statLines", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" end end s.object_type "GameAppearance" do |t| t.field "id", "ID!" t.field "goals", "Int" do |f| - f.sourced_from "gameAppearanceStatLine", "goals" + f.sourced_from "statLine", "goals" end - t.relates_to_one "gameAppearanceStatLine", "StatLine", via: "gameAppearanceId", dir: :in, indexing_only: true do |r| - r.parent_relationship "Player", "playerStatLines" + t.relates_to_one "statLine", "StatLine", via: "gameAppearanceId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Player", "statLines" 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 e056cfca9..bdbb66e49 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 @@ -115,7 +115,7 @@ def static_param_with(value) StaticParam.new(value: value) end - def index_definition_with(route_with: nil, rollover: nil, default_sort_fields: [], current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: false, sourced_from_nested_paths_by_relationship: {}) + def index_definition_with(route_with: nil, rollover: nil, default_sort_fields: [], current_sources: [SELF_RELATIONSHIP_NAME], fields_by_path: {}, has_had_multiple_sources: false, sourced_from_nested_paths_by_qualified_relationship: {}) IndexDefinition.new( route_with: route_with, rollover: rollover, @@ -123,7 +123,7 @@ def index_definition_with(route_with: nil, rollover: nil, default_sort_fields: [ current_sources: current_sources, fields_by_path: fields_by_path, has_had_multiple_sources: has_had_multiple_sources, - sourced_from_nested_paths_by_relationship: sourced_from_nested_paths_by_relationship + sourced_from_nested_paths_by_qualified_relationship: sourced_from_nested_paths_by_qualified_relationship ) end From d7f53e830b645cf9164bfe3d8805dc8511879e58 Mon Sep 17 00:00:00 2001 From: ellisandrews-toast Date: Tue, 9 Jun 2026 17:02:20 -0400 Subject: [PATCH 3/3] Address review feedback on qualified relationship keying --- .../schema_definition/indexing/index.rb | 8 ++-- .../indexing/relationship_chain_resolver.rb | 21 +++++++++- .../sourced_from_update_targets_resolver.rb | 24 ++---------- .../schema_definition/indexing/index.rbs | 2 +- .../indexing/relationship_chain_resolver.rbs | 1 + .../sourced_from_update_targets_resolver.rbs | 1 - .../index_definitions_by_name_spec.rb | 39 +++++++++++++++++++ 7 files changed, 68 insertions(+), 28 deletions(-) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb index 3c91958c4..3618ba386 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/index.rb @@ -40,7 +40,7 @@ module Indexing # @return [Boolean] whether this index has ever had multiple sources # @!attribute [r] sourced_from_nested_paths_by_qualified_relationship # @return [Hash>] - # map from a (leaf) relationship name to the path segments the painless script uses to navigate from this + # map from a qualified (leaf) relationship to the path segments the painless script uses to navigate from this # root index's documents down to the nested elements that receive `sourced_from` data class Index < Struct.new( :name, @@ -283,10 +283,10 @@ def runtime_metadata ) end - # Registers a nested `sourced_from` chain's path segments under its qualified relationship. + # Registers a resolved `parent_relationship` chain on this index. # @api private - def register_sourced_from_nested_paths(qualified_relationship, path_segments) - sourced_from_nested_paths_by_qualified_relationship[qualified_relationship] = path_segments + def register_resolved_relationship_chain(resolved_chain) + sourced_from_nested_paths_by_qualified_relationship[resolved_chain.qualified_relationship] = resolved_chain.sourced_from_nested_paths end private diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb index 6f1e20849..1c7c6f772 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rb @@ -7,6 +7,7 @@ # frozen_string_literal: true require "elastic_graph/errors" +require "elastic_graph/schema_artifacts/runtime_metadata/sourced_from_nested_path_segment" module ElasticGraph module SchemaDefinition @@ -21,7 +22,25 @@ class ResolvedRelationshipChain < ::Data.define( ) # The leaf relationship name qualified by its embedding-field path (hence unique per resolved chain) def qualified_relationship - (path_segments.map { |segment| segment.field.name_in_index } + [leaf_relationship.name]).join(".") + (path_segments.map { |segment| segment.field.name_in_index } + [leaf_relationship.name_in_index]).join(".") + end + + # The runtime-metadata segments the painless script uses to navigate this chain: a `ListPathSegment` for + # each list embedding field (carrying the source field that matches the element) and an `ObjectPathSegment` + # for each object embedding field. + def sourced_from_nested_paths + path_segments.map do |segment| + if (source_field = segment.source_field_name) + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new( + field: segment.field.name_in_index, + source_field: source_field + ) + else + SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new( + field: segment.field.name_in_index + ) + end + end end end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb index 1dfe56915..3e1769e46 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rb @@ -7,7 +7,6 @@ # frozen_string_literal: true require "elastic_graph/errors" -require "elastic_graph/schema_artifacts/runtime_metadata/sourced_from_nested_path_segment" require "elastic_graph/schema_definition/indexing/relationship_chain_resolver" require "elastic_graph/schema_definition/indexing/relationship_resolver" require "elastic_graph/schema_definition/indexing/update_target_resolver" @@ -121,27 +120,10 @@ def resolve_relationship_chains(object_type) # chain has no nested data of its own; its navigation lives in the leaf relationship's chain. next unless relationship_names_with_sourced_fields.include?(relationship.name) - # Register the path segments on the root index, keyed by the chain's qualified relationship. + # Register the resolved chain on its root index, which records the path segments keyed by the + # chain's qualified relationship. root_index = resolved_chain.root_relationship.parent_type.index_def # : Index - root_index.register_sourced_from_nested_paths(resolved_chain.qualified_relationship, build_sourced_from_nested_paths(resolved_chain)) - end - end - - # Converts a resolved chain's path segments into the runtime-metadata segment types the index dumps. - # A segment with a `source_field_name` navigates into a list (and needs it to match an element); - # one without navigates into an object. - def build_sourced_from_nested_paths(resolved_chain) - resolved_chain.path_segments.map do |segment| - if (source_field = segment.source_field_name) - SchemaArtifacts::RuntimeMetadata::ListPathSegment.new( - field: segment.field.name_in_index, - source_field: source_field - ) - else - SchemaArtifacts::RuntimeMetadata::ObjectPathSegment.new( - field: segment.field.name_in_index - ) - end + root_index.register_resolved_relationship_chain(resolved_chain) end end diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs index 1f3eef1da..39ae5957c 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/index.rbs @@ -14,7 +14,7 @@ module ElasticGraph def to_index_config: () -> ::Hash[::String, untyped] def to_index_template_config: () -> ::Hash[::String, untyped] def runtime_metadata: () -> SchemaArtifacts::RuntimeMetadata::IndexDefinition - def register_sourced_from_nested_paths: (::String, ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment]) -> void + def register_resolved_relationship_chain: (ResolvedRelationshipChain) -> void private diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs index 7856465d3..75b80ece0 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/relationship_chain_resolver.rbs @@ -15,6 +15,7 @@ module ElasticGraph class ResolvedRelationshipChain < ResolvedRelationshipChainSuperType def qualified_relationship: () -> ::String + def sourced_from_nested_paths: () -> ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment] end class PathSegment diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs index 1c54b7e2f..c9c2ca72b 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/indexing/sourced_from_update_targets_resolver.rbs @@ -12,7 +12,6 @@ module ElasticGraph def resolve_for_type: (indexableType) { (::Symbol, ::String) -> void } -> ::Array[[::String, SchemaArtifacts::RuntimeMetadata::UpdateTarget]] def resolve_update_target: (indexableType, ResolvedRelationship, ::Array[SchemaElements::Field]) { (::Symbol, ::String) -> void } -> [::String, SchemaArtifacts::RuntimeMetadata::UpdateTarget]? def resolve_relationship_chains: (indexableType) { (::Symbol, ::String) -> void } -> void - def build_sourced_from_nested_paths: (ResolvedRelationshipChain) -> ::Array[SchemaArtifacts::RuntimeMetadata::sourcedFromNestedPathSegment] def sourced_fields_by_relationship_name: (indexableType) -> ::Hash[::String, ::Array[SchemaElements::Field]] def raise_if_errors: (::Array[::String], ::Array[::String]) -> void end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb index 41bea9393..fa39065f9 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/index_definitions_by_name_spec.rb @@ -933,6 +933,45 @@ def build_count_paths_from_mapping(mapping) ) end + it "uses the embedding field's `name_in_index` (not its GraphQL name) in the key and segments" do + # The `players` embedding field has a distinct `name_in_index`, so the qualified relationship key and + # the segment `field` must both use the index name (`players_in_index`), not the GraphQL name. + paths = sourced_from_nested_paths_for_index("teams") do |s| + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "players", "[Player!]!", name_in_index: "players_in_index" do |f| + f.mapping type: "object" + end + t.relates_to_many "statLines", "StatLine", via: "teamId", dir: :in, indexing_only: true + t.index("teams") { |i| i.has_had_multiple_sources! } + end + + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines" + end + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "teamId", "ID" + t.field "playerId", "ID" + t.field "goals", "Int" + t.index "stat_lines" + end + end + + expect(paths).to eq( + "players_in_index.statLine" => [ + SchemaArtifacts::RuntimeMetadata::ListPathSegment.new(field: "players_in_index", source_field: "playerId") + ] + ) + end + it "registers interleaved list and object segments for a chain spanning several levels" do # League -> Team -> Roster -> Player, where `Team` embeds `roster` as an object (not a list). # The resulting path interleaves `ListPathSegment` and `ObjectPathSegment` across its levels.