spring-graphql
spring-graphql copied to clipboard
Can't use Spring Security annotation when injecting DataLoader and returning a CompletableFuture
I'm having trouble combining DataLoaders with Spring Security;
My scenario;
typealias MerchantDataLoader = DataLoader<UUID, Merchant?>
// MerchantDataLoader
registry.forTypePair(UUID::class.java, Merchant::class.java).registerMappedBatchLoader { merchantIds, _ ->
Mono.just(merchantService.getMerchants(merchantIds).associateBy { it.id })
}
// ...
@SchemaMapping
@PreAuthorize("@permissionEvaluator.hasPermission(authentication, #merchant.id, 'Merchant', 'read')")
fun dashboardForDate(
merchant: Merchant,
@Argument date: LocalDate?,
merchantLoader: MerchantDataLoader
): CompletableFuture<Dashboard> = merchantLoader.load(merchant.id).thenApply {
if (it == null) throw EntityNotFoundException()
Dashboard(date, it)
}
For GraphQL Java being able to dispatch the dataloader the method has to return the CompletableFuture
, but for the @PreAuthorize
annotation it "must return an instance of org.reactivestreams.Publisher (i.e. Mono / Flux) or the function must be a Kotlin coroutine function".
I've also tried making it a suspend fun
, while still returning the CompletableFuture
, but then I get an error like does not match the type of the source Object 'class java.util.concurrent.CompletableFuture'
, which makes sense because my controller is not expecting the CompletableFuture
.
Is my assumption correct, or is there a way around this that I don't know about?
Interestingly enough converting to a mono seems to work;
@SchemaMapping
@PreAuthorize("@permissionEvaluator.hasPermission(authentication, #merchant.id, 'Merchant', 'read')")
fun dashboardForDate(
merchant: Merchant,
@Argument date: LocalDate?,
merchantLoader: MerchantDataLoader
): Mono<Dashboard> = merchantLoader.load(merchant.id).thenApply {
if (it == null) throw EntityNotFoundException()
Dashboard(date, it)
}.toMono()
Although I'm not sure if this is coincidental, or that this is supposed to work.
The method needs to return a reactive type for reactive context to propagate. So this is expected, but you're bringing up a good point that our examples with injecting a DataLoader
and using it as is should at least be improved. Maybe we could even consider injecting an alternative ReactiveDataLoader
type that returns Mono
instead of CompletableFuture
.
Maybe we could even consider injecting an alternative
ReactiveDataLoader
type that returnsMono
instead ofCompletableFuture
.
So returning a mono is supposed to work for dataloaders? In comparison; awaiting the CompletableFuture
in a suspend fun causes the application to lock-up.
And a ReactiveDataLoader
sounds like a good idea!
So returning a mono is supposed to work for dataloaders?
Yes that should work. Mono
and Flux
are supported as return values for any DataFetcher
with context propagation, through ContextDataFetcherDecorator
.
Alright, good to know it's spring graphql specific. I suppose this could be extended to support coroutines too?
Probably so, but need to investigate.
@koenpunt since coroutines are now supported and #653 was processed, is anything left to do here?
@rstoyanchev I'm not sure about whether the combination with Spring Security annotations now works, since we stopped using that because it didn't work well together with Spring R2DBC (overfetching data because it doesn't have a cache like JPA does).
So also the need for it disappeared for us.
Closing for now.