improvement: validate accepted and rejected attributes in actions (#395)

lib/ash/resource/dsl.ex

@@ -1212,7 +1212,8 @@ defmodule Ash.Resource.Dsl do
     Ash.Resource.Transformers.RequireUniqueFieldNames,
     Ash.Resource.Transformers.ValidateRelationshipAttributes,
     Ash.Resource.Transformers.ValidateEagerIdentities,
-    Ash.Resource.Transformers.ValidateAggregatesSupported
+    Ash.Resource.Transformers.ValidateAggregatesSupported,
+    Ash.Resource.Transformers.ValidateAccept
   ]
 
   @moduledoc """

lib/ash/resource/transformers/default_accept.ex

@@ -9,6 +9,7 @@ defmodule Ash.Resource.Transformers.DefaultAccept do
     public_attribute_names =
       dsl_state
       |> Transformer.get_entities([:attributes])
+      |> Enum.reject(& &1.private?)
       |> Enum.map(& &1.name)
 
     default_accept =
@@ -22,19 +23,19 @@ defmodule Ash.Resource.Transformers.DefaultAccept do
     |> Transformer.get_entities([:actions])
     |> Enum.reject(&(&1.type == :read))
     |> Enum.reduce({:ok, dsl_state}, fn action, {:ok, dsl_state} ->
-      accept =
+      {accept, reject} =
         case {action.accept, action.reject} do
-          {_, :all} -> []
-          {nil, reject} -> reject(default_accept, reject)
-          {:all, reject} -> reject(public_attribute_names, reject)
-          {accept, reject} -> reject(accept, reject)
+          {_, :all} -> {[], public_attribute_names}
+          {nil, reject} -> {reject(default_accept, reject), reject}
+          {:all, reject} -> {reject(public_attribute_names, reject), reject}
+          {accept, reject} -> {reject(accept, reject), reject}
         end
 
       new_dsl_state =
         Transformer.replace_entity(
           dsl_state,
           [:actions],
-          %{action | accept: accept},
+          %{action | accept: accept, reject: reject},
           &(&1.name == action.name && &1.type == action.type)
         )
 

lib/ash/resource/transformers/validate_accept.ex

@@ -0,0 +1,64 @@
+defmodule Ash.Resource.Transformers.ValidateAccept do
+  @moduledoc "Validates that accept and reject lists only contain valid attributes"
+  use Spark.Dsl.Transformer
+
+  alias Spark.Dsl.Transformer
+  alias Spark.Error.DslError
+
+  @impl true
+  def after_compile?, do: true
+
+  @impl true
+  def transform(dsl_state) do
+    {private_attributes, public_attributes} =
+      dsl_state
+      |> Transformer.get_entities([:attributes])
+      |> Enum.split_with(& &1.private?)
+
+    public_attribute_names = MapSet.new(public_attributes, & &1.name)
+    private_attribute_names = MapSet.new(private_attributes, & &1.name)
+
+    Transformer.get_entities(dsl_state, [:actions])
+    |> Enum.each(fn
+      %{name: action_name, accept: accept, reject: reject} ->
+        validate_attribute_name = fn attribute_name, type ->
+          cond do
+            MapSet.member?(private_attribute_names, attribute_name) ->
+              raise DslError,
+                path: [:actions, action_name, type, attribute_name],
+                message: "#{attribute_name} is a private attribute"
+
+            MapSet.member?(public_attribute_names, attribute_name) ->
+              :ok
+
+            true ->
+              raise DslError,
+                path: [:actions, action_name, type, attribute_name],
+                message: "#{attribute_name} is not an attribute"
+          end
+        end
+
+        Enum.each(
+          accept,
+          &validate_attribute_name.(
+            &1,
+            :accept
+          )
+        )
+
+        Enum.each(
+          reject,
+          &validate_attribute_name.(
+            &1,
+            :reject
+          )
+        )
+
+      # read types do not have accept / reject fields
+      %{type: :read} ->
+        :ok
+    end)
+
+    :ok
+  end
+end

test/resource/validate_accept_test.exs

@@ -0,0 +1,55 @@
+defmodule Ash.Test.Resource.ValidateAcceptTest do
+  @moduledoc false
+  use ExUnit.Case, async: true
+
+  import Ash.Test.Helpers
+  alias Spark.Error.DslError
+
+  test "Accepting an attribute that does not exist raises an error" do
+    assert_raise DslError, ~r/invalid is not an attribute/, fn ->
+      defposts do
+        actions do
+          create :example_action, accept: [:invalid]
+        end
+      end
+    end
+  end
+
+  test "Accepting an attribute that is private raises an error" do
+    assert_raise DslError, ~r/secret is a private attribute/, fn ->
+      defposts do
+        attributes do
+          attribute :secret, :string, private?: true
+        end
+
+        actions do
+          create :example_action, accept: [:secret]
+        end
+      end
+    end
+  end
+
+  test "Rejecting an attribute that does not exist raises an error" do
+    assert_raise DslError, ~r/invalid is not an attribute/, fn ->
+      defposts do
+        actions do
+          create :example_action, reject: [:invalid]
+        end
+      end
+    end
+  end
+
+  test "Rejecting an attribute that is private raises an error" do
+    assert_raise DslError, ~r/secret is a private attribute/, fn ->
+      defposts do
+        attributes do
+          attribute :secret, :string, private?: true
+        end
+
+        actions do
+          create :example_action, reject: [:secret]
+        end
+      end
+    end
+  end
+end