diff --git a/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsIncludeSkipQuery.graphql.swift b/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsIncludeSkipQuery.graphql.swift index 8bd3d1cd7e..b8e9d947af 100644 --- a/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsIncludeSkipQuery.graphql.swift +++ b/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsIncludeSkipQuery.graphql.swift @@ -692,7 +692,6 @@ public class AllAnimalsIncludeSkipQuery: GraphQLQuery { ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.self), ObjectIdentifier(WarmBloodedDetails.self), ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self), - ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.AsWarmBlooded.AsWarmBlooded.self), ObjectIdentifier(HeightInMeters.self) ] )) @@ -853,7 +852,6 @@ public class AllAnimalsIncludeSkipQuery: GraphQLQuery { ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.self), ObjectIdentifier(WarmBloodedDetails.self), ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self), - ObjectIdentifier(AllAnimalsIncludeSkipQuery.Data.AllAnimal.AsPet.AsWarmBlooded.AsWarmBlooded.self), ObjectIdentifier(HeightInMeters.self) ] )) diff --git a/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsQuery.graphql.swift b/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsQuery.graphql.swift index 61c5b7181b..cd6cbaabed 100644 --- a/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsQuery.graphql.swift +++ b/Sources/AnimalKingdomAPI/AnimalKingdomAPI/Sources/Operations/Queries/AllAnimalsQuery.graphql.swift @@ -625,8 +625,7 @@ public class AllAnimalsQuery: GraphQLQuery { ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsWarmBlooded.self), ObjectIdentifier(PetDetails.self), ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.AsWarmBlooded.self) + ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self) ] )) } @@ -822,8 +821,7 @@ public class AllAnimalsQuery: GraphQLQuery { ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsWarmBlooded.self), ObjectIdentifier(PetDetails.self), ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.AsWarmBlooded.self) + ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self) ] )) } @@ -936,8 +934,7 @@ public class AllAnimalsQuery: GraphQLQuery { ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsWarmBlooded.self), ObjectIdentifier(PetDetails.self), ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self), - ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.AsWarmBlooded.self) + ObjectIdentifier(AllAnimalsQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self) ] )) } diff --git a/Sources/ApolloCodegenLib/Templates/SelectionSetTemplate.swift b/Sources/ApolloCodegenLib/Templates/SelectionSetTemplate.swift index 6c98479fe1..435f441373 100644 --- a/Sources/ApolloCodegenLib/Templates/SelectionSetTemplate.swift +++ b/Sources/ApolloCodegenLib/Templates/SelectionSetTemplate.swift @@ -647,6 +647,14 @@ fileprivate extension IR.MergedSelections.MergedSource { nodesToSharedRoot += 1 } + /// If the shared root is the root of the definition, we should just generate the fully + /// qualified name. + if sourceTypePathCurrentNode.isHead { + return SelectionSetNameGenerator.generatedSelectionSetName( + for: self, format: .fullyQualified, pluralizer: pluralizer + ) + } + let sharedRootIndex = typeInfo.entity.location.fieldPath!.count - (nodesToSharedRoot + 1) @@ -657,11 +665,7 @@ fileprivate extension IR.MergedSelections.MergedSource { /// /// Example: The `height` field on `AllAnimals.AsPet` can reference the `AllAnimals.Height` /// object as just `Height`. - /// - /// However, if the shared root is the root of the definition, the component that would be - /// removed is the location's `source`. This is not included in the field path and is already - /// omitted by this function. In this case, the `sharedRootIndex` is `-1`. - let removeFirstComponent = nodesToSharedRoot <= 1 && !(sharedRootIndex < 0) + let removeFirstComponent = nodesToSharedRoot <= 1 let fieldPath = typeInfo.entity.location.fieldPath!.node( at: max(0, sharedRootIndex) @@ -728,7 +732,7 @@ fileprivate extension IR.MergedSelections.MergedSource { mergedFragmentEntityConditionPathNode = node } selectionSetNameComponents.append( - SelectionSetNameGenerator.ConditionPath.path(for: node) + node.value.selectionSetNameComponent ) fulfilledFragments.append(selectionSetNameComponents.joined(separator: ".")) } diff --git a/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift b/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift index b2d16d5faf..d475c7cd70 100644 --- a/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift +++ b/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift @@ -3667,7 +3667,7 @@ class SelectionSetTemplateTests: XCTestCase { expect(actual).to(equalLineByLine(expected, atLine: 12, ignoringExtraLines: true)) } - func test__render_fieldAccessors__givenEntityFieldMergedFromParent_atOperationRoot_rendersFieldAccessorWithNameNotIncludingParent() throws { + func test__render_fieldAccessors__givenEntityFieldMergedFromParent_atOperationRoot_rendersFieldAccessorWithFullyQualifiedName() throws { // given schemaSDL = """ type Query { @@ -3696,7 +3696,7 @@ class SelectionSetTemplateTests: XCTestCase { let expected = """ public var name: String { __data["name"] } - public var allAnimals: [AllAnimal]? { __data["allAnimals"] } + public var allAnimals: [TestOperationQuery.Data.AllAnimal]? { __data["allAnimals"] } """ // when @@ -3767,7 +3767,7 @@ class SelectionSetTemplateTests: XCTestCase { expect(actual).to(equalLineByLine(expected, atLine: 12, ignoringExtraLines: true)) } - func test__render_fieldAccessors__givenEntityFieldMergedFromSiblingTypeCase_atOperationRoot_rendersFieldAccessorWithNotIncludingSharedParent() throws { + func test__render_fieldAccessors__givenEntityFieldMergedFromSiblingTypeCase_atOperationRoot_rendersFieldAccessorWithFullyQualifiedName() throws { // given schemaSDL = """ type Query { @@ -3803,7 +3803,7 @@ class SelectionSetTemplateTests: XCTestCase { let expected = """ public var name: String { __data["name"] } - public var allAnimals: [AsModeratorQuery.AllAnimal]? { __data["allAnimals"] } + public var allAnimals: [TestOperationQuery.Data.AsModeratorQuery.AllAnimal]? { __data["allAnimals"] } """ // when diff --git a/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplate_Initializers_Tests.swift b/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplate_Initializers_Tests.swift index cba5b7db9d..76ea47b284 100644 --- a/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplate_Initializers_Tests.swift +++ b/Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplate_Initializers_Tests.swift @@ -477,6 +477,81 @@ class SelectionSetTemplate_Initializers_Tests: XCTestCase { // then expect(actual).to(equalLineByLine(expected, atLine: 16, ignoringExtraLines: true)) } + + func test__render_givenNestedTypeCasesMergedFromSibling_fulfilledFragmentsIncludesAllTypeCasesInScope() throws { + // given + schemaSDL = """ + type Query { + allAnimals: [Animal!] + } + + interface Animal { + species: String! + } + + interface Pet { + species: String! + } + + interface WarmBlooded { + species: String! + } + + type Cat implements Animal & Pet & WarmBlooded { + species: String! + isJellicle: Boolean! + } + """ + + document = """ + query TestOperation { + allAnimals { + ... on Pet { + ... on WarmBlooded { + species + } + } + ... on Cat { + isJellicle + } + } + } + """ + + let expected = + """ + public init( + isJellicle: Bool, + species: String + ) { + self.init(_dataDict: DataDict( + data: [ + "__typename": TestSchema.Objects.Cat.typename, + "isJellicle": isJellicle, + "species": species, + ], + fulfilledFragments: [ + ObjectIdentifier(TestOperationQuery.Data.AllAnimal.self), + ObjectIdentifier(TestOperationQuery.Data.AllAnimal.AsCat.self), + ObjectIdentifier(TestOperationQuery.Data.AllAnimal.AsPet.self), + ObjectIdentifier(TestOperationQuery.Data.AllAnimal.AsPet.AsWarmBlooded.self) + ] + )) + } + """ + + // when + try buildSubjectAndOperation() + + let allAnimals_asCat = try XCTUnwrap( + operation[field: "query"]?[field: "allAnimals"]?[as: "Cat"] + ) + + let actual = subject.render(inlineFragment: allAnimals_asCat) + + // then + expect(actual).to(equalLineByLine(expected, atLine: 17, ignoringExtraLines: true)) + } // MARK: Selection Tests diff --git a/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/README.MD b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/README.MD new file mode 100644 index 0000000000..c743b3e68b --- /dev/null +++ b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/README.MD @@ -0,0 +1,16 @@ +# Overview + +When merging a selection set with a nested child selection set into a type case that does not also select additional fields on that child selection set, we use a direct reference to the generated selection set in the defining entity selection set. This directly merged child selection set is referenced by its name relative to the type case. + +To calculate the relative name, we determine where the shared root between the definition of the merged child and the target it is being merged into was, and generated the name as only up to their shared root. When the shared root of the directly merged child is the root of the operation, this causes a naming ambiguity problem and a compliation error. + +We must use the fully qualified name in this situation. + +In the example for this test, the `innerChild` field on `AsEventA.Child.AsChildA` would previously have the +type `Child.AsChildA.InnerChild`, which does not exist, because the first component (`Child`) was inferred to be the `AsEventA.Child`. The intention was to point to the `TestFragment.Child.AsChildA.InnerChild`. + +## Reference Issue: https://github.com/apollographql/apollo-ios/pull/3168 + +## Solution + +When the shared root for a directly merged source is the definition root, use the fully qualified selection set name. diff --git a/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/operation.graphql b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/operation.graphql new file mode 100644 index 0000000000..03ccdac483 --- /dev/null +++ b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/operation.graphql @@ -0,0 +1,15 @@ +fragment TestFragment on Event { + child { + ... on ChildA { + innerChild: child { + foo + } + } + } + ... on EventA { + bar + } + ... on EventB { + baz + } +} diff --git a/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/schema.graphqls b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/schema.graphqls new file mode 100644 index 0000000000..6c80f9329b --- /dev/null +++ b/Tests/CodegenIntegrationTests/Tests/3168-mergedSourceFromDefinitionRootNeedsFullyQualifedName/schema.graphqls @@ -0,0 +1,27 @@ +type Query { + test: Event! +} + +interface Event { + child: Child! +} + +type EventA implements Event { + child: Child! + bar: String! +} + +type EventB implements Event { + child: Child! + baz: String! +} + +interface Child { + foo: String! + child: Child +} + +type ChildA implements Child { + foo: String! + child: Child +}