safely remove navigables

wealthfront · Oct 24, 2024 · c9fa578 · c9fa578
1 parent 8ad37ae
commit c9fa578
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 2 deletions.
diff --git a/magellan-library/src/main/java/com/wealthfront/magellan/navigation/LazySetNavigator.kt b/magellan-library/src/main/java/com/wealthfront/magellan/navigation/LazySetNavigator.kt
@@ -1,7 +1,9 @@
 package com.wealthfront.magellan.navigation
 
 import android.content.Context
+import android.os.Build
 import android.view.View
+import androidx.annotation.RequiresApi
 import androidx.annotation.VisibleForTesting
 import com.wealthfront.magellan.Direction
 import com.wealthfront.magellan.ScreenContainer
@@ -43,15 +45,19 @@ public open class LazySetNavigator(
     lifecycleRegistry.attachToLifecycleWithMaxState(navigable, LifecycleLimit.CREATED)
   }
 
+  @RequiresApi(Build.VERSION_CODES.N)
   public fun removeNavigables(navigables: Set<NavigableCompat>) {
     for (navigable in navigables) {
       removeNavigable(navigable)
     }
   }
 
+  @RequiresApi(Build.VERSION_CODES.N)
   public fun removeNavigable(navigable: NavigableCompat) {
-    existingNavigables.remove(navigable)
-    lifecycleRegistry.removeFromLifecycle(navigable)
+    existingNavigables.removeIf { it == navigable }
+    if (lifecycleRegistry.children.contains(navigable)) {
+      lifecycleRegistry.removeFromLifecycle(navigable)
+    }
   }
 
   public fun safeAddNavigable(navigable: NavigableCompat) {
@@ -87,6 +93,7 @@ public open class LazySetNavigator(
 
   override fun onDestroy(context: Context) {
     lifecycleRegistry.children.forEach { lifecycleRegistry.removeFromLifecycle(it) }
+    existingNavigables.clear()
   }
 
   public fun replace(

diff --git a/magellan-library/src/test/java/com/wealthfront/magellan/navigation/LazySetNavigatorTest.kt b/magellan-library/src/test/java/com/wealthfront/magellan/navigation/LazySetNavigatorTest.kt
@@ -182,6 +182,36 @@ class LazySetNavigatorTest {
     verify(exactly = 0) { navigableListener.onNavigatedTo(any()) }
   }
 
+  @Test
+  fun attemptRemoveNavigables_afterOnDestroy() {
+    navigator.addNavigables(setOf(step1, step2, step3, step4))
+    navigator.transitionToState(LifecycleState.Resumed(activityController.get()))
+
+    navigator.replace(step1, CrossfadeTransition())
+    step1.view!!.viewTreeObserver.dispatchOnPreDraw()
+    shadowOf(Looper.getMainLooper()).idle()
+    assertThat(navigator.containerView!!.childCount).isEqualTo(1)
+    assertThat(navigator.containerView!!.getChildAt(0)).isEqualTo(step1.view)
+    assertThat(step1.currentState).isInstanceOf(LifecycleState.Resumed::class.java)
+    assertThat(step2.currentState).isInstanceOf(LifecycleState.Created::class.java)
+    assertThat(step3.currentState).isInstanceOf(LifecycleState.Created::class.java)
+    assertThat(step4.currentState).isInstanceOf(LifecycleState.Created::class.java)
+    assertThat(navigator.existingNavigables).containsExactly(step1, step2, step3, step4)
+
+    verify { navigableListener.beforeNavigation() }
+    verify(exactly = 0) { navigableListener.onNavigatedFrom(any()) }
+    verify { navigableListener.onNavigatedTo(step1) }
+    verify { navigableListener.afterNavigation() }
+    clearMocks(navigableListener)
+    initMocks()
+
+    navigator.transitionToState(LifecycleState.Destroyed)
+    navigator.removeNavigables(setOf(step2, step4))
+
+    assertThat(navigator.existingNavigables).isEmpty()
+    verify(exactly = 0) { navigableListener.onNavigatedTo(any()) }
+  }
+
   @Test
   fun updateMultipleNavigables() {
     navigator.updateNavigables(setOf(step1, step2)) {}