bug: framework selects matching method arbitrarily

[michaelboyles]

Expected behavior

If you have two methods matching the required signature for a query, the framework should complain that the methods are ambiguous, by logging an error.

Actual behavior

The framework selects one method over the other, seemingly arbitrarily. Through testing, it's possibly the last classname alphabetically, but I'm not sure.

Steps to reproduce

Schema

type Query {
    me: User
}

type User {
    name: String!
}

Fetchers

@DgsComponent
public class AliceFetcher {
    @DgsQuery
    public User me() {
        return User.builder().name("Alice").build();
    }
}

@DgsComponent
public class BobFetcher {
    @DgsQuery
    public User me() {
        return User.builder().name("Bob").build();
    }
}

One of Alice or Bob will be returned. Now change the component class names. The name returned will not be consistent.

[srinivasankavitha]

This is by design. The @DgsQuery infers the matching field from the method name. So if you have conflicts, one would override the other. The correct usage would be to specifiy the parent and field names explicitly with @DgsData in such scenarios.

[srinivasankavitha]

As you suggest, we could detect this and provide better error messaging.

[michaelboyles]

Yeah, in case it wasn't clear, if there are 2 matching candidates then it does seem reasonable to select one of them (provided it's deterministic, which it seems to be?).

I categorised this as a "bug" mostly because it's an edge case and so I wasn't sure whether it was the result of an intentional design choice, or just happened to be working by accident.

If you're happy that this is as-designed then #1172 (which is related but a bit more broad in scope) already covers adding additional logging, for this and for some other error cases. In which case, this could be closed.

[srinivasankavitha]

I think this ticket makes sense to implement. I commented on the other one #1172, but in short, that would impose a lot of restrictions on folks if we fail on mismatched method signature etc., so don't think we should add that.

[srinivasankavitha]

Fixed as part of this PR: https://github.com/Netflix/dgs-framework/pull/1202