Minor refactoring in RequestInput

Author: rstoyanchev

spring-graphql/src/main/java/org/springframework/graphql/RequestInput.java

@@ -32,10 +32,10 @@
 import org.springframework.util.CollectionUtils;
 
 /**
- * Common representation for GraphQL request input. This can be converted to
- * {@link ExecutionInput} via {@link #toExecutionInput(boolean)} and the
- * {@code ExecutionInput} further customized via
- * {@link #configureExecutionInput(BiFunction)}.
+ * Common, server-side representation of GraphQL request input independent of
+ * the underlying transport. This can be converted to {@link ExecutionInput}
+ * via {@link #toExecutionInput()} while the resulting {@code ExecutionInput}
+ * can be customized via {@link #configureExecutionInput(BiFunction)} callbacks.
  *
  * @author Rossen Stoyanchev
  * @author Brian Clozel
@@ -60,6 +60,7 @@ public class RequestInput {
 	@Nullable
 	private ExecutionId executionId;
 
+
 	/**
 	 * Create an instance.
 	 * @param query the query, mutation, or subscription for the request
@@ -83,17 +84,37 @@ public RequestInput(
 
 
 	/**
-	 * Return an identifier for the request. This id can be later propagated
-	 * as the {@link ExecutionId} if {@link #executionId(ExecutionId) none has been set}.
-	 * <p>For web transports, this identifier can be used to correlate
-	 * request and response messages on a multiplexed connection.
-	 * @return the request id.
+	 * Return the id for the request selected by the transport handler.
+	 * <ul>
+	 * <li>For Spring MVC, the id is generated via
+	 * {@link org.springframework.util.AlternativeJdkIdGenerator}, which is more
+	 * efficient than {@code UUID.randomUUID()}.
+	 * <li>For WebFlux the id is from the {@code ServerHttpRequest}, which is
+	 * useful to correlate to WebFlux log messages.
+	 * <li>For WebSocket, the id is from the {@code Subscribe} message of the
+	 * GraphQL over WebSocket protocol, which is useful to correlate to
+	 * WebSocket messages.
+	 * </ul>
+	 * <p> By default, the transport id becomes the
+	 * {@link ExecutionInput.Builder#executionId(ExecutionId) executionId} for
+	 * the GraphQL request. You can override this via
+	 * {@link #executionId(ExecutionId)} or by configuring an
+	 * {@link graphql.execution.ExecutionIdProvider} on {@link graphql.GraphQL}.
+	 * @return the request id
 	 * @see <a href="https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md">GraphQL over WebSocket Protocol</a>
 	 */
 	public String getId() {
 		return this.id;
 	}
 
+	/**
+	 * Return the {@code executionId} configured via {@link #executionId(ExecutionId)}.
+	 */
+	@Nullable
+	public ExecutionId getExecutionId() {
+		return this.executionId;
+	}
+
 	/**
 	 * Return the query, mutation, or subscription for the request.
 	 * @return the query, a non-empty string.
@@ -129,8 +150,10 @@ public Locale getLocale() {
 	}
 
 	/**
-	 * Set an {@link ExecutionId} to be used for the {@link ExecutionInput}
-	 * @param executionId the execution id to use with the {@link ExecutionInput}.
+	 * Configure the {@link ExecutionId} to use for the GraphQL request, which
+	 * is set on the {@link ExecutionInput}. This option overrides the
+	 * {@link #getId() id} selected by the transport handler.
+	 * @param executionId the execution id to set on the {@link ExecutionInput}.
 	 */
 	public void executionId(ExecutionId executionId) {
 		Assert.notNull(executionId, "executionId should not be null");
@@ -154,26 +177,21 @@ public void configureExecutionInput(BiFunction<ExecutionInput, ExecutionInput.Bu
 	 * populated from {@link #getQuery()}, {@link #getOperationName()}, and
 	 * {@link #getVariables()}, and is then further customized through
 	 * {@link #configureExecutionInput(BiFunction)}.
-	 * @param useRequestId whether the {@link #getId()} should be used as a fallback for {@link ExecutionId}.
 	 * @return the execution input
 	 */
-	public ExecutionInput toExecutionInput(boolean useRequestId) {
+	public ExecutionInput toExecutionInput() {
 		ExecutionInput.Builder inputBuilder = ExecutionInput.newExecutionInput()
 				.query(this.query)
 				.operationName(this.operationName)
 				.variables(this.variables)
-				.locale(this.locale);
-		if (this.executionId != null) {
-			inputBuilder.executionId(this.executionId);
-		}
-		else if (useRequestId) {
-			inputBuilder.executionId(ExecutionId.from(this.id));
-		}
+				.locale(this.locale)
+				.executionId(this.executionId != null ? this.executionId : ExecutionId.from(this.id));
+
 		ExecutionInput executionInput = inputBuilder.build();
 
 		for (BiFunction<ExecutionInput, ExecutionInput.Builder, ExecutionInput> configurer : this.executionInputConfigurers) {
 			ExecutionInput current = executionInput;
-			executionInput = executionInput.transform((builder) -> configurer.apply(current, builder));
+			executionInput = executionInput.transform(builder -> configurer.apply(current, builder));
 		}
 
 		return executionInput;

spring-graphql/src/main/java/org/springframework/graphql/execution/ExecutionGraphQlService.java

@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.BiFunction;
 
 import graphql.ExecutionInput;
 import graphql.GraphQL;
@@ -39,16 +40,21 @@
  */
 public class ExecutionGraphQlService implements GraphQlService {
 
+	private static final BiFunction<ExecutionInput, ExecutionInput.Builder, ExecutionInput> RESET_EXECUTION_ID_CONFIGURER =
+			(executionInput, builder) -> builder.executionId(null).build();
+
+
 	private final GraphQlSource graphQlSource;
 
 	private final List<DataLoaderRegistrar> dataLoaderRegistrars = new ArrayList<>();
 
-	private final boolean hasDefaultExecutionIdProvider;
+	private final boolean isDefaultExecutionIdProvider;
 
 
 	public ExecutionGraphQlService(GraphQlSource graphQlSource) {
 		this.graphQlSource = graphQlSource;
-		this.hasDefaultExecutionIdProvider = ExecutionIdProvider.DEFAULT_EXECUTION_ID_PROVIDER == graphQlSource.graphQl().getIdProvider();
+		this.isDefaultExecutionIdProvider =
+				(graphQlSource.graphQl().getIdProvider() == ExecutionIdProvider.DEFAULT_EXECUTION_ID_PROVIDER);
 	}
 
 
@@ -65,7 +71,10 @@ public void addDataLoaderRegistrar(DataLoaderRegistrar registrar) {
 	@Override
 	public final Mono<RequestOutput> execute(RequestInput requestInput) {
 		return Mono.deferContextual((contextView) -> {
-			ExecutionInput executionInput = requestInput.toExecutionInput(this.hasDefaultExecutionIdProvider);
+			if (!this.isDefaultExecutionIdProvider && requestInput.getExecutionId() == null) {
+				requestInput.configureExecutionInput(RESET_EXECUTION_ID_CONFIGURER);
+			}
+			ExecutionInput executionInput = requestInput.toExecutionInput();
 			ReactorContextManager.setReactorContext(contextView, executionInput);
 			ExecutionInput updatedExecutionInput = registerDataLoaders(executionInput);
 			return Mono.fromFuture(this.graphQlSource.graphQl().executeAsync(updatedExecutionInput))

spring-graphql/src/test/java/org/springframework/graphql/RequestInputTests.java

@@ -28,23 +28,19 @@
  */
 class RequestInputTests {
 
-	private RequestInput requestInput = new RequestInput("greeting", "Greeting", null, null, "id");
+	private final RequestInput requestInput = new RequestInput("greeting", "Greeting", null, null, "id");
 
-	@Test
-	void shouldUseCustomExecutionIdIfPresent() {
-		ExecutionId customId = ExecutionId.from("customId");
-		this.requestInput.executionId(customId);
-		assertThat(this.requestInput.toExecutionInput(true).getExecutionId()).isEqualTo(customId);
-		assertThat(this.requestInput.toExecutionInput(false).getExecutionId()).isEqualTo(customId);
-	}
 
 	@Test
-	void executionIdShouldFallBackToRequestId() {
-		assertThat(this.requestInput.toExecutionInput(true).getExecutionId()).isEqualTo(ExecutionId.from("id"));
+	void shouldUseRequestId() {
+		assertThat(this.requestInput.toExecutionInput().getExecutionId()).isEqualTo(ExecutionId.from("id"));
 	}
 
 	@Test
-	void executionIdShouldFallBackToProvider() {
-		assertThat(this.requestInput.toExecutionInput(false).getExecutionId()).isNull();
+	void shouldUseExecutionId() {
+		ExecutionId customId = ExecutionId.from("customId");
+		this.requestInput.executionId(customId);
+		assertThat(this.requestInput.toExecutionInput().getExecutionId()).isEqualTo(customId);
 	}
+
 }

spring-graphql/src/test/java/org/springframework/graphql/web/WebInterceptorTests.java

@@ -79,7 +79,7 @@ void executionInputCustomization() {
 
 		WebGraphQlHandler handler = WebGraphQlHandler
 				.builder((input) -> {
-					actualName.set(input.toExecutionInput(true).getOperationName());
+					actualName.set(input.toExecutionInput().getOperationName());
 					return emptyExecutionResult(input);
 				})
 				.interceptor((webInput, next) -> {