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


##########
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>
+            }
+            return ((RxPromise<T>) p).toObservable()

Review Comment:
   Fixed in 592e8d95fc. Rather than reading `subject` in `values()`, 
`toObservable()` itself returns the subject now, so nothing but the constructor 
subscribes to the source; the class is package-scoped and `values()` is the 
method's only caller. The constructor's observer calls `subject.onComplete()` 
after `finished = true`.
   
   Your four features are in: three in `RxPromiseListSpec` and the `then` one 
in `RxPromiseSpec`. They pass now, and `:grails-async-rxjava3:test` (24 tests) 
and `codeStyle` pass with them. `7.0.x` is merged in for the MongoDB lane's 
runner change.



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