Better support for boundary types without resolvers (#135)

README.md

@@ -153,15 +153,15 @@ type Query {
 * The `@stitch` directive is applied to a root query where the merged type may be accessed. The merged type identity is inferred from the field return.
 * The `key: "id"` parameter indicates that an `{ id }` must be selected from prior locations so it may be submitted as an argument to this query. The query argument used to send the key is inferred when possible ([more on arguments](#multiple-query-arguments) later).
 
-Each location that provides a unique variant of a type must provide at least one resolver query for the type. The exception to this requirement are [foreign key types](./docs/mechanics.md##modeling-foreign-keys-for-stitching) that contain only a single key field:
+Each location that provides a unique variant of a type must provide at least one resolver query for the type. The exception to this requirement are [outbound-only types](./docs/mechanics.md#outbound-only-merged-types) and/or [foreign key types](./docs/mechanics.md##modeling-foreign-keys-for-stitching) that contain no exclusive data:
 
 ```graphql
 type Product {
   id: ID!
 }
 ```
 
-The above representation of a `Product` type provides no unique data beyond a key that is available in other locations. Thus, this representation will never require an inbound request to fetch it, and its resolver query may be omitted.
+The above representation of a `Product` type contains nothing but a key that is available in other locations. Therefore, this representation will never require an inbound request to fetch it, and its resolver query may be omitted.
 
 #### List queries
 

docs/mechanics.md

@@ -303,3 +303,45 @@ And produces this result:
 ```
 
 Location B is allowed to return `null` here because its one unique field, `rating`, is nullable (the `id` field can be provided by Location A). If `rating` were non-null, then null bubbling would invalidate the response data.
+
+### Outbound-only merged types
+
+Merged types do not always require a resolver query. For example:
+
+```graphql
+# -- Location A
+
+type Widget {
+  id: ID!
+  name: String
+}
+
+type Query {
+  widgetA(id: ID!): Widget @stitch(key: "id")
+}
+
+# -- Location B
+
+type Widget {
+  id: ID!
+  size: Float
+}
+
+type Query {
+  widgetB(id: ID!): Widget @stitch(key: "id")
+}
+
+# -- Location C
+
+type Widget {
+  id: ID!
+  name: String
+  size: Float
+}
+
+type Query {
+  featuredWidget: Widget
+}
+```
+
+In this graph, `Widget` is a merged type without a resolver query in location C. This works because all of its fields are resolvable in other locations; that means location C can provide outbound representations of this type without ever needing to resolve inbound requests for it. Outbound types do still require a key field (such as `id` above) that allow them to join with data in other resolver locations.

lib/graphql/stitching/composer.rb

@@ -68,6 +68,7 @@ def initialize(
         @boundary_map = nil
         @mapped_type_names = nil
         @candidate_directives_by_name_and_location = nil
+        @candidate_types_by_name_and_location = nil
         @schema_directives = nil
       end
 

lib/graphql/stitching/composer/validate_boundaries.rb

@@ -4,29 +4,29 @@ module GraphQL::Stitching
   class Composer
     class ValidateBoundaries < BaseValidator
 
-      def perform(ctx, composer)
-        ctx.schema.types.each do |type_name, type|
+      def perform(supergraph, composer)
+        supergraph.schema.types.each do |type_name, type|
           # objects and interfaces that are not the root operation types
           next unless type.kind.object? || type.kind.interface?
-          next if ctx.schema.query == type || ctx.schema.mutation == type
+          next if supergraph.schema.query == type || supergraph.schema.mutation == type
           next if type.graphql_name.start_with?("__")
 
           # multiple subschemas implement the type
           candidate_types_by_location = composer.candidate_types_by_name_and_location[type_name]
           next unless candidate_types_by_location.length > 1
 
-          boundaries = ctx.boundaries[type_name]
+          boundaries = supergraph.boundaries[type_name]
           if boundaries&.any?
-            validate_as_boundary(ctx, type, candidate_types_by_location, boundaries)
+            validate_as_boundary(supergraph, type, candidate_types_by_location, boundaries)
           elsif type.kind.object?
-            validate_as_shared(ctx, type, candidate_types_by_location)
+            validate_as_shared(supergraph, type, candidate_types_by_location)
           end
         end
       end
 
       private
 
-      def validate_as_boundary(ctx, type, candidate_types_by_location, boundaries)
+      def validate_as_boundary(supergraph, type, candidate_types_by_location, boundaries)
         # abstract boundaries are expanded with their concrete implementations, which each get validated. Ignore the abstract itself.
         return if type.kind.abstract?
 
@@ -41,33 +41,38 @@ def validate_as_boundary(ctx, type, candidate_types_by_location, boundaries)
           memo[boundary.location][boundary.key] = boundary
         end
 
-        boundary_keys = boundaries.map { _1.key }.uniq
-        key_only_types_by_location = candidate_types_by_location.select do |location, subschema_type|
-          subschema_type.fields.keys.length == 1 && boundary_keys.include?(subschema_type.fields.keys.first)
-        end
+        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)
 
-        # all locations have a boundary, or else are key-only
-        candidate_types_by_location.each do |location, subschema_type|
-          unless boundaries_by_location_and_key[location] || key_only_types_by_location[location]
-            raise Composer::ValidationError, "A boundary query is required for `#{type.graphql_name}` in #{location} because it provides unique fields."
+          if locations.none? { boundaries_by_location_and_key[_1] }
+            where = locations.length > 1 ? "one of #{locations.join(", ")} locations" : locations.first
+            raise Composer::ValidationError, "A boundary query is required for `#{type.graphql_name}` in #{where} to resolve field `#{field_name}`."
           end
         end
 
-        outbound_access_locations = key_only_types_by_location.keys
-        bidirectional_access_locations = candidate_types_by_location.keys - outbound_access_locations
+        # 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
-        (outbound_access_locations + bidirectional_access_locations).each do |location|
-          remote_locations = bidirectional_access_locations.reject { _1 == location }
-          paths = ctx.route_type_to_locations(type.graphql_name, location, remote_locations)
+        resolver_locations = boundaries_by_location_and_key.keys
+        candidate_types_by_location.each_key do |location|
+          remote_locations = resolver_locations.reject { _1 == location }
+          paths = supergraph.route_type_to_locations(type.graphql_name, location, remote_locations)
           if paths.length != remote_locations.length || paths.any? { |_loc, path| path.nil? }
             raise Composer::ValidationError, "Cannot route `#{type.graphql_name}` boundaries in #{location} to all other locations. "\
               "All locations must provide a boundary accessor that uses a conjoining key."
           end
         end
       end
 
-      def validate_as_shared(ctx, type, candidate_types_by_location)
+      def validate_as_shared(supergraph, type, candidate_types_by_location)
         expected_fields = begin
           type.fields.keys.sort
         rescue StandardError => e
@@ -79,8 +84,8 @@ def validate_as_shared(ctx, type, candidate_types_by_location)
           end
         end
 
-        candidate_types_by_location.each do |location, subschema_type|
-          if subschema_type.fields.keys.sort != expected_fields
+        candidate_types_by_location.each do |location, candidate_type|
+          if candidate_type.fields.keys.sort != expected_fields
             raise Composer::ValidationError, "Shared type `#{type.graphql_name}` must have consistent fields across locations, "\
               "or else define boundary queries so that its unique fields may be accessed remotely."
           end

lib/graphql/stitching/version.rb

@@ -2,6 +2,6 @@
 
 module GraphQL
   module Stitching
-    VERSION = "1.2.4"
+    VERSION = "1.2.5"
   end
 end

test/graphql/stitching/composer/validate_boundaries_test.rb

@@ -40,7 +40,7 @@ def test_permits_multiple_boundary_query_keys_per_type_location
     assert compose_definitions({ "a" => a, "b" => b, "c" => c })
   end
 
-  def test_validates_at_least_one_boundary_per_type_location
+  def test_validates_boundary_present_when_providing_unique_fields
     a = %{type T { id:ID! name:String } type Query { a(id: ID!):T @stitch(key: "id") }}
     b = %{type T { id:ID! size:Float } type Query { b:T }}
 
@@ -49,13 +49,40 @@ def test_validates_at_least_one_boundary_per_type_location
     end
   end
 
+  def test_validates_boundary_present_in_multiple_locations_when_providing_unique_fields
+    a = %{type T { id:ID! name:String } type Query { a(id: ID!):T @stitch(key: "id") }}
+    b = %{type T { id:ID! size:Float } type Query { b:T }}
+    c = %{type T { id:ID! size:Float } type Query { c:T }}
+
+    assert_error("A boundary query is required for `T` in one of b, c locations", ValidationError) do
+      compose_definitions({ "a" => a, "b" => b, "c" => c })
+    end
+  end
+
+  def test_permits_no_boundary_query_for_types_that_can_be_fully_resolved_elsewhere
+    a = %{type T { id:ID! name:String } type Query { a(id: ID!):T @stitch(key: "id") }}
+    b = %{type T { id:ID! size:Float } type Query { b(id: ID!):T @stitch(key: "id") }}
+    c = %{type T { id:ID! size:Float name:String } type Query { c:T }}
+
+    assert compose_definitions({ "a" => a, "b" => b, "c" => c })
+  end
+
   def test_permits_no_boundary_query_for_key_only_types
     a = %{type T { id:ID! name:String } type Query { a(id: ID!):T @stitch(key: "id") }}
     b = %{type T { id:ID! } type Query { b:T }}
 
     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 }
@@ -75,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! }

test/graphql/stitching/supergraph_test.rb

@@ -290,27 +290,6 @@ 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
-
   describe "#to_definition / #from_definition" do
     def setup
       alpha = %|