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

GQLGW-2564: Static Args Work #461

Merged
merged 15 commits into from
Oct 11, 2023
3 changes: 3 additions & 0 deletions lib/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ val slf4jVersion = "1.7.25"
dependencies {
api("com.graphql-java:graphql-java:$graphqlJavaVersion")
implementation("org.slf4j:slf4j-api:$slf4jVersion")
implementation("com.graphql-java:graphql-java-extended-scalars:21.0") {
exclude("com.graphql-java", "graphql-java")
}

api(kotlin("stdlib"))
api(kotlin("reflect"))
Expand Down
6 changes: 5 additions & 1 deletion lib/src/main/java/graphql/nadel/dsl/RemoteArgumentSource.kt
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package graphql.nadel.dsl

import graphql.language.Value

// todo this should be a union or sealed class thing
data class RemoteArgumentSource(
val argumentName: String?, // for OBJECT_FIELD
val pathToField: List<String>?,
val sourceType: SourceType?,
val staticValue: Value<*>?,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have to do it for this PR - but I think this well become just astValue when we do the mix or static or dynamic work later

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

val sourceType: SourceType,
) {
enum class SourceType {
ObjectField,
FieldArgument,
StaticArgument
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import graphql.Scalars.GraphQLString
import graphql.language.EnumTypeDefinition
import graphql.language.FieldDefinition
import graphql.language.ImplementingTypeDefinition
import graphql.language.StringValue
import graphql.nadel.Service
import graphql.nadel.dsl.FieldMappingDefinition
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.StaticArgument
import graphql.nadel.dsl.TypeMappingDefinition
import graphql.nadel.dsl.UnderlyingServiceHydration
import graphql.nadel.engine.blueprint.hydration.NadelBatchHydrationMatchStrategy
Expand Down Expand Up @@ -260,6 +262,7 @@ private class Factory(
when (it.valueSource) {
is NadelHydrationActorInputDef.ValueSource.ArgumentValue -> null
is FieldResultValue -> it.valueSource.queryPathToField
is NadelHydrationActorInputDef.ValueSource.StaticValue -> null
}
},
)
Expand Down Expand Up @@ -358,6 +361,7 @@ private class Factory(
when (val hydrationValueSource: NadelHydrationActorInputDef.ValueSource = it.valueSource) {
is NadelHydrationActorInputDef.ValueSource.ArgumentValue -> emptyList()
is FieldResultValue -> selectSourceFieldQueryPaths(hydrationValueSource)
is NadelHydrationActorInputDef.ValueSource.StaticValue -> emptyList()
}
}).toSet()

Expand Down Expand Up @@ -483,7 +487,11 @@ private class Factory(
?: error("No field defined at: ${hydratedFieldParentType.name}.${pathToField.joinToString(".")}"),
)
}
else -> error("Unsupported remote argument source type: '$argSourceType'")
StaticArgument -> {
NadelHydrationActorInputDef.ValueSource.StaticValue(
value = remoteArgDef.remoteArgumentSource.staticValue!!
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again later the naming here is likely to be AstArgument and NadelHydrationActorInputDef.ValueSource.AstValue

}

NadelHydrationActorInputDef(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.nadel.engine.blueprint.hydration

import graphql.language.Value
import graphql.nadel.engine.transform.query.NadelQueryPath
import graphql.normalized.NormalizedInputValue
import graphql.schema.GraphQLArgument
Expand Down Expand Up @@ -44,6 +45,23 @@ data class NadelHydrationActorInputDef(
val argumentDefinition: GraphQLArgument,
val defaultValue: NormalizedInputValue?,
) : ValueSource()

/**
* Represents a static argument value, which is hardcoded in the source code. e.g.
*
* ```graphql
* type Issue {
* id: ID! # Value used as argument to issueId
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
* owner: User @hydrated(from: ["issueOwner"], args: [
* {name: "issueId" value: "issue123"}
* ])
* }
* ```
*/
data class StaticValue (
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
val value: Value<*>,
): ValueSource()

sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package graphql.nadel.engine.transform.hydration

import graphql.language.NullValue
import graphql.language.StringValue
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
import graphql.language.Value
import graphql.nadel.engine.blueprint.NadelHydrationFieldInstruction
import graphql.nadel.engine.blueprint.hydration.NadelHydrationActorInputDef
import graphql.nadel.engine.blueprint.hydration.NadelHydrationActorInputDef.ValueSource
import graphql.nadel.engine.blueprint.hydration.NadelHydrationStrategy
import graphql.nadel.engine.transform.artificial.NadelAliasHelper
import graphql.nadel.engine.transform.result.json.JsonNode
import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
import graphql.nadel.engine.util.AnyAstValue
import graphql.nadel.engine.util.emptyOrSingle
import graphql.nadel.engine.util.flatten
import graphql.nadel.engine.util.javaValueToAstValue
Expand Down Expand Up @@ -127,6 +130,7 @@ internal class NadelHydrationInputBuilder private constructor(
inputDef,
value = getResultValue(valueSource),
)
is ValueSource.StaticValue -> makeInputValue(inputDef, valueSource.value)
}
}

Expand All @@ -140,6 +144,16 @@ internal class NadelHydrationInputBuilder private constructor(
)
}

private fun makeInputValue(
inputDef: NadelHydrationActorInputDef,
value: Value<*>,
): NormalizedInputValue {
return makeNormalizedInputValue(
type = inputDef.actorArgumentDef.type,
value = value,
)
}

private fun getArgumentValue(
valueSource: ValueSource.ArgumentValue,
): NormalizedInputValue? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
import graphql.nadel.engine.util.emptyOrSingle
import graphql.nadel.engine.util.flatten
import graphql.nadel.engine.util.javaValueToAstValue
import graphql.nadel.engine.util.makeNormalizedInputValue
import graphql.nadel.engine.util.mapFrom
import graphql.nadel.hooks.ServiceExecutionHooks
import graphql.normalized.ExecutableNormalizedField
Expand Down Expand Up @@ -44,7 +45,7 @@ internal object NadelBatchHydrationInputBuilder {
instruction.actorInputValueDefs.mapNotNull { actorFieldArg ->
when (val valueSource = actorFieldArg.valueSource) {
is NadelHydrationActorInputDef.ValueSource.ArgumentValue -> {
val argValue = hydrationField.normalizedArguments[valueSource.argumentName]
val argValue: NormalizedInputValue? = hydrationField.normalizedArguments[valueSource.argumentName]
?: valueSource.defaultValue
if (argValue != null) {
actorFieldArg to argValue
Expand All @@ -54,8 +55,15 @@ internal object NadelBatchHydrationInputBuilder {
}
// These are batch values, ignore them
is NadelHydrationActorInputDef.ValueSource.FieldResultValue -> null
is NadelHydrationActorInputDef.ValueSource.StaticValue -> {
val staticValue: NormalizedInputValue = makeNormalizedInputValue(
type = actorFieldArg.actorArgumentDef.type,
value = valueSource.value,
)
actorFieldArg to staticValue
}
}
},
}
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
67 changes: 45 additions & 22 deletions lib/src/main/java/graphql/nadel/schema/NadelDirectives.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.nadel.schema

import graphql.scalars.ExtendedScalars
import graphql.GraphQLContext
import graphql.Scalars
import graphql.Scalars.GraphQLString
Expand All @@ -10,7 +11,9 @@ import graphql.language.DirectiveDefinition.newDirectiveDefinition
import graphql.language.EnumTypeDefinition.newEnumTypeDefinition
import graphql.language.EnumValueDefinition.newEnumValueDefinition
import graphql.language.InputObjectTypeDefinition.newInputObjectDefinition
import graphql.language.ObjectValue
import graphql.language.StringValue
import graphql.language.Value
import graphql.nadel.dsl.FieldMappingDefinition
import graphql.nadel.dsl.RemoteArgumentDefinition
import graphql.nadel.dsl.RemoteArgumentSource
Expand Down Expand Up @@ -72,7 +75,7 @@ object NadelDirectives {
)
.inputValueDefinition(
name = "value",
type = nonNull(GraphQLString),
type = nonNull(ExtendedScalars.Json),

This comment was marked as resolved.

Copy link
Collaborator Author

@sbarker2 sbarker2 Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, this used to only have the options "$source.blah" or "$args.blah" which were always a string.
We only need it now because the static types could be other types (mentioned in description of PR)

)
.build()

Expand Down Expand Up @@ -251,8 +254,7 @@ object NadelDirectives {
val hydrations = fieldDefinition.getAppliedDirectives(hydratedDirectiveDefinition.name)
.asSequence()
.map { directive ->
val argumentValues = resolveArgumentValue<List<Any>>(directive.getArgument("arguments"))
val arguments = createRemoteArgs(argumentValues)
val arguments = createRemoteArgs(directive.getArgument("arguments").argumentValue.value as ArrayValue)

val inputIdentifiedBy = directive.getArgument("inputIdentifiedBy")
val identifiedByValues = resolveArgumentValue<List<Any>>(inputIdentifiedBy)
Expand Down Expand Up @@ -330,19 +332,19 @@ object NadelDirectives {
)
}

private fun createRemoteArgs(arguments: List<Any>): List<RemoteArgumentDefinition> {
private fun createRemoteArgs(arguments: ArrayValue): List<RemoteArgumentDefinition> {
fun Map<String, String>.requireArgument(key: String): String {
return requireNotNull(this[key]) {
"${nadelHydrationArgumentDefinition.name} definition requires '$key' to be not-null"
}
}

return arguments
return arguments.values
.map { arg ->
@Suppress("UNCHECKED_CAST") // trust GraphQL type system and caller
val argMap = arg as Map<String, String>
val remoteArgName = argMap.requireArgument("name")
val remoteArgValue = argMap.requireArgument("value")
val argMap = arg as ObjectValue
val remoteArgName = (argMap.objectFields.single { it.name == "name" }.value as StringValue).value
val remoteArgValue = argMap.objectFields.single { it.name == "value" }.value
val remoteArgumentSource = createRemoteArgumentSource(remoteArgValue)
RemoteArgumentDefinition(remoteArgName, remoteArgumentSource)
}
Expand All @@ -363,21 +365,40 @@ object NadelDirectives {
}
}

private fun createRemoteArgumentSource(value: String): RemoteArgumentSource {
val values = listFromDottedString(value)

return when (values.first()) {
"\$source" -> RemoteArgumentSource(
private fun createRemoteArgumentSource(value: Value<*>): RemoteArgumentSource {
if (value is StringValue) {
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
val values = listFromDottedString(value.value)
return when (values.first()) {
"\$source" -> RemoteArgumentSource(
argumentName = null,
pathToField = values.subList(1, values.size),
staticValue = null,
sourceType = SourceType.ObjectField,
)

"\$argument" -> RemoteArgumentSource(
argumentName = values.subList(1, values.size).single(),
pathToField = null,
staticValue = null,
sourceType = SourceType.FieldArgument,
)

else ->
RemoteArgumentSource(
argumentName = null,
pathToField = null,
staticValue = value,
sourceType = SourceType.StaticArgument,
)
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
}
}
else {
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
return RemoteArgumentSource(
argumentName = null,
pathToField = values.subList(1, values.size),
sourceType = SourceType.ObjectField,
)
"\$argument" -> RemoteArgumentSource(
argumentName = values.subList(1, values.size).single(),
pathToField = null,
sourceType = SourceType.FieldArgument,
)
else -> throw IllegalArgumentException("$value must begin with \$source. or \$argument.")
staticValue = value,
sourceType = SourceType.StaticArgument,
)
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -420,12 +441,14 @@ object NadelDirectives {

var argumentName: String? = null
var path: List<String>? = null
var value: Value<*>? = null
when (argumentType) {
SourceType.ObjectField -> path = values
SourceType.FieldArgument -> argumentName = values.single()
SourceType.StaticArgument -> value = value
}

return RemoteArgumentSource(argumentName, path, argumentType)
return RemoteArgumentSource(argumentName, path, value, argumentType)
}

fun createFieldMapping(fieldDefinition: GraphQLFieldDefinition): FieldMappingDefinition? {
Expand Down
4 changes: 4 additions & 0 deletions lib/src/main/java/graphql/nadel/schema/NeverWiringFactory.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.nadel.schema

import graphql.Assert.assertShouldNeverHappen
import graphql.scalars.ExtendedScalars
import graphql.schema.Coercing
import graphql.schema.DataFetcher
import graphql.schema.GraphQLScalarType
Expand All @@ -24,6 +25,9 @@ open class NeverWiringFactory : WiringFactory {

override fun getScalar(environment: ScalarWiringEnvironment): GraphQLScalarType? {
val scalarName = environment.scalarTypeDefinition.name
if (scalarName == ExtendedScalars.Json.name){
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
return ExtendedScalars.Json
}
return GraphQLScalarType
.newScalar()
.name(scalarName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import graphql.GraphQLException
import graphql.language.FieldDefinition
import graphql.language.ObjectTypeDefinition
import graphql.language.ObjectTypeDefinition.newObjectTypeDefinition
import graphql.language.ScalarTypeDefinition.newScalarTypeDefinition
import graphql.language.SchemaDefinition
import graphql.language.SourceLocation
import graphql.nadel.NadelDefinitionRegistry
Expand All @@ -12,6 +13,7 @@ import graphql.nadel.util.AnyNamedNode
import graphql.nadel.util.AnySDLDefinition
import graphql.nadel.util.AnySDLNamedDefinition
import graphql.nadel.util.isExtensionDef
import graphql.scalars.ExtendedScalars
import graphql.schema.GraphQLSchema
import graphql.schema.idl.RuntimeWiring
import graphql.schema.idl.SchemaGenerator
Expand Down Expand Up @@ -70,6 +72,9 @@ internal class OverallSchemaGenerator {
addIfNotPresent(overallRegistry, allDefinitions, NadelDirectives.nadelHydrationTemplateEnumDefinition)
addIfNotPresent(overallRegistry, allDefinitions, NadelDirectives.hydratedFromDirectiveDefinition)
addIfNotPresent(overallRegistry, allDefinitions, NadelDirectives.hydratedTemplateDirectiveDefinition)
addIfNotPresent(overallRegistry, allDefinitions, newScalarTypeDefinition()
.name(ExtendedScalars.Json.name)
.build())

for (definition in allDefinitions) {
val error = overallRegistry.add(definition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgum
import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource
import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument
import graphql.nadel.validation.NadelSchemaValidationError.MultipleSourceArgsInBatchHydration
import graphql.nadel.validation.NadelSchemaValidationError.NoSourceArgsInBatchHydration
import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument
import graphql.nadel.validation.util.NadelSchemaUtil.getHydrations
import graphql.nadel.validation.util.NadelSchemaUtil.hasRename
Expand Down Expand Up @@ -193,6 +194,8 @@ internal class NadelHydrationValidation(
when {
numberOfSourceArgs > 1 ->
listOf(MultipleSourceArgsInBatchHydration(parent, overallField))
numberOfSourceArgs == 0 ->
listOf(NoSourceArgsInBatchHydration(parent, overallField))

else -> emptyList()
}
Expand Down Expand Up @@ -234,4 +237,4 @@ internal class NadelHydrationValidation(
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,20 @@ sealed interface NadelSchemaValidationError {
override val subject = overallField
}

data class NoSourceArgsInBatchHydration(
val parentType: NadelServiceSchemaElement,
val overallField: GraphQLFieldDefinition,
sbarker2 marked this conversation as resolved.
Show resolved Hide resolved
) : NadelSchemaValidationError {
val service: Service get() = parentType.service

override val message = run {
val fieldCoordinates = makeFieldCoordinates(parentType.overall.name, overallField.name)
"No \$source.xxx arguments for batch hydration. Field: $fieldCoordinates"
}

override val subject = overallField
}

data class MissingArgumentOnUnderlying(
val parentType: NadelServiceSchemaElement,
val overallField: GraphQLFieldDefinition,
Expand Down
Loading
Loading