From 80cd7d0a196900f6f8de5c9c9cb99633e445daff Mon Sep 17 00:00:00 2001 From: izeye Date: Sat, 1 Jan 2022 01:06:20 +0900 Subject: [PATCH] Polish GraphQL changes See gh-29140 --- .../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 893cc2622b23..72c293ff1ba8 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 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 @@ static class RequestMetricsInstrumentationState implements InstrumentationState 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 ded757250279..516fafe0e738 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 @@ * 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 static Tag errorPath(GraphQLError error) { 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 static Tag dataFetchingOutcome(@Nullable Throwable exception) { 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 a085a12ae973..a2fa75a7b5aa 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 @@ void shouldRecordTimerWhenResult() { 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 @@ void shouldRecordDataFetchingMetricWhenSuccess() throws Exception { 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 @@ void shouldRecordDataFetchingMetricWhenSuccessCompletionStage() throws Exception 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 @@ void shouldRecordDataFetchingMetricWhenError() throws Exception { 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 cf876b091b44..68a2d7a797f6 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 @@ DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Except } @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 b33232633c7c..17a4e09bc73c 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 f2a411662937..66ea980ac8a3 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 e2fd01dae724..1554cfc33d35 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.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 fcbb5be480f6..f5a84818b210 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 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 @@ DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Except @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 89d3e218a1d5..95624d8684f2 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.boot.test.autoconfigure.graphql.GraphQlTest; import org.springframework.graphql.test.tester.GraphQlTester; -@GraphQlTest(ProjectsController.class) +@GraphQlTest(ProjectController.class) class ProjectControllerTests { @Autowired