improvement: Add detailed parameter checking for Api read functions (#229)

lib/ash/api/interface.ex

@@ -306,14 +306,19 @@ defmodule Ash.Api.Interface do
       end
 
       # @spec get!(Ash.Resource.t(), term, Keyword.t()) :: Ash.Resource.record() | no_return
-      def get!(resource, id, params \\ []) do
-        Api.get!(__MODULE__, resource, id, params)
+      def get!(resource, id_or_filter, params \\ []) do
+        Ash.Api.Interface.enforce_resource!(resource)
+
+        Api.get!(__MODULE__, resource, id_or_filter, params)
       end
 
       # @spec get(Ash.Resource.t(), term, Keyword.t()) ::
       #         {:ok, Ash.Resource.record() | nil} | {:error, Ash.Error.t()}
-      def get(resource, id, params \\ []) do
-        case Api.get(__MODULE__, resource, id, params) do
+      def get(resource, id_or_filter, params \\ []) do
+        Ash.Api.Interface.enforce_resource!(resource)
+        Ash.Api.Interface.enforce_keyword_list!(params)
+
+        case Api.get(__MODULE__, resource, id_or_filter, params) do
           {:ok, instance} -> {:ok, instance}
           {:error, error} -> {:error, Ash.Error.to_ash_error(error)}
         end
@@ -323,12 +328,18 @@ defmodule Ash.Api.Interface do
       def read!(query, opts \\ [])
 
       def read!(query, opts) do
+        Ash.Api.Interface.enforce_query_or_resource!(query)
+        Ash.Api.Interface.enforce_keyword_list!(opts)
+
         Api.read!(__MODULE__, query, opts)
       end
 
       def read(query, opts \\ [])
 
       def read(query, opts) do
+        Ash.Api.Interface.enforce_query_or_resource!(query)
+        Ash.Api.Interface.enforce_keyword_list!(opts)
+
         case Api.read(__MODULE__, query, opts) do
           {:ok, results, query} -> {:ok, results, query}
           {:ok, results} -> {:ok, results}
@@ -336,15 +347,23 @@ defmodule Ash.Api.Interface do
         end
       end
 
+      # @spec read_one!(Ash.Query.t() | Ash.Resource.t(), Keyword.t()) ::
+      #         {:ok, Ash.Resource.record() | nil} | {:error, Ash.Error.t()} | no_return
       def read_one!(query, opts \\ [])
 
       def read_one!(query, opts) do
+        Ash.Api.Interface.enforce_query_or_resource!(query)
+        Ash.Api.Interface.enforce_keyword_list!(opts)
+
         Api.read_one!(__MODULE__, query, opts)
       end
 
       def read_one(query, opts \\ [])
 
       def read_one(query, opts) do
+        Ash.Api.Interface.enforce_query_or_resource!(query)
+        Ash.Api.Interface.enforce_keyword_list!(opts)
+
         case Api.read_one(__MODULE__, query, opts) do
           {:ok, result} -> {:ok, result}
           {:ok, result, query} -> {:ok, result, query}
@@ -501,4 +520,56 @@ defmodule Ash.Api.Interface do
     )
     |> to_string()
   end
+
+  defmacro enforce_query_or_resource!(query_or_resource) do
+    quote do
+      case Ash.Api.Interface.do_enforce_query_or_resource!(unquote(query_or_resource)) do
+        :ok ->
+          :ok
+
+        _ ->
+          {fun, arity} = __ENV__.function
+          mfa = "#{inspect(__ENV__.module)}.#{fun}/#{arity}"
+
+          raise "#{mfa} expected an %Ash.Query{} or an Ash Resource but instead got #{
+                  inspect(unquote(query_or_resource))
+                }"
+      end
+    end
+  end
+
+  def do_enforce_query_or_resource!(query_or_resource)
+  def do_enforce_query_or_resource!(%Ash.Query{}), do: :ok
+
+  def do_enforce_query_or_resource!(resource) when is_atom(resource) do
+    if Ash.Resource.Info.resource?(resource), do: :ok, else: :error
+  end
+
+  def do_enforce_query_or_resource!(_something), do: :error
+
+  defmacro enforce_resource!(resource) do
+    quote do
+      if Ash.Resource.Info.resource?(unquote(resource)) do
+        :ok
+      else
+        {fun, arity} = __ENV__.function
+        mfa = "#{inspect(__ENV__.module)}.#{fun}/#{arity}"
+
+        raise Ash.Error.Invalid.NoSuchResource,
+          message: "#{mfa} expected an Ash Resource but instead got #{inspect(unquote(resource))}"
+      end
+    end
+  end
+
+  defmacro enforce_keyword_list!(list) do
+    quote do
+      if Keyword.keyword?(unquote(list)) do
+        :ok
+      else
+        {fun, arity} = __ENV__.function
+        mfa = "#{inspect(__ENV__.module)}.#{fun}/#{arity}"
+        raise "#{mfa} expected a keyword list, but instead got #{inspect(unquote(list))}"
+      end
+    end
+  end
 end

lib/ash/error/invalid/no_such_resource.ex

@@ -2,13 +2,14 @@ defmodule Ash.Error.Invalid.NoSuchResource do
   @moduledoc "Used when a resource or alias is provided that doesn't exist"
   use Ash.Error.Exception
 
-  def_ash_error([:resource], class: :invalid)
+  def_ash_error([:resource, :message], class: :invalid)
 
   defimpl Ash.ErrorKind do
     def id(_), do: Ash.UUID.generate()
 
     def code(_), do: "no_such_resource"
 
+    def message(%{message: message}) when message != "", do: message
     def message(%{resource: resource}) do
       "No such resource #{inspect(resource)}"
     end

test/actions/read_test.exs

@@ -94,6 +94,30 @@ defmodule Ash.Test.Actions.ReadTest do
 
       assert fetched_post == post
     end
+
+    test "raises an error when the first argument is not a module" do
+      res = assert_raise Ash.Error.Invalid.NoSuchResource, fn -> Api.get("bogus", 1, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource" do
+      res = assert_raise Ash.Error.Invalid.NoSuchResource, fn -> Api.get(BadModuleName, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the third argument is not a list" do
+      res = assert_raise RuntimeError, fn -> Api.get(Post, "id", 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the third argument is not a valid keyword list" do
+      res = assert_raise RuntimeError, fn -> Api.get(Post, "id", [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
+    end
   end
 
   describe "api.get!/3" do
@@ -110,10 +134,34 @@ defmodule Ash.Test.Actions.ReadTest do
       assert ^post = Api.get!(Post, post.id)
     end
 
-    test "it raises on an error", %{post: post} do
-      assert_raise(Ash.Error.Invalid.NoSuchResource, ~r/\No such resource Something/, fn ->
-        Api.get!(Something, post.id)
-      end)
+    test "raises an error when the first argument is not a module", %{post: post} do
+      res = assert_raise Ash.Error.Invalid.NoSuchResource, fn -> Api.get("bogus", post.id, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource", %{
+      post: post
+    } do
+      res =
+        assert_raise Ash.Error.Invalid.NoSuchResource, fn ->
+          Api.get(BadModuleName, post.id, [])
+        end
+
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the third argument is not a list", %{post: post} do
+      res = assert_raise RuntimeError, fn -> Api.get(Post, post.id, 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the third argument is not a valid keyword list", %{post: post} do
+      res = assert_raise RuntimeError, fn -> Api.get(Post, post.id, [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.get\/3/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
     end
   end
 
@@ -121,12 +169,68 @@ defmodule Ash.Test.Actions.ReadTest do
     test "returns an empty result" do
       assert {:ok, []} = Api.read(Post)
     end
+
+    test "raises an error when the first argument is not a module" do
+      res = assert_raise RuntimeError, fn -> Api.read("bogus", []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource" do
+      res = assert_raise RuntimeError, fn -> Api.read(BadModuleName, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the second argument is not a list" do
+      res = assert_raise RuntimeError, fn -> Api.read(Post, 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the second argument is not a valid keyword list" do
+      res = assert_raise RuntimeError, fn -> Api.read(Post, [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
+    end
   end
 
   describe "Api.read!/2 with no records" do
     test "returns an empty result" do
       assert [] = Api.read!(Post)
     end
+
+    test "raises an error when the first argument is not a module" do
+      res = assert_raise RuntimeError, fn -> Api.read!("bogus", []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read!\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource" do
+      res = assert_raise RuntimeError, fn -> Api.read!(BadModuleName, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read!\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the second argument is not a list" do
+      res = assert_raise RuntimeError, fn -> Api.read!(Post, 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read!\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the second argument is not a valid keyword list" do
+      res = assert_raise RuntimeError, fn -> Api.read!(Post, [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read!\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
+    end
   end
 
   describe "Api.read/2" do
@@ -187,6 +291,66 @@ defmodule Ash.Test.Actions.ReadTest do
     end
   end
 
+  describe "Api.read_one/2" do
+    test "raises an error when the first argument is not a module" do
+      res = assert_raise RuntimeError, fn -> Api.read_one("bogus", []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource" do
+      res = assert_raise RuntimeError, fn -> Api.read_one(BadModuleName, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the second argument is not a list" do
+      res = assert_raise RuntimeError, fn -> Api.read_one(Post, 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the second argument is not a valid keyword list" do
+      res = assert_raise RuntimeError, fn -> Api.read_one(Post, [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
+    end
+  end
+
+  describe "Api.read_one!/2" do
+    test "raises an error when the first argument is not a module" do
+      res = assert_raise RuntimeError, fn -> Api.read_one!("bogus", []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one!\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got "bogus"/
+    end
+
+    test "raises an error when the first argument is a module that is not an ash resource" do
+      res = assert_raise RuntimeError, fn -> Api.read_one!(BadModuleName, []) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one!\/2/
+
+      assert res.message =~
+               ~r/expected an %Ash.Query{} or an Ash Resource but instead got BadModuleName/
+    end
+
+    test "raises an error when the second argument is not a list" do
+      res = assert_raise RuntimeError, fn -> Api.read_one!(Post, 1) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one!\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got 1/
+    end
+
+    test "raises an error when the second argument is not a valid keyword list" do
+      res = assert_raise RuntimeError, fn -> Api.read_one!(Post, [1]) end
+      assert res.message =~ ~r/Ash.Test.Actions.ReadTest.Api.read_one!\/2/
+      assert res.message =~ ~r/expected a keyword list, but instead got \[1\]/
+    end
+  end
+
   describe "filters" do
     setup do
       post1 =