jdaugherty commented on code in PR #16619:
URL: https://github.com/apache/grails-core/pull/16619#discussion_r4340586125


##########
grails-async/rxjava3/src/main/groovy/org/grails/async/factory/rxjava3/RxPromiseFactory.groovy:
##########
@@ -76,24 +76,27 @@ class RxPromiseFactory extends AbstractPromiseFactory {
 
     @Override
     <T> Promise<T> onComplete(List<Promise<T>> promises, Closure<T> callable) {
-        new RxPromise<T>(this, Observable.concat(
-            promises.collect { Promise p ->
-                if (p instanceof BoundPromise) {
-                    return Observable.just(((BoundPromise) p).value) as 
Observable<T>
-                }
-                else {
-                    return ((RxPromise) p).toObservable() as Observable<T>
-                }
-            }
-        ).toList())
-         .onComplete(callable)
+        new RxPromise<T>(this, values(promises)).onComplete(callable)
     }
 
     @Override
     <T> Promise<List<T>> onError(List<Promise<T>> promises, Closure<?> 
callable) {
-        new RxPromise(this, Observable.concat(
-                promises.collect { ((RxPromise<T>) it).toObservable() }
-        ).toList())
-        .onError(callable) as Promise<List<T>>
+        new RxPromise(this, values(promises)).onError(callable) as 
Promise<List<T>>
+    }
+
+    /**
+     * Concatenates the values of the given promises into one list, in order, 
with a {@code null} value restored
+     * from the sentinel it travels through the stream as.
+     */
+    private static <T> Single<List<T>> values(List<Promise<T>> promises) {
+        List<Observable<T>> sources = promises.collect { Promise<T> p ->
+            if (p instanceof BoundPromise) {
+                return Observable.just(RxPromise.wrap(((BoundPromise) 
p).value)) as Observable<T>

Review Comment:
   Fixed in cf6726ed2b, the same change as #16690 at 2497f4de9c: the bound 
branch of `values()` reads the promise through `get()` 
(`Observable.fromCallable { RxPromise.wrap(p.get()) }`), so a failure a 
`then()` callback stored fails the aggregate `get()`, reaches the list 
`onError` callback with the original exception and skips the list `onComplete` 
callback, as `waitAll` already did. The two regressions are in 
`RxPromiseListSpec` (`onError over a list reports the failure a bound promise 
holds`, `onComplete over a list skips its callback when a bound promise holds a 
failure`); the null-value and single-execution features are unchanged. The 
rxjava3 sources and specs are byte-identical to #16690 again, and 
`:grails-async-rxjava3:test` passes 26/26.
   
   ee2d525fbf merges `9.0.x` back in. The one conflict was `upgrading90x.adoc`, 
which gained sections 3-15 upstream: Groovy 6 stays section 1, the RxGORM and 
RxJava deprecation sections are now 17 and 18, and the page's "see section N" 
references are shifted to match.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to