MutationManager now supports nested mutations. Deferred mutations are now actual mutations. Added several mutation-related tests, six of which fail at the moment.

cell/src/commonMain/kotlin/world/phantasmal/cell/MutationManager.kt

@@ -1,16 +1,19 @@
 package world.phantasmal.cell
 
+import world.phantasmal.core.assert
 import kotlin.contracts.InvocationKind.EXACTLY_ONCE
 import kotlin.contracts.contract
 
 // TODO: Throw exception by default when triggering early recomputation during change set. Allow to
-//       to turn this check off, because partial early recomputation might be useful in rare cases.
+//       to turn this check off, because partial, early recomputation might be useful in rare cases.
-//       Dependencies will need to partially apply ListChangeEvents etc. and remember which part of
+//       Dependents will need to partially apply ListChangeEvents etc. and remember which part of
 //       the event they've already applied (i.e. an index into the changes list).
-// TODO: Think about nested change sets. Initially don't allow nesting?
 object MutationManager {
     private val invalidatedLeaves = HashSet<LeafDependent>()
 
+    /** Non-zero when a mutation is active. */
+    private var mutationNestingLevel = 0
+
     /** Whether a dependency's value is changing at the moment. */
     private var dependencyChanging = false
 
@@ -22,20 +25,42 @@ object MutationManager {
             callsInPlace(block, EXACTLY_ONCE)
         }
 
-        // TODO: Implement mutate correctly.
+        mutationStart()
-        block()
+
+        try {
+            block()
+        } finally {
+            mutationEnd()
+        }
     }
 
     fun mutateDeferred(block: () -> Unit) {
-        if (dependencyChanging) {
+        if (dependencyChanging || mutationNestingLevel > 0) {
             deferredMutations.add(block)
         } else {
             block()
         }
     }
 
-    fun invalidated(dependent: LeafDependent) {
+    fun mutationStart() {
-        invalidatedLeaves.add(dependent)
+        mutationNestingLevel++
+    }
+
+    fun mutationEnd() {
+        assert(mutationNestingLevel > 0) { "No mutation was started." }
+
+        mutationNestingLevel--
+
+        if (mutationNestingLevel == 0) {
+            try {
+                for (dependent in invalidatedLeaves) {
+                    dependent.pull()
+                }
+            } finally {
+                invalidatedLeaves.clear()
+                applyDeferredMutations()
+            }
+        }
     }
 
     inline fun changeDependency(block: () -> Unit) {
@@ -43,43 +68,61 @@ object MutationManager {
             callsInPlace(block, EXACTLY_ONCE)
         }
 
-        dependencyStartedChanging()
+        dependencyChangeStart()
 
         try {
             block()
         } finally {
-            dependencyFinishedChanging()
+            dependencyChangeEnd()
         }
     }
 
-    fun dependencyStartedChanging() {
+    fun dependencyChangeStart() {
         check(!dependencyChanging) { "A cell is already changing." }
 
         dependencyChanging = true
     }
 
-    fun dependencyFinishedChanging() {
+    fun dependencyChangeEnd() {
-        try {
+        assert(dependencyChanging) { "No cell was changing." }
-            for (dependent in invalidatedLeaves) {
-                dependent.pull()
-            }
-        } finally {
-            dependencyChanging = false
-            invalidatedLeaves.clear()
 
-            if (!applyingDeferredMutations) {
+        if (mutationNestingLevel == 0) {
-                try {
+            try {
-                    applyingDeferredMutations = true
+                for (dependent in invalidatedLeaves) {
-                    var i = 0
+                    dependent.pull()
-
-                    while (i < deferredMutations.size) {
-                        deferredMutations[i]()
-                        i++
-                    }
-                } finally {
-                    applyingDeferredMutations = false
-                    deferredMutations.clear()
                 }
+            } finally {
+                dependencyChanging = false
+                invalidatedLeaves.clear()
+                applyDeferredMutations()
+            }
+        } else {
+            dependencyChanging = false
+        }
+    }
+
+    fun invalidated(dependent: LeafDependent) {
+        invalidatedLeaves.add(dependent)
+    }
+
+    private fun applyDeferredMutations() {
+        if (!applyingDeferredMutations) {
+            try {
+                applyingDeferredMutations = true
+                // Use index instead of iterator because list can grow while applying deferred
+                // mutations.
+                var idx = 0
+
+                while (idx < deferredMutations.size) {
+                    mutate {
+                        deferredMutations[idx]()
+                    }
+
+                    idx++
+                }
+            } finally {
+                applyingDeferredMutations = false
+                deferredMutations.clear()
             }
         }
     }

cell/src/commonTest/kotlin/world/phantasmal/cell/CellTests.kt

@@ -1,6 +1,8 @@
 package world.phantasmal.cell
 
 import world.phantasmal.cell.test.CellTestSuite
+import world.phantasmal.cell.test.Snapshot
+import world.phantasmal.cell.test.snapshot
 import world.phantasmal.core.disposable.use
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -228,6 +230,109 @@ interface CellTests : CellTestSuite {
         assertEquals(mapped.value, observedValue)
     }
 
+    //
+    // Mutation tests.
+    //
+
+    @Test
+    fun changes_during_a_mutation_are_deferred() = test {
+        val p = createProvider()
+        var changes = 0
+
+        disposer.add(
+            p.cell.observeChange {
+                changes++
+            }
+        )
+
+        mutate {
+            repeat(5) {
+                p.emit()
+
+                // Change should be deferred until this lambda returns.
+                assertEquals(0, changes)
+            }
+        }
+
+        // All changes to the same cell should be collapsed to a single change.
+        assertEquals(1, changes)
+    }
+
+    @Test
+    fun value_can_be_accessed_during_a_mutation() = test {
+        val p = createProvider()
+
+        // Change will be observed exactly once.
+        var observedValue: Snapshot? = null
+
+        disposer.add(
+            p.cell.observeChange {
+                assertNull(observedValue)
+                observedValue = it.value.snapshot()
+            }
+        )
+
+        val v1 = p.cell.value.snapshot()
+        var v3: Snapshot? = null
+
+        mutate {
+            val v2 = p.cell.value.snapshot()
+
+            assertEquals(v1, v2)
+
+            p.emit()
+            v3 = p.cell.value.snapshot()
+
+            assertNotEquals(v2, v3)
+
+            p.emit()
+        }
+
+        val v4 = p.cell.value.snapshot()
+
+        assertNotNull(v3)
+        assertNotEquals(v3, v4)
+        assertEquals(v4, observedValue)
+    }
+
+    @Test
+    fun mutations_can_be_nested() = test {
+        // 3 Cells.
+        val ps = Array(3) { createProvider() }
+        val observedChanges = IntArray(3)
+
+        // Observe each cell.
+        repeat(3) { idx ->
+            disposer.add(
+                ps[idx].cell.observeChange {
+                    assertEquals(0, observedChanges[idx])
+                    observedChanges[idx]++
+                }
+            )
+        }
+
+        mutate {
+            ps[0].emit()
+
+            repeat(3) {
+                mutate {
+                    ps[1].emit()
+
+                    mutate {
+                        ps[2].emit()
+                    }
+
+                    assertTrue(observedChanges.all { it == 0 })
+                }
+
+                assertTrue(observedChanges.all { it == 0 })
+            }
+        }
+
+        // At this point all 3 observers should be called exactly once.
+        assertTrue(observedChanges.all { it == 1 })
+    }
+
     interface Provider {
         val cell: Cell<Any>
 
@@ -237,16 +342,3 @@ interface CellTests : CellTestSuite {
         fun emit()
     }
 }
-
-/** See [snapshot]. */
-private typealias Snapshot = String
-
-/**
- * We use toString to create "snapshots" of values throughout the tests. Most of the time cells will
- * actually have a new value after emitting a change event, but this is not always the case with
- * more complex cells or cells that point to complex values. So instead of keeping references to
- * values and comparing them with == (or using e.g. assertEquals), we compare snapshots.
- *
- * This of course assumes that all values have sensible toString implementations.
- */
-private fun Any?.snapshot(): Snapshot = toString()

cell/src/commonTest/kotlin/world/phantasmal/cell/CellWithDependenciesTests.kt

@@ -62,19 +62,19 @@ interface CellWithDependenciesTests : CellTests {
 
         val cell = createWithDependencies(dependency1, dependency2, dependency3)
 
-        assertTrue(dependency1.publicDependents.isEmpty())
+        assertEquals(0, dependency1.dependentCount)
-        assertTrue(dependency2.publicDependents.isEmpty())
+        assertEquals(0, dependency2.dependentCount)
-        assertTrue(dependency3.publicDependents.isEmpty())
+        assertEquals(0, dependency3.dependentCount)
 
         disposer.add(cell.observeChange { })
 
-        assertEquals(1, dependency1.publicDependents.size)
+        assertEquals(1, dependency1.dependentCount)
-        assertEquals(1, dependency2.publicDependents.size)
+        assertEquals(1, dependency2.dependentCount)
-        assertEquals(1, dependency3.publicDependents.size)
+        assertEquals(1, dependency3.dependentCount)
     }
 
     private class TestCell : AbstractCell<Int>() {
-        val publicDependents: List<Dependent> = dependents
+        val dependentCount: Int get() = dependents.size
 
         override val value: Int = 5
         override val changeEvent: ChangeEvent<Int> = ChangeEvent(value)

cell/src/commonTest/kotlin/world/phantasmal/cell/ChangeTests.kt

@@ -1,36 +0,0 @@
-package world.phantasmal.cell
-
-import world.phantasmal.cell.test.CellTestSuite
-import kotlin.test.Test
-import kotlin.test.assertEquals
-import kotlin.test.assertFails
-
-class ChangeTests : CellTestSuite {
-    @Test
-    fun exceptions_during_a_change_set_are_allowed() = test {
-        val dependency = mutableCell(7)
-        val dependent = dependency.map { 2 * it }
-
-        var dependentObservedValue: Int? = null
-        disposer.add(dependent.observeChange { dependentObservedValue = it.value })
-
-        assertFails {
-            mutate {
-                dependency.value = 11
-                throw Exception()
-            }
-        }
-
-        // The change to dependency is still propagated because it happened before the exception.
-        assertEquals(22, dependentObservedValue)
-        assertEquals(22, dependent.value)
-
-        // The machinery behind change is still in a valid state.
-        mutate {
-            dependency.value = 13
-        }
-
-        assertEquals(26, dependentObservedValue)
-        assertEquals(26, dependent.value)
-    }
-}

cell/src/commonTest/kotlin/world/phantasmal/cell/MutationTests.kt

@@ -3,17 +3,20 @@ package world.phantasmal.cell
 import world.phantasmal.cell.test.CellTestSuite
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFails
 
 class MutationTests : CellTestSuite {
     @Test
-    fun can_change_observed_cell_with_mutateDeferred() = test {
+    fun can_change_observed_cell_with_deferred_mutation() = test {
         val cell = mutableCell(0)
-        var observerCalls = 0
+        var observedChanges = 0
 
         disposer.add(cell.observe {
-            observerCalls++
+            observedChanges++
 
             if (it < 10) {
+                // Changing the cell here would throw an exception because, while a cell is
+                // changing, no other cell can change. Deferring the change is allowed though.
                 mutateDeferred {
                     cell.value++
                 }
@@ -22,7 +25,67 @@ class MutationTests : CellTestSuite {
 
         cell.value = 1
 
-        assertEquals(10, observerCalls)
+        assertEquals(10, observedChanges)
         assertEquals(10, cell.value)
     }
+
+    /**
+     * All deferred mutations happen at the end of the outer mutation.
+     */
+    @Test
+    fun can_nest_deferred_mutations_in_regular_mutations() = test {
+        val cell = mutableCell(0)
+        var observerChanges = 0
+
+        disposer.add(cell.observe {
+            observerChanges++
+        })
+
+        mutate {
+            mutateDeferred {
+                cell.value = 3 // Happens third.
+            }
+
+            mutate {
+                mutateDeferred {
+                    cell.value = 4 // Happens fourth.
+                }
+
+                cell.value = 1 // Happens first.
+            }
+
+            cell.value = 2 // Happens second.
+        }
+
+        assertEquals(3, observerChanges)
+        assertEquals(4, cell.value)
+    }
+
+    @Test
+    fun exceptions_during_a_mutation_are_allowed() = test {
+        val dependency = mutableCell(7)
+        val dependent = dependency.map { 2 * it }
+
+        var dependentObservedValue: Int? = null
+        disposer.add(dependent.observeChange { dependentObservedValue = it.value })
+
+        assertFails {
+            mutate {
+                dependency.value = 11
+                throw Exception()
+            }
+        }
+
+        // The change to dependency is still propagated because it happened before the exception.
+        assertEquals(22, dependentObservedValue)
+        assertEquals(22, dependent.value)
+
+        // The mutation machinery is still in a valid state.
+        mutate {
+            dependency.value = 13
+        }
+
+        assertEquals(26, dependentObservedValue)
+        assertEquals(26, dependent.value)
+    }
 }

cell/src/commonTest/kotlin/world/phantasmal/cell/list/SimpleFilteredListCellTests.kt

@@ -4,6 +4,10 @@ import world.phantasmal.cell.Cell
 
 // TODO: A test suite that tests SimpleFilteredListCell while both types of dependencies are
 //       changing.
+/**
+ * Standard tests are done by [SimpleFilteredListCellListDependencyEmitsTests] and
+ * [SimpleFilteredListCellPredicateDependencyEmitsTests].
+ */
 @Suppress("unused")
 class SimpleFilteredListCellTests : SuperFilteredListCellTests {
     override fun <E> createFilteredListCell(list: ListCell<E>, predicate: Cell<(E) -> Boolean>) =

cell/src/commonTest/kotlin/world/phantasmal/cell/list/SuperFilteredListCellTests.kt

@@ -235,7 +235,7 @@ interface SuperFilteredListCellTests : CellTestSuite {
         val y = "y"
         val z = "z"
         val dependency = SimpleListCell(mutableListOf(x, y, z, x, y, z))
-        val list = createFilteredListCell(dependency, SimpleCell { it != y })
+        val list = createFilteredListCell(dependency, ImmutableCell { it != y })
         var event: ListChangeEvent<String>? = null
 
         disposer.add(list.observeListChange {

cell/src/commonTest/kotlin/world/phantasmal/cell/test/Utils.kt

@@ -7,3 +7,16 @@ fun <E> assertListCellEquals(expected: List<E>, actual: ListCell<E>) {
     assertEquals(expected.size, actual.size.value)
     assertEquals(expected, actual.value)
 }
+
+/** See [snapshot]. */
+typealias Snapshot = String
+
+/**
+ * We use toString to create "snapshots" of values throughout the tests. Most of the time cells will
+ * actually have a new value after emitting a change event, but this is not always the case with
+ * more complex cells or cells that point to complex values. So instead of keeping references to
+ * values and comparing them with == (or using e.g. assertEquals), we compare snapshots.
+ *
+ * This of course assumes that all values have sensible toString implementations.
+ */
+fun Any?.snapshot(): Snapshot = toString()

core/src/commonMain/kotlin/world/phantasmal/core/Assertions.kt

@@ -4,6 +4,10 @@ inline fun assert(value: () -> Boolean) {
     assert(value) { "An assertion failed." }
 }
 
+inline fun assert(value: Boolean, lazyMessage: () -> Any) {
+    assert({ value }, lazyMessage)
+}
+
 expect inline fun assert(value: () -> Boolean, lazyMessage: () -> Any)
 
 inline fun assertUnreachable(lazyMessage: () -> Any) {