Increase code coverage (pt2).

instacart · Sep 12, 2024 · 43329d8 · 43329d8
1 parent 9a9a62f
commit 43329d8
Show file tree

Hide file tree

Showing 14 changed files with 236 additions and 90 deletions.
diff --git a/formula-android/src/main/java/com/instacart/formula/Renderer.kt b/formula-android/src/main/java/com/instacart/formula/Renderer.kt
@@ -12,24 +12,12 @@ package com.instacart.formula
  * renderText("three")
  * ```
  */
-class Renderer<in RenderModel> private constructor(
+class Renderer<in RenderModel>(
     private val renderFunction: (RenderModel) -> Unit
 ) : (RenderModel) -> Unit {
 
     companion object {
 
-        /**
-         * Creates a render function that does nothing.
-         */
-        fun <T> empty() = create<T> { }
-
-        /**
-         * Creates a render function.
-         */
-        operator fun <RenderModel> invoke(render: (RenderModel) -> Unit): Renderer<RenderModel> {
-            return Renderer(renderFunction = render)
-        }
-
         /**
          * Creates a render function.
          */

diff --git a/formula-test/src/main/java/com/instacart/formula/test/RxJavaFormulaTestDelegate.kt b/formula-test/src/main/java/com/instacart/formula/test/RxJavaFormulaTestDelegate.kt
@@ -2,8 +2,6 @@ package com.instacart.formula.test
 
 import com.instacart.formula.IFormula
 import com.instacart.formula.RuntimeConfig
-import com.instacart.formula.plugin.Dispatcher
-import com.instacart.formula.plugin.Inspector
 import com.instacart.formula.rxjava3.toObservable
 import io.reactivex.rxjava3.subjects.BehaviorSubject
 
@@ -12,16 +10,8 @@ import io.reactivex.rxjava3.subjects.BehaviorSubject
  */
 class RxJavaFormulaTestDelegate<Input : Any, Output : Any, FormulaT : IFormula<Input, Output>>(
     override val formula: FormulaT,
-    isValidationEnabled: Boolean = true,
-    inspector: Inspector?,
-    dispatcher: Dispatcher?,
+    runtimeConfig: RuntimeConfig,
 ) : FormulaTestDelegate<Input, Output, FormulaT> {
-    private val runtimeConfig = RuntimeConfig(
-        isValidationEnabled = isValidationEnabled,
-        inspector = inspector,
-        defaultDispatcher = dispatcher,
-    )
-
     private val inputRelay = BehaviorSubject.create<Input>()
     private val observer = formula
         .toObservable(inputRelay, runtimeConfig)

diff --git a/formula-test/src/main/java/com/instacart/formula/test/TestRuntimeExtensions.kt b/formula-test/src/main/java/com/instacart/formula/test/TestRuntimeExtensions.kt
@@ -2,6 +2,7 @@ package com.instacart.formula.test
 
 import com.instacart.formula.Action
 import com.instacart.formula.IFormula
+import com.instacart.formula.RuntimeConfig
 import com.instacart.formula.plugin.Dispatcher
 import com.instacart.formula.plugin.Inspector
 
@@ -15,7 +16,13 @@ fun <Input : Any, Output : Any, F: IFormula<Input, Output>> F.test(
     inspector: Inspector? = null,
     dispatcher: Dispatcher? = null,
 ): TestFormulaObserver<Input, Output, F> {
-    val delegate = RxJavaFormulaTestDelegate(this, isValidationEnabled, inspector, dispatcher)
+    val runtimeConfig = RuntimeConfig(
+        isValidationEnabled = isValidationEnabled,
+        inspector = inspector,
+        defaultDispatcher = dispatcher,
+    )
+
+    val delegate = RxJavaFormulaTestDelegate(this, runtimeConfig)
     return TestFormulaObserver(delegate)
 }
 

diff --git a/formula/src/main/java/com/instacart/formula/internal/ChildrenManager.kt b/formula/src/main/java/com/instacart/formula/internal/ChildrenManager.kt
@@ -3,7 +3,6 @@ package com.instacart.formula.internal
 import com.instacart.formula.FormulaPlugins
 import com.instacart.formula.IFormula
 import com.instacart.formula.plugin.Inspector
-import java.lang.IllegalStateException
 
 /**
  * Keeps track of child formula managers.
@@ -12,7 +11,7 @@ internal class ChildrenManager(
     private val delegate: FormulaManagerImpl<*, *, *>,
     private val inspector: Inspector?,
 ) {
-    private var children: SingleRequestMap<Any, FormulaManager<*, *>>? = null
+    private val children: SingleRequestMap<Any, FormulaManager<*, *>> = LinkedHashMap()
     private var indexes: MutableMap<Any, Int>? = null
     private var pendingRemoval: MutableList<FormulaManager<*, *>>? = null
 
@@ -26,7 +25,7 @@ internal class ChildrenManager(
     fun prepareForPostEvaluation() {
         indexes?.clear()
 
-        children?.clearUnrequested(this::prepareForTermination)
+        children.clearUnrequested(this::prepareForTermination)
     }
 
     fun terminateChildren(evaluationId: Long): Boolean {
@@ -42,20 +41,20 @@ internal class ChildrenManager(
     }
 
     fun markAsTerminated() {
-        children?.forEachValue { it.markAsTerminated() }
+        children.forEachValue { it.markAsTerminated() }
     }
 
     fun performTerminationSideEffects(executeTransitionQueue: Boolean) {
-        children?.forEachValue { it.performTerminationSideEffects(executeTransitionQueue) }
+        children.forEachValue { it.performTerminationSideEffects(executeTransitionQueue) }
     }
 
     fun <ChildInput, ChildOutput> findOrInitChild(
         key: Any,
         formula: IFormula<ChildInput, ChildOutput>,
         input: ChildInput,
     ): FormulaManager<ChildInput, ChildOutput> {
-        val childHolder = childFormulaHolder(key, formula, input)
-        return if (childHolder.requested) {
+        val childManagerEntry = getOrInitChildManager(key, formula, input)
+        return if (childManagerEntry.requested) {
             val logs = duplicateKeyLogs ?: run {
                 val newSet = mutableSetOf<Any>()
                 duplicateKeyLogs = newSet
@@ -76,38 +75,27 @@ internal class ChildrenManager(
                 )
             }
 
-            if (key is IndexedKey) {
-                // This should never happen, but added as safety
-                throw IllegalStateException("Key already indexed (and still duplicate).")
-            }
-
             val index = nextIndex(key)
             val indexedKey = IndexedKey(key, index)
             findOrInitChild(indexedKey, formula, input)
         } else {
-            childHolder.requestAccess {
-                "There already is a child with same key: $key. Override [Formula.key] function."
-            }
+            childManagerEntry.requested = true
+            childManagerEntry.value
         }
     }
 
     private fun prepareForTermination(it: FormulaManager<*, *>) {
-        pendingRemoval = pendingRemoval ?: mutableListOf()
+        val list = pendingRemoval ?: mutableListOf()
+        pendingRemoval = list
         it.markAsTerminated()
-        pendingRemoval?.add(it)
+        list.add(it)
     }
 
-    private fun <ChildInput, ChildOutput>  childFormulaHolder(
+    private fun <ChildInput, ChildOutput> getOrInitChildManager(
         key: Any,
         formula: IFormula<ChildInput, ChildOutput>,
         input: ChildInput,
     ): SingleRequestHolder<FormulaManager<ChildInput, ChildOutput>> {
-        val children = children ?: run {
-            val initialized: SingleRequestMap<Any, FormulaManager<*, *>> = LinkedHashMap()
-            this.children = initialized
-            initialized
-        }
-
         val childFormulaHolder = children.findOrInit(key) {
             val implementation = formula.implementation
             FormulaManagerImpl(
@@ -136,7 +124,12 @@ internal class ChildrenManager(
             initialized
         }
 
-        val index = indexes.getOrElse(key) { 0 } + 1
+        val previousIndex = indexes[key]
+        val index = if (previousIndex == null) {
+            0
+        } else {
+            previousIndex + 1
+        }
         indexes[key] = index
         return index
     }

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,7 +8,7 @@ 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>(val key: Any) : Listener<EventT> {
+internal class ListenerImpl<Input, State, EventT>(private val key: Any) : Listener<EventT> {
 
     @Volatile private var manager: FormulaManagerImpl<Input, State, *>? = null
     @Volatile private var snapshotImpl: SnapshotImpl<Input, State>? = null

diff --git a/formula/src/main/java/com/instacart/formula/internal/Listeners.kt b/formula/src/main/java/com/instacart/formula/internal/Listeners.kt
@@ -1,31 +1,20 @@
 package com.instacart.formula.internal
 
-import java.lang.IllegalStateException
-
 internal class Listeners {
     private var listeners: SingleRequestMap<Any, ListenerImpl<*, *, *>>? = null
     private var indexes: MutableMap<Any, Int>? = null
 
-    private fun duplicateKeyErrorMessage(key: Any): String {
-        return "Listener $key is already defined. Unexpected issue."
-    }
-
     fun <Input, State, Event> initOrFindListener(key: Any, useIndex: Boolean): ListenerImpl<Input, State, Event> {
         val currentHolder = listenerHolder<Input, State, Event>(key)
-        return if (currentHolder.requested && useIndex) {
-            if (key is IndexedKey) {
-                // This should never happen, but added as safety
-                throw IllegalStateException("Key already indexed (and still duplicate).")
-            }
-
+        return if (!currentHolder.requested) {
+            currentHolder.requested = true
+            currentHolder.value as ListenerImpl<Input, State, Event>
+        } else if (useIndex) {
             val index = nextIndex(key)
             val indexedKey = IndexedKey(key, index)
             initOrFindListener(indexedKey, useIndex)
         } else {
-            currentHolder
-                .requestAccess {
-                    duplicateKeyErrorMessage(currentHolder.value.key)
-                } as ListenerImpl<Input, State, Event>
+            throw IllegalStateException("Listener $key is already defined. Unexpected issue.")
         }
     }
 
@@ -61,7 +50,12 @@ internal class Listeners {
             initialized
         }
 
-        val index = indexes.getOrElse(key) { 0 } + 1
+        val previousIndex = indexes[key]
+        val index = if (previousIndex == null) {
+            0
+        } else {
+            previousIndex + 1
+        }
         indexes[key] = index
         return index
     }

diff --git a/formula/src/main/java/com/instacart/formula/internal/SingleRequestHolder.kt b/formula/src/main/java/com/instacart/formula/internal/SingleRequestHolder.kt
@@ -7,15 +7,6 @@ package com.instacart.formula.internal
 internal class SingleRequestHolder<T>(val value: T) {
     var requested: Boolean = false
 
-    inline fun requestAccess(errorMessage: () -> String): T {
-        if (requested) {
-            throw IllegalStateException(errorMessage())
-        }
-
-        requested = true
-        return value
-    }
-
     fun reset() {
         requested = false
     }

diff --git a/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt b/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt
@@ -11,7 +11,7 @@ import com.instacart.formula.TransitionContext
 import java.lang.IllegalStateException
 import kotlin.reflect.KClass
 
-internal class SnapshotImpl<out Input, State> internal constructor(
+internal class SnapshotImpl<out Input, State>(
     override val input: Input,
     override val state: State,
     listeners: Listeners,