improvement: Replace incorrect function_exported?-checks in bulk-acti…

…ons, add has-defs for change modules (#1330) fix: don't refer to non-existent `batch_change/4`
ash-project · Jul 22, 2024 · c2ac47e · c2ac47e
1 parent 0701843
commit c2ac47e
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 32 deletions.
diff --git a/lib/ash/actions/create/bulk.ex b/lib/ash/actions/create/bulk.ex
@@ -761,8 +761,8 @@ defmodule Ash.Actions.Create.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, _opts}}, _} ->
-        function_exported?(module, :batch_change, 3) &&
-          function_exported?(module, :before_batch, 3)
+        module.has_batch_change?() &&
+          module.has_before_batch?()
 
       _ ->
         false
@@ -1366,8 +1366,8 @@ defmodule Ash.Actions.Create.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
-        function_exported?(module, :batch_change, 3) &&
-          function_exported?(module, :after_batch, 3) &&
+        module.has_batch_change?() &&
+          module.has_after_batch?() &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -1556,8 +1556,8 @@ defmodule Ash.Actions.Create.Bulk do
                 Enum.any?(batch, fn item ->
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
-                (function_exported?(module, :batch_change, 3) &&
-                   function_exported?(module, :after_batch, 3) &&
+                (module.has_batch_change?() &&
+                   module.has_after_batch?() &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -1608,8 +1608,8 @@ defmodule Ash.Actions.Create.Bulk do
                   Enum.any?(batch, fn item ->
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
-                  (function_exported?(module, :batch_change, 3) &&
-                     function_exported?(module, :after_batch, 3) &&
+                  (module.has_batch_change?() &&
+                     module.has_after_batch?() &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{
@@ -1632,12 +1632,11 @@ defmodule Ash.Actions.Create.Bulk do
   defp batch_change(module, batch, change_opts, context, actor) do
     case change_opts do
       {:templated, change_opts} ->
-        if function_exported?(module, :batch_change, 4) do
+        if module.has_batch_change?() do
           module.batch_change(
             batch,
             change_opts,
-            struct(struct(Ash.Resource.Change.Context, context), bulk?: true),
-            actor
+            struct(struct(Ash.Resource.Change.Context, context), bulk?: true)
           )
         else
           Enum.map(batch, fn changeset ->

diff --git a/lib/ash/actions/destroy/bulk.ex b/lib/ash/actions/destroy/bulk.ex
@@ -256,7 +256,7 @@ defmodule Ash.Actions.Destroy.Bulk do
               action.changes ++ Ash.Resource.Info.changes(atomic_changeset.resource, :destroy),
               fn
                 %{change: {module, change_opts}} ->
-                  function_exported?(module, :after_batch, 3) &&
+                  module.has_after_batch?() &&
                     module.batch_callbacks?(query, change_opts, context)
 
                 _ ->
@@ -1599,7 +1599,7 @@ defmodule Ash.Actions.Destroy.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, _opts}}, _} ->
-        function_exported?(module, :before_batch, 3)
+        module.has_before_batch?()
 
       _ ->
         false
@@ -2109,8 +2109,8 @@ defmodule Ash.Actions.Destroy.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
-        function_exported?(module, :after_batch, 3) &&
-          function_exported?(module, :batch_change, 3) &&
+        module.has_after_batch?() &&
+          module.has_batch_change?() &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -2277,8 +2277,8 @@ defmodule Ash.Actions.Destroy.Bulk do
                 Enum.any?(batch, fn item ->
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
-                (function_exported?(module, :after_batch, 3) &&
-                   function_exported?(module, :batch_change, 3) &&
+                (module.has_after_batch?() &&
+                   module.has_batch_change?() &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -2328,8 +2328,8 @@ defmodule Ash.Actions.Destroy.Bulk do
                   Enum.any?(batch, fn item ->
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
-                  (function_exported?(module, :after_batch, 3) &&
-                     function_exported?(module, :batch_change, 3) &&
+                  (module.has_after_batch?() &&
+                     module.has_batch_change?() &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{
@@ -2352,12 +2352,11 @@ defmodule Ash.Actions.Destroy.Bulk do
   defp batch_change(module, batch, change_opts, context, actor) do
     case change_opts do
       {:templated, change_opts} ->
-        if function_exported?(module, :batch_change, 4) do
+        if module.has_batch_change?() do
           module.batch_change(
             batch,
             change_opts,
-            struct(struct(Ash.Resource.Change.Context, context), bulk?: true),
-            actor
+            struct(struct(Ash.Resource.Change.Context, context), bulk?: true)
           )
         else
           Enum.map(batch, fn changeset ->

diff --git a/lib/ash/actions/update/bulk.ex b/lib/ash/actions/update/bulk.ex
@@ -210,7 +210,7 @@ defmodule Ash.Actions.Update.Bulk do
                 Ash.Resource.Info.changes(atomic_changeset.resource, atomic_changeset.action_type),
               fn
                 %{change: {module, change_opts}} ->
-                  function_exported?(module, :after_batch, 3) &&
+                  module.has_after_batch?() &&
                     module.batch_callbacks?(query, change_opts, context)
 
                 _ ->
@@ -857,7 +857,7 @@ defmodule Ash.Actions.Update.Bulk do
       |> Stream.with_index()
       |> Enum.filter(fn
         {%{change: {module, change_opts}}, _index} ->
-          function_exported?(module, :after_batch, 3) &&
+          module.has_after_batch?() &&
             module.batch_callbacks?(query, change_opts, context)
 
         _ ->
@@ -1821,7 +1821,7 @@ defmodule Ash.Actions.Update.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, _opts}}, _} ->
-        function_exported?(module, :before_batch, 3)
+        module.has_before_batch?()
 
       _ ->
         false
@@ -2400,8 +2400,8 @@ defmodule Ash.Actions.Update.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
-        function_exported?(module, :after_batch, 3) &&
-          function_exported?(module, :batch_change, 3) &&
+        module.has_after_batch?() &&
+          module.has_batch_change?() &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -2580,8 +2580,8 @@ defmodule Ash.Actions.Update.Bulk do
                 Enum.any?(batch, fn item ->
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
-                (function_exported?(module, :after_batch, 3) &&
-                   function_exported?(module, :batch_change, 3) &&
+                (module.has_after_batch?() &&
+                   module.has_batch_change?() &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -2631,8 +2631,8 @@ defmodule Ash.Actions.Update.Bulk do
                   Enum.any?(batch, fn item ->
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
-                  (function_exported?(module, :after_batch, 3) &&
-                     function_exported?(module, :batch_change, 3) &&
+                  (module.has_after_batch?() &&
+                     module.has_batch_change?() &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{
@@ -2655,7 +2655,7 @@ defmodule Ash.Actions.Update.Bulk do
   defp batch_change(module, batch, change_opts, context, actor) do
     case change_opts do
       {:templated, change_opts} ->
-        if function_exported?(module, :batch_change, 3) do
+        if module.has_batch_change?() do
           {:ok, change_opts} = module.init(change_opts)
           module.batch_change(batch, change_opts, context)
         else

diff --git a/lib/ash/resource/change/change.ex b/lib/ash/resource/change/change.ex
@@ -169,6 +169,10 @@ defmodule Ash.Resource.Change do
 
   @callback has_change?() :: boolean
 
+  @callback has_batch_change?() :: boolean
+  @callback has_after_batch?() :: boolean
+  @callback has_before_batch?() :: boolean
+
   @optional_callbacks before_batch: 3,
                       after_batch: 3,
                       batch_change: 3,
@@ -203,6 +207,30 @@ defmodule Ash.Resource.Change do
         def has_change?, do: false
       end
 
+      if Module.defines?(__MODULE__, {:batch_change, 3}, :def) do
+        @impl true
+        def has_batch_change?, do: true
+      else
+        @impl true
+        def has_batch_change?, do: false
+      end
+
+      if Module.defines?(__MODULE__, {:before_batch, 3}, :def) do
+        @impl true
+        def has_before_batch?, do: true
+      else
+        @impl true
+        def has_before_batch?, do: false
+      end
+
+      if Module.defines?(__MODULE__, {:after_batch, 3}, :def) do
+        @impl true
+        def has_after_batch?, do: true
+      else
+        @impl true
+        def has_after_batch?, do: false
+      end
+
       if Module.defines?(__MODULE__, {:atomic, 3}, :def) do
         unless Module.defines?(__MODULE__, {:atomic?, 0}, :def) do
           @impl true