diff --git a/config/config.exs b/config/config.exs index b01a79fd..45a50ba6 100644 --- a/config/config.exs +++ b/config/config.exs @@ -35,7 +35,9 @@ if Mix.env() == :test do config :ash_postgres, :ash_domains, [AshPostgres.Test.Domain] - config :ash, :custom_expressions, [AshPostgres.Expressions.TrigramWordSimilarity] + config :ash, :custom_expressions, [ + AshPostgres.Expressions.TrigramWordSimilarity + ] config :ash, :known_types, [AshPostgres.Timestamptz, AshPostgres.TimestamptzUsec] diff --git a/documentation/topics/advanced/expressions.md b/documentation/topics/advanced/expressions.md index c5a6a759..383a199c 100644 --- a/documentation/topics/advanced/expressions.md +++ b/documentation/topics/advanced/expressions.md @@ -81,3 +81,17 @@ For example: ```elixir Ash.Query.filter(User, trigram_similarity(first_name, "fred") > 0.8) ``` + +## Required error (ash_required!/2 and required_error/2) + +When the data layer supports the `:required_error` capability, Ash can use `ash_required!/2` for required-attribute validation: it returns the value when present, or returns `Ash.Error.Changes.Required` when the value is nil. This avoids inline `if is_nil(value), do: {:error, ...}, else: {:ok, value}` in changesets. + +Use `required_error/2` with `Ash.Changeset.require_change/3` so that when a value is nil, Ash calls the data layer and gets the standard required error: + +```elixir +change fn changeset, _context -> + Ash.Changeset.require_change(changeset, :title, &AshPostgres.Functions.RequiredError.required_error/2) +end +``` + +The function `AshPostgres.Functions.RequiredError.required_error/2` is provided by the data layer when `can?(:required_error)` is true (default when the ash-functions extension is installed). Ash uses it to build `expr(ash_required!(^value, ^attribute))` for required validation. diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 99036fa3..a63fc6e7 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -777,6 +777,9 @@ defmodule AshPostgres.DataLayer do def can?(resource, :expr_error), do: not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() + def can?(resource, :required_error), + do: not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() + def can?(resource, {:filter_expr, %Ash.Query.Function.Error{}}) do not AshPostgres.DataLayer.Info.repo(resource, :mutate).disable_expr_error?() && "ash-functions" in AshPostgres.DataLayer.Info.repo(resource, :read).installed_extensions() && @@ -877,7 +880,8 @@ defmodule AshPostgres.DataLayer do functions = [ AshPostgres.Functions.Like, AshPostgres.Functions.ILike, - AshPostgres.Functions.Binding + AshPostgres.Functions.Binding, + AshPostgres.Functions.RequiredError ] functions = @@ -2669,6 +2673,7 @@ defmodule AshPostgres.DataLayer do case Ecto.Adapters.Postgres.Connection.to_constraints(error, []) do [] -> constraints = maybe_foreign_key_violation_constraints(error) + if constraints != [] do {:error, changeset @@ -2695,7 +2700,7 @@ defmodule AshPostgres.DataLayer do code = postgres[:code] || postgres["code"] constraint = postgres[:constraint] || postgres["constraint"] - if code in ["23503", 23503, :foreign_key_violation] and is_binary(constraint) do + if code in ["23503", 23_503, :foreign_key_violation] and is_binary(constraint) do [{:foreign_key, constraint}] else [] diff --git a/lib/functions/required_error.ex b/lib/functions/required_error.ex new file mode 100644 index 00000000..26a14330 --- /dev/null +++ b/lib/functions/required_error.ex @@ -0,0 +1,56 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Functions.RequiredError do + @moduledoc """ + Function that returns the value if present, or an error if nil. + Used for required-attribute validation: `ash_required!(^value, ^attribute)`. + + When the data layer supports `:required_error`, Ash can build + `expr(ash_required!(^value, ^attribute))` instead of the inline if/is_nil/error block. + This module is returned from the data layer's `functions/1` so the function is available + when using AshPostgres. + """ + use Ash.Query.Function, name: :ash_required!, predicate?: false + + @impl true + def args, do: [[:any, :any]] + + @impl true + def new([value_expr, attribute]) when is_struct(attribute) or is_map(attribute) do + {:ok, %__MODULE__{arguments: [value_expr, attribute]}} + end + + def new(_), do: {:error, "ash_required! expects (value, attribute)"} + + @impl true + def evaluate(%{arguments: [value, attribute]}) do + if is_nil(value) do + resource = + Map.get(attribute, :resource) || raise("attribute must have :resource for ash_required!") + + field = + Map.get(attribute, :name) || Map.get(attribute, "name") || + raise("attribute must have :name for ash_required!") + + {:error, + Ash.Error.Changes.Required.exception( + field: field, + type: :attribute, + resource: resource + )} + else + {:known, value} + end + end + + @impl true + def can_return_nil?(_), do: false + + @impl true + def evaluate_nil_inputs?, do: true + + @impl true + def returns, do: :unknown +end diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index 5f94972f..cbfbcd28 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -540,7 +540,8 @@ defmodule AshPostgres.MigrationGenerator do if tenant? do Ecto.Migrator.with_repo(repo, fn repo -> for prefix <- repo.all_tenants() do - {repo, query, opts} = Ecto.Migration.SchemaMigration.versions(repo, repo.config(), prefix) + {repo, query, opts} = + Ecto.Migration.SchemaMigration.versions(repo, repo.config(), prefix) repo.transaction(fn -> versions = repo.all(query, Keyword.put(opts, :timeout, :infinity)) diff --git a/test/calculation_test.exs b/test/calculation_test.exs index e504f868..74f6f896 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -1669,4 +1669,5 @@ defmodule AshPostgres.CalculationTest do assert Ash.calculate!(post, :past_datetime1?) assert Ash.calculate!(post, :past_datetime2?) end + end diff --git a/test/destroy_test.exs b/test/destroy_test.exs index d2892469..65e705ec 100644 --- a/test/destroy_test.exs +++ b/test/destroy_test.exs @@ -4,7 +4,7 @@ defmodule AshPostgres.DestroyTest do use AshPostgres.RepoCase, async: false - alias AshPostgres.Test.{Post, Permalink} + alias AshPostgres.Test.{Permalink, Post} test "destroy with restrict on_delete returns would leave records behind error" do post = diff --git a/test/filter_test.exs b/test/filter_test.exs index dc628e31..227ed0a7 100644 --- a/test/filter_test.exs +++ b/test/filter_test.exs @@ -1240,4 +1240,84 @@ defmodule AshPostgres.FilterTest do assert fetched_org.id == organization.id end + + describe "not is_nil(expr) in filters" do + test "not is_nil(expr) compiles to IS NOT NULL in SQL (regression)" do + {query, _vars} = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.data_layer_query!() + |> Map.get(:query) + |> then(&AshPostgres.TestRepo.to_sql(:all, &1)) + + # SQL may be (expr) IS NOT NULL or NOT ((expr) IS NULL); both are equivalent. + assert query =~ "IS NOT NULL" or query =~ "is not null" or + (query =~ "NOT (" and query =~ "IS NULL"), + "Expected filter(not is_nil(...)) to compile to presence check (IS NOT NULL or NOT (... IS NULL)), got: #{query}" + end + + test "not is_nil(expr) filter returns only records where attribute is present (behavioral regression)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "with category", category: "tech"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "no category"}) + |> Ash.create!() + + assert [%{title: "with category"}] = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.read!() + end + + test "not is_nil(expr) returns empty list when no records have attribute set (edge case)" do + Post + |> Ash.Changeset.for_create(:create, %{title: "a"}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "b"}) + |> Ash.create!() + + assert [] = + Post + |> Ash.Query.filter(not is_nil(category)) + |> Ash.read!() + end + + test "not is_nil(expr) includes records where value is 0 or false — required means not null, not truthy (edge case)" do + post_zero = + Post + |> Ash.Changeset.for_create(:create, %{title: "zero score", score: 0}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "nil score"}) + |> Ash.create!() + + assert [%{id: id}] = + Post + |> Ash.Query.filter(title in ["zero score", "nil score"] and not is_nil(score)) + |> Ash.read!() + + assert id == post_zero.id + + post_false = + Post + |> Ash.Changeset.for_create(:create, %{title: "false public", public: false}) + |> Ash.create!() + + Post + |> Ash.Changeset.for_create(:create, %{title: "nil public"}) + |> Ash.create!() + + assert [%{id: id2}] = + Post + |> Ash.Query.filter(title in ["false public", "nil public"] and not is_nil(public)) + |> Ash.read!() + + assert id2 == post_false.id + end + end end diff --git a/test/functions/required_error_test.exs b/test/functions/required_error_test.exs new file mode 100644 index 00000000..4102c03e --- /dev/null +++ b/test/functions/required_error_test.exs @@ -0,0 +1,27 @@ +# SPDX-FileCopyrightText: 2019 ash_postgres contributors +# +# SPDX-License-Identifier: MIT + +defmodule AshPostgres.Functions.RequiredErrorTest do + use ExUnit.Case, async: true + + alias AshPostgres.Functions.RequiredError + + test "ash_required!/2 returns error when value is nil" do + attribute = %{name: :title, resource: MyApp.Post} + + assert {:error, %Ash.Error.Changes.Required{} = err} = + RequiredError.evaluate(%{arguments: [nil, attribute]}) + + assert err.field == :title + assert err.resource == MyApp.Post + assert err.type == :attribute + end + + test "ash_required!/2 returns value when non-nil" do + attribute = %{name: :title, resource: MyApp.Post} + + assert {:known, "hello"} = + RequiredError.evaluate(%{arguments: ["hello", attribute]}) + end +end