fix: scrub :* from the list of states

improvement: detect states used that don't exist and log an error
This commit is contained in:
Zach Daniel 2023-09-12 08:40:18 -04:00
parent 83bbaf3686
commit fa109180e4
3 changed files with 92 additions and 15 deletions

View file

@ -13,6 +13,8 @@ defmodule AshStateMachine do
defstruct [:action, :from, :to, :__identifier__]
end
require Logger
@transition %Spark.Dsl.Entity{
name: :transition,
target: Transition,
@ -56,7 +58,20 @@ defmodule AshStateMachine do
A list of states that have been deprecated.
The list of states is derived from the transitions normally.
Use this option to express that certain types should still
be included in the derived state list even though no transitions
go to/from that state anymore. `:*` transitions will not include
these states.
"""
],
extra_states: [
type: {:list, :atom},
default: [],
doc: """
A list of states that may be used by transitions to/from `:*`
The list of states is derived from the transitions normally.
Use this option to express that certain types should still
be included even though no transitions go to/from that state anymore.
`:*` transitions will include these states.
"""
],
state_attribute: [
@ -122,21 +137,46 @@ defmodule AshStateMachine do
attribute = AshStateMachine.Info.state_machine_state_attribute!(changeset.resource)
old_state = Map.get(changeset.data, attribute)
case Enum.find(transitions, fn transition ->
old_state in List.wrap(transition.from) and target in List.wrap(transition.to)
end) do
nil ->
Ash.Changeset.add_error(
changeset,
AshStateMachine.Errors.NoMatchingTransition.exception(
old_state: old_state,
target: target,
action: changeset.action.name
if target in AshStateMachine.Info.state_machine_all_states(changeset.resource) do
case Enum.find(transitions, fn transition ->
old_state in List.wrap(transition.from) and target in List.wrap(transition.to)
end) do
nil ->
Ash.Changeset.add_error(
changeset,
AshStateMachine.Errors.NoMatchingTransition.exception(
old_state: old_state,
target: target,
action: changeset.action.name
)
)
)
_transition ->
Ash.Changeset.force_change_attribute(changeset, attribute, target)
_transition ->
Ash.Changeset.force_change_attribute(changeset, attribute, target)
end
else
Logger.error("""
Attempted to transition to an unknown state.
This usually means that one of the following is true:
* You have a missing transition definition in your state machine
To remediate this, add a transition.
* You are using `:*` to include a state that appears nowhere in the state machine definition
To remediate this, add the `extra_states` option and include the state #{inspect(target)}
""")
Ash.Changeset.add_error(
changeset,
AshStateMachine.Errors.NoMatchingTransition.exception(
old_state: old_state,
target: target,
action: changeset.action.name
)
)
end
end

View file

@ -16,6 +16,7 @@ defmodule AshStateMachine.Transformers.FillInTransitionDefaults do
List.wrap(transition.from) ++ List.wrap(transition.to)
end)
|> Enum.concat(List.wrap(initial_states))
|> Enum.concat(List.wrap(AshStateMachine.Info.state_machine_extra_states!(dsl_state)))
|> Enum.uniq()
|> Enum.reject(&(&1 == :*))

View file

@ -16,6 +16,8 @@ defmodule AshStateMachineTest do
transition :begin_delivery, from: :confirmed, to: :on_its_way
transition :package_arrived, from: :on_its_way, to: :arrived
transition :error, from: [:pending, :confirmed, :on_its_way], to: :error
transition :abort, from: :*, to: :aborted
transition :reroute, from: :*, to: :rerouted
end
end
@ -45,6 +47,19 @@ defmodule AshStateMachineTest do
accept [:error_state, :error]
change transition_state(:error)
end
update :abort do
# accept [...]
change transition_state(:aborted)
end
update :reroute do
# accept [...]
# The defined transition for this route contains a `from: :*` but does not include `to: :aborted`
# This should never succeed
change transition_state(:aborted)
end
end
changes do
@ -65,6 +80,12 @@ defmodule AshStateMachineTest do
on: [:update]
end
code_interface do
define_for AshStateMachineTest.Api
define :abort
define :reroute
end
attributes do
uuid_primary_key :id
# ...attributes like address/delivery options would go here
@ -151,17 +172,32 @@ defmodule AshStateMachineTest do
assert ThreeStates.complete!(state_machine).state == :complete
end
test "`from: :*` can transition from any state" do
for state <- [:pending, :confirmed, :on_its_way, :arrived, :error] do
assert {:ok, machine} = Order.abort(%Order{state: state})
assert machine.state == :aborted
end
end
test "`from: :*` cannot transition _to_ any state" do
for state <- [:pending, :confirmed, :on_its_way, :arrived, :error] do
assert {:error, reason} = Order.reroute(%Order{state: state})
assert Exception.message(reason) =~ ~r/no matching transition/i
end
end
end
describe "charts" do
test "it generates the appropriate chart" do
AshStateMachine.Charts.mermaid_state_diagram(Order) |> IO.puts()
assert AshStateMachine.Charts.mermaid_flowchart(ThreeStates) ==
"""
flowchart TD
pending --> |begin| executing
executing --> |complete| complete
complete --> pending
executing --> pending
pending --> pending
"""
|> String.trim_trailing()
end