Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Allow transitions in callbacks #1

Closed
wants to merge 2 commits into from

Conversation

annismckenzie
Copy link

@annismckenzie annismckenzie commented Dec 11, 2021

This fixes looplab#43 and looplab#61.

This contains breaking changes, namely:

  • signature change of fsm.Event() from name string, ...args to ctx context.Context, name string, ...args
  • signature change of event handlers from e *fsm.Event to ctx context.Context, e *fsm.Event (refs Context in event handlers looplab/fsm#61)

This allows state transitions to happen in enter state and after event callbacks:

fsm/fsm_test.go

Lines 672 to 711 in d8a0147

func TestTransitionInCallbacks(t *testing.T) {
var fsm *FSM
var afterFinishCalled bool
fsm = NewFSM(
"start",
Events{
{Name: "run", Src: []string{"start"}, Dst: "end"},
{Name: "finish", Src: []string{"end"}, Dst: "finished"},
{Name: "reset", Src: []string{"end", "finished"}, Dst: "start"},
},
Callbacks{
"enter_end": func(ctx context.Context, e *Event) {
if err := e.FSM.Event(ctx, "finish"); err != nil {
fmt.Println(err)
}
},
"after_finish": func(ctx context.Context, e *Event) {
afterFinishCalled = true
if e.Src != "end" {
panic(fmt.Sprintf("source should have been 'end' but was '%s'", e.Src))
}
if err := e.FSM.Event(ctx, "reset"); err != nil {
fmt.Println(err)
}
},
},
)
if err := fsm.Event(context.Background(), "run"); err != nil {
t.Errorf("expected no error, got %v", err)
}
if !afterFinishCalled {
t.Error("expected after_finish callback to have been executed but it wasn't")
}
currentState := fsm.Current()
if currentState != "start" {
t.Errorf("expected state to be 'start', was '%s'", currentState)
}
}

@annismckenzie annismckenzie self-assigned this Dec 11, 2021
fsm.go Outdated Show resolved Hide resolved
event.go Outdated Show resolved Hide resolved
@annismckenzie annismckenzie force-pushed the allow-transitions-in-callbacks branch from 7e09989 to 75ef044 Compare December 12, 2021 15:03
event.go Outdated Show resolved Hide resolved
@annismckenzie annismckenzie force-pushed the allow-transitions-in-callbacks branch 2 times, most recently from 1d81a5a to d8a0147 Compare December 12, 2021 15:34
@annismckenzie annismckenzie marked this pull request as ready for review December 12, 2021 15:40
@annismckenzie annismckenzie force-pushed the allow-transitions-in-callbacks branch 2 times, most recently from d8a0147 to adf7a7d Compare December 12, 2021 15:51
This adds the possibility of "starting" a state machine and have it
execute multiple state transitions in succession, given that no errors
occur.

The equivalent code without this change:
```go
var errTransition error

for errTransition == nil {
	transitions := request.FSM.AvailableTransitions()
	if len(transitions) == 0 {
		break
	}
	if len(transitions) > 1 {
		errTransition = errors.New("only 1 transition should be available")
	}
	errTransition = request.FSM.Event(transitions[0])
}

if errTransition != nil {
	fmt.Println(errTransition)
}
```

Arguably, that’s bad because of several reasons:
1. The state machine is used like a puppet.
2. The state transitions that make up the "happy path" are encoded
   outside the state machine.
3. The code really isn’t good.
4. There’s no way to intervene or make different decisions on which
   state to transition to next (reinforces bullet point 2).
5. There’s no way to add proper error handling.

It is possible to fix a certain number of those problems but not all
of them, especially 2 and 4 but also 1.

The added test is green and uses both an enter state and an after event
callback.

No other test case was touched in any way (besides enhancing the
context one that was added in the previous commit).
This adds a context and cancelation facility to the type `AsyncError`.
Async state transitions can now be canceled by calling `CancelTransition`
on the AsyncError returned by `fsm.Event`. The context on that error can
also be handed off as described in looplab#77 (comment).
@annismckenzie annismckenzie force-pushed the allow-transitions-in-callbacks branch from 751a073 to 94fb353 Compare July 29, 2022 14:11
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2761186635

  • 49 of 51 (96.08%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 93.735%

Changes Missing Coverage Covered Lines Changed/Added Lines %
uncancel_context.go 4 6 66.67%
Totals Coverage Status
Change from base Build 2761174425: 0.1%
Covered Lines: 404
Relevant Lines: 431

💛 - Coveralls

@annismckenzie
Copy link
Author

With looplab#82 merged I can now open a new PR against upstream – closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scheduling internal transitions within callbacks
2 participants