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


##########
grails-async/rxjava3/src/main/groovy/org/grails/async/factory/rxjava3/RxPromiseFactory.groovy:
##########
@@ -76,24 +76,28 @@ 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 results of the given promises into one list, in order, 
with a {@code null} value restored
+     * from the sentinel it travels through the stream as. A bound promise 
contributes its result through
+     * {@code get()}, so one that holds a failure fails the list, as it does 
with every other promise factory.

Review Comment:
   Reworded in 2497f4de9c to "as it does with the default and synchronous 
factories". You're right about the other two: the RxJava 1 and 2 factories emit 
`((BoundPromise) p).value` in list `onComplete` and cast every promise to 
`RxPromise` in list `onError`, so the comparison only held for the two 
factories that read through `get()`. Module tests (26) and `codeStyle` pass 
locally on the new head.



-- 
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