Support using DataLoader from suspend function

[koenpunt]

Rudimentary implementation to support returning a CompletableFuture from a suspend fun.

CoroutinesUtils.invokeSuspendingFunction wraps the return value of the function in a Mono (or Flux). But it also does this when a CompletableFuture is returned, and thus results in a Mono<CompletableFuture<?>>, which isn't captured by graphql-java, and thus the dataloader is never dispatched.

By unwrapping the future, and converting it to a mono (as opposed to wrapping), the dataloader seems to be dispatched correctly.

Haven't added any tests yet, but if you also think this would be the right approach I can work on adding some tests as well.

Related issue: #343

[koenpunt]

This is inspired by the code in CoroutinesUtils; https://github.com/spring-projects/spring-framework/blob/254fb3956727f22e8e5f4e61cd1153b1d5bff93b/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java#L112-L122

[koenpunt]

@rstoyanchev did you or anyone on the team had a chance to look at this?

[rstoyanchev]

Thanks for the ping and apologies for the delay.

Forgive the naive question, but just wondering why declare such a method that uses a DataLoader as a coroutine? It should already be non-blocking.

The implementation looks straight forward otherwise.

[hantsy]

Check and vote up #393

[koenpunt]

Thanks for the ping and apologies for the delay.

Forgive the naive question, but just wondering why declare such a method that uses a DataLoader as a coroutine? It should already be non-blocking.

The implementation looks straight forward otherwise.

When first calling other suspending functions before loading data using the dataloader, e.g.;

We currently use this:

    @SchemaMapping
    fun transactions(
        model: Order,
        orderDataLoader: OrderDataLoader,
        paymentAccountDataLoader: PaymentAccountDataLoader,
        principal: Principal,
    ): CompletableFuture<List<OrderTransaction>> {
        return CoroutineScope(Dispatchers.IO).future {
            orderPaymentService.findAllByOrderId(model.id)
        }.thenCompose { orderPayments ->
            paymentAccountDataLoader
                .loadMany(orderPayments.mapNotNull { it.paymentAccountId })
                .also { paymentAccountDataLoader.dispatch() }

But when we can return a dataloader from a suspend function it would look more like:

    @SchemaMapping
    suspend fun transactions(
        model: Order,
        orderDataLoader: OrderDataLoader,
        paymentAccountDataLoader: PaymentAccountDataLoader,
        principal: Principal,
    ): CompletableFuture<List<OrderTransaction>> {
        val orderPayments = orderPaymentService.findAllByOrderId(model.id)

        return paymentAccountDataLoader
            .loadMany(orderPayments.mapNotNull { it.paymentAccountId })
            // Not sure if manual dispatch would still be necessary here.
            .also { paymentAccountDataLoader.dispatch() }
    }

[rstoyanchev]

Thanks for clarifying.

[hantsy]

@koenpunt Add kotlin-coroutines-jdk8 into your deps, and use future builder to convert the block to a CompleteableFuture.

fun test() : CompletableFuture<List<OrderTransaction>>  = future{
}

[koenpunt]

That doesn't work, because future is a method on the coroutine scope.

This is valid code, but doesn't work because then we're back at the original problem:

suspend fun test(): CompletableFuture<List<OrderTransaction>> = withContext(Dispatchers.IO) {
   future {
      // ...
   }
}