diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 560e73da..62609d9f 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -1,8 +1,5 @@ name: "Pull request" on: - push: - branches-ignore: - - master pull_request: types: [opened, synchronize, reopened] jobs: diff --git a/.gitignore b/.gitignore index cdecd3b5..20acee31 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ build/ *.iml *.ipr *.iws -.idea/ +**/.idea/ target/ /out/ .classpath @@ -11,3 +11,4 @@ target/ .settings bin .DS_Store +/**/out/ diff --git a/examples/osgi/apache-karaf-feature/pom.xml b/examples/osgi/apache-karaf-feature/pom.xml index 2858468a..2bc72eaa 100644 --- a/examples/osgi/apache-karaf-feature/pom.xml +++ b/examples/osgi/apache-karaf-feature/pom.xml @@ -14,7 +14,7 @@ true org.apache.karaf.tooling - 4.0.8 + 4.2.10 @@ -108,7 +108,7 @@ graphql-java-servlet-osgi-examples com.graphql-java-kickstart - 7.3.4-SNAPSHOT + 10.1.0 diff --git a/examples/osgi/apache-karaf-feature/src/main/feature/feature.xml b/examples/osgi/apache-karaf-feature/src/main/feature/feature.xml index cf540b02..124bad9e 100644 --- a/examples/osgi/apache-karaf-feature/src/main/feature/feature.xml +++ b/examples/osgi/apache-karaf-feature/src/main/feature/feature.xml @@ -5,5 +5,6 @@ name="graphql-java-servlet-osgi-examples-karaf-feature"> scr war + http diff --git a/examples/osgi/apache-karaf-package/pom.xml b/examples/osgi/apache-karaf-package/pom.xml index 4ff2f63c..8bb555bf 100644 --- a/examples/osgi/apache-karaf-package/pom.xml +++ b/examples/osgi/apache-karaf-package/pom.xml @@ -110,6 +110,6 @@ graphql-java-servlet-osgi-examples com.graphql-java-kickstart - 7.3.4-SNAPSHOT + 10.1.0 diff --git a/examples/osgi/buildAndRun.sh b/examples/osgi/buildAndRun.sh index 87fdea6d..fec58878 100755 --- a/examples/osgi/buildAndRun.sh +++ b/examples/osgi/buildAndRun.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash mvn clean install -pushd apache-karaf-package/target -tar zxvf graphql-java-servlet-osgi-examples-apache-karaf-package-7.3.4-SNAPSHOT.tar.gz -cd graphql-java-servlet-osgi-examples-apache-karaf-package-7.3.4-SNAPSHOT/bin +pushd apache-karaf-package/target || exit 1 +tar zxvf graphql-java-servlet-osgi-examples-apache-karaf-package-10.1.0.tar.gz +cd graphql-java-servlet-osgi-examples-apache-karaf-package-10.1.0/bin || exit 1 ./karaf debug -popd +popd || exit 1 diff --git a/examples/osgi/pom.xml b/examples/osgi/pom.xml index 38e1efef..c51bd1e0 100644 --- a/examples/osgi/pom.xml +++ b/examples/osgi/pom.xml @@ -14,11 +14,13 @@ pom - 7.3.4-SNAPSHOT - 11.0 - 4.2.4 + 11.0.0-SNAPSHOT + 16.1 + 4.2.10 + 1.8 + 1.8 - 7.3.4-SNAPSHOT + 10.1.0 diff --git a/examples/osgi/providers/pom.xml b/examples/osgi/providers/pom.xml index e088fd99..0a0f1f8d 100644 --- a/examples/osgi/providers/pom.xml +++ b/examples/osgi/providers/pom.xml @@ -8,8 +8,6 @@ maven-bundle-plugin - - true org.apache.felix @@ -51,7 +49,7 @@ graphql-java-servlet-osgi-examples com.graphql-java-kickstart - 7.3.4-SNAPSHOT + 10.1.0 diff --git a/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleGraphQLProvider.java b/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleGraphQLProvider.java index b6acbce7..52a5469e 100644 --- a/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleGraphQLProvider.java +++ b/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleGraphQLProvider.java @@ -1,10 +1,10 @@ -package graphql.kickstart.servlet.examples.osgi; +package graphql.servlet.examples.osgi; import graphql.schema.GraphQLFieldDefinition; import graphql.schema.GraphQLType; -import graphql.servlet.GraphQLMutationProvider; -import graphql.servlet.GraphQLQueryProvider; -import graphql.servlet.GraphQLTypesProvider; +import graphql.kickstart.servlet.osgi.GraphQLMutationProvider; +import graphql.kickstart.servlet.osgi.GraphQLQueryProvider; +import graphql.kickstart.servlet.osgi.GraphQLTypesProvider; import org.osgi.service.component.annotations.Component; import java.util.ArrayList; @@ -38,8 +38,6 @@ public Collection getMutations() { } public Collection getTypes() { - - List types = new ArrayList(); - return types; + return new ArrayList(); } } diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiGraphQLHttpServlet.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiGraphQLHttpServlet.java index 868f3ad5..d908c97a 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiGraphQLHttpServlet.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiGraphQLHttpServlet.java @@ -131,12 +131,7 @@ protected void updateSchema() { updateFuture.cancel(true); } - updateFuture = executor.schedule(new Runnable() { - @Override - public void run() { - doUpdateSchema(); - } - }, schemaUpdateDelay, TimeUnit.MILLISECONDS); + updateFuture = executor.schedule(this::doUpdateSchema, schemaUpdateDelay, TimeUnit.MILLISECONDS); } } @@ -147,17 +142,17 @@ private void doUpdateSchema() { if (!queryProviders.isEmpty()) { for (GraphQLQueryProvider provider : queryProviders) { if (provider.getQueries() != null && !provider.getQueries().isEmpty()) { - provider.getQueries().forEach(queryTypeBuilder::field); + provider.getQueries().forEach(queryTypeBuilder::field); } } } else { // graphql-java enforces Query type to be there with at least some field. queryTypeBuilder.field( - GraphQLFieldDefinition - .newFieldDefinition() - .name("_empty") - .type(Scalars.GraphQLBoolean) - .build()); + GraphQLFieldDefinition + .newFieldDefinition() + .name("_empty") + .type(Scalars.GraphQLBoolean) + .build()); } final Set types = new HashSet<>(); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java index 1b45dde2..984f5ffe 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CacheReader.java @@ -9,10 +9,7 @@ import lombok.extern.slf4j.Slf4j; @Slf4j -public final class CacheReader { - - private CacheReader() { - } +public class CacheReader { /** * Response from cache if possible, if nothing in cache will not produce any response @@ -21,26 +18,24 @@ private CacheReader() { * found or an error occurred while reading value from cache * @throws IOException if can not read value from the cache */ - public static boolean responseFromCache(GraphQLInvocationInput invocationInput, + public boolean responseFromCache(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response, GraphQLResponseCacheManager cacheManager) throws IOException { - CachedResponse cachedResponse = null; try { - cachedResponse = cacheManager.get(request, invocationInput); + CachedResponse cachedResponse = cacheManager.get(request, invocationInput); + if (cachedResponse != null) { + write(response, cachedResponse); + return true; + } } catch (Exception t) { log.warn("Ignore read from cache, unexpected error happened", t); } - if (cachedResponse != null) { - write(response, cachedResponse); - return true; - } - return false; } - private static void write(HttpServletResponse response, CachedResponse cachedResponse) + private void write(HttpServletResponse response, CachedResponse cachedResponse) throws IOException { if (cachedResponse.isError()) { response.sendError(cachedResponse.getErrorStatusCode(), cachedResponse.getErrorMessage()); diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestInvoker.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestInvoker.java index 63df95e3..8c67db18 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestInvoker.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/cache/CachingHttpRequestInvoker.java @@ -9,38 +9,38 @@ import java.io.IOException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @Slf4j +@RequiredArgsConstructor(access = AccessLevel.PROTECTED) public class CachingHttpRequestInvoker implements HttpRequestInvoker { private final GraphQLConfiguration configuration; private final HttpRequestInvoker requestInvoker; + private final CacheReader cacheReader; public CachingHttpRequestInvoker(GraphQLConfiguration configuration) { - this.configuration = configuration; - requestInvoker = new HttpRequestInvokerImpl(configuration, configuration.getGraphQLInvoker(), - new CachingQueryResponseWriterFactory()); + this(configuration, new HttpRequestInvokerImpl(configuration, configuration.getGraphQLInvoker(), + new CachingQueryResponseWriterFactory()), new CacheReader()); } + /** + * Try to return value from cache if cache exists, otherwise process the query normally + */ @Override public void execute(GraphQLInvocationInput invocationInput, HttpServletRequest request, HttpServletResponse response) { - // try to return value from cache if cache exists, otherwise processed the query - boolean returnedFromCache; - try { - returnedFromCache = !CacheReader.responseFromCache( + if (!cacheReader.responseFromCache( invocationInput, request, response, configuration.getResponseCacheManager() - ); + )) { + requestInvoker.execute(invocationInput, request, response); + } } catch (IOException e) { response.setStatus(STATUS_BAD_REQUEST); log.warn("Unexpected error happened during response from cache", e); - return; - } - - if (!returnedFromCache) { - requestInvoker.execute(invocationInput, request, response); } } diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CacheReaderTest.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CacheReaderTest.groovy new file mode 100644 index 00000000..48809d1c --- /dev/null +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CacheReaderTest.groovy @@ -0,0 +1,84 @@ +package graphql.kickstart.servlet.cache + +import graphql.kickstart.execution.input.GraphQLInvocationInput +import spock.lang.Specification + +import javax.servlet.ServletOutputStream +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +class CacheReaderTest extends Specification { + + def cacheManager + def invocationInput + def request + def response + def cacheReader + def cachedResponse + + def setup() { + cacheManager = Mock(GraphQLResponseCacheManager) + invocationInput = Mock(GraphQLInvocationInput) + request = Mock(HttpServletRequest) + response = Mock(HttpServletResponse) + cacheReader = new CacheReader() + cachedResponse = Mock(CachedResponse) + } + + def "should return false if no cached response"() { + given: + cacheManager.get(request, invocationInput) >> null + + when: + def result = cacheReader.responseFromCache(invocationInput, request, response, cacheManager) + + then: + !result + } + + def "should send error response if cached response is error"() { + given: + cachedResponse.isError() >> true + cachedResponse.getErrorStatusCode() >> 10 + cachedResponse.getErrorMessage() >> "some error" + cacheManager.get(request, invocationInput) >> cachedResponse + + when: + def result = cacheReader.responseFromCache(invocationInput, request, response, cacheManager) + + then: + result + 1 * response.sendError(10, "some error") + } + + def "should send success response if cached response is ok"() { + given: + def outputStream = Mock(ServletOutputStream) + cachedResponse.isError() >> false + cachedResponse.getContentBytes() >> [ 00, 01, 02 ] + response.getOutputStream() >> outputStream + cacheManager.get(request, invocationInput) >> cachedResponse + + when: + def result = cacheReader.responseFromCache(invocationInput, request, response, cacheManager) + + then: + result + 1 * response.setContentType("application/json;charset=UTF-8") + 1 * response.setStatus(200) + 1 * response.setCharacterEncoding("UTF-8") + 1 * response.setContentLength(3) + 1 * outputStream.write([ 00, 01, 02 ]) + } + + def "should return false if exception is thrown"() { + given: + cacheManager.get(request, invocationInput) >> {throw new RuntimeException()} + + when: + def result = cacheReader.responseFromCache(invocationInput, request, response, cacheManager) + + then: + !result + } +} 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 new file mode 100644 index 00000000..b996dd0f --- /dev/null +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy @@ -0,0 +1,79 @@ +package graphql.kickstart.servlet.cache + +import graphql.kickstart.execution.input.GraphQLInvocationInput +import graphql.kickstart.servlet.GraphQLConfiguration +import graphql.kickstart.servlet.HttpRequestInvoker +import spock.lang.Specification + +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +class CachingHttpRequestInvokerTest extends Specification { + + def cacheReaderMock + def cachingInvoker + def invocationInputMock + def requestMock + def responseMock + def responseCacheManagerMock + def httpRequestInvokerMock + def configuration + + def setup() { + cacheReaderMock = Mock(CacheReader) + invocationInputMock = Mock(GraphQLInvocationInput) + requestMock = Mock(HttpServletRequest) + responseMock = Mock(HttpServletResponse) + responseCacheManagerMock = Mock(GraphQLResponseCacheManager) + configuration = Mock(GraphQLConfiguration) + httpRequestInvokerMock = Mock(HttpRequestInvoker) + cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock) + + configuration.getResponseCacheManager() >> responseCacheManagerMock + } + + def "should execute regular invoker if cache not exists"() { + given: + cacheReaderMock.responseFromCache(invocationInputMock, requestMock, responseMock, responseCacheManagerMock) >> false + + when: + cachingInvoker.execute(invocationInputMock, requestMock, responseMock) + + then: + 1 * httpRequestInvokerMock.execute(invocationInputMock, requestMock, responseMock) + } + + def "should not execute regular invoker if cache exists"() { + given: + cacheReaderMock.responseFromCache(invocationInputMock, requestMock, responseMock, responseCacheManagerMock) >> true + + when: + cachingInvoker.execute(invocationInputMock, requestMock, responseMock) + + then: + 0 * httpRequestInvokerMock.execute(invocationInputMock, requestMock, responseMock) + } + + def "should return bad request response when ioexception"() { + given: + cacheReaderMock.responseFromCache(invocationInputMock, requestMock, responseMock, responseCacheManagerMock) >> {throw new IOException()} + + when: + cachingInvoker.execute(invocationInputMock, requestMock, responseMock) + + then: + 1 * responseMock.setStatus(400) + } + + def "should initialize completely when using single param constructor"() { + given: + def invoker = new CachingHttpRequestInvoker(configuration) + + when: + invoker.execute(invocationInputMock, requestMock, responseMock) + + then: + noExceptionThrown() + } + +}