don't remove fragments if they are added by other transforms

Author: temaEmelyan

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

@@ -44,7 +44,11 @@ class NadelNoInterfaceToObjectFragmentExpansionTransform : NadelTransform<State>
         val aliasHelper: NadelAliasHelper,
         val exposedOverallImplNames: Set<String>,
         val relaxedFieldResultKey: String,
-    )
+    ) {
+        // Applicability is planned before query transforms run. Record whether the final field was actually relaxed
+        // so result filtering cannot act on stale planned state.
+        var wasRelaxed = false
+    }
 
     override suspend fun isApplicable(
         executionContext: NadelExecutionContext,
@@ -96,6 +100,14 @@ class NadelNoInterfaceToObjectFragmentExpansionTransform : NadelTransform<State>
         state: State,
         transformServiceExecutionContext: NadelTransformServiceExecutionContext?,
     ): NadelTransformFieldResult {
+        // An earlier transform may have narrowed this field since isApplicable was evaluated.
+        val currentExposedImplNames =
+            computeExposedImplNamesIfRelaxable(executionBlueprint, service, field)
+        if (currentExposedImplNames != state.exposedOverallImplNames) {
+            return NadelTransformFieldResult.unmodified(field)
+        }
+        state.wasRelaxed = true
+
         // Keep objectTypeNames honest (only the exposed members). Rather than widen them, flag the field and
         // the injected __typename as forcePrintBare so the forked compiler emits them without a `... on Type`
         // fragment. See [NadelTransformFieldResult.forcePrintBare].
@@ -124,6 +136,10 @@ class NadelNoInterfaceToObjectFragmentExpansionTransform : NadelTransform<State>
         nodes: JsonNodes,
         transformServiceExecutionContext: NadelTransformServiceExecutionContext?,
     ): List<NadelResultInstruction> {
+        if (!state.wasRelaxed) {
+            return emptyList()
+        }
+
         val parentPath = underlyingParentField?.queryPath ?: overallField.parent?.queryPath ?: return emptyList()
         val parentNodes = nodes.getNodesAt(parentPath, flatten = true)
         val key = state.aliasHelper.typeNameResultKey

test/src/test/kotlin/graphql/nadel/tests/next/fixtures/execution/interfaceexpansion/hiddenmembership/HiddenImplRelaxedSelectionNarrowedByEarlierTransformTest.kt

@@ -0,0 +1,75 @@
+package graphql.nadel.tests.next.fixtures.execution.interfaceexpansion.hiddenmembership
+
+import graphql.nadel.Nadel
+import graphql.nadel.NadelExecutionHints
+import graphql.nadel.Service
+import graphql.nadel.ServiceExecutionHydrationDetails
+import graphql.nadel.engine.NadelExecutionContext
+import graphql.nadel.engine.NadelServiceExecutionContext
+import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
+import graphql.nadel.engine.transform.NadelTransformFieldResult
+import graphql.nadel.engine.transform.NadelTransformServiceExecutionContext
+import graphql.nadel.engine.transform.query.NadelQueryTransformer
+import graphql.nadel.engine.util.toBuilder
+import graphql.nadel.tests.util.NadelTransformAdapter
+import graphql.normalized.ExecutableNormalizedField
+
+/**
+ * Simulates an earlier transform narrowing an abstract selection to one implementation. The
+ * [NadelNoInterfaceToObjectFragmentExpansionTransform] transform must re-evaluate the narrowed field instead of
+ * applying its precomputed plan: should keep the fragments on interfaces if they were added by previous transforms.
+ */
+class HiddenImplRelaxedSelectionNarrowedByEarlierTransformTest : HiddenImplementationTestBase(
+    query = """
+        query {
+          nodes {
+            id
+          }
+        }
+    """.trimIndent(),
+) {
+    override fun makeExecutionHints(): NadelExecutionHints.Builder {
+        return super.makeExecutionHints().noInterfaceToObjectFragmentExpansion { _ -> true }
+    }
+
+    override fun makeNadel(): Nadel.Builder {
+        return super.makeNadel()
+            .transforms(listOf(NarrowIdSelectionToIssueTransform()))
+    }
+}
+
+private class NarrowIdSelectionToIssueTransform : NadelTransformAdapter {
+    override suspend fun isApplicable(
+        executionContext: NadelExecutionContext,
+        serviceExecutionContext: NadelServiceExecutionContext,
+        executionBlueprint: NadelOverallExecutionBlueprint,
+        services: Map<String, Service>,
+        service: Service,
+        overallField: ExecutableNormalizedField,
+        transformServiceExecutionContext: NadelTransformServiceExecutionContext?,
+        hydrationDetails: ServiceExecutionHydrationDetails?,
+    ): Unit? {
+        return Unit.takeIf {
+            overallField.name == "id" &&
+                overallField.objectTypeNames.toSet() == setOf("Issue", "Story")
+        }
+    }
+
+    override suspend fun transformField(
+        executionContext: NadelExecutionContext,
+        serviceExecutionContext: NadelServiceExecutionContext,
+        transformer: NadelQueryTransformer,
+        executionBlueprint: NadelOverallExecutionBlueprint,
+        service: Service,
+        field: ExecutableNormalizedField,
+        state: Unit,
+        transformServiceExecutionContext: NadelTransformServiceExecutionContext?,
+    ): NadelTransformFieldResult {
+        return NadelTransformFieldResult(
+            newField = field.toBuilder()
+                .clearObjectTypesNames()
+                .objectTypeNames(listOf("Issue"))
+                .build(),
+        )
+    }
+}

test/src/test/kotlin/graphql/nadel/tests/next/fixtures/execution/interfaceexpansion/hiddenmembership/HiddenImplRelaxedSelectionNarrowedByEarlierTransformTestSnapshot.kt

@@ -0,0 +1,103 @@
+// @formatter:off
+package graphql.nadel.tests.next.fixtures.execution.interfaceexpansion.hiddenmembership
+
+import graphql.nadel.tests.next.ExpectedNadelResult
+import graphql.nadel.tests.next.ExpectedServiceCall
+import graphql.nadel.tests.next.TestSnapshot
+import graphql.nadel.tests.next.listOfJsonStrings
+import kotlin.Suppress
+import kotlin.collections.List
+import kotlin.collections.listOf
+
+private suspend fun main() {
+    graphql.nadel.tests.next.update<HiddenImplRelaxedSelectionNarrowedByEarlierTransformTest>()
+}
+
+/**
+ * This class is generated. Do NOT modify.
+ *
+ * Refer to [graphql.nadel.tests.next.UpdateTestSnapshots]
+ */
+@Suppress("unused")
+public class HiddenImplRelaxedSelectionNarrowedByEarlierTransformTestSnapshot : TestSnapshot() {
+    /**
+     * Query
+     *
+     * ```graphql
+     * query {
+     *   nodes {
+     *     id
+     *   }
+     * }
+     * ```
+     *
+     * Variables
+     *
+     * ```json
+     * {}
+     * ```
+     */
+    override val calls: List<ExpectedServiceCall> = listOf(
+            ExpectedServiceCall(
+                service = "data",
+                query = """
+                | {
+                |   nodes {
+                |     ... on Issue {
+                |       id
+                |     }
+                |   }
+                | }
+                """.trimMargin(),
+                variables = "{}",
+                result = """
+                | {
+                |   "data": {
+                |     "nodes": [
+                |       {
+                |         "id": "ISSUE-1"
+                |       },
+                |       {},
+                |       {}
+                |     ]
+                |   }
+                | }
+                """.trimMargin(),
+                delayedResults = listOfJsonStrings(
+                ),
+            ),
+        )
+
+    /**
+     * ```json
+     * {
+     *   "data": {
+     *     "nodes": [
+     *       {
+     *         "id": "ISSUE-1"
+     *       },
+     *       {},
+     *       {}
+     *     ]
+     *   }
+     * }
+     * ```
+     */
+    override val result: ExpectedNadelResult = ExpectedNadelResult(
+            result = """
+            | {
+            |   "data": {
+            |     "nodes": [
+            |       {
+            |         "id": "ISSUE-1"
+            |       },
+            |       {},
+            |       {}
+            |     ]
+            |   }
+            | }
+            """.trimMargin(),
+            delayedResults = listOfJsonStrings(
+            ),
+        )
+}