-
Notifications
You must be signed in to change notification settings - Fork 38.9k
Description
Current code:
Lines 48 to 51 in 16edf98
| suspend fun <T> TransactionalOperator.executeAndAwait(f: suspend (ReactiveTransaction) -> T): T { | |
| val context = currentCoroutineContext().minusKey(Job.Key) | |
| return execute { status -> mono(context) { f(status) } }.map { value -> Optional.ofNullable(value) } | |
| .defaultIfEmpty(Optional.empty()).awaitLast().orElse(null) |
Recommendation:
suspend fun <T> TransactionalOperator.executeAndAwait(f: suspend (ReactiveTransaction) -> T): T {
val context = currentCoroutineContext().minusKey(Job.Key)
val flux = execute { status -> mono<T & Any>(context) { f(status) } }
// The only reason Mono<T> could produce empty result is via f(status) producing null
// so it fine to case null to T here.
@Suppress("UNCHECKED_CAST")
return flux.awaitFirstOrNull() as T
}There's no need to use Optional, and going with Optional.orElse(null) defeats null safety.
As far as I understand, Flux<T> will never produce null values (they sould produce empty Flux instead), so flux.map { Optional.ofNullable(it) } is a pure overhead.
As far as I understand, executeAndAwait might yield null for two reasons:
a) User-provided code returned null from f(status) call. In that case, awaitFirstOrNull() is perfectly valid to return null
b) Some cancellation happens (e.g. coroutine cancelation or flux cancellation/timeout) before f(status) completes. I'm not sure it is possible, however, it if is possible, then it could result in null value returned without clear indication of the reason. In any case, previously the code was silently converting empty value to null, so my suggestion does not seem to make the case worse than it was.