diff --git a/lib/ash_state_machine.ex b/lib/ash_state_machine.ex index 49472d4..264db8f 100644 --- a/lib/ash_state_machine.ex +++ b/lib/ash_state_machine.ex @@ -137,20 +137,27 @@ defmodule AshStateMachine do end end + def transition_state(%{action_type: :create, action: %{upsert?: true}} = changeset, target) do + attribute = AshStateMachine.Info.state_machine_state_attribute!(changeset.resource) + old_state = Map.get(changeset.data, attribute) + + cond do + target not in AshStateMachine.Info.state_machine_initial_states!(changeset.resource) -> + invalid_initial_state(changeset, target) + + target not in available_upsert_targets(changeset) -> no_matching_transition(changeset, target, old_state) + + true -> Ash.Changeset.force_change_attribute(changeset, attribute, target) + end + end + def transition_state(%{action_type: :create} = changeset, target) do attribute = AshStateMachine.Info.state_machine_state_attribute!(changeset.resource) if target in AshStateMachine.Info.state_machine_initial_states!(changeset.resource) do Ash.Changeset.force_change_attribute(changeset, attribute, target) else - changeset - |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}}) - |> Ash.Changeset.add_error( - AshStateMachine.Errors.InvalidInitialState.exception( - target: target, - action: changeset.action.name - ) - ) + invalid_initial_state(changeset, target) end end @@ -166,15 +173,7 @@ defmodule AshStateMachine do end) |> case do nil -> - changeset - |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}}) - |> Ash.Changeset.add_error( - AshStateMachine.Errors.NoMatchingTransition.exception( - old_state: old_state, - target: target, - action: changeset.action.name - ) - ) + no_matching_transition(changeset, target, old_state) _transition -> Ash.Changeset.force_change_attribute(changeset, attribute, target) @@ -197,6 +196,11 @@ defmodule AshStateMachine do To remediate this, add the `extra_states` option and include the state #{inspect(target)} """) + no_matching_transition(changeset, target, old_state) + end + + @doc false + defp no_matching_transition(changeset, target, old_state) do changeset |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}}) |> Ash.Changeset.add_error( @@ -208,6 +212,17 @@ defmodule AshStateMachine do ) end + defp invalid_initial_state(changeset, target) do + changeset + |> Ash.Changeset.set_context(%{state_machine: %{attempted_change: target}}) + |> Ash.Changeset.add_error( + AshStateMachine.Errors.InvalidInitialState.exception( + target: target, + action: changeset.action.name + ) + ) + end + @doc """ A reusable helper which returns all possible next states for a record (regardless of action). @@ -243,4 +258,11 @@ defmodule AshStateMachine do |> Enum.reject(&(&1 == :*)) |> Enum.uniq() end + + defp available_upsert_targets(changeset) do + AshStateMachine.Info.state_machine_transitions(changeset.resource, changeset.action.name) + |> Enum.map(& &1.to) + |> List.flatten() + |> Enum.uniq() + end end diff --git a/lib/verifiers/verify_transition_actions.ex b/lib/verifiers/verify_transition_actions.ex index 55a02f7..c4c9eac 100644 --- a/lib/verifiers/verify_transition_actions.ex +++ b/lib/verifiers/verify_transition_actions.ex @@ -11,17 +11,48 @@ defmodule AshStateMachine.Verifiers.VerifyTransitionActions do end) |> Enum.each(fn transition -> action = Ash.Resource.Info.action(dsl_state, transition.action) + all_states = dsl_state |> AshStateMachine.Info.state_machine_all_states() - unless action && action.type == :update do - raise Spark.Error.DslError, - module: Spark.Dsl.Verifier.get_persisted(dsl_state, :module), - path: [:state_machine, :transitions, :transition, transition.action], - message: """ - Transition configured with action `:#{transition.action}` but no such update action is defined. - """ + case validate(action, transition, all_states) do + :ok -> + :ok + + {:error, err} -> + raise Spark.Error.DslError, + module: Spark.Dsl.Verifier.get_persisted(dsl_state, :module), + path: [:state_machine, :transitions, :transition, transition.action], + message: """ + #{error_message(err, transition.action)} + """ end end) :ok end + + defp validate(nil, _, _), do: {:error, :no_such_action} + defp validate(%{type: :update}, _, _), do: :ok + defp validate(%{type: :create, upsert?: false}, _, _), do: {:error, :create_must_upsert} + + defp validate(%{type: :create}, %{from: from}, all_states) do + case Enum.sort(from) == Enum.sort(all_states) do + true -> :ok + false -> {:error, :create_must_allow_from_all} + end + end + + defp validate(_, _, _), do: {:error, :no_such_action} + + defp error_message(err, action) do + case err do + :no_such_action -> + "Transition configured with action `:#{action}` but no such create or update action is defined. Actions must be of type update or create with `upsert?: true`" + + :create_must_upsert -> + "Transition configured with non-upsert create action `:#{action}`. Create actions must be configured with `upsert? true` to allow state transitions." + + :create_must_allow_from_all -> + "Transition configured with create action `:#{action}` must allow transitions from all states." + end + end end diff --git a/mix.exs b/mix.exs index 90f2069..5c5bc16 100644 --- a/mix.exs +++ b/mix.exs @@ -21,7 +21,8 @@ defmodule AshStateMachine.MixProject do docs: docs(), description: @description, source_url: "https://github.com/ash-project/ash_state_machine", - homepage_url: "https://github.com/ash-project/ash_state_machine" + homepage_url: "https://github.com/ash-project/ash_state_machine", + consolidate_protocols: Mix.env() != :test ] end diff --git a/test/ash_state_machine_test.exs b/test/ash_state_machine_test.exs index 117235e..a5bb553 100644 --- a/test/ash_state_machine_test.exs +++ b/test/ash_state_machine_test.exs @@ -44,6 +44,90 @@ defmodule AshStateMachineTest do assert Exception.message(reason) =~ ~r/no matching transition/i end end + + test "create actions are allowed with `upsert? true`" do + state_machine = Verification.create!() |> Verification.begin!() + assert Verification.reset!(%{id: state_machine.id}).state == :pending + end + + test "create upsert? actions do not allow invalid states" do + state_machine = Verification.create!() |> Verification.begin!() + assert {:error, reason} = Verification.broken_upsert(%{id: state_machine.id}) + assert Exception.message(reason) =~ ~r/no matching transition/i + end + + test "create actions without `upsert? true` do not compile" do + assert_raise Spark.Error.DslError, ~r/non-upsert create action/, fn -> + defmodule CreateWithoutUpsert do + use Ash.Resource, + domain: nil, + extensions: [AshStateMachine] + + state_machine do + initial_states [:pending] + + transitions do + transition :reset, from: :*, to: :pending + end + end + + actions do + create :reset do + change transition_state(:pending) + end + end + end + end + end + + test "create action transitions without `from: :*` do not compile" do + assert_raise Spark.Error.DslError, ~r/must allow transitions from all states/, fn -> + defmodule CreateWithoutAllowingFromAll do + use Ash.Resource, + domain: nil, + extensions: [AshStateMachine] + + state_machine do + initial_states [:pending, :howdy] + + transitions do + transition :reset, from: :howdy, to: :pending + end + end + + actions do + create :reset do + upsert? true + change transition_state(:pending) + end + end + end + end + end + + test "any action other than update or create with upsert? true does not compile" do + assert_raise Spark.Error.DslError, ~r/no such create or update action/, fn -> + defmodule DeleteAction do + use Ash.Resource, + domain: nil, + extensions: [AshStateMachine] + + state_machine do + initial_states [:pending] + + transitions do + transition :delete, from: :*, to: :pending + end + end + + actions do + destroy :delete do + change transition_state(:pending) + end + end + end + end + end end describe "charts" do diff --git a/test/support/domain.ex b/test/support/domain.ex index f55946e..eb3693a 100644 --- a/test/support/domain.ex +++ b/test/support/domain.ex @@ -6,5 +6,6 @@ defmodule Domain do resource ThreeStates resource Order resource NextStateMachine + resource Verification end end diff --git a/test/support/verification.ex b/test/support/verification.ex new file mode 100644 index 0000000..0e175b4 --- /dev/null +++ b/test/support/verification.ex @@ -0,0 +1,48 @@ +defmodule Verification do + @moduledoc false + use Ash.Resource, + domain: Domain, + data_layer: Ash.DataLayer.Ets, + extensions: [AshStateMachine] + + state_machine do + initial_states [:pending] + default_initial_state :pending + + transitions do + transition(:begin, from: :pending, to: :executing) + transition(:reset, from: :*, to: :pending) + transition(:broken_upsert, from: :*, to: [:foo, :bar]) + end + end + + actions do + default_accept :* + defaults [:read, :create] + + update :begin do + change transition_state(:executing) + end + + create :reset do + upsert? true + change transition_state(:pending) + end + + create :broken_upsert do + upsert? true + change transition_state(:pending) + end + end + + attributes do + uuid_primary_key :id, writable?: true + end + + code_interface do + define :create + define :begin + define :reset + define :broken_upsert + end +end