diff --git a/lib/ash/actions/create.ex b/lib/ash/actions/create.ex index 693a8be6..c8a83c0b 100644 --- a/lib/ash/actions/create.ex +++ b/lib/ash/actions/create.ex @@ -347,6 +347,11 @@ defmodule Ash.Actions.Create do authorize?: authorize?, api: changeset.api }) + |> manage_relationships(api, changeset, + actor: actor, + authorize?: authorize?, + upsert?: upsert? + ) action.manual? -> {:ok, nil} @@ -485,6 +490,21 @@ defmodule Ash.Actions.Create do {:ok, nil, %{notifications: []}} end + defp manage_relationships( + {:ok, created, %{notifications: notifications}}, + api, + changeset, + engine_opts + ) do + case manage_relationships({:ok, created}, api, changeset, engine_opts) do + {:ok, created, info} -> + {:ok, created, Map.update(info, :notifications, notifications, &(&1 ++ notifications))} + + other -> + other + end + end + defp manage_relationships({:ok, created}, api, changeset, engine_opts) do with {:ok, loaded} <- Ash.Actions.ManagedRelationships.load(api, created, changeset, engine_opts), diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index 7e489f4e..0517d98f 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -1206,9 +1206,6 @@ defmodule Ash.Actions.Read do {query, notifications} end end) - |> then(fn {query, notifications} -> - {Ash.Query.put_context(query, :private, %{in_before_action?: false}), notifications} - end) end defp run_after_action(query, results) do diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index 480780af..b3642790 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -375,6 +375,10 @@ defmodule Ash.Actions.Update do authorize?: authorize?, api: changeset.api }) + |> manage_relationships(api, changeset, + actor: actor, + authorize?: authorize? + ) action.manual? -> {:ok, changeset.data, %{notifications: []}} @@ -462,6 +466,21 @@ defmodule Ash.Actions.Update do [authorization_request, commit_request] end + defp manage_relationships( + {:ok, updated, %{notifications: notifications}}, + api, + changeset, + engine_opts + ) do + case manage_relationships({:ok, updated}, api, changeset, engine_opts) do + {:ok, updated, info} -> + {:ok, updated, Map.update(info, :notifications, notifications, &(&1 ++ notifications))} + + other -> + other + end + end + defp manage_relationships({:ok, updated}, api, changeset, engine_opts) do with {:ok, loaded} <- Ash.Actions.ManagedRelationships.load(api, updated, changeset, engine_opts), diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index d931b127..f1bde824 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -150,9 +150,11 @@ defmodule Ash.Changeset do changeset = unquote(changeset) if changeset.__validated_for_action__ && !changeset.context[:private][:in_before_action?] do - raise ArgumentError, """ + IO.warn(""" Changeset has already been validated for action #{inspect(changeset.__validated_for_action__)}. + In the future, this will become an error. + For safety, we prevent any changes after that point because they will bypass validations or other action logic.. To proceed anyway, you can use `#{unquote(alternative)}/#{unquote(arity)}`. However, you should prefer a pattern like the below, which makes any custom changes *before* calling the action. @@ -161,7 +163,7 @@ defmodule Ash.Changeset do |> Ash.Changeset.new() |> Ash.Changeset.#{unquote(function)}(...) |> Ash.Changeset.for_create(...) - """ + """) end end else @@ -169,9 +171,11 @@ defmodule Ash.Changeset do changeset = unquote(changeset) if changeset.__validated_for_action__ && !changeset.context[:private][:in_before_action?] do - raise ArgumentError, """ + IO.warn(""" Changeset has already been validated for action #{inspect(changeset.__validated_for_action__)}. + In the future, this will become an error. + For safety, we prevent any changes using `#{unquote(function)}/#{unquote(arity)}` after that point because they will bypass validations or other action logic. Instead, you should change or set this value before calling the action, like so: @@ -179,7 +183,7 @@ defmodule Ash.Changeset do |> Ash.Changeset.new() |> Ash.Changeset.#{unquote(function)}(...) |> Ash.Changeset.for_create(...) - """ + """) end end end @@ -1544,8 +1548,6 @@ defmodule Ash.Changeset do end ) - changeset = put_context(changeset, :private, %{in_before_action?: false}) - case func.(changeset) do {:ok, result, instructions} -> run_after_actions( diff --git a/lib/ash/flow/executor/ash_engine.ex b/lib/ash/flow/executor/ash_engine.ex index 434054bd..17ca366c 100644 --- a/lib/ash/flow/executor/ash_engine.ex +++ b/lib/ash/flow/executor/ash_engine.ex @@ -410,7 +410,7 @@ defmodule Ash.Flow.Executor.AshEngine do [first | rest] -> # only one of the requests needs to be annotated as touching the resources - # the transaction claims to touch, since the transaction is urn over all touched resources + # the transaction claims to touch, since the transaction is run over all touched resources # in all requests [ %{ diff --git a/lib/ash/policy/authorizer.ex b/lib/ash/policy/authorizer.ex index 25318ef8..11d52ee0 100644 --- a/lib/ash/policy/authorizer.ex +++ b/lib/ash/policy/authorizer.ex @@ -494,7 +494,17 @@ defmodule Ash.Policy.Authorizer do scenario |> Enum.map(fn {{check_module, check_opts}, true} -> - result = check_module.auto_filter(authorizer.actor, authorizer, check_opts) + result = + try do + check_module.auto_filter(authorizer.actor, authorizer, check_opts) + rescue + e -> + reraise Ash.Error.to_ash_error(e, __STACKTRACE__, + error_context: + "Creating filter for check: #{check_module.describe(check_opts)} on resource: #{authorizer.resource}" + ), + __STACKTRACE__ + end if is_nil(result) do false @@ -504,10 +514,19 @@ defmodule Ash.Policy.Authorizer do {{check_module, check_opts}, false} -> result = - if :erlang.function_exported(check_module, :auto_filter_not, 3) do - check_module.auto_filter_not(authorizer.actor, authorizer, check_opts) - else - [not: check_module.auto_filter(authorizer.actor, authorizer, check_opts)] + try do + if :erlang.function_exported(check_module, :auto_filter_not, 3) do + check_module.auto_filter_not(authorizer.actor, authorizer, check_opts) + else + [not: check_module.auto_filter(authorizer.actor, authorizer, check_opts)] + end + rescue + e -> + reraise Ash.Error.to_ash_error(e, __STACKTRACE__, + error_context: + "Creating filter for check: #{check_module.describe(check_opts)} on resource: #{authorizer.resource}" + ), + __STACKTRACE__ end if is_nil(result) do diff --git a/lib/ash/policy/check/relates_to_actor_via.ex b/lib/ash/policy/check/relates_to_actor_via.ex index 18fdf275..d014cccd 100644 --- a/lib/ash/policy/check/relates_to_actor_via.ex +++ b/lib/ash/policy/check/relates_to_actor_via.ex @@ -62,11 +62,21 @@ defmodule Ash.Policy.Check.RelatesToActorVia do defp relationship_info(resource, [rel], to_many?) do rel = Ash.Resource.Info.relationship(resource, rel) + + if !rel do + raise "No such relationship #{rel} for #{resource}, required in `relates_to_actor` check" + end + {rel, to_many? || rel.cardinality == :many} end defp relationship_info(resource, [rel | rest], to_many?) do rel = Ash.Resource.Info.relationship(resource, rel) + + if !rel do + raise "No such relationship #{rel} for #{resource}, required in `relates_to_actor` check" + end + relationship_info(rel.destination, rest, to_many? || rel.cardinality == :many) end end diff --git a/lib/ash/policy/checker.ex b/lib/ash/policy/checker.ex index 76b60526..b201c0d5 100644 --- a/lib/ash/policy/checker.ex +++ b/lib/ash/policy/checker.ex @@ -43,18 +43,27 @@ defmodule Ash.Policy.Checker do check_module = check.check_module opts = check.check_opts - case check_module.strict_check(authorizer.actor, authorizer, opts) do - {:ok, boolean} when is_boolean(boolean) -> - {:ok, authorizer, Map.put(facts, {check_module, opts}, boolean)} + try do + case check_module.strict_check(authorizer.actor, authorizer, opts) do + {:ok, boolean} when is_boolean(boolean) -> + {:ok, authorizer, Map.put(facts, {check_module, opts}, boolean)} - {:ok, :unknown} -> - {:ok, authorizer, facts} + {:ok, :unknown} -> + {:ok, authorizer, facts} - {:error, error} -> - {:error, error} + {:error, error} -> + {:error, error} - other -> - raise "Invalid return value from strict_check call #{check_module}.strict_check(actor, authorizer, #{inspect(opts)}) - #{inspect(other)}" + other -> + raise "Invalid return value from strict_check call #{check_module}.strict_check(actor, authorizer, #{inspect(opts)}) - #{inspect(other)}" + end + rescue + e -> + reraise Ash.Error.to_ash_error(e, __STACKTRACE__, + error_context: + "Strict checking: #{check_module.describe(opts)} on resource: #{authorizer.resource}" + ), + __STACKTRACE__ end end diff --git a/lib/ash/policy/filter_check_with_context.ex b/lib/ash/policy/filter_check_with_context.ex index 3b10fb43..3ed09ace 100644 --- a/lib/ash/policy/filter_check_with_context.ex +++ b/lib/ash/policy/filter_check_with_context.ex @@ -30,6 +30,8 @@ defmodule Ash.Policy.FilterCheckWithContext do end def strict_check(nil, authorizer, opts) do + opts = Keyword.put_new(opts, :resource, authorizer.resource) + if Ash.Filter.template_references_actor?(filter(nil, authorizer, opts)) do {:ok, false} else @@ -153,6 +155,7 @@ defmodule Ash.Policy.FilterCheckWithContext do end def reject(actor, authorizer, opts) do + opts = Keyword.put_new(opts, :resource, authorizer.resource) [not: filter(actor, authorizer, opts)] end diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index be96b6a2..44c1aaaf 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -149,8 +149,8 @@ defmodule Ash.Query do query = unquote(query) if query.__validated_for_action__ && !query.context[:private][:in_before_action?] do - raise ArgumentError, """ - Changeset has already been validated for action #{inspect(query.__validated_for_action__)}. + IO.warn(""" + Query has already been validated for action #{inspect(query.__validated_for_action__)}. For safety, we prevent any changes after that point because they will bypass validations or other action logic. However, you should prefer a pattern like the below, which makes any custom modifications *before* calling the action. @@ -159,7 +159,7 @@ defmodule Ash.Query do |> Ash.Query.new() |> Ash.Query.#{unquote(function)}(...) |> Ash.Query.for_read(...) - """ + """) end end end diff --git a/lib/ash/resource/manual_actions/manual_create.ex b/lib/ash/resource/manual_actions/manual_create.ex index c0edfa9e..c71365ac 100644 --- a/lib/ash/resource/manual_actions/manual_create.ex +++ b/lib/ash/resource/manual_actions/manual_create.ex @@ -15,7 +15,9 @@ defmodule Ash.Resource.ManualCreate do opts :: Keyword.t(), context :: context() ) :: - {:ok, Ash.Resource.record()} | {:error, term} + {:ok, Ash.Resource.record()} + | {:ok, Ash.Resource.record(), %{notifications: [Ash.Notifier.Notification.t()]}} + | {:error, term} defmacro __using__(_) do quote do diff --git a/lib/ash/resource/manual_actions/manual_update.ex b/lib/ash/resource/manual_actions/manual_update.ex index a6775594..6ec7f588 100644 --- a/lib/ash/resource/manual_actions/manual_update.ex +++ b/lib/ash/resource/manual_actions/manual_update.ex @@ -15,7 +15,9 @@ defmodule Ash.Resource.ManualUpdate do opts :: Keyword.t(), context :: context() ) :: - {:ok, Ash.Resource.record()} | {:error, term} + {:ok, Ash.Resource.record()} + | {:ok, Ash.Resource.record(), %{notifications: [Ash.Notifier.Notification.t()]}} + | {:error, term} defmacro __using__(_) do quote do