From 33ca79ced4aa6683ad50fe35b8b13e60574d85ec Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Fri, 25 Dec 2020 11:42:32 +0100 Subject: [PATCH 1/9] Narrow require capability to component fix #276 --- .gitignore | 2 +- .../apache-karaf-feature/src/main/feature/feature.xml | 1 + .../graphql/servlet/examples/osgi/ExampleServlet.java | 11 +++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java diff --git a/.gitignore b/.gitignore index cdecd3b5..43fc6dc1 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ build/ *.iml *.ipr *.iws -.idea/ +**/.idea/ target/ /out/ .classpath 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/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java b/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java new file mode 100644 index 00000000..ade3251f --- /dev/null +++ b/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java @@ -0,0 +1,11 @@ +package graphql.servlet.examples.osgi; + +import graphql.kickstart.servlet.OsgiGraphQLHttpServlet; +import org.osgi.service.component.annotations.Component; + +@Component( + property = { "alias=/graphql", "servlet-name=Example"} +) +public class ExampleServlet extends OsgiGraphQLHttpServlet { + +} From 87118ecb6247850b8727a8c5f0ea1b999b23d14f Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Fri, 25 Dec 2020 11:42:43 +0100 Subject: [PATCH 2/9] Upgrade osgi example --- examples/osgi/apache-karaf-feature/pom.xml | 4 ++-- examples/osgi/apache-karaf-package/pom.xml | 2 +- examples/osgi/buildAndRun.sh | 8 ++++---- examples/osgi/pom.xml | 10 ++++++---- examples/osgi/providers/pom.xml | 4 +--- .../examples/osgi/ExampleGraphQLProvider.java | 12 +++++------- .../kickstart/servlet/OsgiGraphQLHttpServlet.java | 3 ++- 7 files changed, 21 insertions(+), 22 deletions(-) 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-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..e14fc7ed 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 @@ -52,7 +52,8 @@ @Component( service = {javax.servlet.http.HttpServlet.class, javax.servlet.Servlet.class}, - property = {"service.description=GraphQL HTTP Servlet"} + property = {"alias=/graphql", "service.description=GraphQL HTTP Servlet"}, + immediate = true ) @Designate(ocd = OsgiGraphQLHttpServletConfiguration.class, factory = true) public class OsgiGraphQLHttpServlet extends AbstractGraphQLHttpServlet { From 95f494a272b3353df4952342e0cccc13b2538ba2 Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 11:54:44 +0100 Subject: [PATCH 3/9] Fix returning response from cache fix #289 --- .../servlet/examples/osgi/ExampleServlet.java | 11 ---- .../servlet/OsgiGraphQLHttpServlet.java | 22 +++----- .../kickstart/servlet/cache/CacheReader.java | 9 +-- .../cache/CachingHttpRequestInvoker.java | 26 ++++----- .../CachingHttpRequestInvokerTest.groovy | 56 +++++++++++++++++++ 5 files changed, 81 insertions(+), 43 deletions(-) delete mode 100644 examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java create mode 100644 graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy diff --git a/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java b/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java deleted file mode 100644 index ade3251f..00000000 --- a/examples/osgi/providers/src/main/java/graphql/servlet/examples/osgi/ExampleServlet.java +++ /dev/null @@ -1,11 +0,0 @@ -package graphql.servlet.examples.osgi; - -import graphql.kickstart.servlet.OsgiGraphQLHttpServlet; -import org.osgi.service.component.annotations.Component; - -@Component( - property = { "alias=/graphql", "servlet-name=Example"} -) -public class ExampleServlet extends OsgiGraphQLHttpServlet { - -} 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 e14fc7ed..2d90d35f 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 @@ -52,7 +52,7 @@ @Component( service = {javax.servlet.http.HttpServlet.class, javax.servlet.Servlet.class}, - property = {"alias=/graphql", "service.description=GraphQL HTTP Servlet"}, + property = {"service.description=GraphQL HTTP Servlet"}, immediate = true ) @Designate(ocd = OsgiGraphQLHttpServletConfiguration.class, factory = true) @@ -132,12 +132,8 @@ 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); } } @@ -148,17 +144,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..74d4cad1 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,7 +18,7 @@ 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 { @@ -40,7 +37,7 @@ public static boolean responseFromCache(GraphQLInvocationInput invocationInput, 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/CachingHttpRequestInvokerTest.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy new file mode 100644 index 00000000..09061d24 --- /dev/null +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CachingHttpRequestInvokerTest.groovy @@ -0,0 +1,56 @@ +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 setup() { + cacheReaderMock = Mock(CacheReader) + invocationInputMock = Mock(GraphQLInvocationInput) + requestMock = Mock(HttpServletRequest) + responseMock = Mock(HttpServletResponse) + responseCacheManagerMock = Mock(GraphQLResponseCacheManager) + def 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) + } + +} From bef615bb41ad33400c43a59d994e7347a8f707bf Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 11:54:49 +0100 Subject: [PATCH 4/9] Fix returning response from cache fix #289 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 43fc6dc1..20acee31 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ target/ .settings bin .DS_Store +/**/out/ From 482a34a030b01e96328b3c0ce678c49034d0f2dc Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 18:43:16 +0100 Subject: [PATCH 5/9] Increase coverage --- .../kickstart/servlet/HttpRequestHandler.java | 4 +- .../servlet/OsgiGraphQLHttpServlet.java | 3 +- .../kickstart/servlet/cache/CacheReader.java | 12 ++- .../servlet/cache/CacheReaderTest.groovy | 73 +++++++++++++++++++ .../CachingHttpRequestInvokerTest.groovy | 25 ++++++- 5 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CacheReaderTest.groovy diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java index 902604e4..c70117b4 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java @@ -6,8 +6,8 @@ public interface HttpRequestHandler { - String APPLICATION_JSON_UTF8 = "application/json;charset=UTF-8"; - String APPLICATION_EVENT_STREAM_UTF8 = "text/event-stream;charset=UTF-8"; + String APPLICATION_JSON_UTF8 = "application/json"; + String APPLICATION_EVENT_STREAM_UTF8 = "text/event-stream"; int STATUS_OK = 200; int STATUS_BAD_REQUEST = 400; 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 2d90d35f..48ccf462 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 @@ -52,8 +52,7 @@ @Component( service = {javax.servlet.http.HttpServlet.class, javax.servlet.Servlet.class}, - property = {"service.description=GraphQL HTTP Servlet"}, - immediate = true + property = {"service.description=GraphQL HTTP Servlet"} ) @Designate(ocd = OsgiGraphQLHttpServletConfiguration.class, factory = true) public class OsgiGraphQLHttpServlet extends AbstractGraphQLHttpServlet { 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 74d4cad1..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 @@ -22,18 +22,16 @@ 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; } 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..dbf03939 --- /dev/null +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/cache/CacheReaderTest.groovy @@ -0,0 +1,73 @@ +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") + 1 * response.setStatus(200) + 1 * response.setCharacterEncoding("UTF-8") + 1 * response.setContentLength(3) + 1 * outputStream.write([ 00, 01, 02 ]) + } +} 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 09061d24..b996dd0f 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 @@ -17,6 +17,7 @@ class CachingHttpRequestInvokerTest extends Specification { def responseMock def responseCacheManagerMock def httpRequestInvokerMock + def configuration def setup() { cacheReaderMock = Mock(CacheReader) @@ -24,7 +25,7 @@ class CachingHttpRequestInvokerTest extends Specification { requestMock = Mock(HttpServletRequest) responseMock = Mock(HttpServletResponse) responseCacheManagerMock = Mock(GraphQLResponseCacheManager) - def configuration = Mock(GraphQLConfiguration) + configuration = Mock(GraphQLConfiguration) httpRequestInvokerMock = Mock(HttpRequestInvoker) cachingInvoker = new CachingHttpRequestInvoker(configuration, httpRequestInvokerMock, cacheReaderMock) @@ -53,4 +54,26 @@ class CachingHttpRequestInvokerTest extends Specification { 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() + } + } From 385f3af80c0eabb6a9fddd9206d1cee6a7bbbd67 Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 18:47:00 +0100 Subject: [PATCH 6/9] Increase coverage --- .github/workflows/pull-request.yml | 5 +---- .../kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index 560e73da..f4d669a1 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: @@ -19,7 +16,7 @@ jobs: matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] java: [ 8, 11, 15 ] - needs: validation + needs: validationAbstractGraphQLHttpServletSpec runs-on: ${{ matrix.os }} steps: - name: Checkout diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy index 36430285..ca4c3a27 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy @@ -26,8 +26,8 @@ class AbstractGraphQLHttpServletSpec extends Specification { public static final int STATUS_OK = 200 public static final int STATUS_BAD_REQUEST = 400 public static final int STATUS_ERROR = 500 - public static final String CONTENT_TYPE_JSON_UTF8 = 'application/json;charset=UTF-8' - public static final String CONTENT_TYPE_SERVER_SENT_EVENTS = 'text/event-stream;charset=UTF-8' + public static final String CONTENT_TYPE_JSON_UTF8 = 'application/json' + public static final String CONTENT_TYPE_SERVER_SENT_EVENTS = 'text/event-stream' @Shared ObjectMapper mapper = new ObjectMapper() From f5e67e9b7dd07622fc12772d220e7131d29f5588 Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 18:49:10 +0100 Subject: [PATCH 7/9] Increase coverage --- .github/workflows/pull-request.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index f4d669a1..62609d9f 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -16,7 +16,7 @@ jobs: matrix: os: [ ubuntu-latest, macos-latest, windows-latest ] java: [ 8, 11, 15 ] - needs: validationAbstractGraphQLHttpServletSpec + needs: validation runs-on: ${{ matrix.os }} steps: - name: Checkout From b059c1082e9854d2c8264131793e13981bb3b7d0 Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 18:52:44 +0100 Subject: [PATCH 8/9] Reinstate utf8 --- .../java/graphql/kickstart/servlet/HttpRequestHandler.java | 4 ++-- .../kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy | 4 ++-- .../graphql/kickstart/servlet/cache/CacheReaderTest.groovy | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java index c70117b4..902604e4 100644 --- a/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java +++ b/graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestHandler.java @@ -6,8 +6,8 @@ public interface HttpRequestHandler { - String APPLICATION_JSON_UTF8 = "application/json"; - String APPLICATION_EVENT_STREAM_UTF8 = "text/event-stream"; + String APPLICATION_JSON_UTF8 = "application/json;charset=UTF-8"; + String APPLICATION_EVENT_STREAM_UTF8 = "text/event-stream;charset=UTF-8"; int STATUS_OK = 200; int STATUS_BAD_REQUEST = 400; diff --git a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy index ca4c3a27..36430285 100644 --- a/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy +++ b/graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/AbstractGraphQLHttpServletSpec.groovy @@ -26,8 +26,8 @@ class AbstractGraphQLHttpServletSpec extends Specification { public static final int STATUS_OK = 200 public static final int STATUS_BAD_REQUEST = 400 public static final int STATUS_ERROR = 500 - public static final String CONTENT_TYPE_JSON_UTF8 = 'application/json' - public static final String CONTENT_TYPE_SERVER_SENT_EVENTS = 'text/event-stream' + public static final String CONTENT_TYPE_JSON_UTF8 = 'application/json;charset=UTF-8' + public static final String CONTENT_TYPE_SERVER_SENT_EVENTS = 'text/event-stream;charset=UTF-8' @Shared ObjectMapper mapper = new ObjectMapper() 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 index dbf03939..db59abbc 100644 --- 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 @@ -64,7 +64,7 @@ class CacheReaderTest extends Specification { then: result - 1 * response.setContentType("application/json") + 1 * response.setContentType("application/json;charset=UTF-8") 1 * response.setStatus(200) 1 * response.setCharacterEncoding("UTF-8") 1 * response.setContentLength(3) From 5a226d03276e4217ab07348ba11c5a16a145f82d Mon Sep 17 00:00:00 2001 From: Michiel Oliemans Date: Sat, 26 Dec 2020 18:58:59 +0100 Subject: [PATCH 9/9] Increase coverage --- .../kickstart/servlet/OsgiGraphQLHttpServlet.java | 3 +-- .../kickstart/servlet/cache/CacheReaderTest.groovy | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) 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 48ccf462..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,8 +131,7 @@ protected void updateSchema() { updateFuture.cancel(true); } - updateFuture = executor - .schedule(this::doUpdateSchema, schemaUpdateDelay, TimeUnit.MILLISECONDS); + updateFuture = executor.schedule(this::doUpdateSchema, schemaUpdateDelay, TimeUnit.MILLISECONDS); } } 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 index db59abbc..48809d1c 100644 --- 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 @@ -70,4 +70,15 @@ class CacheReaderTest extends Specification { 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 + } }