Skip to content

Commit 80cd7d0

Browse files
committed
Polish GraphQL changes
See spring-projectsgh-29140
1 parent de947e0 commit 80cd7d0

File tree

9 files changed

+27
-21
lines changed

9 files changed

+27
-21
lines changed

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentation.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@
3535
import org.springframework.boot.actuate.metrics.AutoTimer;
3636
import org.springframework.lang.Nullable;
3737

38+
/**
39+
* Micrometer-based {@link SimpleInstrumentation}.
40+
*
41+
* @author Brian Clozel
42+
* @since 2.7.0
43+
*/
3844
public class GraphQlMetricsInstrumentation extends SimpleInstrumentation {
3945

4046
private final MeterRegistry registry;
@@ -127,7 +133,7 @@ static class RequestMetricsInstrumentationState implements InstrumentationState
127133

128134
private Timer.Sample sample;
129135

130-
private AtomicLong dataFetchingCount = new AtomicLong(0L);
136+
private final AtomicLong dataFetchingCount = new AtomicLong();
131137

132138
RequestMetricsInstrumentationState(AutoTimer autoTimer, MeterRegistry registry) {
133139
this.timer = autoTimer.builder("graphql.request");

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/graphql/GraphQlTags.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
* Factory methods for Tags associated with a GraphQL request.
3535
*
3636
* @author Brian Clozel
37-
* @since 1.0.0
37+
* @since 2.7.0
3838
*/
3939
public final class GraphQlTags {
4040

@@ -72,7 +72,7 @@ public static Tag errorPath(GraphQLError error) {
7272
builder.append('$');
7373
for (Object segment : pathSegments) {
7474
try {
75-
int index = Integer.parseUnsignedInt(segment.toString());
75+
Integer.parseUnsignedInt(segment.toString());
7676
builder.append("[*]");
7777
}
7878
catch (NumberFormatException exc) {
@@ -90,13 +90,13 @@ public static Tag dataFetchingOutcome(@Nullable Throwable exception) {
9090

9191
public static Tag dataFetchingPath(InstrumentationFieldFetchParameters parameters) {
9292
ExecutionStepInfo executionStepInfo = parameters.getExecutionStepInfo();
93-
StringBuilder dataFetchingType = new StringBuilder();
93+
StringBuilder dataFetchingPath = new StringBuilder();
9494
if (executionStepInfo.hasParent() && executionStepInfo.getParent().getType() instanceof GraphQLObjectType) {
95-
dataFetchingType.append(((GraphQLObjectType) executionStepInfo.getParent().getType()).getName());
96-
dataFetchingType.append('.');
95+
dataFetchingPath.append(((GraphQLObjectType) executionStepInfo.getParent().getType()).getName());
96+
dataFetchingPath.append('.');
9797
}
98-
dataFetchingType.append(executionStepInfo.getPath().getSegmentName());
99-
return Tag.of("path", dataFetchingType.toString());
98+
dataFetchingPath.append(executionStepInfo.getPath().getSegmentName());
99+
return Tag.of("path", dataFetchingPath.toString());
100100
}
101101

102102
}

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/graphql/GraphQlMetricsInstrumentationTests.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ void shouldRecordTimerWhenResult() {
8585

8686
Timer timer = this.registry.find("graphql.request").timer();
8787
assertThat(timer).isNotNull();
88-
assertThat(timer.takeSnapshot().count()).isEqualTo(1);
88+
assertThat(timer.count()).isEqualTo(1);
8989
}
9090

9191
@Test
@@ -120,7 +120,7 @@ void shouldRecordDataFetchingMetricWhenSuccess() throws Exception {
120120

121121
Timer timer = this.registry.find("graphql.datafetcher").timer();
122122
assertThat(timer).isNotNull();
123-
assertThat(timer.takeSnapshot().count()).isEqualTo(1);
123+
assertThat(timer.count()).isEqualTo(1);
124124
}
125125

126126
@Test
@@ -135,7 +135,7 @@ void shouldRecordDataFetchingMetricWhenSuccessCompletionStage() throws Exception
135135

136136
Timer timer = this.registry.find("graphql.datafetcher").timer();
137137
assertThat(timer).isNotNull();
138-
assertThat(timer.takeSnapshot().count()).isEqualTo(1);
138+
assertThat(timer.count()).isEqualTo(1);
139139
}
140140

141141
@Test
@@ -150,7 +150,7 @@ void shouldRecordDataFetchingMetricWhenError() throws Exception {
150150

151151
Timer timer = this.registry.find("graphql.datafetcher").timer();
152152
assertThat(timer).isNotNull();
153-
assertThat(timer.takeSnapshot().count()).isEqualTo(1);
153+
assertThat(timer.count()).isEqualTo(1);
154154
}
155155

156156
@Test

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/graphql/security/GraphQlWebMvcSecurityAutoConfigurationTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Except
168168
}
169169

170170
@Bean
171-
static InMemoryUserDetailsManager userDetailsService() {
171+
InMemoryUserDetailsManager userDetailsService() {
172172
User.UserBuilder userBuilder = User.withDefaultPasswordEncoder();
173173
UserDetails rob = userBuilder.username("rob").password("rob").roles("USER").build();
174174
UserDetails admin = userBuilder.username("admin").password("admin").roles("USER", "ADMIN").build();

spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/metrics.adoc

+1-1
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,7 @@ A single GraphQL query can involve many `DataFetcher` calls, so there is a dedic
905905

906906

907907
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.
908-
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.
908+
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.
909909
More options are available for <<application-properties#application-properties.actuator.management.metrics.distribution.maximum-expected-value, configuring distributions with application properties>>.
910910

911911
A single response can contain many GraphQL errors, counted by the `graphql.error` counter:

spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/spring-graphql.adoc

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ You can declare `RuntimeWiringConfigurer` beans in your Spring config to get acc
4949
Spring Boot detects such beans and adds them to the {spring-graphql-docs}#execution-graphqlsource[GraphQlSource builder].
5050

5151
Typically, however, applications will not implement `DataFetcher` directly and will instead create {spring-graphql-docs}#controllers[annotated controllers].
52-
Spring Boot will automatically register `@Controller` classes with annotated handler methods and registers those as `DataFetcher`s.
52+
Spring Boot will automatically detect `@Controller` classes with annotated handler methods and register those as `DataFetcher`s.
5353
Here's a sample implementation for our greeting query with a `@Controller` class:
5454

5555
[source,java,indent=0,subs="verbatim"]
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@
2525
import org.springframework.stereotype.Controller;
2626

2727
@Controller
28-
public class ProjectsController {
28+
public class ProjectController {
2929

3030
private final List<Project> projects;
3131

32-
public ProjectsController() {
32+
public ProjectController() {
3333
this.projects = Arrays.asList(new Project("spring-boot", "Spring Boot"),
3434
new Project("spring-graphql", "Spring GraphQL"), new Project("spring-framework", "Spring Framework"));
3535
}

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/main/java/smoketest/graphql/SecurityConfig.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@
2828

2929
import static org.springframework.security.config.Customizer.withDefaults;
3030

31-
@Configuration
31+
@Configuration(proxyBeanMethods = false)
3232
@EnableWebSecurity
3333
@EnableGlobalMethodSecurity(prePostEnabled = true)
3434
public class SecurityConfig {
3535

3636
@Bean
37-
DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Exception {
37+
public DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Exception {
3838
return http.csrf((csrf) -> csrf.disable())
3939
// Demonstrate that method security works
4040
// Best practice to use both for defense in depth
@@ -43,7 +43,7 @@ DefaultSecurityFilterChain springWebFilterChain(HttpSecurity http) throws Except
4343

4444
@Bean
4545
@SuppressWarnings("deprecation")
46-
public static InMemoryUserDetailsManager userDetailsService() {
46+
public InMemoryUserDetailsManager userDetailsService() {
4747
User.UserBuilder userBuilder = User.withDefaultPasswordEncoder();
4848
UserDetails rob = userBuilder.username("rob").password("rob").roles("USER").build();
4949
UserDetails admin = userBuilder.username("admin").password("admin").roles("USER", "ADMIN").build();

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-graphql/src/test/java/smoketest/graphql/ProjectControllerTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import org.springframework.boot.test.autoconfigure.graphql.GraphQlTest;
2323
import org.springframework.graphql.test.tester.GraphQlTester;
2424

25-
@GraphQlTest(ProjectsController.class)
25+
@GraphQlTest(ProjectController.class)
2626
class ProjectControllerTests {
2727

2828
@Autowired

0 commit comments

Comments
 (0)