Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug+fix] Inconsistency in sync/async statemachine execution #444

Closed
vrenken opened this issue Apr 7, 2021 · 1 comment
Closed

[Bug+fix] Inconsistency in sync/async statemachine execution #444

vrenken opened this issue Apr 7, 2021 · 1 comment

Comments

@vrenken
Copy link

vrenken commented Apr 7, 2021

Hi,

during the easter holidays the wetter was bad which gave me some time to work on a PlantUML 2 Stateless Roslyn code generator.

It's amazing to see how such a great library as Stateless benefits when being configured in a more visually way - It completely speeds up the whole development process as the mental model is far easier to grasp.

However, during this testing I also wanted to make sure that both synchronous and asynchronous state machines work as expected, and in doing so I think I might have spotted an issue in how asynchronous calls in Stateless work with superstates/substates.

It's best to demonstrate with two 'identical' state machines.

First the synchronous one (which works as expected):

  [Fact]
  public void SuperStateShouldNotExitOnSubStateTransition_WhenUsingSyncTriggers()
  {
      // Arrange.
      var sm = new StateMachine<State, Trigger>(State.A);
      var record = new List<string>();

      sm.Configure(State.A)
          .OnEntry(() => record.Add("Entered state A"))
          .OnExit(() => record.Add("Exited state A"))
          .Permit(Trigger.X, State.B);
      
      sm.Configure(State.B) // Our super state.
          .InitialTransition(State.C)
          .OnEntry(() => record.Add("Entered super state B"))
          .OnExit(() => record.Add("Exited super state B"));

      sm.Configure(State.C) // Our first sub state.
          .OnEntry(() => record.Add("Entered sub state C"))
          .OnExit(() => record.Add("Exited sub state C"))
          .Permit(Trigger.Y, State.D)
          .SubstateOf(State.B);
      sm.Configure(State.D) // Our second sub state.
          .OnEntry(() => record.Add("Entered sub state D"))
          .OnExit(() => record.Add("Exited sub state D"))
          .SubstateOf(State.B);

      
      // Act.
      sm.Fire(Trigger.X);
      sm.Fire(Trigger.Y);
      
      // Assert.
      Assert.Equal("Exited state A", record[0]);
      Assert.Equal("Entered super state B", record[1]);
      Assert.Equal("Entered sub state C", record[2]);
      Assert.Equal("Exited sub state C", record[3]);
      Assert.Equal("Entered sub state D", record[4]);
  }

And the asynchronous one (which doesn't work as expected)

  [Fact]
  public async Task SuperStateShouldNotExitOnSubStateTransition_WhenUsingAsyncTriggers()
  {
      // Arrange.
      var sm = new StateMachine<State, Trigger>(State.A);
      var record = new List<string>();

      sm.Configure(State.A)
          .OnEntryAsync(() => Task.Run(() => record.Add("Entered state A")))
          .OnExitAsync(() => Task.Run(() => record.Add("Exited state A")))
          .Permit(Trigger.X, State.B);
      
      sm.Configure(State.B) // Our super state.
          .InitialTransition(State.C)
          .OnEntryAsync(() => Task.Run(() => record.Add("Entered super state B")))
          .OnExitAsync(() => Task.Run(() => record.Add("Exited super state B")));

      sm.Configure(State.C) // Our first sub state.
          .OnEntryAsync(() => Task.Run(() => record.Add("Entered sub state C")))
          .OnExitAsync(() => Task.Run(() => record.Add("Exited sub state C")))
          .Permit(Trigger.Y, State.D)
          .SubstateOf(State.B);
      sm.Configure(State.D) // Our second sub state.
          .OnEntryAsync(() => Task.Run(() => record.Add("Entered sub state D")))
          .OnExitAsync(() => Task.Run(() => record.Add("Exited sub state D")))
          .SubstateOf(State.B);

      
      // Act.
      await sm.FireAsync(Trigger.X).ConfigureAwait(false);
      await sm.FireAsync(Trigger.Y).ConfigureAwait(false);
      
      // Assert.
      Assert.Equal("Exited state A", record[0]);
      Assert.Equal("Entered super state B", record[1]);
      Assert.Equal("Entered sub state C", record[2]);
      Assert.Equal("Exited sub state C", record[3]);
      Assert.Equal("Entered sub state D", record[4]); // Before the patch the actual result was "Exited super state B"
  }

Both state machines follow the same transitions as specified below, but in case of the asynchronous executing one It seems that superstate B is exiting even on a transition between its substates.

This is caused by an inconsistency in ExitAsync (in StateRepresentation.Async.cs) compared to Exit (in StateRepresentation.cs). There seem to be some checks missing.
I've got a small correction ready and two additional unit tests to prove its feasibility and correctness.

Happy to assist Stateless with a PR to fix this: #445

@vrenken vrenken changed the title [Bug] Inconsistency in sync/async statemachine execution [Bug+fix] Inconsistency in sync/async statemachine execution Apr 7, 2021
@HenningNT
Copy link
Contributor

HenningNT commented Apr 27, 2021

Thank you, I'll get this released as soon as I have time ;-)

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

No branches or pull requests

2 participants