Skip to content

Commit

Permalink
fix: Type case with only reserved fields is not composite inline frag…
Browse files Browse the repository at this point in the history
…ment (#261)
  • Loading branch information
calvincestari authored and gh-action-runner committed Feb 9, 2024
1 parent d375fa3 commit f911089
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ extension IR.NamedFragment {
inclusionConditions: nil,
givenAllTypesInSchema: .init([type], schemaRootTypes: .mock())
)
]
],
isUserDefined: true
)
let rootEntityField = IR.EntityField.init(
rootField,
Expand Down Expand Up @@ -110,7 +111,8 @@ extension IR.Operation {
forType: .mock(),
inclusionConditions: nil,
givenAllTypesInSchema: .init([], schemaRootTypes: .mock()))
]
],
isUserDefined: true
)
let rootField = IR.EntityField(
.mock(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,6 @@ class FragmentTemplateTests: XCTestCase {
init(_dataDict: DataDict) { __data = _dataDict }
static var __parentType: ApolloAPI.ParentType { TestSchema.Objects.Query }
static var __selections: [ApolloAPI.Selection] { [
] }
}
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,57 @@ class SelectionSetTemplateTests: XCTestCase {
expect(actual).to(equalLineByLine(expected, atLine: 7, ignoringExtraLines: true))
}

// Related to https://github.com/apollographql/apollo-ios/issues/3326
func test__render_selections__givenTypeCaseWithOnlyReservedField_doesNotRenderSelections() async throws {
// given
schemaSDL = """
type Query {
allAnimals: [Animal]
}
union Animal = AnimalObject | AnimalError
type AnimalObject {
species: String!
}
type AnimalError {
code: Int!
}
"""

document = """
query TestOperation {
allAnimals {
... on AnimalObject {
__typename
}
}
}
"""

let expected = """
public struct AsAnimalObject: TestSchema.InlineFragment {
public let __data: DataDict
public init(_dataDict: DataDict) { __data = _dataDict }
public typealias RootEntityType = TestOperationQuery.Data.AllAnimal
public static var __parentType: ApolloAPI.ParentType { TestSchema.Objects.AnimalObject }
}
"""

// when
try await buildSubjectAndOperation()
let allAnimals_asAnimalObject = try XCTUnwrap(
operation[field: "query"]?[field: "allAnimals"]?[as: "AnimalObject"]
)

let actual = subject.test_render(inlineFragment: allAnimals_asAnimalObject.computed)

// then
expect(actual).to(equalLineByLine(expected, atLine: 2, ignoringExtraLines: true))
}

// MARK: Selections - Fragments

func test__render_selections__givenFragments_rendersFragmentSelections() async throws {
Expand Down Expand Up @@ -6952,6 +7003,54 @@ class SelectionSetTemplateTests: XCTestCase {
.to(equalLineByLine(predator_expected, atLine: 21, ignoringExtraLines: true))
}

// Related to https://github.com/apollographql/apollo-ios/issues/3326
func test__render_nestedSelectionSet__givenInlineFragmentWithOnlyReservedField_doesNotRenderAsCompositeInlineFragment() async throws {
// given
schemaSDL = """
type Query {
allAnimals: [Animal]
}
union Animal = AnimalObject | AnimalError
type AnimalObject {
species: String!
}
type AnimalError {
code: Int!
}
"""

document = """
query TestOperation {
allAnimals {
... on AnimalObject {
__typename
}
}
}
"""

let expected = """
public var asAnimalObject: AsAnimalObject? { _asInlineFragment() }
/// AllAnimal.AsAnimalObject
public struct AsAnimalObject: TestSchema.InlineFragment {
"""

// when
try await buildSubjectAndOperation()
let allAnimals = try XCTUnwrap(
operation[field: "query"]?[field: "allAnimals"]?.selectionSet
)

let actual = subject.test_render(childEntity: allAnimals.computed)

// then
expect(actual).to(equalLineByLine(expected, atLine: 12, ignoringExtraLines: true))
}

// MARK: Nested Selection Sets - Reserved Keywords + Special Names

func test__render_nestedSelectionSet__givenEntityFieldWithSwiftKeywordAndApolloReservedTypeNames_rendersSelectionSetWithNameSuffixed() async throws {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Overview

When a type case selection consisted of only GraphQL reserved fields, such as `__typename` the selection was not being recognized and a composite inline fragment was generated. This resulted in empty `__selections` and `__mergedSources` lists. This in turn caused the `_asInlineFragment` accessor to behave incorrectly because it checks whether the definitions contained within `__mergedSources` are a match for the returned type. The empty list always matches and `_asInlineFragment` incorrectly matches the returned type.

We should not be generating the inline fragment with `CompositeInlineFragment` conformance, which will then not generate the empty `__mergedSources` list. Codegen will also no longer generate empty `__selections` lists and this integration test was created to ensure that the generated model still conforms to `SelectionSet` through the extensions default implementation of `__selections`.

## Reference Issue: https://github.com/apollographql/apollo-ios/issues/3326

## Solution

The `TypeInfo` for a selection now has a property indicating whether it was defined by the user or formed through a combination of external selections, i.e.: composite. This allows the root field builder to definitively specify under which conditions a composite inline fragment should be generated.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
query TestOperation {
allAnimals {
... on AnimalObject {
__typename
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
type Query {
allAnimals: [Animal]
}

union Animal = AnimalObject | AnimalError

type AnimalObject {
species: String!
}

type AnimalError {
code: Int!
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ struct SelectionSetTemplate {
\(SelectionSetNameDocumentation(inlineFragment))
\(renderAccessControl())\
struct \(inlineFragment.renderedTypeName): \(SelectionSetType(asInlineFragment: true))\
\(if: inlineFragment.isCompositeSelectionSet, ", \(config.ApolloAPITargetName).CompositeInlineFragment")\
\(if: inlineFragment.isCompositeInlineFragment, ", \(config.ApolloAPITargetName).CompositeInlineFragment")\
\(if: inlineFragment.isDeferred, ", \(config.ApolloAPITargetName).Deferrable")\
{
\(BodyTemplate(context))
Expand Down Expand Up @@ -274,18 +274,25 @@ struct SelectionSetTemplate {
var deprecatedArguments: [DeprecatedArgument]? =
config.options.warningsOnDeprecatedUsage == .include ? [] : nil

let selectionsTemplate = TemplateString(
"""
\(renderAccessControl())\
static var __selections: [\(config.ApolloAPITargetName).Selection] { [
\(if: shouldIncludeTypenameSelection(for: scope), ".field(\"__typename\", String.self),")
\(renderedSelections(groupedSelections.unconditionalSelections, &deprecatedArguments), terminator: ",")
\(groupedSelections.inclusionConditionGroups.map {
renderedConditionalSelectionGroup($0, $1, in: scope, &deprecatedArguments)
}, terminator: ",")
] }
"""
)
let shouldIncludeTypenameSelection = shouldIncludeTypenameSelection(for: scope)
let selectionsTemplate: TemplateString

if !groupedSelections.isEmpty || shouldIncludeTypenameSelection {
selectionsTemplate = TemplateString("""
\(renderAccessControl())\
static var __selections: [\(config.ApolloAPITargetName).Selection] { [
\(if: shouldIncludeTypenameSelection, ".field(\"__typename\", String.self),")
\(renderedSelections(groupedSelections.unconditionalSelections, &deprecatedArguments), terminator: ",")
\(groupedSelections.inclusionConditionGroups.map {
renderedConditionalSelectionGroup($0, $1, in: scope, &deprecatedArguments)
}, terminator: ",")
] }
"""
)
} else {
selectionsTemplate = ""
}

return """
\(if: deprecatedArguments != nil && !deprecatedArguments.unsafelyUnwrapped.isEmpty, """
\(deprecatedArguments.unsafelyUnwrapped.map { """
Expand Down Expand Up @@ -752,12 +759,8 @@ private class SelectionSetNameCache {

extension IR.ComputedSelectionSet {

fileprivate var isCompositeSelectionSet: Bool {
return direct?.isEmpty ?? true
}

fileprivate var isCompositeInlineFragment: Bool {
return !self.isEntityRoot && isCompositeSelectionSet
return !self.isEntityRoot && !self.isUserDefined && (direct?.isEmpty ?? true)
}

fileprivate var shouldBeRendered: Bool {
Expand Down
6 changes: 4 additions & 2 deletions apollo-ios-codegen/Sources/IR/IR+ComputedSelectionSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ extension ComputedSelectionSet {
private func createShallowlyMergedNestedEntityField(from field: EntityField) -> EntityField {
let typeInfo = SelectionSet.TypeInfo(
entity: entityStorage.entity(for: field.underlyingField, on: typeInfo.entity),
scopePath: self.typeInfo.scopePath.appending(field.selectionSet.typeInfo.scope)
scopePath: self.typeInfo.scopePath.appending(field.selectionSet.typeInfo.scope),
isUserDefined: false
)

let newSelectionSet = SelectionSet(
Expand Down Expand Up @@ -149,7 +150,8 @@ extension ComputedSelectionSet {

let typeInfo = SelectionSet.TypeInfo(
entity: self.typeInfo.entity,
scopePath: self.typeInfo.scopePath.mutatingLast { $0.appending(condition) }
scopePath: self.typeInfo.scopePath.mutatingLast { $0.appending(condition) },
isUserDefined: false
)

let selectionSet = SelectionSet(
Expand Down
10 changes: 8 additions & 2 deletions apollo-ios-codegen/Sources/IR/IR+DirectSelections.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ public class DirectSelections: Equatable, CustomDebugStringConvertible {

let typeInfo = SelectionSet.TypeInfo(
entity: existingField.entity,
scopePath: wrapperScope
scopePath: wrapperScope,
isUserDefined: true
)

let selectionSet = SelectionSet(
Expand All @@ -111,7 +112,8 @@ public class DirectSelections: Equatable, CustomDebugStringConvertible {
entity: newField.entity,
scopePath: wrapperField.selectionSet.scopePath.mutatingLast {
$0.appending(newFieldConditions)
}
},
isUserDefined: true
)

let newFieldSelectionSet = SelectionSet(
Expand Down Expand Up @@ -209,6 +211,10 @@ public class DirectSelections: Equatable, CustomDebugStringConvertible {
public private(set) var inclusionConditionGroups:
OrderedDictionary<AnyOf<InclusionConditions>, DirectSelections.ReadOnly> = [:]

public var isEmpty: Bool {
unconditionalSelections.isEmpty && inclusionConditionGroups.isEmpty
}

init(_ directSelections: DirectSelections.ReadOnly) {
for selection in directSelections.fields {
if let condition = selection.value.inclusionConditions {
Expand Down
6 changes: 4 additions & 2 deletions apollo-ios-codegen/Sources/IR/IR+RootFieldBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class RootFieldBuilder {
) async -> SelectionSet {
let typeInfo = SelectionSet.TypeInfo(
entity: entity,
scopePath: scopePath
scopePath: scopePath,
isUserDefined: true
)

var directSelections: DirectSelections? = nil
Expand Down Expand Up @@ -420,7 +421,8 @@ class RootFieldBuilder {

let typeInfo = SelectionSet.TypeInfo(
entity: parentTypeInfo.entity,
scopePath: scopePath
scopePath: scopePath,
isUserDefined: true
)

let fragmentSpread = NamedFragmentSpread(
Expand Down
12 changes: 11 additions & 1 deletion apollo-ios-codegen/Sources/IR/IR+SelectionSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ public class SelectionSet: Hashable, CustomDebugStringConvertible {
/// The selection set's `scope` is the last element in the list.
public let scopePath: LinkedList<ScopeDescriptor>

/// Indicates if the `SelectionSet` was created directly due to a selection set in the user defined `.graphql` definition file.
///
/// If `false`, the selection set was artificially created by the IR. Currently, the only reason for this is a `CompositeInlineFragment` created during calculation of merged selections for field merging.
public var isUserDefined: Bool

// MARK: - Computed Properties

/// Describes all of the types and inclusion conditions the selection set matches.
/// Derived from all the selection set's parents.
public var scope: ScopeDescriptor { scopePath.last.value }
Expand All @@ -28,6 +35,7 @@ public class SelectionSet: Hashable, CustomDebugStringConvertible {
public var deferCondition: CompilationResult.DeferCondition? {
scope.scopePath.last.value.deferCondition
}

public var isDeferred: Bool { deferCondition != nil }

/// Indicates if the `SelectionSet` represents a root selection set.
Expand All @@ -38,10 +46,12 @@ public class SelectionSet: Hashable, CustomDebugStringConvertible {

init(
entity: Entity,
scopePath: LinkedList<ScopeDescriptor>
scopePath: LinkedList<ScopeDescriptor>,
isUserDefined: Bool
) {
self.entity = entity
self.scopePath = scopePath
self.isUserDefined = isUserDefined
}

public static func == (lhs: TypeInfo, rhs: TypeInfo) -> Bool {
Expand Down

0 comments on commit f911089

Please sign in to comment.