Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scalar mapping and adapter configuration improvements #3779

Merged
merged 24 commits into from
Jan 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7ccc7a8
Add a design doc
BoD Jan 10, 2022
7e6486b
Address feedback: avoid using CustomScalarAdapters, instead use the w…
BoD Jan 12, 2022
cb79b03
Apply suggestions from code review
BoD Jan 14, 2022
381e891
Introduce ScalarInfo, AdapterInitializer and mapScalar
BoD Jan 12, 2022
6721a8e
Use scalarMapping in KotlinResolver.resolveIrType
BoD Jan 12, 2022
c034d00
Use scalarMapping in KotlinResolver.adapterInitializer, nonNullableSc…
BoD Jan 12, 2022
026df8b
Introduce ScalarAdapterInstancesBuilder to avoid creating adapter ins…
BoD Jan 13, 2022
65e2639
Rename customScalarsMapping -> scalarMapping
BoD Jan 13, 2022
f818fa0
Improve CustomScalarTest
BoD Jan 13, 2022
a7eb38c
Update CodegenTest with more scalar cases
BoD Jan 13, 2022
e9cf742
Pass / recover scalar mapping in metadata, to work as expected in mul…
BoD Jan 13, 2022
3cd05a4
Cleanup
BoD Jan 14, 2022
6fefb5a
Revert ScalarAdapterInstancesBuilder as it's potentially risky (threa…
BoD Jan 14, 2022
3e95b02
Simplify the user-facing API following review
BoD Jan 14, 2022
afb8a82
Update design doc after review
BoD Jan 14, 2022
30b96f9
Add convenience methods for builtin types/adapters
BoD Jan 14, 2022
a966675
Throw if scalar mapping are defined in several modules
BoD Jan 17, 2022
29d1329
Keep scalar mapping as 2 maps of String instead of using ScalarInfo. …
BoD Jan 17, 2022
7b0994e
Clarify test by explicitly using Longs
BoD Jan 17, 2022
e62a2b7
Rename CustomScalarTarget -> ScalarTarget
BoD Jan 17, 2022
822896f
Update API dump
BoD Jan 17, 2022
1e97e8f
Mark compiled types as deprecated and no longer use them in generated…
BoD Jan 19, 2022
af05c7e
Scalar mapping and adapter configuration improvements: Java codegen (…
BoD Jan 24, 2022
4c82ca9
Tweaks to the scalar adapters PR (#3811)
martinbonnin Jan 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
10 changes: 9 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,19 @@ Logging & Error messages
* There is one exception for the Gradle plugin. It is allowed to log information though the lifecycle() methods.
* Messages should contain "Apollo: " when it's not immediately clear that the message comes from Apollo.

Gradle APIs

Gradle is a bit peculiar because it can be used from both Kotlin and Groovy, has lazy and eager APIs and can sometimes be used as a DSL and sometimes imperatively. The rules we landed on are:

* Lazy properties use names. Example: `packageName.set("com.example")`
* Methods with one or more parameters use verbs. Example: `mapScalar("ID", "kotlin.Long")`
* Except when there is only one parameter that is of `Action<T>` type. Example: `introspection {}`

Misc
* Parameters using milliseconds should have the "Millis" suffix.
* Else use [kotlin.time.Duration]
* `ExperimentalContracts` is ok to use. Since kotlin-stdlib does it, we can too. See https://github.com/Kotlin/KEEP/blob/master/proposals/kotlin-contracts.md#compatibility-notice

## Workflow

We love Github issues! Before working on any new features, please open an issue so that we can agree on the direction,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

package com.apollographql.apollo3.api

import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
import com.apollographql.apollo3.annotations.ApolloDeprecatedSince.Version.v3_0_1
import com.apollographql.apollo3.annotations.ApolloInternal
import com.apollographql.apollo3.api.json.BufferedSinkJsonWriter
import com.apollographql.apollo3.api.json.writeAny
Expand Down Expand Up @@ -239,22 +241,32 @@ fun resolveVariables(value: Any?, variables: Executable.Variables): Any? {
}
}

@Deprecated("Use the generated CustomScalarType instead")
@ApolloDeprecatedSince(v3_0_1)
@SharedImmutable
@JvmField
val CompiledStringType = ScalarType("String")

@Deprecated("Use the generated CustomScalarType instead")
@ApolloDeprecatedSince(v3_0_1)
@SharedImmutable
@JvmField
val CompiledIntType = ScalarType("Int")

@Deprecated("Use the generated CustomScalarType instead")
@ApolloDeprecatedSince(v3_0_1)
@SharedImmutable
@JvmField
val CompiledFloatType = ScalarType("Float")

@Deprecated("Use the generated CustomScalarType instead")
@ApolloDeprecatedSince(v3_0_1)
@SharedImmutable
@JvmField
val CompiledBooleanType = ScalarType("Boolean")

@Deprecated("Use the generated CustomScalarType instead")
@ApolloDeprecatedSince(v3_0_1)
@SharedImmutable
@JvmField
val CompiledIDType = ScalarType("ID")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import okio.BufferedSink
* Use this to map your upload custom scalar and the apollo runtime will be able to extract them
* and send them out of band.
*
* customScalarsMapping.set(mapOf(
* "Upload" to "com.apollographql.apollo3.api.Upload"
* ))
* In your build.gradle file:
* ```
* mapScalarToUpload(Upload)
* ```
*
* If you have a JVM File at hand, see also [com.apollographql.apollo3.api.DefaultUpload.Builder.content]
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,20 @@ open class DefaultTestResolver : TestResolver {
}
}
is CustomScalarType -> {
resolveCustomScalar(path)
}
is CompiledNamedType -> {
when (compiledType.name) {
"Int" -> resolveInt(path)
"Float" -> resolveFloat(path)
"Boolean" -> resolveBoolean(path)
"String" -> resolveString(path)
"ID" -> resolveString(path)
else -> {
resolveComposite(path, ctors ?: error("no ctors for $responseName"))
resolveCustomScalar(path)
}
}
}
is CompiledNamedType -> {
resolveComposite(path, ctors ?: error("no ctors for $responseName"))
}
} as T
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ object ApolloCompiler {
"enclosing one.")
}

checkCustomScalars(schema, options.customScalarsMapping)
checkCustomScalars(schema, options.scalarMapping)

outputDir.deleteRecursively()
outputDir.mkdirs()
Expand Down Expand Up @@ -142,7 +142,7 @@ object ApolloCompiler {
fragmentDefinitions = fragments,
allFragmentDefinitions = allFragmentDefinitions,
alwaysGenerateTypesMatching = alwaysGenerateTypesMatching,
customScalarsMapping = options.customScalarsMapping,
scalarMapping = options.scalarMapping,
codegenModels = options.codegenModels,
generateOptionalOperationVariables = options.generateOptionalOperationVariables
).build()
Expand Down Expand Up @@ -186,7 +186,9 @@ object ApolloCompiler {
generateFragmentImplementations = options.generateFragmentImplementations,
generateQueryDocument = options.generateQueryDocument,
generateSchema = options.generateSchema,
generatedSchemaName = options.generatedSchemaName,
flatten = options.flattenModels,
scalarMapping = options.scalarMapping,
).write(outputDir = outputDir)
}
else -> {
Expand All @@ -203,22 +205,23 @@ object ApolloCompiler {
generateFragmentImplementations = options.generateFragmentImplementations,
generateQueryDocument = options.generateQueryDocument,
generateSchema = options.generateSchema,
generatedSchemaName = options.generatedSchemaName,
generateTestBuilders = options.generateTestBuilders,
flatten = options.flattenModels,
sealedClassesForEnumsMatching = options.sealedClassesForEnumsMatching,
targetLanguageVersion = options.targetLanguage,
scalarMapping = options.scalarMapping,
).write(outputDir = outputDir, testDir = testDir)
}
}


return CompilerMetadata(
fragments = fragments,
resolverInfo = outputResolverInfo,
)
}

private fun checkCustomScalars(schema: Schema, customScalarsMapping: Map<String, String>) {
private fun checkCustomScalars(schema: Schema, scalarMapping: Map<String, ScalarInfo>) {
/**
* Generate the mapping for all custom scalars
*
Expand All @@ -228,10 +231,9 @@ object ApolloCompiler {
.typeDefinitions
.values
.filterIsInstance<GQLScalarTypeDefinition>()
.filter { !it.isBuiltIn() }
.map { type -> type.name }
.toSet()
val unknownScalars = customScalarsMapping.keys.subtract(schemaScalars)
val unknownScalars = scalarMapping.keys.subtract(schemaScalars)
check(unknownScalars.isEmpty()) {
"Apollo: unknown custom scalar(s) in customScalarsMapping: ${unknownScalars.joinToString(",")}"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import com.apollographql.apollo3.ast.Schema
import com.apollographql.apollo3.ast.parseAsGQLDocument
import com.apollographql.apollo3.ast.toSchema
import com.apollographql.apollo3.ast.toUtf8
import com.apollographql.apollo3.ast.validateAsSchema
import com.apollographql.apollo3.compiler.codegen.ResolverInfo
import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.JsonClass
Expand Down Expand Up @@ -89,6 +88,10 @@ data class CommonMetadata(
val schemaPackageName: String,
val pluginVersion: String,
val codegenModels: String,
/**
* Scalar mapping needed for scalars' target types and Adapter initializers
*/
val scalarMapping: Map<String, ScalarInfo>,
)

/**
Expand All @@ -106,5 +109,5 @@ data class CompilerMetadata(
/**
* resolver info used by the codegen to lookup already existing ClassNames
*/
val resolverInfo: ResolverInfo
val resolverInfo: ResolverInfo,
)
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ class IncomingOptions(
val schemaPackageName: String,
) {
companion object {
fun fromMetadata(commonMetadata: CommonMetadata): IncomingOptions {
return IncomingOptions(
schema = commonMetadata.schema,
codegenModels = commonMetadata.codegenModels,
schemaPackageName = commonMetadata.schemaPackageName
)
}

@OptIn(ApolloExperimental::class)
fun resolveSchema(schemaFiles: Collection<File>, rootFolders: List<String>): Pair<Schema, String> {
check(schemaFiles.isNotEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import com.apollographql.apollo3.ast.Schema
import com.apollographql.apollo3.ast.toSchema
import com.apollographql.apollo3.compiler.introspection.toGQLDocument
import com.apollographql.apollo3.compiler.introspection.toSchema
import com.squareup.moshi.JsonClass
import dev.zacsweers.moshix.sealed.annotations.TypeLabel
import java.io.File


Expand Down Expand Up @@ -85,7 +87,7 @@ class Options(
val targetLanguage: TargetLanguage = defaultTargetLanguage,

//========== codegen options ============
val customScalarsMapping: Map<String, String> = defaultCustomScalarsMapping,
val scalarMapping: Map<String, ScalarInfo> = defaultScalarMapping,
val codegenModels: String = defaultCodegenModels,
val flattenModels: Boolean = defaultFlattenModels,
val useSemanticNaming: Boolean = defaultUseSemanticNaming,
Expand Down Expand Up @@ -121,12 +123,24 @@ class Options(
val generateQueryDocument: Boolean = defaultGenerateQueryDocument,

/**
* Whether to generate the __Schema class.
* Whether to generate the Schema class.
*
* __Schema is a special class that contains a list of all composite types (objects, interfaces, unions)
* It can be used to retrieve the list of possible types for a given CompiledType
* The Schema class is a special class that contains a list of all composite types (objects, interfaces, unions).
* It can be used to retrieve the list of possible types for a given CompiledType.
*
* Its name can be configured with [generatedSchemaName].
*
* Default: false
*/
val generateSchema: Boolean = defaultGenerateSchema,

/**
* Class name to use when generating the Schema class.
*
* Default: "__Schema"
*/
val generatedSchemaName: String = defaultGeneratedSchemaName,

/**
* Whether to generate the type safe Data builders. These are mainly used for tests but can also be used for other use
* cases too.
Expand Down Expand Up @@ -190,7 +204,7 @@ class Options(
alwaysGenerateTypesMatching: Set<String> = this.alwaysGenerateTypesMatching,
operationOutputGenerator: OperationOutputGenerator = this.operationOutputGenerator,
incomingCompilerMetadata: List<CompilerMetadata> = this.incomingCompilerMetadata,
customScalarsMapping: Map<String, String> = this.customScalarsMapping,
scalarMapping: Map<String, ScalarInfo> = this.scalarMapping,
codegenModels: String = this.codegenModels,
flattenModels: Boolean = this.flattenModels,
useSemanticNaming: Boolean = this.useSemanticNaming,
Expand All @@ -203,6 +217,7 @@ class Options(
generateResponseFields: Boolean = this.generateResponseFields,
generateQueryDocument: Boolean = this.generateQueryDocument,
generateSchema: Boolean = this.generateSchema,
generatedSchemaName: String = this.generatedSchemaName,
moduleName: String = this.moduleName,
targetLanguage: TargetLanguage = this.targetLanguage,
generateTestBuilders: Boolean = this.generateTestBuilders,
Expand All @@ -221,7 +236,7 @@ class Options(
operationOutputGenerator = operationOutputGenerator,
incomingCompilerMetadata = incomingCompilerMetadata,
targetLanguage = targetLanguage,
customScalarsMapping = customScalarsMapping,
scalarMapping = scalarMapping,
codegenModels = codegenModels,
flattenModels = flattenModels,
useSemanticNaming = useSemanticNaming,
Expand All @@ -234,17 +249,18 @@ class Options(
generateResponseFields = generateResponseFields,
generateQueryDocument = generateQueryDocument,
generateSchema = generateSchema,
generatedSchemaName = generatedSchemaName,
moduleName = moduleName,
generateTestBuilders = generateTestBuilders,
testDir = testDir,
sealedClassesForEnumsMatching = sealedClassesForEnumsMatching,
sealedClassesForEnumsMatching = sealedClassesForEnumsMatching,
generateOptionalOperationVariables = generateOptionalOperationVariables,
)

companion object {
val defaultAlwaysGenerateTypesMatching = emptySet<String>()
val defaultOperationOutputGenerator = OperationOutputGenerator.Default(OperationIdGenerator.Sha256)
val defaultCustomScalarsMapping = emptyMap<String, String>()
val defaultScalarMapping = emptyMap<String, ScalarInfo>()
val defaultLogger = ApolloCompiler.NoOpLogger
const val defaultUseSemanticNaming = true
const val defaultWarnOnDeprecatedUsages = true
Expand All @@ -260,10 +276,34 @@ class Options(
const val defaultFlattenModels = true
val defaultTargetLanguage = TargetLanguage.KOTLIN_1_5
const val defaultGenerateSchema = false
const val defaultGeneratedSchemaName = "__Schema"
const val defaultGenerateTestBuilders = false
val defaultSealedClassesForEnumsMatching = emptyList<String>()
const val defaultGenerateOptionalOperationVariables = true
const val defaultUseSchemaPackageNameForFragments = false
}
}

/**
* Controls how scalar adapters are used in the generated code.
*/
@JsonClass(generateAdapter = true, generator = "sealed:type")
sealed interface AdapterInitializer

/**
* The adapter expression will be used as-is (can be an object, a public val, a class instantiation).
*
* e.g. `"com.example.MyAdapter"` or `"com.example.MyAdapter()"`.
*/
@TypeLabel("Expression")
@JsonClass(generateAdapter = true)
class ExpressionAdapterInitializer(val expression: String) : AdapterInitializer

/**
* The adapter instance will be looked up in the [com.apollographql.apollo3.api.CustomScalarAdapters] provided at runtime.
*/
@TypeLabel("Runtime")
object RuntimeAdapterInitializer : AdapterInitializer

@JsonClass(generateAdapter = true)
data class ScalarInfo(val targetName: String, val adapterInitializer: AdapterInitializer = RuntimeAdapterInitializer)
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ abstract class CodegenLayout(
// variables keep the same case as their declared name
internal fun variableName(name: String) = regularIdentifier(name)
internal fun propertyName(name: String) = regularIdentifier(name)
internal fun schemaName() = "__Schema"

// ------------------------ Helpers ---------------------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ enum class ResolverKeyKind {
Fragment,
FragmentVariablesAdapter,
FragmentSelections,
/**
* CustomScalarTarget is a special case as there is no real generated reference pointing to it.
* Instead it is a String in [CustomScalarType.className]
*/
CustomScalarTarget,
TestBuilder
}

Expand All @@ -47,4 +42,4 @@ class ResolverInfo(
val magic: String,
val version: String,
val entries: List<ResolverEntry>
)
)
Loading