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.

Sign up for GitHub

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

Refactor RemoteArgumentSource to be a sealed class #487

Merged
merged 2 commits into from
Jan 22, 2024
+165 −181
Merged

Refactor RemoteArgumentSource to be a sealed class #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fc306f
Refactor RemoteArgumentSource to be a sealed class
gnawf Dec 13, 2023
347ed87
Fix bug
gnawf Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions lib/src/main/java/graphql/nadel/dsl/RemoteArgumentSource.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package graphql.nadel.dsl

import graphql.language.Value
import graphql.nadel.util.AnyAstValue

// todo this should be a union or sealed class thing
data class RemoteArgumentSource(
val argumentName: String?, // for OBJECT_FIELD
val pathToField: List<String>?,
val staticValue: Value<*>?,
val sourceType: SourceType,
) {
enum class SourceType {
ObjectField,
FieldArgument,
StaticArgument
}
sealed class RemoteArgumentSource {
data class ObjectField(
val pathToField: List<String>,
) : RemoteArgumentSource()

data class FieldArgument(
val argumentName: String,
) : RemoteArgumentSource()

data class StaticArgument(
val staticValue: AnyAstValue,
) : RemoteArgumentSource()
}
20 changes: 9 additions & 11 deletions lib/src/main/java/graphql/nadel/engine/blueprint/NadelExecutionBlueprintFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import graphql.language.FieldDefinition
import graphql.language.ImplementingTypeDefinition
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.RemoteArgumentSource
import graphql.nadel.dsl.TypeMappingDefinition
import graphql.nadel.dsl.NadelHydrationDefinition
import graphql.nadel.engine.blueprint.hydration.NadelBatchHydrationMatchStrategy
Expand Down Expand Up @@ -510,9 +508,9 @@ private class Factory(
actorFieldDef: GraphQLFieldDefinition,
): List<NadelHydrationActorInputDef> {
return hydration.arguments.map { remoteArgDef ->
val valueSource = when (val argSourceType = remoteArgDef.remoteArgumentSource.sourceType) {
FieldArgument -> {
val argumentName = remoteArgDef.remoteArgumentSource.argumentName!!
val valueSource = when (val argSourceType = remoteArgDef.remoteArgumentSource) {
is RemoteArgumentSource.FieldArgument -> {
val argumentName = argSourceType.argumentName
val argumentDef = hydratedFieldDef.getArgument(argumentName)
?: error("No argument '$argumentName' on field ${hydratedFieldParentType.name}.${hydratedFieldDef.name}")
val defaultValue = if (argumentDef.argumentDefaultValue.isLiteral) {
Expand All @@ -525,23 +523,23 @@ private class Factory(
}

NadelHydrationActorInputDef.ValueSource.ArgumentValue(
argumentName = argumentName,
argumentName = argSourceType.argumentName,
argumentDefinition = argumentDef,
defaultValue = defaultValue,
)
}
ObjectField -> {
val pathToField = remoteArgDef.remoteArgumentSource.pathToField!!
is RemoteArgumentSource.ObjectField -> {
val pathToField = argSourceType.pathToField
FieldResultValue(
queryPathToField = NadelQueryPath(pathToField),
fieldDefinition = getUnderlyingType(hydratedFieldParentType)
?.getFieldAt(pathToField)
?: error("No field defined at: ${hydratedFieldParentType.name}.${pathToField.joinToString(".")}"),
)
}
StaticArgument -> {
is RemoteArgumentSource.StaticArgument -> {
NadelHydrationActorInputDef.ValueSource.StaticValue(
value = remoteArgDef.remoteArgumentSource.staticValue!!
value = argSourceType.staticValue,
)
}
}
Expand Down
70 changes: 16 additions & 54 deletions lib/src/main/java/graphql/nadel/schema/NadelDirectives.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import graphql.language.InputObjectTypeDefinition
import graphql.language.ObjectValue
import graphql.language.SDLDefinition
import graphql.language.StringValue
import graphql.language.TypeDefinition
import graphql.language.Value
import graphql.nadel.dsl.FieldMappingDefinition
import graphql.nadel.dsl.RemoteArgumentDefinition
import graphql.nadel.dsl.RemoteArgumentSource
import graphql.nadel.dsl.RemoteArgumentSource.SourceType
import graphql.nadel.dsl.TypeMappingDefinition
import graphql.nadel.dsl.NadelHydrationDefinition
import graphql.nadel.dsl.NadelHydrationConditionDefinition
Expand Down Expand Up @@ -328,37 +328,20 @@ object NadelDirectives {
}

private fun createRemoteArgumentSource(value: Value<*>): RemoteArgumentSource {
if (value is StringValue) {
val values = listFromDottedString(value.value)
return when (values.first()) {
"\$source" -> RemoteArgumentSource(
argumentName = null,
return if (value is StringValue) {
val values = value.value.split('.')

when (values.first()) {
"\$source" -> RemoteArgumentSource.ObjectField(
pathToField = values.subList(1, values.size),
staticValue = null,
sourceType = SourceType.ObjectField,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need to fill in the other values because we create just RemoteArgumentSource.ObjectField now.

)

"\$argument" -> RemoteArgumentSource(
"\$argument" -> RemoteArgumentSource.FieldArgument(
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,
)
else -> RemoteArgumentSource.StaticArgument(staticValue = value)
}
} else {
return RemoteArgumentSource(
argumentName = null,
pathToField = null,
staticValue = value,
sourceType = SourceType.StaticArgument,
)
RemoteArgumentSource.StaticArgument(staticValue = value)
}
}

Expand All @@ -380,37 +363,20 @@ object NadelDirectives {

val remoteArgumentSource = if (remoteArgFieldValue != null && remoteArgArgValue != null) {
throw IllegalArgumentException("$inputObjectTypeName can not have both $valueFromFieldKey and $valueFromArgKey set")
} else if (remoteArgFieldValue != null) {
createTemplatedRemoteArgumentSource(remoteArgFieldValue, SourceType.ObjectField)
} else if (remoteArgArgValue != null) {
createTemplatedRemoteArgumentSource(remoteArgArgValue, SourceType.FieldArgument)
} else {
throw IllegalArgumentException("$inputObjectTypeName requires one of $valueFromFieldKey or $valueFromArgKey to be set")
if (remoteArgFieldValue != null) {
RemoteArgumentSource.ObjectField(remoteArgFieldValue.removePrefix("\$source.").split('.'))
} else if (remoteArgArgValue != null) {
RemoteArgumentSource.FieldArgument(remoteArgArgValue.removePrefix("\$argument."))
} else {
throw IllegalArgumentException("$inputObjectTypeName requires one of $valueFromFieldKey or $valueFromArgKey to be set")
}
}

RemoteArgumentDefinition(remoteArgName, remoteArgumentSource)
}
}

private fun createTemplatedRemoteArgumentSource(value: String, argumentType: SourceType): RemoteArgumentSource {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Inlined this function, it's much simpler now with the sealed classes.

// for backwards compat reasons - we will allow them to specify "$source.field.name" and treat it as just "field.name"
val values = value
.removePrefix("\$source.")
.removePrefix("\$argument.")
.split('.')

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

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

internal fun createFieldMapping(fieldDefinition: GraphQLFieldDefinition): FieldMappingDefinition? {
val directive = fieldDefinition.getAppliedDirective(renamedDirectiveDefinition.name)
?: return null
Expand All @@ -427,10 +393,6 @@ object NadelDirectives {
return TypeMappingDefinition(underlyingName = from, overallName = directivesContainer.name)
}

private fun listFromDottedString(from: String): List<String> {
return from.split('.').toList()
}

private inline fun <reified T : Any> getDirectiveValue(
directive: GraphQLAppliedDirective,
name: String,
Expand Down
53 changes: 32 additions & 21 deletions lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
package graphql.nadel.validation

import graphql.Scalars
import graphql.nadel.dsl.RemoteArgumentDefinition
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument
import graphql.nadel.dsl.NadelHydrationDefinition
import graphql.nadel.dsl.RemoteArgumentDefinition
import graphql.nadel.dsl.RemoteArgumentSource
import graphql.nadel.engine.util.isList
import graphql.nadel.engine.util.isNonNull
import graphql.nadel.engine.util.unwrapNonNull
import graphql.nadel.engine.util.unwrapOne
import graphql.scalars.ExtendedScalars
import graphql.schema.*
import graphql.schema.GraphQLEnumType
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLInputObjectType
import graphql.schema.GraphQLInputType
import graphql.schema.GraphQLNamedInputType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLScalarType
import graphql.schema.GraphQLType

internal class NadelHydrationArgumentValidation() {
internal class NadelHydrationArgumentValidation {
fun validateHydrationInputArg(
hydrationSourceType: GraphQLType,
actorFieldArgType: GraphQLInputType,
Expand All @@ -28,7 +34,7 @@ internal class NadelHydrationArgumentValidation() {

//could have ID feed into [ID] (as a possible batch hydration case).
// in this case we need to unwrap the list and check types
if (isBatchHydration && (!unwrappedHydrationSourceType.isList && unwrappedActorFieldArgType.isList)) {
return if (isBatchHydration && (!unwrappedHydrationSourceType.isList && unwrappedActorFieldArgType.isList)) {
val error = getHydrationInputErrors(
unwrappedHydrationSourceType,
unwrappedActorFieldArgType.unwrapOne(),
Expand All @@ -38,15 +44,18 @@ internal class NadelHydrationArgumentValidation() {
hydration,
actorFieldName
)

if (error != null) {
return NadelSchemaValidationError.IncompatibleHydrationArgumentType(
NadelSchemaValidationError.IncompatibleHydrationArgumentType(
parent,
overallField,
remoteArg,
hydrationSourceType,
actorFieldArgType,
actorFieldName
)
} else {
null
}
}
//could have [ID] feed into ID (non-batch, ManyToOne case explained in NadelHydrationStrategy.kt)
Expand All @@ -62,14 +71,16 @@ internal class NadelHydrationArgumentValidation() {
actorFieldName
)
if (error != null) {
return NadelSchemaValidationError.IncompatibleHydrationArgumentType(
NadelSchemaValidationError.IncompatibleHydrationArgumentType(
parent,
overallField,
remoteArg,
hydrationSourceType,
actorFieldArgType,
actorFieldName
)
} else {
null
}
}
// Otherwise we can just check the types normally
Expand All @@ -84,7 +95,6 @@ internal class NadelHydrationArgumentValidation() {
actorFieldName
)
}
return null
}

private fun getHydrationInputErrors(
Expand All @@ -96,10 +106,9 @@ internal class NadelHydrationArgumentValidation() {
hydration: NadelHydrationDefinition,
actorFieldName: String,
): NadelSchemaValidationError? {

//need to check null compatibility
val sourceType = remoteArg.remoteArgumentSource.sourceType
if (sourceType != ObjectField && actorFieldArgType.isNonNull && !hydrationSourceType.isNonNull) {
// need to check null compatibility
val remoteArgumentSource = remoteArg.remoteArgumentSource
if (remoteArgumentSource !is RemoteArgumentSource.ObjectField && actorFieldArgType.isNonNull && !hydrationSourceType.isNonNull) {
// source must be at least as strict as field argument
return NadelSchemaValidationError.IncompatibleHydrationArgumentType(
parent,
Expand Down Expand Up @@ -136,7 +145,7 @@ internal class NadelHydrationArgumentValidation() {
)
}
// object feed into inputObject (i.e. hydrating with a $source object)
else if (sourceType == ObjectField && unwrappedHydrationSourceType is GraphQLObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) {
else if (remoteArgumentSource is RemoteArgumentSource.ObjectField && unwrappedHydrationSourceType is GraphQLObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) {
validateInputObjectArg(
unwrappedHydrationSourceType,
unwrappedActorFieldArgType,
Expand All @@ -148,7 +157,7 @@ internal class NadelHydrationArgumentValidation() {
)
}
// inputObject feed into inputObject (i.e. hydrating with an $argument object)
else if (sourceType == FieldArgument && unwrappedHydrationSourceType is GraphQLInputObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) {
else if (remoteArgumentSource is RemoteArgumentSource.FieldArgument && unwrappedHydrationSourceType is GraphQLInputObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) {
if (unwrappedHydrationSourceType.name != unwrappedActorFieldArgType.name) {
NadelSchemaValidationError.IncompatibleHydrationArgumentType(
parent,
Expand Down Expand Up @@ -200,11 +209,12 @@ internal class NadelHydrationArgumentValidation() {
): NadelSchemaValidationError? {
val unwrappedHydrationSourceType = hydrationSourceType.unwrapNonNull()
val unwrappedActorFieldArgType = actorFieldArgType.unwrapNonNull()
if (unwrappedHydrationSourceType is GraphQLScalarType && unwrappedActorFieldArgType is GraphQLScalarType) {

return if (unwrappedHydrationSourceType is GraphQLScalarType && unwrappedActorFieldArgType is GraphQLScalarType) {
if (isScalarAssignable(unwrappedHydrationSourceType, unwrappedActorFieldArgType)) {
return null
null
} else {
return NadelSchemaValidationError.IncompatibleHydrationArgumentType(
NadelSchemaValidationError.IncompatibleHydrationArgumentType(
parent,
overallField,
remoteArg,
Expand All @@ -213,8 +223,9 @@ internal class NadelHydrationArgumentValidation() {
actorFieldName
)
}
} else {
null
}
return null
}

private fun isScalarAssignable(typeToAssign: GraphQLNamedInputType, targetType: GraphQLNamedInputType): Boolean {
Expand Down Expand Up @@ -242,8 +253,8 @@ internal class NadelHydrationArgumentValidation() {
hydration: NadelHydrationDefinition,
actorFieldName: String,
): NadelSchemaValidationError? {
var hydrationSourceFieldInnerType: GraphQLType = hydrationSourceType.unwrapNonNull().unwrapOne()
var actorFieldArgInnerType: GraphQLType = actorFieldArgType.unwrapNonNull().unwrapOne()
val hydrationSourceFieldInnerType: GraphQLType = hydrationSourceType.unwrapNonNull().unwrapOne()
val actorFieldArgInnerType: GraphQLType = actorFieldArgType.unwrapNonNull().unwrapOne()
val errorExists = getHydrationInputErrors(
hydrationSourceFieldInnerType,
actorFieldArgInnerType,
Expand Down
5 changes: 4 additions & 1 deletion lib/src/main/java/graphql/nadel/validation/NadelHydrationConditionValidation.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package graphql.nadel.validation
import graphql.Scalars
import graphql.nadel.dsl.NadelHydrationDefinition
import graphql.nadel.dsl.NadelHydrationResultConditionDefinition
import graphql.nadel.dsl.RemoteArgumentSource
import graphql.nadel.engine.util.getFieldAt
import graphql.nadel.engine.util.unwrapAll
import graphql.nadel.engine.util.unwrapNonNull
Expand Down Expand Up @@ -32,7 +33,9 @@ internal class NadelHydrationConditionValidation {

val sourceInputField = hydration.arguments
.asSequence()
.mapNotNull { it.remoteArgumentSource.pathToField }
.map { it.remoteArgumentSource }
.filterIsInstance<RemoteArgumentSource.ObjectField>()
.map { it.pathToField }
.single()

val conditionSourceFieldType = if (pathToConditionSourceField == sourceInputField) {
Expand Down
Loading
Loading