-
-
Notifications
You must be signed in to change notification settings - Fork 371
feat: add extends option for actions
#2646
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ defmodule Ash.Resource.Actions.Create do | |
| :description, | ||
| :error_handler, | ||
| :multitenancy, | ||
| extends: nil, | ||
| accept: nil, | ||
| require_attributes: [], | ||
| allow_nil_input: [], | ||
|
Comment on lines
11
to
16
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ defmodule Ash.Resource.Actions.Destroy do | |
| :description, | ||
| :error_handler, | ||
| :multitenancy, | ||
| extends: nil, | ||
| manual: nil, | ||
| require_atomic?: Application.compile_env(:ash, :require_atomic_by_default?, true), | ||
| skip_unknown_inputs: [], | ||
|
Comment on lines
13
to
18
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ defmodule Ash.Resource.Actions.Read do | |
|
|
||
| defstruct arguments: [], | ||
| description: nil, | ||
| extends: nil, | ||
| filter: nil, | ||
|
Comment on lines
8
to
11
|
||
| filters: [], | ||
| get_by: nil, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ defmodule Ash.Resource.Actions.Update do | |
| :description, | ||
| :error_handler, | ||
| :multitenancy, | ||
| extends: nil, | ||
| accept: nil, | ||
| require_attributes: [], | ||
| allow_nil_input: [], | ||
|
Comment on lines
12
to
17
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,123 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| defmodule Ash.Resource.Transformers.ExtendActions do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @moduledoc "Resolves `extends` option on actions by merging configuration from the base action." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use Spark.Dsl.Transformer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alias Spark.Dsl.Transformer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alias Spark.Error.DslError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def before?(Ash.Resource.Transformers.RequireUniqueActionNames), do: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def before?(Ash.Resource.Transformers.SetPrimaryActions), do: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def before?(Ash.Resource.Transformers.DefaultAccept), do: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def before?(Ash.Resource.Transformers.GetByReadActions), do: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def before?(_), do: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @list_fields %{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| create: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| update: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics)a, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| destroy: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+21
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a, | |
| read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a, | |
| update: | |
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics)a, | |
| destroy: | |
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs)a, | |
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs action_select)a, | |
| read: ~w(arguments preparations filters metadata touches_resources skip_unknown_inputs)a, | |
| update: | |
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs atomics action_select)a, | |
| destroy: | |
| ~w(arguments changes metadata reject require_attributes allow_nil_input notifiers touches_resources skip_unknown_inputs action_select)a, |
Copilot
AI
Mar 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
base = Enum.find(all_actions, &(&1.name == action.extends)) can resolve to the action itself when extends points to its own name (or can participate in a cycle across multiple actions). That will silently duplicate list fields and produce unpredictable merges. Please add an explicit guard that disallows self-extension and detect/raise on cycles (e.g., by tracking a visited set while resolving bases).
Copilot
AI
Mar 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The raised DslError here doesn’t include location, and the path is only [:actions, action.type], which makes it hard to pinpoint the offending extends option. Consider using the action entity’s :extends property anno for location and a more specific path like [:actions, action.name, :extends] (consistent with other action-related transformers).
Copilot
AI
Mar 24, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extends resolution is order-dependent for multi-level inheritance: if action A extends B and B extends C, and A is processed before B, A will merge against the unresolved B and won’t inherit C’s config. Consider resolving the full extends chain recursively/topologically (with cycle detection) before merging, so results don’t depend on action declaration order.
| dsl_state | |
| |> Transformer.get_entities([:actions]) | |
| |> Enum.filter(& &1.extends) | |
| |> Enum.reduce_while({:ok, dsl_state}, fn action, {:ok, dsl_state} -> | |
| all_actions = Transformer.get_entities(dsl_state, [:actions]) | |
| base = Enum.find(all_actions, &(&1.name == action.extends)) | |
| cond do | |
| is_nil(base) -> | |
| {:halt, | |
| {:error, | |
| DslError.exception( | |
| module: Transformer.get_persisted(dsl_state, :module), | |
| path: [:actions, action.type], | |
| message: | |
| "Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists." | |
| )}} | |
| base.type != action.type -> | |
| {:halt, | |
| {:error, | |
| DslError.exception( | |
| module: Transformer.get_persisted(dsl_state, :module), | |
| path: [:actions, action.type], | |
| message: | |
| "Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type." | |
| )}} | |
| true -> | |
| merged = merge_action(action, base) | |
| new_state = | |
| Transformer.replace_entity( | |
| dsl_state, | |
| [:actions], | |
| merged, | |
| &(&1.name == action.name && &1.type == action.type) | |
| ) | |
| {:cont, {:ok, new_state}} | |
| end | |
| end) | |
| actions = Transformer.get_entities(dsl_state, [:actions]) | |
| actions_by_key = | |
| Map.new(actions, fn action -> | |
| {{action.name, action.type}, action} | |
| end) | |
| actions | |
| |> Enum.filter(& &1.extends) | |
| |> Enum.reduce_while({:ok, dsl_state, %{}}, fn action, {:ok, dsl_state, resolved} -> | |
| case resolve_action(action, dsl_state, actions_by_key, resolved, MapSet.new()) do | |
| {:ok, new_state, new_resolved} -> | |
| {:cont, {:ok, new_state, new_resolved}} | |
| {:error, _} = error -> | |
| {:halt, error} | |
| end | |
| end) | |
| |> case do | |
| {:ok, dsl_state, _resolved} -> | |
| {:ok, dsl_state} | |
| {:error, _} = error -> | |
| error | |
| end | |
| end | |
| defp resolve_action(action, dsl_state, actions_by_key, resolved, visiting) do | |
| key = {action.name, action.type} | |
| cond do | |
| Map.has_key?(resolved, key) -> | |
| {:ok, dsl_state, resolved} | |
| MapSet.member?(visiting, key) -> | |
| {:error, | |
| DslError.exception( | |
| module: Transformer.get_persisted(dsl_state, :module), | |
| path: [:actions, action.type], | |
| message: | |
| "Cyclic action inheritance detected starting at `#{action.name}` for type `#{action.type}`." | |
| )} | |
| is_nil(action.extends) -> | |
| {:ok, dsl_state, Map.put(resolved, key, true)} | |
| true -> | |
| base_key = {action.extends, action.type} | |
| base = Map.get(actions_by_key, base_key) | |
| cond do | |
| is_nil(base) -> | |
| {:error, | |
| DslError.exception( | |
| module: Transformer.get_persisted(dsl_state, :module), | |
| path: [:actions, action.type], | |
| message: | |
| "Action `#{action.name}` extends `#{action.extends}`, but no action named `#{action.extends}` exists." | |
| )} | |
| base.type != action.type -> | |
| {:error, | |
| DslError.exception( | |
| module: Transformer.get_persisted(dsl_state, :module), | |
| path: [:actions, action.type], | |
| message: | |
| "Action `#{action.name}` (#{action.type}) extends `#{action.extends}` (#{base.type}), but they must be the same type." | |
| )} | |
| true -> | |
| visiting = MapSet.put(visiting, key) | |
| case resolve_action(base, dsl_state, actions_by_key, resolved, visiting) do | |
| {:ok, dsl_state, resolved} -> | |
| all_actions = Transformer.get_entities(dsl_state, [:actions]) | |
| resolved_base = | |
| Enum.find(all_actions, fn candidate -> | |
| candidate.name == action.extends && candidate.type == action.type | |
| end) | |
| merged = merge_action(action, resolved_base) | |
| new_state = | |
| Transformer.replace_entity( | |
| dsl_state, | |
| [:actions], | |
| merged, | |
| &(&1.name == action.name && &1.type == action.type) | |
| ) | |
| {:ok, new_state, Map.put(resolved, key, true)} | |
| {:error, _} = error -> | |
| error | |
| end | |
| end | |
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extendswas added to the struct, but the@type tspec below doesn’t include the newextendsfield. Please update the type spec to match the struct fields.