fix: ensure all interface members are also entities

Every entity that implements an interface entity must include all `@keys` from the interface definition.
Therefore, we add this extra validation to ensure that.

Author: nogates

lib/apollo-federation/entity.rb

@@ -16,7 +16,7 @@ def self.resolve_type(object, context)
     # is an union. Therefore, we have to extend this validation to allow interfaces as possible types.
     def self.assert_valid_union_member(type_defn)
       if type_defn.is_a?(Module) &&
-          type_defn.included_modules.include?(ApolloFederation::Interface)
+         type_defn.included_modules.include?(ApolloFederation::Interface)
         # It's an interface entity, defined as a module
       else
         super(type_defn)

lib/apollo-federation/schema.rb

@@ -79,13 +79,59 @@ def schema_entities
         # infinite recursion
         types_schema.orphan_types(original_query)
 
-        # Walk through all of the types and determine which ones are entities (any type with a
-        # "key" directive)
-        types_schema.send(:non_introspection_types).values.flatten.select do |type|
-          # TODO: Find Objects that implement interfaces that are entities. Make sure they are also entities.
-          (type.include?(ApolloFederation::Object) || type.include?(ApolloFederation::Interface)) &&
-            type.federation_directives&.any? { |directive| directive[:name] == 'key' }
+        entities_collection, federation_entities, interface_types_map = collect_entitites(types_schema)
+
+        if federation_entities.any?
+          entity_names = entities_collection.map(&:graphql_name)
+
+          federation_entities.each do |interface|
+            members = interface_types_map.fetch(interface.graphql_name, [])
+            not_entity_members = members.reject { |member| entity_names.include?(member) }
+
+            # If all interface members are entities, it is valid so we add it to the collection
+            if not_entity_members.empty?
+              entities_collection << interface
+            else
+              raise "Interface #{interface.graphql_name} is not valid. " \
+                "Types `#{not_entity_members.join(', ')}` do not have a @key directive. " \
+                'All types that implement an interface with a @key directive must also have a @key directive.'
+            end
+          end
         end
+
+        entities_collection
+      end
+
+      # Walk through all of the types and interfaces and determine which ones are entities
+      # (any type with a "key" directive)
+      # However, for interface entities, don't add them straight away, but first check that
+      # all implementing types of the interfaces are also entities.
+      def collect_entitites(types_schema)
+        federation_entities = []
+        interface_types_map = {}
+
+        entities_collection = types_schema.send(:non_introspection_types).values.flatten.select do |type|
+          # keep track of the interfaces -> type relations.
+          if type.respond_to?(:implements)
+            type.implements.each do |interface|
+              interface_types_map[interface.abstract_type.graphql_name] ||= []
+              interface_types_map[interface.abstract_type.graphql_name] << type.graphql_name
+            end
+          end
+
+          # Only add Type entities to the collection
+          # Interface entities will be added later if all implementing types are entities
+          if type.include?(ApolloFederation::Object) && includes_key_directive?(type)
+            true
+          elsif type.include?(ApolloFederation::Interface) && includes_key_directive?(type)
+            federation_entities << type
+            false
+          else
+            false
+          end
+        end
+
+        [entities_collection, federation_entities, interface_types_map]
       end
 
       def federation_query(query_obj)
@@ -110,6 +156,10 @@ def federation_query(query_obj)
         klass.define_service_field
         klass
       end
+
+      def includes_key_directive?(type)
+        type.federation_directives&.any? { |directive| directive[:name] == 'key' }
+      end
     end
   end
 end

spec/apollo-federation/entities_field_interfaces_spec.rb

@@ -36,6 +36,39 @@
     end
 
     context 'when an interface with the key directive exists' do
+      context "when some of the types implementing the inteface don't have the key directive" do
+        let(:offending_class) do
+        end
+        let(:query) do
+          user_class = SpecTypes::User
+          Class.new(SpecTypes::BaseObject) do
+            graphql_name 'Query'
+            field :user, user_class, null: true
+          end
+        end
+
+        it 'raises an error' do
+          query_class = query
+
+          offending_class = Class.new(SpecTypes::BaseObject) do
+            graphql_name 'Manager'
+            implements SpecTypes::User
+
+            field :id, 'ID', null: false
+          end
+
+          schema = Class.new(base_schema) do
+            query query_class
+            orphan_types SpecTypes::AdminType, offending_class
+          end
+
+          expect { schema.to_definition }.to raise_error(
+            'Interface User is not valid. Types `Manager` do not have a @key directive. ' \
+            'All types that implement an interface with a @key directive must also have a @key directive.',
+          )
+        end
+      end
+
       context 'when a Query object is provided' do
         let(:query) do
           user_class = SpecTypes::User