Skip to content

Support for BatchHandlerMethodArgumentResolver #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

dugenkui03
Copy link
Contributor

@dugenkui03 dugenkui03 commented Feb 22, 2022

The purpose of this PR

Replace hard code in BatchLoaderHandlerMethod with BatchHandlerMethodArgumentResolver, and support custom BatchHandlerMethodArgumentResolver.

1、The reason spring-graphql should support BatchHandlerMethodArgumentResolver

In #232 , I suggested that @BatchMapping should support @Argument, because user will define arguments on field, no matter whether the field is fetched by DataLoader way or 'n+1' way.

Furthermore, user will always want to get information about field, such as field definition or directives.

Hence, I think BatchLoaderHandlerMethod should support argumentResolveStrategies which resolving argument values into various method parameters.

2、The way spring-graphql support BatchHandlerMethodArgumentResolver

BatchMappingDataFetcher could pass DataFetchingEnvironment to DataLoader by invoking dataLoader.load(env.getSource(), env)(the following code), and BatchLoaderHandlerMethod and argument resolve strategies will get all the information about the fetched field.

	static class BatchMappingDataFetcher implements DataFetcher<Object> {
                ......
		@Override
		public Object get(DataFetchingEnvironment env) {
			DataLoader<?, ?> dataLoader = env.getDataLoaderRegistry().getDataLoader(this.dataLoaderKey);
			if (dataLoader == null) {
				throw new IllegalStateException("No DataLoader for key '" + this.dataLoaderKey + "'");
			}
                         // pass DataFetchingEnvironment to DataLoader and other component
			return dataLoader.load(env.getSource(), env);
		}
	}

Brief change log

TODO: for validation and doc.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 22, 2022
@dugenkui03 dugenkui03 marked this pull request as draft March 9, 2022 17:22
dugenkui03 added a commit to dugenkui03/spring-graphql that referenced this pull request Mar 10, 2022
@dugenkui03 dugenkui03 force-pushed the supportBatchHandlerMethodArgumentResolver branch from d86fcad to ddcb4e6 Compare March 10, 2022 15:31
@dugenkui03 dugenkui03 closed this Mar 10, 2022
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants