From cc055bd9d6625e6a12381676778cb97dbb0f466d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Vajner?= Date: Sat, 16 Nov 2019 08:59:08 +0100 Subject: [PATCH 1/3] feat: added option to use a transactional query invoker This may prevent LazyInitializationException when working with JPA entities across multiple resolvers (thus making individual resolvers Transactional is not sufficient). By default, this feature is disabled. It can be enabled in application.properties/yaml with: graphql.query-invoker.transactional=true --- README.md | 4 + gradle.properties | 1 + .../build.gradle | 1 + .../boot/GraphQLQueryInvokerProperties.java | 13 ++++ .../boot/GraphQLWebAutoConfiguration.java | 15 +++- ...ansactionalGraphQLQueryInvokerWrapper.java | 58 +++++++++++++++ .../TransactionQueryInvokerWrapperTest.java | 74 +++++++++++++++++++ .../web/GraphQLWebAutoConfigurationTest.java | 30 ++++++++ 8 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLQueryInvokerProperties.java create mode 100644 graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/TransactionalGraphQLQueryInvokerWrapper.java create mode 100644 graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/TransactionQueryInvokerWrapperTest.java diff --git a/README.md b/README.md index 32c4a226..56b48648 100644 --- a/README.md +++ b/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. diff --git a/gradle.properties b/gradle.properties index 91d150d5..5db14a90 100644 --- a/gradle.properties +++ b/gradle.properties @@ -44,6 +44,7 @@ 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 kotlin.version=1.3.31 GRADLE_WRAPPER_VER = 4.10.3 diff --git a/graphql-spring-boot-autoconfigure/build.gradle b/graphql-spring-boot-autoconfigure/build.gradle index 8893cc63..bc1d4b68 100644 --- a/graphql-spring-boot-autoconfigure/build.gradle +++ b/graphql-spring-boot-autoconfigure/build.gradle @@ -23,6 +23,7 @@ dependencies { compile "org.springframework.boot:spring-boot-autoconfigure:$LIB_SPRING_BOOT_VER" compile "org.springframework.boot:spring-boot-starter-websocket:$LIB_SPRING_BOOT_VER" + compile "javax.transaction:javax.transaction-api:$LIB_TRANSACTIONS_API_VERSION" compile "org.springframework.boot:spring-boot-starter-actuator:$LIB_SPRING_BOOT_VER" compile "com.graphql-java-kickstart:graphql-java-servlet:$LIB_GRAPHQL_SERVLET_VER" compile "com.graphql-java-kickstart:graphql-java-tools:$LIB_GRAPHQL_JAVA_TOOLS_VER" diff --git a/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLQueryInvokerProperties.java b/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLQueryInvokerProperties.java new file mode 100644 index 00000000..cd3aaf38 --- /dev/null +++ b/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; +} diff --git a/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLWebAutoConfiguration.java b/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLWebAutoConfiguration.java index f489bfa1..ed6f5860 100644 --- a/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/GraphQLWebAutoConfiguration.java +++ b/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 diff --git a/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/TransactionalGraphQLQueryInvokerWrapper.java b/graphql-spring-boot-autoconfigure/src/main/java/com/oembedler/moon/graphql/boot/TransactionalGraphQLQueryInvokerWrapper.java new file mode 100644 index 00000000..5d274955 --- /dev/null +++ b/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 query(final List batchedInvocationInput, + final ContextSetting contextSetting) { + return wrappedInvoker.query(batchedInvocationInput, contextSetting); + } +} diff --git a/graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/TransactionQueryInvokerWrapperTest.java b/graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/TransactionQueryInvokerWrapperTest.java new file mode 100644 index 00000000..8baf9681 --- /dev/null +++ b/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 invocationInputList = Collections.singletonList(invocationInput); + final ContextSetting contextSetting = ContextSetting.PER_QUERY_WITH_INSTRUMENTATION; + final ExecutionResult expectedExecutionResult = mock(ExecutionResult.class); + final List expectedExecutionResultList = Collections.singletonList(expectedExecutionResult); + given(wrappedInvoker.query(invocationInputList, contextSetting)).willReturn(expectedExecutionResultList); + //WHEN + final List 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); + } +} diff --git a/graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/test/web/GraphQLWebAutoConfigurationTest.java b/graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/test/web/GraphQLWebAutoConfigurationTest.java index 0a332052..962f843b 100644 --- a/graphql-spring-boot-autoconfigure/src/test/java/com/oembedler/moon/graphql/boot/test/web/GraphQLWebAutoConfigurationTest.java +++ b/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 oEmbedler Inc. */ @@ -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); + } } From 7d260f7ab2a8b091b2a38433aaf8a2d08050ccb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Vajner?= Date: Sat, 16 Nov 2019 10:05:53 +0100 Subject: [PATCH 2/3] fix: compilation error on JDK 8 --- gradle.properties | 1 + graphql-spring-boot-autoconfigure/build.gradle | 1 + 2 files changed, 2 insertions(+) diff --git a/gradle.properties b/gradle.properties index 5db14a90..d4760bb7 100644 --- a/gradle.properties +++ b/gradle.properties @@ -45,6 +45,7 @@ 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 diff --git a/graphql-spring-boot-autoconfigure/build.gradle b/graphql-spring-boot-autoconfigure/build.gradle index bc1d4b68..40dea7a8 100644 --- a/graphql-spring-boot-autoconfigure/build.gradle +++ b/graphql-spring-boot-autoconfigure/build.gradle @@ -24,6 +24,7 @@ dependencies { compile "org.springframework.boot:spring-boot-autoconfigure:$LIB_SPRING_BOOT_VER" compile "org.springframework.boot:spring-boot-starter-websocket:$LIB_SPRING_BOOT_VER" compile "javax.transaction:javax.transaction-api:$LIB_TRANSACTIONS_API_VERSION" + compile "javax.interceptor:javax.interceptor-api:$LIB_INTERCEPTOR_API_VERSION" compile "org.springframework.boot:spring-boot-starter-actuator:$LIB_SPRING_BOOT_VER" compile "com.graphql-java-kickstart:graphql-java-servlet:$LIB_GRAPHQL_SERVLET_VER" compile "com.graphql-java-kickstart:graphql-java-tools:$LIB_GRAPHQL_JAVA_TOOLS_VER" From fc611f817575a45e53b0bdeb2073621962c8ba96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Vajner?= Date: Sat, 16 Nov 2019 16:14:55 +0100 Subject: [PATCH 3/3] chore: transaction/interceptor api is now a compileOnly dependency --- graphql-spring-boot-autoconfigure/build.gradle | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/graphql-spring-boot-autoconfigure/build.gradle b/graphql-spring-boot-autoconfigure/build.gradle index 40dea7a8..2b5d8c3e 100644 --- a/graphql-spring-boot-autoconfigure/build.gradle +++ b/graphql-spring-boot-autoconfigure/build.gradle @@ -23,15 +23,16 @@ dependencies { compile "org.springframework.boot:spring-boot-autoconfigure:$LIB_SPRING_BOOT_VER" compile "org.springframework.boot:spring-boot-starter-websocket:$LIB_SPRING_BOOT_VER" - compile "javax.transaction:javax.transaction-api:$LIB_TRANSACTIONS_API_VERSION" - compile "javax.interceptor:javax.interceptor-api:$LIB_INTERCEPTOR_API_VERSION" compile "org.springframework.boot:spring-boot-starter-actuator:$LIB_SPRING_BOOT_VER" compile "com.graphql-java-kickstart:graphql-java-servlet:$LIB_GRAPHQL_SERVLET_VER" compile "com.graphql-java-kickstart:graphql-java-tools:$LIB_GRAPHQL_JAVA_TOOLS_VER" 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"