Merge pull request #299 from BlasiusSecundus/feature/transactional-query-invoker

feat: added option to use a transactional query invoker

Author: oliemansm

README.md

@@ -173,6 +173,10 @@ graphql:
            # if you want to @ExceptionHandler annotation for custom GraphQLErrors
            exception-handlers-enabled: true
            contextSetting: PER_REQUEST_WITH_INSTRUMENTATION
+      query-invoker:
+           # use a transactional query invoker; useful when working with JPA entities across multiple resolvers to 
+           # prevent LazyInitializationException; false by default
+           transactional: true
 ```
 
 By default a global CORS filter is enabled for `/graphql/**` context.

gradle.properties

@@ -44,6 +44,8 @@ LIB_SPRING_BOOT_VER = 2.1.6.RELEASE
 LIB_GRAPHQL_SERVLET_VER = 8.0.0
 LIB_GRAPHQL_JAVA_TOOLS_VER = 5.7.1
 LIB_COMMONS_IO_VER = 2.6
+LIB_TRANSACTIONS_API_VERSION=1.3
+LIB_INTERCEPTOR_API_VERSION=1.2.2
 kotlin.version=1.3.31
 
 GRADLE_WRAPPER_VER = 4.10.3

graphql-spring-boot-autoconfigure/build.gradle

@@ -29,7 +29,10 @@ dependencies {
     compile "commons-io:commons-io:$LIB_COMMONS_IO_VER"
 
     compileOnly "org.springframework.boot:spring-boot-starter-web:$LIB_SPRING_BOOT_VER"
+    compileOnly "javax.transaction:javax.transaction-api:$LIB_TRANSACTIONS_API_VERSION"
+    compileOnly "javax.interceptor:javax.interceptor-api:$LIB_INTERCEPTOR_API_VERSION"
 
+    testCompile "javax.transaction:javax.transaction-api:$LIB_TRANSACTIONS_API_VERSION"
     testCompile "com.graphql-java:graphql-java:$LIB_GRAPHQL_JAVA_VER"
     testCompile "org.springframework.boot:spring-boot-starter-web:$LIB_SPRING_BOOT_VER"
     testCompile "org.springframework.boot:spring-boot-starter-test:$LIB_SPRING_BOOT_VER"

graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLQueryInvokerProperties.java

@@ -0,0 +1,13 @@
+package com.oembedler.moon.graphql.boot;
+
+import lombok.Data;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.annotation.Configuration;
+
+@Data
+@Configuration
+@ConfigurationProperties(prefix = "graphql.query-invoker")
+public class GraphQLQueryInvokerProperties {
+
+    private boolean transactional;
+}

graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLWebAutoConfiguration.java

@@ -86,7 +86,7 @@
 @Conditional(OnSchemaOrSchemaProviderBean.class)
 @ConditionalOnProperty(value = "graphql.servlet.enabled", havingValue = "true", matchIfMissing = true)
 @AutoConfigureAfter({GraphQLJavaToolsAutoConfiguration.class, JacksonAutoConfiguration.class})
-@EnableConfigurationProperties({GraphQLServletProperties.class})
+@EnableConfigurationProperties({GraphQLServletProperties.class, GraphQLQueryInvokerProperties.class})
 public class GraphQLWebAutoConfiguration {
 
 
@@ -213,7 +213,10 @@ public GraphQLInvocationInputFactory invocationInputFactory(GraphQLSchemaProvide
 
     @Bean
     @ConditionalOnMissingBean
-    public GraphQLQueryInvoker queryInvoker(ExecutionStrategyProvider executionStrategyProvider) {
+    public GraphQLQueryInvoker queryInvoker(
+            ExecutionStrategyProvider executionStrategyProvider,
+            GraphQLQueryInvokerProperties graphQLQueryInvokerProperties
+    ) {
         GraphQLQueryInvoker.Builder builder = GraphQLQueryInvoker.newBuilder()
                 .withExecutionStrategyProvider(executionStrategyProvider);
 
@@ -228,7 +231,13 @@ public GraphQLQueryInvoker queryInvoker(ExecutionStrategyProvider executionStrat
             builder.withPreparsedDocumentProvider(preparsedDocumentProvider);
         }
 
-        return builder.build();
+        GraphQLQueryInvoker queryInvoker = builder.build();
+
+        if (graphQLQueryInvokerProperties.isTransactional()) {
+            queryInvoker = new TransactionalGraphQLQueryInvokerWrapper(queryInvoker);
+            log.info("Using transactional query invoker.");
+        }
+        return queryInvoker;
     }
 
     @Bean

graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/TransactionalGraphQLQueryInvokerWrapper.java

@@ -0,0 +1,58 @@
+package com.oembedler.moon.graphql.boot;
+
+import graphql.ExecutionResult;
+import graphql.servlet.context.ContextSetting;
+import graphql.servlet.core.GraphQLQueryInvoker;
+import graphql.servlet.input.GraphQLSingleInvocationInput;
+import lombok.Getter;
+import org.springframework.lang.NonNull;
+
+import javax.transaction.Transactional;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * This is a transactional wrapper for the default {@link GraphQLQueryInvoker} bean. The primary purpose of this class
+ * is to prevent LazyInitializationException when working with nested
+ * {@link com.coxautodev.graphql.tools.GraphQLResolver resolvers} and JPA entities. In these cases making the
+ * individual resolvers {@link Transactional} may not be sufficient.
+ *
+ * Other than making the whole query invocation transactional, this wrapper does not change the behaviour of the
+ * wrapped invoker, and will simply delegate all queries to it.
+ *
+ * To enable the transactional wrapper, set the {@code graphql.query-invoker.transactional} property to {@code true}
+ * in application.properties/yaml.
+ */
+@Getter
+@Transactional
+public class TransactionalGraphQLQueryInvokerWrapper extends GraphQLQueryInvoker {
+    //GraphQLQueryInvoker should be an interface...
+
+    private @NonNull GraphQLQueryInvoker wrappedInvoker;
+
+    /**
+     * Constructor.
+     * @param wrappedInvoker The wrapped query invoker. Must not be null.
+     */
+    public TransactionalGraphQLQueryInvokerWrapper(final @NonNull GraphQLQueryInvoker wrappedInvoker) {
+        super(null, null, null, null);
+        this.wrappedInvoker = Objects.requireNonNull(wrappedInvoker);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public ExecutionResult query(final GraphQLSingleInvocationInput singleInvocationInput) {
+        return wrappedInvoker.query(singleInvocationInput);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public List<ExecutionResult> query(final List<GraphQLSingleInvocationInput> batchedInvocationInput,
+            final ContextSetting contextSetting) {
+        return wrappedInvoker.query(batchedInvocationInput, contextSetting);
+    }
+}

graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/TransactionQueryInvokerWrapperTest.java

@@ -0,0 +1,74 @@
+package com.oembedler.moon.graphql.boot;
+
+import graphql.ExecutionResult;
+import graphql.servlet.context.ContextSetting;
+import graphql.servlet.core.GraphQLQueryInvoker;
+import graphql.servlet.input.GraphQLSingleInvocationInput;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import javax.transaction.Transactional;
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.BDDMockito.then;
+import static org.mockito.Mockito.mock;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TransactionQueryInvokerWrapperTest {
+
+    @Mock
+    private GraphQLQueryInvoker wrappedInvoker;
+
+    private TransactionalGraphQLQueryInvokerWrapper wrapper;
+
+    @Before
+    public void setUp() {
+        wrapper = new TransactionalGraphQLQueryInvokerWrapper(wrappedInvoker);
+    }
+
+    @Test
+    public void shouldHaveTransactionalAnnotation() {
+        assertThat(TransactionalGraphQLQueryInvokerWrapper.class.getAnnotation(Transactional.class)).isNotNull();
+    }
+
+    @Test
+    public void shouldWrapExistingQueryInvokerWithSingleQuery() {
+        //GIVEN
+        final GraphQLSingleInvocationInput invocationInput = mock(GraphQLSingleInvocationInput.class);
+        final ExecutionResult expectedExecutionResult = mock(ExecutionResult.class);
+        given(wrappedInvoker.query(invocationInput)).willReturn(expectedExecutionResult);
+        //WHEN
+        final ExecutionResult actualExecutionResult = wrapper.query(invocationInput);
+        //THEN
+        then(wrappedInvoker).should().query(invocationInput);
+        then(wrappedInvoker).shouldHaveNoMoreInteractions();
+        assertThat(actualExecutionResult)
+                .as("Should call the wrapped invoker, and return the execution result returned by it.")
+                .isEqualTo(expectedExecutionResult);
+    }
+
+    @Test
+    public void shouldWrapExistingQueryInvokerWithBatchedQuery() {
+        //GIVEN
+        final GraphQLSingleInvocationInput invocationInput = mock(GraphQLSingleInvocationInput.class);
+        final List<GraphQLSingleInvocationInput> invocationInputList = Collections.singletonList(invocationInput);
+        final ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION;
+        final ExecutionResult expectedExecutionResult = mock(ExecutionResult.class);
+        final List<ExecutionResult> expectedExecutionResultList = Collections.singletonList(expectedExecutionResult);
+        given(wrappedInvoker.query(invocationInputList, contextSetting)).willReturn(expectedExecutionResultList);
+        //WHEN
+        final List<ExecutionResult> actualExecutionResultList = wrapper.query(invocationInputList, contextSetting);
+        //THEN
+        then(wrappedInvoker).should().query(invocationInputList, contextSetting);
+        then(wrappedInvoker).shouldHaveNoMoreInteractions();
+        assertThat(actualExecutionResultList)
+                .as("Should call the wrapped invoker, and return the execution result list returned by it.")
+                .isEqualTo(expectedExecutionResultList);
+    }
+}

graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/test/web/GraphQLWebAutoConfigurationTest.java

@@ -1,6 +1,7 @@
 package com.oembedler.moon.graphql.boot.test.web;
 
 import com.oembedler.moon.graphql.boot.GraphQLWebAutoConfiguration;
+import com.oembedler.moon.graphql.boot.TransactionalGraphQLQueryInvokerWrapper;
 import com.oembedler.moon.graphql.boot.test.AbstractAutoConfigurationTest;
 import graphql.analysis.MaxQueryComplexityInstrumentation;
 import graphql.analysis.MaxQueryDepthInstrumentation;
@@ -12,12 +13,15 @@
 import graphql.servlet.AbstractGraphQLHttpServlet;
 import graphql.servlet.config.DefaultGraphQLSchemaProvider;
 import graphql.servlet.config.GraphQLSchemaProvider;
+import graphql.servlet.core.GraphQLQueryInvoker;
 import org.junit.Assert;
 import org.junit.Test;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * @author <a href="mailto:[email protected]">oEmbedler Inc.</a>
  */
@@ -162,4 +166,30 @@ public void appContextLoadsWithCustomSchemaProvider() {
 
         Assert.assertNotNull(this.getContext().getBean(AbstractGraphQLHttpServlet.class));
     }
+
+    @Test
+    public void queryInvokerShouldNotBeTransactionalByDefault() {
+        load(SimpleConfiguration.class);
+        assertThatQueryInvokerIsNotTransactional();
+    }
+
+    @Test
+    public void queryInvokerShouldNotBeTransactionalIfDisabled() {
+        load(SimpleConfiguration.class, "graphql.query-invoker.transactional=false");
+        assertThatQueryInvokerIsNotTransactional();
+    }
+
+    @Test
+    public void queryInvokerShouldBeTransactionalIfConfigured() {
+        load(SimpleConfiguration.class, "graphql.query-invoker.transactional=true");
+        assertThat(this.getContext().getBean(GraphQLQueryInvoker.class))
+                .as("Should be a transactional query invoker.")
+                .isInstanceOf(TransactionalGraphQLQueryInvokerWrapper.class);
+    }
+
+    private void assertThatQueryInvokerIsNotTransactional() {
+        assertThat(this.getContext().getBean(GraphQLQueryInvoker.class))
+                .as("Should be a non-transactional query invoker.")
+                .isNotNull().isNotInstanceOf(TransactionalGraphQLQueryInvokerWrapper.class);
+    }
 }