diff --git a/libraries/rib-rx2/src/main/java/com/badoo/ribs/rx2/workflows/RxWorkflowNode.kt b/libraries/rib-rx2/src/main/java/com/badoo/ribs/rx2/workflows/RxWorkflowNode.kt index 833103b2d..349c3877d 100644 --- a/libraries/rib-rx2/src/main/java/com/badoo/ribs/rx2/workflows/RxWorkflowNode.kt +++ b/libraries/rib-rx2/src/main/java/com/badoo/ribs/rx2/workflows/RxWorkflowNode.kt @@ -1,6 +1,6 @@ package com.badoo.ribs.rx2.workflows -import androidx.annotation.VisibleForTesting +import androidx.lifecycle.Lifecycle import com.badoo.ribs.core.Node import com.badoo.ribs.core.modality.BuildParams import com.badoo.ribs.core.plugin.Plugin @@ -8,8 +8,12 @@ import com.badoo.ribs.core.view.RibView import com.badoo.ribs.core.view.ViewFactory import com.jakewharton.rxrelay2.BehaviorRelay import com.jakewharton.rxrelay2.PublishRelay +import io.reactivex.BackpressureStrategy +import io.reactivex.Flowable +import io.reactivex.Maybe import io.reactivex.Observable import io.reactivex.Single +import kotlin.reflect.KClass open class RxWorkflowNode( buildParams: BuildParams<*>, @@ -21,55 +25,91 @@ open class RxWorkflowNode( plugins = plugins ) { - private val childrenAttachesRelay: PublishRelay>? = PublishRelay.create() - val childrenAttaches: Observable>? = childrenAttachesRelay?.hide() - val detachSignal: BehaviorRelay = BehaviorRelay.create() + private val childrenAttachesRelay: PublishRelay> = PublishRelay.create() + private val detachSignalRelay: BehaviorRelay = BehaviorRelay.create() + + protected val childrenAttaches: Observable> = childrenAttachesRelay.hide() + + // Flowable to use in takeUntil() + protected val detachSignal: Flowable = detachSignalRelay.toFlowable(BackpressureStrategy.LATEST) override fun onAttachChildNode(child: Node<*>) { super.onAttachChildNode(child) - childrenAttachesRelay?.accept(child) + childrenAttachesRelay.accept(child) } override fun onDestroy(isRecreating: Boolean) { super.onDestroy(isRecreating) - detachSignal.accept(Unit) + detachSignalRelay.accept(Unit) } /** - * Executes an action and remains on the same hierarchical level + * Executes an action and remains on the same hierarchical level. * * @return the current workflow element + * @throws NodeIsNotAvailableForWorkflowException when execution is not possible */ - protected inline fun executeWorkflow( - crossinline action: () -> Unit - ): Single = Single.fromCallable { - action() - this as T + @Suppress("UNCHECKED_CAST") + protected fun executeWorkflow( + action: () -> Unit + ): Single = Single.defer { + throwExceptionSingleIfDestroyed() ?: Single.fromCallable { + action() + this as T + } } - .takeUntil(detachSignal.firstOrError()) - @VisibleForTesting - internal inline fun executeWorkflowInternal( - crossinline action: () -> Unit - ): Single = executeWorkflow(action) + /** + * Executes an action and remains on the same hierarchical level. + * If execution is not possible returns nothing. + * + * @return the current workflow element + */ + protected fun maybeExecuteWorkflow( + action: () -> Unit + ): Maybe = + executeWorkflow(action) + .toMaybe() + .onErrorComplete { it is NodeIsNotAvailableForWorkflowException } + .takeUntil(detachSignal) /** - * Executes an action and transitions to another workflow element + * Executes an action and transitions to another workflow element. * * @param action an action that's supposed to result in the attach of a child (e.g. router.push()) + * @return the child as the expected workflow element, or error if expected child was not found + * @throws NodeIsNotAvailableForWorkflowException when execution is not possible + */ + protected inline fun attachWorkflow( + noinline action: () -> Unit + ): Single = attachWorkflow(T::class, action) + + /** + * Executes an action and transitions to another workflow element. + * If execution is not possible returns nothing. * + * @param action an action that's supposed to result in the attach of a child (e.g. router.push()) * @return the child as the expected workflow element, or error if expected child was not found */ - @SuppressWarnings("LongMethod") - protected inline fun attachWorkflow( - crossinline action: () -> Unit - ): Single = Single.fromCallable { + protected inline fun maybeAttachWorkflow( + noinline action: () -> Unit + ): Maybe = + attachWorkflow(T::class, action) + .toMaybe() + .onErrorComplete { it is NodeIsNotAvailableForWorkflowException } + .takeUntil(detachSignal) + + protected fun attachWorkflow( + clazz: KClass, + action: () -> Unit + ): Single = Single.defer { + throwExceptionSingleIfDestroyed()?.also { return@defer it } action() - val childNodesOfExpectedType = children.filterIsInstance() + val childNodesOfExpectedType = children.filterIsInstance(clazz.java) if (childNodesOfExpectedType.isEmpty()) { - Single.error( + Single.error( IllegalStateException( - "Expected child of type [${T::class.java}] was not found after executing action. " + + "Expected child of type [${clazz.java}] was not found after executing action. " + "Check that your action actually results in the expected child. " + "Child count: ${children.size}. " + "Last child is: [${children.lastOrNull()}]. " + @@ -80,33 +120,57 @@ open class RxWorkflowNode( Single.just(childNodesOfExpectedType.last()) } } - .flatMap { it } - .takeUntil(detachSignal.firstOrError()) - @VisibleForTesting - internal inline fun attachWorkflowInternal( - crossinline action: () -> Unit - ): Single = attachWorkflow(action) + /** + * Waits until a certain child is attached and returns it as the expected workflow element, + * or returns it immediately if it's already available. + * + * @return the child as the expected workflow element + * @throws NodeIsNotAvailableForWorkflowException when execution is not possible + */ + protected inline fun waitForChildAttached(): Single = + waitForChildAttached(T::class) /** - * Waits until a certain child is attached and returns it as the expected workflow element, or - * returns it immediately if it's already available. + * Waits until a certain child is attached and returns it as the expected workflow element, + * or returns it immediately if it's already available. + * If execution is not possible returns nothing. * * @return the child as the expected workflow element */ - protected inline fun waitForChildAttached(): Single = - Single.fromCallable { - val childNodesOfExpectedType = children.filterIsInstance() + protected inline fun maybeWaitForChildAttached(): Maybe = + waitForChildAttached(T::class) + .toMaybe() + .onErrorComplete { it is NodeIsNotAvailableForWorkflowException } + .takeUntil(detachSignal) + + protected fun waitForChildAttached( + clazz: KClass, + ): Single = + Single.defer { + throwExceptionSingleIfDestroyed()?.also { return@defer it } + val childNodesOfExpectedType = children.filterIsInstance(clazz.java) if (childNodesOfExpectedType.isEmpty()) { - childrenAttaches?.ofType(T::class.java)?.firstOrError() + childrenAttaches.ofType(clazz.java)?.firstOrError() } else { Single.just(childNodesOfExpectedType.last()) } } - .flatMap { it } - .takeUntil(detachSignal.firstOrError()) - @VisibleForTesting - internal inline fun waitForChildAttachedInternal(): Single = - waitForChildAttached() + private fun skipExecutionIfDestroyed(): Maybe? = + if (lifecycle.currentState > Lifecycle.State.DESTROYED) { + Maybe.empty() + } else { + null + } + + private fun throwExceptionSingleIfDestroyed(): Single? = + if (lifecycle.currentState == Lifecycle.State.DESTROYED) { + Single.error(NodeIsNotAvailableForWorkflowException("Node $this is already destroyed, further execution is meaningless")) + } else { + null + } + + class NodeIsNotAvailableForWorkflowException(message: String) : IllegalStateException(message) + } diff --git a/libraries/rib-rx2/src/test/java/com/badoo/ribs/rx2/workflows/WorkflowTest.kt b/libraries/rib-rx2/src/test/java/com/badoo/ribs/rx2/workflows/WorkflowTest.kt index 548a7b725..3f6230a74 100644 --- a/libraries/rib-rx2/src/test/java/com/badoo/ribs/rx2/workflows/WorkflowTest.kt +++ b/libraries/rib-rx2/src/test/java/com/badoo/ribs/rx2/workflows/WorkflowTest.kt @@ -13,12 +13,13 @@ import com.badoo.ribs.core.view.RibView import com.badoo.ribs.core.view.ViewFactory import com.badoo.ribs.routing.Routing import com.badoo.ribs.routing.router.Router +import com.badoo.ribs.rx2.workflows.RxWorkflowNode.NodeIsNotAvailableForWorkflowException import com.badoo.ribs.util.RIBs import com.nhaarman.mockitokotlin2.argThat import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.mock +import io.reactivex.Maybe import io.reactivex.Single -import io.reactivex.observers.TestObserver import kotlinx.parcelize.Parcelize import org.junit.Assert import org.junit.jupiter.api.AfterEach @@ -26,7 +27,7 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test class WorkflowTest { - private lateinit var node: RxWorkflowNode + private lateinit var node: TestRxWorkflowNode private lateinit var view: TestView private lateinit var androidView: ViewGroup private lateinit var parentView: RibView @@ -57,7 +58,7 @@ class WorkflowTest { buildParams: BuildParams = testBuildParams(), viewFactory: ViewFactory = this.viewFactory, plugins: List = emptyList() - ): RxWorkflowNode = RxWorkflowNode( + ): TestRxWorkflowNode = TestRxWorkflowNode( buildParams = buildParams, viewFactory = viewFactory, plugins = plugins @@ -89,73 +90,124 @@ class WorkflowTest { @Test fun `executeWorkflow executes action on subscribe`() { - var actionInvoked = false - val action = { actionInvoked = true } + val action = InvokableStub() val workflow: Single> = node.executeWorkflowInternal(action) - val testObserver = TestObserver>() - workflow.subscribe(testObserver) + val testObserver = workflow.test() - Assert.assertEquals(true, actionInvoked) - testObserver.assertValue(node) - testObserver.assertComplete() + action.assertInvoked() + testObserver.assertResult(node) } @Test fun `executeWorkflow never executes action on lifecycle terminate before subscribe`() { node.onDestroy(isRecreating = false) - var actionInvoked = false - val action = { actionInvoked = true } + val action = InvokableStub() val workflow: Single> = node.executeWorkflowInternal(action) - val testObserver = TestObserver>() - workflow.subscribe(testObserver) + val testObserver = workflow.test() - Assert.assertEquals(false, actionInvoked) - testObserver.assertNever(node) - testObserver.assertNotComplete() + action.assertNotInvoked() + testObserver.assertFailure(NodeIsNotAvailableForWorkflowException::class.java) + } + + @Test + fun `executeWorkflow does not interrupt further actions on lifecycle terminate`() { + val action = InvokableStub() + val workflow: Single = + node + .executeWorkflowInternal>(action) + .flatMap { Single.never() } + val testObserver = workflow.test() + + node.onDestroy(isRecreating = false) + + action.assertInvoked() + testObserver.assertNoValues() + testObserver.assertNotTerminated() + } + + @Test + fun `maybeExecuteWorkflow never executes action on lifecycle terminate before subscribe`() { + node.onDestroy(isRecreating = false) + + val action = InvokableStub() + val workflow: Maybe> = node.maybeExecuteWorkflowInternal(action) + val testObserver = workflow.test() + + action.assertNotInvoked() + testObserver.assertResult() } @Test fun `attachWorkflow executes action on subscribe`() { - var actionInvoked = false - val action = { actionInvoked = true } + val action = InvokableStub() val workflow: Single = node.attachWorkflowInternal(action) - val testObserver = TestObserver() - workflow.subscribe(testObserver) + val testObserver = workflow.test() - Assert.assertEquals(true, actionInvoked) - testObserver.assertValue(child3) - testObserver.assertComplete() + action.assertInvoked() + testObserver.assertResult(child3) } @Test fun `attachWorkflow never executes action on lifecycle terminate before subscribe`() { node.onDestroy(isRecreating = false) - var actionInvoked = false - val action = { actionInvoked = true } + val action = InvokableStub() val workflow: Single = node.attachWorkflowInternal(action) - val testObserver = TestObserver() - workflow.subscribe(testObserver) + val testObserver = workflow.test() + + action.assertNotInvoked() + testObserver.assertFailure(NodeIsNotAvailableForWorkflowException::class.java) + } - Assert.assertEquals(false, actionInvoked) - testObserver.assertNever(child1) - testObserver.assertNever(child2) - testObserver.assertNever(child3) - testObserver.assertNotComplete() + @Test + fun `attachWorkflow does not interrupt further actions on lifecycle terminate`() { + val action = InvokableStub() + val workflow: Single = + node + .attachWorkflowInternal>(action) + .flatMap { Single.never() } + val testObserver = workflow.test() + + node.onDestroy(isRecreating = false) + + action.assertInvoked() + testObserver.assertNoValues() + testObserver.assertNotTerminated() + } + + @Test + fun `maybeAttachWorkflow never executes action on lifecycle terminate before subscribe`() { + node.onDestroy(isRecreating = false) + + val action = InvokableStub() + val workflow: Maybe = node.maybeAttachWorkflowInternal(action) + val testObserver = workflow.test() + + action.assertNotInvoked() + testObserver.assertResult() } @Test fun `waitForChildAttached emits expected child immediately if it's already attached`() { + val testChildNode = TestNode2(buildParams = testBuildParams(ancestryInfo = childAncestry)) + + node.attachChildNode(testChildNode) val workflow: Single = node.waitForChildAttachedInternal() - val testObserver = TestObserver() + val testObserver = workflow.test() + + testObserver.assertResult(testChildNode) + } + + @Test + fun `waitForChildAttached emits expected child after it is attached`() { val testChildNode = TestNode2(buildParams = testBuildParams(ancestryInfo = childAncestry)) + val workflow: Single = node.waitForChildAttachedInternal() + val testObserver = workflow.test() node.attachChildNode(testChildNode) - workflow.subscribe(testObserver) - testObserver.assertValue(testChildNode) - testObserver.assertComplete() + testObserver.assertResult(testChildNode) } @Test @@ -163,11 +215,49 @@ class WorkflowTest { node.onDestroy(isRecreating = false) val workflow: Single = node.waitForChildAttachedInternal() - val testObserver = TestObserver() - workflow.subscribe(testObserver) + val testObserver = workflow.test() + + testObserver.assertFailure(NodeIsNotAvailableForWorkflowException::class.java) + } + + @Test + fun `waitForChildAttached does not interrupt further actions on lifecycle terminate`() { + val workflow: Single = + node + .waitForChildAttachedInternal>() + .flatMap { Single.never() } + val testObserver = workflow.test() + + node.onDestroy(isRecreating = false) testObserver.assertNoValues() - testObserver.assertNotComplete() + testObserver.assertNotTerminated() + } + + @Test + fun `maybeWaitForChildAttached never executes action on lifecycle terminate before subscribe`() { + node.onDestroy(isRecreating = false) + + val workflow: Maybe = node.maybeWaitForChildAttachedInternal() + val testObserver = workflow.test() + + testObserver.assertResult() + } + + private class InvokableStub : () -> Unit { + var isInvoked: Boolean = false + + override fun invoke() { + isInvoked = true + } + + fun assertInvoked() { + Assert.assertEquals("Expected to be invoked", true, isInvoked) + } + + fun assertNotInvoked() { + Assert.assertEquals("Expected to be not invoked", false, isInvoked) + } } private class TestNode( @@ -192,6 +282,35 @@ class WorkflowTest { plugins = plugins + listOf(router) ) + private class TestRxWorkflowNode( + buildParams: BuildParams<*>, + viewFactory: ViewFactory?, + plugins: List, + ) : RxWorkflowNode(buildParams, viewFactory, plugins) { + inline fun waitForChildAttachedInternal(): Single = + waitForChildAttached() + + inline fun maybeWaitForChildAttachedInternal(): Maybe = + maybeWaitForChildAttached() + + inline fun executeWorkflowInternal( + noinline action: () -> Unit + ): Single = executeWorkflow(action) + + inline fun maybeExecuteWorkflowInternal( + noinline action: () -> Unit + ): Maybe = maybeExecuteWorkflow(action) + + inline fun attachWorkflowInternal( + noinline action: () -> Unit + ): Single = attachWorkflow(action) + + inline fun maybeAttachWorkflowInternal( + noinline action: () -> Unit + ): Maybe = maybeAttachWorkflow(action) + + } + private class TestView : AndroidRibView() { override val androidView: ViewGroup = mock() }