From 81c9475e2e0eaf2b65efb39d602ab7953b60fbd5 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 13 Apr 2023 01:41:19 -0400 Subject: [PATCH] chore: fix `return_notifications?` behavior --- lib/ash/actions/create.ex | 2 + lib/ash/actions/destroy.ex | 2 + lib/ash/actions/update.ex | 2 + lib/ash/changeset/changeset.ex | 89 +++++++++++++++++++++++++--------- lib/ash/engine/engine.ex | 48 +++++++----------- 5 files changed, 90 insertions(+), 53 deletions(-) diff --git a/lib/ash/actions/create.ex b/lib/ash/actions/create.ex index cb3cc07c..c3dabb44 100644 --- a/lib/ash/actions/create.ex +++ b/lib/ash/actions/create.ex @@ -89,6 +89,7 @@ defmodule Ash.Actions.Create do timeout: opts[:timeout], upsert_identity: upsert_identity, upsert_keys: upsert_keys, + return_notifications?: opts[:return_notifications?], authorize?: authorize?, actor: actor, tenant: opts[:tenant], @@ -459,6 +460,7 @@ defmodule Ash.Actions.Create do Keyword.get(request_opts, :transaction?, true) && action.transaction?, timeout: request_opts[:timeout], tracer: request_opts[:tracer], + return_notifications?: request_opts[:return_notifications?], transaction_metadata: %{ type: :create, metadata: %{ diff --git a/lib/ash/actions/destroy.ex b/lib/ash/actions/destroy.ex index 9680b6a8..e1333ae7 100644 --- a/lib/ash/actions/destroy.ex +++ b/lib/ash/actions/destroy.ex @@ -94,6 +94,7 @@ defmodule Ash.Actions.Destroy do authorize?: authorize?, actor: actor, timeout: opts[:timeout] || changeset.timeout || Ash.Api.Info.timeout(api), + return_notifications?: opts[:return_notifications?], tracer: opts[:tracer], timeout: opts[:timeout], tenant: opts[:tenant] @@ -327,6 +328,7 @@ defmodule Ash.Actions.Destroy do transaction?: Keyword.get(request_opts, :transaction?, true) && action.transaction?, timeout: request_opts[:timeout], + return_notifications?: request_opts[:return_notifications?], transaction_metadata: %{ type: :destroy, metadata: %{ diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index 498fd93f..fc246f37 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -72,6 +72,7 @@ defmodule Ash.Actions.Update do |> as_requests(resource, api, action, changeset: changeset, authorize?: authorize?, + return_notifications?: opts[:return_notifications?], actor: actor, timeout: opts[:timeout] || changeset.timeout || Ash.Api.Info.timeout(api), tracer: opts[:tracer], @@ -460,6 +461,7 @@ defmodule Ash.Actions.Update do transaction?: Keyword.get(request_opts, :transaction?, true) && action.transaction?, timeout: request_opts[:timeout], + return_notifications?: request_opts[:return_notifications?], transaction_metadata: %{ type: :update, metadata: %{ diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index 12646caa..23ed1b10 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -1563,32 +1563,51 @@ defmodule Ash.Changeset do |> Enum.concat(changeset.action.touches_resources) |> Enum.uniq() - Process.put( - :ash_after_transaction, - Process.get(:ash_after_transaction, []) ++ changeset.after_transaction - ) + notify? = + if Process.get(:ash_started_transaction?) do + false + else + Process.put(:ash_started_transaction?, true) + true + end - resources - |> Enum.reject(&Ash.DataLayer.in_transaction?/1) - |> Ash.DataLayer.transaction( - fn -> - case run_around_actions(changeset, func) do - {:error, error} -> - Ash.DataLayer.rollback(changeset.resource, error) + try do + resources + |> Enum.reject(&Ash.DataLayer.in_transaction?/1) + |> Ash.DataLayer.transaction( + fn -> + case run_around_actions(changeset, func) do + {:error, error} -> + Ash.DataLayer.rollback(changeset.resource, error) - other -> - other - end - end, - changeset.timeout || :infinity, - opts[:transaction_metadata] - ) - |> case do - {:ok, result} -> - result + other -> + other + end + end, + changeset.timeout || :infinity, + opts[:transaction_metadata] + ) + |> case do + {:ok, {:ok, value, changeset, instructions}} -> + notifications = + if notify? && !opts[:return_notifications?] do + Enum.concat( + instructions[:notifications] || [], + Process.delete(:ash_notifications) || [] + ) + else + instructions[:notifications] || [] + end - {:error, error} -> - {:error, error} + {:ok, value, changeset, Map.put(instructions, :notifications, notifications)} + + {:error, error} -> + {:error, error} + end + after + if notify? do + Process.delete(:ash_started_transaction?) + end end end) else @@ -1608,6 +1627,30 @@ defmodule Ash.Changeset do end end) end + |> case do + {:ok, value, changeset, instructions} -> + if opts[:return_notifications?] do + {:ok, value, changeset, instructions} + else + if Process.get(:ash_started_transaction?) do + current_notifications = Process.get(:ash_notifications, []) + + Process.put( + :ash_notifications, + current_notifications ++ (instructions[:notifications] || []) + ) + + {:ok, value, changeset, Map.put(instructions, :notifications, [])} + else + notifications = Ash.Notifier.notify(instructions[:notifications] || []) + + {:ok, value, changeset, Map.put(instructions, :notifications, notifications)} + end + end + + other -> + other + end end defp warn_on_transaction_hooks(_, [], _), do: :ok diff --git a/lib/ash/engine/engine.ex b/lib/ash/engine/engine.ex index c8030ed9..403ea1ab 100644 --- a/lib/ash/engine/engine.ex +++ b/lib/ash/engine/engine.ex @@ -100,10 +100,10 @@ defmodule Ash.Engine do resources -> notify? = - if Process.get(:ash_engine_started_transaction?) do + if Process.get(:ash_started_transaction?) do false else - Process.put(:ash_engine_started_transaction?, true) + Process.put(:ash_started_transaction?, true) true end @@ -124,20 +124,14 @@ defmodule Ash.Engine do ) |> case do {:ok, result} -> - saved_notifications = Process.delete(:ash_engine_notifications) || [] + if notify? do + saved_notifications = Process.delete(:ash_notifications) || [] - if opts[:return_notifications?] do - {:ok, - %{ - result - | resource_notifications: - result.resource_notifications ++ saved_notifications - }} - else - remaining_notifications = - Ash.Notifier.notify(result.resource_notifications ++ saved_notifications) + remaining_notifications = result.resource_notifications ++ saved_notifications {:ok, %{result | resource_notifications: remaining_notifications}} + else + {:ok, result} end {:error, error} -> @@ -145,7 +139,7 @@ defmodule Ash.Engine do end after if notify? do - Process.delete(:ash_engine_started_transaction?) + Process.delete(:ash_started_transaction?) end end end @@ -164,23 +158,17 @@ defmodule Ash.Engine do |> case do {:ok, %{resource_notifications: resource_notifications} = result} -> notifications = - if opts[:return_notifications?] do - resource_notifications + if Process.get(:ash_started_transaction?) && !opts[:return_notifications?] do + current_notifications = Process.get(:ash_notifications, []) + + Process.put( + :ash_notifications, + current_notifications ++ resource_notifications + ) + + [] else - notifications = Ash.Notifier.notify(resource_notifications) - - if Process.get(:ash_engine_started_transaction?) do - current_notifications = Process.get(:ash_engine_notifications, []) - - Process.put( - :ash_engine_notifications, - current_notifications ++ notifications - ) - - [] - else - notifications - end + resource_notifications end {:ok, %{result | resource_notifications: notifications}}