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 creation of empty mono from lambda that returns a nullable value #743

Closed
sbuettner opened this issue Jul 17, 2017 · 9 comments
Closed
Assignees
Labels
status/need-decision This needs a decision from the team warn/api-change Breaking change with compilation errors
Milestone

Comments

@sbuettner
Copy link

sbuettner commented Jul 17, 2017

Imagine you are in kotlin land and want to return a Mono<X> from an existing function fun x(): X?. Currently its only possible to wrap each call to x() using the Mono.create like:

Mono.create { s -> s.success(x()) }

It would be nice to have some kind of generic shorthand method like this to allow a simple creation of a lazy mono like:

fun <T> fromCallableOrEmpty(body:() -> T?): Mono<T> = Mono.create<T> { s -> s.success(body()) }

where you would simply write: fromCallableOrEmpty { x() }.

@simonbasle simonbasle added the status/need-decision This needs a decision from the team label Jul 26, 2017
@smaldini smaldini modified the milestone: 3.1.0.RC1 Aug 2, 2017
@simonbasle simonbasle modified the milestones: 3.1.0.RC1, 3.1.0.RELEASE Aug 17, 2017
@smaldini
Copy link
Contributor

@sdeleuze @rstoyanchev WDYT

@sdeleuze
Copy link
Contributor

sdeleuze commented Sep 12, 2017

Didn't we discuss at some point supporting that (nullable lambda arguments with OrEmpty behavior) out of the box without having to add another variant to the API? I don't remember the final discussion / decision so a refresh on that topic would be welcome as it is IMO related to this request.

@sdeleuze
Copy link
Contributor

sdeleuze commented Sep 14, 2017

Precision: my question/proposal is about the possibility to support "null value to empty Mono" behavior in Mono public "entry points", not in Reactor internals which should remain based on a non-null principle.

Notice also that I am about to send a PR with null-safety of generic type arguments so this kind of behavior will be declared explicitly at API level in any case, avoiding confusion.

simonbasle added a commit that referenced this issue Sep 14, 2017
This has the side-effect that Mono#fromRunnable should always be
requested when using `subscribeOn`.
@simonbasle simonbasle self-assigned this Sep 14, 2017
@simonbasle simonbasle added the warn/api-change Breaking change with compilation errors label Sep 14, 2017
@simonbasle
Copy link
Member

Background on why this has been discussed and deferred before: the Callable must be lazily evaluated, especially it must behave well with backpressure and subscribeOn (ie the call must run on the subscribeOn's thread). Having null value + backpressure was proving difficult in this case.

simonbasle added a commit that referenced this issue Sep 15, 2017
This has the side-effect that Mono#fromRunnable should always be
requested when using `subscribeOn`.
simonbasle added a commit that referenced this issue Sep 15, 2017
This has the side-effect that Mono#fromRunnable should always be
requested when using `subscribeOn`.
@sbuettner
Copy link
Author

sbuettner commented Feb 5, 2018

@simonbasle I had a look at the current implementation and I think it would be great if we could tweak it a little bit. Lets assume that we have a kotlin lambda that returns a nullable value of type: T:

{ val x:String?= null; x }.toMono()

Using the current implementation I would get a Mono<T?>.

This may not be what we want as a Mono should never contain a null value and one would have to null-check inside a map operation. My expectation would be a Mono<T>. Therefore I would vote to change the return type for example of

fun <T> (() -> T).toMono(): Mono<T> = Mono.fromSupplier(this)

to

fun <T> (() -> T?).toMono(): Mono<T> = Mono.fromSupplier(this)

This way we would get a Mono<T> even though the lambda returns a T? and we could write:

{ val x:String?= null; x }.toMono().map { it.toUpperCase() }

@sbuettner
Copy link
Author

@simonbasle should I open a new issue to discuss this proposal?

@simonbasle
Copy link
Member

@sbuettner no I don't think this warrants another issue. would love to get a PR to better evaluate this follow-up, if that is not too much trouble (reference this issue in the PR description)

@sbuettner
Copy link
Author

@simonbasle Ok, I will try to come up with something 😄

@sbuettner
Copy link
Author

@simonbasle #1062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/need-decision This needs a decision from the team warn/api-change Breaking change with compilation errors
Projects
None yet
Development

No branches or pull requests

4 participants