Merge pull request #675 from square/ray/non-null-parent

Truth in advertising: findParentLifecycle cannot return null

Author: rjrjr

workflow-ui/container-android/src/main/java/com/squareup/workflow1/ui/modal/ModalContainer.kt

@@ -48,7 +48,10 @@ public abstract class ModalContainer<ModalRenderingT : Any> @JvmOverloads constr
    * this container's modals. Only valid after the the view has been attached.
    */
   private val parentLifecycleOwner by lazy(mode = LazyThreadSafetyMode.NONE) {
-    WorkflowLifecycleOwner.get(this)
+    WorkflowLifecycleOwner.get(this) ?: error(
+      "Expected to find either a ViewTreeLifecycleOwner in the view tree, or for the " +
+        "context to be a LifecycleOwner, in $this"
+    )
   }
 
   /**
@@ -83,7 +86,7 @@ public abstract class ModalContainer<ModalRenderingT : Any> @JvmOverloads constr
             // any, and so we can use our lifecycle to destroy-on-detach the dialog hierarchy.
             WorkflowLifecycleOwner.installOn(
               dialogView,
-              findParentLifecycle = { parentLifecycleOwner?.lifecycle }
+              findParentLifecycle = { parentLifecycleOwner.lifecycle }
             )
             // Ensure that each dialog has its own ViewTreeSavedStateRegistryOwner,
             // so views in each dialog layer don't clash with other layers.
@@ -101,10 +104,11 @@ public abstract class ModalContainer<ModalRenderingT : Any> @JvmOverloads constr
                 override fun onViewAttachedToWindow(v: View) {
                   // Note this is a different lifecycle than the WorkflowLifecycleOwner – it will
                   // probably be the owning AppCompatActivity.
-                  lifecycle = parentLifecycleOwner?.lifecycle
-                  // Android makes a lot of logcat noise if it has to close the window for us. :/
-                  // https://github.com/square/workflow/issues/51
-                  lifecycle?.addObserver(dismissOnDestroy)
+                  lifecycle = parentLifecycleOwner.lifecycle.also {
+                    // Android makes a lot of logcat noise if it has to close the window for us. :/
+                    // https://github.com/square/workflow/issues/51
+                    it.addObserver(dismissOnDestroy)
+                  }
                 }
 
                 override fun onViewDetachedFromWindow(v: View) {

workflow-ui/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowLifecycleOwner.kt

@@ -74,7 +74,7 @@ public interface WorkflowLifecycleOwner : LifecycleOwner {
      */
     public fun installOn(
       view: View,
-      findParentLifecycle: (View) -> Lifecycle? = { v -> findParentViewTreeLifecycle(v) }
+      findParentLifecycle: (View) -> Lifecycle = { v -> findParentViewTreeLifecycle(v) }
     ) {
       RealWorkflowLifecycleOwner(findParentLifecycle).also {
         ViewTreeLifecycleOwner.set(view, it)
@@ -90,9 +90,10 @@ public interface WorkflowLifecycleOwner : LifecycleOwner {
     public fun get(view: View): WorkflowLifecycleOwner? =
       ViewTreeLifecycleOwner.get(view) as? WorkflowLifecycleOwner
 
-    private fun findParentViewTreeLifecycle(view: View): Lifecycle? {
+    private fun findParentViewTreeLifecycle(view: View): Lifecycle {
       // Start at our view's parent – if we look on our own view, we'll just get this back.
       return (view.parent as? View)?.let(::lifecycleOwnerFromViewTreeOrContextOrNull)?.lifecycle
+        ?: error("Expected parent or context of $view to have or be a ViewTreeLifecycleOwner")
     }
   }
 }
@@ -105,7 +106,7 @@ public interface WorkflowLifecycleOwner : LifecycleOwner {
 @OptIn(WorkflowUiExperimentalApi::class)
 @VisibleForTesting(otherwise = PRIVATE)
 internal class RealWorkflowLifecycleOwner(
-  private val findParentLifecycle: (View) -> Lifecycle?,
+  private val findParentLifecycle: (View) -> Lifecycle,
   enforceMainThread: Boolean = true,
 ) : WorkflowLifecycleOwner,
   LifecycleOwner,
@@ -157,10 +158,7 @@ internal class RealWorkflowLifecycleOwner(
 
     // Always check for a new parent, in case we're attached to different part of the view tree.
     val oldLifecycle = parentLifecycle
-    parentLifecycle = checkNotNull(findParentLifecycle(v)) {
-      "Expected to find either a ViewTreeLifecycleOwner in the view tree, or for the view's" +
-        " context to be a LifecycleOwner."
-    }
+    parentLifecycle = findParentLifecycle(v)
 
     if (parentLifecycle !== oldLifecycle) {
       oldLifecycle?.removeObserver(this)

workflow-ui/core-android/src/test/java/com/squareup/workflow1/ui/androidx/RealWorkflowLifecycleOwnerTest.kt

@@ -17,7 +17,6 @@ import org.junit.Test
 import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.whenever
-import kotlin.test.assertFailsWith
 
 internal class RealWorkflowLifecycleOwnerTest {
 
@@ -28,23 +27,13 @@ internal class RealWorkflowLifecycleOwnerTest {
   private var parentLifecycle: LifecycleRegistry? = null
   private val owner = RealWorkflowLifecycleOwner(
     enforceMainThread = false,
-    findParentLifecycle = { parentLifecycle }
+    findParentLifecycle = { parentLifecycle!! }
   )
 
   @Test fun `lifecycle starts initialized`() {
     assertThat(owner.lifecycle.currentState).isEqualTo(INITIALIZED)
   }
 
-  @Test fun `attach throws when no parent lifecycle found`() {
-    val error = assertFailsWith<IllegalStateException> {
-      makeViewAttached()
-    }
-    assertThat(error.message).isEqualTo(
-      "Expected to find either a ViewTreeLifecycleOwner in the view tree, or for the" +
-        " view's context to be a LifecycleOwner."
-    )
-  }
-
   @Test fun `lifecycle not destroyed when detached`() {
     ensureParentLifecycle()
     makeViewAttached()