Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/main/java/graphql/GraphqlErrorBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -132,6 +133,18 @@ public GraphQLError build() {
return new GraphqlErrorImpl(message, locations, errorType, path, extensions);
}

/**
* A simple implementation of a {@link GraphQLError}.
* <p>
* This provides {@link #hashCode()} and {@link #equals(Object)} methods that afford comparison with other
* {@link GraphQLError} implementations. However, the values provided in the following fields <b>must</b>
* in turn implement {@link #hashCode()} and {@link #equals(Object)} for this to function correctly:
* <ul>
* <li>the values in the {@link #getPath()} {@link List}.
* <li>the {@link #getErrorType()} {@link ErrorClassification}.
* <li>the values in the {@link #getExtensions()} {@link Map}.
* </ul>
*/
private static class GraphqlErrorImpl implements GraphQLError {
private final String message;
private final List<SourceLocation> locations;
Expand Down Expand Up @@ -176,6 +189,28 @@ public Map<String, Object> 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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

graphql.ErrorClassification is an interface so this is a place where technically a bad graphql.ErrorClassification would break you

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now added this in the Javadoc - is there another way you would like this to be handled?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No just articulating a challenge

&& Objects.equals(getPath(), that.getPath())
&& Objects.equals(getExtensions(), that.getExtensions());
}

@Override
public int hashCode() {
return Objects.hash(
getMessage(),
getLocations(),
getErrorType(),
getPath(),
getExtensions());
}
}

/**
Expand Down
64 changes: 63 additions & 1 deletion src/test/groovy/graphql/GraphqlErrorBuilderTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,66 @@ class GraphqlErrorBuilderTest extends Specification {
error.path == null

}
}

def "implements equals/hashCode correctly for matching errors"() {
when:
def firstError = toGraphQLError(first)
def secondError = toGraphQLError(second)

then:
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 equals/hashCode correctly for different errors"() {
when:
def firstError = toGraphQLError(first)
def secondError = toGraphQLError(second)

then:
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<String, Object> 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<SourceLocation>);
break;
case "errorType":
errorBuilder.errorType(value as ErrorClassification);
break;
case "path":
errorBuilder.path(value as List<Object>);
break;
case "extensions":
errorBuilder.extensions(value as Map<String, Object>);
break;
}
}
}
return errorBuilder.build();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we have some more tests cases with the other attrs such as source location, path say please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I've structured this in such a way that should make it easy to add more tests if the existing coverage is not enough.