Fix potential bug with stale transition. (#356)

instacart · Mar 15, 2024 · 66b58e1 · 66b58e1
1 parent 4cae107
commit 66b58e1
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 15 deletions.
diff --git a/formula/src/main/java/com/instacart/formula/internal/DeferredTransition.kt b/formula/src/main/java/com/instacart/formula/internal/DeferredTransition.kt
@@ -1,7 +1,5 @@
 package com.instacart.formula.internal
 
-import com.instacart.formula.Transition
-
 /**
  * A deferred transition contains an [event] and a related [transition] which can
  * be executed using the [execute] function. If the formula is ready, it will be
@@ -11,11 +9,10 @@ import com.instacart.formula.Transition
  */
 class DeferredTransition<Input, State, EventT> internal constructor(
     private val listener: ListenerImpl<Input, State, EventT>,
-    private val transition: Transition<Input, State, EventT>,
     private val event: EventT,
 ) {
 
     fun execute() {
-        listener.snapshotImpl?.dispatch(transition, event)
+        listener.applyInternal(event)
     }
 }
diff --git a/formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt b/formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt
@@ -8,13 +8,13 @@ import com.instacart.formula.plugin.Dispatcher
  * Note: this class is not a data class because equality is based on instance and not [key].
  */
 @PublishedApi
-internal class ListenerImpl<Input, State, EventT>(internal var key: Any) : Listener<EventT> {
+internal class ListenerImpl<Input, State, EventT>(val key: Any) : Listener<EventT> {
 
-    @Volatile internal var manager: FormulaManagerImpl<Input, State, *>? = null
-    @Volatile internal var snapshotImpl: SnapshotImpl<Input, State>? = null
-    @Volatile internal var executionType: Transition.ExecutionType? = null
+    @Volatile private var manager: FormulaManagerImpl<Input, State, *>? = null
+    @Volatile private var snapshotImpl: SnapshotImpl<Input, State>? = null
+    @Volatile private var executionType: Transition.ExecutionType? = null
 
-    internal lateinit var transition: Transition<Input, State, EventT>
+    private lateinit var transition: Transition<Input, State, EventT>
 
     override fun invoke(event: EventT) {
         // TODO: log if null listener (it might be due to formula removal or due to callback removal)
@@ -29,16 +29,32 @@ internal class ListenerImpl<Input, State, EventT>(internal var key: Any) : Liste
         }
     }
 
+    fun setDependencies(
+        manager: FormulaManagerImpl<Input, State, *>?,
+        snapshot: SnapshotImpl<Input, State>?,
+        executionType: Transition.ExecutionType?,
+        transition: Transition<Input, State, EventT>,
+    ) {
+        this.manager = manager
+        this.snapshotImpl = snapshot
+        this.executionType = executionType
+        this.transition = transition
+    }
+
     fun disable() {
         manager = null
         snapshotImpl = null
     }
 
+    fun applyInternal(event: EventT) {
+        snapshotImpl?.dispatch(transition, event)
+    }
+
     private fun handleBatched(type: Transition.Batched, event: EventT) {
         val manager = manager ?: return
 
         manager.batchManager.add(type, event) {
-            val deferredTransition = DeferredTransition(this, transition, event)
+            val deferredTransition = DeferredTransition(this, event)
             manager.onPendingTransition(deferredTransition)
         }
     }
@@ -48,7 +64,7 @@ internal class ListenerImpl<Input, State, EventT>(internal var key: Any) : Liste
 
         dispatcher.dispatch {
             manager.queue.postUpdate {
-                val deferredTransition = DeferredTransition(this, transition, event)
+                val deferredTransition = DeferredTransition(this, event)
                 manager.onPendingTransition(deferredTransition)
             }
         }

diff --git a/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt b/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt
@@ -53,10 +53,7 @@ internal class SnapshotImpl<out Input, State> internal constructor(
     ): Listener<Event> {
         ensureNotRunning()
         val listener = listeners.initOrFindListener<Input, State, Event>(key, useIndex)
-        listener.manager = delegate
-        listener.snapshotImpl = this
-        listener.executionType = executionType
-        listener.transition = transition
+        listener.setDependencies(delegate, this, executionType, transition)
         return listener
     }