From 1b08e74ddec7fc47f350c68fafb2972b5d1f170c Mon Sep 17 00:00:00 2001 From: Zach FettersMoore <4425109+BobaFetters@users.noreply.github.com> Date: Wed, 24 May 2023 11:20:00 -0400 Subject: [PATCH 1/3] Adding more test cases Adding more tests cases for type name conflicts during selection set generation --- .../ApolloCodegenTests.swift | 488 ++++++++++++++++++ 1 file changed, 488 insertions(+) diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 131abe8e6c..d5f6e9bccd 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2702,6 +2702,494 @@ class ApolloCodegenTests: XCTestCase { expect(containingObject).to(equal("ContainerFields")) }) } + + // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from + // a type higher in the selection set hierarchy which will likely cause problems during runtime + func test__validation__selectionSet_typeConflicts_withNamedFragmentCollision_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [Container] + } + + type Container { + value: Value + values: [Value] + user: Int + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + value { + propertyA + propertyB + propertyC + propertyD + } + + ...value + } + } + } + + fragment value on Container { + user + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } +// expect(name).to(equal("values")) +// expect(conflictingName).to(equal("value")) +// expect(containingObject).to(equal("ConflictingQuery")) + }) + } + + // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from + // a type higher in the selection set hierarchy which will likely cause problems during runtime + func test__validation__selectionSet_typeConflicts_withNamedFragmentCollisionWithinInlineFragment_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [ContainerInterface] + } + + interface ContainerInterface { + value: Value + } + + type Container implements ContainerInterface{ + value: Value + values: [Value] + user: Int + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + value { + propertyA + propertyB + propertyC + propertyD + } + ... on Container { + ...value + } + } + } + } + + fragment value on Container { + user + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } +// expect(name).to(equal("values")) +// expect(conflictingName).to(equal("value")) +// expect(containingObject).to(equal("ConflictingQuery")) + }) + } + + func test__validation__selectionSet_typeConflicts_withNamedFragmentFieldCollisionWithinInlineFragment_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [ContainerInterface] + } + + interface ContainerInterface { + value: Value + } + + type Container implements ContainerInterface{ + value: Value + values: [Value] + user: Int + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + value { + propertyA + propertyB + propertyC + propertyD + } + ... on Container { + ...ValueFragment + } + } + } + } + + fragment ValueFragment on Container { + values { + propertyA + propertyC + } + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } + expect(name).to(equal("value")) + expect(conflictingName).to(equal("values")) + expect(containingObject).to(equal("ConflictingQuery")) + }) + } + + // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from + // a type higher in the selection set hierarchy which will likely cause problems during runtime + func test__validation__selectionSet_typeConflicts_withNestedTypeFieldCollision_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [ContainerInterface] + } + + interface ContainerInterface { + value: Value + } + + type Container implements ContainerInterface{ + nestedContainer: NestedContainer + value: Value + values: [Value] + user: Int + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + + type NestedContainer { + values: [Value] + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + value { + propertyA + propertyB + propertyC + propertyD + } + ... on Container { + nestedContainer { + values { + propertyA + propertyC + } + } + } + } + } + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } +// expect(name).to(equal("values")) +// expect(conflictingName).to(equal("value")) +// expect(containingObject).to(equal("ConflictingQuery")) + }) + } + + func test__validation__selectionSet_typeConflicts_withNamedFragmentWithinInlineFragmentTypeCollision_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [ContainerInterface] + } + + interface ContainerInterface { + value: Value + } + + type Container implements ContainerInterface{ + nestedContainer: NestedContainer + value: Value + values: [Value] + user: Int + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + + type NestedContainer { + values: [Value] + description: String + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + value { + propertyA + propertyB + propertyC + propertyD + } + ... on Container { + nestedContainer { + ...value + } + } + } + } + } + + fragment value on NestedContainer { + description + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } +// expect(name).to(equal("values")) +// expect(conflictingName).to(equal("value")) +// expect(containingObject).to(equal("ConflictingQuery")) + }) + } + + func test__validation__selectionSet_typeConflicts_withFieldUsingNamedFragmentCollision_shouldThrowError() throws { + let schemaDefData: Data = { + """ + type Query { + user: User + } + + type User { + containers: [Container] + } + + type Container { + info: Value + } + + type Value { + propertyA: String! + propertyB: String! + propertyC: String! + propertyD: String! + } + """ + }().data(using: .utf8)! + + let operationData: Data = + """ + query ConflictingQuery { + user { + containers { + info { + ...Info + } + } + } + } + + fragment Info on Value { + propertyA + propertyB + propertyD + } + """.data(using: .utf8)! + + try createFile(containing: schemaDefData, named: "schema.graphqls") + try createFile(containing: operationData, named: "operation.graphql") + + let config = ApolloCodegenConfiguration.mock( + input: .init( + schemaSearchPaths: ["schema*.graphqls"], + operationSearchPaths: ["*.graphql"] + ), + output: .init( + schemaTypes: .init(path: "SchemaModule", + moduleType: .swiftPackageManager), + operations: .inSchemaModule + ) + ) + + expect(try ApolloCodegen.build(with: config)) + .to(throwError { error in + guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { + fail("Expected .typeNameConflict, got .\(error)") + return + } +// expect(name).to(equal("values")) +// expect(conflictingName).to(equal("value")) +// expect(containingObject).to(equal("ConflictingQuery")) + }) + } // MARK: Path Match Exclusion Tests From 3e381a30f927c40f2558f4a4300b0c13d083cb69 Mon Sep 17 00:00:00 2001 From: Zach FettersMoore <4425109+BobaFetters@users.noreply.github.com> Date: Thu, 25 May 2023 14:47:26 -0400 Subject: [PATCH 2/3] Updated type conflict algorithm and test cases - Finalize test cases - Update type conflict algorithm to cover test cases --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 49 +++- .../ApolloCodegenTests.swift | 259 +----------------- 2 files changed, 48 insertions(+), 260 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index ab11e28d9e..1287053adf 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -222,24 +222,54 @@ public class ApolloCodegen { } /// Validates that there are no type conflicts within a SelectionSet - static private func validateTypeConflicts(for selectionSet: IR.SelectionSet, with context: ConfigurationContext, in containingObject: String) throws { + static private func validateTypeConflicts(for selectionSet: IR.SelectionSet, + with context: ConfigurationContext, + in containingObject: String, + including parentTypes: [String: String] = [:]) throws { // Check for type conflicts resulting from singularization/pluralization of fields - var fieldNamesByFormattedTypeName = [String: String]() + var typeNamesByFormattedTypeName = [String: String]() + var fields: [IR.EntityField] = selectionSet.selections.direct?.fields.values.compactMap { $0 as? IR.EntityField } ?? [] fields.append(contentsOf: selectionSet.selections.merged.fields.values.compactMap { $0 as? IR.EntityField } ) try fields.forEach { field in let formattedTypeName = field.formattedSelectionSetName(with: context.pluralizer) - if let existingFieldName = fieldNamesByFormattedTypeName[formattedTypeName] { + if let existingFieldName = typeNamesByFormattedTypeName[formattedTypeName] { throw Error.typeNameConflict( name: existingFieldName, conflictingName: field.name, containingObject: containingObject ) } - - fieldNamesByFormattedTypeName[formattedTypeName] = field.name - try validateTypeConflicts(for: field.selectionSet, with: context, in: containingObject) + typeNamesByFormattedTypeName[formattedTypeName] = field.name + } + + // Combine `parentTypes` and `typeNamesByFormattedTypeName` to check against fragment names and + // pass into recursive function calls + var combinedTypeNames = parentTypes + combinedTypeNames.merge(typeNamesByFormattedTypeName) { (current, _) in current } + + // passing each fields selection set for validation after we have fully built our `typeNamesByFormattedTypeName` dictionary + try fields.forEach { field in + try validateTypeConflicts( + for: field.selectionSet, + with: context, + in: containingObject, + including: combinedTypeNames + ) + } + + var fragments: [IR.NamedFragment] = selectionSet.selections.direct?.fragments.values.map { $0.fragment } ?? [] + fragments.append(contentsOf: selectionSet.selections.merged.fragments.values.map { $0.fragment }) + + try fragments.forEach { fragment in + if let existingTypeName = combinedTypeNames[fragment.generatedDefinitionName] { + throw Error.typeNameConflict( + name: existingTypeName, + conflictingName: fragment.name, + containingObject: containingObject + ) + } } // gather nested fragments to loop through and check as well @@ -247,7 +277,12 @@ public class ApolloCodegen { nestedSelectionSets.append(contentsOf: selectionSet.selections.merged.inlineFragments.values) try nestedSelectionSets.forEach { nestedSet in - try validateTypeConflicts(for: nestedSet, with: context, in: containingObject) + try validateTypeConflicts( + for: nestedSet, + with: context, + in: containingObject, + including: combinedTypeNames + ) } } diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index d5f6e9bccd..e131699999 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2703,165 +2703,6 @@ class ApolloCodegenTests: XCTestCase { }) } - // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from - // a type higher in the selection set hierarchy which will likely cause problems during runtime - func test__validation__selectionSet_typeConflicts_withNamedFragmentCollision_shouldThrowError() throws { - let schemaDefData: Data = { - """ - type Query { - user: User - } - - type User { - containers: [Container] - } - - type Container { - value: Value - values: [Value] - user: Int - } - - type Value { - propertyA: String! - propertyB: String! - propertyC: String! - propertyD: String! - } - """ - }().data(using: .utf8)! - - let operationData: Data = - """ - query ConflictingQuery { - user { - containers { - value { - propertyA - propertyB - propertyC - propertyD - } - - ...value - } - } - } - - fragment value on Container { - user - } - """.data(using: .utf8)! - - try createFile(containing: schemaDefData, named: "schema.graphqls") - try createFile(containing: operationData, named: "operation.graphql") - - let config = ApolloCodegenConfiguration.mock( - input: .init( - schemaSearchPaths: ["schema*.graphqls"], - operationSearchPaths: ["*.graphql"] - ), - output: .init( - schemaTypes: .init(path: "SchemaModule", - moduleType: .swiftPackageManager), - operations: .inSchemaModule - ) - ) - - expect(try ApolloCodegen.build(with: config)) - .to(throwError { error in - guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { - fail("Expected .typeNameConflict, got .\(error)") - return - } -// expect(name).to(equal("values")) -// expect(conflictingName).to(equal("value")) -// expect(containingObject).to(equal("ConflictingQuery")) - }) - } - - // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from - // a type higher in the selection set hierarchy which will likely cause problems during runtime - func test__validation__selectionSet_typeConflicts_withNamedFragmentCollisionWithinInlineFragment_shouldThrowError() throws { - let schemaDefData: Data = { - """ - type Query { - user: User - } - - type User { - containers: [ContainerInterface] - } - - interface ContainerInterface { - value: Value - } - - type Container implements ContainerInterface{ - value: Value - values: [Value] - user: Int - } - - type Value { - propertyA: String! - propertyB: String! - propertyC: String! - propertyD: String! - } - """ - }().data(using: .utf8)! - - let operationData: Data = - """ - query ConflictingQuery { - user { - containers { - value { - propertyA - propertyB - propertyC - propertyD - } - ... on Container { - ...value - } - } - } - } - - fragment value on Container { - user - } - """.data(using: .utf8)! - - try createFile(containing: schemaDefData, named: "schema.graphqls") - try createFile(containing: operationData, named: "operation.graphql") - - let config = ApolloCodegenConfiguration.mock( - input: .init( - schemaSearchPaths: ["schema*.graphqls"], - operationSearchPaths: ["*.graphql"] - ), - output: .init( - schemaTypes: .init(path: "SchemaModule", - moduleType: .swiftPackageManager), - operations: .inSchemaModule - ) - ) - - expect(try ApolloCodegen.build(with: config)) - .to(throwError { error in - guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { - fail("Expected .typeNameConflict, got .\(error)") - return - } -// expect(name).to(equal("values")) -// expect(conflictingName).to(equal("value")) -// expect(containingObject).to(equal("ConflictingQuery")) - }) - } - func test__validation__selectionSet_typeConflicts_withNamedFragmentFieldCollisionWithinInlineFragment_shouldThrowError() throws { let schemaDefData: Data = { """ @@ -2945,94 +2786,6 @@ class ApolloCodegenTests: XCTestCase { }) } - // The code gen from this schema doesn't throw a compile error due to multiple types named the same, however the nested type appears to end up referencing the name struct from - // a type higher in the selection set hierarchy which will likely cause problems during runtime - func test__validation__selectionSet_typeConflicts_withNestedTypeFieldCollision_shouldThrowError() throws { - let schemaDefData: Data = { - """ - type Query { - user: User - } - - type User { - containers: [ContainerInterface] - } - - interface ContainerInterface { - value: Value - } - - type Container implements ContainerInterface{ - nestedContainer: NestedContainer - value: Value - values: [Value] - user: Int - } - - type Value { - propertyA: String! - propertyB: String! - propertyC: String! - propertyD: String! - } - - type NestedContainer { - values: [Value] - } - """ - }().data(using: .utf8)! - - let operationData: Data = - """ - query ConflictingQuery { - user { - containers { - value { - propertyA - propertyB - propertyC - propertyD - } - ... on Container { - nestedContainer { - values { - propertyA - propertyC - } - } - } - } - } - } - """.data(using: .utf8)! - - try createFile(containing: schemaDefData, named: "schema.graphqls") - try createFile(containing: operationData, named: "operation.graphql") - - let config = ApolloCodegenConfiguration.mock( - input: .init( - schemaSearchPaths: ["schema*.graphqls"], - operationSearchPaths: ["*.graphql"] - ), - output: .init( - schemaTypes: .init(path: "SchemaModule", - moduleType: .swiftPackageManager), - operations: .inSchemaModule - ) - ) - - expect(try ApolloCodegen.build(with: config)) - .to(throwError { error in - guard case let ApolloCodegen.Error.typeNameConflict(name, conflictingName, containingObject) = error else { - fail("Expected .typeNameConflict, got .\(error)") - return - } -// expect(name).to(equal("values")) -// expect(conflictingName).to(equal("value")) -// expect(containingObject).to(equal("ConflictingQuery")) - }) - } - func test__validation__selectionSet_typeConflicts_withNamedFragmentWithinInlineFragmentTypeCollision_shouldThrowError() throws { let schemaDefData: Data = { """ @@ -3115,9 +2868,9 @@ class ApolloCodegenTests: XCTestCase { fail("Expected .typeNameConflict, got .\(error)") return } -// expect(name).to(equal("values")) -// expect(conflictingName).to(equal("value")) -// expect(containingObject).to(equal("ConflictingQuery")) + expect(name).to(equal("value")) + expect(conflictingName).to(equal("value")) + expect(containingObject).to(equal("ConflictingQuery")) }) } @@ -3185,9 +2938,9 @@ class ApolloCodegenTests: XCTestCase { fail("Expected .typeNameConflict, got .\(error)") return } -// expect(name).to(equal("values")) -// expect(conflictingName).to(equal("value")) -// expect(containingObject).to(equal("ConflictingQuery")) + expect(name).to(equal("info")) + expect(conflictingName).to(equal("Info")) + expect(containingObject).to(equal("ConflictingQuery")) }) } From 853b978c189c1ae07d33afdccf78f8dc789af8f1 Mon Sep 17 00:00:00 2001 From: Zach FettersMoore <4425109+BobaFetters@users.noreply.github.com> Date: Thu, 25 May 2023 16:36:19 -0400 Subject: [PATCH 3/3] Minor formatting fix --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index 1287053adf..ab48f9a916 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -222,10 +222,12 @@ public class ApolloCodegen { } /// Validates that there are no type conflicts within a SelectionSet - static private func validateTypeConflicts(for selectionSet: IR.SelectionSet, - with context: ConfigurationContext, - in containingObject: String, - including parentTypes: [String: String] = [:]) throws { + static private func validateTypeConflicts( + for selectionSet: IR.SelectionSet, + with context: ConfigurationContext, + in containingObject: String, + including parentTypes: [String: String] = [:] + ) throws { // Check for type conflicts resulting from singularization/pluralization of fields var typeNamesByFormattedTypeName = [String: String]()