From c3aa3223e22feb4ef1cf4f4552e195a8a915e015 Mon Sep 17 00:00:00 2001 From: Jem Mawson Date: Thu, 9 May 2024 08:43:00 +1000 Subject: [PATCH] Introduce States argument to Transition (#37) * Introduce States argument to Transition * Add test for States * Update readme and changelog --- CHANGELOG.md | 7 +++++ README.md | 12 ++++---- lib/api/lib.api | 24 +++++++++++++-- .../app/cash/kfsm/InvalidStateTransition.kt | 2 +- lib/src/main/kotlin/app/cash/kfsm/States.kt | 15 ++++++++++ .../main/kotlin/app/cash/kfsm/Transition.kt | 7 ++--- .../main/kotlin/app/cash/kfsm/Transitioner.kt | 2 +- .../cash/kfsm/InvalidStateTransitionTest.kt | 2 +- .../test/kotlin/app/cash/kfsm/StatesTest.kt | 30 +++++++++++++++++++ .../kotlin/app/cash/kfsm/TransitionTest.kt | 8 ++--- .../cash/kfsm/exemplar/HamsterTransitions.kt | 9 ++++-- 11 files changed, 97 insertions(+), 21 deletions(-) create mode 100644 lib/src/main/kotlin/app/cash/kfsm/States.kt create mode 100644 lib/src/test/kotlin/app/cash/kfsm/StatesTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 210ccec..1ed6999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] +### Breaking + +* Introduced States as a proxy for NonEmptySet when defining Transitions. This allows for safer transition definitions + in the non-Arrow library. +* The Arrow specific library will eventually be removed, as the non-Arrow presenting API has equivalent semantics. + + ## [0.4.0] ### Breaking diff --git a/README.md b/README.md index ed34472..2fdd7c8 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,11 @@ instead. ```kotlin abstract class ColorChange( - from: NonEmptySet, + from: States, to: Color ) : Transition(from, to) { // Convenience constructor for when the from set has only one value - constructor(from: Color, to: Color) : this(nonEmptySetOf(from), to) + constructor(from: Color, to: Color) : this(States(from), to) } class Go(private val camera: Camera) : ColorChange(from = Red, to = Green) { @@ -86,7 +86,7 @@ persist values. class LightTransitioner( private val database: Database ) : Transitioner( - persist = { it.also(database::update).right() } + persist = { Result.success(it.also(database::update)) } ) ``` @@ -98,11 +98,11 @@ It is sometimes necessary to execute effects before and after a transition. Thes ```kotlin class LightTransitioner ... { - override suspend fun preHook(value: V, via: T): ErrorOr = Either.catch { + override suspend fun preHook(value: V, via: T): Result = runCatching { globalLock.lock(value) } - override suspend fun postHook(from: S, value: V, via: T): ErrorOr = Either.catch { + override suspend fun postHook(from: S, value: V, via: T): Result = runCatching { globalLock.unlock(value) notificationService.send(via.successNotifications()) } @@ -116,7 +116,7 @@ transitioner. ```kotlin val transitioner = LightTransitioner(database) -val greenLight: ErrorOr = transitioner.transition(redLight, Go) +val greenLight: Result = transitioner.transition(redLight, Go) ``` ### More examples diff --git a/lib/api/lib.api b/lib/api/lib.api index 50e0026..0253405 100644 --- a/lib/api/lib.api +++ b/lib/api/lib.api @@ -27,11 +27,31 @@ public final class app/cash/kfsm/StateMachine { public final fun verify-IoAF18A (Lapp/cash/kfsm/State;)Ljava/lang/Object; } +public final class app/cash/kfsm/States { + public static final field Companion Lapp/cash/kfsm/States$Companion; + public fun (Lapp/cash/kfsm/State;Ljava/util/Set;)V + public fun (Lapp/cash/kfsm/State;[Lapp/cash/kfsm/State;)V + public final fun component1 ()Lapp/cash/kfsm/State; + public final fun component2 ()Ljava/util/Set; + public final fun copy (Lapp/cash/kfsm/State;Ljava/util/Set;)Lapp/cash/kfsm/States; + public static synthetic fun copy$default (Lapp/cash/kfsm/States;Lapp/cash/kfsm/State;Ljava/util/Set;ILjava/lang/Object;)Lapp/cash/kfsm/States; + public fun equals (Ljava/lang/Object;)Z + public final fun getA ()Lapp/cash/kfsm/State; + public final fun getOther ()Ljava/util/Set; + public final fun getSet ()Ljava/util/Set; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public final class app/cash/kfsm/States$Companion { + public final fun toStates (Ljava/util/Set;)Lapp/cash/kfsm/States; +} + public class app/cash/kfsm/Transition { public fun (Lapp/cash/kfsm/State;Lapp/cash/kfsm/State;)V - public fun (Ljava/util/Set;Lapp/cash/kfsm/State;)V + public fun (Lapp/cash/kfsm/States;Lapp/cash/kfsm/State;)V public fun effect-gIAlu-s (Lapp/cash/kfsm/Value;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; - public final fun getFrom ()Ljava/util/Set; + public final fun getFrom ()Lapp/cash/kfsm/States; public final fun getTo ()Lapp/cash/kfsm/State; } diff --git a/lib/src/main/kotlin/app/cash/kfsm/InvalidStateTransition.kt b/lib/src/main/kotlin/app/cash/kfsm/InvalidStateTransition.kt index eaa78ef..d4399ee 100644 --- a/lib/src/main/kotlin/app/cash/kfsm/InvalidStateTransition.kt +++ b/lib/src/main/kotlin/app/cash/kfsm/InvalidStateTransition.kt @@ -2,6 +2,6 @@ package app.cash.kfsm class InvalidStateTransition(transition: Transition<*, *>, value: Value<*, *>) : Exception( "Value cannot transition ${ - transition.from.toList().sortedBy { it.toString() }.joinToString(", ", prefix = "{", postfix = "}") + transition.from.set.toList().sortedBy { it.toString() }.joinToString(", ", prefix = "{", postfix = "}") } to ${transition.to}, because it is currently ${value.state}" ) diff --git a/lib/src/main/kotlin/app/cash/kfsm/States.kt b/lib/src/main/kotlin/app/cash/kfsm/States.kt new file mode 100644 index 0000000..c96ba08 --- /dev/null +++ b/lib/src/main/kotlin/app/cash/kfsm/States.kt @@ -0,0 +1,15 @@ +package app.cash.kfsm + +/** A collection of states that is guaranteed to be non-empty. */ +data class States(val a: S, val other: Set) { + constructor(first: S, vararg others: S) : this(first, others.toSet()) + + val set: Set = other + a + + companion object { + fun Set.toStates(): States = when { + isEmpty() -> throw IllegalArgumentException("Cannot create States from empty set") + else -> toList().let { States(it.first(), it.drop(1).toSet()) } + } + } +} diff --git a/lib/src/main/kotlin/app/cash/kfsm/Transition.kt b/lib/src/main/kotlin/app/cash/kfsm/Transition.kt index 4d6e203..ca50f04 100644 --- a/lib/src/main/kotlin/app/cash/kfsm/Transition.kt +++ b/lib/src/main/kotlin/app/cash/kfsm/Transition.kt @@ -1,15 +1,14 @@ package app.cash.kfsm -open class Transition, S : State>(val from: Set, val to: S) { +open class Transition, S : State>(val from: States, val to: S) { init { - require(from.isNotEmpty()) { "At least one from state must be defined" } - from.filterNot { it.canDirectlyTransitionTo(to) }.let { + from.set.filterNot { it.canDirectlyTransitionTo(to) }.let { require(it.isEmpty()) { "invalid transition(s): ${it.map { from -> "$from->$to" }}" } } } - constructor(from: S, to: S) : this(setOf(from), to) + constructor(from: S, to: S) : this(States(from), to) open suspend fun effect(value: V): Result = Result.success(value) } diff --git a/lib/src/main/kotlin/app/cash/kfsm/Transitioner.kt b/lib/src/main/kotlin/app/cash/kfsm/Transitioner.kt index 1060285..7f2917a 100644 --- a/lib/src/main/kotlin/app/cash/kfsm/Transitioner.kt +++ b/lib/src/main/kotlin/app/cash/kfsm/Transitioner.kt @@ -12,7 +12,7 @@ abstract class Transitioner, V : Value, S : State>( value: V, transition: T ): Result = when { - transition.from.contains(value.state) -> doTheTransition(value, transition) + transition.from.set.contains(value.state) -> doTheTransition(value, transition) // Self-cycled transitions will be effected by the first case. // If we still see a transition to self then this is a no-op. transition.to == value.state -> ignoreAlreadyCompletedTransition(value, transition) diff --git a/lib/src/test/kotlin/app/cash/kfsm/InvalidStateTransitionTest.kt b/lib/src/test/kotlin/app/cash/kfsm/InvalidStateTransitionTest.kt index 9d4980a..fb0a958 100644 --- a/lib/src/test/kotlin/app/cash/kfsm/InvalidStateTransitionTest.kt +++ b/lib/src/test/kotlin/app/cash/kfsm/InvalidStateTransitionTest.kt @@ -10,7 +10,7 @@ class InvalidStateTransitionTest : StringSpec({ } "with many from-states has correct message" { - InvalidStateTransition(LetterTransition(setOf(C, B), D), Letter(E)).message shouldBe + InvalidStateTransition(LetterTransition(States(C, B), D), Letter(E)).message shouldBe "Value cannot transition {B, C} to D, because it is currently E" } }) diff --git a/lib/src/test/kotlin/app/cash/kfsm/StatesTest.kt b/lib/src/test/kotlin/app/cash/kfsm/StatesTest.kt new file mode 100644 index 0000000..21b31f6 --- /dev/null +++ b/lib/src/test/kotlin/app/cash/kfsm/StatesTest.kt @@ -0,0 +1,30 @@ +package app.cash.kfsm + +import app.cash.kfsm.States.Companion.toStates +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.core.spec.style.StringSpec +import io.kotest.matchers.shouldBe +import io.kotest.property.Arb +import io.kotest.property.arbitrary.element +import io.kotest.property.arbitrary.set +import io.kotest.property.checkAll + +class StatesTest : StringSpec({ + + "vararg constructor" { + checkAll(Arb.set(arbChar, range = 1 .. 5)) { set -> + States(set.first(), set).set shouldBe set + set.toStates().set shouldBe set + } + } + + "fails to create from empty set" { + shouldThrow { + emptySet().toStates() + } + } +}) { + companion object { + val arbChar: Arb = Arb.element(A, B, C, D, E) + } +} diff --git a/lib/src/test/kotlin/app/cash/kfsm/TransitionTest.kt b/lib/src/test/kotlin/app/cash/kfsm/TransitionTest.kt index f6b2aa8..2e90f5b 100644 --- a/lib/src/test/kotlin/app/cash/kfsm/TransitionTest.kt +++ b/lib/src/test/kotlin/app/cash/kfsm/TransitionTest.kt @@ -10,14 +10,14 @@ class TransitionTest : StringSpec({ } "cannot create an invalid state transition from a set of states" { - shouldThrow { LetterTransition(setOf(B, A), C) } + shouldThrow { LetterTransition(States(B, A), C) } } }) -open class LetterTransition(from: Set, to: Char): Transition(from, to) { - constructor(from: Char, to: Char) : this(setOf(from), to) +open class LetterTransition(from: States, to: Char): Transition(from, to) { + constructor(from: Char, to: Char) : this(States(from), to) - val specificToThisTransitionType: String = "$from -> $to" + val specificToThisTransitionType: String = "${from.set} -> $to" } diff --git a/lib/src/test/kotlin/app/cash/kfsm/exemplar/HamsterTransitions.kt b/lib/src/test/kotlin/app/cash/kfsm/exemplar/HamsterTransitions.kt index d6d178b..d59eb98 100644 --- a/lib/src/test/kotlin/app/cash/kfsm/exemplar/HamsterTransitions.kt +++ b/lib/src/test/kotlin/app/cash/kfsm/exemplar/HamsterTransitions.kt @@ -1,5 +1,7 @@ package app.cash.kfsm.exemplar +import app.cash.kfsm.States +import app.cash.kfsm.States.Companion.toStates import app.cash.kfsm.Transition import app.cash.kfsm.exemplar.Hamster.Asleep import app.cash.kfsm.exemplar.Hamster.Awake @@ -9,11 +11,14 @@ import app.cash.kfsm.exemplar.Hamster.RunningOnWheel // Create your own base transition class in order to extend your transition collection with common functionality abstract class HamsterTransition( - from: Set, + from: States, to: Hamster.State ) : Transition(from, to) { // Convenience constructor for when the from set has only one value - constructor(from: Hamster.State, to: Hamster.State) : this(setOf(from), to) + constructor(from: Hamster.State, to: Hamster.State) : this(States(from), to) + + // Convenience constructor for the deprecated variant that takes a set instead of States + constructor(from: Set, to: Hamster.State) : this(from.toStates(), to) // Demonstrates how you can add base behaviour to transitions for use in pre and post hooks. open val description: String = ""