diff --git a/CHANGELOG.md b/CHANGELOG.md index 2babd30..65261f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added * Ensure field/assoc/embed exists when listing errors in `flat_errors_on/3`. This prevents accidental test passes on typos in assertions like `refute_errors_on(cs, :sommtypo)`. +* Add ability to disable "tagged" not found errors in `Repo.fetch/2` and friends (local to calls or global option). ## [1.0.0] - 2023-12-21 diff --git a/config/config.exs b/config/config.exs index a196712..db92127 100644 --- a/config/config.exs +++ b/config/config.exs @@ -1,16 +1,21 @@ import Config +config = [ + migration_timestamps: [type: :utc_datetime_usec], + migration_primary_key: [name: :id, type: :binary_id], + database: "bitcrowd_ecto_#{config_env()}", + username: "postgres", + password: "postgres", + hostname: "localhost", + priv: "test/support/test_repo" +] + if config_env() in [:dev, :test] do - config :bitcrowd_ecto, BitcrowdEcto.TestRepo, - migration_timestamps: [type: :utc_datetime_usec], - migration_primary_key: [name: :id, type: :binary_id], - database: "bitcrowd_ecto_#{config_env()}", - username: "postgres", - password: "postgres", - hostname: "localhost", - priv: "test/support/test_repo" + config :bitcrowd_ecto, BitcrowdEcto.TestRepo, config + config :bitcrowd_ecto, BitcrowdEcto.TestRepoWithUntaggedNotFoundErrors, config - config :bitcrowd_ecto, ecto_repos: [BitcrowdEcto.TestRepo] + config :bitcrowd_ecto, + ecto_repos: [BitcrowdEcto.TestRepo, BitcrowdEcto.TestRepoWithUntaggedNotFoundErrors] config :elixir, :time_zone_database, Tzdata.TimeZoneDatabase end @@ -21,6 +26,9 @@ if config_env() == :test do config :bitcrowd_ecto, BitcrowdEcto.TestRepo, pool: Ecto.Adapters.SQL.Sandbox + config :bitcrowd_ecto, BitcrowdEcto.TestRepoWithUntaggedNotFoundErrors, + pool: Ecto.Adapters.SQL.Sandbox + config :ex_cldr, default_backend: BitcrowdEcto.TestCldr, default_locale: "en" diff --git a/lib/bitcrowd_ecto/repo.ex b/lib/bitcrowd_ecto/repo.ex index b872fee..519f51d 100644 --- a/lib/bitcrowd_ecto/repo.ex +++ b/lib/bitcrowd_ecto/repo.ex @@ -23,11 +23,14 @@ defmodule BitcrowdEcto.Repo do @type fetch_option :: {:lock, lock_mode | false} | {:preload, atom | list} - | {:error_tag, any} + | {:error_tag, false | any} | {:raise_cast_error, boolean()} | ecto_option - @type fetch_result :: {:ok, Ecto.Schema.t()} | {:error, {:not_found, Ecto.Queryable.t() | any}} + @type fetch_result :: + {:ok, Ecto.Schema.t()} + | {:error, {:not_found, Ecto.Queryable.t() | any}} + | {:error, :not_found} @ecto_options [:prefix, :timeout, :log, :telemetry_event, :telemetry_options] @@ -39,7 +42,7 @@ defmodule BitcrowdEcto.Repo do | {:telemetry_options, any} @doc """ - Fetches a record by primary key or returns a "tagged" error tuple. + Fetches a record by primary key or returns an error tuple. See `c:fetch_by/3`. """ @@ -47,7 +50,7 @@ defmodule BitcrowdEcto.Repo do @callback fetch(schema :: module, id :: any) :: fetch_result() @doc """ - Fetches a record by given clauses or returns a "tagged" error tuple. + Fetches a record by given clauses or returns an error tuple. See `c:fetch_by/3` for options. """ @@ -55,7 +58,7 @@ defmodule BitcrowdEcto.Repo do @callback fetch(schema :: module, id :: any, [fetch_option()]) :: fetch_result() @doc """ - Fetches a record by given clauses or returns a "tagged" error tuple. + Fetches a record by given clauses or returns an error tuple. See `c:fetch_by/3` for options. """ @@ -63,22 +66,38 @@ defmodule BitcrowdEcto.Repo do @callback fetch_by(queryable :: Ecto.Queryable.t(), clauses :: map | keyword) :: fetch_result() @doc """ - Fetches a record by given clauses or returns a "tagged" error tuple. + Fetches a record by given clauses or returns an error tuple. - On success, the record is wrapped in a `:ok` tuple. - - On error, a "tagged" error tuple is returned that contains the *original* queryable or module - as the tag, e.g. `{:error, {:not_found, Account}}` for a `fetch_by(Account, id: 1)` call. + - On error, an error tuple is returned + + ## Tagged error tuples + + By default, the error tuple will be a "tagged" `:not_found` tuple, e.g. + `{:error, {:not_found, Account}}` for a `fetch_by(Account, id: 1)` call, where the "tag" is + the unmodified `queryable` parameter. The idea behind this is to avoid mix-ups of + naked `:not_found` errors, particularly in `with` clauses. + + Tagging behaviour may be disabled by passing the `error_tag: false` option to return + naked `{:error, :not_found}` tuples instead. For existing applications where untagged errors + are the norm, one may set the `tagged_not_found_errors: false` option when using this module. + + use BitcrowdEcto.Repo, tagged_not_found_errors: false + + ## Automatic conversion of `CastError` Passing invalid values that would normally result in an `Ecto.Query.CastError` will result in - a `:not_found` error tuple as well. + a `:not_found` error tuple. This is useful for low-level handling of invalid UUIDs passed + from a hand-edited URL to the domain layer. - This function can also apply row locks. + This behaviour can be disabled by passing `raise_cast_error: false`. ## Options * `lock` any of `[:no_key_update, :update]` (defaults to `false`) * `preload` allows to preload associations * `error_tag` allows to specify a custom "tag" value (instead of the queryable) + or `false` to disabled tagged error tuples * `raise_cast_error` raise `CastError` instead of converting to `:not_found` (defaults to `false`) ## Ecto options @@ -149,12 +168,18 @@ defmodule BitcrowdEcto.Repo do @doc since: "0.1.0" @callback advisory_xact_lock(atom | binary) :: :ok - defmacro __using__(_) do + defmacro __using__(opts) do + tagged_not_found_errors = Keyword.get(opts, :tagged_not_found_errors, true) + quote do alias BitcrowdEcto.Repo, as: BER @behaviour BER + @doc false + @spec __tagged_not_found_errors__() :: boolean + def __tagged_not_found_errors__, do: unquote(tagged_not_found_errors) + @impl BER def fetch(module, id, opts \\ []) when is_atom(module) do BER.fetch(__MODULE__, module, id, opts) @@ -206,7 +231,7 @@ defmodule BitcrowdEcto.Repo do end) case result do - nil -> {:error, {:not_found, Keyword.get(opts, :error_tag, queryable)}} + nil -> handle_not_found_error(repo, queryable, opts) value -> {:ok, value} end end @@ -247,6 +272,16 @@ defmodule BitcrowdEcto.Repo do end end + defp handle_not_found_error(repo, queryable, opts) do + tag = Keyword.get(opts, :error_tag, queryable) + + if repo.__tagged_not_found_errors__() == false or tag == false do + {:error, :not_found} + else + {:error, {:not_found, tag}} + end + end + @doc false @spec count(module, Ecto.Queryable.t(), keyword) :: non_neg_integer def count(repo, queryable, opts) do diff --git a/test/bitcrowd_ecto/repo_test.exs b/test/bitcrowd_ecto/repo_test.exs index 42a78e6..b0570e3 100644 --- a/test/bitcrowd_ecto/repo_test.exs +++ b/test/bitcrowd_ecto/repo_test.exs @@ -3,6 +3,7 @@ defmodule BitcrowdEcto.RepoTest do use BitcrowdEcto.TestCase, async: true require Ecto.Query + alias BitcrowdEcto.TestRepoWithUntaggedNotFoundErrors defp insert_test_schema(_) do %{resource: insert(:test_schema)} @@ -81,7 +82,7 @@ defmodule BitcrowdEcto.RepoTest do end end - describe "fetch/3" do + describe "fetch/3 when the resource exists" do setup [:insert_test_schema_with_prefix] test "returns the record using the prefix option", %{resource: resource, prefix: prefix} do @@ -90,6 +91,20 @@ defmodule BitcrowdEcto.RepoTest do end end + describe "fetch/3 when the resource does not exist" do + test "error tagging can be disabled" do + assert TestRepo.fetch(TestSchema, Ecto.UUID.generate(), error_tag: false) == + {:error, :not_found} + end + + test "error tagging can be disabled globally" do + start_supervised!(TestRepoWithUntaggedNotFoundErrors) + + assert TestRepoWithUntaggedNotFoundErrors.fetch(TestSchema, Ecto.UUID.generate()) == + {:error, :not_found} + end + end + describe "fetch_by/3" do setup [:insert_test_schema] diff --git a/test/support/test_repo.ex b/test/support/test_repo.ex index be66fa7..8c57630 100644 --- a/test/support/test_repo.ex +++ b/test/support/test_repo.ex @@ -10,3 +10,14 @@ defmodule BitcrowdEcto.TestRepo do use BitcrowdEcto.Repo end + +defmodule BitcrowdEcto.TestRepoWithUntaggedNotFoundErrors do + @moduledoc false + + use Ecto.Repo, + otp_app: :bitcrowd_ecto, + adapter: Ecto.Adapters.Postgres, + priv: "test/support/test_repo" + + use BitcrowdEcto.Repo, tagged_not_found_errors: false +end