data fetchers call static methods #3247
Description
Describe the bug
I've recently attempted to upgrade to v20 and noticed an unexpected behavior
Thankfully caught by our unit tests
a snipped from our unit tests output showed that graphql java failed to serialize a proper and valid response
updateOrganizerSubscriptionSuccessPromoCode() ✘ expected: <{data={updateOrganizerSubscription={success=true, error=null}}}> but was: <{data={updateOrganizerSubscription={success=null, error=null}}, errors=[{message=Can't serialize value (/updateOrganizerSubscription/success) : Expected a value that can be converted to type 'Boolean' but it was a 'UpdateOrganizerSubscriptionPayload', path=[updateOrganizerSubscription, success], extensions={classification=DataFetchingException}}]}>
the schema in question looks like this
type UpdateOrganizerSubscriptionPayload {
"""
A payload in case an organizer subscription was successfully updated.
"""
success: Boolean
"""
A payload in case updating an organizer subscription failed.
"""
error: OrganizerSubscriptionError
}the data fetcher returned class of the that confirms to this
public class UpdateOrganizerSubscriptionPayload {
private final Boolean success;
private final OrganizerSubscriptionError error;
private UpdateOrganizerSubscriptionPayload(Boolean success, OrganizerSubscriptionError error) {
this.success = success;
this.error = error;
}
public static UpdateOrganizerSubscriptionPayload success() { // 👈 note the static factory method for creating a success payload
return new UpdateOrganizerSubscriptionPayload(Boolean.TRUE, null);
}
public static UpdateOrganizerSubscriptionPayload error(OrganizerSubscriptionError error) { // 👈 note the static factory method for creating a success payload
return new UpdateOrganizerSubscriptionPayload(null, error);
}
public Boolean getSuccess() {
return success;
}
public OrganizerSubscriptionError getError() {
return error;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
UpdateOrganizerSubscriptionPayload that = (UpdateOrganizerSubscriptionPayload) o;
return Objects.equals(getSuccess(), that.getSuccess())
&& Objects.equals(getError(), that.getError());
}
@Override
public int hashCode() {
return Objects.hash(getSuccess(), getError());
}
@Override
public String toString() {
return new StringJoiner(
", ", UpdateOrganizerSubscriptionPayload.class.getSimpleName() + "[", "]")
.add("success=" + success)
.add("error=" + error)
.toString();
}
}but the serialization error I get is
Expected a value that can be converted to type 'Boolean' but it was a 'UpdateOrganizerSubscriptionPayload'
I believe this relates to the Record Like Property Fetching Support section of https://github.com/graphql-java/graphql-java/releases/tag/v20.0
I believe what is happening is that the new record like property datafetcher is reflecting over the name success but this is not expected as its static method intended to build a "success" version of the data fetcher payload. previously the getSuccess method was resolved. The release notes call out that the record like property name is preferred so the factory method likely gets picked up and instead of a Boolean indeed I get the type which contains the boolean.
I'm not sure if this is a bug or a feature but I feel like the issue and resolution might be to exclude methods with a static modifier when lifting the property value for serializing the result.
To Reproduce
Please provide a code example or even better a test to reproduce the bug.
I don't have a more limited case but I suspect if you did the following you could reproduce the issue
schema
type Test {
success: Boolean
}data fetcher
class TestFetcher implements DataFetcher<TestFetcher.Test> {
public static class Test {
private boolean success;
public Test(boolean success) {
this.success = success;
}
public boolean getSuccess() { return this.success } // 👈 expect this
public static Test success() { return new Test(true); } // 👈 likely will use this by fail when validating schema
}
public Test get(DataFetchingEnvironment env) {
return Test.success();
}
}again I didn't look but likely the fix for expected behavior in my case with be to omit methods with static modifiers when selecting candidate methods to invoke