From e680867be922ab754b7554315cc29e985029a627 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 25 Oct 2023 17:45:55 -0400 Subject: [PATCH] fix: don't reorder global validations/changes fix: use latest spark, and new persisters callback fix: properly validate belongs_to relationships --- documentation/dsls/DSL:-Ash.Flow.cheatmd | 4 +- .../dsls/DSL:-Ash.Policy.Authorizer.cheatmd | 4 +- documentation/dsls/DSL:-Ash.Resource.cheatmd | 36 ++++----- lib/ash/actions/create/create.ex | 3 +- lib/ash/actions/managed_relationships.ex | 25 +----- lib/ash/changeset/changeset.ex | 7 ++ lib/ash/resource/dsl.ex | 12 ++- lib/ash/resource/info.ex | 77 +++++++++++++------ .../transformers/attributes_by_name.ex | 19 +++-- .../validations_and_changes_for_type.ex | 2 + mix.exs | 2 +- mix.lock | 2 +- 12 files changed, 104 insertions(+), 89 deletions(-) diff --git a/documentation/dsls/DSL:-Ash.Flow.cheatmd b/documentation/dsls/DSL:-Ash.Flow.cheatmd index f66d5cf9..8b66e8b9 100644 --- a/documentation/dsls/DSL:-Ash.Flow.cheatmd +++ b/documentation/dsls/DSL:-Ash.Flow.cheatmd @@ -921,7 +921,7 @@ end - list(module) | module + module | list(module) @@ -2470,7 +2470,7 @@ end - list(list(atom) | atom) + list(atom | list(atom)) diff --git a/documentation/dsls/DSL:-Ash.Policy.Authorizer.cheatmd b/documentation/dsls/DSL:-Ash.Policy.Authorizer.cheatmd index 637d2495..ee97e8bc 100644 --- a/documentation/dsls/DSL:-Ash.Policy.Authorizer.cheatmd +++ b/documentation/dsls/DSL:-Ash.Policy.Authorizer.cheatmd @@ -1291,7 +1291,7 @@ A field policy that, if passed, will skip all following field policies for that - list(atom) | atom + atom | list(atom) @@ -1807,7 +1807,7 @@ for more. - list(atom) | atom + atom | list(atom) diff --git a/documentation/dsls/DSL:-Ash.Resource.cheatmd b/documentation/dsls/DSL:-Ash.Resource.cheatmd index 6a211faa..73d63b79 100644 --- a/documentation/dsls/DSL:-Ash.Resource.cheatmd +++ b/documentation/dsls/DSL:-Ash.Resource.cheatmd @@ -5235,7 +5235,7 @@ validate changing(:email) - list((any -> any) | module) | (any -> any) | module + (any -> any) | module | list((any -> any) | module) [] @@ -5915,7 +5915,7 @@ end - list(atom) | atom + atom | list(atom) @@ -7187,7 +7187,7 @@ validate changing(:email) - list((any -> any) | module) | (any -> any) | module + (any -> any) | module | list((any -> any) | module) [] @@ -8268,7 +8268,7 @@ validate changing(:email) - list((any -> any) | module) | (any -> any) | module + (any -> any) | module | list((any -> any) | module) [] @@ -9001,7 +9001,7 @@ define :get_user_by_id, action: :get_by_id, args: [:id], get?: true - list(atom) | atom + atom | list(atom) @@ -9458,7 +9458,7 @@ identity :full_name, [:first_name, :last_name] - list(atom) | atom + atom | list(atom) @@ -9682,7 +9682,7 @@ change {MyCustomChange, :foo} - list(:create | :update | :destroy) | :create | :update | :destroy + :create | :update | :destroy | list(:create | :update | :destroy) [:create, :update] @@ -9958,7 +9958,7 @@ validate at_least_one_of_present([:first_name, :last_name]) - list((any -> any) | module) | (any -> any) | module + (any -> any) | module | list((any -> any) | module) [] @@ -9979,7 +9979,7 @@ validate at_least_one_of_present([:first_name, :last_name]) - list(:create | :update | :destroy) | :create | :update | :destroy + :create | :update | :destroy | list(:create | :update | :destroy) [:create, :update] @@ -10187,7 +10187,7 @@ end - list(atom) | atom + atom | list(atom) @@ -10492,7 +10492,7 @@ exists :has_ticket, :assigned_tickets - list(atom) | atom + atom | list(atom) @@ -10761,7 +10761,7 @@ end - list(atom) | atom + atom | list(atom) @@ -11068,7 +11068,7 @@ end - list(atom) | atom + atom | list(atom) @@ -11356,7 +11356,7 @@ end - list(atom) | atom + atom | list(atom) @@ -11683,7 +11683,7 @@ end - list(atom) | atom + atom | list(atom) @@ -11970,7 +11970,7 @@ end - list(atom) | atom + atom | list(atom) @@ -12257,7 +12257,7 @@ end - list(atom) | atom + atom | list(atom) @@ -12546,7 +12546,7 @@ end - list(atom) | atom + atom | list(atom) diff --git a/lib/ash/actions/create/create.ex b/lib/ash/actions/create/create.ex index cd729186..002b7f38 100644 --- a/lib/ash/actions/create/create.ex +++ b/lib/ash/actions/create/create.ex @@ -442,8 +442,7 @@ defmodule Ash.Actions.Create do {changeset, _} = Ash.Actions.ManagedRelationships.validate_required_belongs_to( - {changeset, []}, - false + {changeset, []} ) changeset diff --git a/lib/ash/actions/managed_relationships.ex b/lib/ash/actions/managed_relationships.ex index 39f7d170..7ed7a1b2 100644 --- a/lib/ash/actions/managed_relationships.ex +++ b/lib/ash/actions/managed_relationships.ex @@ -61,29 +61,6 @@ defmodule Ash.Actions.ManagedRelationships do end) end - def setup_managed_belongs_to_relationships(%{relationships: relationships} = changeset, _, _) - when relationships in [%{}, nil] do - {changeset, %{notifications: []}} - |> validate_required_belongs_to() - |> case do - {:error, error} -> - {:error, error} - - {changeset, instructions} -> - changeset = - Map.update!(changeset, :relationships, fn relationships -> - Map.new(relationships, fn {rel, inputs} -> - {rel, - Enum.map(inputs, fn {input, config} -> - {input, Keyword.put(config, :handled?, true)} - end)} - end) - end) - - {changeset, instructions} - end - end - def setup_managed_belongs_to_relationships(changeset, actor, engine_opts) do changeset.relationships |> Enum.map(fn {relationship, val} -> @@ -318,7 +295,7 @@ defmodule Ash.Actions.ManagedRelationships do {:halt, {:error, error}} end end) - |> validate_required_belongs_to() + |> validate_required_belongs_to(false) |> case do {:error, error} -> {:error, error} diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index bafb6b75..b29b1ce4 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -887,6 +887,7 @@ defmodule Ash.Changeset do def prepare_changeset_for_action(changeset, action, opts) do changeset |> Map.put(:action, action) + |> reset_arguments() |> handle_errors(action.error_handler) |> set_actor(opts) |> set_authorize(opts) @@ -895,6 +896,12 @@ defmodule Ash.Changeset do |> set_tenant(opts[:tenant] || changeset.tenant || changeset.data.__metadata__[:tenant]) end + defp reset_arguments(%{arguments: arguments} = changeset) do + Enum.reduce(arguments, changeset, fn {key, value}, changeset -> + set_argument(changeset, key, value) + end) + end + def handle_params(changeset, action, params, handle_params_opts \\ []) do if Keyword.get(handle_params_opts, :cast_params?, true) do cast_params(changeset, action, params || %{}) diff --git a/lib/ash/resource/dsl.ex b/lib/ash/resource/dsl.ex index 16333663..2ad89ed5 100644 --- a/lib/ash/resource/dsl.ex +++ b/lib/ash/resource/dsl.ex @@ -1314,9 +1314,6 @@ defmodule Ash.Resource.Dsl do ] @transformers [ - Ash.Resource.Transformers.CacheRelationships, - Ash.Resource.Transformers.AttributesByName, - Ash.Resource.Transformers.ValidationsAndChangesForType, Ash.Resource.Transformers.RequireUniqueActionNames, Ash.Resource.Transformers.SetRelationshipSource, Ash.Resource.Transformers.BelongsToAttribute, @@ -1333,6 +1330,12 @@ defmodule Ash.Resource.Dsl do Ash.Resource.Transformers.GetByReadActions ] + @persisters [ + Ash.Resource.Transformers.CacheRelationships, + Ash.Resource.Transformers.AttributesByName, + Ash.Resource.Transformers.ValidationsAndChangesForType + ] + @verifiers [ Ash.Resource.Verifiers.ValidateRelationshipAttributesMatch, Ash.Resource.Verifiers.VerifyReservedCalculationArguments, @@ -1356,7 +1359,8 @@ defmodule Ash.Resource.Dsl do use Spark.Dsl.Extension, sections: @sections, transformers: @transformers, - verifiers: @verifiers + verifiers: @verifiers, + persisters: @persisters @doc false def identity(x), do: x diff --git a/lib/ash/resource/info.ex b/lib/ash/resource/info.ex index 1b2ba040..1664e320 100644 --- a/lib/ash/resource/info.ex +++ b/lib/ash/resource/info.ex @@ -219,15 +219,10 @@ defmodule Ash.Resource.Info do Ash.Resource.Validation.t() ] def validations(resource, type) do - case Extension.get_persisted(resource, :validations_by_on) do - nil -> - resource - |> validations() - |> Enum.filter(&(type in &1.on)) - - persisted -> - Map.get(persisted, type) || [] - end + Extension.get_persisted(resource, :validations_by_on)[type] || + resource + |> validations() + |> Enum.filter(&(type in &1.on)) end @doc "A list of all validations for the resource" @@ -243,15 +238,10 @@ defmodule Ash.Resource.Info do | Ash.Resource.Change.t() ) def changes(resource, type) do - case Extension.get_persisted(resource, :changes_by_on) do - nil -> - resource - |> changes() - |> Enum.filter(&(type in &1.on)) - - persisted -> - Map.get(persisted, type) || [] - end + Extension.get_persisted(resource, :changes_by_on)[type] || + resource + |> changes() + |> Enum.filter(&(type in &1.on)) end @doc "A list of all changes for the resource" @@ -335,7 +325,11 @@ defmodule Ash.Resource.Info do @doc "The required belongs_to relationships" def required_belongs_to_relationships(resource) do - Extension.get_persisted(resource, :required_belongs_to_relationships) + Extension.get_persisted(resource, :required_belongs_to_relationships) || + Enum.filter( + relationships(resource), + &(&1.type == :belongs_to && !&1.allow_nil?) + ) end @doc "Get a public relationship by name or path" @@ -592,11 +586,19 @@ defmodule Ash.Resource.Info do type :: :create | :update ) :: [Ash.Resource.Attribute.t()] def lazy_non_matching_default_attributes(resource, :create) do - Extension.get_persisted(resource, :create_attributes_with_non_matching_lazy_defaults) || [] + Extension.get_persisted(resource, :create_attributes_with_non_matching_lazy_defaults) || + Enum.filter(attributes(resource), fn attribute -> + !attribute.match_other_defaults? && + (is_function(attribute.default) or match?({_, _, _}, attribute.default)) + end) end def lazy_non_matching_default_attributes(resource, :update) do - Extension.get_persisted(resource, :update_attributes_with_non_matching_lazy_defaults) || [] + Extension.get_persisted(resource, :update_attributes_with_non_matching_lazy_defaults) || + Enum.filter(attributes(resource), fn attribute -> + !attribute.match_other_defaults? && + (is_function(attribute.update_default) or match?({_, _, _}, attribute.update_default)) + end) end @doc "Returns all attributes of a resource with static defaults" @@ -605,11 +607,25 @@ defmodule Ash.Resource.Info do type :: :create | :update ) :: [Ash.Resource.Attribute.t()] def static_default_attributes(resource, :create) do - Extension.get_persisted(resource, :create_attributes_with_static_defaults) || [] + Extension.get_persisted(resource, :create_attributes_with_static_defaults) || + resource + |> attributes + |> Enum.filter(fn attribute -> + not is_nil(attribute.default) && + not (is_function(attribute.default) or + match?({_, _, _}, attribute.default)) + end) end def static_default_attributes(resource, :update) do - Extension.get_persisted(resource, :update_attributes_with_static_defaults) || [] + Extension.get_persisted(resource, :update_attributes_with_static_defaults) || + resource + |> attributes + |> Enum.filter(fn attribute -> + not is_nil(attribute.update_default) && + not (is_function(attribute.update_default) or + match?({_, _, _}, attribute.update_default)) + end) end @doc "Returns all attributes of a resource with lazy matching defaults" @@ -618,11 +634,22 @@ defmodule Ash.Resource.Info do type :: :create | :update ) :: [Ash.Resource.Attribute.t()] def lazy_matching_default_attributes(resource, :create) do - Extension.get_persisted(resource, :create_attributes_with_matching_defaults) || [] + Extension.get_persisted(resource, :create_attributes_with_matching_defaults) || + Enum.filter(attributes(resource), fn attribute -> + attribute.match_other_defaults? && + (is_function(attribute.default) or match?({_, _, _}, attribute.default)) + end) end def lazy_matching_default_attributes(resource, :update) do - Extension.get_persisted(resource, :update_attributes_with_matching_defaults) || [] + Extension.get_persisted(resource, :update_attributes_with_matching_defaults) || + resource + |> attributes() + |> Enum.filter(fn attribute -> + not is_nil(attribute.update_default) && + not (is_function(attribute.update_default) or + match?({_, _, _}, attribute.update_default)) + end) end @doc "Get an attribute name from the resource" diff --git a/lib/ash/resource/transformers/attributes_by_name.ex b/lib/ash/resource/transformers/attributes_by_name.ex index f9b746ce..3d384c29 100644 --- a/lib/ash/resource/transformers/attributes_by_name.ex +++ b/lib/ash/resource/transformers/attributes_by_name.ex @@ -25,40 +25,39 @@ defmodule Ash.Resource.Transformers.AttributesByName do create_attributes_with_static_defaults = attributes |> Enum.filter(fn attribute -> - not is_nil(attribute.default) && - not (is_function(attribute.default) or - match?({_, _, _}, attribute.default)) + not is_nil(attribute.default) and + not is_function(attribute.default) and not match?({_, _, _}, attribute.default) end) create_attributes_with_non_matching_lazy_defaults = Enum.filter(attributes, fn attribute -> - !attribute.match_other_defaults? && + !attribute.match_other_defaults? and (is_function(attribute.default) or match?({_, _, _}, attribute.default)) end) create_attributes_with_matching_defaults = Enum.filter(attributes, fn attribute -> - attribute.match_other_defaults? && + attribute.match_other_defaults? and (is_function(attribute.default) or match?({_, _, _}, attribute.default)) end) update_attributes_with_static_defaults = attributes |> Enum.filter(fn attribute -> - not is_nil(attribute.update_default) && - not (is_function(attribute.update_default) or - match?({_, _, _}, attribute.update_default)) + not is_nil(attribute.update_default) and + not is_function(attribute.update_default) and + not match?({_, _, _}, attribute.update_default) end) update_attributes_with_non_matching_lazy_defaults = Enum.filter(attributes, fn attribute -> - !attribute.match_other_defaults? && + !attribute.match_other_defaults? and (is_function(attribute.update_default) or match?({_, _, _}, attribute.update_default)) end) update_attributes_with_matching_defaults = Enum.filter(attributes, fn attribute -> - attribute.match_other_defaults? && + attribute.match_other_defaults? and (is_function(attribute.update_default) or match?({_, _, _}, attribute.update_default)) end) diff --git a/lib/ash/resource/transformers/validations_and_changes_for_type.ex b/lib/ash/resource/transformers/validations_and_changes_for_type.ex index 2d09c22b..8de5aff6 100644 --- a/lib/ash/resource/transformers/validations_and_changes_for_type.ex +++ b/lib/ash/resource/transformers/validations_and_changes_for_type.ex @@ -19,6 +19,7 @@ defmodule Ash.Resource.Transformers.ValidationsAndChangesForType do Map.update(acc, on, [validation], &[validation | &1]) end) end) + |> Map.new(fn {key, value} -> {key, Enum.reverse(value)} end) changes_by_on = dsl_state @@ -30,6 +31,7 @@ defmodule Ash.Resource.Transformers.ValidationsAndChangesForType do Map.update(acc, on, [change], &[change | &1]) end) end) + |> Map.new(fn {key, value} -> {key, Enum.reverse(value)} end) {:ok, dsl_state diff --git a/mix.exs b/mix.exs index b71216ec..c7be6af4 100644 --- a/mix.exs +++ b/mix.exs @@ -302,7 +302,7 @@ defmodule Ash.MixProject do # Run "mix help deps" to learn about dependencies. defp deps do [ - {:spark, "~> 1.1 and >= 1.1.47"}, + {:spark, "~> 1.1 and >= 1.1.50"}, {:ecto, "~> 3.7"}, {:ets, "~> 0.8"}, {:decimal, "~> 2.0"}, diff --git a/mix.lock b/mix.lock index 315d9d1b..61908722 100644 --- a/mix.lock +++ b/mix.lock @@ -31,7 +31,7 @@ "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"}, "sobelow": {:hex, :sobelow, "0.13.0", "218afe9075904793f5c64b8837cc356e493d88fddde126a463839351870b8d1e", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "cd6e9026b85fc35d7529da14f95e85a078d9dd1907a9097b3ba6ac7ebbe34a0d"}, "sourceror": {:hex, :sourceror, "0.14.0", "b6b8552d0240400d66b6f107c1bab7ac1726e998efc797f178b7b517e928e314", [:mix], [], "hexpm", "809c71270ad48092d40bbe251a133e49ae229433ce103f762a2373b7a10a8d8b"}, - "spark": {:hex, :spark, "1.1.47", "2bc334e542f519709e57853a425aa9304223c61e3ecf130b1cc014c6a4451f9a", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "c1ae778a3779b5d3e5b7c885cc9826577816dca10bbf4c21638198a1262f3df1"}, + "spark": {:hex, :spark, "1.1.50", "809da1214151ad7c592389b3ea85eb4424f727b681b90439d8ffbe5305400ce9", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.5 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:sourceror, "~> 0.1", [hex: :sourceror, repo: "hexpm", optional: false]}], "hexpm", "ed9b1b817b52c3aaeee283032640857ee9d8398b8c4e9e7d78d77929d387b9a1"}, "statistex": {:hex, :statistex, "1.0.0", "f3dc93f3c0c6c92e5f291704cf62b99b553253d7969e9a5fa713e5481cd858a5", [:mix], [], "hexpm", "ff9d8bee7035028ab4742ff52fc80a2aa35cece833cf5319009b52f1b5a86c27"}, "stream_data": {:hex, :stream_data, "0.6.0", "e87a9a79d7ec23d10ff83eb025141ef4915eeb09d4491f79e52f2562b73e5f47", [:mix], [], "hexpm", "b92b5031b650ca480ced047578f1d57ea6dd563f5b57464ad274718c9c29501c"}, "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},