Increase code coverage (pt2).

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
     }

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
     }

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
     }

formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt

@@ -745,6 +745,32 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
         subject.output { assertThat(listeners).containsNoDuplicates() }
     }
 
+    @Test
+    fun `duplicate child formulas are handled by indexing`() {
+        val childFormula = object : StatelessFormula<Int, Int>() {
+            override fun key(input: Int): Any = input
+
+            override fun Snapshot<Int, Unit>.evaluate(): Evaluation<Int> {
+                return Evaluation(
+                    output = 1
+                )
+            }
+        }
+
+        val parentFormula = object : StatelessFormula<Unit, Int>() {
+            override fun Snapshot<Unit, Unit>.evaluate(): Evaluation<Int> {
+                val childCount = listOf(1, 1, 2, 1, 1).sumOf { key ->
+                    context.child(childFormula, key)
+                }
+                return Evaluation(childCount)
+            }
+        }
+
+        runtime.test(parentFormula).input(Unit).output {
+            assertThat(this).isEqualTo(5)
+        }
+    }
+
     @Test
     fun `using key to scope listeners within another function`() {
         val formula = UsingKeyToScopeCallbacksWithinAnotherFunction.TestFormula()
@@ -1313,6 +1339,87 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
         }
     }
 
+    @Test
+    fun `child formula termination triggers parent state transition`() {
+        val relay = runtime.newRelay()
+        val childFormula = object : StatelessFormula<Unit, Unit>() {
+            override fun Snapshot<Unit, Unit>.evaluate(): Evaluation<Unit> {
+                return Evaluation(
+                    output = Unit,
+                    actions = context.actions {
+                        Action.onTerminate().onEvent {
+                            relay.triggerEvent()
+                            none()
+                        }
+                    }
+                )
+            }
+        }
+
+        val parentFormula = object : Formula<Boolean, Int, Int>() {
+            override fun initialState(input: Boolean): Int = 0
+
+            override fun Snapshot<Boolean, Int>.evaluate(): Evaluation<Int> {
+                if (input) {
+                    context.child(childFormula)
+                }
+                return Evaluation(
+                    output = state,
+                    actions = context.actions {
+                        relay.action().onEvent {
+                            transition(state + 1)
+                        }
+                    }
+                )
+            }
+        }
+
+        val observer = runtime.test(parentFormula)
+        observer.input(true)
+        observer.output { assertThat(this).isEqualTo(0) }
+        observer.input(false)
+        observer.output { assertThat(this).isEqualTo(1) }
+        observer.input(true)
+        observer.input(false)
+        observer.output { assertThat(this).isEqualTo(2) }
+    }
+
+    @Test
+    fun `child formula termination triggers parent termination`() {
+        var terminate = {}
+
+        val childFormula = object : StatelessFormula<Unit, Unit>() {
+            override fun Snapshot<Unit, Unit>.evaluate(): Evaluation<Unit> {
+                return Evaluation(
+                    output = Unit,
+                    actions = context.actions {
+                        Action.onTerminate().onEvent {
+                            terminate()
+                            none()
+                        }
+                    }
+                )
+            }
+        }
+
+        val parentFormula = object : StatelessFormula<Boolean, Int>() {
+            override fun Snapshot<Boolean, Unit>.evaluate(): Evaluation<Int> {
+                return if (input) {
+                    context.child(childFormula)
+                    Evaluation(1)
+                } else {
+                    Evaluation(0)
+                }
+            }
+        }
+
+        val observer = runtime.test(parentFormula)
+        terminate = { observer.dispose() }
+        observer.input(true)
+        observer.assertOutputCount(1)
+        observer.input(false)
+        observer.assertOutputCount(1) // Terminated, should not have any more outputs
+    }
 
     // End of child specific test cases
 

formula/src/test/java/com/instacart/formula/subjects/DuplicateListenerKeysHandledByIndexing.kt

@@ -16,7 +16,7 @@ object DuplicateListenerKeysHandledByIndexing {
 
     class TestFormula : StatelessFormula<Unit, TestOutput>() {
         override fun Snapshot<Unit, Unit>.evaluate(): Evaluation<TestOutput> {
-            val keys = listOf(1, 2, 1)
+            val keys = listOf(1, 2, 1, 1, 1)
             return Evaluation(
                 output = TestOutput(
                     listeners = keys.map { key ->