Skip to content

Commit 88a8a78

Browse files
authored
Fix Promise#allOf(Promise<?>...) (#881)
AllOfFuture completes with the list of values from each promise passed into its constructor. The varargs variant of Promise#allOf however has a return type of Void, so the intention seems to be to discard the results of the promises. Because a list of results is disjoint from Void any invocation of this method will throw an exception when the promises complete. To resolve this we explicitly discard the result of the promises by mapping the result to null, the only possible value of Void.
1 parent 00503da commit 88a8a78

File tree

2 files changed

+59
-1
lines changed

2 files changed

+59
-1
lines changed

src/main/java/com/uber/cadence/internal/sync/WorkflowInternal.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ public static <U> Promise<List<U>> promiseAllOf(Collection<Promise<U>> promises)
287287

288288
@SuppressWarnings("unchecked")
289289
public static Promise<Void> promiseAllOf(Promise<?>... promises) {
290-
return new AllOfPromise(promises);
290+
return new AllOfPromise(promises).thenApply(results -> null);
291291
}
292292

293293
public static Promise<Object> promiseAnyOf(Iterable<Promise<?>> promises) {

src/test/java/com/uber/cadence/internal/sync/PromiseTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static java.util.Collections.emptyList;
2121
import static org.junit.Assert.assertEquals;
2222
import static org.junit.Assert.assertFalse;
23+
import static org.junit.Assert.assertNull;
2324
import static org.junit.Assert.assertTrue;
2425
import static org.junit.Assert.fail;
2526

@@ -439,6 +440,63 @@ public void testAllOf() throws Throwable {
439440
trace.setExpected(expected);
440441
}
441442

443+
@Test
444+
public void testAllOf_varArgs() throws Throwable {
445+
DeterministicRunner r =
446+
DeterministicRunner.newRunner(
447+
() -> {
448+
trace.add("root begin");
449+
CompletablePromise<String> f1 = Workflow.newPromise();
450+
CompletablePromise<String> f2 = Workflow.newPromise();
451+
CompletablePromise<String> f3 = Workflow.newPromise();
452+
453+
WorkflowInternal.newThread(
454+
false,
455+
() -> {
456+
trace.add("thread1 begin");
457+
f1.complete("value1");
458+
trace.add("thread1 done");
459+
})
460+
.start();
461+
WorkflowInternal.newThread(
462+
false,
463+
() -> {
464+
trace.add("thread3 begin");
465+
f3.complete("value3");
466+
trace.add("thread3 done");
467+
})
468+
.start();
469+
WorkflowInternal.newThread(
470+
false,
471+
() -> {
472+
trace.add("thread2 begin");
473+
f2.complete("value2");
474+
trace.add("thread2 done");
475+
})
476+
.start();
477+
478+
trace.add("root before allOf");
479+
Promise<Void> all = Promise.allOf(f1, f2, f3);
480+
481+
assertNull(all.get());
482+
trace.add("root done");
483+
});
484+
r.runUntilAllBlocked();
485+
String[] expected =
486+
new String[] {
487+
"root begin",
488+
"root before allOf",
489+
"thread1 begin",
490+
"thread1 done",
491+
"thread3 begin",
492+
"thread3 done",
493+
"thread2 begin",
494+
"thread2 done",
495+
"root done"
496+
};
497+
trace.setExpected(expected);
498+
}
499+
442500
@Test
443501
public void testAllOf_shouldCompleteWithoutBlocking() throws Throwable {
444502
DeterministicRunner r =

0 commit comments

Comments
 (0)