Skip to content

Commit

Permalink
fix key validations.
Browse files Browse the repository at this point in the history
  • Loading branch information
gmac committed May 31, 2024
1 parent 47cd28d commit 4fce9b9
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 22 deletions.
12 changes: 10 additions & 2 deletions lib/graphql/stitching/composer/validate_boundaries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ def validate_as_boundary(supergraph, type, candidate_types_by_location, boundari
memo[boundary.location][boundary.key] = boundary
end

# All non-key fields must be available in at least one boundary location
boundary_keys = boundaries.map(&:key).tap(&:uniq!)
boundary_keys = boundaries.map(&:key).to_set

# All non-key fields must be resolvable in at least one boundary location
supergraph.locations_by_type_and_field[type.graphql_name].each do |field_name, locations|
next if boundary_keys.include?(field_name)

Expand All @@ -52,6 +53,13 @@ def validate_as_boundary(supergraph, type, candidate_types_by_location, boundari
end
end

# All locations of a boundary type must include at least one key field
supergraph.fields_by_type_and_location[type.graphql_name].each do |location, field_names|
if field_names.none? { boundary_keys.include?(_1) }
raise Composer::ValidationError, "A boundary key is required for `#{type.graphql_name}` in #{location} to join with other locations."
end
end

# verify that all outbound locations can access all inbound locations
resolver_locations = boundaries_by_location_and_key.keys
candidate_types_by_location.each_key do |location|
Expand Down
28 changes: 28 additions & 0 deletions test/graphql/stitching/composer/validate_boundaries_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ def test_permits_no_boundary_query_for_key_only_types
assert compose_definitions({ "a" => a, "b" => b })
end

def test_validates_subset_types_have_a_key
a = %{type T { id:ID! name:String } type Query { a(id: ID!):T @stitch(key: "id") }}
b = %{type T { name:String } type Query { b:T }}

assert_error("A boundary key is required for `T` in b", ValidationError) do
compose_definitions({ "a" => a, "b" => b })
end
end

def test_validates_bidirection_types_are_mutually_accessible
a = %{
type T { upc:ID! name:String }
Expand All @@ -93,6 +102,25 @@ def test_validates_bidirection_types_are_mutually_accessible
end
end

def test_validates_key_only_types_are_mutually_accessible
a = %|
type T { upc:ID! }
type Query { a(upc:ID!):T @stitch(key: "upc") }
|
b = %|
type T { id:ID! }
type Query { b(id:ID!):T @stitch(key: "id") }
|
c = %|
type T { id:ID! }
type Query { c(id:ID!):T @stitch(key: "id") }
|

assert_error("Cannot route `T` boundaries in a", ValidationError) do
compose_definitions({ "a" => a, "b" => b, "c" => c })
end
end

def test_validates_outbound_types_can_access_all_bidirection_types
a = %{
type T { upc:ID! }
Expand Down
40 changes: 20 additions & 20 deletions test/graphql/stitching/supergraph_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,26 +290,26 @@ def test_route_type_to_locations_favors_longer_paths_through_necessary_locations
assert routes.none? { |_key, path| path.any? { _1["location"] == "e" } }
end

def test_route_type_to_locations_returns_nil_for_unreachable_locations
a = %|
type T { upc:ID! }
type Query { a(upc:ID!):T @stitch(key: "upc") }
|
b = %|
type T { id:ID! }
type Query { b(id:ID!):T @stitch(key: "id") }
|
c = %|
type T { id:ID! }
type Query { c(id:ID!):T @stitch(key: "id") }
|

supergraph = compose_definitions({ "a" => a, "b" => b, "c" => c })

routes = supergraph.route_type_to_locations("T", "b", ["a", "c"])
assert_equal ["c"], routes["c"].map { _1["location"] }
assert_nil routes["a"]
end
# def test_route_type_to_locations_returns_nil_for_unreachable_locations
# a = %|
# type T { upc:ID! }
# type Query { a(upc:ID!):T @stitch(key: "upc") }
# |
# b = %|
# type T { id:ID! }
# type Query { b(id:ID!):T @stitch(key: "id") }
# |
# c = %|
# type T { id:ID! }
# type Query { c(id:ID!):T @stitch(key: "id") }
# |

# supergraph = compose_definitions({ "a" => a, "b" => b, "c" => c })

# routes = supergraph.route_type_to_locations("T", "b", ["a", "c"])
# assert_equal ["c"], routes["c"].map { _1["location"] }
# assert_nil routes["a"]
# end

describe "#to_definition / #from_definition" do
def setup
Expand Down

0 comments on commit 4fce9b9

Please sign in to comment.