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

[Ignore] add check for hydration arg type against actor field #408

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
package graphql.nadel.validation

import graphql.nadel.Service
import graphql.nadel.dsl.RemoteArgumentSource
import graphql.nadel.dsl.RemoteArgumentDefinition
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument
import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField
import graphql.nadel.dsl.UnderlyingServiceHydration
import graphql.nadel.enginekt.util.getFieldAt
import graphql.nadel.enginekt.util.isList
import graphql.nadel.enginekt.util.isNonNull
import graphql.nadel.enginekt.util.isNotWrapped
import graphql.nadel.enginekt.util.isWrapped
import graphql.nadel.enginekt.util.unwrapAll
import graphql.nadel.enginekt.util.unwrapNonNull
import graphql.nadel.enginekt.util.unwrapOne
import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField
import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument
import graphql.nadel.validation.NadelSchemaValidationError.FieldWithPolymorphicHydrationMustReturnAUnion
import graphql.nadel.validation.NadelSchemaValidationError.HydrationFieldMustBeNullable
import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType
import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorField
import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorService
import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource
Expand All @@ -26,8 +30,11 @@ import graphql.nadel.validation.util.NadelSchemaUtil.getUnderlyingType
import graphql.nadel.validation.util.NadelSchemaUtil.hasRename
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLFieldsContainer
import graphql.schema.GraphQLInputType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLOutputType
import graphql.schema.GraphQLSchema
import graphql.schema.GraphQLType
import graphql.schema.GraphQLUnionType
import graphql.schema.GraphQLUnmodifiedType

Expand Down Expand Up @@ -179,8 +186,7 @@ internal class NadelHydrationValidation(
argument = remoteArg.name,
)
} else {
val remoteArgSource = remoteArg.remoteArgumentSource
getRemoteArgErrors(parent, overallField, remoteArgSource)
getRemoteArgErrors(parent, overallField, remoteArg, actorField)
}
}

Expand All @@ -205,17 +211,25 @@ internal class NadelHydrationValidation(
}

private fun getRemoteArgErrors(
parent: NadelServiceSchemaElement,
overallField: GraphQLFieldDefinition,
remoteArgSource: RemoteArgumentSource,
parent: NadelServiceSchemaElement,
overallField: GraphQLFieldDefinition,
remoteArg: RemoteArgumentDefinition,
actorField: GraphQLFieldDefinition,
): NadelSchemaValidationError? {
val remoteArgSource = remoteArg.remoteArgumentSource
return when (remoteArgSource.sourceType) {
ObjectField -> {
val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!)
if (field == null) {
MissingHydrationFieldValueSource(parent, overallField, remoteArgSource)
} else {
// TODO: check argument type is correct
//check the input types match with hydration and actor fields
val actorArg = actorField.getArgument(remoteArg.name)
val fieldOutputType = field.type
val actorArgInputType = actorArg.type
if (!outputTypeMatchesInputType(fieldOutputType, actorArgInputType)) {
IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType, actorArgInputType)
}
null
}
}
Expand All @@ -233,4 +247,13 @@ internal class NadelHydrationValidation(
}
}
}

/*
* Checks the type of a hydration argument against the type of an actor field argument to see if they match
*
*/
private fun hydrationArgTypesMatch(outputType: GraphQLOutputType, inputType: GraphQLInputType): Boolean {
//TODO
return true
faizan-siddiqui marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import graphql.schema.GraphQLDirective
import graphql.schema.GraphQLEnumValueDefinition
import graphql.schema.GraphQLFieldDefinition
import graphql.schema.GraphQLInputObjectField
import graphql.schema.GraphQLInputType
import graphql.schema.GraphQLNamedSchemaElement
import graphql.schema.GraphQLNamedType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLOutputType
import graphql.schema.GraphQLType
import graphql.schema.GraphQLTypeUtil

Expand Down Expand Up @@ -321,6 +323,28 @@ sealed interface NadelSchemaValidationError {
override val subject = overallField
}

data class IncompatibleHydrationArgumentType(
val parentType: NadelServiceSchemaElement,
val overallField: GraphQLFieldDefinition,
val remoteArg: RemoteArgumentDefinition,
val outputFieldType: GraphQLOutputType,
val actorArgInputType: GraphQLInputType,
) : NadelSchemaValidationError {
val service: Service get() = parentType.service

override val message = run {
val hydrationArgName = remoteArg.name
val of = makeFieldCoordinates(parentType.overall.name, overallField.name)
val uf = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}"
val s = service.name
"Field $of tried to hydrate with argument $hydrationArgName using value from underlying field $uf from " +
faizan-siddiqui marked this conversation as resolved.
Show resolved Hide resolved
"service $s with an argument of $outputFieldType whereas the actor field requires an argument of" +
"type $actorArgInputType"
}

override val subject = overallField
}

data class MissingHydrationArgumentValueSource(
val parentType: NadelServiceSchemaElement,
val overallField: GraphQLFieldDefinition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -920,5 +920,59 @@ class NadelHydrationValidationTest : DescribeSpec({
assert(error.overallField.name == "name")
assert(error.subject == error.overallField)
}

it("fails if hydration argument types are mismatch with actor") {
val fixture = NadelValidationTestFixture(
overallSchema = mapOf(
"issues" to """
faizan-siddiqui marked this conversation as resolved.
Show resolved Hide resolved
type Query {
issue: JiraIssue
}
type JiraIssue @renamed(from: "Issue") {
id: ID!
creator: User @hydrated(
service: "users"
field: "user"
arguments: [
{name: "id", value: "$source.creator"}
]
)
}
""".trimIndent(),
"users" to """
type Query {
user(id: Int!): User
}
type User {
id: ID!
name: String!
}
""".trimIndent(),
),
underlyingSchema = mapOf(
"issues" to """
type Query {
issue: Issue
}
type Issue {
id: ID!
creator: ID!
}
""".trimIndent(),
"users" to """
type Query {
user(id: Int!): User
}
type User {
id: ID!
name: String!
}
""".trimIndent(),
),
)

val errors = validate(fixture)
assert(errors.map { it.message }.isEmpty())
}
}
})