Add async API (2)

[katcharov]

JAVA-5082

This is #1234, but with demo refactoring removed (that is, this is just the async API, with no other changes).

The first commit, "Add async functions", was reviewed as part of #1134 , but can be further reviewed. Other commits are all new non-reviewed code.

[jyemin]

A few comments on the second commit

[jyemin]

Resolve one thread and commented back on another.

[jyemin]

Resolve one thread and commented back on another.

[jyemin]

I prefer that usage documentation is placed in the code rather than the test. It will be easier for maintainers to refer to it if it's here. And that's where all of our existing internal Javadoc is, so it will be less surprising.

Of all the new interfaces, this seems like the best place to put it.

[katcharov]

Moved; I find that I often look at the test code (more often than the docs) to confirm that I am using the API correctly.

[jyemin]

Replacing the whole block with

onResult(result, null)

seems preferable to this cast.

[rozza]

Firstly, I really like where this API is going.

I initially got confused as the language wasn't as I was expected coming from a functional background (eg. Scala and RxJava / Reactor). This got me thinking, and the API could be simplified by adopting some methods akin to those in Project Reactors Mono eg map, flatmap and onErrorResume.

I did a quick POC based off this work and it provides the following methods:

Method	Description
Mono<Void> empty()	produces a new Mono and starts the chain
void finish(final SingleResultCallback<T> callback)	Use the Mono with the supplied callback
Mono<R> map(final Function<T, R> function)	synchronously converts value T to R.
Mono<R> flatMap(Function<T, Mono<R>> transformer)	takes value T and provides a Mono<R>.
Mono<T> then(final Mono<T> mono)	a helper method that uses flatMap ignores the previous result and provides a new Mono<T>.
Mono<T> onErrorResume(Function<? super Throwable, Mono<T>> fallbackFunction)	handles any errors and produces a new Mono<T>.
Mono<T> doWhile(final Mono<T> mono, final Predicate<Throwable> shouldRetry)	handles any looping.

The mapping between this API would be:

AsyncRunnable	Mono
beginAsync	empty
unsafeFinish	unsafeFinish
finish	finish
getAsync	n/a
onErrorIf	onErrorResume
thenAlwaysRunAndFinish	map().finish() / flatMap().finish()
thenRunAndFinish	map().finish() / flatMap().finish()
thenApply	map / flatMap
thenConsume	map / flatMap
thenRun	map / flatMap
thenRunIf	map / flatMap
thenSupply	map / flatMap
thenRunRetryingWhile	doWhile

You can compare the tests here: AsyncFunctionsTest and MonoTest

Basically, its very similar but follows a more conventional reactive API rather that trying to coherse sync Functional types (Runnable, Supplier, Consumer) into an async paradigm.

Let me know what you think?

[katcharov]

@rozza Interesting! Thanks for digging into the options, this is the sort of thing that will help us round out the API. The most important thing, in my view, is the function composition that happens in "then" methods, which allows us to flatten nested async calls, and monadically short-circuit ensuing calls (which is comparable to exception handling). Both API surfaces build on this, so ultimately the APIs are much closer to each other than, for example, our current approach, or using reactive. Of course there are specific differences that arise, which can be individually addressed:

1. I think we should avoid the "indented" formatting approach. Consider:

try {
    plain(0);
    sync(1);
} catch (Throwable t) {
    sync(2);
}
sync(3);

// ========== Option 1:

Mono.empty().then(c2 -> {
    Mono.empty().then(c -> {
        plain(0);
        async(1, c);
    }).onErrorResume(t -> (c) -> {
        async(2, c);
    }).finish(c2);
}).then(c -> {
    async(3, c);
}).finish(callback);

// ========== Option 2:

Mono.empty()
        .then(c2 ->
                Mono.empty()
                        .then(c -> {
                            plain(0);
                            async(1, c);
                        })
                        .onErrorResume(t -> (c) -> async(2, c))
                        .finish(c2))
        .then(c -> async(3, c))
        .finish(callback);

While Option 2 can be shorter (though typically by only 1 line) in simple cases, in typical non-simple cases, where clarity is all the more important, it takes up more lines, and has substantial indentation. It is also less consistent, since it partly uses the "Option 1" approach when there is plain code mixed in (see then, vs onErrorResume above), which often happens. The biggest down-side is that it mixes boilerplate with normal code, which must be compared against a sync counterpart, whereas Option 1 puts all boilerplate on its own lines. I find matching the first option up with its async counterpart trivial (I just skim vertically along the suitable indentation level), but the second much more difficult.

2. The naming and conceptualization is similar to the sync functional types (like Runnable) in part because there are cases where we use a Runnable in sync code, but in the async code, the Runnable must return via callback, which is actually just the AsyncRunnable interface here (and see also the existing AsyncCallbackRunnable). It seems awkward or incorrect to replace it with a Mono. It also seems confusing to think of this as a Mono, which is supposed to be "Publisher with basic rx operators that emits at most one item via the onNext signal then terminates with an onComplete signal". I think we should avoid reactive concepts like publishers and signals, and even certain functional concepts don't seem to fit. For example, I somewhat understand the idea of using flatmap for thenSupply, but conceptually I find it difficult to reason about (functionally, what are we mapping over when an input is "void", perhaps a side-effect?). In my head, only "thenApply" corresponds cleanly to an (asynchronous) map.

3. Some of the other changes I don't have a clear position on. For example, I think a thenIf helper makes it easier to match sync code, since it avoids having a extraneous else with completion boilerplate, but maybe this doesn't happen so often that it really needs a helper. These might be easier to consider in context, as review comments on relevant code.

[rozza]

@katcharov, thanks for the feedback. My aim was not to create new language (and be barrier for learning / adoption) but rather to used established names in functional / composable code.

I picked the name Mono as its a common name that represents a type of Future which eventually emits a single result or errors (even Mono<Void> produces a result (null) or an error). The functions map and flatMap are common across many languages and can be applicable to both sync and async code. In Scala it is the foundation of composable Futures (the language even has syntactic sugar that underneath uses flatMap).

The Mono interface doesn't add any reactive concepts like publishers or signals. It just uses map to transform actual values and flatMap to convert resulting values into another Mono - allowing for composition and chaining. I added then to handle cases where you don't care about the previous result (eg Mono<Void> just produces null) but want to chain the logic - its just a flatMap where the input is ignored. The Mono class could be extended to handle predicates for mapping or error handling but the core methods required are already in its API.

Taking the example code - there are no runnables, just a flow of methods / and a catch:

try {
    plain(0);
    sync(1);
} catch (Throwable t) {
    sync(2);
}
sync(3);

// Can be flatted to its simplest form of (no need to add an extra nested Mono):
Mono.empty()
    .then(c -> {                        // try
        plain(0);
        async(1, c);
    }).onErrorResume(t -> (c) -> {      // catch
        async(2, c);
    })
    .then(c -> {                        // do after
        async(3, c);
    })
.finish(callback);

Even with SingleResultCallbacks there are still cases where code is run synchronously (eg the plain method in the tests) and there are cases where the results are asynchronously produced (eg. the async method). The example is a little artifical as previous side effects of the methods are ignored, so it lacks the passing of values from callback to callback, which is common in the codebase.

Similar to AsyncRunnable, Mono allows for easy replacement of the SingleResultCallback API in stages. However, a benefit by using a single type would be to eventually just pass the Mono's around to build up the composition of asynchronous actions and not have to switch in and out of callback based code.

[rozza]

I like the general approach and had some naming concerns that are pretty trivial.

I will add a ticket for future consideration regarding adapting this approach, and extending it to provide a chainable (Monadic) API. That does not block this work, and as there are 2 LGTMs I've removed myself from the review.