Merge pull request #97 from NeedleInAJayStack/fix/interface-ordering

d-exclaimation · web-flow · commit 955e54977edc · 2023-02-03T18:21:20.000+13:00
Fix: interface and union ordering
diff --git a/Sources/Graphiti/Component/Component.swift b/Sources/Graphiti/Component/Component.swift
@@ -3,9 +3,11 @@ import GraphQL
 open class Component<Resolver, Context> {
     let name: String
     var description: String?
+    var componentType: ComponentType
 
-    init(name: String) {
+    init(name: String, type: ComponentType) {
         self.name = name
+        componentType = type
     }
 
     func update(typeProvider _: SchemaTypeProvider, coders _: Coders) throws {}
@@ -18,3 +20,41 @@ public extension Component {
         return self
     }
 }
+
+/// The type of a component. This is used as opposed to runtime type-checking because the
+/// component types are typically generics (and therefore hard to type-check).
+enum ComponentType {
+    case none
+    case connection
+    case `enum`
+    case input
+    case interface
+    case mutation
+    case query
+    case scalar
+    case subscription
+    case type
+    case types
+    case union
+
+    /// An index used to sort components into the correct schema build order. This order goes
+    /// from "least other type references" to "most". By building in this order we are able to satisfy
+    /// hard ordering requirements (interfaces MUST be built before inheriting types), as well as
+    /// reduce unnecessary TypeReferences.
+    var buildOrder: Int {
+        switch self {
+        case .none: return 0
+        case .scalar: return 1
+        case .enum: return 2
+        case .interface: return 3
+        case .input: return 4
+        case .type: return 5
+        case .types: return 6
+        case .union: return 7
+        case .connection: return 8
+        case .query: return 9
+        case .mutation: return 10
+        case .subscription: return 11
+        }
+    }
+}
diff --git a/Sources/Graphiti/Connection/ConnectionType.swift b/Sources/Graphiti/Connection/ConnectionType.swift
@@ -22,7 +22,7 @@ public final class ConnectionType<
 
         let edge = Type<Resolver, Context, Edge<ObjectType>>(
             Edge<ObjectType>.self,
-            as: name+"Edge"
+            as: name + "Edge"
         ) {
             Field("node", at: \.node)
             Field("cursor", at: \.cursor)
@@ -32,7 +32,7 @@ public final class ConnectionType<
 
         let connection = Type<Resolver, Context, Connection<ObjectType>>(
             Connection<ObjectType>.self,
-            as: name+"Connection"
+            as: name + "Connection"
         ) {
             Field("edges", at: \.edges)
             Field("pageInfo", at: \.pageInfo)
@@ -45,7 +45,10 @@ public final class ConnectionType<
         type _: ObjectType.Type,
         name: String?
     ) {
-        super.init(name: name ?? Reflection.name(for: ObjectType.self))
+        super.init(
+            name: name ?? Reflection.name(for: ObjectType.self),
+            type: .connection
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Enum/Enum.swift b/Sources/Graphiti/Enum/Enum.swift
@@ -23,8 +23,7 @@ public final class Enum<
             }
         )
 
-        try typeProvider.map(EnumType.self, to: enumType)
-        typeProvider.types.append(enumType)
+        try typeProvider.add(type: EnumType.self, as: enumType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -37,7 +36,10 @@ public final class Enum<
         values: [Value<EnumType>]
     ) {
         self.values = values
-        super.init(name: name ?? Reflection.name(for: EnumType.self))
+        super.init(
+            name: name ?? Reflection.name(for: EnumType.self),
+            type: .enum
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Input/Input.swift b/Sources/Graphiti/Input/Input.swift
@@ -17,8 +17,7 @@ public final class Input<
             fields: fields(typeProvider: typeProvider)
         )
 
-        try typeProvider.map(InputObjectType.self, to: inputObjectType)
-        typeProvider.types.append(inputObjectType)
+        try typeProvider.add(type: InputObjectType.self, as: inputObjectType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -42,7 +41,10 @@ public final class Input<
         fields: [InputFieldComponent<InputObjectType, Context>]
     ) {
         self.fields = fields
-        super.init(name: name ?? Reflection.name(for: InputObjectType.self))
+        super.init(
+            name: name ?? Reflection.name(for: InputObjectType.self),
+            type: .connection
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Interface/Interface.swift b/Sources/Graphiti/Interface/Interface.swift
@@ -14,8 +14,7 @@ public final class Interface<Resolver, Context, InterfaceType>: TypeComponent<
             resolveType: nil
         )
 
-        try typeProvider.map(InterfaceType.self, to: interfaceType)
-        typeProvider.types.append(interfaceType)
+        try typeProvider.add(type: InterfaceType.self, as: interfaceType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -39,7 +38,10 @@ public final class Interface<Resolver, Context, InterfaceType>: TypeComponent<
         fields: [FieldComponent<InterfaceType, Context>]
     ) {
         self.fields = fields
-        super.init(name: name ?? Reflection.name(for: InterfaceType.self))
+        super.init(
+            name: name ?? Reflection.name(for: InterfaceType.self),
+            type: .interface
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Mutation/Mutation.swift b/Sources/Graphiti/Mutation/Mutation.swift
@@ -32,7 +32,10 @@ public final class Mutation<Resolver, Context>: Component<Resolver, Context> {
         fields: [FieldComponent<Resolver, Context>]
     ) {
         self.fields = fields
-        super.init(name: name)
+        super.init(
+            name: name,
+            type: .mutation
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Query/Query.swift b/Sources/Graphiti/Query/Query.swift
@@ -32,7 +32,10 @@ public final class Query<Resolver, Context>: Component<Resolver, Context> {
         fields: [FieldComponent<Resolver, Context>]
     ) {
         self.fields = fields
-        super.init(name: name)
+        super.init(
+            name: name,
+            type: .query
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Scalar/Scalar.swift b/Sources/Graphiti/Scalar/Scalar.swift
@@ -33,8 +33,7 @@ open class Scalar<Resolver, Context, ScalarType: Codable>: TypeComponent<Resolve
             }
         )
 
-        try typeProvider.map(ScalarType.self, to: scalarType)
-        typeProvider.types.append(scalarType)
+        try typeProvider.add(type: ScalarType.self, as: scalarType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -53,7 +52,10 @@ open class Scalar<Resolver, Context, ScalarType: Codable>: TypeComponent<Resolve
         type _: ScalarType.Type,
         name: String?
     ) {
-        super.init(name: name ?? Reflection.name(for: ScalarType.self))
+        super.init(
+            name: name ?? Reflection.name(for: ScalarType.self),
+            type: .scalar
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Schema/Schema.swift b/Sources/Graphiti/Schema/Schema.swift
@@ -15,8 +15,11 @@ public final class Schema<Resolver, Context> {
             try component.setGraphQLName(typeProvider: typeProvider)
         }
 
-        // Then build up GraphQLTypes
-        for component in components {
+        // Order component by componentType build order
+        let sortedComponents = components.sorted {
+            $0.componentType.buildOrder <= $1.componentType.buildOrder
+        }
+        for component in sortedComponents {
             try component.update(typeProvider: typeProvider, coders: coders)
         }
 
diff --git a/Sources/Graphiti/Schema/SchemaTypeProvider.swift b/Sources/Graphiti/Schema/SchemaTypeProvider.swift
@@ -20,4 +20,9 @@ final class SchemaTypeProvider: TypeProvider {
     var subscription: GraphQLObjectType?
     var types: [GraphQLNamedType] = []
     var directives: [GraphQLDirective] = []
+
+    func add(type: Any.Type, as graphQLType: GraphQLNamedType) throws {
+        try map(type, to: graphQLType)
+        types.append(graphQLType)
+    }
 }
diff --git a/Sources/Graphiti/Subscription/Subscription.swift b/Sources/Graphiti/Subscription/Subscription.swift
@@ -32,7 +32,10 @@ public final class Subscription<Resolver, Context>: Component<Resolver, Context>
         fields: [FieldComponent<Resolver, Context>]
     ) {
         self.fields = fields
-        super.init(name: name)
+        super.init(
+            name: name,
+            type: .subscription
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Type/Type.swift b/Sources/Graphiti/Type/Type.swift
@@ -22,8 +22,7 @@ public final class Type<Resolver, Context, ObjectType: Encodable>: TypeComponent
             isTypeOf: isTypeOf
         )
 
-        try typeProvider.map(ObjectType.self, to: objectType)
-        typeProvider.types.append(objectType)
+        try typeProvider.add(type: ObjectType.self, as: objectType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -49,7 +48,10 @@ public final class Type<Resolver, Context, ObjectType: Encodable>: TypeComponent
     ) {
         self.interfaces = interfaces
         self.fields = fields
-        super.init(name: name ?? Reflection.name(for: ObjectType.self))
+        super.init(
+            name: name ?? Reflection.name(for: ObjectType.self),
+            type: .type
+        )
     }
 }
 
diff --git a/Sources/Graphiti/Types/Types.swift b/Sources/Graphiti/Types/Types.swift
@@ -12,7 +12,10 @@ public final class Types<Resolver, Context>: Component<Resolver, Context> {
 
     init(types: [Any.Type]) {
         self.types = types
-        super.init(name: "")
+        super.init(
+            name: "",
+            type: .types
+        )
     }
 
     public convenience init(_ types: Any.Type...) {
diff --git a/Sources/Graphiti/Union/Union.swift b/Sources/Graphiti/Union/Union.swift
@@ -13,7 +13,7 @@ public final class Union<Resolver, Context, UnionType>: TypeComponent<Resolver,
             }
         )
 
-        try typeProvider.map(UnionType.self, to: unionType)
+        try typeProvider.add(type: UnionType.self, as: unionType)
     }
 
     override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {
@@ -26,7 +26,10 @@ public final class Union<Resolver, Context, UnionType>: TypeComponent<Resolver,
         members: [Any.Type]
     ) {
         self.members = members
-        super.init(name: name ?? Reflection.name(for: UnionType.self))
+        super.init(
+            name: name ?? Reflection.name(for: UnionType.self),
+            type: .union
+        )
     }
 }
 
diff --git a/Tests/GraphitiTests/PartialSchemaTests.swift b/Tests/GraphitiTests/PartialSchemaTests.swift
@@ -164,4 +164,37 @@ class PartialSchemaTests: XCTestCase {
             ])
         )
     }
+
+    func testPartialSchemaOutOfOrder() throws {
+        let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
+
+        /// Double check if ordering of partial schema doesn't matter
+        let schema = try Schema.create(from: [SearchSchema(), BaseSchema()])
+
+        struct PartialSchemaTestAPI: API {
+            let resolver: StarWarsResolver
+            let schema: Schema<StarWarsResolver, StarWarsContext>
+        }
+
+        let api = PartialSchemaTestAPI(resolver: StarWarsResolver(), schema: schema)
+
+        XCTAssertEqual(
+            try api.execute(
+                request: """
+                query {
+                    human(id: "1000") {
+                        name
+                    }
+                }
+                """,
+                context: StarWarsContext(),
+                on: group
+            ).wait(),
+            GraphQLResult(data: [
+                "human": [
+                    "name": "Luke Skywalker",
+                ],
+            ])
+        )
+    }
 }
diff --git a/Tests/GraphitiTests/SchemaBuilderTests.swift b/Tests/GraphitiTests/SchemaBuilderTests.swift
@@ -42,20 +42,7 @@ class SchemaBuilderTests: XCTestCase {
             }.description("One of the films in the Star Wars Trilogy.")
         }
         builder.add {
-            Interface(Character.self) {
-                Field("id", at: \.id)
-                    .description("The id of the character.")
-                Field("name", at: \.name)
-                    .description("The name of the character.")
-                Field("friends", at: \.friends, as: [TypeReference<Character>].self)
-                    .description(
-                        "The friends of the character, or an empty list if they have none."
-                    )
-                Field("appearsIn", at: \.appearsIn)
-                    .description("Which movies they appear in.")
-                Field("secretBackstory", at: \.secretBackstory)
-                    .description("All secrets about their past.")
-            }
+            Union(SearchResult.self, members: Planet.self, Human.self, Droid.self)
         }.add {
             Type(Human.self, interfaces: [Character.self]) {
                 Field("id", at: \.id)
@@ -78,7 +65,20 @@ class SchemaBuilderTests: XCTestCase {
                     .description("Where are they from and how they came to be who they are.")
             }.description("A mechanical creature in the Star Wars universe.")
         }.add {
-            Union(SearchResult.self, members: Planet.self, Human.self, Droid.self)
+            Interface(Character.self) {
+                Field("id", at: \.id)
+                    .description("The id of the character.")
+                Field("name", at: \.name)
+                    .description("The name of the character.")
+                Field("friends", at: \.friends, as: [TypeReference<Character>].self)
+                    .description(
+                        "The friends of the character, or an empty list if they have none."
+                    )
+                Field("appearsIn", at: \.appearsIn)
+                    .description("Which movies they appear in.")
+                Field("secretBackstory", at: \.secretBackstory)
+                    .description("All secrets about their past.")
+            }
         }
         builder.addQuery {
             Field("human", at: StarWarsResolver.human) {
@@ -122,7 +122,7 @@ class SchemaBuilderTests: XCTestCase {
                 ],
             ])
         )
-        
+
         XCTAssert(
             api.schema.schema.typeMap.contains(where: { key, _ in
                 key == "PlanetConnection"

Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,7 @@ public final class Enum<`
`23`	`23`	`}`
`24`	`24`	`)`
`25`	`25`
`26`		`- try typeProvider.map(EnumType.self, to: enumType)`
`27`		`- typeProvider.types.append(enumType)`
	`26`	`+ try typeProvider.add(type: EnumType.self, as: enumType)`
`28`	`27`	`}`
`29`	`28`
`30`	`29`	`override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {`
`@@ -37,7 +36,10 @@ public final class Enum<`
`37`	`36`	`values: [Value<EnumType>]`
`38`	`37`	`) {`
`39`	`38`	`self.values = values`
`40`		`- super.init(name: name ?? Reflection.name(for: EnumType.self))`
	`39`	`+ super.init(`
	`40`	`+ name: name ?? Reflection.name(for: EnumType.self),`
	`41`	`+ type: .enum`
	`42`	`+ )`
`41`	`43`	`}`
`42`	`44`	`}`
`43`	`45`
Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,7 @@ public final class Input<`
`17`	`17`	`fields: fields(typeProvider: typeProvider)`
`18`	`18`	`)`
`19`	`19`
`20`		`- try typeProvider.map(InputObjectType.self, to: inputObjectType)`
`21`		`- typeProvider.types.append(inputObjectType)`
	`20`	`+ try typeProvider.add(type: InputObjectType.self, as: inputObjectType)`
`22`	`21`	`}`
`23`	`22`
`24`	`23`	`override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {`
`@@ -42,7 +41,10 @@ public final class Input<`
`42`	`41`	`fields: [InputFieldComponent<InputObjectType, Context>]`
`43`	`42`	`) {`
`44`	`43`	`self.fields = fields`
`45`		`- super.init(name: name ?? Reflection.name(for: InputObjectType.self))`
	`44`	`+ super.init(`
	`45`	`+ name: name ?? Reflection.name(for: InputObjectType.self),`
	`46`	`+ type: .connection`
	`47`	`+ )`
`46`	`48`	`}`
`47`	`49`	`}`
`48`	`50`
Original file line number	Diff line number	Diff line change
`@@ -14,8 +14,7 @@ public final class Interface<Resolver, Context, InterfaceType>: TypeComponent<`
`14`	`14`	`resolveType: nil`
`15`	`15`	`)`
`16`	`16`
`17`		`- try typeProvider.map(InterfaceType.self, to: interfaceType)`
`18`		`- typeProvider.types.append(interfaceType)`
	`17`	`+ try typeProvider.add(type: InterfaceType.self, as: interfaceType)`
`19`	`18`	`}`
`20`	`19`
`21`	`20`	`override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {`
`@@ -39,7 +38,10 @@ public final class Interface<Resolver, Context, InterfaceType>: TypeComponent<`
`39`	`38`	`fields: [FieldComponent<InterfaceType, Context>]`
`40`	`39`	`) {`
`41`	`40`	`self.fields = fields`
`42`		`- super.init(name: name ?? Reflection.name(for: InterfaceType.self))`
	`41`	`+ super.init(`
	`42`	`+ name: name ?? Reflection.name(for: InterfaceType.self),`
	`43`	`+ type: .interface`
	`44`	`+ )`
`43`	`45`	`}`
`44`	`46`	`}`
`45`	`47`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,10 @@ public final class Mutation<Resolver, Context>: Component<Resolver, Context> {`
`32`	`32`	`fields: [FieldComponent<Resolver, Context>]`
`33`	`33`	`) {`
`34`	`34`	`self.fields = fields`
`35`		`- super.init(name: name)`
	`35`	`+ super.init(`
	`36`	`+ name: name,`
	`37`	`+ type: .mutation`
	`38`	`+ )`
`36`	`39`	`}`
`37`	`40`	`}`
`38`	`41`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,10 @@ public final class Query<Resolver, Context>: Component<Resolver, Context> {`
`32`	`32`	`fields: [FieldComponent<Resolver, Context>]`
`33`	`33`	`) {`
`34`	`34`	`self.fields = fields`
`35`		`- super.init(name: name)`
	`35`	`+ super.init(`
	`36`	`+ name: name,`
	`37`	`+ type: .query`
	`38`	`+ )`
`36`	`39`	`}`
`37`	`40`	`}`
`38`	`41`
Original file line number	Diff line number	Diff line change
`@@ -33,8 +33,7 @@ open class Scalar<Resolver, Context, ScalarType: Codable>: TypeComponent<Resolve`
`33`	`33`	`}`
`34`	`34`	`)`
`35`	`35`
`36`		`- try typeProvider.map(ScalarType.self, to: scalarType)`
`37`		`- typeProvider.types.append(scalarType)`
	`36`	`+ try typeProvider.add(type: ScalarType.self, as: scalarType)`
`38`	`37`	`}`
`39`	`38`
`40`	`39`	`override func setGraphQLName(typeProvider: SchemaTypeProvider) throws {`
`@@ -53,7 +52,10 @@ open class Scalar<Resolver, Context, ScalarType: Codable>: TypeComponent<Resolve`
`53`	`52`	`type _: ScalarType.Type,`
`54`	`53`	`name: String?`
`55`	`54`	`) {`
`56`		`- super.init(name: name ?? Reflection.name(for: ScalarType.self))`
	`55`	`+ super.init(`
	`56`	`+ name: name ?? Reflection.name(for: ScalarType.self),`
	`57`	`+ type: .scalar`
	`58`	`+ )`
`57`	`59`	`}`
`58`	`60`	`}`
`59`	`61`
Original file line number	Diff line number	Diff line change
`@@ -15,8 +15,11 @@ public final class Schema<Resolver, Context> {`
`15`	`15`	`try component.setGraphQLName(typeProvider: typeProvider)`
`16`	`16`	`}`
`17`	`17`
`18`		`- // Then build up GraphQLTypes`
`19`		`- for component in components {`
	`18`	`+ // Order component by componentType build order`
	`19`	`+ let sortedComponents = components.sorted {`
	`20`	`+ $0.componentType.buildOrder <= $1.componentType.buildOrder`
	`21`	`+ }`
	`22`	`+ for component in sortedComponents {`
`20`	`23`	`try component.update(typeProvider: typeProvider, coders: coders)`
`21`	`24`	`}`
`22`	`25`