From 04c044094db51027221777966649c1617a1aa9a8 Mon Sep 17 00:00:00 2001 From: Ray Ryan Date: Tue, 24 Sep 2024 13:04:32 -0700 Subject: [PATCH] safeAction, safeEventHandler People are confused by the fact that a `WorkflowAction` can't assume that a sealed class / interface `StateT` is the same subtype that it was at render time when the action fires. And those that do understand it resent this boilerplate: ```kotlin action { (state as? SpecificState)?.let { currentState -> // whatever } } ``` So we introduce `StatefulWorkflow.safeAction` and `StatefulWorkflow.RenderContext.safeEventHandler` as conveniences to do that cast for you. --- .../sample/gameworkflow/RunGameWorkflow.kt | 61 ++-- workflow-core/api/workflow-core.api | 1 + .../squareup/workflow1/StatefulWorkflow.kt | 330 +++++++++++++++++- 3 files changed, 355 insertions(+), 37 deletions(-) diff --git a/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt b/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt index e9a4389214..dbe15b1f8f 100644 --- a/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt +++ b/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt @@ -18,7 +18,6 @@ import com.squareup.sample.gameworkflow.SyncState.SAVING import com.squareup.workflow1.Snapshot import com.squareup.workflow1.StatefulWorkflow import com.squareup.workflow1.Workflow -import com.squareup.workflow1.action import com.squareup.workflow1.runningWorker import com.squareup.workflow1.rx2.asWorker import com.squareup.workflow1.ui.Screen @@ -88,8 +87,12 @@ class RealRunGameWorkflow( namePrompt = NewGameScreen( renderState.defaultXName, renderState.defaultOName, - onCancel = context.eventHandler { setOutput(CanceledStart) }, - onStartGame = context.eventHandler { x, o -> state = Playing(PlayerInfo(x, o)) } + onCancel = context.safeEventHandler { + setOutput(CanceledStart) + }, + onStartGame = context.safeEventHandler { _, x, o -> + state = Playing(PlayerInfo(x, o)) + } ) ) } @@ -119,15 +122,11 @@ class RealRunGameWorkflow( message = "Do you really want to concede the game?", positive = "I Quit", negative = "No", - confirmQuit = context.eventHandler { - (state as? MaybeQuitting)?.let { oldState -> - state = MaybeQuittingForSure(oldState.playerInfo, oldState.completedGame) - } + confirmQuit = context.safeEventHandler { oldState -> + state = MaybeQuittingForSure(oldState.playerInfo, oldState.completedGame) }, - continuePlaying = context.eventHandler { - (state as? MaybeQuitting)?.let { oldState -> - state = Playing(oldState.playerInfo, oldState.completedGame.lastTurn) - } + continuePlaying = context.safeEventHandler { oldState -> + state = Playing(oldState.playerInfo, oldState.completedGame.lastTurn) } ) ) @@ -142,15 +141,11 @@ class RealRunGameWorkflow( message = "Really?", positive = "Yes!!", negative = "Sigh, no", - confirmQuit = context.eventHandler { - (state as? MaybeQuittingForSure)?.let { oldState -> - state = GameOver(oldState.playerInfo, oldState.completedGame) - } + confirmQuit = context.safeEventHandler { oldState -> + state = GameOver(oldState.playerInfo, oldState.completedGame) }, - continuePlaying = context.eventHandler { - (state as? MaybeQuittingForSure)?.let { oldState -> - state = Playing(oldState.playerInfo, oldState.completedGame.lastTurn) - } + continuePlaying = context.safeEventHandler { oldState -> + state = Playing(oldState.playerInfo, oldState.completedGame.lastTurn) } ) ) @@ -169,43 +164,37 @@ class RealRunGameWorkflow( renderState, onTrySaveAgain = context.trySaveAgain(), onPlayAgain = context.playAgain(), - onExit = context.eventHandler { setOutput(FinishedPlaying) } + onExit = context.safeEventHandler { setOutput(FinishedPlaying) } ) ) } } - private fun stopPlaying(game: CompletedGame) = action { - val oldState = state as Playing + private fun stopPlaying(game: CompletedGame) = safeAction("stopPlaying") { oldState -> state = when (game.ending) { Quitted -> MaybeQuitting(oldState.playerInfo, game) else -> GameOver(oldState.playerInfo, game) } } - private fun handleLogGame(result: GameLog.LogResult) = action { - val oldState = state as GameOver + private fun handleLogGame(result: GameLog.LogResult) = safeAction { oldState -> state = when (result) { TRY_LATER -> oldState.copy(syncState = SAVE_FAILED) LOGGED -> oldState.copy(syncState = SAVED) } } - private fun RenderContext.playAgain() = eventHandler { - (state as? GameOver)?.let { oldState -> - val (x, o) = oldState.playerInfo - state = NewGame(x, o) - } + private fun RenderContext.playAgain() = safeEventHandler { oldState -> + val (x, o) = oldState.playerInfo + state = NewGame(x, o) } - private fun RenderContext.trySaveAgain() = eventHandler { - (state as? GameOver)?.let { oldState -> - check(oldState.syncState == SAVE_FAILED) { - "Should only fire trySaveAgain in syncState $SAVE_FAILED, " + - "was ${oldState.syncState}" - } - state = oldState.copy(syncState = SAVING) + private fun RenderContext.trySaveAgain() = safeEventHandler { oldState -> + check(oldState.syncState == SAVE_FAILED) { + "Should only fire trySaveAgain in syncState $SAVE_FAILED, " + + "was ${oldState.syncState}" } + state = oldState.copy(syncState = SAVING) } override fun snapshotState(state: RunGameState): Snapshot = state.toSnapshot() diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api index 3c80e3c26a..01b49cc96b 100644 --- a/workflow-core/api/workflow-core.api +++ b/workflow-core/api/workflow-core.api @@ -155,6 +155,7 @@ public final class com/squareup/workflow1/Snapshots { public abstract class com/squareup/workflow1/StatefulWorkflow : com/squareup/workflow1/IdCacheable, com/squareup/workflow1/Workflow { public fun ()V public final fun asStatefulWorkflow ()Lcom/squareup/workflow1/StatefulWorkflow; + public final fun defaultOnFailedCast (Ljava/lang/String;Lkotlin/reflect/KClass;Ljava/lang/Object;)V public fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier; public abstract fun initialState (Ljava/lang/Object;Lcom/squareup/workflow1/Snapshot;)Ljava/lang/Object; public fun initialState (Ljava/lang/Object;Lcom/squareup/workflow1/Snapshot;Lkotlinx/coroutines/CoroutineScope;)Ljava/lang/Object; diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt index a805048c05..1b909560e7 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt @@ -8,6 +8,8 @@ import com.squareup.workflow1.WorkflowAction.Companion.toString import kotlinx.coroutines.CoroutineScope import kotlin.jvm.JvmMultifileClass import kotlin.jvm.JvmName +import kotlin.reflect.KClass +import kotlin.reflect.safeCast /** * A composable, stateful object that can [handle events][RenderContext.actionSink], @@ -73,7 +75,333 @@ public abstract class StatefulWorkflow< public inner class RenderContext internal constructor( baseContext: BaseRenderContext - ) : BaseRenderContext<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT> by baseContext + ) : BaseRenderContext<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT> by baseContext { + + /** + * Like [eventHandler], but no-ops if [state][WorkflowAction.Updater.state] has + * changed to a different type than [CurrentStateT] by the time [update] fires. + * + * when(renderState) { + * is NewGame -> { + * NewGameScreen( + * onCancel = context.safeEventHandler { + * setOutput(CanceledStart) + * }, + * onStartGame = context.safeEventHandler { oldState, x, o -> + * state = Playing(oldState.gameType, PlayerInfo(x, o)) + * } + * ) + * } + * + * This is not an uncommon case. Consider accidental rapid taps on + * a button, where the first tap event moves the receiving [StatefulWorkflow] + * to a new state. There is no reason to expect that the later taps will not + * fire the (now stale) event handler a few more times. No promise can be + * made that the [state][WorkflowAction.Updater.state] received by a [WorkflowAction] + * will be of the same type as the `renderState` parameter that was received by + * the [render] call that created it. + * + * @param name A string describing the handler for debugging. + * @param onFailedCast Optional function invoked when casting fails. Default implementation + * logs a warning with [println] + * @param update Function that defines the workflow update. + */ + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + // Type variance issue: https://github.com/square/workflow-kotlin/issues/891 + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT + ) -> Unit + ): () -> Unit { + return eventHandler({ name }) { + CurrentStateT::class.safeCast(state)?.let { currentState -> this.update(currentState) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + event: EventT + ) -> Unit + ): (EventT) -> Unit { + return eventHandler({ name }) { event: EventT -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, event) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2 + ) -> Unit + ): (E1, E2) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3 + ) -> Unit + ): (E1, E2, E3) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2, e3: E3 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4 + ) -> Unit + ): (E1, E2, E3, E4) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2, e3: E3, e4: E4 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5 + ) -> Unit + ): (E1, E2, E3, E4, E5) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4, e5) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun < + reified CurrentStateT : StateT & Any, E1, E2, E3, E4, E5, E6 + > safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5, + e6: E6 + ) -> Unit + ): (E1, E2, E3, E4, E5, E6) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5, e6: E6 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4, e5, e6) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun < + reified CurrentStateT : StateT & Any, E1, E2, E3, E4, E5, E6, E7 + > safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5, + e6: E6, + e7: E7 + ) -> Unit + ): (E1, E2, E3, E4, E5, E6, E7) -> Unit { + return eventHandler({ name }) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5, e6: E6, e7: E7 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4, e5, e6, e7) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun < + reified CurrentStateT : StateT & Any, E1, E2, E3, E4, E5, E6, E7, E8 + > safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5, + e6: E6, + e7: E7, + e8: E8 + ) -> Unit + ): (E1, E2, E3, E4, E5, E6, E7, E8) -> Unit { + return eventHandler( + { name } + ) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5, e6: E6, e7: E7, e8: E8 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4, e5, e6, e7, e8) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun < + reified CurrentStateT : StateT & Any, E1, E2, E3, E4, E5, E6, E7, E8, E9 + > safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5, + e6: E6, + e7: E7, + e8: E8, + e9: E9 + ) -> Unit + ): (E1, E2, E3, E4, E5, E6, E7, E8, E9) -> Unit { + return eventHandler( + { name } + ) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5, e6: E6, e7: E7, e8: E8, e9: E9 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> this.update(currentState, e1, e2, e3, e4, e5, e6, e7, e8, e9) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + + public inline fun < + reified CurrentStateT : StateT & Any, E1, E2, E3, E4, E5, E6, E7, E8, E9, E10 + > safeEventHandler( + name: String = "safeEventHandler", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + crossinline update: + WorkflowAction<@UnsafeVariance PropsT, StateT, @UnsafeVariance OutputT>.Updater.( + currentState: CurrentStateT, + e1: E1, + e2: E2, + e3: E3, + e4: E4, + e5: E5, + e6: E6, + e7: E7, + e8: E8, + e9: E9, + e10: E10 + ) -> Unit + ): (E1, E2, E3, E4, E5, E6, E7, E8, E9, E10) -> Unit { + return eventHandler( + { name } + ) { e1: E1, e2: E2, e3: E3, e4: E4, e5: E5, e6: E6, e7: E7, e8: E8, e9: E9, e10: E10 -> + CurrentStateT::class.safeCast(state) + ?.let { currentState -> + this.update(currentState, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10) + } + ?: onFailedCast(name, CurrentStateT::class, state) + } + } + } + + /** + * Like [action], but no-ops if [state][WorkflowAction.Updater.state] has + * changed to a different type than [CurrentStateT] by the time [update] fires. + * + * private fun stopPlaying( + * game: CompletedGame + * ) = safeAction("stopPlaying") { oldState -> + * state = when (game.ending) { + * Quitting -> MaybeQuitting(oldState.playerInfo, game) + * else -> GameOver(oldState.playerInfo, game) + * } + * } + * + * This is not an uncommon case. Consider accidental rapid taps on + * a button, where the first tap event moves the receiving [StatefulWorkflow] + * to a new state. There is no reason to expect that the later taps will not + * fire the (now stale) event handler a few more times. No promise can be + * made that the [state][WorkflowAction.Updater.state] received by a [WorkflowAction] + * will be of the same type as the `renderState` parameter that was received by + * the [render] call that created it. + * + * @param name A string describing the action for debugging. + * @param onFailedCast Optional function invoked when casting fails. Default implementation + * logs a warning with [println] + * @param update Function that defines the workflow update. + */ + public inline fun safeAction( + name: String = "safeAction", + crossinline onFailedCast: (name: String, type: KClass<*>, state: StateT) -> Unit = + ::defaultOnFailedCast, + noinline update: WorkflowAction.Updater.( + currentState: CurrentStateT + ) -> Unit + ): WorkflowAction = action({ name }) { + CurrentStateT::class.safeCast(state)?.let { currentState -> this.update(currentState) } + ?: onFailedCast(name, CurrentStateT::class, state) + } + + @PublishedApi + internal fun defaultOnFailedCast( + name: String, + expectedType: KClass<*>, + state: StateT + ) { + println("$name expected state of type ${expectedType.qualifiedName}, got $state") + } /** * Called from [RenderContext.renderChild] when the state machine is first started, to get the