From f6fa5a98dd6534e6d5874f12f447e6ab6f14215e Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Sun, 18 Apr 2021 13:45:38 -1000 Subject: [PATCH] improvement: Add detailed parameter checking for Api read functions (#229) --- lib/ash/api/interface.ex | 79 +++++++++- lib/ash/error/invalid/no_such_resource.ex | 3 +- test/actions/read_test.exs | 172 +++++++++++++++++++++- 3 files changed, 245 insertions(+), 9 deletions(-) diff --git a/lib/ash/api/interface.ex b/lib/ash/api/interface.ex index df66741a..098f936a 100644 --- a/lib/ash/api/interface.ex +++ b/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 diff --git a/lib/ash/error/invalid/no_such_resource.ex b/lib/ash/error/invalid/no_such_resource.ex index bb84d256..0b05fe4c 100644 --- a/lib/ash/error/invalid/no_such_resource.ex +++ b/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 diff --git a/test/actions/read_test.exs b/test/actions/read_test.exs index 629372af..7d09c46c 100644 --- a/test/actions/read_test.exs +++ b/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 =