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 c6f64e7a4..3b9435d97 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 @@ -76,9 +76,9 @@ def resolve(starting_relationship) private - # Recursively walks from leaf to root, collecting relationships and building path segments in reverse. - # Returns the root relationship (the one with no parent_ref) on success, or nil if an error was - # encountered. + # Recursively walks from leaf to root, collecting relationships and their path segments (leaf-to-root; + # `resolve` reverses both to root-to-leaf). Returns the root relationship (the one with no parent_ref) on + # success, or nil if an error was encountered. def resolve_chain(current_rel, path_segments, relationships, errors) relationships << current_rel @@ -88,7 +88,7 @@ def resolve_chain(current_rel, path_segments, relationships, errors) parent_rel = resolve_parent_ref(current_rel, parent_ref, relationships, errors) return nil unless parent_rel - build_path_segment(current_rel, parent_rel.parent_type, path_segments, errors) + path_segments.concat(build_path_segments(current_rel, parent_rel.parent_type, errors)) return nil if errors.any? resolve_chain(parent_rel, path_segments, relationships, errors) @@ -136,41 +136,54 @@ def resolve_parent_ref(current_rel, ref, relationships, errors) parent_rel end - # Builds a PathSegment for the current level and appends it to path_segments. - # Uses the explicitly specified field name if provided, otherwise auto-discovers it. - def build_path_segment(current_rel, parent_type, path_segments, errors) - parent_ref = current_rel.parent_ref # : SchemaElements::Relationship::ParentRef - field = resolve_field(parent_ref, parent_type, current_rel, errors) - return unless field - - # For list fields, `source_field_name` identifies which element to update: the one whose - # `id` matches `event[source_field_name]`. We implicitly match on `id` because ElasticGraph - # relationships always join on `id` via foreign keys. For non-list fields, it's nil since - # there's no ambiguity. - path_segments << if field.type.list? - PathSegment.new( - field: field, - source_field_name: current_rel.foreign_key - ) - else - PathSegment.new( - field: field, - source_field_name: nil - ) + # Builds this link's PathSegment(s). The embedding field may be a dotted path through intermediate object + # fields, so each path component becomes a segment. Only the final field, when a list, identifies which + # element to update: it carries the relationship's foreign key, matched against the element's `id`. + def build_path_segments(current_rel, parent_type, errors) + parts = resolve_embedding_fields(current_rel, parent_type, errors) + + link_segments = parts.map.with_index(1) do |field, index| + source_field_name = current_rel.foreign_key if index == parts.size && field.type.list? + PathSegment.new(field: field, source_field_name: source_field_name) end + + # `resolve_chain` walks leaf-to-root and reverses the whole list once at the end, so reverse + # the link segments to restore root-to-leaf order within the link. + link_segments.reverse end - def resolve_field(parent_ref, parent_type, current_rel, errors) - if parent_ref.field_name - field = parent_type.indexing_fields_by_name_in_index[parent_ref.field_name] - unless field - errors << "#{rel_description(current_rel)} references field `#{parent_type.name}.#{parent_ref.field_name}` " \ - "via `parent_relationship`, but that field does not exist." + # Resolves this link's embedding field(s) on `parent_type`, one per dotted-path component. An explicit + # `parent_field_name` is resolved by public name (consistent with `sourced_from` and `equivalent_field`); + # the resolved `name_in_index` is what flows into the qualified relationship and the painless script. + # + # Only the final path component may be a list: it carries the relationship's foreign key, matched against + # the element's `id`. An intermediate list would need its own match value that a single link can't supply, + # so we refuse to descend through it (which halts `resolve_public_path`) and report the dedicated error. + # Returns `[]` (and records an error) when it doesn't resolve. + def resolve_embedding_fields(current_rel, parent_type, errors) + parent_ref = current_rel.parent_ref # : SchemaElements::Relationship::ParentRef + field_name = parent_ref.field_name + return [find_field_by_type(parent_type, current_rel, errors)].compact unless field_name + + field_path = @schema_def_state.field_path_resolver.resolve_public_path(parent_type, field_name) do |parent_field| + if parent_field.type.list? + errors << "#{rel_description(current_rel)} embeds through list field `#{parent_field.parent_type.name}.#{parent_field.name}` " \ + "via `parent_relationship`, but only the final embedding field may be a list. To source through an " \ + "intermediate list, give its embedded type its own `relates_to_one`/`relates_to_many` to the source " \ + "type with a `parent_relationship`, so that list level is matched by its own foreign key." + return [] # : ::Array[SchemaElements::Field] end - field - else - find_field_by_type(parent_type, current_rel, errors) + + true end + + unless field_path + errors << "#{rel_description(current_rel)} references field `#{parent_type.name}.#{field_name}` " \ + "via `parent_relationship`, but that field does not exist." + return [] # : ::Array[SchemaElements::Field] + end + + field_path.path_parts end def find_field_by_type(parent_type, current_rel, errors) 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 9538dff3b..cd4654d2a 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 @@ -34,14 +34,13 @@ module ElasticGraph ::Array[::String] ) -> SchemaElements::Relationship? - def build_path_segment: ( + def build_path_segments: ( SchemaElements::Relationship, indexableType, - ::Array[PathSegment], ::Array[::String] - ) -> void + ) -> ::Array[PathSegment] - def resolve_field: (SchemaElements::Relationship::ParentRef, indexableType, SchemaElements::Relationship, ::Array[::String]) -> SchemaElements::Field? + def resolve_embedding_fields: (SchemaElements::Relationship, indexableType, ::Array[::String]) -> ::Array[SchemaElements::Field] def find_field_by_type: (indexableType, SchemaElements::Relationship, ::Array[::String]) -> SchemaElements::Field? def rel_description: (SchemaElements::Relationship) -> ::String 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 07557cd8a..0b2c26c36 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 @@ -1513,6 +1513,64 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) expect_statline_update_target_with(metadata) end + it "resolves a `parent_field_name` by public field name even when the embedding field has a distinct `name_in_index`" do + metadata = nested_sourced_from_schema( + on_team: ->(t) { + t.field "players", "[Player!]!", name_in_index: "the_players" do |f| + f.mapping type: "object" + end + }, + players_field: nil, + on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines", parent_field_name: "players" } + ) + + expect_statline_update_target_with(metadata, relationship: "the_players.statLine") + end + + context "when the embedding field is reached via a dotted `parent_field_name` path" do + it "descends an object segment to a leaf list segment, keying the target by the full qualified relationship" do + metadata = nested_sourced_from_schema( + embed_players_under: "TeamNestedFields", + on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines", parent_field_name: "nested.players" } + ) + + expect_statline_update_target_with(metadata, relationship: "nested.players.statLine") + end + + it "omits `path_identifier_params` when the leaf of the dotted path is an object (not a list)" do + metadata = nested_sourced_from_schema( + embed_players_under: "TeamNestedFields", + players_field: "Player!", + on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines", parent_field_name: "nested.players" } + ) + + expect_statline_update_target_with(metadata, relationship: "nested.players.statLine", path_identifier_params: {}) + end + + it "raises a clear error pointing at the multi-link solution when an intermediate segment is a list" do + expect { + nested_sourced_from_schema( + embed_players_under: "[TeamNestedFields!]!", + on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines", parent_field_name: "nested.players" } + ) + }.to raise_error Errors::SchemaError, a_string_including( + "`Player.statLine` embeds through list field `Team.nested` via `parent_relationship`, but only the final embedding field may be a list.", + "give its embedded type its own `relates_to_one`/`relates_to_many` to the source type with a `parent_relationship`" + ) + end + + it "raises a clear error when the dotted path does not resolve to a field" do + expect { + nested_sourced_from_schema( + embed_players_under: "TeamNestedFields", + on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines", parent_field_name: "nested.nonexistent" } + ) + }.to raise_error Errors::SchemaError, a_string_including( + "`Player.statLine` references field `Team.nested.nonexistent` via `parent_relationship`, but that field does not exist." + ) + end + end + it "raises an error when `parent_relationship` is called twice on the same relationship" do expect { nested_sourced_from_schema( @@ -1635,6 +1693,77 @@ def raise_error_about_workspace_relationship(details, sourced_fields: true) expect(nested_update_targets_by_relationship(metadata).keys).to contain_exactly("teams.players.gameAppearances.statLine") end + it "resolves a deeply nested chain mixing object dotted-path hops, multiple list links, and `name_in_index` renames" do + # Embedding path (root -> leaf), * = list level. Each list level is its own `parent_relationship` link; + # the object hops are folded into the links' dotted `parent_field_name`s: + # Team .roster(the_roster) .squads* .lineup .players(the_players)* -> Player.goals (sourced) + metadata = + object_type_metadata_for "StatLine" do |s| + s.object_type "Team" do |t| + t.field "id", "ID!" + t.field "roster", "Roster", name_in_index: "the_roster" + 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 "Roster" do |t| + t.field "squads", "[Squad!]!" do |f| + f.mapping type: "object" + end + end + + s.object_type "Squad" do |t| + t.field "id", "ID!" + t.field "lineup", "Lineup" + t.relates_to_many "statLines", "StatLine", via: "squadId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Team", "statLines", parent_field_name: "roster.squads" + end + end + + s.object_type "Lineup" do |t| + t.field "players", "[Player!]!", name_in_index: "the_players" do |f| + f.mapping type: "object" + end + end + + s.object_type "Player" do |t| + t.field "id", "ID!" + t.field "goals", "Int" do |f| + f.sourced_from "statLine", "stats.goals" + end + t.relates_to_one "statLine", "StatLine", via: "playerId", dir: :in, indexing_only: true do |r| + r.parent_relationship "Squad", "statLines", parent_field_name: "lineup.players" + end + end + + s.object_type "StatLineStats" do |t| + t.field "goals", "Int" + end + + s.object_type "StatLine" do |t| + t.field "id", "ID!" + t.field "teamId", "ID" + t.field "squadId", "ID" + t.field "playerId", "ID" + t.field "stats", "StatLineStats" + t.index "stat_lines" + end + end + + target = nested_update_targets_by_relationship(metadata).fetch("the_roster.squads.lineup.the_players.statLine") + + expect(target.type).to eq "Team" + expect(target.id_source).to eq "teamId" + expect(target.sourced_from_nested_params.field_params).to eq( + "goals" => dynamic_param_with(source_path: "stats.goals", cardinality: :one) + ) + # One identifier per list level; the object hops contribute none. + expect(target.sourced_from_nested_params.path_identifier_params).to eq( + "squadId" => dynamic_param_with(source_path: "squadId", cardinality: :one), + "playerId" => dynamic_param_with(source_path: "playerId", cardinality: :one) + ) + end + it "raises an error when the parent type does not exist" do expect { nested_sourced_from_schema( @@ -1816,6 +1945,7 @@ def nested_sourced_from_schema( on_player_relationship: ->(r) { r.parent_relationship "Team", "statLines" }, player_indexing_only: true, players_field: "[Player!]!", + embed_players_under: nil, index_teams: true, index_players: false, multiple_sources: true, @@ -1828,12 +1958,24 @@ def nested_sourced_from_schema( ) # `StatLine` is the source type, so its metadata carries the nested update targets we assert on. object_type_metadata_for "StatLine" do |s| + if embed_players_under + s.object_type s.state.type_ref(embed_players_under).fully_unwrapped.name do |t| + t.field "players", players_field do |f| + f.mapping type: "object" if f.type.list? + end + end + end + s.object_type "Team" do |t| t.field "id", "ID!" - if players_field + if embed_players_under + t.field "nested", embed_players_under do |f| + f.mapping type: "object" if f.type.list? + end + elsif players_field 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 end