From 7e576b1eaad6db2804bdb7f71527a44067ae521c Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Fri, 11 Oct 2024 22:22:14 +1100 Subject: [PATCH 1/2] Implement equals/hashCode for GraphqlErrorImpl We have a great many tests that verify that the errors emitted from a `DataFetcher` fit a certain shape. However, verifying equality of these errors is fiddly and error-prone, as we must directly check each individual attribute on every error - this is painful especially when we are trying to perform assertions on a `List` of `GraphQLError`s. To this end, we add `#equals` / `#hashCode` methods on `GraphqlErrorImpl`. However, it is important to note that `equals` will return `true` if all the attributes match, even if the implementing class is _not_ a `GraphqlErrorImpl`. This is done so that different implementations may be swapped in and out with causing test failures. --- .../java/graphql/GraphqlErrorBuilder.java | 30 ++++++++++++ .../graphql/GraphqlErrorBuilderTest.groovy | 48 ++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/GraphqlErrorBuilder.java b/src/main/java/graphql/GraphqlErrorBuilder.java index 4cef5beabe..711b71879a 100644 --- a/src/main/java/graphql/GraphqlErrorBuilder.java +++ b/src/main/java/graphql/GraphqlErrorBuilder.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import static graphql.Assert.assertNotNull; @@ -132,6 +133,13 @@ public GraphQLError build() { return new GraphqlErrorImpl(message, locations, errorType, path, extensions); } + /** + * A simple implementation of a {@link GraphQLError}. + *

+ * This provides {@link #hashCode()} and {@link #equals(Object)} methods that afford comparison with other + * {@link GraphQLError} implementations. However, the values in the {@link #getExtensions()} {@link Map} must + * in turn implement {@code hashCode()} and {@link #equals(Object)} for this to function correctly. + */ private static class GraphqlErrorImpl implements GraphQLError { private final String message; private final List locations; @@ -176,6 +184,28 @@ public Map getExtensions() { public String toString() { return message; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof GraphQLError)) return false; + GraphQLError that = (GraphQLError) o; + return Objects.equals(getMessage(), that.getMessage()) + && Objects.equals(getLocations(), that.getLocations()) + && Objects.equals(getErrorType(), that.getErrorType()) + && Objects.equals(getPath(), that.getPath()) + && Objects.equals(getExtensions(), that.getExtensions()); + } + + @Override + public int hashCode() { + return Objects.hash( + getMessage(), + getLocations(), + getErrorType(), + getPath(), + getExtensions()); + } } /** diff --git a/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy b/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy index 944e1fef35..0c31ccbe0d 100644 --- a/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy +++ b/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy @@ -152,4 +152,50 @@ class GraphqlErrorBuilderTest extends Specification { error.path == null } -} \ No newline at end of file + + def "implements equals correctly"() { + when: + def error1 = GraphQLError.newError().message("msg") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + def error2 = GraphQLError.newError().message("msg") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + def error3 = GraphQLError.newError().message("msg3") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + then: + error1 == error2 + error1 != error3 + error2 != error3 + } + + def "implements hashCode correctly"() { + when: + def error1 = GraphQLError.newError().message("msg") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + def error2 = GraphQLError.newError().message("msg") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + def error3 = GraphQLError.newError().message("msg3") + .locations(null) + .extensions([x : "y"]) + .path(null) + .build() + def errors = [error1, error2, error3] as Set + then: + errors == [error1, error3] as Set + errors == [error2, error3] as Set + } +} From 25a3b9fbb2efa4ff93ed68be73b72efafb3c3712 Mon Sep 17 00:00:00 2001 From: Alexandre Carlton Date: Sat, 16 Nov 2024 15:13:48 +1100 Subject: [PATCH 2/2] Include more thorough testing of GraphqlErrorImpl#equals/hashCode This now tests the other attributes, and also checks inequality explicitly. --- .../java/graphql/GraphqlErrorBuilder.java | 9 +- .../graphql/GraphqlErrorBuilderTest.groovy | 92 +++++++++++-------- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/src/main/java/graphql/GraphqlErrorBuilder.java b/src/main/java/graphql/GraphqlErrorBuilder.java index 711b71879a..db71eb20e0 100644 --- a/src/main/java/graphql/GraphqlErrorBuilder.java +++ b/src/main/java/graphql/GraphqlErrorBuilder.java @@ -137,8 +137,13 @@ public GraphQLError build() { * A simple implementation of a {@link GraphQLError}. *

* This provides {@link #hashCode()} and {@link #equals(Object)} methods that afford comparison with other - * {@link GraphQLError} implementations. However, the values in the {@link #getExtensions()} {@link Map} must - * in turn implement {@code hashCode()} and {@link #equals(Object)} for this to function correctly. + * {@link GraphQLError} implementations. However, the values provided in the following fields must + * in turn implement {@link #hashCode()} and {@link #equals(Object)} for this to function correctly: + *

*/ private static class GraphqlErrorImpl implements GraphQLError { private final String message; diff --git a/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy b/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy index 0c31ccbe0d..8713100d02 100644 --- a/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy +++ b/src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy @@ -153,49 +153,65 @@ class GraphqlErrorBuilderTest extends Specification { } - def "implements equals correctly"() { + def "implements equals/hashCode correctly for matching errors"() { when: - def error1 = GraphQLError.newError().message("msg") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() - def error2 = GraphQLError.newError().message("msg") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() - def error3 = GraphQLError.newError().message("msg3") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() + def firstError = toGraphQLError(first) + def secondError = toGraphQLError(second) + then: - error1 == error2 - error1 != error3 - error2 != error3 + firstError == secondError + firstError.hashCode() == secondError.hashCode() + + where: + first | second + [message: "msg"] | [message: "msg"] + [message: "msg", locations: [new SourceLocation(1, 2)]] | [message: "msg", locations: [new SourceLocation(1, 2)]] + [message: "msg", errorType: ErrorType.InvalidSyntax] | [message: "msg", errorType: ErrorType.InvalidSyntax] + [message: "msg", path: ["items", 1, "item"]] | [message: "msg", path: ["items", 1, "item"]] + [message: "msg", extensions: [aBoolean: true, aString: "foo"]] | [message: "msg", extensions: [aBoolean: true, aString: "foo"]] } - def "implements hashCode correctly"() { + def "implements equals/hashCode correctly for different errors"() { when: - def error1 = GraphQLError.newError().message("msg") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() - def error2 = GraphQLError.newError().message("msg") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() - def error3 = GraphQLError.newError().message("msg3") - .locations(null) - .extensions([x : "y"]) - .path(null) - .build() - def errors = [error1, error2, error3] as Set + def firstError = toGraphQLError(first) + def secondError = toGraphQLError(second) + then: - errors == [error1, error3] as Set - errors == [error2, error3] as Set + firstError != secondError + firstError.hashCode() != secondError.hashCode() + + where: + first | second + [message: "msg"] | [message: "different msg"] + [message: "msg", locations: [new SourceLocation(1, 2)]] | [message: "msg", locations: [new SourceLocation(3, 4)]] + [message: "msg", errorType: ErrorType.InvalidSyntax] | [message: "msg", errorType: ErrorType.DataFetchingException] + [message: "msg", path: ["items", "1", "item"]] | [message: "msg", path: ["items"]] + [message: "msg", extensions: [aBoolean: false]] | [message: "msg", extensions: [aString: "foo"]] + } + + private static GraphQLError toGraphQLError(Map errorFields) { + def errorBuilder = GraphQLError.newError(); + errorFields.forEach { key, value -> + if (value != null) { + switch (key) { + case "message": + errorBuilder.message(value as String); + break; + case "locations": + errorBuilder.locations(value as List); + break; + case "errorType": + errorBuilder.errorType(value as ErrorClassification); + break; + case "path": + errorBuilder.path(value as List); + break; + case "extensions": + errorBuilder.extensions(value as Map); + break; + } + } + } + return errorBuilder.build(); } }