Bugfix/response from cache

.github/workflows/pull-request.yml

@@ -1,8 +1,5 @@
 name: "Pull request"
 on:
-  push:
-    branches-ignore:
-      - master
   pull_request:
     types: [opened, synchronize, reopened]
 jobs:

.gitignore

@@ -3,11 +3,12 @@ build/
 *.iml
 *.ipr
 *.iws
-.idea/
+**/.idea/
 target/
 /out/
 .classpath
 .project
 .settings
 bin
 .DS_Store
+/**/out/

examples/osgi/apache-karaf-feature/pom.xml

@@ -14,7 +14,7 @@
         </configuration>
         <extensions>true</extensions>
         <groupId>org.apache.karaf.tooling</groupId>
-        <version>4.0.8</version>
+        <version>4.2.10</version>
       </plugin>
 
       <plugin>
@@ -108,7 +108,7 @@
   <parent>
     <artifactId>graphql-java-servlet-osgi-examples</artifactId>
     <groupId>com.graphql-java-kickstart</groupId>
-    <version>7.3.4-SNAPSHOT</version>
+    <version>10.1.0</version>
   </parent>
 
   <properties>

examples/osgi/apache-karaf-feature/src/main/feature/feature.xml

@@ -5,5 +5,6 @@
     name="graphql-java-servlet-osgi-examples-karaf-feature">
     <feature dependency="false" prerequisite="true">scr</feature>
     <feature dependency="false" prerequisite="true">war</feature>
+    <feature dependency="false" prerequisite="true">http</feature>
   </feature>
 </features>

examples/osgi/apache-karaf-package/pom.xml

@@ -110,6 +110,6 @@
   <parent>
     <artifactId>graphql-java-servlet-osgi-examples</artifactId>
     <groupId>com.graphql-java-kickstart</groupId>
-    <version>7.3.4-SNAPSHOT</version>
+    <version>10.1.0</version>
   </parent>
 </project>

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

examples/osgi/pom.xml

@@ -14,11 +14,13 @@
   <packaging>pom</packaging>
 
   <properties>
-    <graphql.java.servlet.version>7.3.4-SNAPSHOT</graphql.java.servlet.version>
-    <graphql.java.version>11.0</graphql.java.version>
-    <karaf.version>4.2.4</karaf.version>
+    <graphql.java.servlet.version>11.0.0-SNAPSHOT</graphql.java.servlet.version>
+    <graphql.java.version>16.1</graphql.java.version>
+    <karaf.version>4.2.10</karaf.version>
+    <maven.compiler.source>1.8</maven.compiler.source>
+    <maven.compiler.target>1.8</maven.compiler.target>
   </properties>
 
-  <version>7.3.4-SNAPSHOT</version>
+  <version>10.1.0</version>
 
 </project>

examples/osgi/providers/pom.xml

@@ -8,8 +8,6 @@
       <plugin>
         <artifactId>maven-bundle-plugin</artifactId>
         <configuration>
-          <instructions>
-          </instructions>
         </configuration>
         <extensions>true</extensions>
         <groupId>org.apache.felix</groupId>
@@ -51,7 +49,7 @@
   <parent>
     <artifactId>graphql-java-servlet-osgi-examples</artifactId>
     <groupId>com.graphql-java-kickstart</groupId>
-    <version>7.3.4-SNAPSHOT</version>
+    <version>10.1.0</version>
   </parent>
 
 </project>

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<GraphQLFieldDefinition> getMutations() {
   }
 
   public Collection<GraphQLType> getTypes() {
-
-    List<GraphQLType> types = new ArrayList<GraphQLType>();
-    return types;
+    return new ArrayList<GraphQLType>();
   }
 }

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<GraphQLType> types = new HashSet<>();

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());

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);
     }
   }
 

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
+    }
+}