diff --git a/.github/workflows/invoke_test_runner.yml b/.github/workflows/invoke_test_runner.yml index d2d495fd42..f30e105394 100644 --- a/.github/workflows/invoke_test_runner.yml +++ b/.github/workflows/invoke_test_runner.yml @@ -34,7 +34,7 @@ jobs: if: github.repository == 'graphql-java/graphql-java' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: actions/setup-node@v3 with: node-version: '14' diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index deb7678d26..975b85c197 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -13,7 +13,7 @@ jobs: MAVEN_CENTRAL_PGP_KEY: ${{ secrets.MAVEN_CENTRAL_PGP_KEY }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: gradle/wrapper-validation-action@v1 - name: Set up JDK 11 uses: actions/setup-java@v3 diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 006a2c915d..b361a48d8f 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -15,7 +15,7 @@ jobs: buildAndTest: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: gradle/wrapper-validation-action@v1 - name: Set up JDK 11 uses: actions/setup-java@v3 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6de60e8c86..0d1b866047 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ jobs: RELEASE_VERSION: ${{ github.event.inputs.version }} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: gradle/wrapper-validation-action@v1 - name: Set up JDK 11 uses: actions/setup-java@v3 diff --git a/README.md b/README.md index 1d68fdf6bb..5a68fa5ba7 100644 --- a/README.md +++ b/README.md @@ -5,15 +5,17 @@ Discuss and ask questions in our Discussions: https://github.com/graphql-java/gr This is a [GraphQL](https://github.com/graphql/graphql-spec) Java implementation. [![Build](https://github.com/graphql-java/graphql-java/actions/workflows/master.yml/badge.svg)](https://github.com/graphql-java/graphql-java/actions/workflows/master.yml) -[![Latest Release](https://img.shields.io/maven-central/v/com.graphql-java/graphql-java?versionPrefix=20.)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/graphql-java/) +[![Latest Release](https://img.shields.io/maven-central/v/com.graphql-java/graphql-java?versionPrefix=21.)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/graphql-java/) [![Latest Snapshot](https://img.shields.io/maven-central/v/com.graphql-java/graphql-java?label=maven-central%20snapshot&versionPrefix=0)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/graphql-java/) [![MIT licensed](https://img.shields.io/badge/license-MIT-green)](https://github.com/graphql-java/graphql-java/blob/master/LICENSE.md) ### Documentation -We have a tutorial for beginners: [Getting started with GraphQL Java and Spring Boot](https://www.graphql-java.com/tutorials/getting-started-with-spring-boot/) +See our tutorial for beginners: [Getting started with GraphQL Java and Spring Boot](https://www.graphql-java.com/tutorials/getting-started-with-spring-boot/) -For details how to use `graphql-java` please look at the documentation: https://www.graphql-java.com/documentation/getting-started +For further details, please see the documentation: https://www.graphql-java.com/documentation/getting-started + +If you're looking to learn more, we (the maintainers) have written a book! [GraphQL with Java and Spring](https://leanpub.com/graphql-java) includes everything you need to know to build a production ready GraphQL service. The book is available on [Leanpub](https://leanpub.com/graphql-java) and [Amazon](https://www.amazon.com/GraphQL-Java-Spring-Andreas-Marek-ebook/dp/B0C96ZYWPF/). Please take a look at our [list of releases](https://github.com/graphql-java/graphql-java/releases) if you want to learn more about new releases and the changelog. diff --git a/SECURITY.md b/SECURITY.md index 9657523631..2b7a57f5d6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,13 +2,7 @@ ## Supported Versions -We support the latest release with security updates. - -We retain the discretion to backport security updates, this is decided on a case-by-case basis. - -| Version | Supported | -| ------- | ------------------ | -| v20.x | :white_check_mark: | +As stated in our [Release Policy](https://www.graphql-java.com/blog/release-policy/), we will backport critical bugfixes and security fixes for versions dating back 18 months. These fixes will be backported depending on severity and demand. ## Reporting a Vulnerability diff --git a/build.gradle b/build.gradle index d545b92572..e416a40faa 100644 --- a/build.gradle +++ b/build.gradle @@ -20,36 +20,52 @@ java { } } +def makeDevelopmentVersion(parts) { + def version = String.join("-", parts) + println "created development version: $version" + return version +} + def getDevelopmentVersion() { + def dateTime = new SimpleDateFormat('yyyy-MM-dd\'T\'HH-mm-ss').format(new Date()) def gitCheckOutput = new StringBuilder() def gitCheckError = new StringBuilder() - def gitCheck = ["git", "rev-parse", "--is-inside-work-tree"].execute() + def gitCheck = ["git", "-C", projectDir.toString(), "rev-parse", "--is-inside-work-tree"].execute() gitCheck.waitForProcessOutput(gitCheckOutput, gitCheckError) def isGit = gitCheckOutput.toString().trim() if (isGit != "true") { - def version = "0.0.0-" + new SimpleDateFormat('yyyy-MM-dd\'T\'HH-mm-ss').format(new Date()) + "-no-git" - println "created development version: $version" - return version + return makeDevelopmentVersion(["0.0.0", dateTime, "no-git"]) } - def gitHashOutput = new StringBuilder() - def gitHashError = new StringBuilder() - def gitShortHash = ["git", "-C", projectDir.toString(), "rev-parse", "--short", "HEAD"].execute() - gitShortHash.waitForProcessOutput(gitHashOutput, gitHashError) - def gitHash = gitHashOutput.toString().trim() - if (gitHash.isEmpty()) { - println "git hash is empty: error: ${error.toString()}" - throw new IllegalStateException("git hash could not be determined") + def isCi = Boolean.parseBoolean(System.env.CI) + if (isCi) { + def gitHashOutput = new StringBuilder() + def gitHashError = new StringBuilder() + def gitShortHash = ["git", "-C", projectDir.toString(), "rev-parse", "--short", "HEAD"].execute() + gitShortHash.waitForProcessOutput(gitHashOutput, gitHashError) + def gitHash = gitHashOutput.toString().trim() + if (gitHash.isEmpty()) { + println "git hash is empty: error: ${gitHashError.toString()}" + throw new IllegalStateException("git hash could not be determined") + } + + return makeDevelopmentVersion(["0.0.0", dateTime, gitHash]) } - def version = "0.0.0-" + new SimpleDateFormat('yyyy-MM-dd\'T\'HH-mm-ss').format(new Date()) + "-" + gitHash - println "created development version: $version" - version + + def gitRevParseOutput = new StringBuilder() + def gitRevParseError = new StringBuilder() + def gitRevParse = ["git", "-C", projectDir.toString(), "rev-parse", "--abbrev-ref", "HEAD"].execute() + gitRevParse.waitForProcessOutput(gitRevParseOutput, gitRevParseError) + def branchName = gitRevParseOutput.toString().trim() + + return makeDevelopmentVersion(["0.0.0", branchName, "SNAPSHOT"]) } def reactiveStreamsVersion = '1.0.3' def slf4jVersion = '2.0.7' def releaseVersion = System.env.RELEASE_VERSION def antlrVersion = '4.11.1' // https://mvnrepository.com/artifact/org.antlr/antlr4-runtime +def guavaVersion = '32.1.2-jre' version = releaseVersion ? releaseVersion : getDevelopmentVersion() group = 'com.graphql-java' @@ -86,17 +102,17 @@ dependencies { compileOnly 'org.jetbrains:annotations:24.0.1' implementation 'org.antlr:antlr4-runtime:' + antlrVersion implementation 'org.slf4j:slf4j-api:' + slf4jVersion - api 'com.graphql-java:java-dataloader:3.2.0' + api 'com.graphql-java:java-dataloader:3.2.1' api 'org.reactivestreams:reactive-streams:' + reactiveStreamsVersion antlr 'org.antlr:antlr4:' + antlrVersion - implementation 'com.google.guava:guava:32.1.1-jre' + implementation 'com.google.guava:guava:' + guavaVersion testImplementation group: 'junit', name: 'junit', version: '4.13.2' testImplementation 'org.spockframework:spock-core:2.0-groovy-3.0' - testImplementation 'org.codehaus.groovy:groovy:3.0.18' - testImplementation 'org.codehaus.groovy:groovy-json:3.0.18' + testImplementation 'org.codehaus.groovy:groovy:3.0.19' + testImplementation 'org.codehaus.groovy:groovy-json:3.0.19' testImplementation 'com.google.code.gson:gson:2.10.1' testImplementation 'org.eclipse.jetty:jetty-server:11.0.15' - testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.15.2' + testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.15.3' testImplementation 'org.slf4j:slf4j-simple:' + slf4jVersion testImplementation 'org.awaitility:awaitility-groovy:4.2.0' testImplementation 'com.github.javafaker:javafaker:1.0.2' @@ -106,10 +122,10 @@ dependencies { testImplementation 'org.testng:testng:7.8.0' // use for reactive streams test inheritance - testImplementation 'org.openjdk.jmh:jmh-core:1.36' - testAnnotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:1.36' - jmh 'org.openjdk.jmh:jmh-core:1.36' - jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.36' + testImplementation 'org.openjdk.jmh:jmh-core:1.37' + testAnnotationProcessor 'org.openjdk.jmh:jmh-generator-annprocess:1.37' + jmh 'org.openjdk.jmh:jmh-core:1.37' + jmh 'org.openjdk.jmh:jmh-generator-annprocess:1.37' } shadowJar { @@ -124,7 +140,7 @@ shadowJar { } relocate('org.antlr.v4.runtime', 'graphql.org.antlr.v4.runtime') dependencies { - include(dependency('com.google.guava:guava:32.1.1-jre')) + include(dependency('com.google.guava:guava:' + guavaVersion)) include(dependency('org.antlr:antlr4-runtime:' + antlrVersion)) } from "LICENSE.md" @@ -154,7 +170,7 @@ shadowJar { bnd(''' -exportcontents: graphql.* -removeheaders: Private-Package -Import-Package: !com.google.*,!org.checkerframework.*,!javax.annotation.*,!graphql.com.google.*,!org.antlr.*,!graphql.org.antlr.*,!sun.misc.*,* +Import-Package: !android.os.*,!com.google.*,!org.checkerframework.*,!javax.annotation.*,!graphql.com.google.*,!org.antlr.*,!graphql.org.antlr.*,!sun.misc.*,* ''') } @@ -202,7 +218,10 @@ generateGrammarSource { arguments += ["-visitor"] outputDirectory = file("${project.buildDir}/generated-src/antlr/main/graphql/parser/antlr") } -generateGrammarSource.inputs.dir('src/main/antlr') +generateGrammarSource.inputs + .dir('src/main/antlr') + .withPropertyName('sourceDir') + .withPathSensitivity(PathSensitivity.RELATIVE) task sourcesJar(type: Jar) { diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index caff79a7d5..50e570e14a 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -15,6 +15,7 @@ import static graphql.introspection.Introspection.DirectiveLocation.FRAGMENT_SPREAD; import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT; import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION; +import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT; import static graphql.introspection.Introspection.DirectiveLocation.SCALAR; import static graphql.language.DirectiveLocation.newDirectiveLocation; import static graphql.language.InputValueDefinition.newInputValueDefinition; @@ -31,10 +32,13 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String DEPRECATED = "deprecated"; + private static final String ONE_OF = "oneOf"; public static final String NO_LONGER_SUPPORTED = "No longer supported"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition SPECIFIED_BY_DIRECTIVE_DEFINITION; + @ExperimentalApi + public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION; static { @@ -65,6 +69,12 @@ public class Directives { .type(newNonNullType(newTypeName().name("String").build()).build()) .build()) .build(); + + ONE_OF_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(ONE_OF) + .directiveLocation(newDirectiveLocation().name(INPUT_OBJECT.name()).build()) + .description(createDescription("Indicates an Input Object is a OneOf Input Object.")) + .build(); } public static final GraphQLDirective IncludeDirective = GraphQLDirective.newDirective() @@ -119,6 +129,14 @@ public class Directives { .definition(SPECIFIED_BY_DIRECTIVE_DEFINITION) .build(); + @ExperimentalApi + public static final GraphQLDirective OneOfDirective = GraphQLDirective.newDirective() + .name(ONE_OF) + .description("Indicates an Input Object is a OneOf Input Object.") + .validLocations(INPUT_OBJECT) + .definition(ONE_OF_DIRECTIVE_DEFINITION) + .build(); + private static Description createDescription(String s) { return new Description(s, null, false); } diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 5b49617255..2ccb54b955 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -1,6 +1,7 @@ package graphql; import graphql.execution.AbortExecutionException; +import graphql.execution.Async; import graphql.execution.AsyncExecutionStrategy; import graphql.execution.AsyncSerialExecutionStrategy; import graphql.execution.DataFetcherExceptionHandler; @@ -421,31 +422,33 @@ public CompletableFuture executeAsync(ExecutionInput executionI if (logNotSafe.isDebugEnabled()) { logNotSafe.debug("Executing request. operation name: '{}'. query: '{}'. variables '{}'", executionInput.getOperationName(), executionInput.getQuery(), executionInput.getVariables()); } - executionInput = ensureInputHasId(executionInput); + ExecutionInput executionInputWithId = ensureInputHasId(executionInput); - InstrumentationState instrumentationState = instrumentation.createState(new InstrumentationCreateStateParameters(this.graphQLSchema, executionInput)); - try { - InstrumentationExecutionParameters inputInstrumentationParameters = new InstrumentationExecutionParameters(executionInput, this.graphQLSchema, instrumentationState); - executionInput = instrumentation.instrumentExecutionInput(executionInput, inputInstrumentationParameters, instrumentationState); - - CompletableFuture beginExecutionCF = new CompletableFuture<>(); - InstrumentationExecutionParameters instrumentationParameters = new InstrumentationExecutionParameters(executionInput, this.graphQLSchema, instrumentationState); - InstrumentationContext executionInstrumentation = nonNullCtx(instrumentation.beginExecution(instrumentationParameters, instrumentationState)); - executionInstrumentation.onDispatched(beginExecutionCF); - - GraphQLSchema graphQLSchema = instrumentation.instrumentSchema(this.graphQLSchema, instrumentationParameters, instrumentationState); - - CompletableFuture executionResult = parseValidateAndExecute(executionInput, graphQLSchema, instrumentationState); - // - // finish up instrumentation - executionResult = executionResult.whenComplete(completeInstrumentationCtxCF(executionInstrumentation, beginExecutionCF)); - // - // allow instrumentation to tweak the result - executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters, instrumentationState)); - return executionResult; - } catch (AbortExecutionException abortException) { - return handleAbortException(executionInput, instrumentationState, abortException); - } + CompletableFuture instrumentationStateCF = instrumentation.createStateAsync(new InstrumentationCreateStateParameters(this.graphQLSchema, executionInput)); + return Async.orNullCompletedFuture(instrumentationStateCF).thenCompose(instrumentationState -> { + try { + InstrumentationExecutionParameters inputInstrumentationParameters = new InstrumentationExecutionParameters(executionInputWithId, this.graphQLSchema, instrumentationState); + ExecutionInput instrumentedExecutionInput = instrumentation.instrumentExecutionInput(executionInputWithId, inputInstrumentationParameters, instrumentationState); + + CompletableFuture beginExecutionCF = new CompletableFuture<>(); + InstrumentationExecutionParameters instrumentationParameters = new InstrumentationExecutionParameters(instrumentedExecutionInput, this.graphQLSchema, instrumentationState); + InstrumentationContext executionInstrumentation = nonNullCtx(instrumentation.beginExecution(instrumentationParameters, instrumentationState)); + executionInstrumentation.onDispatched(beginExecutionCF); + + GraphQLSchema graphQLSchema = instrumentation.instrumentSchema(this.graphQLSchema, instrumentationParameters, instrumentationState); + + CompletableFuture executionResult = parseValidateAndExecute(instrumentedExecutionInput, graphQLSchema, instrumentationState); + // + // finish up instrumentation + executionResult = executionResult.whenComplete(completeInstrumentationCtxCF(executionInstrumentation, beginExecutionCF)); + // + // allow instrumentation to tweak the result + executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters, instrumentationState)); + return executionResult; + } catch (AbortExecutionException abortException) { + return handleAbortException(executionInput, instrumentationState, abortException); + } + }); } private CompletableFuture handleAbortException(ExecutionInput executionInput, InstrumentationState instrumentationState, AbortExecutionException abortException) { diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index ec71e2bdc9..56f7a2f9be 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -2,6 +2,8 @@ import graphql.Assert; import graphql.Internal; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.Collection; @@ -207,4 +209,15 @@ public static CompletableFuture exceptionallyCompletedFuture(Throwable ex return result; } + /** + * If the passed in CompletableFuture is null then it creates a CompletableFuture that resolves to null + * + * @param completableFuture the CF to use + * @param for two + * + * @return the completableFuture if it's not null or one that always resoles to null + */ + public static @NotNull CompletableFuture orNullCompletedFuture(@Nullable CompletableFuture completableFuture) { + return completableFuture != null ? completableFuture : CompletableFuture.completedFuture(null); + } } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index a517f5eea9..50db89b566 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -18,6 +18,7 @@ import graphql.normalized.ExecutableNormalizedOperationFactory; import graphql.schema.GraphQLSchema; import graphql.util.FpKit; +import graphql.util.LockKit; import org.dataloader.DataLoaderRegistry; import java.util.HashSet; @@ -49,6 +50,7 @@ public class ExecutionContext { private final Object localContext; private final Instrumentation instrumentation; private final AtomicReference> errors = new AtomicReference<>(ImmutableKit.emptyList()); + private final LockKit.ReentrantLock errorsLock = new LockKit.ReentrantLock(); private final Set errorPaths = new HashSet<>(); private final DataLoaderRegistry dataLoaderRegistry; private final Locale locale; @@ -130,6 +132,7 @@ public CoercedVariables getCoercedVariables() { /** * @param for two + * * @return the legacy context * * @deprecated use {@link #getGraphQLContext()} instead @@ -178,7 +181,7 @@ public ValueUnboxer getValueUnboxer() { * @param fieldPath the field path to put it under */ public void addError(GraphQLError error, ResultPath fieldPath) { - synchronized (this) { + errorsLock.runLocked(() -> { // // see https://spec.graphql.org/October2021/#sec-Handling-Field-Errors about how per // field errors should be handled - ie only once per field if it's already there for nullability @@ -188,7 +191,7 @@ public void addError(GraphQLError error, ResultPath fieldPath) { return; } this.errors.set(ImmutableKit.addToList(this.errors.get(), error)); - } + }); } /** @@ -198,7 +201,7 @@ public void addError(GraphQLError error, ResultPath fieldPath) { * @param error the error to add */ public void addError(GraphQLError error) { - synchronized (this) { + errorsLock.runLocked(() -> { // see https://github.com/graphql-java/graphql-java/issues/888 on how the spec is unclear // on how exactly multiple errors should be handled - ie only once per field or not outside the nullability // aspect. @@ -207,7 +210,7 @@ public void addError(GraphQLError error) { this.errorPaths.add(path); } this.errors.set(ImmutableKit.addToList(this.errors.get(), error)); - } + }); } /** @@ -220,9 +223,9 @@ public void addErrors(List errors) { if (errors.isEmpty()) { return; } - // we are synchronised because we set two fields at once - but we only ever read one of them later + // we are locked because we set two fields at once - but we only ever read one of them later // in getErrors so no need for synchronised there. - synchronized (this) { + errorsLock.runLocked(() -> { Set newErrorPaths = new HashSet<>(); for (GraphQLError error : errors) { // see https://github.com/graphql-java/graphql-java/issues/888 on how the spec is unclear @@ -235,7 +238,7 @@ public void addErrors(List errors) { } this.errorPaths.addAll(newErrorPaths); this.errors.set(ImmutableKit.concatLists(this.errors.get(), errors)); - } + }); } /** diff --git a/src/main/java/graphql/execution/OneOfNullValueException.java b/src/main/java/graphql/execution/OneOfNullValueException.java new file mode 100644 index 0000000000..40e5bbccae --- /dev/null +++ b/src/main/java/graphql/execution/OneOfNullValueException.java @@ -0,0 +1,30 @@ +package graphql.execution; + +import graphql.ErrorType; +import graphql.GraphQLError; +import graphql.GraphQLException; +import graphql.PublicApi; +import graphql.language.SourceLocation; + +import java.util.List; + +/** + * The input map to One Of Input Types MUST only have 1 entry with a non null value + */ +@PublicApi +public class OneOfNullValueException extends GraphQLException implements GraphQLError { + + public OneOfNullValueException(String message) { + super(message); + } + + @Override + public List getLocations() { + return null; + } + + @Override + public ErrorType getErrorType() { + return ErrorType.ValidationError; + } +} diff --git a/src/main/java/graphql/execution/OneOfTooManyKeysException.java b/src/main/java/graphql/execution/OneOfTooManyKeysException.java new file mode 100644 index 0000000000..973d7360d3 --- /dev/null +++ b/src/main/java/graphql/execution/OneOfTooManyKeysException.java @@ -0,0 +1,32 @@ +package graphql.execution; + +import graphql.ErrorType; +import graphql.GraphQLError; +import graphql.GraphQLException; +import graphql.PublicApi; +import graphql.language.SourceLocation; +import graphql.schema.GraphQLType; +import graphql.schema.GraphQLTypeUtil; + +import java.util.List; + +/** + * The input map to One Of Input Types MUST only have 1 entry + */ +@PublicApi +public class OneOfTooManyKeysException extends GraphQLException implements GraphQLError { + + public OneOfTooManyKeysException(String message) { + super(message); + } + + @Override + public List getLocations() { + return null; + } + + @Override + public ErrorType getErrorType() { + return ErrorType.ValidationError; + } +} diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index b7ba457910..cc59de6f83 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -1,10 +1,12 @@ package graphql.execution; +import graphql.Assert; import graphql.GraphQLContext; import graphql.Internal; import graphql.collect.ImmutableKit; import graphql.execution.values.InputInterceptor; +import graphql.i18n.I18n; import graphql.language.Argument; import graphql.language.ArrayValue; import graphql.language.NullValue; @@ -373,12 +375,42 @@ private static Map getArgumentValuesImpl( locale); coercedValues.put(argumentName, value); } + // @oneOf input must be checked now that all variables and literals have been converted + if (argumentType instanceof GraphQLInputObjectType) { + GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) argumentType; + if (inputObjectType.isOneOf() && ! ValuesResolverConversion.isNullValue(value)) { + validateOneOfInputTypes(inputObjectType, argumentValue, argumentName, value, locale); + } + } } } return coercedValues; } + @SuppressWarnings("unchecked") + private static void validateOneOfInputTypes(GraphQLInputObjectType oneOfInputType, Value argumentValue, String argumentName, Object inputValue, Locale locale) { + Assert.assertTrue(inputValue instanceof Map, () -> String.format("The coerced argument %s GraphQLInputObjectType is unexpectedly not a map", argumentName)); + Map objectMap = (Map) inputValue; + int mapSize; + if (argumentValue instanceof ObjectValue) { + mapSize = ((ObjectValue) argumentValue).getObjectFields().size(); + } else { + mapSize = objectMap.size(); + } + if (mapSize != 1) { + String msg = I18n.i18n(I18n.BundleType.Execution, locale) + .msg("Execution.handleOneOfNotOneFieldError", oneOfInputType.getName()); + throw new OneOfTooManyKeysException(msg); + } + String fieldName = objectMap.keySet().iterator().next(); + if (objectMap.get(fieldName) == null) { + String msg = I18n.i18n(I18n.BundleType.Execution, locale) + .msg("Execution.handleOneOfValueIsNullError", oneOfInputType.getName() + "." + fieldName); + throw new OneOfNullValueException(msg); + } + } + private static Map argumentMap(List arguments) { Map result = new LinkedHashMap<>(arguments.size()); for (Argument argument : arguments) { diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index 83f5285b0d..eff4d1cef1 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -69,11 +69,17 @@ static Object valueToLiteralImpl(GraphqlFieldVisibility fieldVisibility, if (valueMode == NORMALIZED) { return assertShouldNeverHappen("can't infer normalized structure"); } - return ValuesResolverLegacy.valueToLiteralLegacy( + Value value = ValuesResolverLegacy.valueToLiteralLegacy( inputValueWithState.getValue(), type, graphqlContext, locale); + // + // the valueToLiteralLegacy() nominally cant know if null means never set or is set to a null value + // but this code can know - its is SET to a value so, it MUST be a Null Literal + // this method would assert at the end of it if inputValueWithState.isNotSet() were true + // + return value == null ? NullValue.of() : value; } if (inputValueWithState.isLiteral()) { return inputValueWithState.getValue(); diff --git a/src/main/java/graphql/execution/directives/DirectivesResolver.java b/src/main/java/graphql/execution/directives/DirectivesResolver.java index 84722c3e40..4163c268cb 100644 --- a/src/main/java/graphql/execution/directives/DirectivesResolver.java +++ b/src/main/java/graphql/execution/directives/DirectivesResolver.java @@ -11,6 +11,7 @@ import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -25,14 +26,14 @@ public class DirectivesResolver { public DirectivesResolver() { } - public Map resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { + public Map> resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry(); - Map directiveMap = new LinkedHashMap<>(); + Map> directiveMap = new LinkedHashMap<>(); directives.forEach(directive -> { GraphQLDirective protoType = schema.getDirective(directive.getName()); if (protoType != null) { GraphQLDirective newDirective = protoType.transform(builder -> buildArguments(builder, codeRegistry, protoType, directive, variables, graphQLContext, locale)); - directiveMap.put(newDirective.getName(), newDirective); + directiveMap.computeIfAbsent(newDirective.getName(), k -> new ArrayList<>()).add(newDirective); } }); return ImmutableMap.copyOf(directiveMap); @@ -60,4 +61,4 @@ private void buildArguments(GraphQLDirective.Builder directiveBuilder, } }); } -} \ No newline at end of file +} diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 85468c3470..68c9b46b57 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -11,6 +11,8 @@ import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; +import graphql.util.FpKit; +import graphql.util.LockKit; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -22,7 +24,7 @@ /** * These objects are ALWAYS in the context of a single MergedField - * + *

* Also note we compute these values lazily */ @Internal @@ -34,6 +36,8 @@ public class QueryDirectivesImpl implements QueryDirectives { private final Map variables; private final GraphQLContext graphQLContext; private final Locale locale; + + private final LockKit.ComputedOnce computedOnce = new LockKit.ComputedOnce(); private volatile ImmutableMap> fieldDirectivesByField; private volatile ImmutableMap> fieldDirectivesByName; private volatile ImmutableMap> fieldAppliedDirectivesByField; @@ -48,20 +52,17 @@ public QueryDirectivesImpl(MergedField mergedField, GraphQLSchema schema, Map { final Map> byField = new LinkedHashMap<>(); final Map> byFieldApplied = new LinkedHashMap<>(); mergedField.getFields().forEach(field -> { List directives = field.getDirectives(); - ImmutableList resolvedDirectives = ImmutableList.copyOf( + ImmutableList resolvedDirectives = ImmutableList.copyOf(FpKit.flatList( directivesResolver .resolveDirectives(directives, schema, variables, graphQLContext, locale) .values() - ); + )); byField.put(field, resolvedDirectives); // at some point we will only use applied byFieldApplied.put(field, ImmutableKit.map(resolvedDirectives, this::toAppliedDirective)); @@ -80,7 +81,7 @@ private void computeValuesLazily() { this.fieldDirectivesByField = ImmutableMap.copyOf(byField); this.fieldAppliedDirectivesByName = ImmutableMap.copyOf(byNameApplied); this.fieldAppliedDirectivesByField = ImmutableMap.copyOf(byFieldApplied); - } + }); } private QueryAppliedDirective toAppliedDirective(GraphQLDirective directive) { diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index bfafc49ed7..70e7bd063f 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -22,6 +22,7 @@ import graphql.schema.GraphQLSchema; import graphql.validation.ValidationError; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.Arrays; import java.util.List; @@ -80,10 +81,19 @@ private InstrumentationContext chainedCtx(Function(mapAndDropNulls(instrumentations, mapper)); } + @Override + public InstrumentationState createState() { + return Assert.assertShouldNeverHappen("createStateAsync should only ever be used"); + } + + @Override + public @Nullable InstrumentationState createState(InstrumentationCreateStateParameters parameters) { + return Assert.assertShouldNeverHappen("createStateAsync should only ever be used"); + } @Override - public InstrumentationState createState(InstrumentationCreateStateParameters parameters) { - return new ChainedInstrumentationState(instrumentations, parameters); + public @NotNull CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + return ChainedInstrumentationState.combineAll(instrumentations, parameters); } @Override @@ -349,18 +359,31 @@ public CompletableFuture instrumentExecutionResult(ExecutionRes } static class ChainedInstrumentationState implements InstrumentationState { - private final Map instrumentationStates; + private final Map instrumentationToStates; - private ChainedInstrumentationState(List instrumentations, InstrumentationCreateStateParameters parameters) { - instrumentationStates = Maps.newLinkedHashMapWithExpectedSize(instrumentations.size()); - instrumentations.forEach(i -> instrumentationStates.put(i, i.createState(parameters))); + private ChainedInstrumentationState(List instrumentations, List instrumentationStates) { + instrumentationToStates = Maps.newLinkedHashMapWithExpectedSize(instrumentations.size()); + for (int i = 0; i < instrumentations.size(); i++) { + Instrumentation instrumentation = instrumentations.get(i); + InstrumentationState instrumentationState = instrumentationStates.get(i); + instrumentationToStates.put(instrumentation, instrumentationState); + } } private InstrumentationState getState(Instrumentation instrumentation) { - return instrumentationStates.get(instrumentation); + return instrumentationToStates.get(instrumentation); } + private static CompletableFuture combineAll(List instrumentations, InstrumentationCreateStateParameters parameters) { + Async.CombinedBuilder builder = Async.ofExpectedSize(instrumentations.size()); + for (Instrumentation instrumentation : instrumentations) { + // state can be null including the CF so handle that + CompletableFuture stateCF = Async.orNullCompletedFuture(instrumentation.createStateAsync(parameters)); + builder.add(stateCF); + } + return builder.await().thenApply(instrumentationStates -> new ChainedInstrumentationState(instrumentations, instrumentationStates)); + } } private static class ChainedInstrumentationContext implements InstrumentationContext { diff --git a/src/main/java/graphql/execution/instrumentation/Instrumentation.java b/src/main/java/graphql/execution/instrumentation/Instrumentation.java index 989364c482..77c4c6bd83 100644 --- a/src/main/java/graphql/execution/instrumentation/Instrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/Instrumentation.java @@ -63,11 +63,26 @@ default InstrumentationState createState() { * * @return a state object that is passed to each method */ + @Deprecated + @DeprecatedAt("2023-08-25") @Nullable default InstrumentationState createState(InstrumentationCreateStateParameters parameters) { return createState(); } + /** + * This will be called just before execution to create an object, in an asynchronous manner, that is given back to all instrumentation methods + * to allow them to have per execution request state + * + * @param parameters the parameters to this step + * + * @return a state object that is passed to each method + */ + @Nullable + default CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + return CompletableFuture.completedFuture(createState(parameters)); + } + /** * This is called right at the start of query execution, and it's the first step in the instrumentation chain. * diff --git a/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java b/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java index b7835ffddd..8ad5f8eeff 100644 --- a/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java @@ -56,6 +56,12 @@ public InstrumentationState createState() { return null; } + @Override + public @Nullable CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + InstrumentationState state = createState(parameters); + return state == null ? null : CompletableFuture.completedFuture(state); + } + @Override public @NotNull InstrumentationContext beginExecution(InstrumentationExecutionParameters parameters) { return assertShouldNeverHappen("The deprecated " + "beginExecution" + " was called"); diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java index e6e9a5330a..7da689db9a 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java @@ -10,6 +10,7 @@ import graphql.execution.instrumentation.InstrumentationState; import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; +import graphql.util.LockKit; import org.dataloader.DataLoaderRegistry; import org.slf4j.Logger; @@ -29,6 +30,8 @@ public class FieldLevelTrackingApproach { private static class CallStack implements InstrumentationState { + private final LockKit.ReentrantLock lock = new LockKit.ReentrantLock(); + private final LevelMap expectedFetchCountPerLevel = new LevelMap(); private final LevelMap fetchCountPerLevel = new LevelMap(); private final LevelMap expectedStrategyCallsPerLevel = new LevelMap(); @@ -124,10 +127,10 @@ ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationEx int parentLevel = path.getLevel(); int curLevel = parentLevel + 1; int fieldCount = parameters.getExecutionStrategyParameters().getFields().size(); - synchronized (callStack) { + callStack.lock.runLocked(() -> { callStack.increaseExpectedFetchCount(curLevel, fieldCount); callStack.increaseHappenedStrategyCalls(curLevel); - } + }); return new ExecutionStrategyInstrumentationContext() { @Override @@ -142,10 +145,9 @@ public void onCompleted(ExecutionResult result, Throwable t) { @Override public void onFieldValuesInfo(List fieldValueInfoList) { - boolean dispatchNeeded; - synchronized (callStack) { - dispatchNeeded = handleOnFieldValuesInfo(fieldValueInfoList, callStack, curLevel); - } + boolean dispatchNeeded = callStack.lock.callLocked(() -> + handleOnFieldValuesInfo(fieldValueInfoList, callStack, curLevel) + ); if (dispatchNeeded) { dispatch(); } @@ -153,9 +155,9 @@ public void onFieldValuesInfo(List fieldValueInfoList) { @Override public void onFieldValuesException() { - synchronized (callStack) { - callStack.increaseHappenedOnFieldValueCalls(curLevel); - } + callStack.lock.runLocked(() -> + callStack.increaseHappenedOnFieldValueCalls(curLevel) + ); } }; } @@ -187,19 +189,17 @@ public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchP CallStack callStack = (CallStack) rawState; ResultPath path = parameters.getEnvironment().getExecutionStepInfo().getPath(); int level = path.getLevel(); - return new InstrumentationContext() { + return new InstrumentationContext<>() { @Override public void onDispatched(CompletableFuture result) { - boolean dispatchNeeded; - synchronized (callStack) { + boolean dispatchNeeded = callStack.lock.callLocked(() -> { callStack.increaseFetchCount(level); - dispatchNeeded = dispatchIfNeeded(callStack, level); - } + return dispatchIfNeeded(callStack, level); + }); if (dispatchNeeded) { dispatch(); } - } @Override diff --git a/src/main/java/graphql/execution/reactive/CompletionStageMappingPublisher.java b/src/main/java/graphql/execution/reactive/CompletionStageMappingPublisher.java index 44965cf2ae..18bfabbc6c 100644 --- a/src/main/java/graphql/execution/reactive/CompletionStageMappingPublisher.java +++ b/src/main/java/graphql/execution/reactive/CompletionStageMappingPublisher.java @@ -1,6 +1,7 @@ package graphql.execution.reactive; import graphql.Internal; +import graphql.util.LockKit; import org.reactivestreams.Publisher; import org.reactivestreams.Subscriber; import org.reactivestreams.Subscription; @@ -44,6 +45,7 @@ public void subscribe(Subscriber downstreamSubscriber) { /** * Get instance of an upstreamPublisher + * * @return upstream instance of {@link Publisher} */ public Publisher getUpstreamPublisher() { @@ -56,6 +58,7 @@ public class CompletionStageSubscriber implements Subscriber { private final Subscriber downstreamSubscriber; Subscription delegatingSubscription; final Queue> inFlightDataQ; + final LockKit.ReentrantLock lock = new LockKit.ReentrantLock(); final AtomicReference onCompleteOrErrorRun; final AtomicBoolean onCompleteOrErrorRunCalled; @@ -137,6 +140,7 @@ public void onComplete() { /** * Get instance of downstream subscriber + * * @return {@link Subscriber} */ public Subscriber getDownstreamSubscriber() { @@ -153,23 +157,21 @@ private void onCompleteOrError(Runnable doneCodeToRun) { } private void offerToInFlightQ(CompletionStage completionStage) { - synchronized (inFlightDataQ) { - inFlightDataQ.offer(completionStage); - } + lock.runLocked(() -> + inFlightDataQ.offer(completionStage) + ); } private boolean removeFromInFlightQAndCheckIfEmpty(CompletionStage completionStage) { // uncontested locks in java are cheap - we don't expect much contention here - synchronized (inFlightDataQ) { + return lock.callLocked(() -> { inFlightDataQ.remove(completionStage); return inFlightDataQ.isEmpty(); - } + }); } private boolean inFlightQIsEmpty() { - synchronized (inFlightDataQ) { - return inFlightDataQ.isEmpty(); - } + return lock.callLocked(inFlightDataQ::isEmpty); } } } diff --git a/src/main/java/graphql/introspection/Introspection.java b/src/main/java/graphql/introspection/Introspection.java index 9b874f7f1b..d496e03702 100644 --- a/src/main/java/graphql/introspection/Introspection.java +++ b/src/main/java/graphql/introspection/Introspection.java @@ -399,6 +399,14 @@ private static String printDefaultValue(InputValueWithState inputValueWithState, return null; }; + private static final IntrospectionDataFetcher isOneOfFetcher = environment -> { + Object type = environment.getSource(); + if (type instanceof GraphQLInputObjectType) { + return ((GraphQLInputObjectType) type).isOneOf(); + } + return null; + }; + public static final GraphQLObjectType __Type = newObject() .name("__Type") .field(newFieldDefinition() @@ -440,6 +448,10 @@ private static String printDefaultValue(InputValueWithState inputValueWithState, .field(newFieldDefinition() .name("ofType") .type(typeRef("__Type"))) + .field(newFieldDefinition() + .name("isOneOf") + .description("This field is considered experimental because it has not yet been ratified in the graphql specification") + .type(GraphQLBoolean)) .field(newFieldDefinition() .name("specifiedByURL") .type(GraphQLString)) @@ -460,6 +472,7 @@ private static String printDefaultValue(InputValueWithState inputValueWithState, register(__Type, "enumValues", enumValuesTypesFetcher); register(__Type, "inputFields", inputFieldsFetcher); register(__Type, "ofType", OfTypeFetcher); + register(__Type, "isOneOf", isOneOfFetcher); register(__Type, "specifiedByURL", specifiedByUrlDataFetcher); register(__Type, "specifiedByUrl", specifiedByUrlDataFetcher); // note that this field is deprecated } diff --git a/src/main/java/graphql/introspection/IntrospectionQueryBuilder.java b/src/main/java/graphql/introspection/IntrospectionQueryBuilder.java index b49af872e4..21024349f8 100644 --- a/src/main/java/graphql/introspection/IntrospectionQueryBuilder.java +++ b/src/main/java/graphql/introspection/IntrospectionQueryBuilder.java @@ -33,6 +33,7 @@ public static class Options { private final boolean descriptions; private final boolean specifiedByUrl; + private final boolean isOneOf; private final boolean directiveIsRepeatable; @@ -44,12 +45,14 @@ public static class Options { private Options(boolean descriptions, boolean specifiedByUrl, + boolean isOneOf, boolean directiveIsRepeatable, boolean schemaDescription, boolean inputValueDeprecation, int typeRefFragmentDepth) { this.descriptions = descriptions; this.specifiedByUrl = specifiedByUrl; + this.isOneOf = isOneOf; this.directiveIsRepeatable = directiveIsRepeatable; this.schemaDescription = schemaDescription; this.inputValueDeprecation = inputValueDeprecation; @@ -64,6 +67,10 @@ public boolean isSpecifiedByUrl() { return specifiedByUrl; } + public boolean isOneOf() { + return isOneOf; + } + public boolean isDirectiveIsRepeatable() { return directiveIsRepeatable; } @@ -85,6 +92,7 @@ public static Options defaultOptions() { true, false, true, + true, false, true, 7 @@ -101,6 +109,7 @@ public static Options defaultOptions() { public Options descriptions(boolean flag) { return new Options(flag, this.specifiedByUrl, + this.isOneOf, this.directiveIsRepeatable, this.schemaDescription, this.inputValueDeprecation, @@ -117,12 +126,32 @@ public Options descriptions(boolean flag) { public Options specifiedByUrl(boolean flag) { return new Options(this.descriptions, flag, + this.isOneOf, this.directiveIsRepeatable, this.schemaDescription, this.inputValueDeprecation, this.typeRefFragmentDepth); } + /** + * This will allow you to include the `isOneOf` field for one of input types in the introspection query. + *

+ * This option is only needed while `@oneOf` input types are new and in time the reason for this + * option will go away. + * + * @param flag whether to include them + * + * @return options + */ + public Options isOneOf(boolean flag) { + return new Options(this.descriptions, + this.specifiedByUrl, + flag, + this.directiveIsRepeatable, + this.schemaDescription, + this.inputValueDeprecation, + this.typeRefFragmentDepth); + } /** * This will allow you to include the `isRepeatable` field for directives in the introspection query. * @@ -133,6 +162,7 @@ public Options specifiedByUrl(boolean flag) { public Options directiveIsRepeatable(boolean flag) { return new Options(this.descriptions, this.specifiedByUrl, + this.isOneOf, flag, this.schemaDescription, this.inputValueDeprecation, @@ -149,6 +179,7 @@ public Options directiveIsRepeatable(boolean flag) { public Options schemaDescription(boolean flag) { return new Options(this.descriptions, this.specifiedByUrl, + this.isOneOf, this.directiveIsRepeatable, flag, this.inputValueDeprecation, @@ -165,6 +196,7 @@ public Options schemaDescription(boolean flag) { public Options inputValueDeprecation(boolean flag) { return new Options(this.descriptions, this.specifiedByUrl, + this.isOneOf, this.directiveIsRepeatable, this.schemaDescription, flag, @@ -181,6 +213,7 @@ public Options inputValueDeprecation(boolean flag) { public Options typeRefFragmentDepth(int typeRefFragmentDepth) { return new Options(this.descriptions, this.specifiedByUrl, + this.isOneOf, this.directiveIsRepeatable, this.schemaDescription, this.inputValueDeprecation, @@ -249,6 +282,7 @@ public static Document buildDocument(Options options) { newField("name").build(), options.descriptions ? newField("description").build() : null, options.specifiedByUrl ? newField("specifiedByURL").build() : null, + options.isOneOf ? newField("isOneOf").build() : null, newField("fields") .arguments(ImmutableList.of( newArgument("includeDeprecated", BooleanValue.of(true)).build() diff --git a/src/main/java/graphql/language/SourceLocation.java b/src/main/java/graphql/language/SourceLocation.java index a7cb05fa78..a4ae90a039 100644 --- a/src/main/java/graphql/language/SourceLocation.java +++ b/src/main/java/graphql/language/SourceLocation.java @@ -2,6 +2,11 @@ import graphql.PublicApi; +import graphql.schema.GraphQLModifiedType; +import graphql.schema.GraphQLNamedSchemaElement; +import graphql.schema.GraphQLSchemaElement; +import graphql.schema.GraphQLTypeUtil; +import graphql.schema.idl.SchemaGenerator; import java.io.Serializable; import java.util.Objects; @@ -74,4 +79,27 @@ public String toString() { (sourceName != null ? ", sourceName=" + sourceName : "") + '}'; } + + + /** + * This method can return {@link SourceLocation} that help create the given schema element. If the + * schema is created from input files and {@link SchemaGenerator.Options#isCaptureAstDefinitions()} + * is set to true then schema elements contain a reference to the {@link SourceLocation} that helped + * create that runtime schema element. + * + * @param schemaElement the schema element + * + * @return the source location if available or null if it's not. + */ + public static SourceLocation getLocation(GraphQLSchemaElement schemaElement) { + if (schemaElement instanceof GraphQLModifiedType) { + schemaElement = GraphQLTypeUtil.unwrapAllAs((GraphQLModifiedType) schemaElement); + } + if (schemaElement instanceof GraphQLNamedSchemaElement) { + Node node = ((GraphQLNamedSchemaElement) schemaElement).getDefinition(); + return node != null ? node.getSourceLocation() : null; + } + return null; + } + } diff --git a/src/main/java/graphql/language/VariableReference.java b/src/main/java/graphql/language/VariableReference.java index ac085c694b..335e7bb6ea 100644 --- a/src/main/java/graphql/language/VariableReference.java +++ b/src/main/java/graphql/language/VariableReference.java @@ -91,6 +91,10 @@ public TraversalControl accept(TraverserContext context, NodeVisitor visit return visitor.visitVariableReference(this, context); } + public static VariableReference of(String name) { + return newVariableReference().name(name).build(); + } + public static Builder newVariableReference() { return new Builder(); } diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index d5e0d4db80..1124c9b7ce 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -7,6 +7,7 @@ import graphql.GraphQLContext; import graphql.PublicApi; import graphql.collect.ImmutableKit; +import graphql.execution.AbortExecutionException; import graphql.execution.CoercedVariables; import graphql.execution.MergedField; import graphql.execution.RawVariables; @@ -64,6 +65,85 @@ */ @PublicApi public class ExecutableNormalizedOperationFactory { + public static class Options { + private final GraphQLContext graphQLContext; + private final Locale locale; + private final int maxChildrenDepth; + + private Options(GraphQLContext graphQLContext, + Locale locale, + int maxChildrenDepth) { + this.graphQLContext = graphQLContext; + this.locale = locale; + this.maxChildrenDepth = maxChildrenDepth; + } + + public static Options defaultOptions() { + return new Options( + GraphQLContext.getDefault(), + Locale.getDefault(), + Integer.MAX_VALUE); + } + + /** + * Locale to use when parsing the query. + *

+ * e.g. can be passed to {@link graphql.schema.Coercing} for parsing. + * + * @param locale the locale to use + * @return new options object to use + */ + public Options locale(Locale locale) { + return new Options(this.graphQLContext, locale, this.maxChildrenDepth); + } + + /** + * Context object to use when parsing the operation. + *

+ * Can be used to intercept input values e.g. using {@link graphql.execution.values.InputInterceptor}. + * + * @param graphQLContext the context to use + * @return new options object to use + */ + public Options graphQLContext(GraphQLContext graphQLContext) { + return new Options(graphQLContext, this.locale, this.maxChildrenDepth); + } + + /** + * Controls the maximum depth of the operation. Can be used to prevent + * against malicious operations. + * + * @param maxChildrenDepth the max depth + * @return new options object to use + */ + public Options maxChildrenDepth(int maxChildrenDepth) { + return new Options(this.graphQLContext, this.locale, maxChildrenDepth); + } + + /** + * @return context to use during operation parsing + * @see #graphQLContext(GraphQLContext) + */ + public GraphQLContext getGraphQLContext() { + return graphQLContext; + } + + /** + * @return locale to use during operation parsing + * @see #locale(Locale) + */ + public Locale getLocale() { + return locale; + } + + /** + * @return maximum children depth before aborting parsing + * @see #maxChildrenDepth(int) + */ + public int getMaxChildrenDepth() { + return maxChildrenDepth; + } + } private final ConditionalNodes conditionalNodes = new ConditionalNodes(); @@ -90,8 +170,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperation( getOperationResult.fragmentsByName, coercedVariableValues, null, - GraphQLContext.getDefault(), - Locale.getDefault()); + Options.defaultOptions()); } /** @@ -114,8 +193,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperation( fragments, coercedVariableValues, null, - GraphQLContext.getDefault(), - Locale.getDefault()); + Options.defaultOptions()); } /** @@ -137,8 +215,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW document, operationName, rawVariables, - GraphQLContext.getDefault(), - Locale.getDefault()); + Options.defaultOptions()); } @@ -163,40 +240,64 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW GraphQLContext graphQLContext, Locale locale ) { + return createExecutableNormalizedOperationWithRawVariables( + graphQLSchema, + document, + operationName, + rawVariables, + Options.defaultOptions().graphQLContext(graphQLContext).locale(locale)); + } + + + /** + * This will create a runtime representation of the graphql operation that would be executed + * in a runtime sense. + * + * @param graphQLSchema the schema to be used + * @param document the {@link Document} holding the operation text + * @param operationName the operation name to use + * @param rawVariables the raw variables that have not yet been coerced + * @param options the {@link Options} to use for parsing + * + * @return a runtime representation of the graphql operation. + */ + public static ExecutableNormalizedOperation createExecutableNormalizedOperationWithRawVariables(GraphQLSchema graphQLSchema, + Document document, + String operationName, + RawVariables rawVariables, + Options options) { NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operationName); + return new ExecutableNormalizedOperationFactory().createExecutableNormalizedOperationImplWithRawVariables(graphQLSchema, getOperationResult.operationDefinition, getOperationResult.fragmentsByName, rawVariables, - graphQLContext, - locale); + options + ); } private ExecutableNormalizedOperation createExecutableNormalizedOperationImplWithRawVariables(GraphQLSchema graphQLSchema, OperationDefinition operationDefinition, Map fragments, RawVariables rawVariables, - GraphQLContext graphQLContext, - Locale locale) { - + Options options) { List variableDefinitions = operationDefinition.getVariableDefinitions(); CoercedVariables coercedVariableValues = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, rawVariables, - graphQLContext, - locale); + options.getGraphQLContext(), + options.getLocale()); Map normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables, - graphQLContext, - locale); + options.getGraphQLContext(), + options.getLocale()); return createNormalizedQueryImpl(graphQLSchema, operationDefinition, fragments, coercedVariableValues, normalizedVariableValues, - graphQLContext, - locale); + options); } /** @@ -207,7 +308,7 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr Map fragments, CoercedVariables coercedVariableValues, @Nullable Map normalizedVariableValues, - GraphQLContext graphQLContext, Locale locale) { + Options options) { FieldCollectorNormalizedQueryParams parameters = FieldCollectorNormalizedQueryParams .newParameters() .fragments(fragments) @@ -226,8 +327,8 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr ImmutableListMultimap.Builder coordinatesToNormalizedFields = ImmutableListMultimap.builder(); BiConsumer captureMergedField = (enf, mergedFld) -> { - //QueryDirectivesImpl is a lazy object and only computes itself when asked for - QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, coercedVariableValues.toMap(), graphQLContext, locale); + // QueryDirectivesImpl is a lazy object and only computes itself when asked for + QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, coercedVariableValues.toMap(), options.getGraphQLContext(), options.getLocale()); normalizedFieldToQueryDirectives.put(enf, queryDirectives); normalizedFieldToMergedField.put(enf, mergedFld); }; @@ -241,16 +342,17 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr updateFieldToNFMap(topLevel, fieldAndAstParents, fieldToNormalizedField); updateCoordinatedToNFMap(coordinatesToNormalizedFields, topLevel); - buildFieldWithChildren(topLevel, + buildFieldWithChildren( + topLevel, fieldAndAstParents, parameters, fieldToNormalizedField, captureMergedField, coordinatesToNormalizedFields, - 1); - + 1, + options.getMaxChildrenDepth()); } - for (FieldCollectorNormalizedQueryParams.PossibleMerger possibleMerger : parameters.possibleMergerList) { + for (FieldCollectorNormalizedQueryParams.PossibleMerger possibleMerger : parameters.getPossibleMergerList()) { List childrenWithSameResultKey = possibleMerger.parent.getChildrenWithSameResultKey(possibleMerger.resultKey); ENFMerger.merge(possibleMerger.parent, childrenWithSameResultKey, graphQLSchema); } @@ -272,7 +374,12 @@ private void buildFieldWithChildren(ExecutableNormalizedField executableNormaliz ImmutableListMultimap.Builder fieldNormalizedField, BiConsumer captureMergedField, ImmutableListMultimap.Builder coordinatesToNormalizedFields, - int curLevel) { + int curLevel, + int maxLevel) { + if (curLevel > maxLevel) { + throw new AbortExecutionException("Maximum query depth exceeded " + curLevel + " > " + maxLevel); + } + CollectNFResult nextLevel = collectFromMergedField(fieldCollectorNormalizedQueryParams, executableNormalizedField, fieldAndAstParents, curLevel + 1); for (ExecutableNormalizedField childENF : nextLevel.children) { @@ -291,7 +398,8 @@ private void buildFieldWithChildren(ExecutableNormalizedField executableNormaliz fieldNormalizedField, captureMergedField, coordinatesToNormalizedFields, - curLevel + 1); + curLevel + 1, + maxLevel); } } diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java index 2d68b2f821..7dc3d11f32 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java @@ -4,9 +4,11 @@ import com.google.common.collect.ImmutableMap; import graphql.Assert; import graphql.PublicApi; +import graphql.execution.directives.QueryDirectives; import graphql.introspection.Introspection; import graphql.language.Argument; import graphql.language.ArrayValue; +import graphql.language.Directive; import graphql.language.Document; import graphql.language.Field; import graphql.language.InlineFragment; @@ -30,6 +32,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static graphql.collect.ImmutableKit.emptyList; import static graphql.collect.ImmutableKit.map; @@ -78,7 +81,7 @@ public Map getVariables() { } /** - * This will compile a operation text {@link Document} with possibly variables from the given {@link ExecutableNormalizedField}s + * This will compile an operation text {@link Document} with possibly variables from the given {@link ExecutableNormalizedField}s * * The {@link VariablePredicate} is used called to decide if the given argument values should be made into a variable * OR inlined into the operation text as a graphql literal. @@ -96,10 +99,34 @@ public static CompilerResult compileToDocument(@NotNull GraphQLSchema schema, @Nullable String operationName, @NotNull List topLevelFields, @Nullable VariablePredicate variablePredicate) { + return compileToDocument(schema,operationKind,operationName,topLevelFields,Map.of(),variablePredicate); + } + + /** + * This will compile an operation text {@link Document} with possibly variables from the given {@link ExecutableNormalizedField}s + * + * The {@link VariablePredicate} is used called to decide if the given argument values should be made into a variable + * OR inlined into the operation text as a graphql literal. + * + * @param schema the graphql schema to use + * @param operationKind the kind of operation + * @param operationName the name of the operation to use + * @param topLevelFields the top level {@link ExecutableNormalizedField}s to start from + * @param normalizedFieldToQueryDirectives the map of normalized field to query directives + * @param variablePredicate the variable predicate that decides if arguments turn into variables or not during compilation + * + * @return a {@link CompilerResult} object + */ + public static CompilerResult compileToDocument(@NotNull GraphQLSchema schema, + @NotNull OperationDefinition.Operation operationKind, + @Nullable String operationName, + @NotNull List topLevelFields, + @NotNull Map normalizedFieldToQueryDirectives, + @Nullable VariablePredicate variablePredicate) { GraphQLObjectType operationType = getOperationType(schema, operationKind); VariableAccumulator variableAccumulator = new VariableAccumulator(variablePredicate); - List> selections = subselectionsForNormalizedField(schema, operationType.getName(), topLevelFields, variableAccumulator); + List> selections = subselectionsForNormalizedField(schema, operationType.getName(), topLevelFields, normalizedFieldToQueryDirectives, variableAccumulator); SelectionSet selectionSet = new SelectionSet(selections); OperationDefinition.Builder definitionBuilder = OperationDefinition.newOperationDefinition() @@ -120,6 +147,7 @@ public static CompilerResult compileToDocument(@NotNull GraphQLSchema schema, private static List> subselectionsForNormalizedField(GraphQLSchema schema, @NotNull String parentOutputType, List executableNormalizedFields, + @NotNull Map normalizedFieldToQueryDirectives, VariableAccumulator variableAccumulator) { ImmutableList.Builder> selections = ImmutableList.builder(); @@ -129,13 +157,13 @@ private static List> subselectionsForNormalizedField(GraphQLSchema for (ExecutableNormalizedField nf : executableNormalizedFields) { if (nf.isConditional(schema)) { - selectionForNormalizedField(schema, nf, variableAccumulator) + selectionForNormalizedField(schema, nf, normalizedFieldToQueryDirectives, variableAccumulator) .forEach((objectTypeName, field) -> fieldsByTypeCondition .computeIfAbsent(objectTypeName, ignored -> new ArrayList<>()) .add(field)); } else { - selections.add(selectionForNormalizedField(schema, parentOutputType, nf, variableAccumulator)); + selections.add(selectionForNormalizedField(schema, parentOutputType, nf, normalizedFieldToQueryDirectives,variableAccumulator)); } } @@ -156,11 +184,12 @@ private static List> subselectionsForNormalizedField(GraphQLSchema */ private static Map selectionForNormalizedField(GraphQLSchema schema, ExecutableNormalizedField executableNormalizedField, + @NotNull Map normalizedFieldToQueryDirectives, VariableAccumulator variableAccumulator) { Map groupedFields = new LinkedHashMap<>(); for (String objectTypeName : executableNormalizedField.getObjectTypeNames()) { - groupedFields.put(objectTypeName, selectionForNormalizedField(schema, objectTypeName, executableNormalizedField, variableAccumulator)); + groupedFields.put(objectTypeName, selectionForNormalizedField(schema, objectTypeName, executableNormalizedField,normalizedFieldToQueryDirectives, variableAccumulator)); } return groupedFields; @@ -172,6 +201,7 @@ private static Map selectionForNormalizedField(GraphQLSchema sche private static Field selectionForNormalizedField(GraphQLSchema schema, String objectTypeName, ExecutableNormalizedField executableNormalizedField, + @NotNull Map normalizedFieldToQueryDirectives, VariableAccumulator variableAccumulator) { final List> subSelections; if (executableNormalizedField.getChildren().isEmpty()) { @@ -184,6 +214,7 @@ private static Field selectionForNormalizedField(GraphQLSchema schema, schema, fieldOutputType.getName(), executableNormalizedField.getChildren(), + normalizedFieldToQueryDirectives, variableAccumulator ); } @@ -191,12 +222,22 @@ private static Field selectionForNormalizedField(GraphQLSchema schema, SelectionSet selectionSet = selectionSetOrNullIfEmpty(subSelections); List arguments = createArguments(executableNormalizedField, variableAccumulator); - return newField() + QueryDirectives queryDirectives = normalizedFieldToQueryDirectives.get(executableNormalizedField); + + + Field.Builder builder = newField() .name(executableNormalizedField.getFieldName()) .alias(executableNormalizedField.getAlias()) .selectionSet(selectionSet) - .arguments(arguments) - .build(); + .arguments(arguments); + if(queryDirectives == null || queryDirectives.getImmediateAppliedDirectivesByField().isEmpty() ){ + return builder.build(); + }else { + List directives = queryDirectives.getImmediateAppliedDirectivesByField().keySet().stream().flatMap(field -> field.getDirectives().stream()).collect(Collectors.toList()); + return builder + .directives(directives) + .build(); + } } @Nullable diff --git a/src/main/java/graphql/normalized/FieldCollectorNormalizedQueryParams.java b/src/main/java/graphql/normalized/FieldCollectorNormalizedQueryParams.java index f7e6675896..8b4d03bf3e 100644 --- a/src/main/java/graphql/normalized/FieldCollectorNormalizedQueryParams.java +++ b/src/main/java/graphql/normalized/FieldCollectorNormalizedQueryParams.java @@ -23,7 +23,7 @@ public class FieldCollectorNormalizedQueryParams { private final GraphQLContext graphQLContext; private final Locale locale; - public List possibleMergerList = new ArrayList<>(); + private final List possibleMergerList = new ArrayList<>(); public static class PossibleMerger { ExecutableNormalizedField parent; @@ -39,6 +39,10 @@ public void addPossibleMergers(ExecutableNormalizedField parent, String resultKe possibleMergerList.add(new PossibleMerger(parent, resultKey)); } + public List getPossibleMergerList() { + return possibleMergerList; + } + public GraphQLSchema getGraphQLSchema() { return graphQLSchema; } diff --git a/src/main/java/graphql/parser/GraphqlAntlrToLanguage.java b/src/main/java/graphql/parser/GraphqlAntlrToLanguage.java index 440c4c5ea0..006499f4ff 100644 --- a/src/main/java/graphql/parser/GraphqlAntlrToLanguage.java +++ b/src/main/java/graphql/parser/GraphqlAntlrToLanguage.java @@ -669,7 +669,12 @@ protected Value createValue(GraphqlParser.ValueWithVariableContext ctx) { addCommonData(intValue, ctx); return captureRuleContext(intValue.build(), ctx); } else if (ctx.FloatValue() != null) { - FloatValue.Builder floatValue = FloatValue.newFloatValue().value(new BigDecimal(ctx.FloatValue().getText())); + FloatValue.Builder floatValue; + try { + floatValue = FloatValue.newFloatValue().value(new BigDecimal(ctx.FloatValue().getText())); + } catch (NumberFormatException e) { + throw new InvalidSyntaxException("Invalid floating point value", null, ctx.FloatValue().getText(), null, e); + } addCommonData(floatValue, ctx); return captureRuleContext(floatValue.build(), ctx); } else if (ctx.BooleanValue() != null) { diff --git a/src/main/java/graphql/parser/MultiSourceReader.java b/src/main/java/graphql/parser/MultiSourceReader.java index a85906d824..acbcef40e0 100644 --- a/src/main/java/graphql/parser/MultiSourceReader.java +++ b/src/main/java/graphql/parser/MultiSourceReader.java @@ -2,6 +2,7 @@ import graphql.Assert; import graphql.PublicApi; +import graphql.util.LockKit; import java.io.IOException; import java.io.LineNumberReader; @@ -27,6 +28,7 @@ public class MultiSourceReader extends Reader { private int currentIndex = 0; private int overallLineNumber = 0; private final boolean trackData; + private final LockKit.ReentrantLock readerLock = new LockKit.ReentrantLock(); private MultiSourceReader(Builder builder) { @@ -37,7 +39,8 @@ private MultiSourceReader(Builder builder) { @Override public int read(char[] cbuf, int off, int len) throws IOException { while (true) { - synchronized (this) { + readerLock.lock(); + try { if (currentIndex >= sourceParts.size()) { return -1; } @@ -50,6 +53,8 @@ public int read(char[] cbuf, int off, int len) throws IOException { trackData(cbuf, off, read); return read; } + } finally { + readerLock.unlock(); } } } @@ -147,7 +152,7 @@ public SourceAndLine getSourceAndLineFromOverallLine(int overallLineNumber) { * @return the line number of the current source. This is zeroes based like {@link java.io.LineNumberReader#getLineNumber()} */ public int getLineNumber() { - synchronized (this) { + return readerLock.callLocked(() -> { if (sourceParts.isEmpty()) { return 0; } @@ -155,14 +160,14 @@ public int getLineNumber() { return sourceParts.get(sourceParts.size() - 1).lineReader.getLineNumber(); } return sourceParts.get(currentIndex).lineReader.getLineNumber(); - } + }); } /** * @return The name of the current source */ public String getSourceName() { - synchronized (this) { + return readerLock.callLocked(() -> { if (sourceParts.isEmpty()) { return null; } @@ -170,7 +175,7 @@ public String getSourceName() { return sourceParts.get(sourceParts.size() - 1).sourceName; } return sourceParts.get(currentIndex).sourceName; - } + }); } /** @@ -198,13 +203,16 @@ public List getData() { @Override public void close() throws IOException { - synchronized (this) { + readerLock.lock(); + try { for (SourcePart sourcePart : sourceParts) { if (!sourcePart.closed) { sourcePart.lineReader.close(); sourcePart.closed = true; } } + } finally { + readerLock.unlock(); } } diff --git a/src/main/java/graphql/parser/UnicodeUtil.java b/src/main/java/graphql/parser/UnicodeUtil.java index b32cbfea6a..368d46ec65 100644 --- a/src/main/java/graphql/parser/UnicodeUtil.java +++ b/src/main/java/graphql/parser/UnicodeUtil.java @@ -33,7 +33,12 @@ public static int parseAndWriteUnicode(I18n i18n, StringWriter writer, String st int continueIndex = isBracedEscape(string, i) ? endIndexExclusive : endIndexExclusive - 1; String hexStr = string.substring(startIndex, endIndexExclusive); - int codePoint = Integer.parseInt(hexStr, 16); + int codePoint; + try { + codePoint = Integer.parseInt(hexStr, 16); + } catch (NumberFormatException e) { + throw new InvalidUnicodeSyntaxException(i18n, "InvalidUnicode.invalidHexString", sourceLocation, offendingToken(string, i, continueIndex)); + } if (isTrailingSurrogateValue(codePoint)) { throw new InvalidUnicodeSyntaxException(i18n, "InvalidUnicode.trailingLeadingSurrogate", sourceLocation, offendingToken(string, i, continueIndex)); diff --git a/src/main/java/graphql/schema/DataFetchingFieldSelectionSetImpl.java b/src/main/java/graphql/schema/DataFetchingFieldSelectionSetImpl.java index ac000facae..da71043740 100644 --- a/src/main/java/graphql/schema/DataFetchingFieldSelectionSetImpl.java +++ b/src/main/java/graphql/schema/DataFetchingFieldSelectionSetImpl.java @@ -5,6 +5,7 @@ import graphql.Internal; import graphql.collect.ImmutableKit; import graphql.normalized.ExecutableNormalizedField; +import graphql.util.LockKit; import java.io.File; import java.nio.file.FileSystems; @@ -89,7 +90,7 @@ public static DataFetchingFieldSelectionSet newCollector(GraphQLSchema schema, G private final Supplier normalizedFieldSupplier; - private volatile boolean computedValues; + private LockKit.ComputedOnce computedOnce = new LockKit.ComputedOnce(); // we have multiple entries in this map so that we can do glob matching in multiple ways // however it needs to be normalised back to a set of unique fields when give back out to // the caller. @@ -101,7 +102,6 @@ public static DataFetchingFieldSelectionSet newCollector(GraphQLSchema schema, G private DataFetchingFieldSelectionSetImpl(Supplier normalizedFieldSupplier, GraphQLSchema schema) { this.schema = schema; this.normalizedFieldSupplier = normalizedFieldSupplier; - this.computedValues = false; } @Override @@ -205,23 +205,19 @@ public Map> getFieldsGroupedByResultKey(String field } private void computeValuesLazily() { - if (computedValues) { + if (computedOnce.hasBeenComputed()) { return; } // this supplier is a once only thread synced call - so do it outside this lock // if only to have only 1 lock in action at a time ExecutableNormalizedField currentNormalisedField = normalizedFieldSupplier.get(); - synchronized (this) { - if (computedValues) { - return; - } + computedOnce.runOnce(() -> { flattenedFieldsForGlobSearching = new LinkedHashSet<>(); normalisedSelectionSetFields = new LinkedHashMap<>(); ImmutableList.Builder immediateFieldsBuilder = ImmutableList.builder(); traverseSubSelectedFields(currentNormalisedField, immediateFieldsBuilder, "", "", true); immediateFields = immediateFieldsBuilder.build(); - computedValues = true; - } + }); } @@ -280,7 +276,7 @@ private List mkIterable(String fieldGlobPattern, String[] fieldGlobPatte @Override public String toString() { - if (!computedValues) { + if (!computedOnce.hasBeenComputed()) { return "notComputed"; } return String.join("\n", flattenedFieldsForGlobSearching); diff --git a/src/main/java/graphql/schema/GraphQLInputObjectType.java b/src/main/java/graphql/schema/GraphQLInputObjectType.java index ae22565a74..81b0d63160 100644 --- a/src/main/java/graphql/schema/GraphQLInputObjectType.java +++ b/src/main/java/graphql/schema/GraphQLInputObjectType.java @@ -2,7 +2,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import graphql.Directives; import graphql.DirectivesUtil; +import graphql.ExperimentalApi; import graphql.Internal; import graphql.PublicApi; import graphql.language.InputObjectTypeDefinition; @@ -34,6 +36,7 @@ public class GraphQLInputObjectType implements GraphQLNamedInputType, GraphQLUnmodifiedType, GraphQLNullableType, GraphQLInputFieldsContainer, GraphQLDirectiveContainer { private final String name; + private final boolean isOneOf; private final String description; private final ImmutableMap fieldMap; private final InputObjectTypeDefinition definition; @@ -60,6 +63,7 @@ private GraphQLInputObjectType(String name, this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions); this.directives = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives); this.fieldMap = buildDefinitionMap(fields); + this.isOneOf = hasOneOf(directives, appliedDirectives); } private ImmutableMap buildDefinitionMap(List fieldDefinitions) { @@ -67,11 +71,33 @@ private ImmutableMap buildDefinitionMap(List assertShouldNeverHappen("Duplicated definition for field '%s' in type '%s'", fld1.getName(), this.name))); } + private boolean hasOneOf(List directives, List appliedDirectives) { + if (appliedDirectives.stream().anyMatch(d -> Directives.OneOfDirective.getName().equals(d.getName()))) { + return true; + } + // eventually GraphQLDirective as applied directive goes away + return directives.stream().anyMatch(d -> Directives.OneOfDirective.getName().equals(d.getName())); + } + @Override public String getName() { return name; } + + /** + * An Input Object is considered a OneOf Input Object if it has the `@oneOf` directive applied to it. + *

+ * This API is currently considered experimental since the graphql specification has not yet ratified + * this approach. + * + * @return true if it's a OneOf Input Object + */ + @ExperimentalApi + public boolean isOneOf() { + return isOneOf; + } + public String getDescription() { return description; } diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java index 9b0a1b561c..335c694af6 100644 --- a/src/main/java/graphql/schema/GraphQLSchema.java +++ b/src/main/java/graphql/schema/GraphQLSchema.java @@ -917,6 +917,9 @@ private GraphQLSchema buildImpl() { if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.SpecifiedByDirective.getName()))) { additionalDirectives.add(Directives.SpecifiedByDirective); } + if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.OneOfDirective.getName()))) { + additionalDirectives.add(Directives.OneOfDirective); + } // quick build - no traversing final GraphQLSchema partiallyBuiltSchema = new GraphQLSchema(this); diff --git a/src/main/java/graphql/schema/diff/DiffSet.java b/src/main/java/graphql/schema/diff/DiffSet.java index 6c9ee4fb53..3823c30d59 100644 --- a/src/main/java/graphql/schema/diff/DiffSet.java +++ b/src/main/java/graphql/schema/diff/DiffSet.java @@ -1,6 +1,7 @@ package graphql.schema.diff; import graphql.Assert; +import graphql.DeprecatedAt; import graphql.ExecutionResult; import graphql.GraphQL; import graphql.PublicApi; @@ -15,6 +16,8 @@ * {@link graphql.introspection.IntrospectionQuery}. */ @PublicApi +@Deprecated +@DeprecatedAt("2023-10-04") public class DiffSet { private final Map introspectionOld; @@ -39,7 +42,6 @@ public Map getNew() { return introspectionNew; } - /** * Creates a diff set out of the result of 2 introspection queries. * diff --git a/src/main/java/graphql/schema/diff/SchemaDiff.java b/src/main/java/graphql/schema/diff/SchemaDiff.java index 18e34967f9..9012c217f9 100644 --- a/src/main/java/graphql/schema/diff/SchemaDiff.java +++ b/src/main/java/graphql/schema/diff/SchemaDiff.java @@ -1,5 +1,7 @@ package graphql.schema.diff; +import graphql.Assert; +import graphql.DeprecatedAt; import graphql.PublicSpi; import graphql.introspection.IntrospectionResultToSchema; import graphql.language.Argument; @@ -119,27 +121,43 @@ public SchemaDiff(Options options) { * * @return the number of API breaking changes */ + @Deprecated + @DeprecatedAt("2023-10-04") @SuppressWarnings("unchecked") public int diffSchema(DiffSet diffSet, DifferenceReporter reporter) { - CountingReporter countingReporter = new CountingReporter(reporter); - diffSchemaImpl(diffSet, countingReporter); + Document oldDoc = new IntrospectionResultToSchema().createSchemaDefinition(diffSet.getOld()); + Document newDoc = new IntrospectionResultToSchema().createSchemaDefinition(diffSet.getNew()); + diffSchemaImpl(oldDoc, newDoc, countingReporter); return countingReporter.breakingCount; } - private void diffSchemaImpl(DiffSet diffSet, DifferenceReporter reporter) { - Map oldApi = diffSet.getOld(); - Map newApi = diffSet.getNew(); + /** + * This will perform a difference on the two schemas. The reporter callback + * interface will be called when differences are encountered. + * + * @param schemaDiffSet the two schemas to compare for difference + * @param reporter the place to report difference events to + * + * @return the number of API breaking changes + */ + @SuppressWarnings("unchecked") + public int diffSchema(SchemaDiffSet schemaDiffSet, DifferenceReporter reporter) { + if (options.enforceDirectives) { + Assert.assertTrue(schemaDiffSet.supportsEnforcingDirectives(), () -> + "The provided schema diff set implementation does not supporting enforcing directives during schema diff."); + } - Document oldDoc = new IntrospectionResultToSchema().createSchemaDefinition(oldApi); - Document newDoc = new IntrospectionResultToSchema().createSchemaDefinition(newApi); + CountingReporter countingReporter = new CountingReporter(reporter); + diffSchemaImpl(schemaDiffSet.getOldSchemaDefinitionDoc(), schemaDiffSet.getNewSchemaDefinitionDoc(), countingReporter); + return countingReporter.breakingCount; + } + private void diffSchemaImpl(Document oldDoc, Document newDoc, DifferenceReporter reporter) { DiffCtx ctx = new DiffCtx(reporter, oldDoc, newDoc); - Optional oldSchemaDef = getSchemaDef(oldDoc); Optional newSchemaDef = getSchemaDef(newDoc); - // check query operation checkOperation(ctx, "query", oldSchemaDef, newSchemaDef); checkOperation(ctx, "mutation", oldSchemaDef, newSchemaDef); diff --git a/src/main/java/graphql/schema/diff/SchemaDiffSet.java b/src/main/java/graphql/schema/diff/SchemaDiffSet.java new file mode 100644 index 0000000000..c0dd0b74d7 --- /dev/null +++ b/src/main/java/graphql/schema/diff/SchemaDiffSet.java @@ -0,0 +1,134 @@ +package graphql.schema.diff; + +import graphql.Assert; +import graphql.ExecutionResult; +import graphql.GraphQL; +import graphql.PublicApi; +import graphql.introspection.IntrospectionQuery; +import graphql.introspection.IntrospectionResultToSchema; +import graphql.language.Document; +import graphql.parser.Parser; +import graphql.schema.GraphQLSchema; +import graphql.schema.idl.SchemaPrinter; + +import java.util.Map; + +/** + * Interface used to define 2 schemas that can be diffed by the {@link SchemaDiff} operation. + */ +@PublicApi +public class SchemaDiffSet { + + private final Document oldSchemaDoc; + private final Document newSchemaDoc; + private final boolean supportsEnforcingDirectives; + + private SchemaDiffSet(final Document oldSchemaDoc, + final Document newSchemaDoc, + final boolean supportsEnforcingDirectives) { + this.oldSchemaDoc = oldSchemaDoc; + this.newSchemaDoc = newSchemaDoc; + this.supportsEnforcingDirectives = supportsEnforcingDirectives; + } + + /** + * @return Returns a IDL document that represents the old schema as part of a SchemaDiff operation. + */ + public Document getOldSchemaDefinitionDoc() { + return this.oldSchemaDoc; + } + + /** + * @return Returns a IDL document that represents the new schema created from the introspection result. + */ + public Document getNewSchemaDefinitionDoc() { + return this.newSchemaDoc; + } + + /** + * @return Flag indicating whether this diffset implementation can be used to enforce directives when performing schema diff. + */ + public boolean supportsEnforcingDirectives() { + return this.supportsEnforcingDirectives; + } + + /** + * Creates an schema diff set out of the result of 2 introspection queries. + * + * @param introspectionOld the older introspection query + * @param introspectionNew the newer introspection query + * + * @return a diff set representing them which will not support enforcing directives. + */ + public static SchemaDiffSet diffSetFromIntrospection(final Map introspectionOld, + final Map introspectionNew) { + final Document oldDoc = getDocumentFromIntrospection(introspectionOld); + final Document newDoc = getDocumentFromIntrospection(introspectionNew); + return new SchemaDiffSet(oldDoc, newDoc, false); + } + + /** + * Creates an schema diff set out of the result of 2 introspection queries. + * + * @param oldSchema the older GraphQLSchema object to introspect. + * @param newSchema the new GraphQLSchema object to introspect. + * + * @return a diff set representing them which will not support enforcing directives. + */ + public static SchemaDiffSet diffSetFromIntrospection(final GraphQLSchema oldSchema, + final GraphQLSchema newSchema) { + final Map introspectionOld = introspect(oldSchema); + final Map introspectionNew = introspect(newSchema); + return diffSetFromIntrospection(introspectionOld, introspectionNew); + } + + /** + * Creates an schema diff set out of the two SDL definition Strings. + * + * @param oldSchemaSdl the older SDL definition String. + * @param newSchemaSdl the newer SDL definition String. + * + * @return a diff set representing them which will support enforcing directives. + */ + public static SchemaDiffSet diffSetFromSdl(final String oldSchemaSdl, + final String newSchemaSdl) { + final Document oldDoc = getDocumentFromSDLString(oldSchemaSdl); + final Document newDoc = getDocumentFromSDLString(newSchemaSdl); + return new SchemaDiffSet(oldDoc, newDoc, true); + } + + /** + * Creates an schema diff set out of the two SDL definition Strings. + * + * @param oldSchema the older SDL definition String. + * @param newSchema the newer SDL definition String. + * + * @return a diff set representing them which will support enforcing directives. + */ + public static SchemaDiffSet diffSetFromSdl(final GraphQLSchema oldSchema, + final GraphQLSchema newSchema) { + final String oldSchemaSdl = getSchemaSdl(oldSchema); + final String newSchemaSdl = getSchemaSdl(newSchema); + return diffSetFromSdl(oldSchemaSdl, newSchemaSdl); + } + + private static Document getDocumentFromIntrospection(final Map introspectionResult) { + return new IntrospectionResultToSchema().createSchemaDefinition(introspectionResult); + } + + private static Document getDocumentFromSDLString(final String sdlString) { + return Parser.parse(sdlString); + } + + private static String getSchemaSdl(GraphQLSchema schema) { + final SchemaPrinter schemaPrinter = new SchemaPrinter(); + return schemaPrinter.print(schema); + } + + private static Map introspect(GraphQLSchema schema) { + GraphQL gql = GraphQL.newGraphQL(schema).build(); + ExecutionResult result = gql.execute(IntrospectionQuery.INTROSPECTION_QUERY); + Assert.assertTrue(result.getErrors().size() == 0, () -> "The schema has errors during Introspection"); + return result.getData(); + } +} diff --git a/src/main/java/graphql/schema/diffing/DiffImpl.java b/src/main/java/graphql/schema/diffing/DiffImpl.java index 9cfd39b9cf..efff71cdd0 100644 --- a/src/main/java/graphql/schema/diffing/DiffImpl.java +++ b/src/main/java/graphql/schema/diffing/DiffImpl.java @@ -400,19 +400,21 @@ private double calcLowerBoundMappingCost(Vertex v, Map isolatedVerticesCache, Map nonFixedParentRestrictions) { if (nonFixedParentRestrictions.containsKey(v) || partialMapping.hasParentRestriction(v)) { - Vertex uParentRestriction = nonFixedParentRestrictions.get(v); - if (uParentRestriction == null) { - uParentRestriction = partialMapping.getParentRestriction(v); - } + if (!u.isIsolated()) { // Always allow mapping to isolated nodes + Vertex uParentRestriction = nonFixedParentRestrictions.get(v); + if (uParentRestriction == null) { + uParentRestriction = partialMapping.getParentRestriction(v); + } - Collection parentEdges = completeTargetGraph.getAdjacentEdgesInverseNonCopy(u); - if (parentEdges.size() != 1) { - return Integer.MAX_VALUE; - } + Collection parentEdges = completeTargetGraph.getAdjacentEdgesInverseNonCopy(u); + if (parentEdges.size() != 1) { + return Integer.MAX_VALUE; + } - Vertex uParent = parentEdges.iterator().next().getFrom(); - if (uParent != uParentRestriction) { - return Integer.MAX_VALUE; + Vertex uParent = parentEdges.iterator().next().getFrom(); + if (uParent != uParentRestriction) { + return Integer.MAX_VALUE; + } } } diff --git a/src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java b/src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java index acf39508ad..97e6a324aa 100644 --- a/src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java +++ b/src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java @@ -244,7 +244,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isObjectDeleted(object.getName())) { return; } - AppliedDirectiveObjectLocation location = new AppliedDirectiveObjectLocation(object.getName()); + AppliedDirectiveObjectLocation location = new AppliedDirectiveObjectLocation(object.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.INTERFACE)) { @@ -252,7 +252,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isInterfaceDeleted(interfaze.getName())) { return; } - AppliedDirectiveInterfaceLocation location = new AppliedDirectiveInterfaceLocation(interfaze.getName()); + AppliedDirectiveInterfaceLocation location = new AppliedDirectiveInterfaceLocation(interfaze.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getInterfaceModification(interfaze.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.SCALAR)) { @@ -260,7 +260,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isScalarDeleted(scalar.getName())) { return; } - AppliedDirectiveScalarLocation location = new AppliedDirectiveScalarLocation(scalar.getName()); + AppliedDirectiveScalarLocation location = new AppliedDirectiveScalarLocation(scalar.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getScalarModification(scalar.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.ENUM)) { @@ -268,7 +268,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isEnumDeleted(enumVertex.getName())) { return; } - AppliedDirectiveEnumLocation location = new AppliedDirectiveEnumLocation(enumVertex.getName()); + AppliedDirectiveEnumLocation location = new AppliedDirectiveEnumLocation(enumVertex.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getEnumModification(enumVertex.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.ENUM_VALUE)) { @@ -280,7 +280,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isEnumValueDeletedFromExistingEnum(enumVertex.getName(), enumValue.getName())) { return; } - AppliedDirectiveEnumValueLocation location = new AppliedDirectiveEnumValueLocation(enumVertex.getName(), enumValue.getName()); + AppliedDirectiveEnumValueLocation location = new AppliedDirectiveEnumValueLocation(enumVertex.getName(), enumValue.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getEnumModification(enumVertex.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.INPUT_OBJECT)) { @@ -288,7 +288,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isInputObjectDeleted(inputObject.getName())) { return; } - AppliedDirectiveInputObjectLocation location = new AppliedDirectiveInputObjectLocation(inputObject.getName()); + AppliedDirectiveInputObjectLocation location = new AppliedDirectiveInputObjectLocation(inputObject.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getInputObjectModification(inputObject.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.INPUT_FIELD)) { @@ -300,7 +300,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isInputFieldDeletedFromExistingInputObject(inputObject.getName(), inputField.getName())) { return; } - AppliedDirectiveInputObjectFieldLocation location = new AppliedDirectiveInputObjectFieldLocation(inputObject.getName(), inputField.getName()); + AppliedDirectiveInputObjectFieldLocation location = new AppliedDirectiveInputObjectFieldLocation(inputObject.getName(), inputField.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getInputObjectModification(inputObject.getName()).getDetails().add(appliedDirectiveDeletion); } else if (container.isOfType(SchemaGraph.UNION)) { @@ -308,7 +308,7 @@ private void appliedDirectiveDeleted(EditOperation editOperation) { if (isUnionDeleted(union.getName())) { return; } - AppliedDirectiveUnionLocation location = new AppliedDirectiveUnionLocation(union.getName()); + AppliedDirectiveUnionLocation location = new AppliedDirectiveUnionLocation(union.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getUnionModification(union.getName()).getDetails().add(appliedDirectiveDeletion); } @@ -327,7 +327,7 @@ private void appliedDirectiveArgumentDeleted(EditOperation editOperation) { if (isObjectDeleted(object.getName())) { return; } - AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName()); + AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName(), appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(new AppliedDirectiveArgumentDeletion(location, deletedArgument.getName())); } else { assertTrue(interfaceOrObjective.isOfType(SchemaGraph.INTERFACE)); @@ -335,7 +335,7 @@ private void appliedDirectiveArgumentDeleted(EditOperation editOperation) { if (isInterfaceDeleted(interfaze.getName())) { return; } - AppliedDirectiveInterfaceFieldLocation location = new AppliedDirectiveInterfaceFieldLocation(interfaze.getName(), field.getName()); + AppliedDirectiveInterfaceFieldLocation location = new AppliedDirectiveInterfaceFieldLocation(interfaze.getName(), field.getName(), appliedDirective.getName()); getInterfaceModification(interfaze.getName()).getDetails().add(new AppliedDirectiveArgumentDeletion(location, deletedArgument.getName())); } } @@ -359,7 +359,7 @@ private void appliedDirectiveArgumentChanged(EditOperation editOperation) { Vertex interfaceOrObjective = newSchemaGraph.getFieldsContainerForField(field); if (interfaceOrObjective.isOfType(SchemaGraph.OBJECT)) { Vertex object = interfaceOrObjective; - AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName()); + AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName(), appliedDirective.getName()); if (valueChanged) { AppliedDirectiveArgumentValueModification argumentValueModification = new AppliedDirectiveArgumentValueModification(location, newArgumentName, oldValue, newValue); getObjectModification(object.getName()).getDetails().add(argumentValueModification); @@ -385,7 +385,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isObjectAdded(object.getName())) { return; } - AppliedDirectiveObjectLocation location = new AppliedDirectiveObjectLocation(object.getName()); + AppliedDirectiveObjectLocation location = new AppliedDirectiveObjectLocation(object.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.INTERFACE)) { @@ -393,7 +393,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isInterfaceAdded(interfaze.getName())) { return; } - AppliedDirectiveInterfaceLocation location = new AppliedDirectiveInterfaceLocation(interfaze.getName()); + AppliedDirectiveInterfaceLocation location = new AppliedDirectiveInterfaceLocation(interfaze.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getInterfaceModification(interfaze.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.SCALAR)) { @@ -401,7 +401,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isScalarAdded(scalar.getName())) { return; } - AppliedDirectiveScalarLocation location = new AppliedDirectiveScalarLocation(scalar.getName()); + AppliedDirectiveScalarLocation location = new AppliedDirectiveScalarLocation(scalar.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getScalarModification(scalar.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.ENUM)) { @@ -409,7 +409,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isEnumAdded(enumVertex.getName())) { return; } - AppliedDirectiveEnumLocation location = new AppliedDirectiveEnumLocation(enumVertex.getName()); + AppliedDirectiveEnumLocation location = new AppliedDirectiveEnumLocation(enumVertex.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getEnumModification(enumVertex.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.ENUM_VALUE)) { @@ -421,7 +421,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isNewEnumValueForExistingEnum(enumVertex.getName(), enumValue.getName())) { return; } - AppliedDirectiveEnumValueLocation location = new AppliedDirectiveEnumValueLocation(enumVertex.getName(), enumValue.getName()); + AppliedDirectiveEnumValueLocation location = new AppliedDirectiveEnumValueLocation(enumVertex.getName(), enumValue.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getEnumModification(enumVertex.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.INPUT_OBJECT)) { @@ -429,7 +429,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isInputObjectAdded(inputObject.getName())) { return; } - AppliedDirectiveInputObjectLocation location = new AppliedDirectiveInputObjectLocation(inputObject.getName()); + AppliedDirectiveInputObjectLocation location = new AppliedDirectiveInputObjectLocation(inputObject.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getInputObjectModification(inputObject.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.INPUT_FIELD)) { @@ -441,7 +441,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isNewInputFieldExistingInputObject(inputObject.getName(), inputField.getName())) { return; } - AppliedDirectiveInputObjectFieldLocation location = new AppliedDirectiveInputObjectFieldLocation(inputObject.getName(), inputField.getName()); + AppliedDirectiveInputObjectFieldLocation location = new AppliedDirectiveInputObjectFieldLocation(inputObject.getName(), inputField.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getInputObjectModification(inputObject.getName()).getDetails().add(appliedDirectiveAddition); } else if (container.isOfType(SchemaGraph.UNION)) { @@ -449,7 +449,7 @@ private void appliedDirectiveAdded(EditOperation editOperation) { if (isUnionAdded(union.getName())) { return; } - AppliedDirectiveUnionLocation location = new AppliedDirectiveUnionLocation(union.getName()); + AppliedDirectiveUnionLocation location = new AppliedDirectiveUnionLocation(union.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getUnionModification(union.getName()).getDetails().add(appliedDirectiveAddition); } @@ -467,7 +467,7 @@ private void appliedDirectiveDeletedFromField(Vertex appliedDirective, Vertex co if (isFieldDeletedFromExistingObject(object.getName(), field.getName())) { return; } - AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName()); + AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveDeletion); } @@ -485,7 +485,7 @@ private void appliedDirectiveAddedToField(Vertex appliedDirective, Vertex contai if (isFieldNewForExistingObject(object.getName(), field.getName())) { return; } - AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName()); + AppliedDirectiveObjectFieldLocation location = new AppliedDirectiveObjectFieldLocation(object.getName(), field.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveAddition); } @@ -508,7 +508,7 @@ private void appliedDirectiveDeletedFromArgument(Vertex appliedDirective, Vertex if (isArgumentDeletedFromExistingObjectField(object.getName(), field.getName(), argument.getName())) { return; } - AppliedDirectiveObjectFieldArgumentLocation location = new AppliedDirectiveObjectFieldArgumentLocation(object.getName(), field.getName(), argument.getName()); + AppliedDirectiveObjectFieldArgumentLocation location = new AppliedDirectiveObjectFieldArgumentLocation(object.getName(), field.getName(), argument.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveDeletion); } else { @@ -523,7 +523,7 @@ private void appliedDirectiveDeletedFromArgument(Vertex appliedDirective, Vertex if (isArgumentDeletedFromExistingInterfaceField(interfaze.getName(), field.getName(), argument.getName())) { return; } - AppliedDirectiveInterfaceFieldArgumentLocation location = new AppliedDirectiveInterfaceFieldArgumentLocation(interfaze.getName(), field.getName(), argument.getName()); + AppliedDirectiveInterfaceFieldArgumentLocation location = new AppliedDirectiveInterfaceFieldArgumentLocation(interfaze.getName(), field.getName(), argument.getName(), appliedDirective.getName()); AppliedDirectiveDeletion appliedDirectiveDeletion = new AppliedDirectiveDeletion(location, appliedDirective.getName()); getInterfaceModification(interfaze.getName()).getDetails().add(appliedDirectiveDeletion); } @@ -559,7 +559,7 @@ private void appliedDirectiveAddedToArgument(Vertex appliedDirective, Vertex con if (isArgumentNewForExistingObjectField(object.getName(), field.getName(), argument.getName())) { return; } - AppliedDirectiveObjectFieldArgumentLocation location = new AppliedDirectiveObjectFieldArgumentLocation(object.getName(), field.getName(), argument.getName()); + AppliedDirectiveObjectFieldArgumentLocation location = new AppliedDirectiveObjectFieldArgumentLocation(object.getName(), field.getName(), argument.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getObjectModification(object.getName()).getDetails().add(appliedDirectiveAddition); } else { @@ -574,7 +574,7 @@ private void appliedDirectiveAddedToArgument(Vertex appliedDirective, Vertex con if (isArgumentNewForExistingInterfaceField(interfaze.getName(), field.getName(), argument.getName())) { return; } - AppliedDirectiveInterfaceFieldArgumentLocation location = new AppliedDirectiveInterfaceFieldArgumentLocation(interfaze.getName(), field.getName(), argument.getName()); + AppliedDirectiveInterfaceFieldArgumentLocation location = new AppliedDirectiveInterfaceFieldArgumentLocation(interfaze.getName(), field.getName(), argument.getName(), appliedDirective.getName()); AppliedDirectiveAddition appliedDirectiveAddition = new AppliedDirectiveAddition(location, appliedDirective.getName()); getInterfaceModification(interfaze.getName()).getDetails().add(appliedDirectiveAddition); } diff --git a/src/main/java/graphql/schema/diffing/ana/SchemaDifference.java b/src/main/java/graphql/schema/diffing/ana/SchemaDifference.java index 8d809269cd..04172d4e91 100644 --- a/src/main/java/graphql/schema/diffing/ana/SchemaDifference.java +++ b/src/main/java/graphql/schema/diffing/ana/SchemaDifference.java @@ -1250,10 +1250,12 @@ interface AppliedDirectiveLocationDetail { class AppliedDirectiveObjectFieldLocation implements AppliedDirectiveLocationDetail { private final String objectName; private final String fieldName; + private final String directiveName; - public AppliedDirectiveObjectFieldLocation(String objectName, String fieldName) { + public AppliedDirectiveObjectFieldLocation(String objectName, String fieldName, String directiveName) { this.objectName = objectName; this.fieldName = fieldName; + this.directiveName = directiveName; } public String getFieldName() { @@ -1263,15 +1265,23 @@ public String getFieldName() { public String getObjectName() { return objectName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveInterfaceFieldLocation implements AppliedDirectiveLocationDetail { private final String interfaceName; private final String fieldName; - public AppliedDirectiveInterfaceFieldLocation(String interfaceName, String fieldName) { + private final String directiveName; + + + public AppliedDirectiveInterfaceFieldLocation(String interfaceName, String fieldName, String directiveName) { this.interfaceName = interfaceName; this.fieldName = fieldName; + this.directiveName = directiveName; } public String getFieldName() { @@ -1281,18 +1291,29 @@ public String getFieldName() { public String getInterfaceName() { return interfaceName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveScalarLocation implements AppliedDirectiveLocationDetail { private final String name; - public AppliedDirectiveScalarLocation(String name) { + private final String directiveName; + + public AppliedDirectiveScalarLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { return name; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveSchemaLocation implements AppliedDirectiveLocationDetail { @@ -1301,37 +1322,53 @@ class AppliedDirectiveSchemaLocation implements AppliedDirectiveLocationDetail { class AppliedDirectiveObjectLocation implements AppliedDirectiveLocationDetail { private final String name; + private final String directiveName; + - public AppliedDirectiveObjectLocation(String name) { + public AppliedDirectiveObjectLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { return name; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveInterfaceLocation implements AppliedDirectiveLocationDetail { private final String name; + private final String directiveName; - public AppliedDirectiveInterfaceLocation(String name) { + public AppliedDirectiveInterfaceLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { return name; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveObjectFieldArgumentLocation implements AppliedDirectiveLocationDetail { private final String objectName; private final String fieldName; private final String argumentName; + private final String directiveName; - public AppliedDirectiveObjectFieldArgumentLocation(String objectName, String fieldName, String argumentName) { + + public AppliedDirectiveObjectFieldArgumentLocation(String objectName, String fieldName, String argumentName, String directiveName) { this.objectName = objectName; this.fieldName = fieldName; this.argumentName = argumentName; + this.directiveName = directiveName; } public String getObjectName() { @@ -1345,6 +1382,10 @@ public String getFieldName() { public String getArgumentName() { return argumentName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveDirectiveArgumentLocation implements AppliedDirectiveLocationDetail { @@ -1369,11 +1410,14 @@ class AppliedDirectiveInterfaceFieldArgumentLocation implements AppliedDirective private final String interfaceName; private final String fieldName; private final String argumentName; + private final String directiveName; + - public AppliedDirectiveInterfaceFieldArgumentLocation(String interfaceName, String fieldName, String argumentName) { + public AppliedDirectiveInterfaceFieldArgumentLocation(String interfaceName, String fieldName, String argumentName, String directiveName) { this.interfaceName = interfaceName; this.fieldName = fieldName; this.argumentName = argumentName; + this.directiveName = directiveName; } public String getInterfaceName() { @@ -1387,13 +1431,20 @@ public String getFieldName() { public String getArgumentName() { return argumentName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveUnionLocation implements AppliedDirectiveLocationDetail { private final String name; + private final String directiveName; + - public AppliedDirectiveUnionLocation(String name) { + public AppliedDirectiveUnionLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { @@ -1403,23 +1454,33 @@ public String getName() { class AppliedDirectiveEnumLocation implements AppliedDirectiveLocationDetail { private final String name; + private final String directiveName; + - public AppliedDirectiveEnumLocation(String name) { + public AppliedDirectiveEnumLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { return name; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveEnumValueLocation implements AppliedDirectiveLocationDetail { private final String enumName; private final String valueName; + private final String directiveName; + - public AppliedDirectiveEnumValueLocation(String enumName, String valueName) { + public AppliedDirectiveEnumValueLocation(String enumName, String valueName, String directiveName) { this.enumName = enumName; this.valueName = valueName; + this.directiveName = directiveName; } public String getEnumName() { @@ -1429,28 +1490,42 @@ public String getEnumName() { public String getValueName() { return valueName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveInputObjectLocation implements AppliedDirectiveLocationDetail { private final String name; + private final String directiveName; + - public AppliedDirectiveInputObjectLocation(String name) { + public AppliedDirectiveInputObjectLocation(String name, String directiveName) { this.name = name; + this.directiveName = directiveName; } public String getName() { return name; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveInputObjectFieldLocation implements AppliedDirectiveLocationDetail { private final String inputObjectName; private final String fieldName; + private final String directiveName; - public AppliedDirectiveInputObjectFieldLocation(String inputObjectName, String fieldName) { + + public AppliedDirectiveInputObjectFieldLocation(String inputObjectName, String fieldName, String directiveName) { this.inputObjectName = inputObjectName; this.fieldName = fieldName; + this.directiveName = directiveName; } public String getInputObjectName() { @@ -1460,6 +1535,10 @@ public String getInputObjectName() { public String getFieldName() { return fieldName; } + + public String getDirectiveName() { + return directiveName; + } } class AppliedDirectiveAddition implements diff --git a/src/main/java/graphql/schema/idl/DirectiveInfo.java b/src/main/java/graphql/schema/idl/DirectiveInfo.java index 4b6159ee21..428a97aceb 100644 --- a/src/main/java/graphql/schema/idl/DirectiveInfo.java +++ b/src/main/java/graphql/schema/idl/DirectiveInfo.java @@ -22,7 +22,9 @@ public class DirectiveInfo { Directives.IncludeDirective, Directives.SkipDirective, Directives.DeprecatedDirective, - Directives.SpecifiedByDirective); + Directives.SpecifiedByDirective, + Directives.OneOfDirective + ); /** * A map from directive name to directive which provided by specification @@ -31,7 +33,9 @@ public class DirectiveInfo { Directives.IncludeDirective.getName(), Directives.IncludeDirective, Directives.SkipDirective.getName(), Directives.SkipDirective, Directives.DeprecatedDirective.getName(), Directives.DeprecatedDirective, - Directives.SpecifiedByDirective.getName(), Directives.SpecifiedByDirective); + Directives.SpecifiedByDirective.getName(), Directives.SpecifiedByDirective, + Directives.OneOfDirective.getName(), Directives.OneOfDirective + ); /** * Returns true if a directive with provided directiveName has been defined in graphql specification diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index 097d320ea9..317828aaf8 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -82,6 +82,7 @@ import static graphql.Directives.DEPRECATED_DIRECTIVE_DEFINITION; import static graphql.Directives.IncludeDirective; import static graphql.Directives.NO_LONGER_SUPPORTED; +import static graphql.Directives.ONE_OF_DIRECTIVE_DEFINITION; import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION; import static graphql.Directives.SkipDirective; import static graphql.Directives.SpecifiedByDirective; @@ -1079,6 +1080,7 @@ void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry) { // we synthesize this into the type registry - no need for them to add it typeRegistry.add(DEPRECATED_DIRECTIVE_DEFINITION); typeRegistry.add(SPECIFIED_BY_DIRECTIVE_DEFINITION); + typeRegistry.add(ONE_OF_DIRECTIVE_DEFINITION); } private Optional getOperationNamed(String name, Map operationTypeDefs) { diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index 6f3cbc64d9..84c5c7e6e2 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -6,7 +6,9 @@ import graphql.PublicApi; import graphql.execution.ValuesResolver; import graphql.language.AstPrinter; +import graphql.language.Comment; import graphql.language.Description; +import graphql.language.DirectiveDefinition; import graphql.language.Document; import graphql.language.EnumTypeDefinition; import graphql.language.EnumValueDefinition; @@ -61,6 +63,7 @@ import java.util.stream.Stream; import static graphql.Directives.DeprecatedDirective; +import static graphql.Scalars.GraphQLString; import static graphql.schema.visibility.DefaultGraphqlFieldVisibility.DEFAULT_FIELD_VISIBILITY; import static graphql.util.EscapeUtil.escapeJsonString; import static java.util.Optional.ofNullable; @@ -72,14 +75,6 @@ */ @PublicApi public class SchemaPrinter { - // - // we use this so that we get the simple "@deprecated" as text and not a full exploded - // text with arguments (but only when we auto add this) - // - private static final GraphQLAppliedDirective DeprecatedAppliedDirective4Printing = GraphQLAppliedDirective.newDirective() - .name("deprecated") - .build(); - /** * This predicate excludes all directives which are specified by the GraphQL Specification. * Printing these directives is optional. @@ -109,6 +104,8 @@ public static class Options { private final GraphqlTypeComparatorRegistry comparatorRegistry; + private final boolean includeAstDefinitionComments; + private Options(boolean includeIntrospectionTypes, boolean includeScalars, boolean includeSchemaDefinition, @@ -117,7 +114,8 @@ private Options(boolean includeIntrospectionTypes, boolean descriptionsAsHashComments, Predicate includeDirective, Predicate includeSchemaElement, - GraphqlTypeComparatorRegistry comparatorRegistry) { + GraphqlTypeComparatorRegistry comparatorRegistry, + boolean includeAstDefinitionComments) { this.includeIntrospectionTypes = includeIntrospectionTypes; this.includeScalars = includeScalars; this.includeSchemaDefinition = includeSchemaDefinition; @@ -127,6 +125,7 @@ private Options(boolean includeIntrospectionTypes, this.descriptionsAsHashComments = descriptionsAsHashComments; this.comparatorRegistry = comparatorRegistry; this.includeSchemaElement = includeSchemaElement; + this.includeAstDefinitionComments = includeAstDefinitionComments; } public boolean isIncludeIntrospectionTypes() { @@ -165,6 +164,8 @@ public boolean isUseAstDefinitions() { return useAstDefinitions; } + public boolean isIncludeAstDefinitionComments() { return includeAstDefinitionComments; } + public static Options defaultOptions() { return new Options(false, true, @@ -174,7 +175,8 @@ public static Options defaultOptions() { false, directive -> true, element -> true, - DefaultGraphqlTypeComparatorRegistry.defaultComparators()); + DefaultGraphqlTypeComparatorRegistry.defaultComparators(), + false); } /** @@ -193,7 +195,8 @@ public Options includeIntrospectionTypes(boolean flag) { this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -212,7 +215,8 @@ public Options includeScalarTypes(boolean flag) { this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -234,7 +238,8 @@ public Options includeSchemaDefinition(boolean flag) { this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -258,7 +263,8 @@ public Options includeDirectiveDefinitions(boolean flag) { this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -278,7 +284,8 @@ public Options includeDirectives(boolean flag) { this.descriptionsAsHashComments, directive -> flag, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -297,7 +304,8 @@ public Options includeDirectives(Predicate includeDirective) { this.descriptionsAsHashComments, includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -317,7 +325,8 @@ public Options includeSchemaElement(Predicate includeSchem this.descriptionsAsHashComments, this.includeDirective, includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -337,7 +346,8 @@ public Options useAstDefinitions(boolean flag) { this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -359,7 +369,8 @@ public Options descriptionsAsHashComments(boolean flag) { flag, this.includeDirective, this.includeSchemaElement, - this.comparatorRegistry); + this.comparatorRegistry, + this.includeAstDefinitionComments); } /** @@ -380,7 +391,30 @@ public Options setComparators(GraphqlTypeComparatorRegistry comparatorRegistry) this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, - comparatorRegistry); + comparatorRegistry, + this.includeAstDefinitionComments); + } + + /** + * Sometimes it is useful to allow printing schema comments. This can be achieved by providing comments in the AST definitions. + *

+ * The default is to ignore these for backward compatibility and due to this being relatively uncommon need. + * + * @param flag whether to include AST definition comments. + * + * @return new instance of Options + */ + public Options includeAstDefinitionComments(boolean flag) { + return new Options(this.includeIntrospectionTypes, + this.includeScalars, + this.includeSchemaDefinition, + this.includeDirectiveDefinitions, + this.useAstDefinitions, + this.descriptionsAsHashComments, + this.includeDirective, + this.includeSchemaElement, + comparatorRegistry, + flag); } } @@ -741,11 +775,11 @@ private SchemaElementPrinter schemaPrinter() { } if (needsSchemaPrinted) { - if (hasDescription(schema)) { + if (hasAstDefinitionComments(schema) || hasDescription(schema)) { out.print(printComments(schema, "")); } List directives = DirectivesUtil.toAppliedDirectives(schema.getSchemaAppliedDirectives(), schema.getSchemaDirectives()); - out.format("schema %s{\n", directivesString(GraphQLSchemaElement.class, false, directives)); + out.format("schema %s{\n", directivesString(GraphQLSchemaElement.class, directives)); if (queryType != null) { out.format(" query: %s\n", queryType.getName()); } @@ -777,9 +811,10 @@ String argsString(List arguments) { } String argsString(Class parent, List arguments) { + boolean hasAstDefinitionComments = arguments.stream().anyMatch(this::hasAstDefinitionComments); boolean hasDescriptions = arguments.stream().anyMatch(this::hasDescription); - String halfPrefix = hasDescriptions ? " " : ""; - String prefix = hasDescriptions ? " " : ""; + String halfPrefix = hasAstDefinitionComments || hasDescriptions ? " " : ""; + String prefix = hasAstDefinitionComments || hasDescriptions ? " " : ""; int count = 0; StringBuilder sb = new StringBuilder(); @@ -795,11 +830,11 @@ String argsString(Class parent, List parent, List !it.isEmpty()) - .forEach(directiveString -> sb.append(" ").append(directiveString)); + sb.append(directivesString(GraphQLArgument.class, argument.isDeprecated(), argument)); count++; } if (count > 0) { - if (hasDescriptions) { + if (hasAstDefinitionComments || hasDescriptions) { sb.append("\n"); } sb.append(halfPrefix).append(")"); @@ -833,15 +864,16 @@ public String directivesString(Class parentType, } String directivesString(Class parentType, boolean isDeprecated, GraphQLDirectiveContainer directiveContainer) { - List directives = DirectivesUtil.toAppliedDirectives(directiveContainer); - return directivesString(parentType, isDeprecated, directives); - } - - private String directivesString(Class parentType, boolean isDeprecated, List directives) { + List directives; if (isDeprecated) { - directives = addDeprecatedDirectiveIfNeeded(directives); + directives = addDeprecatedDirectiveIfNeeded(directiveContainer); + } else { + directives = DirectivesUtil.toAppliedDirectives(directiveContainer); } + return directivesString(parentType, directives); + } + private String directivesString(Class parentType, List directives) { directives = directives.stream() // @deprecated is special - we always print it if something is deprecated .filter(directive -> options.getIncludeDirective().test(directive.getName()) || isDeprecatedDirective(directive)) @@ -926,14 +958,43 @@ private boolean hasDeprecatedDirective(List directives) .count() == 1; } - private List addDeprecatedDirectiveIfNeeded(List directives) { + private List addDeprecatedDirectiveIfNeeded(GraphQLDirectiveContainer directiveContainer) { + List directives = DirectivesUtil.toAppliedDirectives(directiveContainer); if (!hasDeprecatedDirective(directives)) { directives = new ArrayList<>(directives); - directives.add(DeprecatedAppliedDirective4Printing); + String reason = getDeprecationReason(directiveContainer); + GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() + .name("reason") + .valueProgrammatic(reason) + .type(GraphQLString) + .build(); + GraphQLAppliedDirective directive = GraphQLAppliedDirective.newDirective() + .name("deprecated") + .argument(arg) + .build(); + directives.add(directive); } return directives; } + private String getDeprecationReason(GraphQLDirectiveContainer directiveContainer) { + if (directiveContainer instanceof GraphQLFieldDefinition) { + GraphQLFieldDefinition type = (GraphQLFieldDefinition) directiveContainer; + return type.getDeprecationReason(); + } else if (directiveContainer instanceof GraphQLEnumValueDefinition) { + GraphQLEnumValueDefinition type = (GraphQLEnumValueDefinition) directiveContainer; + return type.getDeprecationReason(); + } else if (directiveContainer instanceof GraphQLInputObjectField) { + GraphQLInputObjectField type = (GraphQLInputObjectField) directiveContainer; + return type.getDeprecationReason(); + } else if (directiveContainer instanceof GraphQLArgument) { + GraphQLArgument type = (GraphQLArgument) directiveContainer; + return type.getDeprecationReason(); + } else { + return Assert.assertShouldNeverHappen(); + } + } + private String directiveDefinition(GraphQLDirective directive) { StringBuilder sb = new StringBuilder(); @@ -1027,18 +1088,26 @@ private String printComments(Object graphQLType, String prefix) { private void printComments(PrintWriter out, Object graphQLType, String prefix) { String descriptionText = getDescription(graphQLType); - if (isNullOrEmpty(descriptionText)) { - return; + if (!isNullOrEmpty(descriptionText)) { + List lines = Arrays.asList(descriptionText.split("\n")); + if (options.isDescriptionsAsHashComments()) { + printMultiLineHashDescription(out, prefix, lines); + } else if (!lines.isEmpty()) { + if (lines.size() > 1) { + printMultiLineDescription(out, prefix, lines); + } else { + printSingleLineDescription(out, prefix, lines.get(0)); + } + } } - List lines = Arrays.asList(descriptionText.split("\n")); - if (options.isDescriptionsAsHashComments()) { - printMultiLineHashDescription(out, prefix, lines); - } else if (!lines.isEmpty()) { - if (lines.size() > 1) { - printMultiLineDescription(out, prefix, lines); - } else { - printSingleLineDescription(out, prefix, lines.get(0)); + if (options.isIncludeAstDefinitionComments()) { + String commentsText = getAstDefinitionComments(graphQLType); + if (!isNullOrEmpty(commentsText)) { + List lines = Arrays.asList(commentsText.split("\n") ); + if (!lines.isEmpty()) { + printMultiLineHashDescription(out, prefix, lines); + } } } } @@ -1062,6 +1131,61 @@ private void printSingleLineDescription(PrintWriter out, String prefix, String s out.printf("%s\"%s\"\n", prefix, desc); } + private boolean hasAstDefinitionComments(Object commentHolder) { + String comments = getAstDefinitionComments(commentHolder); + return !isNullOrEmpty(comments); + } + + private String getAstDefinitionComments(Object commentHolder) { + if (commentHolder instanceof GraphQLObjectType) { + GraphQLObjectType type = (GraphQLObjectType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(ObjectTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLEnumType) { + GraphQLEnumType type = (GraphQLEnumType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(EnumTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLFieldDefinition) { + GraphQLFieldDefinition type = (GraphQLFieldDefinition) commentHolder; + return comments(ofNullable(type.getDefinition()).map(FieldDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLEnumValueDefinition) { + GraphQLEnumValueDefinition type = (GraphQLEnumValueDefinition) commentHolder; + return comments(ofNullable(type.getDefinition()).map(EnumValueDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLUnionType) { + GraphQLUnionType type = (GraphQLUnionType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(UnionTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLInputObjectType) { + GraphQLInputObjectType type = (GraphQLInputObjectType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(InputObjectTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLInputObjectField) { + GraphQLInputObjectField type = (GraphQLInputObjectField) commentHolder; + return comments(ofNullable(type.getDefinition()).map(InputValueDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLInterfaceType) { + GraphQLInterfaceType type = (GraphQLInterfaceType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(InterfaceTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLScalarType) { + GraphQLScalarType type = (GraphQLScalarType) commentHolder; + return comments(ofNullable(type.getDefinition()).map(ScalarTypeDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLArgument) { + GraphQLArgument type = (GraphQLArgument) commentHolder; + return comments(ofNullable(type.getDefinition()).map(InputValueDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLDirective) { + GraphQLDirective type = (GraphQLDirective) commentHolder; + return comments(ofNullable(type.getDefinition()).map(DirectiveDefinition::getComments).orElse(null)); + } else if (commentHolder instanceof GraphQLSchema) { + GraphQLSchema type = (GraphQLSchema) commentHolder; + return comments(ofNullable(type.getDefinition()).map(SchemaDefinition::getComments).orElse(null)); + } else { + return Assert.assertShouldNeverHappen(); + } + } + + private String comments(List comments) { + if ( comments == null || comments.isEmpty() ) { + return null; + } + String s = comments.stream().map(c -> c.getContent()).collect(joining("\n", "", "\n")); + return s; + } + private boolean hasDescription(Object descriptionHolder) { String description = getDescription(descriptionHolder); return !isNullOrEmpty(description); diff --git a/src/main/java/graphql/schema/usage/SchemaUsageSupport.java b/src/main/java/graphql/schema/usage/SchemaUsageSupport.java index e540534d3b..2f86da5c76 100644 --- a/src/main/java/graphql/schema/usage/SchemaUsageSupport.java +++ b/src/main/java/graphql/schema/usage/SchemaUsageSupport.java @@ -1,6 +1,8 @@ package graphql.schema.usage; import graphql.PublicApi; +import graphql.schema.GraphQLAppliedDirective; +import graphql.schema.GraphQLAppliedDirectiveArgument; import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLFieldDefinition; @@ -20,6 +22,7 @@ import graphql.schema.SchemaTraverser; import graphql.util.TraversalControl; import graphql.util.TraverserContext; +import org.jetbrains.annotations.Nullable; import java.util.HashSet; import java.util.List; @@ -59,6 +62,10 @@ private void recordBackReference(GraphQLNamedSchemaElement referencedElement, Gr String typeName = ((GraphQLDirective) referencingElement).getName(); builder.elementBackReferences.computeIfAbsent(referencedElementName, k -> new HashSet<>()).add(typeName); } + if (referencingElement instanceof GraphQLAppliedDirective) { + String typeName = ((GraphQLAppliedDirective) referencingElement).getName(); + builder.elementBackReferences.computeIfAbsent(referencedElementName, k -> new HashSet<>()).add(typeName); + } } private void memberInterfaces(GraphQLNamedType containingType, List members) { @@ -84,6 +91,19 @@ public TraversalControl visitGraphQLArgument(GraphQLArgument node, TraverserCont return CONTINUE; } + @Override + public TraversalControl visitGraphQLAppliedDirectiveArgument(GraphQLAppliedDirectiveArgument node, TraverserContext context) { + GraphQLNamedType inputType = GraphQLTypeUtil.unwrapAll(node.getType()); + builder.argReferenceCount.compute(inputType.getName(), incCount()); + + GraphQLSchemaElement parentElement = context.getParentNode(); + if (parentElement instanceof GraphQLAppliedDirective) { + parentElement = context.getParentContext().getParentNode(); + } + recordBackReference(inputType, parentElement); + return CONTINUE; + } + @Override public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, TraverserContext context) { GraphQLNamedType fieldType = GraphQLTypeUtil.unwrapAll(node.getType()); @@ -108,11 +128,24 @@ public TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField nod @Override public TraversalControl visitGraphQLDirective(GraphQLDirective directive, TraverserContext context) { + GraphQLSchemaElement parentElement = visitDirectiveLike(context, directive.getName()); + recordBackReference(directive, parentElement); + return CONTINUE; + } + + @Override + public TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective appliedDirective, TraverserContext context) { + GraphQLSchemaElement parentElement = visitDirectiveLike(context, appliedDirective.getName()); + recordBackReference(appliedDirective, parentElement); + return CONTINUE; + } + + private GraphQLSchemaElement visitDirectiveLike(TraverserContext context, String directiveName) { GraphQLSchemaElement parentElement = context.getParentNode(); if (parentElement != null) { // a null parent is a directive definition // we record a count if the directive is applied to something - not just defined - builder.directiveReferenceCount.compute(directive.getName(), incCount()); + builder.directiveReferenceCount.compute(directiveName, incCount()); } if (parentElement instanceof GraphQLArgument) { context = context.getParentContext(); @@ -126,8 +159,7 @@ public TraversalControl visitGraphQLDirective(GraphQLDirective directive, Traver context = context.getParentContext(); parentElement = context.getParentNode(); } - recordBackReference(directive, parentElement); - return CONTINUE; + return parentElement; } @Override diff --git a/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java b/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java new file mode 100644 index 0000000000..87e39fc321 --- /dev/null +++ b/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java @@ -0,0 +1,41 @@ +package graphql.schema.validation; + +import graphql.ExperimentalApi; +import graphql.schema.GraphQLInputObjectField; +import graphql.schema.GraphQLInputObjectType; +import graphql.schema.GraphQLSchemaElement; +import graphql.schema.GraphQLTypeUtil; +import graphql.schema.GraphQLTypeVisitorStub; +import graphql.util.TraversalControl; +import graphql.util.TraverserContext; + +import static java.lang.String.format; + +/* + * Spec: If the Input Object is a OneOf Input Object then: + * The type of the input field must be nullable. + * The input field must not have a default value. + */ +@ExperimentalApi +public class OneOfInputObjectRules extends GraphQLTypeVisitorStub { + + @Override + public TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField inputObjectField, TraverserContext context) { + GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) context.getParentNode(); + if (!inputObjectType.isOneOf()) { + return TraversalControl.CONTINUE; + } + SchemaValidationErrorCollector errorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class); + // error messages take from the reference implementation + if (inputObjectField.hasSetDefaultValue()) { + String message = format("OneOf input field %s.%s cannot have a default value.", inputObjectType.getName(), inputObjectField.getName()); + errorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.OneOfDefaultValueOnField, message)); + } + + if (GraphQLTypeUtil.isNonNull(inputObjectField.getType())) { + String message = format("OneOf input field %s.%s must be nullable.", inputObjectType.getName(), inputObjectField.getName()); + errorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.OneOfNonNullableField, message)); + } + return TraversalControl.CONTINUE; + } +} diff --git a/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java b/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java index 567baf4d2f..08209bb6b2 100644 --- a/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java +++ b/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java @@ -20,4 +20,6 @@ public enum SchemaValidationErrorType implements SchemaValidationErrorClassifica InvalidAppliedDirective, OutputTypeUsedInInputTypeContext, InputTypeUsedInOutputTypeContext, + OneOfDefaultValueOnField, + OneOfNonNullableField } diff --git a/src/main/java/graphql/schema/validation/SchemaValidator.java b/src/main/java/graphql/schema/validation/SchemaValidator.java index 18da8ee7ef..0d8d7f271b 100644 --- a/src/main/java/graphql/schema/validation/SchemaValidator.java +++ b/src/main/java/graphql/schema/validation/SchemaValidator.java @@ -25,6 +25,7 @@ public SchemaValidator() { rules.add(new AppliedDirectivesAreValid()); rules.add(new AppliedDirectiveArgumentsAreValid()); rules.add(new InputAndOutputTypesUsedAppropriately()); + rules.add(new OneOfInputObjectRules()); } public List getRules() { diff --git a/src/main/java/graphql/util/FpKit.java b/src/main/java/graphql/util/FpKit.java index 86ea96374b..a538450ad3 100644 --- a/src/main/java/graphql/util/FpKit.java +++ b/src/main/java/graphql/util/FpKit.java @@ -265,7 +265,7 @@ public static CompletableFuture> flatList(CompletableFuture List flatList(List> listLists) { + public static List flatList(Collection> listLists) { return listLists.stream() .flatMap(List::stream) .collect(ImmutableList.toImmutableList()); diff --git a/src/main/java/graphql/util/InterThreadMemoizedSupplier.java b/src/main/java/graphql/util/InterThreadMemoizedSupplier.java index 8b1a445b02..072d5635cd 100644 --- a/src/main/java/graphql/util/InterThreadMemoizedSupplier.java +++ b/src/main/java/graphql/util/InterThreadMemoizedSupplier.java @@ -5,7 +5,7 @@ import java.util.function.Supplier; /** - * This memoizing supplier DOES use synchronised double locking to set its value. + * This memoizing supplier DOES use locked double locking to set its value. * * @param for two */ @@ -14,6 +14,7 @@ public class InterThreadMemoizedSupplier implements Supplier { private final Supplier delegate; private volatile boolean initialized; + private final LockKit.ReentrantLock lock = new LockKit.ReentrantLock(); private T value; public InterThreadMemoizedSupplier(Supplier delegate) { @@ -24,13 +25,16 @@ public InterThreadMemoizedSupplier(Supplier delegate) { @Override public T get() { if (!initialized) { - synchronized (this) { + lock.lock(); + try { if (initialized) { return value; } value = delegate.get(); initialized = true; return value; + } finally { + lock.unlock(); } } return value; diff --git a/src/main/java/graphql/util/LockKit.java b/src/main/java/graphql/util/LockKit.java new file mode 100644 index 0000000000..f2513d8628 --- /dev/null +++ b/src/main/java/graphql/util/LockKit.java @@ -0,0 +1,87 @@ +package graphql.util; + +import graphql.Internal; + +import java.util.concurrent.locks.Lock; +import java.util.function.Supplier; + +/** + * This provides reentrant locking support for our code base. Future worlds like Loom virtual threads don't like + * synchronised calls since they pin the VT to the carrier thread. Word on the street is that locks are preferred + * to synchronised. + */ +@Internal +public class LockKit { + + /** + * A class to run code inside a reentrant lock + */ + public static class ReentrantLock { + private final Lock lock = new java.util.concurrent.locks.ReentrantLock(); + + /** + * Sometimes you need to directly lock things like for checked exceptions + *

+ * It's on you to unlock it! + */ + public void lock() { + lock.lock(); + } + + public void unlock() { + lock.unlock(); + } + + public void runLocked(Runnable codeToRun) { + lock.lock(); + try { + codeToRun.run(); + } finally { + lock.unlock(); + } + } + + public E callLocked(Supplier codeToRun) { + lock.lock(); + try { + return codeToRun.get(); + } finally { + lock.unlock(); + } + } + } + + + /** + * Will allow for lazy computation of some values just once + */ + public static class ComputedOnce { + + private volatile boolean beenComputed = false; + private final ReentrantLock lock = new ReentrantLock(); + + + public boolean hasBeenComputed() { + return beenComputed; + } + + public void runOnce(Runnable codeToRunOnce) { + if (beenComputed) { + return; + } + lock.runLocked(() -> { + // double lock check + if (beenComputed) { + return; + } + try { + codeToRunOnce.run(); + beenComputed = true; + } finally { + // even for exceptions we will say its computed + beenComputed = true; + } + }); + } + } +} diff --git a/src/main/java/graphql/validation/TraversalContext.java b/src/main/java/graphql/validation/TraversalContext.java index dfd3e4920d..93bbdb62e6 100644 --- a/src/main/java/graphql/validation/TraversalContext.java +++ b/src/main/java/graphql/validation/TraversalContext.java @@ -32,6 +32,7 @@ import graphql.schema.GraphQLType; import graphql.schema.GraphQLUnionType; import graphql.schema.GraphQLUnmodifiedType; +import graphql.schema.InputValueWithState; import java.util.ArrayList; import java.util.List; @@ -47,6 +48,7 @@ public class TraversalContext implements DocumentVisitor { private final List outputTypeStack = new ArrayList<>(); private final List parentTypeStack = new ArrayList<>(); private final List inputTypeStack = new ArrayList<>(); + private final List defaultValueStack = new ArrayList<>(); private final List fieldDefStack = new ArrayList<>(); private final List nameStack = new ArrayList<>(); private GraphQLDirective directive; @@ -155,6 +157,7 @@ private void enterImpl(Argument argument) { } addInputType(argumentType != null ? argumentType.getType() : null); + addDefaultValue(argumentType != null ? argumentType.getArgumentDefaultValue() : null); this.argument = argumentType; } @@ -165,23 +168,30 @@ private void enterImpl(ArrayValue arrayValue) { inputType = (GraphQLInputType) unwrapOne(nullableType); } addInputType(inputType); + // List positions never have a default value. See graphql-js impl for inspiration + addDefaultValue(null); } private void enterImpl(ObjectField objectField) { GraphQLUnmodifiedType objectType = unwrapAll(getInputType()); GraphQLInputType inputType = null; + GraphQLInputObjectField inputField = null; if (objectType instanceof GraphQLInputObjectType) { GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) objectType; - GraphQLInputObjectField inputField = schema.getFieldVisibility().getFieldDefinition(inputObjectType, objectField.getName()); - if (inputField != null) + inputField = schema.getFieldVisibility().getFieldDefinition(inputObjectType, objectField.getName()); + if (inputField != null) { inputType = inputField.getType(); + } } addInputType(inputType); + addDefaultValue(inputField != null ? inputField.getInputFieldDefaultValue() : null); } private GraphQLArgument find(List arguments, String name) { for (GraphQLArgument argument : arguments) { - if (argument.getName().equals(name)) return argument; + if (argument.getName().equals(name)) { + return argument; + } } return null; } @@ -190,29 +200,32 @@ private GraphQLArgument find(List arguments, String name) { @Override public void leave(Node node, List ancestors) { if (node instanceof OperationDefinition) { - outputTypeStack.remove(outputTypeStack.size() - 1); + pop(outputTypeStack); } else if (node instanceof SelectionSet) { - parentTypeStack.remove(parentTypeStack.size() - 1); + pop(parentTypeStack); } else if (node instanceof Field) { leaveName(((Field) node).getName()); - fieldDefStack.remove(fieldDefStack.size() - 1); - outputTypeStack.remove(outputTypeStack.size() - 1); + pop(fieldDefStack); + pop(outputTypeStack); } else if (node instanceof Directive) { directive = null; } else if (node instanceof InlineFragment) { - outputTypeStack.remove(outputTypeStack.size() - 1); + pop(outputTypeStack); } else if (node instanceof FragmentDefinition) { leaveName(((FragmentDefinition) node).getName()); - outputTypeStack.remove(outputTypeStack.size() - 1); + pop(outputTypeStack); } else if (node instanceof VariableDefinition) { inputTypeStack.remove(inputTypeStack.size() - 1); } else if (node instanceof Argument) { argument = null; - inputTypeStack.remove(inputTypeStack.size() - 1); + pop(inputTypeStack); + pop(defaultValueStack); } else if (node instanceof ArrayValue) { - inputTypeStack.remove(inputTypeStack.size() - 1); + pop(inputTypeStack); + pop(defaultValueStack); } else if (node instanceof ObjectField) { - inputTypeStack.remove(inputTypeStack.size() - 1); + pop(inputTypeStack); + pop(defaultValueStack); } } @@ -249,10 +262,16 @@ private void addOutputType(GraphQLOutputType type) { } private T lastElement(List list) { - if (list.size() == 0) return null; + if (list.isEmpty()) { + return null; + } return list.get(list.size() - 1); } + private T pop(List list) { + return list.remove(list.size() - 1); + } + /** * @return can be null if the parent is not a CompositeType */ @@ -267,11 +286,18 @@ private void addParentType(GraphQLCompositeType compositeType) { public GraphQLInputType getInputType() { return lastElement(inputTypeStack); } + public InputValueWithState getDefaultValue() { + return lastElement(defaultValueStack); + } private void addInputType(GraphQLInputType graphQLInputType) { inputTypeStack.add(graphQLInputType); } + private void addDefaultValue(InputValueWithState defaultValue) { + defaultValueStack.add(defaultValue); + } + public GraphQLFieldDefinition getFieldDef() { return lastElement(fieldDefStack); } diff --git a/src/main/java/graphql/validation/ValidationContext.java b/src/main/java/graphql/validation/ValidationContext.java index 2646afcdbb..18fe678177 100644 --- a/src/main/java/graphql/validation/ValidationContext.java +++ b/src/main/java/graphql/validation/ValidationContext.java @@ -14,6 +14,7 @@ import graphql.schema.GraphQLInputType; import graphql.schema.GraphQLOutputType; import graphql.schema.GraphQLSchema; +import graphql.schema.InputValueWithState; import java.util.LinkedHashMap; import java.util.List; @@ -41,8 +42,10 @@ public ValidationContext(GraphQLSchema schema, Document document, I18n i18n) { } private void buildFragmentMap() { - for (Definition definition : document.getDefinitions()) { - if (!(definition instanceof FragmentDefinition)) continue; + for (Definition definition : document.getDefinitions()) { + if (!(definition instanceof FragmentDefinition)) { + continue; + } FragmentDefinition fragmentDefinition = (FragmentDefinition) definition; fragmentDefinitionMap.put(fragmentDefinition.getName(), fragmentDefinition); } @@ -72,6 +75,10 @@ public GraphQLInputType getInputType() { return traversalContext.getInputType(); } + public InputValueWithState getDefaultValue() { + return traversalContext.getDefaultValue(); + } + public GraphQLFieldDefinition getFieldDef() { return traversalContext.getFieldDef(); } diff --git a/src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java b/src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java index 526d11e514..a674e46929 100644 --- a/src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java +++ b/src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java @@ -135,7 +135,7 @@ private void collectFieldsForField(Map> fieldMap, Grap GraphQLFieldDefinition fieldDefinition = getVisibleFieldDefinition(fieldsContainer, field); fieldType = fieldDefinition != null ? fieldDefinition.getType() : null; } - fieldMap.get(responseName).add(new FieldAndType(field, fieldType, parentType)); + fieldMap.get(responseName).add(new FieldAndType(field, fieldType, unwrappedParent)); } private GraphQLFieldDefinition getVisibleFieldDefinition(GraphQLFieldsContainer fieldsContainer, Field field) { diff --git a/src/main/java/graphql/validation/rules/VariableTypesMatch.java b/src/main/java/graphql/validation/rules/VariableTypesMatch.java index de395850af..ef7c4069f4 100644 --- a/src/main/java/graphql/validation/rules/VariableTypesMatch.java +++ b/src/main/java/graphql/validation/rules/VariableTypesMatch.java @@ -59,24 +59,25 @@ public void checkVariable(VariableReference variableReference) { if (variableType == null) { return; } - GraphQLInputType expectedType = getValidationContext().getInputType(); - Optional schemaDefault = Optional.ofNullable(getValidationContext().getArgument()).map(v -> v.getArgumentDefaultValue()); - Value schemaDefaultValue = null; - if (schemaDefault.isPresent() && schemaDefault.get().isLiteral()) { - schemaDefaultValue = (Value) schemaDefault.get().getValue(); - } else if (schemaDefault.isPresent() && schemaDefault.get().isSet()) { - schemaDefaultValue = ValuesResolver.valueToLiteral(schemaDefault.get(), expectedType, getValidationContext().getGraphQLContext(), getValidationContext().getI18n().getLocale()); - } - if (expectedType == null) { - // we must have a unknown variable say to not have a known type + GraphQLInputType locationType = getValidationContext().getInputType(); + Optional locationDefault = Optional.ofNullable(getValidationContext().getDefaultValue()); + if (locationType == null) { + // we must have an unknown variable say to not have a known type return; } - if (!variablesTypesMatcher.doesVariableTypesMatch(variableType, variableDefinition.getDefaultValue(), expectedType) && - !variablesTypesMatcher.doesVariableTypesMatch(variableType, schemaDefaultValue, expectedType)) { + Value locationDefaultValue = null; + if (locationDefault.isPresent() && locationDefault.get().isLiteral()) { + locationDefaultValue = (Value) locationDefault.get().getValue(); + } else if (locationDefault.isPresent() && locationDefault.get().isSet()) { + locationDefaultValue = ValuesResolver.valueToLiteral(locationDefault.get(), locationType, getValidationContext().getGraphQLContext(), getValidationContext().getI18n().getLocale()); + } + boolean variableDefMatches = variablesTypesMatcher.doesVariableTypesMatch(variableType, variableDefinition.getDefaultValue(), locationType, locationDefaultValue); + if (!variableDefMatches) { GraphQLType effectiveType = variablesTypesMatcher.effectiveType(variableType, variableDefinition.getDefaultValue()); String message = i18n(VariableTypeMismatch, "VariableTypesMatchRule.unexpectedType", + variableDefinition.getName(), GraphQLTypeUtil.simplePrint(effectiveType), - GraphQLTypeUtil.simplePrint(expectedType)); + GraphQLTypeUtil.simplePrint(locationType)); addError(VariableTypeMismatch, variableReference.getSourceLocation(), message); } } diff --git a/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java b/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java index 8208eba393..0b47fcec78 100644 --- a/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java +++ b/src/main/java/graphql/validation/rules/VariablesTypesMatcher.java @@ -9,16 +9,38 @@ import static graphql.schema.GraphQLNonNull.nonNull; import static graphql.schema.GraphQLTypeUtil.isList; import static graphql.schema.GraphQLTypeUtil.isNonNull; +import static graphql.schema.GraphQLTypeUtil.unwrapNonNull; import static graphql.schema.GraphQLTypeUtil.unwrapOne; @Internal public class VariablesTypesMatcher { - public boolean doesVariableTypesMatch(GraphQLType variableType, Value variableDefaultValue, GraphQLType expectedType) { - return checkType(effectiveType(variableType, variableDefaultValue), expectedType); + /** + * This method and variable naming was inspired from the reference graphql-js implementation + * + * @param varType the variable type + * @param varDefaultValue the default value for the variable + * @param locationType the location type where the variable was encountered + * @param locationDefaultValue the default value for that location + * + * @return true if the variable matches ok + */ + public boolean doesVariableTypesMatch(GraphQLType varType, Value varDefaultValue, GraphQLType locationType, Value locationDefaultValue) { + if (isNonNull(locationType) && !isNonNull(varType)) { + boolean hasNonNullVariableDefaultValue = + varDefaultValue != null && !(varDefaultValue instanceof NullValue); + boolean hasLocationDefaultValue = locationDefaultValue != null; + if (!hasNonNullVariableDefaultValue && !hasLocationDefaultValue) { + return false; + } + GraphQLType nullableLocationType = unwrapNonNull(locationType); + return checkType(varType, nullableLocationType); + } + return checkType(varType, locationType); } - public GraphQLType effectiveType(GraphQLType variableType, Value defaultValue) { + + public GraphQLType effectiveType(GraphQLType variableType, Value defaultValue) { if (defaultValue == null || defaultValue instanceof NullValue) { return variableType; } diff --git a/src/main/resources/i18n/Execution.properties b/src/main/resources/i18n/Execution.properties index 62b872a9d3..396c87025f 100644 --- a/src/main/resources/i18n/Execution.properties +++ b/src/main/resources/i18n/Execution.properties @@ -3,4 +3,6 @@ # # REMEMBER - a single quote ' in MessageFormat means things that are never replaced within them # so use 2 '' characters to make it one ' on output. This will take for the form ''{0}'' -# \ No newline at end of file +# +Execution.handleOneOfNotOneFieldError=Exactly one key must be specified for OneOf type ''{0}''. +Execution.handleOneOfValueIsNullError=OneOf type field ''{0}'' must be non-null. diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties index 9733f79738..82c754db3e 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -76,7 +76,7 @@ VariableDefaultValuesOfCorrectType.badDefault=Validation error ({0}) : Bad defau # VariablesAreInputTypes.wrongType=Validation error ({0}) : Input variable ''{1}'' type ''{2}'' is not an input type # -VariableTypesMatchRule.unexpectedType=Validation error ({0}) : Variable type ''{1}'' does not match expected type ''{2}'' +VariableTypesMatchRule.unexpectedType=Validation error ({0}) : Variable ''{1}'' of type ''{2}'' used in position expecting type ''{3}'' # UniqueObjectFieldName.duplicateFieldName=Validation Error ({0}) : There can be only one field named ''{1}'' # @@ -98,4 +98,3 @@ ArgumentValidationUtil.handleNotObjectError=Validation error ({0}) : argument '' ArgumentValidationUtil.handleMissingFieldsError=Validation error ({0}) : argument ''{1}'' with value ''{2}'' is missing required fields ''{3}'' # suppress inspection "UnusedProperty" ArgumentValidationUtil.handleExtraFieldError=Validation error ({0}) : argument ''{1}'' with value ''{2}'' contains a field not in ''{3}'': ''{4}'' -# diff --git a/src/main/resources/i18n/Validation_de.properties b/src/main/resources/i18n/Validation_de.properties index def39f94ea..c15e3bb550 100644 --- a/src/main/resources/i18n/Validation_de.properties +++ b/src/main/resources/i18n/Validation_de.properties @@ -76,7 +76,7 @@ VariableDefaultValuesOfCorrectType.badDefault=Validierungsfehler ({0}) : Ung # VariablesAreInputTypes.wrongType=Validierungsfehler ({0}) : Eingabevariable ''{1}'' Typ ''{2}'' ist kein Eingabetyp # -VariableTypesMatchRule.unexpectedType=Validierungsfehler ({0}) : Der Variablentyp ''{1}'' stimmt nicht mit dem erwarteten Typ ''{2}'' überein +VariableTypesMatchRule.unexpectedType=Validierungsfehler ({0}) : Variable ''{1}'' vom Typ ''{2}'' verwendet in Position, die Typ ''{3}'' erwartet # # These are used but IDEA cant find them easily as being called # diff --git a/src/test/groovy/graphql/Issue2141.groovy b/src/test/groovy/graphql/Issue2141.groovy index 145a984b95..100058c710 100644 --- a/src/test/groovy/graphql/Issue2141.groovy +++ b/src/test/groovy/graphql/Issue2141.groovy @@ -36,6 +36,9 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." diff --git a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy index 2c0f2aaf71..9b374bfbd3 100644 --- a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy +++ b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy @@ -430,6 +430,6 @@ class StarWarsIntrospectionTests extends Specification { schemaParts.get('mutationType').size() == 1 schemaParts.get('subscriptionType') == null schemaParts.get('types').size() == 17 - schemaParts.get('directives').size() == 4 + schemaParts.get('directives').size() == 5 } } diff --git a/src/test/groovy/graphql/execution/ExecutionStrategyErrorsTest.groovy b/src/test/groovy/graphql/execution/ExecutionStrategyErrorsTest.groovy new file mode 100644 index 0000000000..b2ffa9e3cf --- /dev/null +++ b/src/test/groovy/graphql/execution/ExecutionStrategyErrorsTest.groovy @@ -0,0 +1,132 @@ +package graphql.execution + +import graphql.ExceptionWhileDataFetching +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.GraphQL +import graphql.SerializationError +import graphql.TestUtil +import graphql.TypeMismatchError +import graphql.execution.instrumentation.Instrumentation +import graphql.execution.instrumentation.InstrumentationContext +import graphql.execution.instrumentation.InstrumentationState +import graphql.execution.instrumentation.SimplePerformantInstrumentation +import graphql.execution.instrumentation.parameters.InstrumentationFieldCompleteParameters +import graphql.schema.DataFetcher +import spock.lang.Specification + +/** + * A test of errors that can happen inside a ES + */ +class ExecutionStrategyErrorsTest extends Specification { + + def "can capture certain errors"() { + def sdl = ''' + type Query { + notAList : [String] + notAFloat : Float + notAnProperObject : ImproperObject + } + + + type ImproperObject { + name : String + diceyListCall : [DiceyCall!]! + diceyListCallAbort : [DiceyCall!]! + diceyCall : DiceyCall + } + + type DiceyCall { + bang : String + abort : String + nonNull : String! + } + ''' + + DataFetcher dfNotAList = { env -> "noAList" } + DataFetcher dfNotAFloat = { env -> "noAFloat" } + DataFetcher dfDiceyBang = { + env -> throw new RuntimeException("dicey call") + } + DataFetcher dfDiceyAbort = { + env -> throw new AbortExecutionException("abort abort") + } + DataFetcher dfDiceyListCall = { + env -> ["x", null] + } + DataFetcher dfReturnsNull = { + env -> null + } + + def schema = TestUtil.schema(sdl, [ + Query : + [notAList: dfNotAList, notAFloat: dfNotAFloat], + ImproperObject: + [diceyListCall: dfDiceyListCall], + DiceyCall : + [bang: dfDiceyBang, abort: dfDiceyAbort, nonNull: dfReturnsNull], + ] + ) + + Instrumentation instrumentation = new SimplePerformantInstrumentation() { + @Override + InstrumentationContext beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { + if (parameters.field.name == "diceyListCallAbort") { + throw new AbortExecutionException("No lists for you") + } + return super.beginFieldListComplete(parameters, state) + } + } + def graphQL = GraphQL.newGraphQL(schema).instrumentation(instrumentation).build() + + + when: + def ei = ExecutionInput.newExecutionInput() + .query(""" + query q { + notAList + notAFloat + notAnProperObject { + name + diceyListCallAbort { + bang + } + diceyListCall { + bang + abort + nonNull + } + } + } + """) + .root([notAnProperObject: ["name" : "make it drive errors", + "diceyListCall" : [[:]], + "diceyListCallAbort": [[:]], + "diceyCall" : [:] + ]]) + .build() + def er = graphQL.execute(ei) + + then: + er.errors.size() == 6 + er.errors[0] instanceof TypeMismatchError + er.errors[0].path == ["notAList"] + + er.errors[1] instanceof SerializationError + er.errors[1].path == ["notAFloat"] + + er.errors[2] instanceof ExceptionWhileDataFetching + er.errors[2].path ==["notAnProperObject", "diceyListCall", 0, "bang"] + ((ExceptionWhileDataFetching)er.errors[2]).exception.message == "dicey call" + + er.errors[3] instanceof ExceptionWhileDataFetching + er.errors[3].path ==["notAnProperObject", "diceyListCall", 0, "abort"] + ((ExceptionWhileDataFetching)er.errors[3]).exception.message == "abort abort" + + er.errors[4] instanceof NonNullableFieldWasNullError + er.errors[4].path ==["notAnProperObject", "diceyListCall", 0, "nonNull"] + + er.errors[5] instanceof NonNullableFieldWasNullError + er.errors[5].path ==["notAnProperObject", "diceyListCall", 1] // the entry itself was null in a non null list entry + } +} diff --git a/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy b/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy new file mode 100644 index 0000000000..590d84e277 --- /dev/null +++ b/src/test/groovy/graphql/execution/ValuesResolverE2ETest.groovy @@ -0,0 +1,136 @@ +package graphql.execution + +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.GraphQL +import graphql.Scalars +import graphql.TestUtil +import graphql.schema.DataFetcher +import graphql.schema.DataFetchingEnvironment +import graphql.schema.FieldCoordinates +import graphql.schema.GraphQLCodeRegistry +import graphql.schema.GraphQLInputObjectType +import graphql.schema.GraphQLList +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLSchema +import spock.lang.Specification + +class ValuesResolverE2ETest extends Specification { + + def "issue 3276 - reported bug on validation problems as SDL"() { + def sdl = ''' + type Query { + items(pagination: Pagination = {limit: 10, offset: 0}): [String] + } + + input Pagination { + limit: Int + offset: Int + } + ''' + DataFetcher df = { DataFetchingEnvironment env -> + def pagination = env.getArgument("pagination") as Map + def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value } + return strings + } + def schema = TestUtil.schema(sdl, [Query: [items: df]]) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + def ei = ExecutionInput.newExecutionInput(''' + query Items($limit: Int, $offset: Int) { + items(pagination: {limit: $limit, offset: $offset}) + } + ''').variables([limit: 5, offset: 0]).build() + def er = graphQL.execute(ei) + then: + er.errors.isEmpty() + er.data == [items : ["limit=5", "offset=0"]] + } + + def "issue 3276 - reported bug on validation problems as reported code"() { + DataFetcher dataFetcher = { env -> + def pagination = env.getArgument("pagination") as Map + def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value } + return strings + } + GraphQLSchema schema = GraphQLSchema.newSchema() + .query(GraphQLObjectType.newObject() + .name("Query") + .field(items -> items + .name("items") + .type(GraphQLList.list(Scalars.GraphQLString)) + .argument(pagination -> pagination + .name("pagination") + //skipped adding the default limit/offset values as it doesn't change anything + .defaultValueProgrammatic(new HashMap<>()) + .type(GraphQLInputObjectType.newInputObject() + .name("Pagination") + .field(limit -> limit + .name("limit") + .type(Scalars.GraphQLInt)) + .field(offset -> offset + .name("offset") + .type(Scalars.GraphQLInt)) + .build()))) + .build()) + .codeRegistry(GraphQLCodeRegistry.newCodeRegistry() + .dataFetcher(FieldCoordinates.coordinates("Query", "items"), dataFetcher) + .build()) + .build() + + GraphQL gql = GraphQL.newGraphQL(schema).build() + + Map vars = new HashMap<>() + vars.put("limit", 5) + vars.put("offset", 0) + + ExecutionInput ei = ExecutionInput.newExecutionInput() + .query("query Items( \$limit: Int, \$offset: Int) {\n" + + " items(\n" + + " pagination: {limit: \$limit, offset: \$offset} \n" + + " )\n" + + "}") + .variables(vars) + .build() + + when: + ExecutionResult result = gql.execute( ei) + then: + result.errors.isEmpty() + result.data == [items : ["limit=5", "offset=0"]] + } + + def "issue 3276 - should end up in validation errors because location defaults are not present"() { + def sdl = ''' + type Query { + items(pagination: Pagination = {limit: 1, offset: 1}): [String] + } + input Pagination { + limit: Int! #non-null this time, no default value + offset: Int! #non-null this time, no default value + } + ''' + DataFetcher df = { DataFetchingEnvironment env -> + def pagination = env.getArgument("pagination") as Map + def strings = pagination.entrySet().collect { entry -> entry.key + "=" + entry.value } + return strings + } + def schema = TestUtil.schema(sdl, [Query: [items: df]]) + def graphQL = GraphQL.newGraphQL(schema).build() + + when: + def ei = ExecutionInput.newExecutionInput(''' + query Items( $limit: Int, $offset: Int) { + items( + pagination: {limit: $limit, offset: $offset} + ) + } + ''').variables([limit: 5, offset: null]).build() + def er = graphQL.execute(ei) + then: + er.errors.size() == 2 + er.errors[0].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'limit' of type 'Int' used in position expecting type 'Int!'" + er.errors[1].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'offset' of type 'Int' used in position expecting type 'Int!'" + } +} diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 3d9a94ebf5..dea17a2c46 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -1,5 +1,6 @@ package graphql.execution +import graphql.Directives import graphql.ErrorType import graphql.ExecutionInput import graphql.GraphQLContext @@ -137,22 +138,22 @@ class ValuesResolverTest extends Specification { given: def schema = TestUtil.schemaWithInputType(list(GraphQLString)) VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String"))) - List value = ["hello","world"] + List value = ["hello", "world"] when: def resolvedValues = ValuesResolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: value]), graphQLContext, locale) then: - resolvedValues.get('variable') == ['hello','world'] + resolvedValues.get('variable') == ['hello', 'world'] } def "getVariableValues: array value gets resolved to a list when the type is a List"() { given: def schema = TestUtil.schemaWithInputType(list(GraphQLString)) VariableDefinition variableDefinition = new VariableDefinition("variable", new ListType(new TypeName("String"))) - String[] value = ["hello","world"] as String[] + String[] value = ["hello", "world"] as String[] when: def resolvedValues = ValuesResolver.coerceVariableValues(schema, [variableDefinition], RawVariables.of([variable: value]), graphQLContext, locale) then: - resolvedValues.get('variable') == ['hello','world'] + resolvedValues.get('variable') == ['hello', 'world'] } def "getArgumentValues: resolves argument with variable reference"() { @@ -345,6 +346,162 @@ class ValuesResolverTest extends Specification { values['arg'] == ['world'] } + def "getArgumentValues: invalid oneOf input because of duplicate keys - #testCase"() { + given: "schema defining input object" + def inputObjectType = newInputObject() + .name("oneOfInputObject") + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .field(newInputObjectField() + .name("a") + .type(GraphQLString) + .build()) + .field(newInputObjectField() + .name("b") + .type(GraphQLInt) + .build()) + .build() + def fieldArgument = newArgument().name("arg").type(inputObjectType).build() + + when: + def argument = new Argument("arg", inputValue) + ValuesResolver.getArgumentValues([fieldArgument], [argument], variables, graphQLContext, locale) + + then: + def e = thrown(OneOfTooManyKeysException) + e.message == "Exactly one key must be specified for OneOf type 'oneOfInputObject'." + + where: + // from https://github.com/graphql/graphql-spec/pull/825/files#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R1692 + testCase | inputValue | variables + + '{ a: "abc", b: 123 } {}' | buildObjectLiteral([ + a: StringValue.of("abc"), + b: IntValue.of(123) + ]) | CoercedVariables.emptyVariables() + + '{ a: null, b: 123 } {}' | buildObjectLiteral([ + a: NullValue.of(), + b: IntValue.of(123) + ]) | CoercedVariables.emptyVariables() + + '{ a: $var, b: 123 } { var: null }' | buildObjectLiteral([ + a: VariableReference.of("var"), + b: IntValue.of(123) + ]) | CoercedVariables.of(["var": null]) + + '{ a: $var, b: 123 } {}' | buildObjectLiteral([ + a: VariableReference.of("var"), + b: IntValue.of(123) + ]) | CoercedVariables.emptyVariables() + + '{ a : "abc", b : null} {}' | buildObjectLiteral([ + a: StringValue.of("abc"), + b: NullValue.of() + ]) | CoercedVariables.emptyVariables() + + '{ a : null, b : null} {}' | buildObjectLiteral([ + a: NullValue.of(), + b: NullValue.of() + ]) | CoercedVariables.emptyVariables() + + '{ a : $a, b : $b} {a : "abc"}' | buildObjectLiteral([ + a: VariableReference.of("a"), + b: VariableReference.of("v") + ]) | CoercedVariables.of(["a": "abc"]) + + '$var {var : { a : "abc", b : 123}}' | VariableReference.of("var") + | CoercedVariables.of(["var": ["a": "abc", "b": 123]]) + + '$var {var : {}}' | VariableReference.of("var") + | CoercedVariables.of(["var": [:]]) + } + + def "getArgumentValues: invalid oneOf input because of null value - #testCase"() { + given: "schema defining input object" + def inputObjectType = newInputObject() + .name("oneOfInputObject") + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .field(newInputObjectField() + .name("a") + .type(GraphQLString) + .build()) + .field(newInputObjectField() + .name("b") + .type(GraphQLInt) + .build()) + .build() + def fieldArgument = newArgument().name("arg").type(inputObjectType).build() + + when: + def argument = new Argument("arg", inputValue) + ValuesResolver.getArgumentValues([fieldArgument], [argument], variables, graphQLContext, locale) + + then: + def e = thrown(OneOfNullValueException) + e.message == "OneOf type field 'oneOfInputObject.a' must be non-null." + + where: + // from https://github.com/graphql/graphql-spec/pull/825/files#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R1692 + testCase | inputValue | variables + + '`{ a: null }` {}' | buildObjectLiteral([ + a: NullValue.of() + ]) | CoercedVariables.emptyVariables() + + '`{ a: $var }` { var : null}' | buildObjectLiteral([ + a: VariableReference.of("var") + ]) | CoercedVariables.of(["var": null]) + + } + + def "getArgumentValues: valid oneOf input - #testCase"() { + given: "schema defining input object" + def inputObjectType = newInputObject() + .name("oneOfInputObject") + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .field(newInputObjectField() + .name("a") + .type(GraphQLString) + .build()) + .field(newInputObjectField() + .name("b") + .type(GraphQLInt) + .build()) + .build() + def fieldArgument = newArgument().name("arg").type(inputObjectType).build() + + when: + def argument = new Argument("arg", inputValue) + def values = ValuesResolver.getArgumentValues([fieldArgument], [argument], variables, graphQLContext, locale) + + then: + values == expectedValues + + where: + // from https://github.com/graphql/graphql-spec/pull/825/files#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R1692 + testCase | inputValue | variables | expectedValues + + '{ b: 123 }` {}' | buildObjectLiteral([ + b: IntValue.of(123) + ]) | CoercedVariables.emptyVariables() | [arg: [b: 123]] + + '`$var` { var: { b: 123 } }' | VariableReference.of("var") + | CoercedVariables.of([var: [b: 123]]) | [arg: [b: 123]] + + '{ a: "abc" }` {}' | buildObjectLiteral([ + a: StringValue.of("abc") + ]) | CoercedVariables.emptyVariables() | [arg: [a: "abc"]] + + + '`$var` { var: { a: "abc" } }' | VariableReference.of("var") + | CoercedVariables.of([var: [a: "abc"]]) | [arg: [a: "abc"]] + + '{ a: $var }` { var : "abc"}' | buildObjectLiteral([ + a: VariableReference.of("var") + ]) | CoercedVariables.of([var: "abc"]) | [arg: [a: "abc"]] + + } + def "getVariableValues: enum as variable input"() { given: def enumDef = newEnum() @@ -594,7 +751,7 @@ class ValuesResolverTest extends Specification { def executionInput = ExecutionInput.newExecutionInput() .query(mutation) - .variables([input: [name: 'Name', position: 'UNKNOWN_POSITION'] ]) + .variables([input: [name: 'Name', position: 'UNKNOWN_POSITION']]) .build() def executionResult = graphQL.execute(executionInput) @@ -632,7 +789,7 @@ class ValuesResolverTest extends Specification { def executionInput = ExecutionInput.newExecutionInput() .query(mutation) - .variables([input: [name: 'Name', hilarious: 'sometimes'] ]) + .variables([input: [name: 'Name', hilarious: 'sometimes']]) .build() def executionResult = graphQL.execute(executionInput) @@ -670,7 +827,7 @@ class ValuesResolverTest extends Specification { def executionInput = ExecutionInput.newExecutionInput() .query(mutation) - .variables([input: [name: 'Name', laughsPerMinute: 'none'] ]) + .variables([input: [name: 'Name', laughsPerMinute: 'none']]) .build() def executionResult = graphQL.execute(executionInput) diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy index 80c2a861d1..227681b555 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy @@ -16,6 +16,8 @@ class QueryDirectivesImplTest extends Specification { directive @cached(forMillis : Int = 99) on FIELD | QUERY directive @upper(place : String) on FIELD + + directive @rep(place : String) repeatable on FIELD type Query { f : String @@ -24,9 +26,7 @@ class QueryDirectivesImplTest extends Specification { def schema = TestUtil.schema(sdl) - def "can get immediate directives"() { - def f1 = TestUtil.parseField("f1 @cached @upper") def f2 = TestUtil.parseField("f2 @cached(forMillis : \$var) @timeout") @@ -68,7 +68,6 @@ class QueryDirectivesImplTest extends Specification { } def "builder works as expected"() { - def f1 = TestUtil.parseField("f1 @cached @upper") def f2 = TestUtil.parseField("f2 @cached(forMillis : \$var) @timeout") @@ -87,6 +86,28 @@ class QueryDirectivesImplTest extends Specification { then: appliedDirectivesByName.keySet().sort() == ["cached", "timeout", "upper"] + } + + def "gets repeated definitions"() { + def f1 = TestUtil.parseField("f1 @rep(place: \$var) @rep(place: \"HELLO\")") + def mergedField = MergedField.newMergedField([f1]).build() + + def queryDirectives = QueryDirectives.newQueryDirectives() + .mergedField(mergedField) + .schema(schema) + .coercedVariables(CoercedVariables.of([var: "ABC"])) + .graphQLContext(GraphQLContext.getDefault()) + .locale(Locale.getDefault()) + .build() + + when: + def appliedDirectivesByName = queryDirectives.getImmediateAppliedDirectivesByName() + + then: + appliedDirectivesByName.keySet() == ["rep"] as Set + appliedDirectivesByName["rep"].size() == 2 + // Groovy is a pathway to many abilities some consider to be unnatural + appliedDirectivesByName["rep"].arguments.value.flatten().sort() == ["ABC", "HELLO"] } } diff --git a/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy b/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy index c2c1ffb8ca..f1812e3955 100644 --- a/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/ChainedInstrumentationStateTest.groovy @@ -1,17 +1,13 @@ package graphql.execution.instrumentation +import graphql.ExecutionInput import graphql.ExecutionResult import graphql.GraphQL import graphql.StarWarsSchema import graphql.execution.AsyncExecutionStrategy -import graphql.execution.instrumentation.parameters.InstrumentationExecuteOperationParameters +import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters -import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters -import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters -import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters -import graphql.language.Document -import graphql.schema.DataFetcher import graphql.validation.ValidationError import spock.lang.Specification @@ -279,6 +275,75 @@ class ChainedInstrumentationStateTest extends Specification { } + + class StringInstrumentationState implements InstrumentationState { + StringInstrumentationState(String value) { + this.value = value + } + + String value + } + + def "can have an multiple async createState() calls in play"() { + + + given: + + def query = '''query Q($var: String!) { + human(id: $var) { + id + name + } + } + ''' + + + def instrumentation1 = new SimplePerformantInstrumentation() { + @Override + CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + return CompletableFuture.supplyAsync { + return new StringInstrumentationState("I1") + } as CompletableFuture + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return CompletableFuture.completedFuture( + executionResult.transform { it.addExtension("i1", ((StringInstrumentationState) state).value) } + ) + } + } + def instrumentation2 = new SimplePerformantInstrumentation() { + @Override + CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + return CompletableFuture.supplyAsync { + return new StringInstrumentationState("I2") + } as CompletableFuture + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return CompletableFuture.completedFuture( + executionResult.transform { it.addExtension("i2", ((StringInstrumentationState) state).value) } + ) + } + + } + + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .instrumentation(new ChainedInstrumentation([instrumentation1, instrumentation2])) + .doNotAddDefaultInstrumentations() // important, otherwise a chained one wil be used + .build() + + when: + def variables = [var: "1001"] + def er = graphQL.execute(ExecutionInput.newExecutionInput().query(query).variables(variables)) // Luke + + then: + er.extensions == [i1: "I1", i2: "I2"] + } + private void assertCalls(NamedInstrumentation instrumentation) { assert instrumentation.dfInvocations[0].getFieldDefinition().name == 'hero' assert instrumentation.dfInvocations[0].getExecutionStepInfo().getPath().toList() == ['hero'] diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy index 694e100c0d..85b274089a 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy @@ -5,6 +5,7 @@ import graphql.ExecutionResult import graphql.GraphQL import graphql.StarWarsSchema import graphql.execution.AsyncExecutionStrategy +import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters @@ -404,4 +405,56 @@ class InstrumentationTest extends Specification { instrumentation.executionList == expected } + + class StringInstrumentationState implements InstrumentationState { + StringInstrumentationState(String value) { + this.value = value + } + + String value + } + + def "can have an single async createState() in play"() { + + + given: + + def query = '''query Q($var: String!) { + human(id: $var) { + id + name + } + } + ''' + + + def instrumentation1 = new SimplePerformantInstrumentation() { + @Override + CompletableFuture createStateAsync(InstrumentationCreateStateParameters parameters) { + return CompletableFuture.supplyAsync { + return new StringInstrumentationState("I1") + } as CompletableFuture + } + + @Override + CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters, InstrumentationState state) { + return CompletableFuture.completedFuture( + executionResult.transform { it.addExtension("i1", ((StringInstrumentationState) state).value) } + ) + } + } + + def graphQL = GraphQL + .newGraphQL(StarWarsSchema.starWarsSchema) + .instrumentation(instrumentation1) + .doNotAddDefaultInstrumentations() // important, otherwise a chained one wil be used + .build() + + when: + def variables = [var: "1001"] + def er = graphQL.execute(ExecutionInput.newExecutionInput().query(query).variables(variables)) // Luke + + then: + er.extensions == [i1: "I1"] + } } diff --git a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy index 712ac8048c..62e27138d1 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy @@ -1,5 +1,6 @@ package graphql.introspection + import graphql.TestUtil import graphql.schema.DataFetcher import graphql.schema.FieldCoordinates @@ -438,7 +439,7 @@ class IntrospectionTest extends Specification { def "test AST printed introspection query is equivalent to original string"() { when: - def oldIntrospectionQuery = "\n" + + def oldIntrospectionQuery = "\n" + " query IntrospectionQuery {\n" + " __schema {\n" + " queryType { name }\n" + @@ -463,6 +464,7 @@ class IntrospectionTest extends Specification { " kind\n" + " name\n" + " description\n" + + " isOneOf\n" + " fields(includeDeprecated: true) {\n" + " name\n" + " description\n" + @@ -540,12 +542,13 @@ class IntrospectionTest extends Specification { " }\n" + "\n" - def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY; + def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY + then: - oldIntrospectionQuery.replaceAll("\\s+","").equals( - newIntrospectionQuery.replaceAll("\\s+","") - ) + def oldQuery = oldIntrospectionQuery.replaceAll("\\s+", "") + def newQuery = newIntrospectionQuery.replaceAll("\\s+","") + oldQuery == newQuery } def "test parameterized introspection queries"() { @@ -582,44 +585,107 @@ class IntrospectionTest extends Specification { def parseExecutionResult = { [ - it.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "notDeprecated"}["description"] != null, // descriptions is true - it.data["__schema"]["types"].find{it["name"] == "UUID"}["specifiedByURL"] != null, // specifiedByUrl is true - it.data["__schema"]["directives"].find{it["name"] == "repeatableDirective"}["isRepeatable"] != null, // directiveIsRepeatable is true - it.data["__schema"]["description"] != null, // schemaDescription is true - it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true + it.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "notDeprecated" }["description"] != null, // descriptions is true + it.data["__schema"]["types"].find { it["name"] == "UUID" }["specifiedByURL"] != null, // specifiedByUrl is true + it.data["__schema"]["directives"].find { it["name"] == "repeatableDirective" }["isRepeatable"] != null, // directiveIsRepeatable is true + it.data["__schema"]["description"] != null, // schemaDescription is true + it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true ] } when: - def allFalseExecutionResult = graphQL.execute( + def allFalseExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(false) - .specifiedByUrl(false) - .directiveIsRepeatable(false) - .schemaDescription(false) - .inputValueDeprecation(false) - .typeRefFragmentDepth(5) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(false) + .specifiedByUrl(false) + .directiveIsRepeatable(false) + .schemaDescription(false) + .inputValueDeprecation(false) + .typeRefFragmentDepth(5) ) - ) + ) then: - !parseExecutionResult(allFalseExecutionResult).any() - allFalseExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 + !parseExecutionResult(allFalseExecutionResult).any() + allFalseExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 when: - def allTrueExecutionResult = graphQL.execute( + def allTrueExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(true) - .specifiedByUrl(true) - .directiveIsRepeatable(true) - .schemaDescription(true) - .inputValueDeprecation(true) - .typeRefFragmentDepth(7) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(true) + .specifiedByUrl(true) + .directiveIsRepeatable(true) + .schemaDescription(true) + .inputValueDeprecation(true) + .typeRefFragmentDepth(7) ) - ) + ) + then: + parseExecutionResult(allTrueExecutionResult).every() + allTrueExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + } + + def "issue 3285 - deprecated defaultValue on programmatic args prints AST literal as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = newSchema().query(queryObjType).build() + def graphQL = newGraphQL(schema).build() + + + when: + def executionResult = graphQL.execute(IntrospectionQuery.INTROSPECTION_QUERY) + then: + executionResult.errors.isEmpty() + + def types = executionResult.data['__schema']['types'] as List + def queryType = types.find { it['name'] == 'Query' } + def fField = (queryType['fields'] as List)[0] + def arg = (fField['args'] as List)[0] + arg['name'] == "arg" + arg['defaultValue'] == "null" // printed AST + } + + + def "introspection for oneOf support"() { + def spec = ''' + + type Query { + oneOfNamedField(arg : OneOfInputType) : Enum + namedField(arg : InputType) : Enum + } + enum Enum { + RED + BLUE + } + input InputType { + inputField : String + } + input OneOfInputType @oneOf { + inputFieldA : String + inputFieldB : String + } + ''' + + when: + def graphQL = TestUtil.graphQL(spec).build() + def executionResult = graphQL.execute(IntrospectionQuery.INTROSPECTION_QUERY) + then: - parseExecutionResult(allTrueExecutionResult).every() - allTrueExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + executionResult.errors.isEmpty() + + def types = executionResult.data['__schema']['types'] as List + + def inputType = types.find { it['name'] == 'InputType' } + inputType["isOneOf"] == false + + def oneOfInputType = types.find { it['name'] == 'OneOfInputType' } + oneOfInputType["isOneOf"] == true + + def queryType = types.find { it['name'] == 'Query' } + queryType["isOneOf"] == null } + } diff --git a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy index 002fa0b408..262770eb31 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy @@ -89,7 +89,10 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def schemaType = er.data["__schema"] - schemaType["directives"] == [[name: "include"], [name: "skip"], [name: "example"], [name: "secret"], [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"]] + schemaType["directives"] == [ + [name: "include"], [name: "skip"], [name: "example"], [name: "secret"], + [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] + ] schemaType["appliedDirectives"] == [[name: "example", args: [[name: "argName", value: '"onSchema"']]]] @@ -170,7 +173,7 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def definedDirectives = er.data["__schema"]["directives"] // secret is filter out - definedDirectives == [[name: "include"], [name: "skip"], [name: "example"], [name: "deprecated"], [name: "specifiedBy"]] + definedDirectives == [[name: "include"], [name: "skip"], [name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"]] } def "can set prefixes onto the Applied types"() { diff --git a/src/test/groovy/graphql/language/SourceLocationTest.groovy b/src/test/groovy/graphql/language/SourceLocationTest.groovy new file mode 100644 index 0000000000..f5e71e6f72 --- /dev/null +++ b/src/test/groovy/graphql/language/SourceLocationTest.groovy @@ -0,0 +1,71 @@ +package graphql.language + +import graphql.parser.MultiSourceReader +import graphql.schema.idl.RuntimeWiring +import graphql.schema.idl.SchemaGenerator +import graphql.schema.idl.SchemaParser +import spock.lang.Specification + +import static graphql.schema.FieldCoordinates.coordinates + +class SourceLocationTest extends Specification { + + def "can get source location"() { + def sdl = """ + type Query { + a : A! + } + + type A { + b : B + } + + type B { + c : String + } + """ + + def sourceReader = MultiSourceReader.newMultiSourceReader().string(sdl, "sourceName").build() + + def definitionRegistry = new SchemaParser().parse(sourceReader) + when: + def schema = new SchemaGenerator().makeExecutableSchema(definitionRegistry, RuntimeWiring.MOCKED_WIRING) + def schemaElement = schema.getType("Query") + def location = SourceLocation.getLocation(schemaElement) + + then: + location.sourceName == "sourceName" + location.line == 2 + location.column == 13 + + when: + schemaElement = schema.getFieldDefinition(coordinates("Query", "a")) + location = SourceLocation.getLocation(schemaElement) + + then: + location.sourceName == "sourceName" + location.line == 3 + location.column == 17 + + when: + schemaElement = schema.getFieldDefinition(coordinates("Query", "a")).getType() + // unwrapped + location = SourceLocation.getLocation(schemaElement) + + then: + location.sourceName == "sourceName" + location.line == 6 + location.column == 13 + + when: + schemaElement = schema.getType("A") + location = SourceLocation.getLocation(schemaElement) + + then: + location.sourceName == "sourceName" + location.line == 6 + location.column == 13 + + + } +} diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy index e5e6724abd..89ea656b81 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy @@ -19,13 +19,15 @@ import graphql.util.TraverserContext import graphql.util.TraverserVisitorStub import spock.lang.Specification +import java.util.stream.Collectors +import java.util.stream.IntStream + import static graphql.TestUtil.schema import static graphql.language.AstPrinter.printAst import static graphql.parser.Parser.parseValue import static graphql.schema.FieldCoordinates.coordinates class ExecutableNormalizedOperationFactoryTest extends Specification { - def "test"() { String schema = """ type Query{ @@ -2694,5 +2696,183 @@ fragment personName on Person { ] } + def "query cannot exceed max depth"() { + String schema = """ + type Query { + animal: Animal + } + interface Animal { + name: String + friends: [Animal] + } + type Bird implements Animal { + name: String + friends: [Animal] + } + type Cat implements Animal { + name: String + friends: [Animal] + breed: String + } + type Dog implements Animal { + name: String + breed: String + friends: [Animal] + } + """ + GraphQLSchema graphQLSchema = TestUtil.schema(schema) + + // We generate two less fields than the given depth + // One is due to the top level field + // One is due to the leaf selection + def animalSubselection = IntStream.rangeClosed(1, queryDepth - 2) + .mapToObj { + "" + } + .reduce("CHILD") { acc, value -> + acc.replace("CHILD", "friends { CHILD }") + } + .replace("CHILD", "name") + + // Note: there is a total of 51 fields here + String query = """ + { + animal { + $animalSubselection + } + } + """ + + def limit = 50 + + assertValidQuery(graphQLSchema, query) + + Document document = TestUtil.parseQuery(query) + + when: + Exception exception + try { + ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables( + graphQLSchema, + document, + null, + RawVariables.emptyVariables(), + ExecutableNormalizedOperationFactory.Options.defaultOptions().maxChildrenDepth(limit)) + } catch (Exception e) { + exception = e + } + + then: + if (queryDepth > limit) { + assert exception != null + assert exception.message.contains("depth exceeded") + assert exception.message.contains("> 50") + } else { + assert exception == null + } + + where: + _ | queryDepth + _ | 49 + _ | 50 + _ | 51 + } + + def "big query is fine as long as depth is under limit"() { + String schema = """ + type Query { + animal: Animal + } + interface Animal { + name: String + friends: [Friend] + } + union Pet = Dog | Cat + type Friend { + name: String + isBirdOwner: Boolean + isCatOwner: Boolean + pets: [Pet] + } + type Bird implements Animal { + name: String + friends: [Friend] + } + type Cat implements Animal { + name: String + friends: [Friend] + breed: String + } + type Dog implements Animal { + name: String + breed: String + friends: [Friend] + } + """ + + def garbageFields = IntStream.range(0, 1000) + .mapToObj { + """test_$it: friends { name }""" + } + .collect(Collectors.joining("\n")) + + GraphQLSchema graphQLSchema = TestUtil.schema(schema) + + String query = """ + { + animal { + name + otherName: name + ... on Animal { + name + } + ... on Cat { + name + friends { + ... on Friend { + isCatOwner + pets { + ... on Dog { + name + } + } + } + } + } + ... on Bird { + friends { + isBirdOwner + } + friends { + name + pets { + ... on Cat { + breed + } + } + } + } + ... on Dog { + name + } + $garbageFields + } + } + """ + + assertValidQuery(graphQLSchema, query) + + Document document = TestUtil.parseQuery(query) + + when: + def result = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables( + graphQLSchema, + document, + null, + RawVariables.emptyVariables(), + ExecutableNormalizedOperationFactory.Options.defaultOptions().maxChildrenDepth(5)) + then: + noExceptionThrown() + } } diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index 726a98eb63..dd074ce7a8 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -4,10 +4,13 @@ import graphql.GraphQL import graphql.TestUtil import graphql.execution.RawVariables import graphql.language.AstPrinter +import graphql.language.Field +import graphql.language.OperationDefinition import graphql.language.AstSorter import graphql.language.Document import graphql.language.IntValue import graphql.language.StringValue +import graphql.parser.Parser import graphql.schema.GraphQLSchema import graphql.schema.idl.RuntimeWiring import graphql.schema.idl.TestLiveMockedWiringFactory @@ -1238,6 +1241,62 @@ class ExecutableNormalizedOperationToAstCompilerTest extends Specification { ''' } + + + def "test query directive"() { + def sdl = ''' + type Query { + foo1(arg: I): String + + } + type Subscription { + foo1(arg: I): DevOps + + } + input I { + arg1: String + } + + type DevOps{ + name: String + } + + directive @optIn(to : [String!]!) repeatable on FIELD + ''' + def query = '''subscription { + foo1 (arg: { + arg1: "Subscription" + }) @optIn(to: "foo") { + name @optIn(to: "devOps") + } + + + } + ''' + GraphQLSchema schema = mkSchema(sdl) + Document document = new Parser().parse(query) + ExecutableNormalizedOperation eno = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(schema,document, null,RawVariables.emptyVariables()) + + + when: + def result = compileToDocument(schema, SUBSCRIPTION, null, eno.topLevelFields, eno.normalizedFieldToQueryDirectives, noVariables) + OperationDefinition operationDefinition = result.document.getDefinitionsOfType(OperationDefinition.class)[0] + def fooField = (Field)operationDefinition.selectionSet.children[0] + def nameField = (Field)fooField.selectionSet.children[0] + def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + + then: + + fooField.directives.size() == 1 + nameField.directives.size() == 1 + documentPrinted == '''subscription { + foo1(arg: {arg1 : "Subscription"}) @optIn(to: "foo") { + name @optIn(to: "devOps") + } +} +''' + } + def "test redundant inline fragments specified in original query"() { def sdl = ''' type Query { diff --git a/src/test/groovy/graphql/schema/GraphQLInputObjectTypeTest.groovy b/src/test/groovy/graphql/schema/GraphQLInputObjectTypeTest.groovy index 81a7d19c20..09ed18b731 100644 --- a/src/test/groovy/graphql/schema/GraphQLInputObjectTypeTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLInputObjectTypeTest.groovy @@ -1,7 +1,11 @@ package graphql.schema +import graphql.Directives +import graphql.ExecutionInput +import graphql.GraphQL import graphql.GraphQLContext import graphql.StarWarsSchema +import graphql.TestUtil import graphql.language.ObjectValue import graphql.validation.ValidationUtil import spock.lang.Specification @@ -79,4 +83,101 @@ class GraphQLInputObjectTypeTest extends Specification { expect: validationUtil.isValidLiteralValue(objectValue.build(), inputObjectType, schema, graphQLContext, Locale.ENGLISH) } + + def "can detect one of support"() { + when: + def inputObjectType = newInputObject().name("TestInputObjectType") + .field(newInputObjectField().name("NAME").type(GraphQLInt)) + .build() + then: + !inputObjectType.isOneOf() + + when: + inputObjectType = newInputObject().name("TestInputObjectType") + .field(newInputObjectField().name("NAME").type(GraphQLInt)) + .withDirective(Directives.OneOfDirective) + .build() + then: + inputObjectType.isOneOf() + + when: + inputObjectType = newInputObject().name("TestInputObjectType") + .field(newInputObjectField().name("NAME").type(GraphQLInt)) + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .build() + then: + inputObjectType.isOneOf() + } + + def "e2e test of oneOf support"() { + def sdl = ''' + type Query { + f(arg : OneOf) : [KV] + } + + type KV { + key : String + value : String + } + + input OneOf @oneOf { + a : String + b : Int + } + ''' + + DataFetcher df = { DataFetchingEnvironment env -> + Map arg = env.getArgument("arg") + + def l = [] + for (Map.Entry entry : arg.entrySet()) { + l.add(["key": entry.getKey(), "value": String.valueOf(entry.getValue())]) + } + return l + } + + def graphQLSchema = TestUtil.schema(sdl, [Query: [f: df]]) + def graphQL = GraphQL.newGraphQL(graphQLSchema).build() + + when: + def er = graphQL.execute('query q { f( arg : {a : "abc"}) { key value }}') + def l = (er.data["f"] as List) + then: + er.errors.isEmpty() + l.size() == 1 + l[0]["key"] == "a" + l[0]["value"] == "abc" + + when: + er = graphQL.execute('query q { f( arg : {b : 123}) { key value }}') + l = (er.data["f"] as List) + + then: + er.errors.isEmpty() + l.size() == 1 + l[0]["key"] == "b" + l[0]["value"] == "123" + + when: + er = graphQL.execute('query q { f( arg : {a : "abc", b : 123}) { key value }}') + then: + !er.errors.isEmpty() + er.errors[0].message == "Exception while fetching data (/f) : Exactly one key must be specified for OneOf type 'OneOf'." + + when: + def ei = ExecutionInput.newExecutionInput('query q($var : OneOf) { f( arg : $var) { key value }}').variables([var: [a: "abc", b: 123]]).build() + er = graphQL.execute(ei) + then: + !er.errors.isEmpty() + er.errors[0].message == "Exception while fetching data (/f) : Exactly one key must be specified for OneOf type 'OneOf'." + + when: + ei = ExecutionInput.newExecutionInput('query q($var : OneOf) { f( arg : $var) { key value }}').variables([var: [a: null]]).build() + er = graphQL.execute(ei) + then: + !er.errors.isEmpty() + er.errors[0].message == "Exception while fetching data (/f) : OneOf type field 'OneOf.a' must be non-null." + + // lots more covered in unit tests + } } diff --git a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy index 3ddf718f71..de2a51bf04 100644 --- a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy @@ -136,22 +136,22 @@ class GraphQLSchemaTest extends Specification { when: "no additional directives have been specified" def schema = schemaBuilder.build() then: - schema.directives.size() == 4 + schema.directives.size() == 5 when: "clear directives is called" schema = schemaBuilder.clearDirectives().build() then: - schema.directives.size() == 2 // @deprecated and @specifiedBy is ALWAYS added if missing + schema.directives.size() == 3 // @deprecated and @specifiedBy and @oneOf is ALWAYS added if missing when: "clear directives is called with more directives" schema = schemaBuilder.clearDirectives().additionalDirective(Directives.SkipDirective).build() then: - schema.directives.size() == 3 + schema.directives.size() == 4 when: "the schema is transformed, things are copied" schema = schema.transform({ builder -> builder.additionalDirective(Directives.IncludeDirective) }) then: - schema.directives.size() == 4 + schema.directives.size() == 5 } def "clear additional types works as expected"() { diff --git a/src/test/groovy/graphql/schema/SchemaPrinterComparatorsTest.groovy b/src/test/groovy/graphql/schema/SchemaPrinterComparatorsTest.groovy index f2a12e3b7e..35ef8ab026 100644 --- a/src/test/groovy/graphql/schema/SchemaPrinterComparatorsTest.groovy +++ b/src/test/groovy/graphql/schema/SchemaPrinterComparatorsTest.groovy @@ -180,7 +180,7 @@ scalar TestScalar @a(a : 0, bb : 0) @bb(a : 0, bb : 0) when: def options = defaultOptions() - def result = new SchemaPrinter(options).directivesString(null, false, directives) + def result = new SchemaPrinter(options).directivesString(null, directives) then: result == ''' @a(a : 0, bb : 0) @bb(a : 0, bb : 0)''' diff --git a/src/test/groovy/graphql/schema/diff/SchemaDiffTest.groovy b/src/test/groovy/graphql/schema/diff/SchemaDiffTest.groovy index b945abc143..12dc7cf87f 100644 --- a/src/test/groovy/graphql/schema/diff/SchemaDiffTest.groovy +++ b/src/test/groovy/graphql/schema/diff/SchemaDiffTest.groovy @@ -1,5 +1,6 @@ package graphql.schema.diff +import graphql.AssertException import graphql.TestUtil import graphql.language.Argument import graphql.language.Directive @@ -15,6 +16,7 @@ import graphql.language.TypeName import graphql.schema.Coercing import graphql.schema.DataFetcher import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLSchema import graphql.schema.PropertyDataFetcher import graphql.schema.TypeResolver import graphql.schema.diff.reporting.CapturingReporter @@ -30,8 +32,10 @@ import spock.lang.Specification import java.util.stream.Collectors class SchemaDiffTest extends Specification { - private CapturingReporter reporter - private ChainedReporter chainedReporter + private CapturingReporter introspectionReporter + private CapturingReporter sdlReporter + private ChainedReporter introspectionChainedReporter + private ChainedReporter sdlChainedReporter private static final TypeResolver NULL_TYPE_RESOLVER = { env -> null } @@ -92,18 +96,55 @@ class SchemaDiffTest extends Specification { } void setup() { - reporter = new CapturingReporter() - chainedReporter = new ChainedReporter(reporter, new PrintStreamReporter()) + introspectionReporter = new CapturingReporter() + sdlReporter = new CapturingReporter() + introspectionChainedReporter = new ChainedReporter(introspectionReporter, new PrintStreamReporter()) + sdlChainedReporter = new ChainedReporter(sdlReporter, new PrintStreamReporter()) } - DiffSet diffSet(String newFile) { + void compareDiff(String newFile) { + SchemaDiffSet introspectionSchemaDiffSet = introspectionSchemaDiffSet(newFile) + SchemaDiffSet sdlSchemaDiffSet = sdlSchemaDiffSet(newFile) + + def diff = new SchemaDiff() + diff.diffSchema(introspectionSchemaDiffSet, introspectionChainedReporter) + diff.diffSchema(sdlSchemaDiffSet, sdlChainedReporter) + } + + void compareDiff(GraphQLSchema oldSchema, GraphQLSchema newSchema) { + SchemaDiffSet introspectionSchemaDiffSet = SchemaDiffSet.diffSetFromIntrospection(oldSchema, newSchema) + SchemaDiffSet sdlSchemaDiffSet = SchemaDiffSet.diffSetFromSdl(oldSchema, newSchema) + + def diff = new SchemaDiff() + diff.diffSchema(introspectionSchemaDiffSet, introspectionChainedReporter) + diff.diffSchema(sdlSchemaDiffSet, sdlChainedReporter) + } + + void validateReportersAreEqual() { + introspectionReporter.events == sdlReporter.events + introspectionReporter.infos == sdlReporter.infos + introspectionReporter.dangers == sdlReporter.dangers + introspectionReporter.breakages == sdlReporter.breakages + introspectionReporter.breakageCount == sdlReporter.breakageCount + introspectionReporter.infoCount == sdlReporter.infoCount + introspectionReporter.dangerCount == sdlReporter.dangerCount + } + + SchemaDiffSet introspectionSchemaDiffSet(String newFile) { def schemaOld = TestUtil.schemaFile("diff/" + "schema_ABaseLine.graphqls", wireWithNoFetching()) def schemaNew = TestUtil.schemaFile("diff/" + newFile, wireWithNoFetching()) - def diffSet = DiffSet.diffSet(schemaOld, schemaNew) + def diffSet = SchemaDiffSet.diffSetFromIntrospection(schemaOld, schemaNew) diffSet } + SchemaDiffSet sdlSchemaDiffSet(String newFile) { + def schemaOld = TestUtil.schemaFile("diff/" + "schema_ABaseLine.graphqls", wireWithNoFetching()) + def schemaNew = TestUtil.schemaFile("diff/" + newFile, wireWithNoFetching()) + + def diffSet = SchemaDiffSet.diffSetFromSdl(schemaOld, schemaNew) + diffSet + } def "change_in_null_ness_input_or_arg"() { @@ -194,7 +235,7 @@ class SchemaDiffTest extends Specification { def "directives_controlled_via_options"() { given: - DiffCtx ctx = new DiffCtx(reporter, null, null) + DiffCtx ctx = new DiffCtx(introspectionReporter, null, null) TypeDefinition left = new ObjectTypeDefinition("fooType") @@ -203,11 +244,11 @@ class SchemaDiffTest extends Specification { def diff = new SchemaDiff() diff.checkDirectives(ctx, left, twoDirectives, oneDirective) - def notChecked = lastBreakage(reporter) + def notChecked = lastBreakage(introspectionReporter) diff = new SchemaDiff(SchemaDiff.Options.defaultOptions().enforceDirectives()) diff.checkDirectives(ctx, left, twoDirectives, oneDirective) - def missingDirective = lastBreakage(reporter) + def missingDirective = lastBreakage(introspectionReporter) expect: notChecked == null @@ -218,7 +259,7 @@ class SchemaDiffTest extends Specification { def "directives enforced to be the same"() { given: - DiffCtx ctx = new DiffCtx(reporter, null, null) + DiffCtx ctx = new DiffCtx(introspectionReporter, null, null) TypeDefinition left = new ObjectTypeDefinition("fooType") @@ -229,7 +270,7 @@ class SchemaDiffTest extends Specification { def diff = new SchemaDiff(SchemaDiff.Options.defaultOptions().enforceDirectives()) diff.checkDirectives(ctx, left, twoDirectives, oneDirective) - def missingDirective = lastBreakage(reporter) + def missingDirective = lastBreakage(introspectionReporter) def oldDirective = new Directive("foo", [ new Argument("arg1", new StringValue("p1")), @@ -242,7 +283,7 @@ class SchemaDiffTest extends Specification { ]) diff.checkDirectives(ctx, left, [oldDirective], [newDirective]) - def missingArgs = lastBreakage(reporter) + def missingArgs = lastBreakage(introspectionReporter) def newDirectiveDiffDefaultType = new Directive("foo", [ @@ -251,35 +292,31 @@ class SchemaDiffTest extends Specification { ]) diff.checkDirectives(ctx, left, [oldDirective], [newDirectiveDiffDefaultType]) - def changedType = lastBreakage(reporter) + def changedType = lastBreakage(introspectionReporter) expect: missingDirective.category == DiffCategory.MISSING missingArgs.category == DiffCategory.MISSING changedType.category == DiffCategory.INVALID - reporter.getBreakageCount() == 3 + introspectionReporter.getBreakageCount() == 3 } def "same schema diff"() { - DiffSet diffSet = diffSet("schema_ABaseLine.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_ABaseLine.graphqls") expect: - reporter.breakageCount == 0 + validateReportersAreEqual() + introspectionReporter.breakageCount == 0 } def "additional field"() { - DiffSet diffSet = diffSet("schema_with_additional_field.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_with_additional_field.graphqls") expect: - reporter.breakageCount == 0 + validateReportersAreEqual() + introspectionReporter.breakageCount == 0 - List newFieldEvents = reporter.infos.stream() + List newFieldEvents = introspectionReporter.infos.stream() .filter { de -> de.typeName == "Ainur" && de.fieldName == "surname" } .collect(Collectors.toList()) @@ -291,286 +328,257 @@ class SchemaDiffTest extends Specification { } def "missing fields on interface"() { - DiffSet diffSet = diffSet("schema_interface_fields_missing.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_interface_fields_missing.graphqls") expect: - reporter.breakageCount == 8 // 2 fields removed from interface, affecting 3 types - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Interface + validateReportersAreEqual() + introspectionReporter.breakageCount == 8 // 2 fields removed from interface, affecting 3 types + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Interface - reporter.breakages[1].category == DiffCategory.MISSING - reporter.breakages[1].typeKind == TypeKind.Interface + introspectionReporter.breakages[1].category == DiffCategory.MISSING + introspectionReporter.breakages[1].typeKind == TypeKind.Interface } def "missing members on union"() { - DiffSet diffSet = diffSet("schema_missing_union_members.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_union_members.graphqls") expect: - reporter.breakageCount == 1 // 1 member removed - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Union - + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 // 1 member removed + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Union } def "missing fields on object"() { - DiffSet diffSet = diffSet("schema_missing_object_fields.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_object_fields.graphqls") expect: - reporter.breakageCount == 2 // 2 fields removed - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Object - reporter.breakages[0].fieldName == 'colour' + validateReportersAreEqual() + introspectionReporter.breakageCount == 2 // 2 fields removed + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Object + introspectionReporter.breakages[0].fieldName == 'colour' - reporter.breakages[1].category == DiffCategory.MISSING - reporter.breakages[1].typeKind == TypeKind.Object - reporter.breakages[1].fieldName == 'temperament' + introspectionReporter.breakages[1].category == DiffCategory.MISSING + introspectionReporter.breakages[1].typeKind == TypeKind.Object + introspectionReporter.breakages[1].fieldName == 'temperament' } def "missing operation"() { - DiffSet diffSet = diffSet("schema_missing_operation.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_operation.graphqls") expect: - reporter.breakageCount == 1 - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Operation - reporter.breakages[0].typeName == 'Mutation' - + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Operation + introspectionReporter.breakages[0].typeName == 'Mutation' } def "missing input object fields"() { - DiffSet diffSet = diffSet("schema_missing_input_object_fields.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_input_object_fields.graphqls") expect: - reporter.breakageCount == 1 - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.InputObject - reporter.breakages[0].fieldName == 'queryTarget' + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.InputObject + introspectionReporter.breakages[0].fieldName == 'queryTarget' } def "changed nested input object field types"() { - DiffSet diffSet = diffSet("schema_changed_nested_input_object_fields.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_changed_nested_input_object_fields.graphqls") expect: - reporter.breakageCount == 1 - reporter.breakages[0].category == DiffCategory.INVALID - reporter.breakages[0].typeName == 'NestedInput' - reporter.breakages[0].typeKind == TypeKind.InputObject - reporter.breakages[0].fieldName == 'nestedInput' - + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages[0].category == DiffCategory.INVALID + introspectionReporter.breakages[0].typeName == 'NestedInput' + introspectionReporter.breakages[0].typeKind == TypeKind.InputObject + introspectionReporter.breakages[0].fieldName == 'nestedInput' } def "changed input object field types"() { - DiffSet diffSet = diffSet("schema_changed_input_object_fields.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_changed_input_object_fields.graphqls") expect: - reporter.breakageCount == 4 - reporter.breakages[0].category == DiffCategory.STRICTER - reporter.breakages[0].typeName == 'Query' - reporter.breakages[0].typeKind == TypeKind.Object - reporter.breakages[0].fieldName == 'being' + validateReportersAreEqual() + introspectionReporter.breakageCount == 4 + introspectionReporter.breakages[0].category == DiffCategory.STRICTER + introspectionReporter.breakages[0].typeName == 'Query' + introspectionReporter.breakages[0].typeKind == TypeKind.Object + introspectionReporter.breakages[0].fieldName == 'being' - reporter.breakages[1].category == DiffCategory.STRICTER - reporter.breakages[1].typeName == 'Questor' - reporter.breakages[1].typeKind == TypeKind.InputObject - reporter.breakages[1].fieldName == 'nestedInput' + introspectionReporter.breakages[1].category == DiffCategory.STRICTER + introspectionReporter.breakages[1].typeName == 'Questor' + introspectionReporter.breakages[1].typeKind == TypeKind.InputObject + introspectionReporter.breakages[1].fieldName == 'nestedInput' - reporter.breakages[2].category == DiffCategory.INVALID - reporter.breakages[2].typeName == 'Questor' - reporter.breakages[2].typeKind == TypeKind.InputObject - reporter.breakages[2].fieldName == 'queryTarget' + introspectionReporter.breakages[2].category == DiffCategory.INVALID + introspectionReporter.breakages[2].typeName == 'Questor' + introspectionReporter.breakages[2].typeKind == TypeKind.InputObject + introspectionReporter.breakages[2].fieldName == 'queryTarget' - reporter.breakages[3].category == DiffCategory.STRICTER - reporter.breakages[3].typeName == 'Questor' - reporter.breakages[3].typeKind == TypeKind.InputObject - reporter.breakages[3].fieldName == 'newMandatoryField' + introspectionReporter.breakages[3].category == DiffCategory.STRICTER + introspectionReporter.breakages[3].typeName == 'Questor' + introspectionReporter.breakages[3].typeKind == TypeKind.InputObject + introspectionReporter.breakages[3].fieldName == 'newMandatoryField' } def "changed type kind"() { - DiffSet diffSet = diffSet("schema_changed_type_kind.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_changed_type_kind.graphqls") expect: - reporter.breakageCount == 1 - reporter.breakages[0].category == DiffCategory.INVALID - reporter.breakages[0].typeName == 'Character' - reporter.breakages[0].typeKind == TypeKind.Union - + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages[0].category == DiffCategory.INVALID + introspectionReporter.breakages[0].typeName == 'Character' + introspectionReporter.breakages[0].typeKind == TypeKind.Union } def "missing object field args"() { - DiffSet diffSet = diffSet("schema_missing_field_arguments.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_field_arguments.graphqls") expect: - reporter.breakageCount == 2 + validateReportersAreEqual() + introspectionReporter.breakageCount == 2 - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Object - reporter.breakages[0].typeName == "Mutation" - reporter.breakages[0].fieldName == 'being' - reporter.breakages[0].components.contains("questor") + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Object + introspectionReporter.breakages[0].typeName == "Mutation" + introspectionReporter.breakages[0].fieldName == 'being' + introspectionReporter.breakages[0].components.contains("questor") - reporter.breakages[1].category == DiffCategory.MISSING - reporter.breakages[1].typeKind == TypeKind.Object - reporter.breakages[0].typeName == "Mutation" - reporter.breakages[1].fieldName == 'sword' + introspectionReporter.breakages[1].category == DiffCategory.MISSING + introspectionReporter.breakages[1].typeKind == TypeKind.Object + introspectionReporter.breakages[0].typeName == "Mutation" + introspectionReporter.breakages[1].fieldName == 'sword' } def "missing enum value"() { - DiffSet diffSet = diffSet("schema_missing_enum_value.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_missing_enum_value.graphqls") expect: - reporter.breakageCount == 1 - reporter.breakages[0].category == DiffCategory.MISSING - reporter.breakages[0].typeKind == TypeKind.Enum - reporter.breakages[0].typeName == 'Temperament' - reporter.breakages[0].components.contains("Duplicitous") + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages[0].category == DiffCategory.MISSING + introspectionReporter.breakages[0].typeKind == TypeKind.Enum + introspectionReporter.breakages[0].typeName == 'Temperament' + introspectionReporter.breakages[0].components.contains("Duplicitous") } def "changed object field args"() { - DiffSet diffSet = diffSet("schema_changed_field_arguments.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_changed_field_arguments.graphqls") expect: - reporter.breakageCount == 2 - reporter.breakages[0].category == DiffCategory.INVALID - reporter.breakages[0].typeKind == TypeKind.Object - reporter.breakages[0].fieldName == 'sword' + validateReportersAreEqual() + introspectionReporter.breakageCount == 2 + introspectionReporter.breakages[0].category == DiffCategory.INVALID + introspectionReporter.breakages[0].typeKind == TypeKind.Object + introspectionReporter.breakages[0].fieldName == 'sword' - reporter.breakages[1].category == DiffCategory.INVALID - reporter.breakages[1].typeKind == TypeKind.Object - reporter.breakages[1].fieldName == 'sword' + introspectionReporter.breakages[1].category == DiffCategory.INVALID + introspectionReporter.breakages[1].typeKind == TypeKind.Object + introspectionReporter.breakages[1].fieldName == 'sword' } def "changed type on object"() { - DiffSet diffSet = diffSet("schema_changed_object_fields.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_changed_object_fields.graphqls") expect: - reporter.breakageCount == 3 - reporter.breakages[0].category == DiffCategory.STRICTER - reporter.breakages[0].typeName == 'Istari' - reporter.breakages[0].typeKind == TypeKind.Object - reporter.breakages[0].fieldName == 'temperament' + validateReportersAreEqual() + introspectionReporter.breakageCount == 3 + introspectionReporter.breakages[0].category == DiffCategory.STRICTER + introspectionReporter.breakages[0].typeName == 'Istari' + introspectionReporter.breakages[0].typeKind == TypeKind.Object + introspectionReporter.breakages[0].fieldName == 'temperament' - reporter.breakages[1].category == DiffCategory.INVALID - reporter.breakages[1].typeName == 'Query' - reporter.breakages[1].typeKind == TypeKind.Object - reporter.breakages[1].fieldName == 'beings' + introspectionReporter.breakages[1].category == DiffCategory.INVALID + introspectionReporter.breakages[1].typeName == 'Query' + introspectionReporter.breakages[1].typeKind == TypeKind.Object + introspectionReporter.breakages[1].fieldName == 'beings' - reporter.breakages[2].category == DiffCategory.INVALID - reporter.breakages[2].typeName == 'Query' - reporter.breakages[2].typeKind == TypeKind.Object - reporter.breakages[2].fieldName == 'customScalar' + introspectionReporter.breakages[2].category == DiffCategory.INVALID + introspectionReporter.breakages[2].typeName == 'Query' + introspectionReporter.breakages[2].typeKind == TypeKind.Object + introspectionReporter.breakages[2].fieldName == 'customScalar' } def "dangerous changes"() { - DiffSet diffSet = diffSet("schema_dangerous_changes.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_dangerous_changes.graphqls") expect: - reporter.breakageCount == 0 - reporter.dangerCount == 3 - - reporter.dangers[0].category == DiffCategory.ADDITION - reporter.dangers[0].typeName == "Temperament" - reporter.dangers[0].typeKind == TypeKind.Enum - reporter.dangers[0].components.contains("Nonplussed") - - reporter.dangers[1].category == DiffCategory.ADDITION - reporter.dangers[1].typeName == "Character" - reporter.dangers[1].typeKind == TypeKind.Union - reporter.dangers[1].components.contains("BenignFigure") + validateReportersAreEqual() + introspectionReporter.breakageCount == 0 + introspectionReporter.dangerCount == 3 - reporter.dangers[2].category == DiffCategory.DIFFERENT - reporter.dangers[2].typeName == "Query" - reporter.dangers[2].typeKind == TypeKind.Object - reporter.dangers[2].fieldName == "being" - reporter.dangers[2].components.contains("type") + introspectionReporter.dangers[0].category == DiffCategory.ADDITION + introspectionReporter.dangers[0].typeName == "Temperament" + introspectionReporter.dangers[0].typeKind == TypeKind.Enum + introspectionReporter.dangers[0].components.contains("Nonplussed") + introspectionReporter.dangers[1].category == DiffCategory.ADDITION + introspectionReporter.dangers[1].typeName == "Character" + introspectionReporter.dangers[1].typeKind == TypeKind.Union + introspectionReporter.dangers[1].components.contains("BenignFigure") + introspectionReporter.dangers[2].category == DiffCategory.DIFFERENT + introspectionReporter.dangers[2].typeName == "Query" + introspectionReporter.dangers[2].typeKind == TypeKind.Object + introspectionReporter.dangers[2].fieldName == "being" + introspectionReporter.dangers[2].components.contains("type") } def "deprecated fields are unchanged"() { def schema = TestUtil.schemaFile("diff/" + "schema_deprecated_fields_new.graphqls", wireWithNoFetching()) - DiffSet diffSet = DiffSet.diffSet(schema, schema) + SchemaDiffSet introspectionSchemaDiffSet = SchemaDiffSet.diffSetFromIntrospection(schema, schema) + SchemaDiffSet sdlSchemaDiffSet = SchemaDiffSet.diffSetFromSdl(schema, schema) def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + diff.diffSchema(introspectionSchemaDiffSet, introspectionChainedReporter) + diff.diffSchema(sdlSchemaDiffSet, sdlChainedReporter) expect: - reporter.dangerCount == 0 - reporter.breakageCount == 0 + validateReportersAreEqual() + introspectionReporter.dangerCount == 0 + introspectionReporter.breakageCount == 0 } def "field was deprecated"() { - DiffSet diffSet = diffSet("schema_deprecated_fields_new.graphqls") - - def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + compareDiff("schema_deprecated_fields_new.graphqls") expect: - reporter.dangerCount == 14 - reporter.breakageCount == 0 - reporter.dangers.every { + validateReportersAreEqual() + introspectionReporter.dangerCount == 14 + introspectionReporter.breakageCount == 0 + introspectionReporter.dangers.every { it.getCategory() == DiffCategory.DEPRECATION_ADDED } - } def "deprecated field was removed"() { def schemaOld = TestUtil.schemaFile("diff/" + "schema_deprecated_fields_new.graphqls", wireWithNoFetching()) def schemaNew = TestUtil.schemaFile("diff/" + "schema_deprecated_fields_removed.graphqls", wireWithNoFetching()) - DiffSet diffSet = DiffSet.diffSet(schemaOld, schemaNew) + SchemaDiffSet introspectionSchemaDiffSet = SchemaDiffSet.diffSetFromIntrospection(schemaOld, schemaNew) + SchemaDiffSet sdlSchemaDiffSet = SchemaDiffSet.diffSetFromSdl(schemaOld, schemaNew) def diff = new SchemaDiff() - diff.diffSchema(diffSet, chainedReporter) + diff.diffSchema(introspectionSchemaDiffSet, introspectionChainedReporter) + diff.diffSchema(sdlSchemaDiffSet, sdlChainedReporter) expect: - reporter.dangerCount == 0 - reporter.breakageCount == 12 - reporter.breakages.every { + validateReportersAreEqual() + introspectionReporter.dangerCount == 0 + introspectionReporter.breakageCount == 12 + introspectionReporter.breakages.every { it.getCategory() == DiffCategory.DEPRECATION_REMOVED } } @@ -601,16 +609,15 @@ class SchemaDiffTest extends Specification { b: String } ''') - def reporter = new CapturingReporter() - DiffSet diffSet = DiffSet.diffSet(oldSchema, newSchema) - def diff = new SchemaDiff() + when: - diff.diffSchema(diffSet, reporter) + compareDiff(oldSchema, newSchema) then: - reporter.dangerCount == 0 - reporter.breakageCount == 1 - reporter.breakages.every { + validateReportersAreEqual() + introspectionReporter.dangerCount == 0 + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages.every { it.getCategory() == DiffCategory.MISSING } @@ -627,19 +634,16 @@ class SchemaDiffTest extends Specification { hello2: String } ''') - def reporter = new CapturingReporter() - DiffSet diffSet = DiffSet.diffSet(oldSchema, newSchema) - def diff = new SchemaDiff() when: - diff.diffSchema(diffSet, reporter) + compareDiff(oldSchema, newSchema) then: // the old hello field is missing - reporter.breakageCount == 1 - reporter.breakages.every { + validateReportersAreEqual() + introspectionReporter.breakageCount == 1 + introspectionReporter.breakages.every { it.getCategory() == DiffCategory.MISSING } - } def "interface renamed"() { def oldSchema = TestUtil.schema(''' @@ -666,15 +670,14 @@ class SchemaDiffTest extends Specification { hello: String } ''') - def reporter = new CapturingReporter() - DiffSet diffSet = DiffSet.diffSet(oldSchema, newSchema) - def diff = new SchemaDiff() when: - diff.diffSchema(diffSet, reporter) + + compareDiff(oldSchema, newSchema) then: // two breakages for World and Query not implementing Hello anymore - reporter.breakageCount == 2 + validateReportersAreEqual() + introspectionReporter.breakageCount == 2 } @@ -685,7 +688,7 @@ class SchemaDiffTest extends Specification { when: def capturingReporter = new CapturingReporter() def schemaDiff = new SchemaDiff() - def breakingCount = schemaDiff.diffSchema(DiffSet.diffSet(schema1, schema1), capturingReporter) + def breakingCount = schemaDiff.diffSchema(SchemaDiffSet.diffSetFromIntrospection(schema1, schema1), capturingReporter) then: breakingCount == capturingReporter.getBreakageCount() breakingCount == 0 @@ -693,10 +696,76 @@ class SchemaDiffTest extends Specification { when: capturingReporter = new CapturingReporter() schemaDiff = new SchemaDiff() - breakingCount = schemaDiff.diffSchema(DiffSet.diffSet(schema1, schema2), capturingReporter) + breakingCount = schemaDiff.diffSchema(SchemaDiffSet.diffSetFromIntrospection(schema1, schema2), capturingReporter) then: breakingCount == capturingReporter.getBreakageCount() breakingCount == 1 } + + def "directives are removed should be breaking when enforceDirectives is enabled"() { + def oldSchema = TestUtil.schema(''' + directive @someDirective on FIELD_DEFINITION + + type test { + version: String! @someDirective + } + + type Query { + getTests: [test]! + } + ''') + def newSchema = TestUtil.schema(''' + type test { + version: String! + } + + type Query { + getTests: [test]! + } + ''') + def reporter = new CapturingReporter() + SchemaDiffSet diffSet = SchemaDiffSet.diffSetFromSdl(oldSchema, newSchema) + def diff = new SchemaDiff(SchemaDiff.Options.defaultOptions().enforceDirectives()) + when: + diff.diffSchema(diffSet, reporter) + + then: + reporter.dangerCount == 0 + reporter.breakageCount == 1 + reporter.breakages.every { + it.getCategory() == DiffCategory.MISSING + } + } + + def "When enforceDirectives is enabled, IntrospectionSchemaDiffSet should assert"() { + def oldSchema = TestUtil.schema(''' + directive @someDirective on FIELD_DEFINITION + + type test { + version: String! @someDirective + } + + type Query { + getTests: [test]! + } + ''') + def newSchema = TestUtil.schema(''' + type test { + version: String! + } + + type Query { + getTests: [test]! + } + ''') + def reporter = new CapturingReporter() + SchemaDiffSet diffSet = SchemaDiffSet.diffSetFromIntrospection(oldSchema, newSchema) + def diff = new SchemaDiff(SchemaDiff.Options.defaultOptions().enforceDirectives()) + when: + diff.diffSchema(diffSet, reporter) + + then: + thrown(AssertException) + } } diff --git a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy index 11f1dea63b..425252519f 100644 --- a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy +++ b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy @@ -34,14 +34,14 @@ class SchemaDiffingTest extends Specification { schemaGraph.getVerticesByType(SchemaGraph.INTERFACE).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.UNION).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.SCALAR).size() == 2 - schemaGraph.getVerticesByType(SchemaGraph.FIELD).size() == 39 + schemaGraph.getVerticesByType(SchemaGraph.FIELD).size() == 40 schemaGraph.getVerticesByType(SchemaGraph.ARGUMENT).size() == 9 schemaGraph.getVerticesByType(SchemaGraph.INPUT_FIELD).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.INPUT_OBJECT).size() == 0 - schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 4 + schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 5 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_ARGUMENT).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_DIRECTIVE).size() == 0 - schemaGraph.size() == 91 + schemaGraph.size() == 93 } diff --git a/src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy b/src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy index ebc9463eb5..2ab7bbd7b2 100644 --- a/src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy +++ b/src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy @@ -3044,6 +3044,111 @@ class EditOperationAnalyzerTest extends Specification { userModification.details.isEmpty() } + def "deleted field with fixed parent binding can map to isolated node"() { + given: + def oldSdl = ''' + type Query { + notifications: NotificationQuery + } + type NotificationQuery { + notificationFeed( + feedFilter: NotificationFeedFilter + first: Int = 25 + after: String + ): NotificationGroupedConnection! + unseenNotificationCount(workspaceId: String, product: String): Int! + } + input NotificationFeedFilter { + workspaceId: String + productFilter: String + groupId: String + } + type NotificationItem { + notificationId: ID! + workspaceId: String + } + type NotificationGroupedItem { + groupId: ID! + groupSize: Int! + headNotification: NotificationItem! + childItems(first: Int, after: String): [NotificationItem!] + } + type NotificationGroupedConnection { + nodes: [NotificationGroupedItem!]! + } + ''' + def newSdl = ''' + type Query { + notifications: NotificationQuery + } + type NotificationQuery { + notificationFeed( + filter: NotificationFilter + first: Int = 25 + after: String + ): NotificationFeedConnection! + notificationGroup( + groupId: String! + filter: NotificationFilter + first: Int = 25 + after: String + ): NotificationGroupConnection! + unseenNotificationCount(workspaceId: String, product: String): Int! + } + input NotificationFilter { + workspaceId: String + productFilter: String + } + type NotificationEntityModel{ + objectId: String! + containerId: String + workspaceId: String + cloudId: String + } + type NotificationItem { + notificationId: ID! + entityModel: NotificationEntityModel + workspaceId: String + } + type NotificationHeadItem { + groupId: ID! + groupSize: Int! + readStates: [String]! + additionalTypes: [String!]! + headNotification: NotificationItem! + endCursor: String + } + type NotificationFeedConnection { + nodes: [NotificationHeadItem!]! + } + type NotificationGroupConnection { + nodes: [NotificationItem!]! + } + ''' + + when: + def changes = calcDiff(oldSdl, newSdl) + + then: + changes.objectDifferences["NotificationGroupedItem"] === changes.objectDifferences["NotificationHeadItem"] + changes.objectDifferences["NotificationGroupedConnection"] === changes.objectDifferences["NotificationFeedConnection"] + changes.objectDifferences["NotificationGroupedItem"] instanceof ObjectModification + changes.objectDifferences["NotificationGroupedConnection"] instanceof ObjectModification + changes.objectDifferences["NotificationEntityModel"] instanceof ObjectAddition + changes.objectDifferences["NotificationGroupConnection"] instanceof ObjectAddition + changes.objectDifferences["NotificationItem"] instanceof ObjectModification + changes.objectDifferences["NotificationQuery"] instanceof ObjectModification + + changes.inputObjectDifferences["NotificationFeedFilter"] === changes.inputObjectDifferences["NotificationFilter"] + changes.inputObjectDifferences["NotificationFeedFilter"] instanceof InputObjectModification + + def notificationFeedFilterChange = changes.inputObjectDifferences["NotificationFeedFilter"] as InputObjectModification + notificationFeedFilterChange.details.size() == 1 + notificationFeedFilterChange.details[0] instanceof InputObjectFieldDeletion + def groupIdInputObjectFieldDeletion = notificationFeedFilterChange.details[0] as InputObjectFieldDeletion + groupIdInputObjectFieldDeletion.name == "groupId" + } + EditOperationAnalysisResult calcDiff( String oldSdl, String newSdl diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy index 0214a6a86f..824db92e15 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy @@ -58,6 +58,7 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "deprecated", "foo", "include", + "oneOf", "skip", "specifiedBy", ] @@ -107,6 +108,7 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "deprecated", "foo", "include", + "oneOf", "skip", "specifiedBy", ] diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy index a8e9ce3617..0d2be323d0 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy @@ -2052,11 +2052,13 @@ class SchemaGeneratorTest extends Specification { directives = schema.getDirectives() then: - directives.size() == 7 // built in ones : include / skip and deprecated + directives.size() == 8 // built in ones : include / skip and deprecated def directiveNames = directives.collect { it.name } directiveNames.contains("include") directiveNames.contains("skip") directiveNames.contains("deprecated") + directiveNames.contains("specifiedBy") + directiveNames.contains("oneOf") directiveNames.contains("sd1") directiveNames.contains("sd2") directiveNames.contains("sd3") @@ -2065,10 +2067,11 @@ class SchemaGeneratorTest extends Specification { directivesMap = schema.getDirectivesByName() then: - directivesMap.size() == 7 // built in ones + directivesMap.size() == 8 // built in ones directivesMap.containsKey("include") directivesMap.containsKey("skip") directivesMap.containsKey("deprecated") + directivesMap.containsKey("oneOf") directivesMap.containsKey("sd1") directivesMap.containsKey("sd2") directivesMap.containsKey("sd3") @@ -2536,4 +2539,26 @@ class SchemaGeneratorTest extends Specification { newSchema.getDirectives().findAll { it.name == "skip" }.size() == 1 newSchema.getDirectives().findAll { it.name == "include" }.size() == 1 } + + def "oneOf directive is available implicitly"() { + def sdl = ''' + type Query { + f(arg : OneOfInputType) : String + } + + input OneOfInputType @oneOf { + a : String + b : String + } + ''' + + when: + def schema = TestUtil.schema(sdl) + then: + schema.getDirectives().findAll { it.name == "oneOf" }.size() == 1 + + GraphQLInputObjectType inputObjectType = schema.getTypeAs("OneOfInputType") + inputObjectType.isOneOf() + inputObjectType.hasAppliedDirective("oneOf") + } } diff --git a/src/test/groovy/graphql/schema/idl/SchemaParserTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaParserTest.groovy index 24d9af718a..762cce719f 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaParserTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaParserTest.groovy @@ -480,4 +480,14 @@ type Virus { def printedSchema = new SchemaPrinter(options).print(graphQL.graphQLSchema) printedSchema == schema } + + def "testNumberFormatException"() { + when: + SchemaParser parser = new SchemaParser(); + parser.parse("{B(t:66E3333333320,t:#\n66666666660)},622»» »»»6666662}}6666660t:z6666") + + then: + thrown(SchemaProblem) + } + } diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index e946a06278..c0c83ddca8 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -3,13 +3,21 @@ package graphql.schema.idl import graphql.GraphQL import graphql.TestUtil import graphql.TypeResolutionEnvironment +import graphql.introspection.Introspection import graphql.introspection.IntrospectionQuery import graphql.introspection.IntrospectionResultToSchema +import graphql.language.Comment +import graphql.language.DirectiveDefinition +import graphql.language.EnumValueDefinition +import graphql.language.FieldDefinition import graphql.language.IntValue +import graphql.language.ScalarTypeDefinition +import graphql.language.SchemaDefinition import graphql.language.StringValue import graphql.schema.Coercing import graphql.schema.GraphQLAppliedDirective import graphql.schema.GraphQLCodeRegistry +import graphql.schema.GraphQLDirective import graphql.schema.GraphQLEnumType import graphql.schema.GraphQLEnumValueDefinition import graphql.schema.GraphQLFieldDefinition @@ -30,11 +38,19 @@ import spock.lang.Specification import java.util.function.Predicate import java.util.function.UnaryOperator +import java.util.stream.Collectors +import static graphql.Scalars.GraphQLID import static graphql.Scalars.GraphQLInt import static graphql.Scalars.GraphQLString import static graphql.TestUtil.mockScalar import static graphql.TestUtil.mockTypeRuntimeWiring +import static graphql.language.EnumTypeDefinition.newEnumTypeDefinition +import static graphql.language.InputObjectTypeDefinition.newInputObjectDefinition +import static graphql.language.InputValueDefinition.newInputValueDefinition +import static graphql.language.InterfaceTypeDefinition.newInterfaceTypeDefinition +import static graphql.language.ObjectTypeDefinition.newObjectTypeDefinition +import static graphql.language.UnionTypeDefinition.newUnionTypeDefinition import static graphql.schema.GraphQLArgument.newArgument import static graphql.schema.GraphQLEnumType.newEnum import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition @@ -44,6 +60,7 @@ import static graphql.schema.GraphQLList.list import static graphql.schema.GraphQLNonNull.nonNull import static graphql.schema.GraphQLObjectType.newObject import static graphql.schema.GraphQLScalarType.newScalar +import static graphql.schema.GraphQLTypeReference.typeRef import static graphql.schema.GraphQLUnionType.newUnionType import static graphql.schema.idl.RuntimeWiring.newRuntimeWiring import static graphql.schema.idl.SchemaPrinter.ExcludeGraphQLSpecifiedDirectivesPredicate @@ -980,6 +997,9 @@ directive @interfaceImplementingTypeDirective on OBJECT directive @interfaceTypeDirective on INTERFACE +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + directive @query1 repeatable on OBJECT directive @query2(arg1: String) on OBJECT @@ -1133,6 +1153,9 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." @@ -1230,6 +1253,9 @@ directive @include( directive @moreComplex(arg1: String = "default", arg2: Int) on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." @@ -1295,6 +1321,9 @@ directive @include( directive @moreComplex(arg1: String = "default", arg2: Int) on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." @@ -1427,6 +1456,9 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." @@ -1929,6 +1961,9 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to skip this field or fragment when the `if` argument is true." directive @skip( "Skipped when true." @@ -2135,6 +2170,9 @@ directive @skip( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2249,4 +2287,417 @@ type TestObjectB { } ''' } -} \ No newline at end of file + + final def SDL_WITH_COMMENTS = '''#schema comment 1 +# schema comment 2 with leading spaces +schema { + query: Query + mutation: Mutation +} + +"Marks the field, argument, input field or enum value as deprecated" +directive @deprecated( + "The reason for the deprecation" + reason: String = "No longer supported" + ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION + +" custom directive 'example' description 1" +# custom directive 'example' comment 1 +directive @example on ENUM_VALUE + +"Directs the executor to include this field or fragment only when the `if` argument is true" +directive @include( + "Included when true." + if: Boolean! + ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + +"Directs the executor to skip this field or fragment when the `if` argument is true." +directive @skip( + "Skipped when true." + if: Boolean! + ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +"Exposes a URL that specifies the behaviour of this scalar." +directive @specifiedBy( + "The URL that specifies the behaviour of this scalar." + url: String! + ) on SCALAR + +# interface Character comment 1 +# interface Character comment 2 +interface Character implements Node { + appearsIn: [Episode] + friends: [Character] + id: ID! + name: String +} + +interface Node { + id: ID! +} + +# union type Humanoid comment 1 +union Humanoid = Droid | Human + +type Droid implements Character & Node { + appearsIn: [Episode]! + friends: [Character] + id: ID! + madeOn: Planet + name: String! + primaryFunction: String +} + +type Human implements Character & Node { + appearsIn: [Episode]! + friends: [Character] + homePlanet: String + id: ID! + name: String! +} + +type Mutation { + shoot( + # arg 'id\' + id: String!, + # arg 'with\' + with: Gun + ): Query +} + +type Planet { + hitBy: Asteroid + name: String +} + +# type query comment 1 +# type query comment 2 +type Query { + # query field 'hero' comment + hero(episode: Episode): Character + # query field 'humanoid' comment + humanoid(id: ID!): Humanoid +} + +# enum Episode comment 1 +# enum Episode comment 2 +enum Episode { + # enum value EMPIRE comment 1 + EMPIRE + JEDI + NEWHOPE @example +} + +"desc" +# scalar Asteroid comment 1 +scalar Asteroid + +# input type Gun comment 1 +input Gun { + # gun 'caliber' input value comment + caliber: Int + # gun 'name' input value comment + name: String +} +''' + + static List makeComments(String... strings) { + return strings.stream() + .map(s -> new Comment(s, null)) + .collect(Collectors.toList()) + } + + def "prints with AST comments"() { + given: + def exampleDirective = GraphQLDirective.newDirective().name("example").validLocation(Introspection.DirectiveLocation.ENUM_VALUE) + .description(" custom directive 'example' description 1") + .definition(DirectiveDefinition.newDirectiveDefinition().comments(makeComments(" custom directive 'example' comment 1")).build()).build() + def asteroidType = newScalar().name("Asteroid").description("desc") + .definition(ScalarTypeDefinition.newScalarTypeDefinition().comments(makeComments(" scalar Asteroid comment 1")).build()) + .coercing(TestUtil.mockCoercing()) + .build() + def nodeType = newInterface().name("Node") + .field(newFieldDefinition().name("id").type(nonNull(GraphQLID)).build()) + .build() + def planetType = newObject().name("Planet") + .field(newFieldDefinition().name("hitBy").type(asteroidType).build()) + .field(newFieldDefinition().name("name").type(GraphQLString).build()) + .build() + def episodeType = newEnum().name("Episode") + .definition(newEnumTypeDefinition().comments( + makeComments(" enum Episode comment 1", " enum Episode comment 2")).build()) + .values(List.of( + GraphQLEnumValueDefinition.newEnumValueDefinition().name("EMPIRE") + .definition(EnumValueDefinition.newEnumValueDefinition().comments(makeComments(" enum value EMPIRE comment 1")).build()).build(), + GraphQLEnumValueDefinition.newEnumValueDefinition().name("JEDI").build(), + GraphQLEnumValueDefinition.newEnumValueDefinition().name("NEWHOPE").withDirective(exampleDirective).build())) + .build() + def characterType = newInterface().name("Character").withInterface(nodeType) + .definition(newInterfaceTypeDefinition().comments( + makeComments(" interface Character comment 1", " interface Character comment 2")).build()) + .field(newFieldDefinition().name("appearsIn").type(list(episodeType)).build()) + .field(newFieldDefinition().name("friends").type(list(typeRef("Character"))).build()) + .field(newFieldDefinition().name("id").type(nonNull(GraphQLID)).build()) + .field(newFieldDefinition().name("name").type(GraphQLString).build()) + .build() + def droidType = newObject().name("Droid").withInterfaces(characterType, nodeType) + .field(newFieldDefinition().name("appearsIn").type(nonNull(list(episodeType))).build()) + .field(newFieldDefinition().name("friends").type(list(typeRef("Character"))).build()) + .field(newFieldDefinition().name("id").type(nonNull(GraphQLID)).build()) + .field(newFieldDefinition().name("madeOn").type(planetType).build()) + .field(newFieldDefinition().name("name").type(nonNull(GraphQLString)).build()) + .field(newFieldDefinition().name("primaryFunction").type(GraphQLString).build()) + .build() + def humanType = newObject().name("Human").withInterfaces(characterType, nodeType) + .field(newFieldDefinition().name("appearsIn").type(nonNull(list(episodeType))).build()) + .field(newFieldDefinition().name("friends").type(list(typeRef("Character"))).build()) + .field(newFieldDefinition().name("homePlanet").type(GraphQLString).build()) + .field(newFieldDefinition().name("id").type(nonNull(GraphQLID)).build()) + .field(newFieldDefinition().name("name").type(nonNull(GraphQLString)).build()) + .build() + def humanoidType = newUnionType().name("Humanoid") + .definition(newUnionTypeDefinition().comments(makeComments(" union type Humanoid comment 1")).build()) + .possibleTypes(humanType, droidType) + .build() + def queryType = newObject().name("Query") + .definition(newObjectTypeDefinition().comments(makeComments(" type query comment 1", " type query comment 2")).build()) + .field(newFieldDefinition().name("hero").type(characterType) + .definition(FieldDefinition.newFieldDefinition().comments(makeComments(" query field 'hero' comment")).build()) + .argument(newArgument().name("episode").type(episodeType).build()) + .build()) + .field(newFieldDefinition().name("humanoid").type(humanoidType) + .definition(FieldDefinition.newFieldDefinition().comments(makeComments(" query field 'humanoid' comment")).build()) + .argument(newArgument().name("id").type(nonNull(GraphQLID)).build()) + .build()) + .build() + def gunType = GraphQLInputObjectType.newInputObject().name("Gun") + .definition(newInputObjectDefinition().comments(makeComments(" input type Gun comment 1")).build()) + .field(newInputObjectField().name("name").type(GraphQLString) + .definition(newInputValueDefinition().comments(makeComments(" gun 'name' input value comment")).build()).build()) + .field(newInputObjectField().name("caliber").type(GraphQLInt) + .definition(newInputValueDefinition().comments(makeComments(" gun 'caliber' input value comment")).build()).build()) + .build() + def schema = GraphQLSchema.newSchema() + .additionalDirective(exampleDirective) + .codeRegistry(GraphQLCodeRegistry.newCodeRegistry() + .typeResolver(characterType, resolver) + .typeResolver(humanoidType, resolver) + .typeResolver(nodeType, resolver) + .build()) + .definition(SchemaDefinition.newSchemaDefinition().comments( + makeComments("schema comment 1", " schema comment 2 with leading spaces")).build()) + .mutation(newObject().name("Mutation") + .field(newFieldDefinition().name("shoot").type(queryType).arguments(List.of( + newArgument().name("id").type(nonNull(GraphQLString)) + .definition(newInputValueDefinition().comments(makeComments(" arg 'id'")).build()).build(), + newArgument().name("with").type(gunType) + .definition(newInputValueDefinition().comments(makeComments(" arg 'with'")).build()).build())) + .build()) + .build()) + .query(queryType) + .build() + when: + def result = new SchemaPrinter(defaultOptions().includeSchemaDefinition(true).includeAstDefinitionComments(true)).print(schema) + println(result) + + then: + result == SDL_WITH_COMMENTS + } + + def "parses, generates and prints with AST comments"() { + given: + def registry = new SchemaParser().parse(SDL_WITH_COMMENTS) + def wiring = newRuntimeWiring() + .scalar(mockScalar(registry.scalars().get("Asteroid"))) + .type(mockTypeRuntimeWiring("Character", true)) + .type(mockTypeRuntimeWiring("Humanoid", true)) + .type(mockTypeRuntimeWiring("Node", true)) + .build() + def options = SchemaGenerator.Options.defaultOptions().useCommentsAsDescriptions(false) + def schema = new SchemaGenerator().makeExecutableSchema(options, registry, wiring) + when: + def result = new SchemaPrinter(defaultOptions().includeSchemaDefinition(true).includeAstDefinitionComments(true)).print(schema) + println(result) + + // @TODO: Schema Parser seems to be ignoring directive and scalar comments and needs to be fixed. + // The expected result below should be the same as the SDL_WITH_COMMENTS above BUT with the two comments temporarily removed. + then: + result == '''#schema comment 1 +# schema comment 2 with leading spaces +schema { + query: Query + mutation: Mutation +} + +"Marks the field, argument, input field or enum value as deprecated" +directive @deprecated( + "The reason for the deprecation" + reason: String = "No longer supported" + ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION + +" custom directive 'example' description 1" +directive @example on ENUM_VALUE + +"Directs the executor to include this field or fragment only when the `if` argument is true" +directive @include( + "Included when true." + if: Boolean! + ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + +"Directs the executor to skip this field or fragment when the `if` argument is true." +directive @skip( + "Skipped when true." + if: Boolean! + ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT + +"Exposes a URL that specifies the behaviour of this scalar." +directive @specifiedBy( + "The URL that specifies the behaviour of this scalar." + url: String! + ) on SCALAR + +# interface Character comment 1 +# interface Character comment 2 +interface Character implements Node { + appearsIn: [Episode] + friends: [Character] + id: ID! + name: String +} + +interface Node { + id: ID! +} + +# union type Humanoid comment 1 +union Humanoid = Droid | Human + +type Droid implements Character & Node { + appearsIn: [Episode]! + friends: [Character] + id: ID! + madeOn: Planet + name: String! + primaryFunction: String +} + +type Human implements Character & Node { + appearsIn: [Episode]! + friends: [Character] + homePlanet: String + id: ID! + name: String! +} + +type Mutation { + shoot( + # arg 'id\' + id: String!, + # arg 'with\' + with: Gun + ): Query +} + +type Planet { + hitBy: Asteroid + name: String +} + +# type query comment 1 +# type query comment 2 +type Query { + # query field 'hero' comment + hero(episode: Episode): Character + # query field 'humanoid' comment + humanoid(id: ID!): Humanoid +} + +# enum Episode comment 1 +# enum Episode comment 2 +enum Episode { + # enum value EMPIRE comment 1 + EMPIRE + JEDI + NEWHOPE @example +} + +"desc" +scalar Asteroid + +# input type Gun comment 1 +input Gun { + # gun 'caliber' input value comment + caliber: Int + # gun 'name' input value comment + name: String +} +''' + } + + def "issue 3285 - deprecated defaultValue on programmatic args prints as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = GraphQLSchema.newSchema().query(queryObjType).build() + + + when: + def options = defaultOptions().includeDirectiveDefinitions(false) + def sdl = new SchemaPrinter(options).print(schema) + then: + sdl == '''type Query { + f(arg: String = null): String +} +''' + } + + def "deprecated directive with custom reason"() { + given: + def enumType = newEnum().name("Enum") + .values(List.of( + GraphQLEnumValueDefinition.newEnumValueDefinition().name("DEPRECATED_WITH_REASON").deprecationReason("Custom enum value reason").build())) + .build() + def fieldType = newObject().name("Field") + .field(newFieldDefinition().name("deprecatedWithReason").type(enumType).deprecate("Custom field reason").build()) + .build() + def inputType = GraphQLInputObjectType.newInputObject().name("Input") + .field(newInputObjectField().name("deprecatedWithReason").type(enumType).deprecate("Custom input reason").build()) + .build() + def queryType = newObject().name("Query") + .field(newFieldDefinition().name("field").type(fieldType) + .argument(newArgument().name("deprecatedWithReason").type(inputType).deprecate("Custom argument reason").build()).build()) + .build() + def schema = GraphQLSchema.newSchema() + .query(queryType) + .build() + when: + def result = "\n" + new SchemaPrinter(noDirectivesOption).print(schema) + println(result) + + then: + result == """ +type Field { + deprecatedWithReason: Enum @deprecated(reason : "Custom field reason") +} + +type Query { + field(deprecatedWithReason: Input @deprecated(reason : "Custom argument reason")): Field +} + +enum Enum { + DEPRECATED_WITH_REASON @deprecated(reason : "Custom enum value reason") +} + +input Input { + deprecatedWithReason: Enum @deprecated(reason : "Custom input reason") +} +""" + } +} diff --git a/src/test/groovy/graphql/schema/usage/SchemaUsageSupportTest.groovy b/src/test/groovy/graphql/schema/usage/SchemaUsageSupportTest.groovy index f0cff6c7fc..07c2db114c 100644 --- a/src/test/groovy/graphql/schema/usage/SchemaUsageSupportTest.groovy +++ b/src/test/groovy/graphql/schema/usage/SchemaUsageSupportTest.groovy @@ -1,6 +1,11 @@ package graphql.schema.usage import graphql.TestUtil +import graphql.schema.GraphQLAppliedDirective +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.SchemaTransformer +import graphql.schema.visitor.GraphQLSchemaTraversalControl +import graphql.schema.visitor.GraphQLSchemaVisitor import spock.lang.Specification class SchemaUsageSupportTest extends Specification { @@ -193,10 +198,10 @@ class SchemaUsageSupportTest extends Specification { !schemaUsage.isStronglyReferenced(schema, "UnRefInputTypeDirective") !schemaUsage.isStronglyReferenced(schema, "UnRefDirectiveInputType") - schemaUsage.getUnReferencedElements(schema).collect {it.name}.sort() == + schemaUsage.getUnReferencedElements(schema).collect { it.name }.sort() == ["UnIRef1", "UnRef1", "UnRefDirectiveInputType", "UnRefEnum1", "UnRefHangingInputType", "UnRefHangingInputType2", "UnRefHangingInputType3", - "UnRefHangingType","UnRefHangingType2", "UnRefInput1", + "UnRefHangingType", "UnRefHangingType2", "UnRefInput1", "UnRefFieldDirective", "UnRefInputTypeDirective", "UnRefHangingArgDirective"].sort() } @@ -233,17 +238,43 @@ class SchemaUsageSupportTest extends Specification { def schemaUsage = SchemaUsageSupport.getSchemaUsage(schema) then: - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingType") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingType2") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingType3") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType2") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType3") - ! schemaUsage.isStronglyReferenced(schema, "UnRefHangingArgDirective") - - schemaUsage.getDirectiveReferenceCounts()["UnRefHangingArgDirective"] == 1 + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingType") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingType2") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingType3") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType2") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingInputType3") + !schemaUsage.isStronglyReferenced(schema, "UnRefHangingArgDirective") + + // 2 because of the dual nature of directives and applied directives + schemaUsage.getDirectiveReferenceCounts()["UnRefHangingArgDirective"] == 2 schemaUsage.getArgumentReferenceCounts()["UnRefHangingInputType"] == 1 schemaUsage.getFieldReferenceCounts()["UnRefHangingType2"] == 2 - schemaUsage.getArgumentReferenceCounts()["UnRefHangingInputType3"] == 2 + schemaUsage.getArgumentReferenceCounts()["UnRefHangingInputType3"] == 3 + } + + def "can handle cleared directives"() { + // https://github.com/graphql-java/graphql-java/issues/3267 + + + def schema = TestUtil.schema(sdl) + schema = new SchemaTransformer().transform(schema, new GraphQLSchemaVisitor() { + + @Override + GraphQLSchemaTraversalControl visitFieldDefinition(GraphQLFieldDefinition fieldDef, GraphQLSchemaVisitor.FieldDefinitionVisitorEnvironment env) { + if (fieldDef.getAppliedDirective("RefFieldDirective") != null) { + List directives = fieldDef.getAppliedDirectives(); + fieldDef = fieldDef.transform( + f -> f.clearDirectives().replaceAppliedDirectives(directives) + ) + } + return env.changeNode(fieldDef) + } + }.toTypeVisitor()) + + when: + def schemaUsage = SchemaUsageSupport.getSchemaUsage(schema) + then: + schemaUsage.isStronglyReferenced(schema, "RefFieldDirective") } } diff --git a/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy b/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy new file mode 100644 index 0000000000..813e54672b --- /dev/null +++ b/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy @@ -0,0 +1,36 @@ +package graphql.schema.validation + +import graphql.TestUtil +import graphql.schema.idl.SchemaGenerator +import graphql.schema.idl.SchemaParser +import spock.lang.Specification + +class OneOfInputObjectRulesTest extends Specification { + + def "oneOf fields must be the right shape"() { + + def sdl = """ + type Query { + f(arg : OneOfInputType) : String + } + + input OneOfInputType @oneOf { + ok : String + badNonNull : String! + badDefaulted : String = "default" + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 2 + schemaProblem.errors[0].description == "OneOf input field OneOfInputType.badNonNull must be nullable." + schemaProblem.errors[0].classification == SchemaValidationErrorType.OneOfNonNullableField + schemaProblem.errors[1].description == "OneOf input field OneOfInputType.badDefaulted cannot have a default value." + schemaProblem.errors[1].classification == SchemaValidationErrorType.OneOfDefaultValueOnField + } +} diff --git a/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy b/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy index e69ed6012d..6efd8acbe8 100644 --- a/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy +++ b/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy @@ -11,7 +11,7 @@ class SchemaValidatorTest extends Specification { def validator = new SchemaValidator() def rules = validator.rules then: - rules.size() == 7 + rules.size() == 8 rules[0] instanceof NoUnbrokenInputCycles rules[1] instanceof TypesImplementInterfaces rules[2] instanceof TypeAndFieldRule @@ -19,6 +19,6 @@ class SchemaValidatorTest extends Specification { rules[4] instanceof AppliedDirectivesAreValid rules[5] instanceof AppliedDirectiveArgumentsAreValid rules[6] instanceof InputAndOutputTypesUsedAppropriately + rules[7] instanceof OneOfInputObjectRules } - } diff --git a/src/test/groovy/graphql/util/LockKitTest.groovy b/src/test/groovy/graphql/util/LockKitTest.groovy new file mode 100644 index 0000000000..e205ff0574 --- /dev/null +++ b/src/test/groovy/graphql/util/LockKitTest.groovy @@ -0,0 +1,70 @@ +package graphql.util + +import spock.lang.Specification + +class LockKitTest extends Specification { + + + def "can run code under a lock (simple test)"() { + def sideEffect = 0 + + when: + LockKit.ReentrantLock lockedCode = new LockKit.ReentrantLock() + lockedCode.runLocked { sideEffect++ } + + then: + sideEffect == 1 + } + + def "can call code under a lock (simple test)"() { + + when: + LockKit.ReentrantLock lockedCode = new LockKit.ReentrantLock() + def val = lockedCode.callLocked { return "x" } + + then: + val == "x" + } + + def "is reentrant"() { + + def sideEffect = 0 + + when: + LockKit.ReentrantLock lockedCode = new LockKit.ReentrantLock() + lockedCode.runLocked { + sideEffect++ + lockedCode.runLocked { + sideEffect++ + } + } + + then: + sideEffect == 2 + } + + def "can compute once"() { + def sideEffect = 0 + + when: + LockKit.ComputedOnce computedOnce = new LockKit.ComputedOnce() + + then: + !computedOnce.hasBeenComputed() + sideEffect == 0 + + when: + computedOnce.runOnce { sideEffect++ } + + then: + computedOnce.hasBeenComputed() + sideEffect == 1 + + when: + computedOnce.runOnce { sideEffect++ } + + then: + computedOnce.hasBeenComputed() + sideEffect == 1 + } +} diff --git a/src/test/groovy/graphql/validation/SpecValidationSchema.java b/src/test/groovy/graphql/validation/SpecValidationSchema.java index 398dd1ad3e..45a6b2637f 100644 --- a/src/test/groovy/graphql/validation/SpecValidationSchema.java +++ b/src/test/groovy/graphql/validation/SpecValidationSchema.java @@ -1,5 +1,6 @@ package graphql.validation; +import graphql.Directives; import graphql.Scalars; import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLCodeRegistry; @@ -184,6 +185,18 @@ public class SpecValidationSchema { .validLocations(FIELD, FRAGMENT_SPREAD, FRAGMENT_DEFINITION, INLINE_FRAGMENT, QUERY) .build(); + public static final GraphQLInputObjectType oneOfInputType = GraphQLInputObjectType.newInputObject() + .name("oneOfInputType") + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .field(GraphQLInputObjectField.newInputObjectField() + .name("a") + .type(GraphQLString)) + .field(GraphQLInputObjectField.newInputObjectField() + .name("b") + .type(GraphQLString)) + .build(); + + public static final GraphQLObjectType queryRoot = GraphQLObjectType.newObject() .name("QueryRoot") .field(newFieldDefinition().name("dog").type(dog) @@ -191,6 +204,9 @@ public class SpecValidationSchema { .withDirective(dogDirective) ) .field(newFieldDefinition().name("pet").type(pet)) + .field(newFieldDefinition().name("oneOfField").type(GraphQLString) + .argument(newArgument().name("oneOfArg").type(oneOfInputType).build()) + ) .build(); public static final GraphQLObjectType subscriptionRoot = GraphQLObjectType.newObject() diff --git a/src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy b/src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy index f9586b337f..babf549a0a 100644 --- a/src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy +++ b/src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy @@ -337,6 +337,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification { def schema = schema(''' type Dog { nickname: String + name : String } type Query { dog: Dog } ''') @@ -349,6 +350,58 @@ class OverlappingFieldsCanBeMergedTest extends Specification { errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)] } + def 'issue 3332 - Alias masking direct field access non fragment'() { + given: + def query = """ + { dog { + name: nickname + name + }} + """ + def schema = schema(''' + type Dog { + name : String + nickname: String + } + type Query { dog: Dog } + ''') + when: + traverse(query, schema) + + then: + errorCollector.getErrors().size() == 1 + errorCollector.getErrors()[0].message == "Validation error (FieldsConflict@[dog]) : 'name' : 'nickname' and 'name' are different fields" + errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)] + } + + def 'issue 3332 -Alias masking direct field access non fragment with non null parent type'() { + given: + def query = """ + query GetCat { + cat { + foo1 + foo1: foo2 + } + } + """ + def schema = schema(''' + type Query { + cat: Cat! # non null parent type + } + type Cat { + foo1: String! + foo2: String! + } + ''') + when: + traverse(query, schema) + + then: + errorCollector.getErrors().size() == 1 + errorCollector.getErrors()[0].message == "Validation error (FieldsConflict@[cat]) : 'foo1' : 'foo1' and 'foo2' are different fields" + errorCollector.getErrors()[0].locations == [new SourceLocation(4, 17), new SourceLocation(5, 17)] + } + def 'conflicting args'() { given: def query = """ diff --git a/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy b/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy index dab3510daa..5ccaff13dd 100644 --- a/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy +++ b/src/test/groovy/graphql/validation/rules/VariableTypesMatchTest.groovy @@ -2,8 +2,10 @@ package graphql.validation.rules import graphql.StarWarsSchema +import graphql.TestUtil import graphql.i18n.I18n import graphql.parser.Parser +import graphql.schema.GraphQLSchema import graphql.validation.LanguageTraversal import graphql.validation.RulesVisitor import graphql.validation.ValidationContext @@ -14,10 +16,10 @@ import spock.lang.Specification class VariableTypesMatchTest extends Specification { ValidationErrorCollector errorCollector = new ValidationErrorCollector() - def traverse(String query) { + def traverse(String query, GraphQLSchema schema = StarWarsSchema.starWarsSchema) { def document = Parser.parse(query) I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH) - def validationContext = new ValidationContext(StarWarsSchema.starWarsSchema, document, i18n) + def validationContext = new ValidationContext(schema, document, i18n) def variableTypesMatchRule = new VariableTypesMatch(validationContext, errorCollector) def languageTraversal = new LanguageTraversal() languageTraversal.traverse(document, new RulesVisitor(validationContext, [variableTypesMatchRule])) @@ -56,7 +58,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) // #991: describe which types were mismatched in error message - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "invalid variables in fragment spread"() { @@ -78,7 +80,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'xid' of type 'String' used in position expecting type 'String!'" } def "mixed validity operations, valid first"() { @@ -104,7 +106,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "mixed validity operations, invalid first"() { @@ -130,7 +132,7 @@ class VariableTypesMatchTest extends Specification { then: errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) - errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } def "multiple invalid operations"() { @@ -158,11 +160,97 @@ class VariableTypesMatchTest extends Specification { errorCollector.getErrors().size() == 2 errorCollector.errors.any { it.validationErrorType == ValidationErrorType.VariableTypeMismatch && - it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'String' does not match expected type 'String!'" + it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'String' used in position expecting type 'String!'" } errorCollector.errors.any { it.validationErrorType == ValidationErrorType.VariableTypeMismatch && - it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable type 'Boolean' does not match expected type 'String!'" + it.message == "Validation error (VariableTypeMismatch@[QueryType/human]) : Variable 'id' of type 'Boolean' used in position expecting type 'String!'" } } + + + def "issue 3276 - invalid variables in object field values with no defaults in location"() { + + def sdl = ''' + type Query { + items(pagination: Pagination = {limit: 1, offset: 1}): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $limit: Int, $offset: Int) { + items( + pagination: {limit: $limit, offset: $offset} + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.containsValidationError(ValidationErrorType.VariableTypeMismatch) + errorCollector.errors[0].message == "Validation error (VariableTypeMismatch@[items]) : Variable 'limit' of type 'Int' used in position expecting type 'Int!'" + } + + def "issue 3276 - valid variables because of schema defaults with nullable variable"() { + + def sdl = ''' + type Query { + items(pagination: Pagination! = {limit: 1, offset: 1}): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $var : Pagination) { + items( + pagination: $var + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.errors.isEmpty() + } + + def "issue 3276 - valid variables because of variable defaults"() { + + def sdl = ''' + type Query { + items(pagination: Pagination!): [String] + } + input Pagination { + limit: Int! + offset: Int! + } + ''' + def schema = TestUtil.schema(sdl) + given: + def query = ''' + query Items( $var : Pagination = {limit: 1, offset: 1}) { + items( + pagination: $var + ) + } + ''' + + when: + traverse(query, schema) + + then: + errorCollector.errors.isEmpty() + } } diff --git a/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy b/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy index c6bead4ef6..b05e425090 100644 --- a/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy +++ b/src/test/groovy/graphql/validation/rules/VariablesTypesMatcherTest.groovy @@ -1,6 +1,7 @@ package graphql.validation.rules import graphql.language.BooleanValue +import graphql.language.StringValue import spock.lang.Specification import spock.lang.Unroll @@ -18,7 +19,7 @@ class VariablesTypesMatcherTest extends Specification { def "#variableType with default value #defaultValue and expected #expectedType should result: #result "() { expect: - typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType) == result + typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType, null) == result where: variableType | defaultValue | expectedType || result @@ -27,4 +28,18 @@ class VariablesTypesMatcherTest extends Specification { nonNull(GraphQLBoolean) | new BooleanValue(true) | GraphQLBoolean || true nonNull(GraphQLString) | null | list(GraphQLString) || false } + + @Unroll + def "issue 3276 - #variableType with default value #defaultValue and expected #expectedType with #locationDefaultValue should result: #result "() { + + expect: + typesMatcher.doesVariableTypesMatch(variableType, defaultValue, expectedType, locationDefaultValue) == result + + where: + variableType | defaultValue | expectedType | locationDefaultValue || result + GraphQLString | null | nonNull(GraphQLString) | null || false + GraphQLString | null | nonNull(GraphQLString) | StringValue.of("x") || true + GraphQLString | StringValue.of("x") | nonNull(GraphQLString) | StringValue.of("x") || true + GraphQLString | StringValue.of("x") | nonNull(GraphQLString) | null || true + } } diff --git a/src/test/groovy/readme/InstrumentationExamples.java b/src/test/groovy/readme/InstrumentationExamples.java index fe27b6e161..f404ca4ee5 100644 --- a/src/test/groovy/readme/InstrumentationExamples.java +++ b/src/test/groovy/readme/InstrumentationExamples.java @@ -15,6 +15,7 @@ import graphql.execution.instrumentation.fieldvalidation.FieldValidationEnvironment; import graphql.execution.instrumentation.fieldvalidation.FieldValidationInstrumentation; import graphql.execution.instrumentation.fieldvalidation.SimpleFieldValidation; +import graphql.execution.instrumentation.parameters.InstrumentationCreateStateParameters; import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; import graphql.execution.instrumentation.tracing.TracingInstrumentation; @@ -64,7 +65,7 @@ void recordTiming(String key, long time) { class CustomInstrumentation extends SimplePerformantInstrumentation { @Override - public InstrumentationState createState() { + public @Nullable InstrumentationState createState(InstrumentationCreateStateParameters parameters) { // // instrumentation state is passed during each invocation of an Instrumentation method // and allows you to put stateful data away and reference it during the query execution