diff --git a/flow/src/main/java/flow/Flow.java b/flow/src/main/java/flow/Flow.java index efe11ee..2619bda 100644 --- a/flow/src/main/java/flow/Flow.java +++ b/flow/src/main/java/flow/Flow.java @@ -261,18 +261,39 @@ public void set(@NonNull final Object newTopKey) { } /** - * Go back one key. + * Go back one key. Typically called from {@link Activity#onBackPressed()}, with + * the return value determining whether or not to call super. E.g. + *
+   * public void onBackPressed() {
+   *   if (!Flow.get(this).goBack()) {
+   *     super.onBackPressed();
+   *   }
+   * }
+   * 
* - * @return false if going back is not possible or a traversal is in progress. + * @return false if going back is not possible. */ @CheckResult public boolean goBack() { boolean canGoBack = history.size() > 1 || (pendingTraversal != null && pendingTraversal.state != TraversalState.FINISHED); if (!canGoBack) return false; - History.Builder builder = history.buildUpon(); - builder.pop(); - final History newHistory = builder.build(); - setHistory(newHistory, Direction.BACKWARD); + + move(new PendingTraversal() { + @Override void doExecute() { + if (history.size() <= 1) { + // The history shrank while this op was pending. It happens, let's + // no-op. See lengthy discussions: + // https://github.com/square/flow/issues/195 + // https://github.com/square/flow/pull/197 + return; + } + + History.Builder builder = history.buildUpon(); + builder.pop(); + final History newHistory = builder.build(); + dispatch(newHistory, Direction.BACKWARD); + } + }); return true; } @@ -316,11 +337,13 @@ private static History preserveEquivalentPrefix(History current, History propose private enum TraversalState { /** {@link PendingTraversal#execute} has not been called. */ ENQUEUED, + /** * {@link PendingTraversal#execute} was called, waiting for {@link * PendingTraversal#onTraversalCompleted}. */ DISPATCHED, + /** * {@link PendingTraversal#onTraversalCompleted} was called. */ diff --git a/flow/src/test/java/flow/ReentranceTest.java b/flow/src/test/java/flow/ReentranceTest.java index 799c897..64c235c 100644 --- a/flow/src/test/java/flow/ReentranceTest.java +++ b/flow/src/test/java/flow/ReentranceTest.java @@ -18,14 +18,15 @@ import android.support.annotation.NonNull; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; import java.util.concurrent.atomic.AtomicInteger; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import static flow.Direction.FORWARD; - import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Fail.fail; import static org.mockito.MockitoAnnotations.initMocks; @@ -43,7 +44,8 @@ public class ReentranceTest { @Test public void reentrantGo() { Dispatcher dispatcher = new Dispatcher() { - @Override public void dispatch(@NonNull Traversal navigation, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal navigation, @NonNull TraversalCallback callback) { lastStack = navigation.destination; Object next = navigation.destination.top(); if (next instanceof Detail) { @@ -64,7 +66,8 @@ public class ReentranceTest { Dispatcher dispatcher = new Dispatcher() { boolean loading = true; - @Override public void dispatch(@NonNull Traversal navigation, @NonNull TraversalCallback onComplete) { + @Override + public void dispatch(@NonNull Traversal navigation, @NonNull TraversalCallback onComplete) { lastStack = navigation.destination; Object next = navigation.destination.top(); if (loading) { @@ -93,7 +96,8 @@ public class ReentranceTest { @Test public void reentrantForwardThenGo() { Flow flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastStack = traversal.destination; Object next = traversal.destination.top(); if (next instanceof Detail) { @@ -110,9 +114,61 @@ public class ReentranceTest { verifyHistory(lastStack, new Error(), new Loading(), new Detail()); } + @Test public void goBackQueuesUp() { + final Queue callbacks = new LinkedList<>(); + + Flow flow = new Flow(keyManager, History.single(new Catalog())); + flow.setDispatcher(new Dispatcher() { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + callbacks.add(callback); + lastStack = traversal.destination; + } + }); + + flow.set(new Detail()); + flow.set(new Error()); + assertThat(flow.goBack()).isTrue(); + + while (!callbacks.isEmpty()) { + callbacks.poll().onTraversalCompleted(); + } + + verifyHistory(lastStack, new Detail(), new Catalog()); + } + + @Test public void overwflowQueuedBackupsNoOp() { + final Queue callbacks = new LinkedList<>(); + + Flow flow = new Flow(keyManager, History.single(new Catalog())); + flow.setDispatcher(new Dispatcher() { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + callbacks.add(callback); + lastStack = traversal.destination; + } + }); + + flow.set(new Detail()); + + for (int i = 0; i < 20; i++) { + assertThat(flow.goBack()).isTrue(); + } + + int callbackCount = 0; + while (!callbacks.isEmpty()) { + callbackCount++; + callbacks.poll().onTraversalCompleted(); + } + + assertThat(callbackCount).isEqualTo(3); + verifyHistory(lastStack, new Catalog()); + } + @Test public void reentranceWaitsForCallback() { Dispatcher dispatcher = new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastStack = traversal.destination; lastCallback = callback; Object next = traversal.destination.top(); @@ -140,7 +196,8 @@ public class ReentranceTest { @Test public void onCompleteThrowsIfCalledTwice() { flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastStack = traversal.destination; lastCallback = callback; } @@ -159,7 +216,8 @@ public class ReentranceTest { flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastStack = traversal.destination; callback.onTraversalCompleted(); } @@ -174,7 +232,8 @@ public class ReentranceTest { flow.set(new Detail()); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { dispatchCount.incrementAndGet(); lastStack = traversal.destination; callback.onTraversalCompleted(); @@ -192,7 +251,8 @@ public class ReentranceTest { flow.set(new Error()); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastCallback = callback; } }); @@ -208,7 +268,8 @@ public class ReentranceTest { flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { flow.set(new Loading()); flow.removeDispatcher(this); callback.onTraversalCompleted(); @@ -218,7 +279,8 @@ public class ReentranceTest { verifyHistory(flow.getHistory(), new Catalog()); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { callback.onTraversalCompleted(); } }); @@ -229,12 +291,14 @@ public class ReentranceTest { @Test public void dispatcherSetInMidFlightWaitsForBootstrap() { flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastCallback = callback; } }); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastStack = traversal.destination; callback.onTraversalCompleted(); } @@ -249,13 +313,15 @@ public class ReentranceTest { final AtomicInteger secondDispatcherCount = new AtomicInteger(0); flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { flow.set(new Detail()); lastCallback = callback; } }); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { secondDispatcherCount.incrementAndGet(); lastStack = traversal.destination; callback.onTraversalCompleted(); @@ -273,7 +339,8 @@ public class ReentranceTest { flow = new Flow(keyManager, History.single(new Catalog())); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { lastCallback = callback; flow.removeDispatcher(this); flow.set(new Loading()); @@ -283,7 +350,8 @@ public class ReentranceTest { verifyHistory(flow.getHistory(), new Catalog()); flow.setDispatcher(new Dispatcher() { - @Override public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { + @Override + public void dispatch(@NonNull Traversal traversal, @NonNull TraversalCallback callback) { secondDispatcherCount.incrementAndGet(); callback.onTraversalCompleted(); }