Skip to content
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

Support for BatchHandlerMethodArgumentResolver #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dugenkui03
Copy link
Owner

@dugenkui03 dugenkui03 commented Mar 10, 2022

The purpose of this PR

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

1、Why spring-graphql should support BatchHandlerMethodArgumentResolver

User will define arguments on field, no matter whether the field is fetched by DataLoader or fetched directly( n+1 way). And user will always want to get additional information about field, such as Directives, FieldDefinition.

In conclusion, we'd better suport pass all the information of DataFetchingEnvironment to batch handler method. That said BatchLoaderHandlerMethod should support argumentResolve strategies which resolving argument values into various method parameters, and developers could custom their BatchLoaderHandlerMethod.

2、The way spring-graphql support BatchHandlerMethodArgumentResolver

Replace dataLoader.load(env.getSource()) with dataLoader.load(env.getSource(), env).

	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

  • replace BatchLoaderHandlerMethod#resolveArgument with BatchHandlerMethodArgumentResolver, and support custom BatchLoaderHandlerMethod ;
  • add strategies: ArgumentBatchMethodArgumentResolverArgumentMapBatchMethodArgumentResolverArgumentsBatchMethodArgumentResolver and update document;
  • support for validate batchHandlerMethod input;
  • add unit tests for new batch method argument resolve strategies and the capability of custom BatchHandlerMethodArgumentResolver.

@dugenkui03 dugenkui03 force-pushed the support-for-BatchHandlerMethodArgumentResolver branch 12 times, most recently from ad0e803 to e4350e1 Compare March 12, 2022 16:10
@dugenkui03 dugenkui03 force-pushed the support-for-BatchHandlerMethodArgumentResolver branch from e4350e1 to b9fb512 Compare March 12, 2022 16:28
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.

1 participant