fix: detect transaction hooks added by around_transaction and before_…

…transaction
ash-project · Oct 31, 2024 · e699525 · e699525
1 parent b78377a
commit e699525
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 38 deletions.
diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex
@@ -3432,10 +3432,15 @@ defmodule Ash.Changeset do
   end
 
   def with_hooks(changeset, func, opts) do
+    data_layer_prefers_transaction? = Ash.DataLayer.prefer_transaction?(changeset.resource)
+    # We check if *all* hooks that *could* add transaction hooks are empty
+    # Later before starting the transaction we do the same but only checking
+    # the actual hooks
     prefer_transaction? =
-      if Enum.empty?(changeset.before_action) && Enum.empty?(changeset.after_action) &&
+      if Enum.empty?(changeset.before_transaction) && Enum.empty?(changeset.around_transaction) &&
+           Enum.empty?(changeset.before_action) && Enum.empty?(changeset.after_action) &&
            Enum.empty?(changeset.around_action) do
-        Ash.DataLayer.prefer_transaction?(changeset.resource)
+        data_layer_prefers_transaction?
       else
         true
       end
@@ -3459,46 +3464,55 @@ defmodule Ash.Changeset do
 
         resources = Enum.reject(resources, &Ash.DataLayer.in_transaction?/1)
 
-        try do
-          resources
-          |> Ash.DataLayer.transaction(
-            fn ->
-              case run_around_actions(changeset, func) do
-                {:error, error} ->
-                  if opts[:rollback_on_error?] do
-                    Ash.DataLayer.rollback(
-                      changeset.resource,
-                      error
-                    )
-                  else
-                    {:error, error}
-                  end
+        do_transaction? =
+          data_layer_prefers_transaction? ||
+            !(Enum.empty?(changeset.before_action) && Enum.empty?(changeset.after_action) &&
+                Enum.empty?(changeset.around_action))
 
-                other ->
-                  other
-              end
-            end,
-            changeset.timeout || :infinity,
-            Map.put(
-              opts[:transaction_metadata],
-              :data_layer_context,
-              changeset.context[:data_layer] || %{}
+        if do_transaction? do
+          try do
+            resources
+            |> Ash.DataLayer.transaction(
+              fn ->
+                case run_around_actions(changeset, func) do
+                  {:error, error} ->
+                    if opts[:rollback_on_error?] do
+                      Ash.DataLayer.rollback(
+                        changeset.resource,
+                        error
+                      )
+                    else
+                      {:error, error}
+                    end
+
+                  other ->
+                    other
+                end
+              end,
+              changeset.timeout || :infinity,
+              Map.put(
+                opts[:transaction_metadata],
+                :data_layer_context,
+                changeset.context[:data_layer] || %{}
+              )
             )
-          )
-          |> case do
-            {:ok, {:ok, value, changeset, instructions}} ->
-              {:ok, value, changeset, Map.put(instructions, :gather_notifications?, notify?)}
+            |> case do
+              {:ok, {:ok, value, changeset, instructions}} ->
+                {:ok, value, changeset, Map.put(instructions, :gather_notifications?, notify?)}
 
-            {:ok, {:error, error}} ->
-              {:error, error}
+              {:ok, {:error, error}} ->
+                {:error, error}
 
-            {:error, error} ->
-              {:error, error}
-          end
-        after
-          if notify? do
-            Process.delete(:ash_started_transaction?)
+              {:error, error} ->
+                {:error, error}
+            end
+          after
+            if notify? do
+              Process.delete(:ash_started_transaction?)
+            end
           end
+        else
+          run_around_actions(changeset, func)
         end
       end)
     else

diff --git a/lib/ash/resource/calculation/load_relationship.ex b/lib/ash/resource/calculation/load_relationship.ex
@@ -34,7 +34,8 @@ defmodule Ash.Resource.Calculation.LoadRelationship do
       raise "Must provide the `domain` option to load #{inspect(__MODULE__)}"
     end
 
-    load_opts = Ash.Context.to_opts(context, Keyword.put(opts[:opts] || [], :domain, opts[:domain]))
+    load_opts =
+      Ash.Context.to_opts(context, Keyword.put(opts[:opts] || [], :domain, opts[:domain]))
 
     Ash.load(results, [{relationship.name, query}], load_opts)
     |> case do