Replace argument type with @Source to determine source/parent

[dugenkui03]

Annotation way will be more precise.

leniency model: argument type

I think argumentType way may lead mismatch. There will be various parameter types when spring-graphql support custom HandlerMethodArgumentResolver.

This way also require strict restriction of code: SourceMethodArgumentResolver must be put after other HandlerMethodArgumentResolver.

strictness model: @Source

Developers will easily know whether argument is source/parent by @Source, they don't have to look at the implementations of HandlerMethodArgumentResolver to determine which one supports the parameter.

And there is no need to take SourceMethodArgumentResolver as fallback.

[rstoyanchev]

Thanks for the proposal, and I admit there is something appealing about having an @Source annotation that explicitly marks the method parameter, and it's something we did consider.

The pause that I have is that the annotation does not serve any real purpose. We can work it out just fine without it, without any ambiguity that we've come across so far. In terms of developers wondering, well if someone new wants to find out how to get a hold of the parent, the answer is just declare it. I think that's as simple as it gets and you don't need to know more.

From the opposite perspective as well, if we had @Source, it would be legitimate to ask, should it be required and if so why? I don't think there is a good reason to require it, so that leads back again to having some controller methods with it and some without it. This reminds of @ModelAttribute in Spring MVC where you could have it or not, but the point there is that when you do add it, it can serve a purpose. There would be no such case here.

[rstoyanchev]

To address specifically the arguments brought up:

There will be various parameter types when spring-graphql support custom HandlerMethodArgumentResolver.

I don't see how it could lead to a mismatch since custom arguments are before the SourceMethodArgumentResolver.

This way also require strict restriction of code: SourceMethodArgumentResolver must be put after other HandlerMethodArgumentResolver.

This is an internal implementation detail, and there is no reason to expose it.

[dugenkui03]

Thanks for your reply, I respect your final decision. I further explain the reason of this proposal.

about mismatch(match incorrectly)

With @Source, developers can easily avoid mismatching the 'parent/source' in their custom MethodArgumentResolver.

Conversely, if the parent/source and parameter parsed by custom MethodArgumentResolver share many characteristics, parent/source maybe easy to mismatch——That's extreme scene, maybe we can ignore.

about readability of strictness model

Developers will easily know whether argument is source/parent by @Source, they don't have to look at the implementations of HandlerMethodArgumentResolver to determine which one supports the parameter.

Consider the @Controller method：

@SchemaMapping
public X  test(A a, B b, C c){
      // do something and return
}

Maybe developer must read source code about custom HandlerMethodArgumentResolver and the DataFetcher of parent for determining which one is parent/source(and which one will be resolved by custom HandlerMethodArgumentResolver).

All the strategies about resolving parameter in @SchemaMapping are marked with concrete class types(DataFetchingEnvironment,GraphQLContext ,Principal,) or annotation(@Argument,@Arguments), except for source.

[dugenkui03]

The same thing occur in @BatchMapping((List<V>)). And developer may want to define list-type-argument on batch method, the code below will have problem, that's another issue discussed in #324.

spring-graphql/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java

Line 312 in b2b1e2d

if (Collection.class.isAssignableFrom(parameter.getParameterType())) {

[rstoyanchev]

Thanks for elaborating. For now, I'm closing this but we can always reconsider when it becomes an actual issue.