Remove short circuit empty query hint (#747)

Author: temaEmelyan

lib/src/main/java/graphql/nadel/NadelExecutionHints.kt

@@ -12,15 +12,13 @@ import graphql.nadel.hints.NadelNoInterfaceToObjectFragmentExpansionHint
 import graphql.nadel.hints.NadelReachableUnderlyingServiceTypesHint
 import graphql.nadel.hints.NadelShadowUnderlyingTypeNameInvestigation
 import graphql.nadel.hints.NadelSharedTypeRenamesHint
-import graphql.nadel.hints.NadelShortCircuitEmptyQueryHint
 import graphql.nadel.hints.NadelVirtualTypeSupportHint
 
 data class NadelExecutionHints(
     val legacyOperationNames: LegacyOperationNamesHint,
     val allDocumentVariablesHint: AllDocumentVariablesHint,
     val deferSupport: NadelDeferSupportHint,
     val sharedTypeRenames: NadelSharedTypeRenamesHint,
-    val shortCircuitEmptyQuery: NadelShortCircuitEmptyQueryHint,
     val virtualTypeSupport: NadelVirtualTypeSupportHint,
     val executeOnEngineSchema: NadelExecuteOnEngineSchemaHint,
     val hydrationFilterObjectTypes: NadelHydrationFilterObjectTypesHint,
@@ -45,7 +43,6 @@ data class NadelExecutionHints(
         private var legacyOperationNames = LegacyOperationNamesHint { false }
         private var allDocumentVariablesHint = AllDocumentVariablesHint { false }
         private var deferSupport = NadelDeferSupportHint { false }
-        private var shortCircuitEmptyQuery = NadelShortCircuitEmptyQueryHint { false }
         private var sharedTypeRenames = NadelSharedTypeRenamesHint { false }
         private var virtualTypeSupport = NadelVirtualTypeSupportHint { false }
         private var executeOnEngineSchema = NadelExecuteOnEngineSchemaHint { false }
@@ -63,7 +60,6 @@ data class NadelExecutionHints(
             legacyOperationNames = nadelExecutionHints.legacyOperationNames
             allDocumentVariablesHint = nadelExecutionHints.allDocumentVariablesHint
             deferSupport = nadelExecutionHints.deferSupport
-            shortCircuitEmptyQuery = nadelExecutionHints.shortCircuitEmptyQuery
             sharedTypeRenames = nadelExecutionHints.sharedTypeRenames
             virtualTypeSupport = nadelExecutionHints.virtualTypeSupport
             executeOnEngineSchema = nadelExecutionHints.executeOnEngineSchema
@@ -91,11 +87,6 @@ data class NadelExecutionHints(
             return this
         }
 
-        fun shortCircuitEmptyQuery(flag: NadelShortCircuitEmptyQueryHint): Builder {
-            shortCircuitEmptyQuery = flag
-            return this
-        }
-
         fun sharedTypeRenames(flag: NadelSharedTypeRenamesHint): Builder {
             sharedTypeRenames = flag
             return this
@@ -152,7 +143,6 @@ data class NadelExecutionHints(
                 allDocumentVariablesHint,
                 deferSupport,
                 sharedTypeRenames,
-                shortCircuitEmptyQuery,
                 virtualTypeSupport,
                 executeOnEngineSchema,
                 hydrationFilterObjectTypes,

lib/src/main/java/graphql/nadel/NextgenEngine.kt

@@ -26,6 +26,7 @@ import graphql.nadel.engine.transform.query.DynamicServiceResolution
 import graphql.nadel.engine.transform.query.NadelFieldToService
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultTransformer
+import graphql.nadel.engine.transform.skipInclude.NadelSkipIncludeTransform.Companion.isSkipIncludeArtificialField
 import graphql.nadel.engine.util.MutableJsonMap
 import graphql.nadel.engine.util.beginExecute
 import graphql.nadel.engine.util.compileToDocument
@@ -411,7 +412,7 @@ internal class NextgenEngine(
                 .firstOrNull() ?: topLevelFields.first(),
         )
 
-        val serviceExecution = getServiceExecution(service, topLevelFields, executionContext.hints)
+        val serviceExecution = getServiceExecution(service, topLevelFields)
         val serviceExecResult = try {
             serviceExecution.execute(serviceExecParams)
                 .asDeferred()
@@ -469,13 +470,12 @@ internal class NextgenEngine(
     private fun getServiceExecution(
         service: Service,
         topLevelFields: List<ExecutableNormalizedField>,
-        hints: NadelExecutionHints,
     ): ServiceExecution {
-        if (hints.shortCircuitEmptyQuery(service) && isOnlyTopLevelFieldTypename(topLevelFields, service)) {
-            return engineSchemaIntrospectionService.serviceExecution
+        return if (isOnlyTopLevelFieldTypename(topLevelFields, service)) {
+            engineSchemaIntrospectionService.serviceExecution
+        } else {
+            service.serviceExecution
         }
-
-        return service.serviceExecution
     }
 
     private fun isOnlyTopLevelFieldTypename(
@@ -491,6 +491,7 @@ internal class NextgenEngine(
         return isNamespacedFieldLike(service, topLevelField)
             && topLevelField.hasChildren()
             && topLevelField.children.all { it.name == TypeNameMetaFieldDef.name }
+            && topLevelField.children.none(::isSkipIncludeArtificialField)
     }
 
     private fun getDocumentVariablePredicate(hints: NadelExecutionHints, service: Service): VariablePredicate {

lib/src/main/java/graphql/nadel/engine/transform/skipInclude/NadelSkipIncludeTransform.kt

@@ -38,10 +38,16 @@ import graphql.schema.GraphQLSchema
 internal class NadelSkipIncludeTransform : NadelTransform<State> {
     companion object {
         private const val skipFieldName = "__skip"
+        private const val skipIncludeTag = "skip_include"
 
         fun isSkipIncludeSpecialField(enf: ExecutableNormalizedField): Boolean {
             return enf.name == skipFieldName
         }
+
+        fun isSkipIncludeArtificialField(enf: ExecutableNormalizedField): Boolean {
+            return enf.name == Introspection.TypeNameMetaFieldDef.name
+                && enf.resultKey == "${Introspection.TypeNameMetaFieldDef.name}__${skipIncludeTag}__${skipFieldName}"
+        }
     }
 
     class State(
@@ -81,7 +87,7 @@ internal class NadelSkipIncludeTransform : NadelTransform<State> {
         return if (overallField.name == skipFieldName) {
             State(
                 aliasHelper = NadelAliasHelper.forField(
-                    tag = "skip_include",
+                    tag = skipIncludeTag,
                     field = overallField,
                 ),
             )

lib/src/main/java/graphql/nadel/hints/NadelShortCircuitEmptyQueryHint.kt

@@ -3,7 +3,6 @@ package graphql.nadel.tests.hooks
 import graphql.ErrorClassification
 import graphql.GraphQLError
 import graphql.nadel.Nadel
-import graphql.nadel.NadelExecutionHints
 import graphql.nadel.hooks.NadelExecutionHooks
 import graphql.nadel.tests.EngineTestHook
 import graphql.nadel.tests.UseHook
@@ -98,61 +97,46 @@ class `one-of-top-level-fields-is-removed` : EngineTestHook {
 @UseHook
 class `top-level-field-is-removed` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `top-level-field-is-removed-for-a-subscription` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `top-level-field-is-removed-for-a-subscription-with-namespaced-field` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
-}
-
-@UseHook
-class `top-level-field-is-removed-hint-is-off` : EngineTestHook {
-    override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { false }
 }
 
 @UseHook
 class `hydration-top-level-field-is-removed` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `namespaced-hydration-top-level-field-is-removed` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `hidden-namespaced-hydration-top-level-field-is-removed` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `namespaced-field-is-removed` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `namespaced-field-is-removed-with-renames` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 @UseHook
 class `renamed-top-level-field-is-not-removed-short-circuit-hint-is-on` : EngineTestHook {
     override val customTransforms = listOf(RemoveFieldTestTransform())
-    override fun makeExecutionHints(builder: NadelExecutionHints.Builder) = builder.shortCircuitEmptyQuery { true }
 }
 
 // @UseHook

test/src/test/kotlin/graphql/nadel/tests/hooks/remove-fields.kt

@@ -21,28 +21,6 @@ private suspend fun main() {
 @Suppress("unused")
 public class `basic hydration with static arg boolean snapshot` : TestSnapshot() {
     override val calls: List<ExpectedServiceCall> = listOf(
-            ExpectedServiceCall(
-                service = "service1",
-                query = """
-                | {
-                |   foo {
-                |     __typename__hydration__bar: __typename
-                |   }
-                | }
-                """.trimMargin(),
-                variables = "{}",
-                result = """
-                | {
-                |   "data": {
-                |     "foo": {
-                |       "__typename__hydration__bar": "Foo"
-                |     }
-                |   }
-                | }
-                """.trimMargin(),
-                delayedResults = listOfJsonStrings(
-                ),
-            ),
             ExpectedServiceCall(
                 service = "service2",
                 query = """

test/src/test/kotlin/graphql/nadel/tests/legacy/field removed/top level field is removed hint is off snapshot.kt

@@ -21,28 +21,6 @@ private suspend fun main() {
 @Suppress("unused")
 public class `basic hydration with static arg float snapshot` : TestSnapshot() {
     override val calls: List<ExpectedServiceCall> = listOf(
-            ExpectedServiceCall(
-                service = "service1",
-                query = """
-                | {
-                |   foo {
-                |     __typename__hydration__bar: __typename
-                |   }
-                | }
-                """.trimMargin(),
-                variables = "{}",
-                result = """
-                | {
-                |   "data": {
-                |     "foo": {
-                |       "__typename__hydration__bar": "Foo"
-                |     }
-                |   }
-                | }
-                """.trimMargin(),
-                delayedResults = listOfJsonStrings(
-                ),
-            ),
             ExpectedServiceCall(
                 service = "service2",
                 query = """