From d063eb760a96c582f5b6e07ce5c9b0eea9be8a19 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 4 Mar 2022 14:39:59 +0000 Subject: [PATCH] Simplify filtered error handling in GraphQlTester See gh-317 --- .../test/tester/DefaultGraphQlTester.java | 94 ++++---- .../graphql/test/tester/TestGraphQlError.java | 211 ------------------ .../graphql/test/tester/TypeRefAdapter.java | 60 ----- .../test/tester/GraphQlTesterTests.java | 4 +- .../graphql/client/DefaultGraphQlClient.java | 33 +++ .../graphql/client/TypeRefAdapter.java | 60 ----- .../graphql/support/MapGraphQlError.java | 3 +- 7 files changed, 90 insertions(+), 375 deletions(-) delete mode 100644 spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TestGraphQlError.java delete mode 100644 spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TypeRefAdapter.java delete mode 100644 spring-graphql/src/main/java/org/springframework/graphql/client/TypeRefAdapter.java 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 836a6cc4..2f631297 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 @@ -16,27 +16,26 @@ package org.springframework.graphql.test.tester; +import java.lang.reflect.Type; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.function.Supplier; -import java.util.stream.Collectors; import com.jayway.jsonpath.Configuration; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; -import com.jayway.jsonpath.PathNotFoundException; import com.jayway.jsonpath.TypeRef; import graphql.ExecutionResult; import graphql.GraphQLError; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.ResolvableType; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.client.GraphQlTransport; import org.springframework.graphql.support.DocumentSource; @@ -201,18 +200,13 @@ final class DefaultGraphQlTester implements GraphQlTester { */ private final static class ResponseDelegate { - private static final TypeRef> ERROR_LIST_TYPE = new TypeRef>() {}; - - private static final JsonPath ERRORS_PATH = JsonPath.compile("$.errors"); - - private static final Predicate MATCH_ALL_PREDICATE = (error) -> true; - - private final DocumentContext jsonDoc; private final Supplier jsonContent; - private final List errors; + private final List errors; + + private final List unexpectedErrors; private final Consumer assertDecorator; @@ -223,18 +217,12 @@ final class DefaultGraphQlTester implements GraphQlTester { this.jsonDoc = JsonPath.parse(result.toSpecification(), jsonPathConfig); this.jsonContent = this.jsonDoc::jsonString; - this.errors = readErrors(this.jsonDoc); + this.errors = result.getErrors(); + this.unexpectedErrors = new ArrayList<>(this.errors); this.assertDecorator = assertDecorator; - filterErrors(errorFilter); - } - - private static List readErrors(DocumentContext documentContext) { - try { - return documentContext.read(ERRORS_PATH, ERROR_LIST_TYPE); - } - catch (PathNotFoundException ex) { - return Collections.emptyList(); + if (errorFilter != null) { + filterErrors(errorFilter); } } @@ -260,39 +248,34 @@ final class DefaultGraphQlTester implements GraphQlTester { this.assertDecorator.accept(task); } - boolean filterErrors(@Nullable Predicate predicate) { + boolean filterErrors(Predicate predicate) { boolean filtered = false; - if (predicate != null) { - for (TestGraphQlError error : this.errors) { - filtered |= error.apply(predicate); + for (GraphQLError error : this.errors) { + if (predicate.test(error)) { + this.unexpectedErrors.remove(error); + filtered = true; } } return filtered; } - void expectErrors(@Nullable Predicate predicate) { + void expectErrors(Predicate predicate) { boolean filtered = filterErrors(predicate); this.assertDecorator.accept(() -> AssertionErrors.assertTrue("No matching errors.", filtered)); } void consumeErrors(Consumer> consumer) { - filterErrors(MATCH_ALL_PREDICATE); - consumer.accept(new ArrayList<>(this.errors)); + filterErrors(error -> true); + consumer.accept(this.errors); } void verifyErrors() { - List unexpected = this.errors.stream() - .filter(error -> !error.isExpected()) - .collect(Collectors.toList()); - - this.assertDecorator - .accept(() -> AssertionErrors.assertTrue( - "Response has " + unexpected.size() + " unexpected error(s)" - + ((unexpected.size() != this.errors.size()) - ? " of " + this.errors.size() + " total" : "") - + ". " + "If expected, please use ResponseSpec#errors to filter them out: " - + unexpected, - CollectionUtils.isEmpty(unexpected))); + this.assertDecorator.accept(() -> + AssertionErrors.assertTrue( + "Response has " + this.unexpectedErrors.size() + " unexpected error(s) " + + "of " + this.errors.size() + " total. " + + "If expected, please filter them out: " + this.unexpectedErrors, + CollectionUtils.isEmpty(this.unexpectedErrors))); } } @@ -630,4 +613,35 @@ final class DefaultGraphQlTester implements GraphQlTester { } } + + /** + * Adapt JSONPath {@link TypeRef} to {@link ParameterizedTypeReference}. + */ + private static final class TypeRefAdapter extends TypeRef { + + private final Type type; + + TypeRefAdapter(Class clazz) { + this.type = clazz; + } + + TypeRefAdapter(ParameterizedTypeReference typeReference) { + this.type = typeReference.getType(); + } + + TypeRefAdapter(Class clazz, Class generic) { + this.type = ResolvableType.forClassWithGenerics(clazz, generic).getType(); + } + + TypeRefAdapter(Class clazz, ParameterizedTypeReference generic) { + this.type = ResolvableType.forClassWithGenerics(clazz, ResolvableType.forType(generic)).getType(); + } + + @Override + public Type getType() { + return this.type; + } + + } + } diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TestGraphQlError.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TestGraphQlError.java deleted file mode 100644 index d7359e55..00000000 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TestGraphQlError.java +++ /dev/null @@ -1,211 +0,0 @@ -/* - * Copyright 2002-2021 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.graphql.test.tester; - -import java.util.List; -import java.util.Map; -import java.util.function.Predicate; -import java.util.stream.Collectors; - -import graphql.ErrorClassification; -import graphql.GraphQLError; -import graphql.GraphqlErrorBuilder; -import graphql.execution.ResultPath; -import graphql.language.SourceLocation; - -import org.springframework.lang.Nullable; - -/** - * {@link GraphQLError} with setters, for internal use to deserialize from a response. - * - * @author Rossen Stoyanchev - */ -@SuppressWarnings("serial") -final class TestGraphQlError implements GraphQLError { - - @Nullable - private String message; - - @Nullable - private List locations; - - @Nullable - private List path; - - @Nullable - private Map extensions; - - private boolean expected; - - @SuppressWarnings("unused") - void setMessage(String message) { - this.message = message; - } - - @Override - @Nullable - public String getMessage() { - return this.message; - } - - @SuppressWarnings("unused") - void setLocations(List locations) { - this.locations = TestSourceLocation.toSourceLocations(locations); - } - - @Override - @Nullable - public List getLocations() { - return this.locations; - } - - @Override - @Nullable - public ErrorClassification getErrorType() { - // Attempt the reverse of how errorType is serialized in GraphqlErrorHelper.toSpecification. - // However we can only do that for ErrorClassification enums that we know of. - String value = (getExtensions() != null ? (String) getExtensions().get("classification") : null); - if (value != null) { - try { - return graphql.ErrorType.valueOf(value); - } - catch (IllegalArgumentException ex) { - // ignore - } - try { - return org.springframework.graphql.execution.ErrorType.valueOf(value); - } - catch (IllegalArgumentException ex) { - // ignore - } - } - return null; - } - - @SuppressWarnings("unused") - void setPath(List path) { - this.path = path; - } - - @Override - @Nullable - public List getPath() { - return this.path; - } - - @SuppressWarnings("unused") - void setExtensions(Map extensions) { - this.extensions = extensions; - } - - @Override - @Nullable - public Map getExtensions() { - return this.extensions; - } - - /** - * Whether the error is marked as filtered out as expected. - * @return whether the error is marked as expected - */ - boolean isExpected() { - return this.expected; - } - - @Override - public Map toSpecification() { - GraphqlErrorBuilder builder = GraphqlErrorBuilder.newError(); - if (this.message != null) { - builder.message(this.message); - } - if (this.locations != null) { - this.locations.forEach(builder::location); - } - if (this.path != null) { - builder.path(ResultPath.fromList(this.path)); - } - if (this.extensions != null) { - builder.extensions(this.extensions); - } - return builder.build().toSpecification(); - } - - /** - * Mark this error as expected if it matches the predicate. - * @param predicate the error predicate - * @return whether the predicate matched - */ - boolean apply(Predicate predicate) { - boolean match = predicate.test(this); - this.expected |= match; - return match; - } - - @Override - public String toString() { - return toSpecification().toString(); - } - - private static final class TestSourceLocation { - - private int line; - - private int column; - - @Nullable - private String sourceName; - - @SuppressWarnings("unused") - void setLine(int line) { - this.line = line; - } - - @SuppressWarnings("unused") - int getLine() { - return this.line; - } - - @SuppressWarnings("unused") - void setColumn(int column) { - this.column = column; - } - - @SuppressWarnings("unused") - int getColumn() { - return this.column; - } - - @SuppressWarnings("unused") - void setSourceName(String sourceName) { - this.sourceName = sourceName; - } - - @Nullable - @SuppressWarnings("unused") - String getSourceName() { - return this.sourceName; - } - - static List toSourceLocations(List locations) { - return locations.stream() - .map(loc -> new SourceLocation(loc.line, loc.column, loc.sourceName)) - .collect(Collectors.toList()); - } - - } - -} diff --git a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TypeRefAdapter.java b/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TypeRefAdapter.java deleted file mode 100644 index b150f366..00000000 --- a/spring-graphql-test/src/main/java/org/springframework/graphql/test/tester/TypeRefAdapter.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2002-2021 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.graphql.test.tester; - -import java.lang.reflect.Type; - -import com.jayway.jsonpath.TypeRef; - -import org.springframework.core.ParameterizedTypeReference; -import org.springframework.core.ResolvableType; - -/** - * Adapt a JSONPath {@link TypeRef} to {@link ParameterizedTypeReference} and - * {@link ResolvableType} for classes with generics. - * - * @author Rossen Stoyanchev - * @since 1.0.0 - */ -final class TypeRefAdapter extends TypeRef { - - private final Type type; - - - TypeRefAdapter(Class clazz) { - this.type = clazz; - } - - TypeRefAdapter(ParameterizedTypeReference typeReference) { - this.type = typeReference.getType(); - } - - TypeRefAdapter(Class clazz, Class generic) { - this.type = ResolvableType.forClassWithGenerics(clazz, generic).getType(); - } - - TypeRefAdapter(Class clazz, ParameterizedTypeReference generic) { - this.type = ResolvableType.forClassWithGenerics(clazz, ResolvableType.forType(generic)).getType(); - } - - - @Override - public Type getType() { - return this.type; - } - -} diff --git a/spring-graphql-test/src/test/java/org/springframework/graphql/test/tester/GraphQlTesterTests.java b/spring-graphql-test/src/test/java/org/springframework/graphql/test/tester/GraphQlTesterTests.java index 9ebf9964..d0114e8b 100644 --- a/spring-graphql-test/src/test/java/org/springframework/graphql/test/tester/GraphQlTesterTests.java +++ b/spring-graphql-test/src/test/java/org/springframework/graphql/test/tester/GraphQlTesterTests.java @@ -193,7 +193,7 @@ public class GraphQlTesterTests extends GraphQlTesterTestSupport { setMockResponse(GraphqlErrorBuilder.newError().message("Invalid query").build()); assertThatThrownBy(() -> graphQlTester().document(document).executeAndVerify()) - .hasMessageContaining("Response has 1 unexpected error(s)."); + .hasMessageContaining("Response has 1 unexpected error(s)"); assertThat(requestInput().getDocument()).contains(document); } @@ -205,7 +205,7 @@ public class GraphQlTesterTests extends GraphQlTesterTestSupport { setMockResponse(GraphqlErrorBuilder.newError().message("Invalid query").build()); assertThatThrownBy(() -> graphQlTester().document(document).execute().path("me")) - .hasMessageContaining("Response has 1 unexpected error(s)."); + .hasMessageContaining("Response has 1 unexpected error(s)"); assertThat(requestInput().getDocument()).contains(document); } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java index 1bdb2cb8..ae4811a6 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/client/DefaultGraphQlClient.java @@ -15,6 +15,7 @@ */ package org.springframework.graphql.client; +import java.lang.reflect.Type; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -23,12 +24,14 @@ import java.util.function.Consumer; import com.jayway.jsonpath.Configuration; import com.jayway.jsonpath.DocumentContext; import com.jayway.jsonpath.JsonPath; +import com.jayway.jsonpath.TypeRef; import graphql.ExecutionResult; import graphql.GraphQLError; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.ResolvableType; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.support.DocumentSource; import org.springframework.lang.Nullable; @@ -229,4 +232,34 @@ final class DefaultGraphQlClient implements GraphQlClient { } + + /** + * Adapt JSONPath {@link TypeRef} to {@link ParameterizedTypeReference}. + */ + private static final class TypeRefAdapter extends TypeRef { + + private final Type type; + + TypeRefAdapter(Class clazz) { + this.type = clazz; + } + + TypeRefAdapter(ParameterizedTypeReference typeReference) { + this.type = typeReference.getType(); + } + + TypeRefAdapter(Class clazz, Class generic) { + this.type = ResolvableType.forClassWithGenerics(clazz, generic).getType(); + } + + TypeRefAdapter(Class clazz, ParameterizedTypeReference generic) { + this.type = ResolvableType.forClassWithGenerics(clazz, ResolvableType.forType(generic)).getType(); + } + + @Override + public Type getType() { + return this.type; + } + + } } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/client/TypeRefAdapter.java b/spring-graphql/src/main/java/org/springframework/graphql/client/TypeRefAdapter.java deleted file mode 100644 index 44ba56c0..00000000 --- a/spring-graphql/src/main/java/org/springframework/graphql/client/TypeRefAdapter.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2002-2022 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.graphql.client; - -import java.lang.reflect.Type; - -import com.jayway.jsonpath.TypeRef; - -import org.springframework.core.ParameterizedTypeReference; -import org.springframework.core.ResolvableType; - -/** - * Adapt a JSONPath {@link TypeRef} to {@link ParameterizedTypeReference} and - * {@link ResolvableType} for classes with generics. - * - * @author Rossen Stoyanchev - * @since 1.0.0 - */ -final class TypeRefAdapter extends TypeRef { - - private final Type type; - - - TypeRefAdapter(Class clazz) { - this.type = clazz; - } - - TypeRefAdapter(ParameterizedTypeReference typeReference) { - this.type = typeReference.getType(); - } - - TypeRefAdapter(Class clazz, Class generic) { - this.type = ResolvableType.forClassWithGenerics(clazz, generic).getType(); - } - - TypeRefAdapter(Class clazz, ParameterizedTypeReference generic) { - this.type = ResolvableType.forClassWithGenerics(clazz, ResolvableType.forType(generic)).getType(); - } - - - @Override - public Type getType() { - return this.type; - } - -} diff --git a/spring-graphql/src/main/java/org/springframework/graphql/support/MapGraphQlError.java b/spring-graphql/src/main/java/org/springframework/graphql/support/MapGraphQlError.java index dc65de09..f2313a4b 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/support/MapGraphQlError.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/support/MapGraphQlError.java @@ -29,7 +29,6 @@ import graphql.language.SourceLocation; import org.springframework.graphql.execution.ErrorType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; /** * Implementation of {@link GraphQLError} backed by a {@link Map}. @@ -63,9 +62,9 @@ public final class MapGraphQlError implements GraphQLError { (int) map.getOrDefault("column", 0), (String) map.get("sourceName"))) .collect(Collectors.toList()); - } + @Override @Nullable public String getMessage() {