Only handle the most applicable entry in the spec

[doverdho]

Hi,

I have a statemachine where I have events that are only handled in certain states. I've written handlers for these states, but it may be possible that these events are received outside of their intended state. In that case, I would like to log this event, so that if this occurs we can check what needs to be done. I've added the inState (with all states deriving from BaseState) and added an event handler that is able to handle my BaseEvent. I've noticed now that all my events are handled twice, once in the current state and once in the BaseState handler. This seems quite strange to me, my assumption was that the state machine implementation would pick the best match instead of all matches. Can this be considered as a feature?

[sockeqwe]

Hi @doverdho
thanks for asking and bringing up your idea.

The described behavior (all actions are broadcasted everywhere) is by design.
The overall idea is that your state machine should be state-driven, NOT, action-driven. That means, what counts is the state that your statemachine is in and not so much which action is fired. In fact, other inState {} blocks are simply not triggering as the current state is diffent, therefore broadcasting all actions should not be a problem in theory.

With that said, I am wondering what the problem exactly is that you are struggling with (and therefore your suggestion is coming from)? Do you have a real-world example where your motivation is coming from? What is exactly the problem you try to solve?

[doverdho]

Hello @sockeqwe , I hope you're doing well. I was wondering if you had a chance to take a look at the example I provided in this thread? Your input would be greatly appreciated. Thank you!

[doverdho]

Hi @sockeqwe ,

My team has identified the following two use cases for only hitting the most fitting entry:

1. Override a stateless handler in a state. We have many events that are triggered from outside our application. Many of these events are only handled in specific states. If such an event is triggered when not in one of these states, we want to handle them later by queueing them internally.
2. Catching unhandled events. Since we have a large amount of events it is possible that a handler is not written. If we could make a default handler for unhandled events we could log these events. This would make debugging issues with the state machine later on easier.

While 2 is considered a nice to have within our team, the first use case is heavily used in other non-Kotlin products that we have, therefore it would make it easier for us if this was available in FlowReduxStateMachine.

I've attached a small sample project with the following scenario: An event that is known to the state is dispatched, so we expect that handler to execute and trigger the state change. What we also see is that the base state handling for that specific event is called, which calls the queueEvent handler. We also notice that the unhandledEvent handler is also called, resulting in the following logs:

IdleState dfs.payment.app.main I Handling event service button pressed
ControllerState dfs.payment.app.main I Queueing event dfs.payment.app.testmodule.ServiceButtonPressed@d5ba5b3
ControllerState dfs.payment.app.main E Unhandled event dfs.payment.app.testmodule.ServiceButtonPressed@d5ba5b3
Controller dfs.payment.app.main I State: dfs.payment.app.testmodule.ServiceState@166bae9
Controller dfs.payment.app.main I State: dfs.payment.app.testmodule.ErrorState@ee6c36e

Sometimes I notice that the ServiceState is not entered, as the logging is not made. I can't explain this, as I thought that the ExecutionPolicy.Ordered would make sure that the full handling is done before another handler is called.

testmodule.zip

[sockeqwe]

If such an event is triggered when not in one of these states, we want to handle them later by queueing them internally.

I think that goes against the design of a state machine in general (not unique to FlowRedux). I don't think we will support something like this in FlowRedux. To me, it sounds like you have to revisit either your design choice: why do you need to queue events to get started with? maybe your state machine is not properly designed? If an event is important enough to be queued while your state machine is in a certain state, then maybe that state should handle that event. Handling this "event" in that state means to me reacting to it: reacting to it could mean also something like having a state.queue and the action handler adds it to the states queue and then somewhen this queue (working with conditions and "sub-states") could be run through to do something (while you are still in that state). It is really hard for me to tell you how to approach it best as I have no context, but to me from a "theoretical" point of view it seems that your state machine design is wrong.

only hitting the most fitting entry

How do we know what is the "most fitting entry"? it is impossible to know just from source code and DSL definition, correct?

Catching unhandled events. Since we have a large amount of events it is possible that a handler is not written. If we could make a default handler for unhandled events we could log these events. This would make debugging issues with the state machine later on easier.

State Machines by definition can "ignore" events where there is not handler / state transition designed for. I see that it could potentially be also a workaround for the "queue" issue that you have described above. If you want to write a proposal how to implement an "unhandled" action / event solution in FlowRedux, we are open to discussing this or what do you think @gabrielittner ? But for me personally, this "feature" has low priority.

Regarding your test example: Do you mind creating an "Issue" out of it as it seems to be unrelated to the proposal you are making here but rather a bug in FlowRedux or missing coverage in the documentation? I will only have time to look at this though somwhen mid next week.

But if it is related to ExecutionPolicy, then there is a misunderstanding: ExecutionPolicy is only for each handler. For example:

inState<S1>{ 
   on<A1>(ExcecutionPolicy.Ordered) { action, state ->  delay( randomLong() ); state.mutate { ... } }
   on<A2>(ExcecutionPolicy.Ordered) { a, s -> ... }   
}

the ExcecutionPolicy only applies to the specified on<Action> block for the same type of action and not globally on the whole state machine. For example when you dispatch A1 2 times fast after each other while you are in the state S1 then ExcecutionPolicy.Ordered ensures that the first A1's state.mutate { ... } is executed before the second A1's state.mutate { ... }.

With ExcecutionPolicy. UNORDERED on the same example above, FlowRedux would not ensure any ordering of execution, thus the second A1's state.mutate { ... } could be executed before the first A1's state.mutate { ... }.

The important bit though is that this ordering / or not ordering is only for the on<A1>. It doesn't affect ordering of on<A2>. on<A2>(ExcecutionPolicy.Ordered is again only setting a Execution policy for handling the A2 action (which is independent from handling the A1 action)

[gabrielittner]

State Machines by definition can "ignore" events where there is not handler / state transition designed for. I see that it could potentially be also a workaround for the "queue" issue that you have described above. If you want to write a proposal how to implement an "unhandled" action / event solution in FlowRedux, we are open to discussing this or what do you think @gabrielittner ? But for me personally, this "feature" has low priority.

I can see this as a possibility but I'm not sure how useful it is. Let's say you have an error state with a retry button, the user double taps that. The first tap changes the state to loading, so the second action is ignored. If this would go to the unhandled event callback what do you do with that there? Another general issue would be sub state machines, we don't know whether they'll do something with the event or not. Maybe this would be better as a static analysis check that sees that each action (if it's a sealed interface/class) is handled at least once? That warning could be suppressed in cases where it is intentional.

Regarding your test example: Do you mind creating an "Issue" out of it as it seems to be unrelated to the proposal you are making here but rather a bug in FlowRedux or missing coverage in the documentation? I will only have time to look at this though somwhen mid next week.

The sample generally looks fine to me and I think it's just the misunderstanding of what ORDERED means. Since all handlers run in parallel the unhandled event handler can be first. Then it switches from IdleState to ErrorState. The specific ServiceButtonPressed handler still runs, but the ChangeState object it returns won't be reduced since we left IdleState. This doesn't happen the other way around since the undhandled event handler is in the base state so its returned ChangeState objects will always be reduced.

The important bit though is that this ordering / or not ordering is only for the on. It doesn't affect ordering of on. on(ExcecutionPolicy.Ordered is again only setting a Execution policy for handling the A2 action (which is independent from handling the A1 action)

Maybe we can change the naming (introducing new cases to the enum and deprecating the old ones)?

[sockeqwe]

Maybe this would be better as a static analysis check that sees that each action (if it's a sealed interface/class) is handled at least once?

I was also thinking into the LINTING / static code analysis direction.

Maybe we can change the naming (introducing new cases to the enum and deprecating the old ones)?

What names do you suggest? To me it sounds the enum class per see "ExecutionPolicy" is ambiguous?

[doverdho]

The sample generally looks fine to me and I think it's just the misunderstanding of what ORDERED means.

This is indeed the case, my team assumed that it meant ordered in the whole action handling, which isn't the case. Thank you for the clear explanation of this.

I don't think we will support something like this in FlowRedux.

I apologise that my intention was not clear enough. We don't need FlowRedux to do the queueing, this was something that we would implement ourselves in a catch-all handler.

I'll be discussing with my team how we're going to go forward with the issue that we're experiencing.

[gabrielittner]

I've played a bit with the idea of only calling the most important handler and it's generally possible to build this on top of the existing API. This is what I got and based on the TODOs I think it's hard for us to build this in a generic way into the library. Like Hannes mentioned above we don't really know how to prioritize things.

This adds an inStateExclusive function to the DSL:

 @OptIn(ExperimentalCoroutinesApi::class)
 inline fun <reified S : Any, reified A: Any> FlowReduxStoreBuilder<S, A>.inStateExclusive(block: InStateExclusiveBuilder<S, A, S, A>.() -> Unit) {
     val builder = InStateExclusiveBuilder<S, A, S, A>(S::class).apply(block)
     val handlers = builder.build()
     
     // TODO: does not handle deeper type hierarchies because sealedSubclasses only returns direct subclasses
     //  so this would probably need to be recursive but that would also make the handler prioritization harder
     //  because someone could have a fallback handler in the intermediate state class
     S::class.sealedSubclasses.forEach { subState ->
         A::class.sealedSubclasses.forEach { subAction ->
             val handler = handlers[subState to subAction] ?:
                 // fall back to base action type first in sub state
                 handlers[subState to A::class] ?:
                 // fall back to concrete action in base state
                 // TODO: it's not clear whether this one or the one above should take precedence
                 handlers[S::class to subAction] ?:
                 // fall back to concrete action in base state
                 handlers[S::class to A::class]
             
             // the if could alternatively be a checkNotNull to enforce having a handler
             if (handler != null) {
                 @Suppress("invisible_member") // the internal API is marked with @PublishedApi so it's safe to call
                 inState(subState) {
                     on(subAction, ExecutionPolicy.CANCEL_PREVIOUS) { action, state ->
                         @Suppress("UNCHECKED_CAST") // this is safe
                         handler(action, state as State<S>)
                     }
                 }
             }
         }
     }
 }

class InStateExclusiveBuilder<BaseS: Any, BaseA : Any, S : BaseS, A : BaseA>(val stateType: KClass<S>) {
     val actionHandlers = mutableMapOf<Pair<KClass<out BaseS>, KClass<out BaseA>>, (BaseA, State<BaseS>) -> ChangedState<BaseS>> ()

     // TODO: needs all other dsl function
     inline fun <reified SubA: A> on(noinline handler: (SubA, State<S>) -> ChangedState<BaseS>) {
         @Suppress("UNCHECKED_CAST")
         actionHandlers[stateType to SubA::class] = handler as Function2<BaseA, State<BaseS>, ChangedState<BaseS>>
     }

     // nesting blocks for subclasses of subclasses
     inline fun <reified SubS : S> inSubStateExclusive(block: InStateExclusiveBuilder<BaseS, BaseA, SubS, A>.() -> Unit) {
         val builder = InStateExclusiveBuilder<BaseS, BaseA, SubS, A>(SubS::class).apply(block)
         actionHandlers.putAll(builder.build())
     }
     fun build() = actionHandlers.toMap()
 }

Example usage:

sealed interface BaseState
data object State1 : BaseState
data object State2 : BaseState
data object State3 : BaseState

sealed interface BaseAction
data object A : BaseAction
data object B : BaseAction
data object C : BaseAction
data object ChangeToState2 : BaseAction
data object ChangeToState3 : BaseAction

 @OptIn(ExperimentalCoroutinesApi::class)
 class MyStateMachine : FlowReduxStateMachine<BaseState, BaseAction>(State1) {
     init {
         spec {
             inStateExclusive<BaseState, BaseAction> {
                 on<BaseAction> { action, state ->
                     println("BaseState, BaseAction handler received $action")
                     state.noChange()
                 }
                 on<A> { action, state ->
                     println("BaseState, A handler received $action")
                     state.noChange()
                 }
                 on<B> { action, state ->
                     println("BaseState, B handler received $action")
                     state.noChange()
                 }

                 inSubStateExclusive<State1> {
                     on<A> { action, state ->
                         println("State1, A handler received $action")
                         state.noChange()
                     }
                     on<ChangeToState2> { _, state ->
                         state.override { State2 }
                     }
                 }

                 inSubStateExclusive<State2> {
                     on<B> { action, state ->
                         println("State2, B handler received $action")
                         state.noChange()
                     }
                     on<ChangeToState3> { _, state ->
                         state.override { State3 }
                     }
                 }
             }
         }
     }
 }

 fun main() = runBlocking {
     val sm = MyStateMachine()
     sm.state.test {
         println("SM in state ${awaitItem()}")
         sm.dispatch(A)
         sm.dispatch(B)
         sm.dispatch(C)

         sm.dispatch(ChangeToState2)
         println("SM in state ${awaitItem()}")
         sm.dispatch(A)
         sm.dispatch(B)
         sm.dispatch(C)

         sm.dispatch(ChangeToState3)
         println("SM in state ${awaitItem()}")
         sm.dispatch(A)
         sm.dispatch(B)
         sm.dispatch(C)
     }
 }

The above gives this output

SM in state State1
State1, A handler received A
BaseState, B handler received B
BaseState, BaseAction handler received C
SM in state State2
BaseState, A handler received A
State2, B handler received B
BaseState, BaseAction handler received C
SM in state State3
BaseState, A handler received A
BaseState, B handler received B
BaseState, BaseAction handler received C

Because of the mentioned complexity/uncertainty above it might be best to just build a solution like this on top of the existing DSL. That way you can make sure it fits the specific needs of your use case.

[doverdho]

Thank you for providing this sample code of the feature. My team will discuss it further internally to check if we can solve our issues with this.