From 0d616b892492109e5800c9a523017528d7740fa1 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 3 Jan 2022 16:00:04 +0100 Subject: [PATCH 1/3] Fix WebGraphQlTester auto-registration for SpringBootTest Prior to this commit, the `GraphQlTesterContextCustomizer` would register a `WebGraphQlTester` instance as a `GraphQlTester` bean., only exposing the `GraphQlTester` type. This is not in line with the documentation and also does not register the bean definition with the most specific type. With this issue, a `@SpringBootTest` integration test will not be injected with a `WebGraphQlTester` if it asks one. This commit ensures that the `WebGraphQlTester` is registered as such and that all related classes are renamed as a result. Fixes gh-29250 --- ...=> WebGraphQlTesterContextCustomizer.java} | 31 +++++++++---------- ...raphQlTesterContextCustomizerFactory.java} | 10 +++--- .../main/resources/META-INF/spring.factories | 2 +- ...terContextCustomizerIntegrationTests.java} | 8 ++--- ...extCustomizerWithCustomBasePathTests.java} | 8 ++--- ...CustomizerWithCustomContextPathTests.java} | 8 ++--- 6 files changed, 33 insertions(+), 34 deletions(-) rename spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/{GraphQlTesterContextCustomizer.java => WebGraphQlTesterContextCustomizer.java} (87%) rename spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/{GraphQlTesterContextCustomizerFactory.java => WebGraphQlTesterContextCustomizerFactory.java} (82%) rename spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/{GraphQlTesterContextCustomizerIntegrationTests.java => WebGraphQlTesterContextCustomizerIntegrationTests.java} (92%) rename spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/{GraphQlTesterContextCustomizerWithCustomBasePathTests.java => WebGraphQlTesterContextCustomizerWithCustomBasePathTests.java} (91%) rename spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/{GraphQlTesterContextCustomizerWithCustomContextPathTests.java => WebGraphQlTesterContextCustomizerWithCustomContextPathTests.java} (89%) diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizer.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizer.java similarity index 87% rename from spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizer.java rename to spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizer.java index d079bd5b76..6039ff36c6 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizer.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizer.java @@ -35,7 +35,6 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.core.Ordered; -import org.springframework.graphql.test.tester.GraphQlTester; import org.springframework.graphql.test.tester.WebGraphQlTester; import org.springframework.test.context.ContextCustomizer; import org.springframework.test.context.MergedContextConfiguration; @@ -46,32 +45,32 @@ import org.springframework.util.StringUtils; import org.springframework.web.context.WebApplicationContext; /** - * {@link ContextCustomizer} for {@link GraphQlTester}. + * {@link ContextCustomizer} for {@link WebGraphQlTester}. * * @author Brian Clozel */ -class GraphQlTesterContextCustomizer implements ContextCustomizer { +class WebGraphQlTesterContextCustomizer implements ContextCustomizer { @Override public void customizeContext(ConfigurableApplicationContext context, MergedContextConfiguration mergedConfig) { SpringBootTest springBootTest = TestContextAnnotationUtils.findMergedAnnotation(mergedConfig.getTestClass(), SpringBootTest.class); if (springBootTest.webEnvironment().isEmbedded()) { - registerGraphQlTester(context); + registerWebGraphQlTester(context); } } - private void registerGraphQlTester(ConfigurableApplicationContext context) { + private void registerWebGraphQlTester(ConfigurableApplicationContext context) { ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); if (beanFactory instanceof BeanDefinitionRegistry) { - registerGraphQlTester((BeanDefinitionRegistry) beanFactory); + registerWebGraphQlTester((BeanDefinitionRegistry) beanFactory); } } - private void registerGraphQlTester(BeanDefinitionRegistry registry) { - RootBeanDefinition definition = new RootBeanDefinition(GraphQlTesterRegistrar.class); + private void registerWebGraphQlTester(BeanDefinitionRegistry registry) { + RootBeanDefinition definition = new RootBeanDefinition(WebGraphQlTesterRegistrar.class); definition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); - registry.registerBeanDefinition(GraphQlTesterRegistrar.class.getName(), definition); + registry.registerBeanDefinition(WebGraphQlTesterRegistrar.class.getName(), definition); } @Override @@ -84,7 +83,7 @@ class GraphQlTesterContextCustomizer implements ContextCustomizer { return getClass().hashCode(); } - private static class GraphQlTesterRegistrar + private static class WebGraphQlTesterRegistrar implements BeanDefinitionRegistryPostProcessor, Ordered, BeanFactoryAware { private BeanFactory beanFactory; @@ -97,9 +96,9 @@ class GraphQlTesterContextCustomizer implements ContextCustomizer { @Override public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { if (BeanFactoryUtils.beanNamesForTypeIncludingAncestors((ListableBeanFactory) this.beanFactory, - GraphQlTester.class, false, false).length == 0) { + WebGraphQlTester.class, false, false).length == 0) { registry.registerBeanDefinition(WebGraphQlTester.class.getName(), - new RootBeanDefinition(GraphQlTesterFactory.class)); + new RootBeanDefinition(WebGraphQlTesterFactory.class)); } } @@ -115,7 +114,7 @@ class GraphQlTesterContextCustomizer implements ContextCustomizer { } - public static class GraphQlTesterFactory implements FactoryBean, ApplicationContextAware { + public static class WebGraphQlTesterFactory implements FactoryBean, ApplicationContextAware { private static final String SERVLET_APPLICATION_CONTEXT_CLASS = "org.springframework.web.context.WebApplicationContext"; @@ -123,7 +122,7 @@ class GraphQlTesterContextCustomizer implements ContextCustomizer { private ApplicationContext applicationContext; - private GraphQlTester object; + private WebGraphQlTester object; @Override public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { @@ -137,11 +136,11 @@ class GraphQlTesterContextCustomizer implements ContextCustomizer { @Override public Class getObjectType() { - return GraphQlTester.class; + return WebGraphQlTester.class; } @Override - public GraphQlTester getObject() throws Exception { + public WebGraphQlTester getObject() throws Exception { if (this.object == null) { this.object = createGraphQlTester(); } diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerFactory.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerFactory.java similarity index 82% rename from spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerFactory.java rename to spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerFactory.java index 497db17727..d0642a568f 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerFactory.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerFactory.java @@ -30,11 +30,11 @@ import org.springframework.util.ClassUtils; * {@link ContextCustomizerFactory} for {@link GraphQlTester}. * * @author Brian Clozel - * @see GraphQlTesterContextCustomizer + * @see WebGraphQlTesterContextCustomizer */ -class GraphQlTesterContextCustomizerFactory implements ContextCustomizerFactory { +class WebGraphQlTesterContextCustomizerFactory implements ContextCustomizerFactory { - private static final String GRAPHQLTESTER_CLASS = "org.springframework.graphql.test.tester.GraphQlTester"; + private static final String WEBGRAPHQLTESTER_CLASS = "org.springframework.graphql.test.tester.WebGraphQlTester"; private static final String WEBTESTCLIENT_CLASS = "org.springframework.test.web.reactive.server.WebTestClient"; @@ -43,12 +43,12 @@ class GraphQlTesterContextCustomizerFactory implements ContextCustomizerFactory List configAttributes) { SpringBootTest springBootTest = TestContextAnnotationUtils.findMergedAnnotation(testClass, SpringBootTest.class); - return (springBootTest != null && isGraphQlTesterPresent()) ? new GraphQlTesterContextCustomizer() : null; + return (springBootTest != null && isGraphQlTesterPresent()) ? new WebGraphQlTesterContextCustomizer() : null; } private boolean isGraphQlTesterPresent() { return ClassUtils.isPresent(WEBTESTCLIENT_CLASS, getClass().getClassLoader()) - && ClassUtils.isPresent(GRAPHQLTESTER_CLASS, getClass().getClassLoader()); + && ClassUtils.isPresent(WEBGRAPHQLTESTER_CLASS, getClass().getClassLoader()); } } diff --git a/spring-boot-project/spring-boot-test/src/main/resources/META-INF/spring.factories b/spring-boot-project/spring-boot-test/src/main/resources/META-INF/spring.factories index 3dd1783cd9..a0dac2ca69 100644 --- a/spring-boot-project/spring-boot-test/src/main/resources/META-INF/spring.factories +++ b/spring-boot-project/spring-boot-test/src/main/resources/META-INF/spring.factories @@ -2,7 +2,7 @@ org.springframework.test.context.ContextCustomizerFactory=\ org.springframework.boot.test.context.ImportsContextCustomizerFactory,\ org.springframework.boot.test.context.filter.ExcludeFilterContextCustomizerFactory,\ -org.springframework.boot.test.graphql.tester.GraphQlTesterContextCustomizerFactory,\ +org.springframework.boot.test.graphql.tester.WebGraphQlTesterContextCustomizerFactory,\ org.springframework.boot.test.json.DuplicateJsonObjectContextCustomizerFactory,\ org.springframework.boot.test.mock.mockito.MockitoContextCustomizerFactory,\ org.springframework.boot.test.web.client.TestRestTemplateContextCustomizerFactory,\ diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerIntegrationTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerIntegrationTests.java similarity index 92% rename from spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerIntegrationTests.java rename to spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerIntegrationTests.java index b9c334ca88..622c05a162 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerIntegrationTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerIntegrationTests.java @@ -28,7 +28,7 @@ import org.springframework.boot.web.embedded.tomcat.TomcatReactiveWebServerFacto import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.buffer.DefaultDataBufferFactory; -import org.springframework.graphql.test.tester.GraphQlTester; +import org.springframework.graphql.test.tester.WebGraphQlTester; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.server.reactive.ContextPathCompositeHandler; @@ -38,17 +38,17 @@ import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.test.annotation.DirtiesContext; /** - * Integration test for {@link GraphQlTesterContextCustomizer}. + * Integration test for {@link WebGraphQlTesterContextCustomizer}. * * @author Brian Clozel */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, properties = "spring.main.web-application-type=reactive") @DirtiesContext -class GraphQlTesterContextCustomizerIntegrationTests { +class WebGraphQlTesterContextCustomizerIntegrationTests { @Autowired - GraphQlTester graphQlTester; + WebGraphQlTester graphQlTester; @Test void shouldHandleGraphQlRequests() { diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomBasePathTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomBasePathTests.java similarity index 91% rename from spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomBasePathTests.java rename to spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomBasePathTests.java index 6c1ecb4e19..fea9634620 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomBasePathTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomBasePathTests.java @@ -28,7 +28,7 @@ import org.springframework.boot.web.embedded.tomcat.TomcatReactiveWebServerFacto import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.buffer.DefaultDataBufferFactory; -import org.springframework.graphql.test.tester.GraphQlTester; +import org.springframework.graphql.test.tester.WebGraphQlTester; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.server.reactive.ContextPathCompositeHandler; @@ -38,17 +38,17 @@ import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.test.context.TestPropertySource; /** - * Tests for {@link GraphQlTesterContextCustomizer} with a custom context path for a + * Tests for {@link WebGraphQlTesterContextCustomizer} with a custom context path for a * Reactive web application. * * @author Brian Clozel */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @TestPropertySource(properties = { "spring.main.web-application-type=reactive", "spring.webflux.base-path=/test" }) -class GraphQlTesterContextCustomizerWithCustomBasePathTests { +class WebGraphQlTesterContextCustomizerWithCustomBasePathTests { @Autowired - GraphQlTester graphQlTester; + WebGraphQlTester graphQlTester; @Test void shouldHandleGraphQlRequests() { diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomContextPathTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomContextPathTests.java similarity index 89% rename from spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomContextPathTests.java rename to spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomContextPathTests.java index 095e432041..2a3bec2a55 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/GraphQlTesterContextCustomizerWithCustomContextPathTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/graphql/tester/WebGraphQlTesterContextCustomizerWithCustomContextPathTests.java @@ -24,7 +24,7 @@ import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactor import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; -import org.springframework.graphql.test.tester.GraphQlTester; +import org.springframework.graphql.test.tester.WebGraphQlTester; import org.springframework.http.MediaType; import org.springframework.test.context.TestPropertySource; import org.springframework.web.bind.annotation.PostMapping; @@ -32,17 +32,17 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.servlet.DispatcherServlet; /** - * Tests for {@link GraphQlTesterContextCustomizer} with a custom context path for a + * Tests for {@link WebGraphQlTesterContextCustomizer} with a custom context path for a * Servlet web application. * * @author Brian Clozel */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @TestPropertySource(properties = "server.servlet.context-path=/test") -class GraphQlTesterContextCustomizerWithCustomContextPathTests { +class WebGraphQlTesterContextCustomizerWithCustomContextPathTests { @Autowired - GraphQlTester graphQlTester; + WebGraphQlTester graphQlTester; @Test void shouldHandleGraphQlRequests() { From 728206dba0dbd5be773c17c3625158b8285bf19f Mon Sep 17 00:00:00 2001 From: izeye Date: Sat, 1 Jan 2022 01:06:20 +0900 Subject: [PATCH 2/3] Polish GraphQL changes See gh-29140 Closes gh-29194 --- .../graphql/GraphQlMetricsInstrumentation.java | 8 +++++++- .../boot/actuate/metrics/graphql/GraphQlTags.java | 14 +++++++------- .../GraphQlMetricsInstrumentationTests.java | 8 ++++---- ...raphQlWebMvcSecurityAutoConfigurationTests.java | 2 +- .../src/docs/asciidoc/actuator/metrics.adoc | 2 +- .../src/docs/asciidoc/web/spring-graphql.adoc | 2 +- ...jectsController.java => ProjectController.java} | 4 ++-- .../java/smoketest/graphql/SecurityConfig.java | 6 +++--- .../smoketest/graphql/ProjectControllerTests.java | 2 +- 9 files changed, 27 insertions(+), 21 deletions(-) rename spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/{ProjectsController.java => ProjectController.java} (95%) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentation.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentation.java index 893cc2622b..72c293ff1b 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentation.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentation.java @@ -35,6 +35,12 @@ import io.micrometer.core.instrument.Timer; import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.lang.Nullable; +/** + * Micrometer-based {@link SimpleInstrumentation}. + * + * @author Brian Clozel + * @since 2.7.0 + */ public class GraphQlMetricsInstrumentation extends SimpleInstrumentation { private final MeterRegistry registry; @@ -127,7 +133,7 @@ public class GraphQlMetricsInstrumentation extends SimpleInstrumentation { private Timer.Sample sample; - private AtomicLong dataFetchingCount = new AtomicLong(0L); + private final AtomicLong dataFetchingCount = new AtomicLong(); RequestMetricsInstrumentationState(AutoTimer autoTimer, MeterRegistry registry) { this.timer = autoTimer.builder("graphql.request"); diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlTags.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlTags.java index ded7572502..516fafe0e7 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlTags.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlTags.java @@ -34,7 +34,7 @@ import org.springframework.util.CollectionUtils; * Factory methods for Tags associated with a GraphQL request. * * @author Brian Clozel - * @since 1.0.0 + * @since 2.7.0 */ public final class GraphQlTags { @@ -72,7 +72,7 @@ public final class GraphQlTags { builder.append('$'); for (Object segment : pathSegments) { try { - int index = Integer.parseUnsignedInt(segment.toString()); + Integer.parseUnsignedInt(segment.toString()); builder.append("[*]"); } catch (NumberFormatException exc) { @@ -90,13 +90,13 @@ public final class GraphQlTags { public static Tag dataFetchingPath(InstrumentationFieldFetchParameters parameters) { ExecutionStepInfo executionStepInfo = parameters.getExecutionStepInfo(); - StringBuilder dataFetchingType = new StringBuilder(); + StringBuilder dataFetchingPath = new StringBuilder(); if (executionStepInfo.hasParent() && executionStepInfo.getParent().getType() instanceof GraphQLObjectType) { - dataFetchingType.append(((GraphQLObjectType) executionStepInfo.getParent().getType()).getName()); - dataFetchingType.append('.'); + dataFetchingPath.append(((GraphQLObjectType) executionStepInfo.getParent().getType()).getName()); + dataFetchingPath.append('.'); } - dataFetchingType.append(executionStepInfo.getPath().getSegmentName()); - return Tag.of("path", dataFetchingType.toString()); + dataFetchingPath.append(executionStepInfo.getPath().getSegmentName()); + return Tag.of("path", dataFetchingPath.toString()); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentationTests.java index a085a12ae9..a2fa75a7b5 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentationTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentationTests.java @@ -85,7 +85,7 @@ class GraphQlMetricsInstrumentationTests { Timer timer = this.registry.find("graphql.request").timer(); assertThat(timer).isNotNull(); - assertThat(timer.takeSnapshot().count()).isEqualTo(1); + assertThat(timer.count()).isEqualTo(1); } @Test @@ -120,7 +120,7 @@ class GraphQlMetricsInstrumentationTests { Timer timer = this.registry.find("graphql.datafetcher").timer(); assertThat(timer).isNotNull(); - assertThat(timer.takeSnapshot().count()).isEqualTo(1); + assertThat(timer.count()).isEqualTo(1); } @Test @@ -135,7 +135,7 @@ class GraphQlMetricsInstrumentationTests { Timer timer = this.registry.find("graphql.datafetcher").timer(); assertThat(timer).isNotNull(); - assertThat(timer.takeSnapshot().count()).isEqualTo(1); + assertThat(timer.count()).isEqualTo(1); } @Test @@ -150,7 +150,7 @@ class GraphQlMetricsInstrumentationTests { Timer timer = this.registry.find("graphql.datafetcher").timer(); assertThat(timer).isNotNull(); - assertThat(timer.takeSnapshot().count()).isEqualTo(1); + assertThat(timer.count()).isEqualTo(1); } @Test diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/security/GraphQlWebMvcSecurityAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/security/GraphQlWebMvcSecurityAutoConfigurationTests.java index cf876b091b..68a2d7a797 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/security/GraphQlWebMvcSecurityAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/security/GraphQlWebMvcSecurityAutoConfigurationTests.java @@ -168,7 +168,7 @@ class GraphQlWebMvcSecurityAutoConfigurationTests { } @Bean - static InMemoryUserDetailsManager userDetailsService() { + InMemoryUserDetailsManager userDetailsService() { User.UserBuilder userBuilder = User.withDefaultPasswordEncoder(); UserDetails rob = userBuilder.username("rob").password("rob").roles("USER").build(); UserDetails admin = userBuilder.username("admin").password("admin").roles("USER", "ADMIN").build(); diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc index b33232633c..17a4e09bc7 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc @@ -905,7 +905,7 @@ A single GraphQL query can involve many `DataFetcher` calls, so there is a dedic The `graphql.request.datafetch.count` https://micrometer.io/docs/concepts#_distribution_summaries[distribution summary] counts the number of non-trivial `DataFetcher` calls made per request. -This metric is useful for detecting "N+1" data fetching issues and consider batch loading; it provides the `"TOTAL"` number of data fetcher calls made over the `"COUNT"` of recorded requests, as well as the `"MAX"` calls made for a single request over the considered period. +This metric is useful for detecting "N+1" data fetching issues and considering batch loading; it provides the `"TOTAL"` number of data fetcher calls made over the `"COUNT"` of recorded requests, as well as the `"MAX"` calls made for a single request over the considered period. More options are available for <>. A single response can contain many GraphQL errors, counted by the `graphql.error` counter: diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc index f2a4116629..66ea980ac8 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc @@ -49,7 +49,7 @@ You can declare `RuntimeWiringConfigurer` beans in your Spring config to get acc Spring Boot detects such beans and adds them to the {spring-graphql-docs}#execution-graphqlsource[GraphQlSource builder]. Typically, however, applications will not implement `DataFetcher` directly and will instead create {spring-graphql-docs}#controllers[annotated controllers]. -Spring Boot will automatically register `@Controller` classes with annotated handler methods and registers those as `DataFetcher`s. +Spring Boot will automatically detect `@Controller` classes with annotated handler methods and register those as `DataFetcher`s. Here's a sample implementation for our greeting query with a `@Controller` class: [source,java,indent=0,subs="verbatim"] diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectsController.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectController.java similarity index 95% rename from spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectsController.java rename to spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectController.java index e2fd01dae7..1554cfc33d 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectsController.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/ProjectController.java @@ -25,11 +25,11 @@ import org.springframework.graphql.data.method.annotation.QueryMapping; import org.springframework.stereotype.Controller; @Controller -public class ProjectsController { +public class ProjectController { private final List projects; - public ProjectsController() { + public ProjectController() { this.projects = Arrays.asList(new Project("spring-boot", "Spring Boot"), new Project("spring-graphql", "Spring GraphQL"), new Project("spring-framework", "Spring Framework")); } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/SecurityConfig.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/SecurityConfig.java index fcbb5be480..f5a84818b2 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/SecurityConfig.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/SecurityConfig.java @@ -28,13 +28,13 @@ import org.springframework.security.web.DefaultSecurityFilterChain; import static org.springframework.security.config.Customizer.withDefaults; -@Configuration +@Configuration(proxyBeanMethods = false) @EnableWebSecurity @EnableGlobalMethodSecurity(prePostEnabled = true) public class SecurityConfig { @Bean - DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Exception { + public DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Exception { return http.csrf((csrf) -> csrf.disable()) // Demonstrate that method security works // Best practice to use both for defense in depth @@ -43,7 +43,7 @@ public class SecurityConfig { @Bean @SuppressWarnings("deprecation") - public static InMemoryUserDetailsManager userDetailsService() { + public InMemoryUserDetailsManager userDetailsService() { User.UserBuilder userBuilder = User.withDefaultPasswordEncoder(); UserDetails rob = userBuilder.username("rob").password("rob").roles("USER").build(); UserDetails admin = userBuilder.username("admin").password("admin").roles("USER", "ADMIN").build(); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/test/java/smoketest/graphql/ProjectControllerTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/test/java/smoketest/graphql/ProjectControllerTests.java index 89d3e218a1..95624d8684 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/test/java/smoketest/graphql/ProjectControllerTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/test/java/smoketest/graphql/ProjectControllerTests.java @@ -22,7 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.graphql.GraphQlTest; import org.springframework.graphql.test.tester.GraphQlTester; -@GraphQlTest(ProjectsController.class) +@GraphQlTest(ProjectController.class) class ProjectControllerTests { @Autowired From c5817f21eb30a8e4a19070a9a67f65784158c622 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 3 Jan 2022 17:09:40 +0100 Subject: [PATCH 3/3] Add property for disabling GraphQL schema introspection Prior to this commit, the GraphQL schema assembled by the auto-configuration would provide no option for disabling the field introspection. While this feature is essential for many tools (including GraphiQL), some prefer disabling it because this allows clients to gather information about types and schema easily. This commit introduces a new `spring.graphql.schema.introspection.enabled` configuration property. Because potential attackers can still gather this information and this feature is a core concern in the GraphQL spec, introspection is enabled by default for Spring Boot applications. Closes gh-29248 --- .../graphql/GraphQlAutoConfiguration.java | 5 ++++ .../graphql/GraphQlProperties.java | 23 +++++++++++++++++++ .../GraphQlAutoConfigurationTests.java | 21 +++++++++++++++++ .../src/docs/asciidoc/web/spring-graphql.adoc | 2 ++ 4 files changed, 51 insertions(+) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfiguration.java index 6b2327d2d0..5cb3f77b0b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfiguration.java @@ -24,6 +24,7 @@ import java.util.stream.Collectors; import graphql.GraphQL; import graphql.execution.instrumentation.Instrumentation; +import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -77,6 +78,10 @@ public class GraphQlAutoConfiguration { .schemaResources(schemaResources.toArray(new Resource[0])) .exceptionResolvers(exceptionResolversProvider.orderedStream().collect(Collectors.toList())) .instrumentation(instrumentationsProvider.orderedStream().collect(Collectors.toList())); + if (!properties.getSchema().getIntrospection().isEnabled()) { + builder.configureRuntimeWiring((wiring) -> wiring + .fieldVisibility(NoIntrospectionGraphqlFieldVisibility.NO_INTROSPECTION_FIELD_VISIBILITY)); + } wiringConfigurers.orderedStream().forEach(builder::configureRuntimeWiring); sourceCustomizers.orderedStream().forEach((customizer) -> customizer.customize(builder)); try { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlProperties.java index 3df070e745..02df4a3095 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/graphql/GraphQlProperties.java @@ -73,6 +73,8 @@ public class GraphQlProperties { */ private String[] fileExtensions = new String[] { ".graphqls", ".gqls" }; + private final Introspection introspection = new Introspection(); + private final Printer printer = new Printer(); public String[] getLocations() { @@ -96,10 +98,31 @@ public class GraphQlProperties { .toArray(String[]::new); } + public Introspection getIntrospection() { + return this.introspection; + } + public Printer getPrinter() { return this.printer; } + public static class Introspection { + + /** + * Whether field introspection should be enabled at the schema level. + */ + private boolean enabled = true; + + public boolean isEnabled() { + return this.enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + } + public static class Printer { /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfigurationTests.java index 5f1f388f9a..e769425c76 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/GraphQlAutoConfigurationTests.java @@ -21,6 +21,8 @@ import graphql.execution.instrumentation.ChainedInstrumentation; import graphql.execution.instrumentation.Instrumentation; import graphql.schema.GraphQLSchema; import graphql.schema.idl.RuntimeWiring; +import graphql.schema.visibility.DefaultGraphqlFieldVisibility; +import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; @@ -145,6 +147,25 @@ class GraphQlAutoConfigurationTests { }); } + @Test + void fieldIntrospectionShouldBeEnabledByDefault() { + this.contextRunner.run((context) -> { + GraphQlSource graphQlSource = context.getBean(GraphQlSource.class); + GraphQLSchema schema = graphQlSource.schema(); + assertThat(schema.getCodeRegistry().getFieldVisibility()).isInstanceOf(DefaultGraphqlFieldVisibility.class); + }); + } + + @Test + void shouldDisableFieldIntrospection() { + this.contextRunner.withPropertyValues("spring.graphql.schema.introspection.enabled:false").run((context) -> { + GraphQlSource graphQlSource = context.getBean(GraphQlSource.class); + GraphQLSchema schema = graphQlSource.schema(); + assertThat(schema.getCodeRegistry().getFieldVisibility()) + .isInstanceOf(NoIntrospectionGraphqlFieldVisibility.class); + }); + } + @Configuration(proxyBeanMethods = false) static class CustomGraphQlBuilderConfiguration { diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc index 66ea980ac8..e197774c32 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc @@ -40,6 +40,8 @@ In the following sections, we'll consider this sample GraphQL schema, defining t include::{docs-resources}/graphql/schema.graphqls[] ---- +NOTE: By default, https://spec.graphql.org/draft/#sec-Introspection[field introspection] will be allowed on the schema as it is required for tools such as GraphiQL. +If you wish to not expose information about the schema, you can disable introspection by setting configprop:spring.graphql.schema.introspection.enabled[] to `false`. [[web.graphql.runtimewiring]] === GraphQL RuntimeWiring