Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nested root scopes #123

Merged
merged 1 commit into from
Feb 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions lib/graphql/stitching/executor/root_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,22 @@ def fetch(ops)
result = @executor.request.supergraph.execute_at_location(op.location, query_document, query_variables, @executor.request)
@executor.query_count += 1

@executor.data.merge!(result["data"]) if result["data"]
if result["data"]
if op.path.any?
# Nested root scopes must expand their pathed origin set
origin_set = op.path.reduce([@executor.data]) do |set, ns|
set.flat_map { |obj| obj && obj[ns] }.tap(&:compact!)
end

origin_set.each { _1.merge!(result["data"]) }
else
# Actual root scopes merge directly into results data
@executor.data.merge!(result["data"])
end
end

if result["errors"]&.any?
result["errors"].each { _1.delete("locations") }
@executor.errors.concat(result["errors"])
@executor.errors.concat(format_errors!(result["errors"], op.path))
end

ops.map(&:step)
Expand Down Expand Up @@ -51,6 +63,16 @@ def build_document(op, operation_name = nil, operation_directives = nil)
doc << op.selections
doc
end

# Format response errors without a document location (because it won't match the request doc),
# and prepend any insertion path for the scope into error paths.
def format_errors!(errors, path)
errors.each do |err|
err.delete("locations")
err["path"].unshift(*path) if err["path"] && path.any?
end
errors
end
end
end
end
26 changes: 14 additions & 12 deletions lib/graphql/stitching/planner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -276,27 +276,29 @@ def extract_locale_selections(
routes.each_value do |route|
route.reduce(locale_selections) do |parent_selections, boundary|
# E.1) Add the key of each boundary query into the prior location's selection set.
foreign_key = ExportSelection.key(boundary.key)
has_key = false
has_typename = false

parent_selections.each do |node|
next unless node.is_a?(GraphQL::Language::Nodes::Field)
has_key ||= node.alias == foreign_key
has_typename ||= node.alias == ExportSelection.typename_node.alias
if boundary.key
foreign_key = ExportSelection.key(boundary.key)
has_key = false
has_typename = false

parent_selections.each do |node|
next unless node.is_a?(GraphQL::Language::Nodes::Field)
has_key ||= node.alias == foreign_key
has_typename ||= node.alias == ExportSelection.typename_node.alias
end

parent_selections << ExportSelection.key_node(boundary.key) unless has_key
parent_selections << ExportSelection.typename_node unless has_typename
end

parent_selections << ExportSelection.key_node(boundary.key) unless has_key
parent_selections << ExportSelection.typename_node unless has_typename

# E.2) Add a planner step for each new entrypoint location.
add_step(
location: boundary.location,
parent_index: parent_index,
parent_type: parent_type,
selections: remote_selections_by_location[boundary.location] || [],
path: path.dup,
boundary: boundary,
boundary: boundary.key ? boundary : nil,
).selections
end
end
Expand Down
31 changes: 22 additions & 9 deletions lib/graphql/stitching/supergraph.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,11 @@ def locations_by_type
# ("Type") => ["id", ...]
def possible_keys_for_type(type_name)
@possible_keys_by_type[type_name] ||= begin
@boundaries[type_name].map(&:key).tap(&:uniq!)
if type_name == @schema.query.graphql_name
GraphQL::Stitching::EMPTY_ARRAY
else
@boundaries[type_name].map(&:key).tap(&:uniq!)
end
end
end

Expand All @@ -262,16 +266,25 @@ def possible_keys_for_type_and_location(type_name, location)
# For a given type, route from one origin location to one or more remote locations
# used to connect a partial type across locations via boundary queries
def route_type_to_locations(type_name, start_location, goal_locations)
if possible_keys_for_type(type_name).length > 1
key_count = possible_keys_for_type(type_name).length

if key_count.zero?
# nested root scopes have no boundary keys and just return a location
goal_locations.each_with_object({}) do |goal_location, memo|
memo[goal_location] = [Boundary.new(location: goal_location)]
end

elsif key_count > 1
# multiple keys use an A* search to traverse intermediary locations
return route_type_to_locations_via_search(type_name, start_location, goal_locations)
end
route_type_to_locations_via_search(type_name, start_location, goal_locations)

# types with a single key attribute must all be within a single hop of each other,
# so can use a simple match to collect boundaries for the goal locations.
@boundaries[type_name].each_with_object({}) do |boundary, memo|
if goal_locations.include?(boundary.location)
memo[boundary.location] = [boundary]
else
# types with a single key attribute must all be within a single hop of each other,
# so can use a simple match to collect boundaries for the goal locations.
@boundaries[type_name].each_with_object({}) do |boundary, memo|
if goal_locations.include?(boundary.location)
memo[boundary.location] = [boundary]
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/stitching/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

module GraphQL
module Stitching
VERSION = "1.2.1"
VERSION = "1.2.2"
end
end
88 changes: 88 additions & 0 deletions test/graphql/stitching/integration/nested_root_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# frozen_string_literal: true

require "test_helper"
require_relative "../../../schemas/nested_root"

describe 'GraphQL::Stitching, nested root scopes' do
def setup
@supergraph = compose_definitions({
"a" => Schemas::NestedRoot::Alpha,
"b" => Schemas::NestedRoot::Bravo,
})
end

def test_nested_root_scopes
source = %|
mutation {
doStuff {
apple
banana
}
}
|

expected = {
"doStuff" => {
"apple" => "red",
"banana" => "yellow",
}
}

result = plan_and_execute(@supergraph, source)
assert_equal expected, result["data"]
end

def test_nested_root_scopes_with_complex_paths
source = %|
mutation {
doThings {
query {
apple
banana
}
}
}
|

expected = {
"doThings" => [
{
"query" => {
"apple" => "red",
"banana" => "yellow",
}
},
{
"query" => {
"apple" => "red",
"banana" => "yellow",
}
},
]
}

result = plan_and_execute(@supergraph, source)
assert_equal expected, result["data"]
end

def test_nested_root_scopes_repath_errors
source = %|
mutation {
doThing {
query {
errorA
errorB
}
}
}
|

expected = [
{ "message" => "a", "path" => ["doThing", "query", "errorA"] },
{ "message" => "b", "path" => ["doThing", "query", "errorB"] },
]

result = plan_and_execute(@supergraph, source)
assert_equal expected, result["errors"]
end
end
68 changes: 68 additions & 0 deletions test/schemas/nested_root.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# frozen_string_literal: true

module Schemas
module NestedRoot
class Alpha < GraphQL::Schema
class Query < GraphQL::Schema::Object
field :apple, String, null: false
field :error_a, String, null: false

def apple
"red"
end

def error_a
raise GraphQL::ExecutionError.new("a")
end
end

class Thing < GraphQL::Schema::Object
field :query, Query, null: false

def query
{}
end
end

class Mutation < GraphQL::Schema::Object
field :do_stuff, Query, null: false

def do_stuff
{}
end

field :do_thing, Thing, null: false

def do_thing
{}
end

field :do_things, [Thing], null: false

def do_things
[{}, {}]
end
end

query Query
mutation Mutation
end

class Bravo < GraphQL::Schema
class Query < GraphQL::Schema::Object
field :banana, String, null: false
field :error_b, String, null: false

def banana
"yellow"
end

def error_b
raise GraphQL::ExecutionError.new("b")
end
end

query Query
end
end
end
Loading