Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
14 changes: 14 additions & 0 deletions documentation/topics/advanced/expressions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
9 changes: 7 additions & 2 deletions lib/data_layer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -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
[]
Expand Down
56 changes: 56 additions & 0 deletions lib/functions/required_error.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# SPDX-FileCopyrightText: 2019 ash_postgres contributors <https://github.com/ash-project/ash_postgres/graphs/contributors>
#
# SPDX-License-Identifier: MIT

defmodule AshPostgres.Functions.RequiredError do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should also live in ash core. We would just need to add support for it in ash_sql, update the dep here, and then add it to the capabilities list.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to help get this implemented in ash core if you would like. Do we need to have this in a separate issue for ash core?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also go in and make changes to ash_sql if you would like.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please feel free 😄 There are other functions in AshSql and Ash core that you can use as reference. LMK if you have any questions! So first the change in Ash, second the change in AshSql, third the change in AshPostgres.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, just to clarify. What changes need to be made in ash_sql? As of right now I know that I am moving the required_error to ash core and then updating ash_sql and ash_postgres. I just want to make sure I know what needs to be changed in ash_sql. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, actually 🤔 You wouldn't make any changes in AshSql come to think of it. You can modify the SQL implementation file in ash_postgres to add a handler for the new function in ash core. So we just need to first add the function to ash, and then add an expression handler here 😄 Sorry for misleading.

@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
3 changes: 2 additions & 1 deletion lib/migration_generator/migration_generator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions test/calculation_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1669,4 +1669,5 @@ defmodule AshPostgres.CalculationTest do
assert Ash.calculate!(post, :past_datetime1?)
assert Ash.calculate!(post, :past_datetime2?)
end

end
2 changes: 1 addition & 1 deletion test/destroy_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
80 changes: 80 additions & 0 deletions test/filter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 27 additions & 0 deletions test/functions/required_error_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# SPDX-FileCopyrightText: 2019 ash_postgres contributors <https://github.com/ash-project/ash_postgres/graphs/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