Make id mandatory for RequestInput

This commit is a follow up of 2e4158c and makes the following changes:

* the id is now a mandatory attribute for `WebInput` and `RequestInput`
* a consistent IdGenerator strategy is configured for all requests

Closes gh-183
This commit is contained in:
Brian Clozel
2021-11-25 12:30:56 +01:00
parent 9ce4b6801e
commit cfd0403008
13 changed files with 67 additions and 39 deletions

View File

@@ -21,7 +21,9 @@ import java.util.Map;
import org.springframework.graphql.RequestInput;
import org.springframework.lang.Nullable;
import org.springframework.util.AlternativeJdkIdGenerator;
import org.springframework.util.Assert;
import org.springframework.util.IdGenerator;
/**
* Base class support for implementations of
@@ -32,6 +34,8 @@ import org.springframework.util.Assert;
*/
class GraphQlTesterRequestSpecSupport {
private static final IdGenerator idGenerator = new AlternativeJdkIdGenerator();
private final String query;
@Nullable
@@ -66,7 +70,7 @@ class GraphQlTesterRequestSpecSupport {
}
protected RequestInput createRequestInput() {
return new RequestInput(this.query, this.operationName, this.variables, this.locale, null);
return new RequestInput(this.query, this.operationName, this.variables, this.locale, idGenerator.generateId().toString());
}
}

View File

@@ -38,6 +38,7 @@ import org.springframework.util.CollectionUtils;
* {@link #configureExecutionInput(BiFunction)}.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 1.0.0
*/
public class RequestInput {
@@ -47,14 +48,13 @@ public class RequestInput {
@Nullable
private final String operationName;
@Nullable
private final String id;
private final Map<String, Object> variables;
@Nullable
private final Locale locale;
private final String id;
private final List<BiFunction<ExecutionInput, ExecutionInput.Builder, ExecutionInput>> executionInputConfigurers = new ArrayList<>();
@@ -64,13 +64,14 @@ public class RequestInput {
* @param operationName an optional, explicit name assigned to the query
* @param variables variables by which the query is parameterized
* @param locale the locale associated with the request, if any
* @param id an optional request id, to be used as the execution id
* @param id the request id, to be used as the {@link ExecutionId}
*/
public RequestInput(
String query, @Nullable String operationName, @Nullable Map<String, Object> variables,
@Nullable Locale locale, @Nullable String id) {
@Nullable Locale locale, String id) {
Assert.notNull(query, "'query' is required");
Assert.notNull(id, "'id' is required");
this.query = query;
this.operationName = operationName;
this.variables = ((variables != null) ? variables : Collections.emptyMap());
@@ -86,9 +87,8 @@ public class RequestInput {
/**
* Return the explicitly assigned request id.
* @return the request id or {@code null}.
* @return the request id.
*/
@Nullable
public String getId() {
return this.id;
}
@@ -152,7 +152,7 @@ public class RequestInput {
.operationName(this.operationName)
.variables(this.variables)
.locale(this.locale)
.executionId((this.id != null) ? ExecutionId.from(this.id) : null)
.executionId(ExecutionId.from(this.id))
.build();
for (BiFunction<ExecutionInput, ExecutionInput.Builder, ExecutionInput> configurer : this.executionInputConfigurers) {

View File

@@ -24,7 +24,6 @@ import org.springframework.graphql.RequestInput;
import org.springframework.http.HttpHeaders;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.util.UriComponents;
@@ -57,7 +56,7 @@ public class WebInput extends RequestInput {
*/
public WebInput(
URI uri, HttpHeaders headers, Map<String, Object> body,
@Nullable Locale locale, @Nullable String id) {
@Nullable Locale locale, String id) {
super(getKey("query", body), getKey("operationName", body), getKey("variables", body), locale, id);
Assert.notNull(uri, "URI is required'");
@@ -89,14 +88,4 @@ public class WebInput extends RequestInput {
return this.headers;
}
/**
* Return an identifier for the request. This is useful to correlate
* request and response messages on a multiplexed connection.
* @see <a href="https://github.com/enisdenjo/graphql-ws/blob/master/PROTOCOL.md">GraphQL over WebSocket Protocol</a>
*/
@Override
public String getId() {
return (super.getId() != null) ? super.getId() : ObjectUtils.getIdentityHexString(this);
}
}

View File

@@ -25,7 +25,9 @@ import reactor.core.publisher.Mono;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.graphql.web.WebGraphQlHandler;
import org.springframework.graphql.web.WebInput;
import org.springframework.util.AlternativeJdkIdGenerator;
import org.springframework.util.Assert;
import org.springframework.util.IdGenerator;
import org.springframework.web.reactive.function.server.ServerRequest;
import org.springframework.web.reactive.function.server.ServerResponse;
@@ -33,6 +35,7 @@ import org.springframework.web.reactive.function.server.ServerResponse;
* WebFlux.fn Handler for GraphQL over HTTP requests.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 1.0.0
*/
public class GraphQlHttpHandler {
@@ -40,17 +43,31 @@ public class GraphQlHttpHandler {
private static final Log logger = LogFactory.getLog(GraphQlHttpHandler.class);
private static final ParameterizedTypeReference<Map<String, Object>> MAP_PARAMETERIZED_TYPE_REF =
new ParameterizedTypeReference<Map<String, Object>>() {};
new ParameterizedTypeReference<Map<String, Object>>() {
};
private final WebGraphQlHandler graphQlHandler;
private final IdGenerator idGenerator;
/**
* Create a new instance.
* @param graphQlHandler common handler for GraphQL over HTTP requests
*/
public GraphQlHttpHandler(WebGraphQlHandler graphQlHandler) {
this(graphQlHandler, new AlternativeJdkIdGenerator());
}
/**
* Create a new instance.
* @param graphQlHandler common handler for GraphQL over HTTP requests
* @param idGenerator Id generator for requests
*/
public GraphQlHttpHandler(WebGraphQlHandler graphQlHandler, IdGenerator idGenerator) {
Assert.notNull(graphQlHandler, "WebGraphQlHandler is required");
Assert.notNull(idGenerator, "IdGenerator is required");
this.graphQlHandler = graphQlHandler;
this.idGenerator = idGenerator;
}
/**
@@ -64,7 +81,7 @@ public class GraphQlHttpHandler {
WebInput input = new WebInput(
request.uri(), request.headers().asHttpHeaders(), body,
request.exchange().getLocaleContext().getLocale(),
request.exchange().getRequest().getId());
this.idGenerator.generateId().toString());
if (logger.isDebugEnabled()) {
logger.debug("Executing: " + input);
}

View File

@@ -29,8 +29,9 @@ import org.springframework.context.i18n.LocaleContextHolder;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.graphql.web.WebGraphQlHandler;
import org.springframework.graphql.web.WebInput;
import org.springframework.util.AlternativeJdkIdGenerator;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.IdGenerator;
import org.springframework.web.HttpMediaTypeNotSupportedException;
import org.springframework.web.server.ServerWebInputException;
import org.springframework.web.servlet.function.ServerRequest;
@@ -53,13 +54,26 @@ public class GraphQlHttpHandler {
private final WebGraphQlHandler graphQlHandler;
private final IdGenerator idGenerator;
/**
* Create a new instance.
* @param graphQlHandler common handler for GraphQL over HTTP requests
*/
public GraphQlHttpHandler(WebGraphQlHandler graphQlHandler) {
this(graphQlHandler, new AlternativeJdkIdGenerator());
}
/**
* Create a new instance.
* @param graphQlHandler common handler for GraphQL over HTTP requests
* @param idGenerator Id generator for requests
*/
public GraphQlHttpHandler(WebGraphQlHandler graphQlHandler, IdGenerator idGenerator) {
Assert.notNull(graphQlHandler, "WebGraphQlHandler is required");
Assert.notNull(idGenerator, "IdGenerator is required");
this.graphQlHandler = graphQlHandler;
this.idGenerator = idGenerator;
}
/**
@@ -73,7 +87,7 @@ public class GraphQlHttpHandler {
WebInput input = new WebInput(
request.uri(), request.headers().asHttpHeaders(), readBody(request),
LocaleContextHolder.getLocale(), ObjectUtils.getIdentityHexString(request));
LocaleContextHolder.getLocale(), this.idGenerator.generateId().toString());
if (logger.isDebugEnabled()) {
logger.debug("Executing: " + input);

View File

@@ -71,7 +71,7 @@ public class BatchMappingInvocationTests extends BatchMappingTestSupport {
"}";
Mono<ExecutionResult> resultMono = createGraphQlService(controller)
.execute(new RequestInput(query, null, null, null, null));
.execute(new RequestInput(query, null, null, null, "1"));
List<Course> actualCourses = GraphQlResponse.from(resultMono).toList("courses", Course.class);
List<Course> courses = Course.allCourses();
@@ -104,7 +104,7 @@ public class BatchMappingInvocationTests extends BatchMappingTestSupport {
"}";
Mono<ExecutionResult> resultMono = createGraphQlService(controller)
.execute(new RequestInput(query, null, null, null, null));
.execute(new RequestInput(query, null, null, null, "1"));
List<Course> actualCourses = GraphQlResponse.from(resultMono).toList("courses", Course.class);
List<Course> courses = Course.allCourses();

View File

@@ -95,7 +95,7 @@ public class BatchMappingPrincipalMethodArgumentResolverTests extends BatchMappi
Mono<ExecutionResult> resultMono = Mono.delay(Duration.ofMillis(10))
.flatMap(aLong -> {
String query = "{ courses { id instructor { id } } }";
return createGraphQlService(controller).execute(new RequestInput(query, null, null, null, null));
return createGraphQlService(controller).execute(new RequestInput(query, null, null, null, "1"));
})
.contextWrite(contextWriter);

View File

@@ -68,7 +68,7 @@ public class SchemaMappingInvocationTests {
" }" +
"}";
Mono<ExecutionResult> resultMono = graphQlService().execute(new RequestInput(query, null, null, null, null));
Mono<ExecutionResult> resultMono = graphQlService().execute(new RequestInput(query, null, null, null, "1"));
Book book = GraphQlResponse.from(resultMono).toEntity("bookById", Book.class);
assertThat(book.getId()).isEqualTo(1);
@@ -88,7 +88,7 @@ public class SchemaMappingInvocationTests {
" }" +
"}";
Mono<ExecutionResult> resultMono = graphQlService().execute(new RequestInput(query, null, null, null, null));
Mono<ExecutionResult> resultMono = graphQlService().execute(new RequestInput(query, null, null, null, "1"));
List<Book> bookList = GraphQlResponse.from(resultMono).toList("booksByCriteria", Book.class);
assertThat(bookList).hasSize(2);
@@ -107,7 +107,7 @@ public class SchemaMappingInvocationTests {
"}";
AtomicReference<GraphQLContext> contextRef = new AtomicReference<>();
RequestInput requestInput = new RequestInput(query, null, null, null, null);
RequestInput requestInput = new RequestInput(query, null, null, null, "1");
requestInput.configureExecutionInput((executionInput, builder) -> {
contextRef.set(executionInput.getGraphQLContext());
return executionInput;
@@ -134,7 +134,7 @@ public class SchemaMappingInvocationTests {
"}";
Mono<ExecutionResult> resultMono = graphQlService()
.execute(new RequestInput(operation, null, null, null, null));
.execute(new RequestInput(operation, null, null, null, "1"));
Author author = GraphQlResponse.from(resultMono).toEntity("addAuthor", Author.class);
assertThat(author.getId()).isEqualTo(99);
@@ -152,7 +152,7 @@ public class SchemaMappingInvocationTests {
"}";
Mono<ExecutionResult> resultMono = graphQlService()
.execute(new RequestInput(operation, null, null, null, null));
.execute(new RequestInput(operation, null, null, null, "1"));
Flux<Book> bookFlux = GraphQlResponse.forSubscription(resultMono)
.map(response -> response.toEntity("bookSearch", Book.class));

View File

@@ -163,7 +163,7 @@ public class SchemaMappingPrincipalMethodArgumentResolverTests {
.toGraphQlService();
return Mono.delay(Duration.ofMillis(10))
.flatMap(aLong -> graphQlService.execute(new RequestInput(op, null, null, null, null)))
.flatMap(aLong -> graphQlService.execute(new RequestInput(op, null, null, null, "1")))
.contextWrite(contextWriter);
}

View File

@@ -76,7 +76,7 @@ public class BatchLoadingTests {
.dataLoaders(this.registry)
.toGraphQlService();
Mono<ExecutionResult> resultMono = service.execute(new RequestInput(query, null, null, null, null));
Mono<ExecutionResult> resultMono = service.execute(new RequestInput(query, null, null, null, "1"));
List<Book> books = GraphQlResponse.from(resultMono).toList("booksByCriteria", Book.class);
assertThat(books).hasSize(2);

View File

@@ -85,7 +85,7 @@ public class ClassNameTypeResolverTests {
Mono<ExecutionResult> resultMono = graphQlSetup.queryFetcher("animals", env -> animalList)
.toGraphQlService()
.execute(new RequestInput(query, null, null, null, null));
.execute(new RequestInput(query, null, null, null, "1"));
GraphQlResponse response = GraphQlResponse.from(resultMono);
for (int i = 0; i < animalList.size(); i++) {
@@ -128,7 +128,7 @@ public class ClassNameTypeResolverTests {
Mono<ExecutionResult> result = graphQlSetup.queryFetcher("sightings", env -> animalAndPlantList)
.typeResolver(typeResolver)
.toGraphQlService()
.execute(new RequestInput(query, null, null, null, null));
.execute(new RequestInput(query, null, null, null, "1"));
GraphQlResponse response = GraphQlResponse.from(result);
for (int i = 0; i < animalAndPlantList.size(); i++) {

View File

@@ -19,6 +19,7 @@ import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.UUID;
import com.jayway.jsonpath.DocumentContext;
import com.jayway.jsonpath.JsonPath;
@@ -38,6 +39,7 @@ import org.springframework.web.reactive.result.view.ViewResolver;
import org.springframework.web.server.ServerWebExchange;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
/**
* Tests for {@link GraphQlHttpHandler}.
@@ -74,7 +76,7 @@ public class GraphQlHttpHandlerTests {
DocumentContext document = JsonPath.parse(httpResponse.getBodyAsString().block());
String id = document.read("data.showId", String.class);
assertThat(id).isEqualTo(httpRequest.getId());
assertThatNoException().isThrownBy(() -> UUID.fromString(id));
}
private MockServerHttpResponse handleRequest(

View File

@@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import javax.servlet.ServletException;
@@ -38,6 +39,7 @@ import org.springframework.web.servlet.function.ServerRequest;
import org.springframework.web.servlet.function.ServerResponse;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNoException;
/**
* Tests for {@link GraphQlHttpHandler}.
@@ -79,7 +81,7 @@ public class GraphQlHttpHandlerTests {
MockHttpServletResponse servletResponse = handleRequest(servletRequest, handler);
DocumentContext document = JsonPath.parse(servletResponse.getContentAsString());
String id = document.read("data.showId", String.class);
assertThat(id).hasSize(8);
assertThatNoException().isThrownBy(() -> UUID.fromString(id));
}
private MockHttpServletRequest createServletRequest(String query) {