Skip to content

Commit 442d95c

Browse files
temaEmelyantemaEmelyan
committed
don do run catching, allow multiple interfaces
1 parent 41ec6f3 commit 442d95c

5 files changed

Lines changed: 451 additions & 41 deletions

File tree

‎lib/src/main/java/graphql/nadel/engine/transform/NadelNoInterfaceToObjectFragmentExpansionTransform.kt‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ import graphql.nadel.engine.util.JsonMap
2020
import graphql.nadel.engine.util.queryPath
2121
import graphql.normalized.ExecutableNormalizedField
2222
import graphql.normalized.ExecutableNormalizedField.newNormalizedField
23+
import graphql.schema.GraphQLFieldsContainer
2324
import graphql.schema.GraphQLInterfaceType
25+
import graphql.schema.GraphQLNamedType
26+
import graphql.schema.GraphQLSchema
27+
import graphql.schema.GraphQLType
2428
import graphql.schema.GraphQLTypeUtil.unwrapAll
2529
import graphql.schema.GraphQLUnionType
2630

@@ -68,9 +72,9 @@ class NadelNoInterfaceToObjectFragmentExpansionTransform : NadelTransform<State>
6872
?: return null
6973

7074
// Do nothing for fields with instructions (rename/hydration/stub/…)
71-
val hasFieldInstructions = runCatching {
72-
executionBlueprint.getTypeNameToInstructionMap<NadelFieldInstruction>(overallField).isNotEmpty()
73-
}.getOrDefault(true)
75+
val hasFieldInstructions = overallField.objectTypeNames.any { objectTypeName ->
76+
executionBlueprint.fieldInstructions.get(objectTypeName, overallField.name)?.isNotEmpty() == true
77+
}
7478
if (hasFieldInstructions) {
7579
return null
7680
}
@@ -145,71 +149,70 @@ class NadelNoInterfaceToObjectFragmentExpansionTransform : NadelTransform<State>
145149

146150
/**
147151
* The exposed overall implementation names if [overallField] is relaxable, else `null`. Relaxable means: the
148-
* parent is a single interface/union, the field is selectable at that level (an interface field, or
149-
* `__typename`), the selection covers exactly the exposed members, and at least one underlying member is
150-
* hidden. Doesn't consider the hint or field instructions - the caller does.
152+
* parent's output is one or more interfaces (or a single union), the field is selectable at that level (an
153+
* interface field present on every parent interface, or `__typename`), the selection covers exactly the exposed
154+
* members, and at least one underlying member is hidden. Doesn't consider the hint or field instructions - the
155+
* caller does.
151156
*/
152157
private fun computeExposedImplNamesIfRelaxable(
153158
executionBlueprint: NadelOverallExecutionBlueprint,
154159
service: Service,
155160
overallField: ExecutableNormalizedField,
156161
): Set<String>? {
157162
val parent = overallField.parent ?: return null
158-
159163
val engineSchema = executionBlueprint.engineSchema
160164
val isTypename = overallField.fieldName == TypeNameMetaFieldDef.name
161165

162-
// getFieldDefinitions can throw while planning a hydration backing query; any failure => not a candidate.
163-
val parentOutputTypes = runCatching {
164-
parent.getFieldDefinitions(engineSchema)
165-
.asSequence()
166-
.map { unwrapAll(it.type) }
167-
.toSet()
168-
}.getOrNull() ?: return null
169-
170-
// Exposed members, gated on the field being selectable at that level (union => __typename only).
171-
val exposedOverallImplNames: Set<String>
172-
val parentAbstractTypeName: String
173-
when (val parentType = parentOutputTypes.singleOrNull()) {
174-
is GraphQLInterfaceType -> {
175-
if (!isTypename && parentType.getFieldDefinition(overallField.fieldName) == null) {
166+
val parentOutputTypes = parent.objectTypeNames.map { parentTypeName ->
167+
val parentType = engineSchema.getType(parentTypeName) as? GraphQLFieldsContainer ?: return null
168+
val parentFieldDef = parentType.getFieldDefinition(parent.fieldName) ?: return null
169+
unwrapAll(parentFieldDef.type)
170+
}.toSet()
171+
172+
val parentAbstractTypes: List<GraphQLNamedType> =
173+
if (parentOutputTypes.isNotEmpty() && parentOutputTypes.all { it is GraphQLInterfaceType }) {
174+
val interfaces = parentOutputTypes.filterIsInstance<GraphQLInterfaceType>()
175+
val fieldIsOnEveryInterface =
176+
isTypename || interfaces.all { it.getFieldDefinition(overallField.fieldName) != null }
177+
if (!fieldIsOnEveryInterface) {
176178
return null
177179
}
178-
exposedOverallImplNames = engineSchema.getImplementations(parentType).map { it.name }.toSet()
179-
parentAbstractTypeName = parentType.name
180-
}
181-
is GraphQLUnionType -> {
180+
interfaces
181+
} else {
182+
val union = parentOutputTypes.singleOrNull() as? GraphQLUnionType ?: return null
182183
if (!isTypename) {
183184
return null
184185
}
185-
exposedOverallImplNames = parentType.types.map { it.name }.toSet()
186-
parentAbstractTypeName = parentType.name
186+
listOf(union)
187187
}
188-
else -> return null
189-
}
190188

189+
val exposedOverallImplNames =
190+
parentAbstractTypes.flatMap { abstractMemberNames(engineSchema, it) ?: return null }.toSet()
191191
// Means the client used fragments with explicit objects. don't relax.
192192
if (overallField.objectTypeNames.toSet() != exposedOverallImplNames) {
193193
return null
194194
}
195195

196-
val underlyingTypeName = executionBlueprint.getUnderlyingTypeName(parentAbstractTypeName)
197-
val underlyingMemberNames = when (val underlyingType = service.underlyingSchema.getType(underlyingTypeName)) {
198-
is GraphQLInterfaceType -> service.underlyingSchema.getImplementations(underlyingType).map { it.name }
199-
is GraphQLUnionType -> underlyingType.types.map { it.name }
200-
else -> return null
196+
val underlyingMemberNames = parentAbstractTypes.flatMap { abstractType ->
197+
val underlyingTypeName = executionBlueprint.getUnderlyingTypeName(abstractType.name)
198+
val underlyingType = service.underlyingSchema.getType(underlyingTypeName)
199+
abstractMemberNames(service.underlyingSchema, underlyingType) ?: return null
201200
}
202-
if (underlyingMemberNames.isEmpty()) {
203-
return null
204-
}
205-
206-
// Nothing to hide unless a member is hidden (and graphql-java already prints bare when none is).
207-
val hasHiddenImpl = underlyingMemberNames.any { underlyingName ->
201+
// Relax only if some underlying member is hidden from the overall (graphql-java already prints bare when none is).
202+
val hasHiddenMember = underlyingMemberNames.any { underlyingName ->
208203
executionBlueprint.getOverallTypeName(service, underlyingName) !in exposedOverallImplNames
209204
}
210-
if (!hasHiddenImpl) {
205+
if (!hasHiddenMember) {
211206
return null
212207
}
213208

214209
return exposedOverallImplNames
215210
}
211+
212+
/** The implementation/member type names of an interface or union in [schema], or `null` if [type] is neither. */
213+
private fun abstractMemberNames(schema: GraphQLSchema, type: GraphQLType?): List<String>? =
214+
when (type) {
215+
is GraphQLInterfaceType -> schema.getImplementations(type).map { it.name }
216+
is GraphQLUnionType -> type.types.map { it.name }
217+
else -> null
218+
}

‎test/src/test/kotlin/graphql/nadel/tests/next/fixtures/execution/interfaceexpansion/multiinterface/MultiInterfaceHiddenImplBareInterfaceFieldTestSnapshot.kt‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// @formatter:off
2+
package graphql.nadel.tests.next.fixtures.execution.interfaceexpansion.multiinterface
3+
4+
import graphql.nadel.tests.next.ExpectedNadelResult
5+
import graphql.nadel.tests.next.ExpectedServiceCall
6+
import graphql.nadel.tests.next.TestSnapshot
7+
import graphql.nadel.tests.next.listOfJsonStrings
8+
import kotlin.Suppress
9+
import kotlin.collections.List
10+
import kotlin.collections.listOf
11+
12+
private suspend fun main() {
13+
graphql.nadel.tests.next.update<MultiInterfaceHiddenImplBareInterfaceFieldTest>()
14+
}
15+
16+
/**
17+
* This class is generated. Do NOT modify.
18+
*
19+
* Refer to [graphql.nadel.tests.next.UpdateTestSnapshots]
20+
*/
21+
@Suppress("unused")
22+
public class MultiInterfaceHiddenImplBareInterfaceFieldTestSnapshot : TestSnapshot() {
23+
/**
24+
* Query
25+
*
26+
* ```graphql
27+
* query {
28+
* containers {
29+
* item {
30+
* id
31+
* }
32+
* }
33+
* }
34+
* ```
35+
*
36+
* Variables
37+
*
38+
* ```json
39+
* {}
40+
* ```
41+
*/
42+
override val calls: List<ExpectedServiceCall> = listOf(
43+
ExpectedServiceCall(
44+
service = "data",
45+
query = """
46+
| {
47+
| containers {
48+
| item {
49+
| ... on Post {
50+
| id
51+
| }
52+
| ... on Wiki {
53+
| id
54+
| }
55+
| }
56+
| }
57+
| }
58+
""".trimMargin(),
59+
variables = "{}",
60+
result = """
61+
| {
62+
| "data": {
63+
| "containers": [
64+
| {
65+
| "item": {
66+
| "id": "WIKI-1"
67+
| }
68+
| },
69+
| {
70+
| "item": {
71+
| "id": "POST-1"
72+
| }
73+
| }
74+
| ]
75+
| }
76+
| }
77+
""".trimMargin(),
78+
delayedResults = listOfJsonStrings(
79+
),
80+
),
81+
)
82+
83+
/**
84+
* ```json
85+
* {
86+
* "data": {
87+
* "containers": [
88+
* {
89+
* "item": {
90+
* "id": "WIKI-1"
91+
* }
92+
* },
93+
* {
94+
* "item": {
95+
* "id": "POST-1"
96+
* }
97+
* }
98+
* ]
99+
* }
100+
* }
101+
* ```
102+
*/
103+
override val result: ExpectedNadelResult = ExpectedNadelResult(
104+
result = """
105+
| {
106+
| "data": {
107+
| "containers": [
108+
| {
109+
| "item": {
110+
| "id": "WIKI-1"
111+
| }
112+
| },
113+
| {
114+
| "item": {
115+
| "id": "POST-1"
116+
| }
117+
| }
118+
| ]
119+
| }
120+
| }
121+
""".trimMargin(),
122+
delayedResults = listOfJsonStrings(
123+
),
124+
)
125+
}

‎test/src/test/kotlin/graphql/nadel/tests/next/fixtures/execution/interfaceexpansion/multiinterface/MultiInterfaceHiddenImplRelaxedBareInterfaceFieldTestSnapshot.kt‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
// @formatter:off
2+
package graphql.nadel.tests.next.fixtures.execution.interfaceexpansion.multiinterface
3+
4+
import graphql.nadel.tests.next.ExpectedNadelResult
5+
import graphql.nadel.tests.next.ExpectedServiceCall
6+
import graphql.nadel.tests.next.TestSnapshot
7+
import graphql.nadel.tests.next.listOfJsonStrings
8+
import kotlin.Suppress
9+
import kotlin.collections.List
10+
import kotlin.collections.listOf
11+
12+
private suspend fun main() {
13+
graphql.nadel.tests.next.update<MultiInterfaceHiddenImplRelaxedBareInterfaceFieldTest>()
14+
}
15+
16+
/**
17+
* This class is generated. Do NOT modify.
18+
*
19+
* Refer to [graphql.nadel.tests.next.UpdateTestSnapshots]
20+
*/
21+
@Suppress("unused")
22+
public class MultiInterfaceHiddenImplRelaxedBareInterfaceFieldTestSnapshot : TestSnapshot() {
23+
/**
24+
* Query
25+
*
26+
* ```graphql
27+
* query {
28+
* containers {
29+
* item {
30+
* id
31+
* }
32+
* }
33+
* }
34+
* ```
35+
*
36+
* Variables
37+
*
38+
* ```json
39+
* {}
40+
* ```
41+
*/
42+
override val calls: List<ExpectedServiceCall> = listOf(
43+
ExpectedServiceCall(
44+
service = "data",
45+
query = """
46+
| {
47+
| containers {
48+
| item {
49+
| __typename__abstract_member__item: __typename
50+
| id
51+
| }
52+
| }
53+
| }
54+
""".trimMargin(),
55+
variables = "{}",
56+
result = """
57+
| {
58+
| "data": {
59+
| "containers": [
60+
| {
61+
| "item": {
62+
| "id": "WIKI-1",
63+
| "__typename__abstract_member__item": "Wiki"
64+
| }
65+
| },
66+
| {
67+
| "item": {
68+
| "id": "POST-1",
69+
| "__typename__abstract_member__item": "Post"
70+
| }
71+
| }
72+
| ]
73+
| }
74+
| }
75+
""".trimMargin(),
76+
delayedResults = listOfJsonStrings(
77+
),
78+
),
79+
)
80+
81+
/**
82+
* ```json
83+
* {
84+
* "data": {
85+
* "containers": [
86+
* {
87+
* "item": {
88+
* "id": "WIKI-1"
89+
* }
90+
* },
91+
* {
92+
* "item": {
93+
* "id": "POST-1"
94+
* }
95+
* }
96+
* ]
97+
* }
98+
* }
99+
* ```
100+
*/
101+
override val result: ExpectedNadelResult = ExpectedNadelResult(
102+
result = """
103+
| {
104+
| "data": {
105+
| "containers": [
106+
| {
107+
| "item": {
108+
| "id": "WIKI-1"
109+
| }
110+
| },
111+
| {
112+
| "item": {
113+
| "id": "POST-1"
114+
| }
115+
| }
116+
| ]
117+
| }
118+
| }
119+
""".trimMargin(),
120+
delayedResults = listOfJsonStrings(
121+
),
122+
)
123+
}

0 commit comments

Comments
 (0)