Navigation Menu

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

Fix #882, variable input type mismatch throws AssertException #883

Merged
merged 1 commit into from Jan 9, 2018

Conversation

tinnou
Copy link
Contributor

@tinnou tinnou commented Jan 5, 2018

Description

Added a null check in getTypeFromAST, similarly to what graphql-js does:
They return void (undefined in js) or the type itself.

export function typeFromAST(schema, typeNode) {
  /* eslint-enable no-redeclare */
  let innerType;
  if (typeNode.kind === Kind.LIST_TYPE) {
    innerType = typeFromAST(schema, typeNode.type);
    return innerType && GraphQLList(innerType);
  }
  if (typeNode.kind === Kind.NON_NULL_TYPE) {
    innerType = typeFromAST(schema, typeNode.type);
    return innerType && GraphQLNonNull(innerType);
  }
  if (typeNode.kind === Kind.NAMED_TYPE) {
    return schema.getType(typeNode.name.value);
  }
  /* istanbul ignore next */
  throw new Error(`Unexpected type kind: ${(typeNode.kind: empty)}.`);
}

Testing

Added unit tests to cover this issue + bonus spec cases we missed earlier.

class SpecValidation573Test extends SpecValidationBase {

def '5.7.3 Variables Are Input Types - type mismatch (must be non-null)'() {
def query = """
Copy link
Member

Choose a reason for hiding this comment

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

If you want to use $ in a groovy multi line string, then just use ''' quotes instead. No need for quoting the $

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh neat I didn't know you could do that! Thanks for the merge by the way.

@bbakerman bbakerman merged commit 68f5975 into graphql-java:master Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants