Merge pull request #497 from graphql-java-kickstart/sonar-fixes

Sonar fixes

Author: oliemansm

.github/workflows/pull-request.yml

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

example-spring-common/src/main/java/graphql/kickstart/spring/web/boot/sample/SimpleListConnection.java

@@ -3,53 +3,46 @@
 import com.oembedler.moon.graphql.engine.relay.ConnectionObjectType;
 import com.oembedler.moon.graphql.engine.relay.EdgeObjectType;
 import com.oembedler.moon.graphql.engine.relay.PageInfoObjectType;
-import graphql.relay.ConnectionCursor;
-import graphql.relay.DefaultConnectionCursor;
 import graphql.schema.DataFetchingEnvironment;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
+import lombok.AccessLevel;
+import lombok.RequiredArgsConstructor;
 
 /**
  * @author <a href="mailto:[email protected]">oEmbedler Inc.</a>
  */
-public class SimpleListConnection {
+@RequiredArgsConstructor(access = AccessLevel.PROTECTED)
+public abstract class SimpleListConnection<T> {
 
   private static final String DUMMY_CURSOR_PREFIX = "simple-cursor";
-  private final List<?> data;
+  private final List<T> data;
 
-  public SimpleListConnection(List<?> data) {
-    this.data = data;
-  }
-
-  public <E extends EdgeObjectType> E createEdgeObject() {
-    return (E) new EdgeObjectType();
-  }
+  public abstract <E extends EdgeObjectType<T>> E createEdgeObject();
 
-  public <E extends ConnectionObjectType> E createConnectionObject() {
-    return (E) new ConnectionObjectType();
-  }
+  public abstract <C extends ConnectionObjectType<? extends EdgeObjectType<T>, ? extends PageInfoObjectType>> C createConnectionObject();
 
-  private List<EdgeObjectType> buildEdges() {
-    List<EdgeObjectType> edges = new ArrayList<>();
+  private List<EdgeObjectType<T>> buildEdges() {
+    List<EdgeObjectType<T>> edges = new ArrayList<>();
     int ix = 0;
-    for (Object object : data) {
-      EdgeObjectType edge = createEdgeObject();
+    for (T object : data) {
+      EdgeObjectType<T> edge = createEdgeObject();
       edge.setNode(object);
       edge.setCursor(createCursor(ix++));
       edges.add(edge);
     }
     return edges;
   }
 
-  public <C extends ConnectionObjectType> C get(DataFetchingEnvironment environment) {
+  public <C extends ConnectionObjectType<? extends EdgeObjectType<T>, ? extends PageInfoObjectType>> C get(
+      DataFetchingEnvironment environment) {
+    List<EdgeObjectType<T>> edges = buildEdges();
 
-    List<EdgeObjectType> edges = buildEdges();
-
-    int afterOffset = getOffsetFromCursor(environment.<String>getArgument("after"), -1);
+    int afterOffset = getOffsetFromCursor(environment.getArgument("after"), -1);
     int begin = Math.max(afterOffset, -1) + 1;
-    int beforeOffset = getOffsetFromCursor(environment.<String>getArgument("before"), edges.size());
+    int beforeOffset = getOffsetFromCursor(environment.getArgument("before"), edges.size());
     int end = Math.min(beforeOffset, edges.size());
 
     edges = edges.subList(begin, end);
@@ -74,34 +67,30 @@ public <C extends ConnectionObjectType> C get(DataFetchingEnvironment environmen
       return emptyConnection();
     }
 
-    EdgeObjectType firstEdge = edges.get(0);
-    EdgeObjectType lastEdge = edges.get(edges.size() - 1);
+    EdgeObjectType<T> firstEdge = edges.get(0);
+    EdgeObjectType<T> lastEdge = edges.get(edges.size() - 1);
 
     PageInfoObjectType pageInfo = new PageInfoObjectType();
     pageInfo.setStartCursor(firstEdge.getCursor());
     pageInfo.setEndCursor(lastEdge.getCursor());
     pageInfo.setHasPreviousPage(!firstEdge.getCursor().equals(firstPresliceCursor));
     pageInfo.setHasNextPage(!lastEdge.getCursor().equals(lastPresliceCursor));
 
-    ConnectionObjectType connection = createConnectionObject();
+    ConnectionObjectType<EdgeObjectType<T>, PageInfoObjectType> connection = createConnectionObject();
     connection.setEdges(edges);
     connection.setPageInfo(pageInfo);
 
+    //noinspection unchecked
     return (C) connection;
   }
 
-  private <E extends ConnectionObjectType> E emptyConnection() {
-    ConnectionObjectType connection = createConnectionObject();
+  private <E extends ConnectionObjectType<? extends EdgeObjectType<T>, ? extends PageInfoObjectType>> E emptyConnection() {
+    ConnectionObjectType<EdgeObjectType<T>, PageInfoObjectType> connection = createConnectionObject();
     connection.setPageInfo(new PageInfoObjectType());
+    //noinspection unchecked
     return (E) connection;
   }
 
-  public ConnectionCursor cursorForObjectInConnection(Object object) {
-    int index = data.indexOf(object);
-    String cursor = createCursor(index);
-    return new DefaultConnectionCursor(cursor);
-  }
-
   private int getOffsetFromCursor(String cursor, int defaultValue) {
     if (cursor == null) {
       return defaultValue;

example-spring-common/src/main/java/graphql/kickstart/spring/web/boot/sample/TodoSimpleListConnection.java

@@ -1,27 +1,28 @@
 package graphql.kickstart.spring.web.boot.sample;
 
-import com.oembedler.moon.graphql.engine.relay.ConnectionObjectType;
-import com.oembedler.moon.graphql.engine.relay.EdgeObjectType;
 import graphql.kickstart.spring.web.boot.sample.schema.objecttype.TodoObjectType;
+import graphql.kickstart.spring.web.boot.sample.schema.objecttype.TodoObjectType.TodoConnectionObjectType;
+import graphql.kickstart.spring.web.boot.sample.schema.objecttype.TodoObjectType.TodoEdgeObjectType;
 import java.util.List;
 
 /**
  * @author <a href="mailto:[email protected]">oEmbedler Inc.</a>
  */
-public class TodoSimpleListConnection extends SimpleListConnection {
+@SuppressWarnings("unchecked")
+public class TodoSimpleListConnection extends SimpleListConnection<TodoObjectType> {
 
-  public TodoSimpleListConnection(List<?> data) {
+  public TodoSimpleListConnection(List<TodoObjectType> data) {
     super(data);
   }
 
   @Override
-  public <T extends EdgeObjectType> T createEdgeObject() {
-    return (T) new TodoObjectType.TodoEdgeObjectType();
+  public TodoEdgeObjectType createEdgeObject() {
+    return new TodoObjectType.TodoEdgeObjectType();
   }
 
   @Override
-  public <T extends ConnectionObjectType> T createConnectionObject() {
-    return (T) new TodoObjectType.TodoConnectionObjectType();
+  public TodoConnectionObjectType createConnectionObject() {
+    return new TodoObjectType.TodoConnectionObjectType();
   }
 
 }

graphql-kickstart-spring-support/src/main/java/graphql/kickstart/spring/error/ReflectiveGraphQLErrorFactory.java

@@ -36,10 +36,10 @@ public Optional<Class<? extends Throwable>> mostConcrete(Throwable t) {
   @Override
   public Collection<GraphQLError> create(Throwable t, ErrorContext errorContext) {
     try {
-      method.setAccessible(true);
       if (singularReturnType) {
         return singletonList((GraphQLError) invoke(t, errorContext));
       }
+      //noinspection unchecked
       return (Collection<GraphQLError>) invoke(t, errorContext);
     } catch (IllegalAccessException | InvocationTargetException e) {
       log.error("Cannot create GraphQLError from throwable {}", t.getClass().getSimpleName(), e);

graphql-spring-boot-autoconfigure/src/test/java/graphql/kickstart/spring/web/boot/GraphQLErrorHandlerTest.java

@@ -43,8 +43,8 @@ void illegalArgumentExceptionShouldBeHandledConcretely() {
     TestUtils.assertGraphQLError(
         gql,
         "query { illegalArgumentException }",
-        new ThrowableGraphQLError(new IllegalArgumentException("Illegal argument"),
-            "Illegal argument"),
+        new ThrowableGraphQLError(new IllegalArgumentException("Some argument"),
+            "Custom illegal argument"),
         objectMapper
     );
   }
@@ -70,12 +70,12 @@ boolean illegalStateException() {
       }
 
       @ExceptionHandler(IllegalArgumentException.class)
-      ThrowableGraphQLError handle(IllegalArgumentException e) {
-        return new ThrowableGraphQLError(e, "Illegal argument");
+      public ThrowableGraphQLError handle(IllegalArgumentException e) {
+        return new ThrowableGraphQLError(e, "Custom illegal argument");
       }
 
       @ExceptionHandler(Throwable.class)
-      GraphQLError handle(Throwable e) {
+      public GraphQLError handle(Throwable e) {
         return new ThrowableGraphQLError(e, "Catch all handler");
       }
 

graphql-spring-boot-test/build.gradle

@@ -21,6 +21,7 @@ dependencies {
     implementation("org.springframework.boot:spring-boot-starter-test")
     implementation("com.fasterxml.jackson.core:jackson-databind")
     implementation("com.jayway.jsonpath:json-path")
+    implementation 'org.awaitility:awaitility:4.0.3'
     compileOnly("com.graphql-java:graphql-java:$LIB_GRAPHQL_JAVA_VER")
     compileOnly("com.graphql-java-kickstart:graphql-java-servlet:$LIB_GRAPHQL_SERVLET_VER")
     testImplementation("org.springframework.boot:spring-boot-starter-web")

graphql-spring-boot-test/src/main/java/com/graphql/spring/boot/test/GraphQLTestSubscription.java

@@ -1,6 +1,7 @@
 package com.graphql.spring.boot.test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -18,6 +19,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Queue;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 import javax.websocket.ClientEndpointConfig;
@@ -49,7 +51,6 @@ public class GraphQLTestSubscription {
 
   private static final WebSocketContainer WEB_SOCKET_CONTAINER = ContainerProvider
       .getWebSocketContainer();
-  private static final int SLEEP_INTERVAL_MS = 100;
   private static final int ACKNOWLEDGEMENT_AND_CONNECTION_TIMEOUT = 60000;
   private static final AtomicInteger ID_COUNTER = new AtomicInteger(1);
   private static final UriBuilderFactory URI_BUILDER_FACTORY = new DefaultUriBuilderFactory();
@@ -115,7 +116,7 @@ public GraphQLTestSubscription init(@Nullable final Object payload) {
     sendMessage(message);
     state.setInitialized(true);
     awaitAcknowledgement();
-    log.debug("Subscription successfully initialized.");
+    log.debug("Subscription successfully initialized");
     return this;
   }
 
@@ -134,12 +135,12 @@ public GraphQLTestSubscription start(@NonNull final String graphQLResource) {
   /**
    * Sends the "start" message to the GraphQL Subscription.
    *
-   * @param graphGLResource the GraphQL resource, which contains the query for the subscription
+   * @param graphQLResource the GraphQL resource, which contains the query for the subscription
    * start payload.
    * @param variables the variables needed for the query to be evaluated.
    * @return self reference
    */
-  public GraphQLTestSubscription start(@NonNull final String graphGLResource,
+  public GraphQLTestSubscription start(@NonNull final String graphQLResource,
       @Nullable final Object variables) {
     if (!isInitialized()) {
       init();
@@ -149,7 +150,7 @@ public GraphQLTestSubscription start(@NonNull final String graphGLResource,
     }
     state.setStarted(true);
     ObjectNode payload = objectMapper.createObjectNode();
-    payload.put("query", loadQuery(graphGLResource));
+    payload.put("query", loadQuery(graphQLResource));
     payload.set("variables", getFinalPayload(variables));
     ObjectNode message = objectMapper.createObjectNode();
     message.put("type", "start");
@@ -298,18 +299,11 @@ public List<GraphQLResponse> awaitAndGetNextResponses(
     if (isStopped()) {
       fail("Subscription already stopped. Forgot to call reset after test case?");
     }
-    int elapsedTime = 0;
-    while (
-        ((state.getResponses().size() < numExpectedResponses) || numExpectedResponses <= 0)
-            && elapsedTime < timeout
-    ) {
-      try {
-        Thread.sleep(SLEEP_INTERVAL_MS);
-        elapsedTime += SLEEP_INTERVAL_MS;
-      } catch (InterruptedException e) {
-        fail("Test execution error - Thread.sleep failed.", e);
-      }
-    }
+
+    await()
+        .atMost(timeout, TimeUnit.MILLISECONDS)
+        .until(() -> state.getResponses().size() >= numExpectedResponses);
+
     if (stopAfter) {
       stop();
     }
@@ -420,29 +414,21 @@ private void sendMessage(final Object message) {
   }
 
   private void awaitAcknowledgement() {
-    await(GraphQLTestSubscription::isAcknowledged,
-        "Connection was not acknowledged by the GraphQL server.");
+    awaitAcknowledgementOrConnection(GraphQLTestSubscription::isAcknowledged,
+        "Connection was acknowledged by the GraphQL server.");
   }
 
   private void awaitStop() {
-    await(GraphQLTestSubscription::isStopped, "Connection was not stopped in time.");
+    awaitAcknowledgementOrConnection(GraphQLTestSubscription::isStopped,
+        "Connection was stopped in time.");
   }
 
-  private void await(final Predicate<GraphQLTestSubscription> condition,
+  private void awaitAcknowledgementOrConnection(final Predicate<GraphQLTestSubscription> condition,
       final String timeoutDescription) {
-    int elapsedTime = 0;
-    while (!condition.test(this) && elapsedTime < ACKNOWLEDGEMENT_AND_CONNECTION_TIMEOUT) {
-      try {
-        Thread.sleep(SLEEP_INTERVAL_MS);
-        elapsedTime += SLEEP_INTERVAL_MS;
-      } catch (InterruptedException e) {
-        fail("Test execution error - Thread.sleep failed.", e);
-      }
-    }
+    await(timeoutDescription)
+        .atMost(ACKNOWLEDGEMENT_AND_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS)
+        .until(() -> condition.test(this));
 
-    if (!condition.test(this)) {
-      fail("Timeout: " + timeoutDescription);
-    }
   }
 
   @RequiredArgsConstructor
@@ -460,28 +446,35 @@ public void onMessage(final String message) {
         assertThat(typeNode).as("GraphQL messages should have a type field.").isNotNull();
         assertThat(typeNode.isNull()).as("GraphQL messages type should not be null.").isFalse();
         final String type = typeNode.asText();
-        if (type.equals("complete")) {
-          state.setCompleted(true);
-          log.debug("Subscription completed.");
-        } else if (type.equals("connection_ack")) {
-          state.setAcknowledged(true);
-          log.debug("WebSocket connection acknowledged by the GraphQL Server.");
-        } else if (type.equals("data") || type.equals("error")) {
-          final JsonNode payload = jsonNode.get(PAYLOAD);
-          assertThat(payload).as("Data/error messages must have a payload.").isNotNull();
-          final String payloadString = objectMapper.writeValueAsString(payload);
-          final GraphQLResponse graphQLResponse = new GraphQLResponse(
-              ResponseEntity.ok(payloadString),
-              objectMapper);
-          if (state.isStopped() || state.isCompleted()) {
-            log.debug(
-                "Response discarded because subscription was stopped or completed in the meanwhile.");
-          } else {
-            synchronized (STATE_LOCK) {
-              state.getResponses().add(graphQLResponse);
+        switch (type) {
+          case "complete":
+            state.setCompleted(true);
+            log.debug("Subscription completed.");
+            break;
+          case "connection_ack":
+            state.setAcknowledged(true);
+            log.debug("WebSocket connection acknowledged by the GraphQL Server.");
+            break;
+          case "data":
+          case "error":
+            final JsonNode payload = jsonNode.get(PAYLOAD);
+            assertThat(payload).as("Data/error messages must have a payload.").isNotNull();
+            final String payloadString = objectMapper.writeValueAsString(payload);
+            final GraphQLResponse graphQLResponse = new GraphQLResponse(
+                ResponseEntity.ok(payloadString),
+                objectMapper);
+            if (state.isStopped() || state.isCompleted()) {
+              log.debug(
+                  "Response discarded because subscription was stopped or completed in the meanwhile.");
+            } else {
+              synchronized (STATE_LOCK) {
+                state.getResponses().add(graphQLResponse);
+              }
+              log.debug("New response recorded.");
             }
-            log.debug("New response recorded.");
-          }
+            break;
+          default:
+            break;
         }
       } catch (JsonProcessingException e) {
         fail("Exception while parsing server response. Response is not a valid GraphQL response.",