Skip to content

Add maxBatchSize attribute to @BatchMapping #520

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

Closed
frehov opened this issue Oct 27, 2022 · 1 comment
Closed

Add maxBatchSize attribute to @BatchMapping #520

frehov opened this issue Oct 27, 2022 · 1 comment
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@frehov
Copy link

frehov commented Oct 27, 2022

Hi, sorry if this has been requested before

As the title suggests, I would like to put forward a feature request for having the @BatchMapping-annotation expose a maxBatchSize which will be set on the default DataLoaderOptions for that DataLoader. As MappingInfo is collected before registration I would carefully suggest that the batchloader boolean be changed to it's own separate object which would enclose additional properties for a batchloader like this.

Annotation annotation = annotations.iterator().next();
if (annotation instanceof SchemaMapping) {
	SchemaMapping mapping = (SchemaMapping) annotation;
	typeName = mapping.typeName();
	field = (StringUtils.hasText(mapping.field()) ? mapping.field() : method.getName());
}
else {
	BatchMapping mapping = (BatchMapping) annotation;
	typeName = mapping.typeName();
	field = (StringUtils.hasText(mapping.field()) ? mapping.field() : method.getName());
	batchMapping = BatchOptions.withMaximumBatchSize(mapping.maxBatchSize);
}

And these changes would be necessary in the MappingInfo class, and the BatchOptions class has been omitted as that would be an implementation detail for the spring team to consider if at all implemented.

private static class MappingInfo {

	private final FieldCoordinates coordinates;

	private final BatchOptions batchOptions;

	private final HandlerMethod handlerMethod;

	public MappingInfo(String typeName, String field, BatchOptions BatchOptions, HandlerMethod handlerMethod) {
		this.coordinates = FieldCoordinates.coordinates(typeName, field);
		this.handlerMethod = handlerMethod;
		this.batchOptions= batchOptions;
	}

	public FieldCoordinates getCoordinates() {
		return this.coordinates;
	}

	@SuppressWarnings("BooleanMethodIsAlwaysInverted")
	public boolean isBatchMapping() {
		return this.batchOptions != null;
	}

        public boolean getBatchOptions() {
                return this.batchOptions;
        }

	public HandlerMethod getHandlerMethod() {
		return this.handlerMethod;
	}

	@Override
	public String toString() {
		return this.coordinates + " -> " + this.handlerMethod;
	}
}

Secondary I would like to suggest the addition of ConfigurationProperties that could act as the "default" DataLoaderOptions and be injected into the BatchLoaderRegistry for convenience to be able to set properties like max batch size globally, perhaps under the key spring.graphql.dataloader.options

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 27, 2022
@rstoyanchev rstoyanchev changed the title [Feature Request] Add maxBatchSize to @BatchMapping Add maxBatchSize attribute to @BatchMapping Oct 31, 2022
@rstoyanchev
Copy link
Contributor

Thanks for the suggestions. Exposing maxBatchSize on @BatchMapping sounds good to me, and so does the second suggestion for default DataLoaderOptions but I've created a separate issue #522 for that.

@rstoyanchev rstoyanchev self-assigned this Oct 31, 2022
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 31, 2022
@rstoyanchev rstoyanchev added this to the 1.1.0 milestone Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants