Skip to content

Commit bc93f20

Browse files
sav007tasomaniac
authored andcommitted
Fix issue with fragment condition type coercing (#2090)
* Fix issue with fragment condition type coercing Closes #1975 * Feedback (cherry picked from commit 64d1716)
1 parent e9afbba commit bc93f20

File tree

8 files changed

+874
-48
lines changed

8 files changed

+874
-48
lines changed

apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/parser/GraphQLDocumentParser.kt

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
280280
val fields = selectionSet().parse(schemaFieldType)
281281
val fragmentRefs = selectionSet().fragmentRefs()
282282
val inlineFragments = selectionSet()?.selection()?.mapNotNull { ctx ->
283-
ctx.inlineFragment()?.parse(parentSelectionSet = selectionSet(), parentSchemaType = schemaFieldType)
283+
ctx.inlineFragment()?.parse(parentSchemaType = schemaFieldType, parentFields = fields)
284284
}?.flatten() ?: ParseResult(result = emptyList())
285285

286286
val inlineFragmentFieldsToMerge = inlineFragments.result
@@ -387,38 +387,55 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
387387
}
388388

389389
private fun GraphQLParser.InlineFragmentContext.parse(
390-
parentSelectionSet: GraphQLParser.SelectionSetContext,
391-
parentSchemaType: Schema.Type
390+
parentSchemaType: Schema.Type,
391+
parentFields: ParseResult<List<Field>>
392+
392393
): ParseResult<InlineFragment> {
393394
val typeCondition = typeCondition().typeName().NAME().text
394395
val schemaType = schema[typeCondition] ?: throw ParseException(
395396
message = "Unknown type`$typeCondition}`",
396397
token = typeCondition().typeName().start
397398
)
398399

399-
if (!parentSchemaType.isAssignableFrom(schema = schema, other = schemaType)) {
400+
if (!parentSchemaType.isAssignableFrom(other = schemaType, schema = schema)) {
400401
throw ParseException(
401-
message = "Fragment cannot be spread here as objects of type `${parentSchemaType.name}` can never be of type `$typeCondition`",
402+
message = "Fragment cannot be spread here as result can never be of type `$typeCondition`",
402403
token = typeCondition().typeName().start
403404
)
404405
}
405406

406-
val possibleTypes = when (schemaType) {
407-
is Schema.Type.Interface -> schemaType.possibleTypes?.map { it.rawType.name!! } ?: emptyList()
408-
is Schema.Type.Union -> schemaType.possibleTypes?.map { it.rawType.name!! } ?: emptyList()
409-
else -> listOf(typeCondition)
410-
}.distinct()
407+
val decoratedParentFields = parentFields.let { (parentFields, usedTypes) ->
408+
// if inline fragment conditional type contains the same field as parent type
409+
// carry over meta info such as: `description`, `isDeprecated`, `deprecationReason`
410+
val decoratedFields = parentFields.map { parentField ->
411+
when (schemaType) {
412+
is Schema.Type.Interface -> schemaType.fields?.find { it.name == parentField.fieldName }
413+
is Schema.Type.Object -> schemaType.fields?.find { it.name == parentField.fieldName }
414+
is Schema.Type.Union -> schemaType.fields?.find { it.name == parentField.fieldName }
415+
else -> null
416+
}?.let { field ->
417+
parentField.copy(
418+
description = field.description ?: parentField.description,
419+
isDeprecated = field.isDeprecated,
420+
deprecationReason = field.deprecationReason ?: ""
421+
)
422+
} ?: parentField
423+
}
424+
ParseResult(
425+
result = decoratedFields,
426+
usedTypes = usedTypes
427+
)
428+
}
411429

412-
val fields = parentSelectionSet.parse(schemaType).plus(
413-
selectionSet().parse(schemaType)
414-
) { left, right -> left.union(right) }
430+
val fields = decoratedParentFields.plus(selectionSet().parse(schemaType)) { left, right -> left.union(right) }
415431
if (fields.result.isEmpty()) {
416432
throw ParseException(
417433
message = "Inline fragment `$typeCondition` must have a selection of sub-fields",
418434
token = typeCondition().typeName().NAME().symbol
419435
)
420436
}
421437

438+
val possibleTypes = schemaType.possibleTypes(schema).toList()
422439
return ParseResult(
423440
result = InlineFragment(
424441
typeCondition = typeCondition,
@@ -449,12 +466,7 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
449466
token = typeCondition().typeName().NAME().symbol
450467
)
451468

452-
val possibleTypes = when (schemaType) {
453-
is Schema.Type.Interface -> schemaType.possibleTypes?.map { it.rawType.name!! } ?: emptyList()
454-
is Schema.Type.Union -> schemaType.possibleTypes?.map { it.rawType.name!! } ?: emptyList()
455-
else -> listOf(typeCondition)
456-
}.distinct()
457-
469+
val possibleTypes = schemaType.possibleTypes(schema)
458470
val fields = selectionSet().parse(schemaType)
459471
if (fields.result.isEmpty()) {
460472
throw ParseException(
@@ -466,7 +478,7 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
466478
val fragmentRefs = selectionSet().fragmentRefs()
467479

468480
val inlineFragments = selectionSet()?.selection()?.mapNotNull { ctx ->
469-
ctx.inlineFragment()?.parse(parentSelectionSet = selectionSet(), parentSchemaType = schemaType)
481+
ctx.inlineFragment()?.parse(parentSchemaType = schemaType, parentFields = fields)
470482
}?.flatten() ?: ParseResult(result = emptyList())
471483

472484
val mergeInlineFragmentFields = inlineFragments.result
@@ -482,7 +494,7 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
482494
fragmentName = fragmentName,
483495
typeCondition = typeCondition,
484496
source = graphQLDocumentSource,
485-
possibleTypes = possibleTypes,
497+
possibleTypes = possibleTypes.toList(),
486498
fields = fields.result.mergeFields(mergeInlineFragmentFields),
487499
fragmentRefs = fragmentRefs.union(mergeInlineFragmentRefs).toList(),
488500
inlineFragments = inlineFragments.result.filter { it.typeCondition != typeCondition },
@@ -744,8 +756,7 @@ class GraphQLDocumentParser(val schema: Schema, private val packageNameProvider:
744756
}.also { possibleTypes ->
745757
if (fragment.possibleTypes.intersect(possibleTypes).isEmpty()) {
746758
throw GraphQLDocumentParseException(
747-
message = "Fragment `${ref.name}` can't be spread here as objects of type `$typeCondition` can never be of " +
748-
"type `${fragment.typeCondition}`",
759+
message = "Fragment `${ref.name}` can't be spread here as result can never be of type `${fragment.typeCondition}`",
749760
sourceLocation = ref.sourceLocation,
750761
graphQLFilePath = filePath
751762
)

apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/parser/Utils.kt

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,12 @@ internal fun Schema.Type.possibleTypes(schema: Schema): Set<String> {
2525
}
2626
}
2727

28-
internal fun Schema.Type.isAssignableFrom(schema: Schema, other: Schema.Type): Boolean {
29-
if (name == other.name) {
30-
return true
31-
}
32-
return when (this) {
33-
is Schema.Type.Union -> possibleTypes(schema).intersect(other.possibleTypes(schema)).isNotEmpty()
34-
is Schema.Type.Interface -> {
35-
val possibleTypes = (possibleTypes ?: emptyList()).mapNotNull { it.rawType.name }
36-
possibleTypes.contains(other.name) || possibleTypes.any { typeName ->
37-
val schemaType = schema[typeName] ?: throw GraphQLParseException(
38-
message = "Unknown possible type `$typeName` for INTERFACE `$name`"
39-
)
40-
schemaType.isAssignableFrom(schema = schema, other = other)
41-
}
42-
}
43-
else -> false
44-
}
28+
internal fun Schema.Type.isAssignableFrom(other: Schema.Type, schema: Schema): Boolean {
29+
return Schema.TypeRef(kind = kind, name = name)
30+
.isAssignableFrom(
31+
other = Schema.TypeRef(kind = other.kind, name = other.name),
32+
schema = schema
33+
)
4534
}
4635

4736
internal operator fun SourceLocation.Companion.invoke(token: Token) = SourceLocation(
@@ -57,25 +46,27 @@ internal fun Schema.TypeRef.asGraphQLType(): String {
5746
}
5847
}
5948

60-
internal fun Schema.TypeRef.isAssignableFrom(other: Schema.TypeRef): Boolean {
49+
internal fun Schema.TypeRef.isAssignableFrom(other: Schema.TypeRef, schema: Schema): Boolean {
6150
return when (kind) {
6251
Schema.Kind.NON_NULL -> {
63-
other.kind == Schema.Kind.NON_NULL && ofType!!.isAssignableFrom(other.ofType!!)
52+
other.kind == Schema.Kind.NON_NULL && ofType!!.isAssignableFrom(other = other.ofType!!, schema = schema)
6453
}
6554

6655
Schema.Kind.LIST -> {
6756
if (other.kind == Schema.Kind.NON_NULL) {
68-
isAssignableFrom(other.ofType!!)
57+
isAssignableFrom(other = other.ofType!!, schema = schema)
6958
} else {
70-
other.kind == Schema.Kind.LIST && ofType!!.isAssignableFrom(other.ofType!!)
59+
other.kind == Schema.Kind.LIST && ofType!!.isAssignableFrom(other = other.ofType!!, schema = schema)
7160
}
7261
}
7362

7463
else -> {
7564
if (other.kind == Schema.Kind.NON_NULL) {
76-
isAssignableFrom(other.ofType!!)
65+
isAssignableFrom(other = other.ofType!!, schema = schema)
7766
} else {
78-
kind == other.kind && name == other.name
67+
val possibleTypes = schema.resolveType(this).possibleTypes(schema)
68+
val otherPossibleTypes = schema.resolveType(other).possibleTypes(schema)
69+
possibleTypes.intersect(otherPossibleTypes).isNotEmpty()
7970
}
8071
}
8172
}
@@ -99,3 +90,7 @@ internal fun Schema.resolveType(graphqlType: String): Schema.TypeRef = when {
9990
)
10091
}
10192
} ?: throw GraphQLParseException("Unknown type `$graphqlType`")
93+
94+
internal fun Schema.resolveType(typeRef: Schema.TypeRef): Schema.Type {
95+
return this[typeRef.name] ?: throw GraphQLParseException("Unknown type `${typeRef.name}`")
96+
}

apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/parser/Validation.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ private fun Operation.validateVariableType(name: String, expectedType: Schema.Ty
224224
"Variable `$name` is not defined by operation `${operationName}`"
225225
)
226226
val variableType = schema.resolveType(variable.type)
227-
if (!expectedType.isAssignableFrom(variableType)) {
227+
if (!expectedType.isAssignableFrom(other = variableType, schema = schema)) {
228228
throw GraphQLParseException(
229229
"Variable `$name` of type `${variableType.asGraphQLType()}` used in position expecting type `${expectedType.asGraphQLType()}`"
230230
)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
query TestQuery {
2+
foo {
3+
foo
4+
... on Bar {
5+
bar
6+
}
7+
}
8+
}

0 commit comments

Comments
 (0)