Omit scalar definitions from SDL (#5132)

martinbonnin · web-flow · commit b5db319ea233 · 2023-07-27T18:21:03.000+02:00
* omit scalars from SDL

* keep toSDL() experimental
diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt
@@ -2,7 +2,6 @@ package com.apollographql.apollo3.ast
 
 import com.apollographql.apollo3.annotations.ApolloExperimental
 import com.apollographql.apollo3.annotations.ApolloInternal
-import okio.Buffer
 
 /**
  * A wrapper around a schema [GQLDocument] that ensures the [GQLDocument] is valid and caches
@@ -49,7 +48,7 @@ class Schema internal constructor(
   fun toGQLDocument(): GQLDocument = GQLDocument(
       definitions = definitions,
       filePath = null
-  ).withoutBuiltinDefinitions()
+  )
 
   /**
    * @param name the current name of the directive (like "kotlin_labs__nonnull")
@@ -131,7 +130,7 @@ class Schema internal constructor(
   @ApolloInternal
   fun toMap(): Map<String, Any> {
     return mapOf(
-        "sdl" to GQLDocument(definitions, sourceLocation = null).toUtf8(),
+        "sdl" to GQLDocument(definitions, sourceLocation = null).toSDL(),
         "keyFields" to keyFields.mapValues { it.value.toList().sorted() },
         "foreignNames" to foreignNames,
         "directivesToStrip" to directivesToStrip,
@@ -217,7 +216,7 @@ class Schema internal constructor(
     @ApolloInternal
     fun fromMap(map: Map<String, Any>): Schema {
       return Schema(
-          definitions = (map["sdl"] as String).parseAsGQLDocument().getOrThrow().definitions,
+          definitions = combineDefinitions((map["sdl"] as String).parseAsGQLDocument().getOrThrow().definitions, builtinDefinitions(), ConflictResolution.TakeLeft),
           keyFields = (map["keyFields"]!! as Map<String, Collection<String>>).mapValues { it.value.toSet() },
           foreignNames = map["foreignNames"]!! as Map<String, String>,
           directivesToStrip = map["directivesToStrip"]!! as List<String>,
diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldocument.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldocument.kt
@@ -1,16 +1,28 @@
 package com.apollographql.apollo3.ast
 
+import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
+import com.apollographql.apollo3.annotations.ApolloExperimental
 import com.apollographql.apollo3.ast.internal.apollo_v0_1_definitionsStr
 import com.apollographql.apollo3.ast.internal.apollo_v0_2_definitionsStr
 import com.apollographql.apollo3.ast.internal.builtinsDefinitionsStr
-import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
 import com.apollographql.apollo3.ast.internal.linkDefinitionsStr
-import okio.Buffer
 
+/**
+ * Add builtin definitions from the latest spec version to the [GQLDocument]
+ *
+ * SDL representations must skip scalars and may skip directive definitions. This function adds them back to form a full schema.
+ *
+ * If a definition already exists, it is kept as is
+ *
+ * Scalars: https://spec.graphql.org/draft/#sel-GAHXJHABAB_D4G
+ * Directives: https://spec.graphql.org/draft/#sel-FAHnBPLCAACCcooU
+ */
 fun GQLDocument.withBuiltinDefinitions(): GQLDocument {
   return withDefinitions(builtinDefinitions())
 }
 
+@Deprecated("Use GQLDocument.toSDL() to write a GQLDocument without the scalar directives")
+@ApolloDeprecatedSince(ApolloDeprecatedSince.Version.v4_0_0)
 fun GQLDocument.withoutBuiltinDefinitions(): GQLDocument {
   return withoutDefinitions(builtinDefinitions())
 }
@@ -99,17 +111,24 @@ internal fun combineDefinitions(left: List<GQLDefinition>, right: List<GQLDefini
 
   return mergedDefinitions
 }
+private fun GQLDocument.withDefinitions(definitions: List<GQLDefinition>): GQLDocument {
+  return copy(
+      definitions = combineDefinitions(this.definitions, definitions, ConflictResolution.TakeLeft)
+  )
+}
 
 /**
- * Adds [definitions] to the [GQLDocument]
+ * Outputs a schema document to SDL. For executable documents, use toUtf8()
  *
- * If a definition already exists, it is kept as is and a warning is logged
- * See https://spec.graphql.org/draft/#sel-FAHnBPLCAACCcooU
+ * SDL representations must skip scalars definitions.
  *
- * A better implementation might verify that the definitions match or are compatible
+ * See https://spec.graphql.org/draft/#sel-GAHXJHABAB_D4G
  */
-private fun GQLDocument.withDefinitions(definitions: List<GQLDefinition>): GQLDocument {
-  return copy(
-      definitions = combineDefinitions(this.definitions, definitions, ConflictResolution.TakeLeft)
-  )
+@ApolloExperimental
+fun GQLDocument.toSDL(indent: String = "  "): String {
+  return this.copy(
+      definitions = definitions.filter {
+        it !is GQLScalarTypeDefinition || it.name !in GQLTypeDefinition.builtInTypes
+      }
+  ).toUtf8(indent)
 }
diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqlnode.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqlnode.kt
@@ -17,4 +17,3 @@ fun GQLNode.toUtf8(indent: String = "  "): String {
   toUtf8(buffer, indent)
   return buffer.readUtf8()
 }
-
diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/introspection/introspection_to_schema.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/introspection/introspection_to_schema.kt
@@ -315,10 +315,6 @@ private class GQLDocumentBuilder(private val introspectionSchema: IntrospectionS
 
 /**
  * Parses the [IntrospectionSchema] into a [GQLDocument]
- *
- * The returned [GQLDocument] does not contain any of the builtin definitions (scalars, directives, introspection)
- *
- * See https://spec.graphql.org/draft/#sel-GAHXJHABuCB_Dn6F
  */
 @ApolloExperimental
 fun IntrospectionSchema.toGQLDocument(filePath: String? = null): GQLDocument = GQLDocumentBuilder(this, filePath)
diff --git a/libraries/apollo-ast/src/commonTest/kotlin/com/apollographql/apollo3/graphql/ast/test/SDLWriterTest.kt b/libraries/apollo-ast/src/commonTest/kotlin/com/apollographql/apollo3/graphql/ast/test/SDLWriterTest.kt
@@ -1,17 +1,14 @@
 package com.apollographql.apollo3.graphql.ast.test
 
-import com.apollographql.apollo3.ast.SDLWriter
-import com.apollographql.apollo3.ast.Schema
+import com.apollographql.apollo3.ast.GQLDocument
 import com.apollographql.apollo3.ast.parseAsGQLDocument
-import com.apollographql.apollo3.ast.toSchema
-import com.apollographql.apollo3.ast.toUtf8
-import okio.Buffer
+import com.apollographql.apollo3.ast.toSDL
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
 class SDLWriterTest {
   @Test
-  fun schemaMayContainBuiltinDirectives() {
+  fun simpleTest() {
     val schemaString = """
       schema {
           query: Query
@@ -43,11 +40,9 @@ class SDLWriterTest {
 
     """.trimIndent()
 
-    val schema: Schema = schemaString.toSchema()
-    val writerBuffer = Buffer()
-    val sdlWriter = SDLWriter(writerBuffer, "    ")
-    sdlWriter.write(schema.toGQLDocument())
-    assertEquals(writerBuffer.readUtf8(), schemaString)
+    val schema: GQLDocument = schemaString.parseAsGQLDocument().getOrThrow()
+    val sdl = schema.toSDL("    ")
+    assertEquals(schemaString, sdl)
   }
 
   @Test
@@ -108,7 +103,7 @@ class SDLWriterTest {
 
     val expected = schemaString.parseAsGQLDocument().getOrThrow()
 
-    val serialized = expected.toUtf8()
+    val serialized = expected.toSDL()
 
     serialized.parseAsGQLDocument().getOrThrow()
     // Enable when we have hashCode and equals on GQLNode
diff --git a/libraries/apollo-tooling/src/main/kotlin/com/apollographql/apollo3/tooling/SchemaDownloader.kt b/libraries/apollo-tooling/src/main/kotlin/com/apollographql/apollo3/tooling/SchemaDownloader.kt
@@ -7,13 +7,12 @@ import com.apollographql.apollo3.ast.introspection.IntrospectionSchema
 import com.apollographql.apollo3.ast.introspection.toGQLDocument
 import com.apollographql.apollo3.ast.introspection.toIntrospectionSchema
 import com.apollographql.apollo3.ast.introspection.writeTo
+import com.apollographql.apollo3.ast.toSDL
 import com.apollographql.apollo3.ast.toSchema
-import com.apollographql.apollo3.ast.toUtf8
 import com.apollographql.apollo3.exception.ApolloGraphQLException
 import com.apollographql.apollo3.network.okHttpClient
 import com.apollographql.apollo3.tooling.platformapi.public.DownloadSchemaQuery
 import kotlinx.coroutines.runBlocking
-import okio.Buffer
 import java.io.File
 import com.apollographql.apollo3.tooling.graphql.draft.IntrospectionQuery as GraphQLDraftIntrospectionQuery
 import com.apollographql.apollo3.tooling.graphql.june2018.IntrospectionQuery as GraphQLJune2018IntrospectionQuery
@@ -121,7 +120,7 @@ object SchemaDownloader {
       if (introspectionSchema == null) {
         check(sdlSchema != null)
         // Convert from SDL to JSON
-        Buffer().writeUtf8(sdlSchema)
+        sdlSchema
             .toSchema()
             .toIntrospectionSchema()
             .writeTo(schema)
@@ -134,7 +133,7 @@ object SchemaDownloader {
       if (sdlSchema == null) {
         check(introspectionSchema != null)
         // Convert from JSON to SDL
-        schema.writeText(introspectionSchema.toGQLDocument().toUtf8(indent = "  "))
+        schema.writeText(introspectionSchema.toGQLDocument().toSDL(indent = "  "))
       } else {
         // Copy SDL verbatim
         schema.writeText(sdlSchema)

Original file line number	Diff line number	Diff line change
`@@ -17,4 +17,3 @@ fun GQLNode.toUtf8(indent: String = " "): String {`
`17`	`17`	`toUtf8(buffer, indent)`
`18`	`18`	`return buffer.readUtf8()`
`19`	`19`	`}`
`20`		`-`