diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index dd09d092..6d2cae3b 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -11,33 +11,6 @@ jobs: - uses: actions/checkout@v2 - uses: gradle/wrapper-validation-action@v1 - verify-google-java-format: - name: Google Java Format Verification - runs-on: ubuntu-latest - needs: validation - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Setup Java - uses: actions/setup-java@v2 - with: - distribution: 'zulu' - java-version: 15 - - name: Cache Gradle - uses: actions/cache@v2 - env: - java-version: 15 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-${{ env.java-version }}-gradle-${{ hashFiles('**/*.gradle*') }} - restore-keys: ${{ runner.os }}-${{ env.java-version }}-gradle- - - name: Make gradlew executable - run: chmod +x ./gradlew - - name: Gradle Check - run: ./gradlew --info build -x test - test: name: Test run strategy: @@ -70,11 +43,11 @@ jobs: run: chmod +x ./gradlew - name: Gradle Check (non-Windows) if: matrix.os != 'windows-latest' - run: ./gradlew --info check -x verifyGoogleJavaFormat + run: ./gradlew --info check - name: Gradle Check (Windows) if: matrix.os == 'windows-latest' shell: cmd - run: gradlew --info check -x verifyGoogleJavaFormat + run: gradlew --info check build: name: Sonar analysis @@ -112,4 +85,4 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: ./gradlew build jacocoTestReport sonarqube --info -x verifyGoogleJavaFormat + run: ./gradlew build jacocoTestReport sonarqube --info diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 407451bf..f2fd3bde 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,7 +35,7 @@ jobs: - name: Make gradlew executable run: chmod +x ./gradlew - name: Gradle Check - run: ./gradlew --info check -x verifyGoogleJavaFormat + run: ./gradlew --info check build: name: Publish release diff --git a/.github/workflows/snapshot.yml b/.github/workflows/snapshot.yml index d89b53ae..b08ccb1c 100644 --- a/.github/workflows/snapshot.yml +++ b/.github/workflows/snapshot.yml @@ -12,33 +12,6 @@ jobs: - uses: actions/checkout@v2 - uses: gradle/wrapper-validation-action@v1 - verify-google-java-format: - name: Google Java Format Verification - runs-on: ubuntu-latest - needs: validation - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Setup Java - uses: actions/setup-java@v2 - with: - distribution: 'zulu' - java-version: 15 - - name: Cache Gradle - uses: actions/cache@v2 - env: - java-version: 15 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-${{ env.java-version }}-gradle-${{ hashFiles('**/*.gradle*') }} - restore-keys: ${{ runner.os }}-${{ env.java-version }}-gradle- - - name: Make gradlew executable - run: chmod +x ./gradlew - - name: Gradle Check - run: ./gradlew --info build -x test - test: name: Test run needs: validation @@ -65,7 +38,7 @@ jobs: - name: Make gradlew executable run: chmod +x ./gradlew - name: Gradle Check - run: ./gradlew --info check -x verifyGoogleJavaFormat + run: ./gradlew --info check build: name: Publish snapshot @@ -97,7 +70,7 @@ jobs: env: OSS_USER_TOKEN_KEY: ${{ secrets.OSS_USER_TOKEN_KEY }} OSS_USER_TOKEN_PASS: ${{ secrets.OSS_USER_TOKEN_PASS }} - run: ./gradlew clean build publish -x test -x verifyGoogleJavaFormat + run: ./gradlew clean build publish -x test sonar: name: Sonar analysis @@ -129,4 +102,4 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} if: env.SONAR_TOKEN != null - run: ./gradlew build jacocoTestReport sonarqube --info -x verifyGoogleJavaFormat + run: ./gradlew build jacocoTestReport sonarqube --info diff --git a/build.gradle b/build.gradle index a0936f5c..9639c85f 100644 --- a/build.gradle +++ b/build.gradle @@ -31,7 +31,6 @@ plugins { id "org.sonarqube" version "3.2.0" id "jacoco" id 'io.codearte.nexus-staging' version '0.30.0' - id 'com.github.sherter.google-java-format' version '0.9' apply false } sonarqube { @@ -49,7 +48,6 @@ subprojects { apply plugin: 'java' apply plugin: 'maven-publish' apply plugin: 'signing' - apply plugin: 'com.github.sherter.google-java-format' repositories { mavenLocal() @@ -80,8 +78,6 @@ subprojects { compileJava.dependsOn(processResources) - compileJava.mustRunAfter verifyGoogleJavaFormat - test { useJUnitPlatform() diff --git a/github-build.sh b/github-build.sh index 6e9d498f..cfec06b2 100644 --- a/github-build.sh +++ b/github-build.sh @@ -45,7 +45,7 @@ git config --global user.name "GitHub Actions" echo "Deploying release to Maven Central" removeSnapshots -./gradlew clean build publish closeAndReleaseRepository -x verifyGoogleJavaFormat +./gradlew clean build publish closeAndReleaseRepository commitRelease bumpVersion diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureBatchedExecutionResult.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureBatchedExecutionResult.java new file mode 100644 index 00000000..a97c3391 --- /dev/null +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureBatchedExecutionResult.java @@ -0,0 +1,26 @@ +package graphql.kickstart.execution; + +import graphql.ExecutionResult; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import lombok.Getter; +import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor +class FutureBatchedExecutionResult implements FutureExecutionResult { + + @Getter + private final GraphQLInvocationInput invocationInput; + private final CompletableFuture> batched; + + @Override + public CompletableFuture thenApplyQueryResult() { + return batched.thenApply(GraphQLQueryResult::create); + } + + @Override + public void cancel() { + batched.cancel(true); + } +} diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureErrorExecutionResult.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureErrorExecutionResult.java new file mode 100644 index 00000000..745eeace --- /dev/null +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureErrorExecutionResult.java @@ -0,0 +1,26 @@ +package graphql.kickstart.execution; + +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import java.util.concurrent.CompletableFuture; +import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor +class FutureErrorExecutionResult implements FutureExecutionResult { + + private final GraphQLErrorQueryResult errorQueryResult; + + @Override + public CompletableFuture thenApplyQueryResult() { + return CompletableFuture.completedFuture(errorQueryResult); + } + + @Override + public GraphQLInvocationInput getInvocationInput() { + return null; + } + + @Override + public void cancel() { + // nothing to do + } +} diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureExecutionResult.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureExecutionResult.java new file mode 100644 index 00000000..cfbf6b06 --- /dev/null +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureExecutionResult.java @@ -0,0 +1,29 @@ +package graphql.kickstart.execution; + +import graphql.ExecutionResult; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import java.util.List; +import java.util.concurrent.CompletableFuture; + +public interface FutureExecutionResult { + + static FutureExecutionResult single( + GraphQLInvocationInput invocationInput, CompletableFuture single) { + return new FutureSingleExecutionResult(invocationInput, single); + } + + static FutureExecutionResult batched( + GraphQLInvocationInput invocationInput, CompletableFuture> batched) { + return new FutureBatchedExecutionResult(invocationInput, batched); + } + + static FutureExecutionResult error(GraphQLErrorQueryResult result) { + return new FutureErrorExecutionResult(result); + } + + CompletableFuture thenApplyQueryResult(); + + GraphQLInvocationInput getInvocationInput(); + + void cancel(); +} diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureSingleExecutionResult.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureSingleExecutionResult.java new file mode 100644 index 00000000..5e2e70d2 --- /dev/null +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/FutureSingleExecutionResult.java @@ -0,0 +1,25 @@ +package graphql.kickstart.execution; + +import graphql.ExecutionResult; +import graphql.kickstart.execution.input.GraphQLInvocationInput; +import java.util.concurrent.CompletableFuture; +import lombok.Getter; +import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor +class FutureSingleExecutionResult implements FutureExecutionResult { + + @Getter + private final GraphQLInvocationInput invocationInput; + private final CompletableFuture single; + + @Override + public CompletableFuture thenApplyQueryResult() { + return single.thenApply(GraphQLQueryResult::create); + } + + @Override + public void cancel() { + single.cancel(true); + } +} diff --git a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java index 435fb260..0d566ea2 100644 --- a/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java +++ b/graphql-java-kickstart/src/main/java/graphql/kickstart/execution/GraphQLInvoker.java @@ -12,7 +12,9 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +@Slf4j @RequiredArgsConstructor public class GraphQLInvoker { @@ -20,6 +22,15 @@ public class GraphQLInvoker { private final BatchedDataLoaderGraphQLBuilder batchedDataLoaderGraphQLBuilder; private final GraphQLInvokerProxy proxy = GraphQL::executeAsync; + public FutureExecutionResult execute(GraphQLInvocationInput invocationInput) { + if (invocationInput instanceof GraphQLSingleInvocationInput) { + return FutureExecutionResult.single( + invocationInput, executeAsync((GraphQLSingleInvocationInput) invocationInput)); + } + return FutureExecutionResult.batched( + invocationInput, executeAsync((GraphQLBatchedInvocationInput) invocationInput)); + } + public CompletableFuture executeAsync( GraphQLSingleInvocationInput invocationInput) { GraphQL graphQL = graphQLBuilder.build(invocationInput.getSchema()); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AsyncTimeoutListener.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AsyncTimeoutListener.java new file mode 100644 index 00000000..7d39ad89 --- /dev/null +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/AsyncTimeoutListener.java @@ -0,0 +1,14 @@ +package graphql.kickstart.servlet; + +import java.io.IOException; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; + +interface AsyncTimeoutListener extends AsyncListener { + + default void onComplete(AsyncEvent event) throws IOException {} + + default void onError(AsyncEvent event) throws IOException {} + + default void onStartAsync(AsyncEvent event) throws IOException {} +} diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java index cc1b39ec..30af5490 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java @@ -1,9 +1,11 @@ package graphql.kickstart.servlet; -import static graphql.kickstart.servlet.HttpRequestHandler.STATUS_BAD_REQUEST; - +import graphql.ExecutionResult; +import graphql.ExecutionResultImpl; +import graphql.kickstart.execution.FutureExecutionResult; import graphql.kickstart.execution.GraphQLInvoker; import graphql.kickstart.execution.GraphQLQueryResult; +import graphql.kickstart.execution.error.GenericGraphQLError; import graphql.kickstart.execution.input.GraphQLBatchedInvocationInput; import graphql.kickstart.execution.input.GraphQLInvocationInput; import graphql.kickstart.execution.input.GraphQLSingleInvocationInput; @@ -11,7 +13,11 @@ import graphql.kickstart.servlet.input.BatchInputPreProcessor; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Optional; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.AsyncContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -31,73 +37,118 @@ public void execute( GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { + ListenerHandler listenerHandler = + ListenerHandler.start(request, response, configuration.getListeners()); if (request.isAsyncSupported()) { - AsyncContext asyncContext = - request.isAsyncStarted() - ? request.getAsyncContext() - : request.startAsync(request, response); - asyncContext.setTimeout(configuration.getAsyncTimeout()); - invokeAndHandle(invocationInput, request, response) - .thenAccept(aVoid -> asyncContext.complete()); + invokeAndHandleAsync(invocationInput, request, response, listenerHandler); } else { - invokeAndHandle(invocationInput, request, response).join(); + handle(invoke(invocationInput, request, response), request, response, listenerHandler).join(); } } - private CompletableFuture invokeAndHandle( + private void invokeAndHandleAsync( GraphQLInvocationInput invocationInput, HttpServletRequest request, - HttpServletResponse response) { - ListenerHandler listenerHandler = - ListenerHandler.start(request, response, configuration.getListeners()); - return invoke(invocationInput, request, response) + HttpServletResponse response, + ListenerHandler listenerHandler) { + AsyncContext asyncContext = + request.isAsyncStarted() + ? request.getAsyncContext() + : request.startAsync(request, response); + asyncContext.setTimeout(configuration.getAsyncTimeout()); + AtomicReference futureHolder = new AtomicReference<>(); + AsyncTimeoutListener timeoutListener = + event -> Optional.ofNullable(futureHolder.get()).ifPresent(FutureExecutionResult::cancel); + asyncContext.addListener(timeoutListener); + asyncContext.start( + () -> { + FutureExecutionResult futureResult = invoke(invocationInput, request, response); + futureHolder.set(futureResult); + handle(futureResult, request, response, listenerHandler); + }); + } + + private CompletableFuture handle( + FutureExecutionResult futureResult, + HttpServletRequest request, + HttpServletResponse response, + ListenerHandler listenerHandler) { + return futureResult + .thenApplyQueryResult() .thenAccept( - result -> - writeResultResponse(invocationInput, result, request, response, listenerHandler)) - .exceptionally(t -> writeErrorResponse(t, response, listenerHandler)) - .thenAccept(aVoid -> listenerHandler.onFinally()); + it -> writeResultResponse(futureResult.getInvocationInput(), it, request, response)) + .thenAccept(it -> listenerHandler.onSuccess()) + .exceptionally( + t -> + writeErrorResponse( + futureResult.getInvocationInput(), request, response, listenerHandler, t)) + .thenAccept(it -> listenerHandler.onFinally()); } private void writeResultResponse( GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult, HttpServletRequest request, - HttpServletResponse response, - ListenerHandler listenerHandler) { + HttpServletResponse response) { QueryResponseWriter queryResponseWriter = createWriter(invocationInput, queryResult); try { queryResponseWriter.write(request, response); - listenerHandler.onSuccess(); } catch (IOException e) { throw new UncheckedIOException(e); } } + private Void writeErrorResponse( + GraphQLInvocationInput invocationInput, + HttpServletRequest request, + HttpServletResponse response, + ListenerHandler listenerHandler, + Throwable t) { + Throwable cause = getCause(t); + if (!response.isCommitted()) { + writeResultResponse( + invocationInput, GraphQLQueryResult.create(toErrorResult(cause)), request, response); + listenerHandler.onError(cause); + } else { + log.warn( + "Cannot write GraphQL response, because the HTTP response is already committed. It most likely timed out."); + } + return null; + } + + private Throwable getCause(Throwable t) { + return t instanceof CompletionException && t.getCause() != null ? t.getCause() : t; + } + + private ExecutionResult toErrorResult(Throwable t) { + String message = + t instanceof CancellationException + ? "Execution canceled because timeout of " + + configuration.getAsyncTimeout() + + " millis was reached" + : t.getMessage(); + if (message == null) { + message = "Unexpected error occurred"; + } + return new ExecutionResultImpl(new GenericGraphQLError(message)); + } + protected QueryResponseWriter createWriter( GraphQLInvocationInput invocationInput, GraphQLQueryResult queryResult) { return queryResponseWriterFactory.createWriter(invocationInput, queryResult, configuration); } - private Void writeErrorResponse( - Throwable t, HttpServletResponse response, ListenerHandler listenerHandler) { - response.setStatus(STATUS_BAD_REQUEST); - log.info( - "Bad request: path was not \"/schema.json\" or no query variable named \"query\" given", t); - listenerHandler.onError(t); - return null; - } - - private CompletableFuture invoke( + private FutureExecutionResult invoke( GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { if (invocationInput instanceof GraphQLSingleInvocationInput) { - return graphQLInvoker.queryAsync(invocationInput); + return graphQLInvoker.execute(invocationInput); } return invokeBatched((GraphQLBatchedInvocationInput) invocationInput, request, response); } - private CompletableFuture invokeBatched( + private FutureExecutionResult invokeBatched( GraphQLBatchedInvocationInput batchedInvocationInput, HttpServletRequest request, HttpServletResponse response) { @@ -105,10 +156,10 @@ private CompletableFuture invokeBatched( BatchInputPreProcessResult result = preprocessor.preProcessBatch(batchedInvocationInput, request, response); if (result.isExecutable()) { - return graphQLInvoker.queryAsync(result.getBatchedInvocationInput()); + return graphQLInvoker.execute(result.getBatchedInvocationInput()); } - return CompletableFuture.completedFuture( + return FutureExecutionResult.error( GraphQLQueryResult.createError(result.getStatusCode(), result.getStatusMessage())); } } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java index 86af54df..86568d29 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/SingleQueryResponseWriter.java @@ -23,5 +23,6 @@ public void write(HttpServletRequest request, HttpServletResponse response) thro byte[] contentBytes = graphQLObjectMapper.serializeResultAsBytes(result); response.setContentLength(contentBytes.length); response.getOutputStream().write(contentBytes); + response.getOutputStream().flush(); } } diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy index 790d8591..6305c27e 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy @@ -1,12 +1,16 @@ package graphql.kickstart.servlet.cache +import graphql.ExecutionResult +import graphql.kickstart.execution.FutureExecutionResult import graphql.kickstart.execution.GraphQLInvoker +import graphql.kickstart.execution.GraphQLObjectMapper import graphql.kickstart.execution.GraphQLQueryResult import graphql.kickstart.execution.input.GraphQLSingleInvocationInput import graphql.kickstart.servlet.GraphQLConfiguration import graphql.kickstart.servlet.HttpRequestInvoker import spock.lang.Specification +import javax.servlet.ServletOutputStream import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse import java.util.concurrent.CompletableFuture @@ -22,6 +26,8 @@ class CachingHttpRequestInvokerTest extends Specification { def httpRequestInvokerMock def graphqlInvoker def configuration + def graphqlObjectMapper + def outputStreamMock def setup() { cacheReaderMock = Mock(CacheReader) @@ -32,11 +38,18 @@ class CachingHttpRequestInvokerTest extends Specification { configuration = Mock(GraphQLConfiguration) httpRequestInvokerMock = Mock(HttpRequestInvoker) graphqlInvoker = Mock(GraphQLInvoker) + graphqlObjectMapper = Mock(GraphQLObjectMapper) + outputStreamMock = Mock(ServletOutputStream) + graphqlInvoker.execute(invocationInputMock) >> FutureExecutionResult.single(invocationInputMock, CompletableFuture.completedFuture(Mock(GraphQLQueryResult))) cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock) configuration.getResponseCacheManager() >> responseCacheManagerMock configuration.getGraphQLInvoker() >> graphqlInvoker + configuration.getObjectMapper() >> graphqlObjectMapper + graphqlObjectMapper.serializeResultAsBytes(_ as ExecutionResult) >> new byte[0] graphqlInvoker.queryAsync(invocationInputMock) >> CompletableFuture.completedFuture(Mock(GraphQLQueryResult)) + + responseMock.getOutputStream() >> outputStreamMock } def "should execute regular invoker if cache not exists"() {