Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Webflux returns Mono#toString in response if used in Query/Mutation #326

Closed
Sam-Kruglov opened this issue Jan 11, 2020 · 6 comments · Fixed by #514
Closed

Webflux returns Mono#toString in response if used in Query/Mutation #326

Sam-Kruglov opened this issue Jan 11, 2020 · 6 comments · Fixed by #514
Milestone

Comments

@Sam-Kruglov
Copy link

Pretty much sums it up for me.

I suppose that you only supposed to use reactive types in a subscription? Is this limitation really necessary? I think for Mono it could very well be inside query or mutation. What is more, if, inside my mutation resolver method, I call service.getStuff().block() then Spring throws an exception that you can't block here, so I have to put my mutation inside a subscription instead, which feel a bit wrong

@Sam-Kruglov
Copy link
Author

Sam-Kruglov commented Jan 12, 2020

It works using query if I convert Mono into CompletableFuture. IMO should work with Mono as well

@Sam-Kruglov
Copy link
Author

Sam-Kruglov commented Jan 12, 2020

And about subscriptions: I don't think it makes a lot of sense to put every Flux into the subscription because I feel it's semantically different.

Since we are building the API in a reactive way, every single object becomes a promise of that object, so it is still a query (and it works with futures, as I found out). Same goes for a list of objects, which becomes a Flux of objects and each of them can appear on the screen separately and it is still considered a query.

Mutations will also return a promise.

Subscriptions are for something we would like to be up to date about, like pizza delivery status. But if we are just selecting menu items, that should be a query although it will be a flux as well. I could collect the flux into a Mono<List<...>> but that wouldn't the so cool. Maybe, for some reason, some items are fast to load, some are long, so the screen won't stutter and fill out the UI table in real-time one by one which is very cool.

Let me know what you think, I may be wrong tho

@oliemansm
Copy link
Member

Hi @Sam-Kruglov. As you've found out yourself already and I did too after some googling is that graphql-java doesn't support Mono directly, but you have to convert it to CompletableFuture instead. See also https://spectrum.chat/graphql-java/general/how-do-create-graphql-java-spring-webflux-based-data-fetcher-if-my-data-fetcher-returns-mono-it-is-throwing-error~19defe59-f91b-4db4-8296-78c0dcd90a38.

I'd like to be able to work with Mono and Flux directly as well instead of having to do that kind of conversion, so I think it would be a nice enhancement for this webflux starter to support that. My work on that will primarily be driven by my own business need, so any contributions in that area to speed that up are welcome.

@oliemansm oliemansm changed the title Webflux returns Mono#toString in respose if used in Query/Mutation Webflux returns Mono#toString in response if used in Query/Mutation Apr 5, 2020
@callajd
Copy link

callajd commented Jul 10, 2020

@oliemansm @Sam-Kruglov

I agree this feature would make the starter much more friendly for this stack. I guess it will become more and more common too.

I have the bandwidth to contribute here, some pointers into relevant files / types / line numbers would definitely kickstart the process...

@oliemansm
Copy link
Member

Hi @callajd,

Totally missed your offer to help out with this. Not sure if you still have bandwidth available to help out? First step would be to create a couple of (failing) unit tests to show the desired use. I'll have to dive in a bit more to be able to give you more explicit pointers.

@ooga
Copy link

ooga commented Jan 14, 2021

You can customize wrappers to work with Mono in your resolvers
Flux is already supported by default wrappers https://github.com/graphql-java-kickstart/graphql-java-tools/blob/master/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt#L143

@Bean
List<SchemaParserOptions.GenericWrapper> genericWrappers() {
	return List.of(
		GenericWrapper.withTransformer(
            Mono.class,
            0,
            Mono::toFuture,
            t -> t
        )
	);
}

@oliemansm oliemansm added this to the 11.0.0 milestone Jan 16, 2021
@oliemansm oliemansm linked a pull request Jan 16, 2021 that will close this issue
oliemansm added a commit that referenced this issue Jan 16, 2021
…support

Autoconfigure Generic wrapper for Mono fix #326
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants