From f0f883cc3979678970d352e7bdeebe6be2ffd1aa Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 30 Sep 2021 14:39:21 +0100 Subject: [PATCH] Inline RequestSpecDelegate in DefaultGraphQlTester The delegate has almost no logic, simply holding fields and calling the RequestStrategy, so it adds cognitive overhead without benefit. --- .../test/tester/DefaultGraphQlTester.java | 88 +++++-------------- .../test/tester/DefaultWebGraphQlTester.java | 38 +++++--- 2 files changed, 49 insertions(+), 77 deletions(-) diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java index ed1dfcbd..67f165dc 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultGraphQlTester.java @@ -114,9 +114,9 @@ class DefaultGraphQlTester implements GraphQlTester { public GraphQlTester build() { return new DefaultGraphQlTester(this.service, this.builderConfig); } - } + /** * Internal strategy abstracting how a GraphQL request is performed. */ @@ -138,6 +138,7 @@ class DefaultGraphQlTester implements GraphQlTester { } + /** * Base class for a {@link RequestStrategy} that perform GraphQL requests * without an underlying transport and where {@link RequestInput} provides @@ -199,9 +200,9 @@ class DefaultGraphQlTester implements GraphQlTester { } }; } - } + /** * {@link RequestStrategy} that performs requests through a {@link GraphQlService}. */ @@ -222,11 +223,11 @@ class DefaultGraphQlTester implements GraphQlTester { } } + /** - * Assist with collecting the input for {@link GraphQlTester.RequestSpec}, - * helping to avoid challenges with generics in the builder hierarchy. + * {@link RequestSpec} that collects the query, operationName, and variables. */ - final static class RequestSpecDelegate { + static class DefaultRequestSpec implements RequestSpec { private final RequestStrategy requestStrategy; @@ -237,92 +238,46 @@ class DefaultGraphQlTester implements GraphQlTester { private final Map variables = new LinkedHashMap<>(); - protected RequestSpecDelegate(RequestStrategy requestStrategy, String query) { + protected DefaultRequestSpec(RequestStrategy requestStrategy, String query) { Assert.notNull(requestStrategy, "RequestStrategy is required"); Assert.notNull(query, "`query` is required"); this.requestStrategy = requestStrategy; this.query = query; } - public void operationName(@Nullable String name) { - this.operationName = name; - } - - public void variable(String name, Object value) { - this.variables.put(name, value); - } - - public ResponseSpec execute() { - return execute(createRequestInput()); - } - - public ResponseSpec execute(RequestInput input) { - return this.requestStrategy.execute(input); - } - - public void executeAndVerify() { - executeAndVerify(createRequestInput()); - } - - public void executeAndVerify(RequestInput input) { - ResponseSpec spec = this.requestStrategy.execute(input); - spec.path("$.errors").valueIsEmpty(); - } - - public SubscriptionSpec executeSubscription() { - return executeSubscription(createRequestInput()); - } - - public SubscriptionSpec executeSubscription(RequestInput input) { - return this.requestStrategy.executeSubscription(input); - } - - public RequestInput createRequestInput() { - return new RequestInput(this.query, this.operationName, this.variables); - } - - } - - /** - * {@link RequestSpec} that collects the query, operationName, and variables. - */ - static class DefaultRequestSpec implements RequestSpec { - - private final RequestSpecDelegate delegate; - - protected DefaultRequestSpec(RequestStrategy requestStrategy, String query) { - this.delegate = new RequestSpecDelegate(requestStrategy, query); - } - @Override public DefaultRequestSpec operationName(@Nullable String name) { - this.delegate.operationName(name); + this.operationName = name; return this; } @Override public DefaultRequestSpec variable(String name, Object value) { - this.delegate.variable(name, value); + this.variables.put(name, value); return this; } @Override public ResponseSpec execute() { - return this.delegate.execute(); + return this.requestStrategy.execute(createRequestInput()); } @Override public void executeAndVerify() { - this.delegate.executeAndVerify(); + execute().path("$.errors").valueIsEmpty(); } @Override public SubscriptionSpec executeSubscription() { - return this.delegate.executeSubscription(); + return this.requestStrategy.executeSubscription(createRequestInput()); } + public RequestInput createRequestInput() { + return new RequestInput(this.query, this.operationName, this.variables); + } } + private static class ErrorsContainer { private static final Predicate MATCH_ALL_PREDICATE = (error) -> true; @@ -374,9 +329,9 @@ class DefaultGraphQlTester implements GraphQlTester { + unexpected, CollectionUtils.isEmpty(unexpected))); } - } + /** * Container for a GraphQL response with access to data and errors. */ @@ -426,9 +381,9 @@ class DefaultGraphQlTester implements GraphQlTester { T read(JsonPath jsonPath, TypeRef typeRef) { return this.documentContext.read(jsonPath, typeRef); } - } + /** * {@link ResponseSpec} that operates on the response from a GraphQL HTTP request. */ @@ -614,9 +569,9 @@ class DefaultGraphQlTester implements GraphQlTester { } }); } - } + /** * {@link EntitySpec} implementation. */ @@ -697,9 +652,9 @@ class DefaultGraphQlTester implements GraphQlTester { private T self() { return (T) this; } - } + /** * {@link ListEntitySpec} implementation. */ @@ -767,9 +722,9 @@ class DefaultGraphQlTester implements GraphQlTester { (getEntity() != null && getEntity().size() > boundary))); return this; } - } + /** * {@link SubscriptionSpec} implementation that operates on a {@link Publisher} of * {@link ExecutionResult}. @@ -802,7 +757,6 @@ class DefaultGraphQlTester implements GraphQlTester { return new DefaultResponseSpec(context, this.errorFilter, this.assertDecorator); }); } - } } diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultWebGraphQlTester.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultWebGraphQlTester.java index 51d0c12e..d5206922 100644 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultWebGraphQlTester.java +++ b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/DefaultWebGraphQlTester.java @@ -19,6 +19,8 @@ package org.springframework.graphql.test.tester; import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.function.Supplier; @@ -228,12 +230,22 @@ class DefaultWebGraphQlTester extends DefaultGraphQlTester implements WebGraphQl private static final URI DEFAULT_URL = URI.create(""); - private final RequestSpecDelegate delegate; + private final RequestStrategy requestStrategy; + + private final String query; + + @Nullable + private String operationName; + + private final Map variables = new LinkedHashMap<>(); private final HttpHeaders headers = new HttpHeaders(); DefaultWebRequestSpec(RequestStrategy requestStrategy, String query, @Nullable HttpHeaders headers) { - this.delegate = new RequestSpecDelegate(requestStrategy, query); + Assert.notNull(requestStrategy, "RequestStrategy is required"); + Assert.notNull(query, "`query` is required"); + this.requestStrategy = requestStrategy; + this.query = query; if (!CollectionUtils.isEmpty(headers)) { this.headers.putAll(headers); } @@ -255,36 +267,42 @@ class DefaultWebGraphQlTester extends DefaultGraphQlTester implements WebGraphQl @Override public WebRequestSpec operationName(@Nullable String name) { - this.delegate.operationName(name); + this.operationName = name; return this; } @Override public WebRequestSpec variable(String name, Object value) { - this.delegate.variable(name, value); + this.variables.put(name, value); return this; } @Override public ResponseSpec execute() { - return this.delegate.execute(createRequestInput()); + return this.requestStrategy.execute(createRequestInput()); } @Override public void executeAndVerify() { - this.delegate.executeAndVerify(createRequestInput()); + execute().path("$.errors").valueIsEmpty(); } @Override public SubscriptionSpec executeSubscription() { - return this.delegate.executeSubscription(createRequestInput()); + return this.requestStrategy.executeSubscription(createRequestInput()); } private RequestInput createRequestInput() { - RequestInput requestInput = this.delegate.createRequestInput(); - return new WebInput(DEFAULT_URL, this.headers, requestInput.toMap(), null); + Map body = new LinkedHashMap<>(3); + body.put("query", this.query); + if (this.operationName != null) { + body.put("operationName", this.operationName); + } + if (!CollectionUtils.isEmpty(this.variables)) { + body.put("variables", new LinkedHashMap<>(this.variables)); + } + return new WebInput(DEFAULT_URL, this.headers, body, null); } - } }