improvement: support nils_distinct? on identities

improvement: support `where` option on `identities`
improvement: allow calculations in identity keys

closes #1001
closes #1182
This commit is contained in:
Zach Daniel 2024-05-24 00:24:42 -04:00
parent 8b1ce5e5a0
commit dc94f3a743
13 changed files with 162 additions and 64 deletions

View file

@ -140,6 +140,7 @@ spark_locals_without_parens = [
module: 1,
multitenancy: 1,
name: 1,
nils_distinct?: 1,
no_attributes?: 1,
not_found_error?: 1,
not_found_message: 1,

View file

@ -2184,6 +2184,8 @@ identity :full_name, [:first_name, :last_name]
| Name | Type | Default | Docs |
|------|------|---------|------|
| [`where`](#identities-identity-where){: #identities-identity-where } | `any` | | A filter that expresses only matching records are unique on the provided keys. Ignored on embedded resources. |
| [`nils_distinct?`](#identities-identity-nils_distinct?){: #identities-identity-nils_distinct? } | `boolean` | `true` | Whether or not `nil` values are considered always distinct from eachother. `nil` values won't conflict with eachother unless you set this option to `false`. |
| [`eager_check?`](#identities-identity-eager_check?){: #identities-identity-eager_check? } | `boolean` | `false` | Whether or not this identity is validated to be unique at validation time. |
| [`eager_check_with`](#identities-identity-eager_check_with){: #identities-identity-eager_check_with } | `module` | | Validates that the unique identity provided is unique at validation time, outside of any transactions, using the domain module provided. Will default to resource's domain. |
| [`pre_check?`](#identities-identity-pre_check?){: #identities-identity-pre_check? } | `boolean` | `false` | Whether or not this identity is validated to be unique in a before_action hook. |

View file

@ -569,7 +569,14 @@ defmodule Ash.Actions.ManagedRelationships do
Ash.Resource.Info.primary_key(relationship.destination)
identity ->
Ash.Resource.Info.identity(relationship.destination, identity).keys
identity = Ash.Resource.Info.identity(relationship.destination, identity)
if is_nil(identity.where) do
identity.keys
else
raise ArgumentError,
"Cannot currently use identities with a `where` statement in managed_relationships. Got #{inspect(relationship.destination)}.#{identity.name}"
end
end)
end

View file

@ -1831,56 +1831,73 @@ defmodule Ash.Changeset do
if changeset.context[:private][:upsert_identity] == identity.name do
changeset
else
if Enum.any?(identity.keys, &changing_attribute?(changeset, &1)) do
if changeset.action_type == :create ||
Enum.any?(identity.keys, &changing_attribute?(changeset, &1)) do
action = Ash.Resource.Info.primary_action(changeset.resource, :read).name
if Enum.any?(identity.keys, fn key ->
Ash.Resource.Info.calculation(changeset.resource, key)
end) do
raise ArgumentError, "Cannot pre or eager check an identity based on calculated fields."
end
values =
Enum.map(identity.keys, fn key ->
{key, Ash.Changeset.get_attribute(changeset, key)}
case Ash.Changeset.get_attribute(changeset, key) do
nil ->
{key, is_nil: true}
value ->
{key, value}
end
end)
tenant =
if identity.all_tenants? do
unless Ash.Resource.Info.multitenancy_global?(changeset.resource) do
raise ArgumentError,
message: """
Cannot pre or eager check an identity that has `all_tenants?: true`
unless the resource supports global multitenancy.
"""
if identity.nils_distinct? && Enum.any?(values, &(elem(&1, 1) == [is_nil: true])) do
changeset
else
tenant =
if identity.all_tenants? do
unless Ash.Resource.Info.multitenancy_global?(changeset.resource) do
raise ArgumentError,
message: """
Cannot pre or eager check an identity that has `all_tenants?: true`
unless the resource supports global multitenancy.
"""
end
nil
else
changeset.tenant
end
nil
else
changeset.tenant
changeset.resource
|> Ash.Query.for_read(action, %{},
tenant: tenant,
actor: changeset.context[:private][:actor],
authorize?: changeset.context[:private][:authorize?],
tracer: changeset.context[:private][:tracer],
domain: domain
)
|> Ash.Query.do_filter(values)
|> Ash.Query.limit(1)
|> Ash.Query.set_context(%{private: %{internal?: true}})
|> Ash.read_one(authorize?: false)
|> case do
{:ok, nil} ->
changeset
{:ok, _} ->
error =
Ash.Error.Changes.InvalidChanges.exception(
fields: identity.keys,
message: identity.message || "has already been taken"
)
add_error(changeset, error)
{:error, error} ->
add_error(changeset, error)
end
changeset.resource
|> Ash.Query.for_read(action, %{},
tenant: tenant,
actor: changeset.context[:private][:actor],
authorize?: changeset.context[:private][:authorize?],
tracer: changeset.context[:private][:tracer],
domain: domain
)
|> Ash.Query.do_filter(values)
|> Ash.Query.limit(1)
|> Ash.Query.set_context(%{private: %{internal?: true}})
|> Ash.read_one(authorize?: false)
|> case do
{:ok, nil} ->
changeset
{:ok, _} ->
error =
Ash.Error.Changes.InvalidChanges.exception(
fields: identity.keys,
message: identity.message || "has already been taken"
)
add_error(changeset, error)
{:error, error} ->
add_error(changeset, error)
end
else
changeset

View file

@ -610,7 +610,7 @@ defmodule Ash.EmbeddableType do
def apply_constraints_array(term, constraints) do
case find_duplicates(
term,
__MODULE__ |> Ash.Resource.Info.unique_keys() |> Enum.map(& &1.keys)
__MODULE__ |> Ash.Resource.Info.unique_keys()
) do
nil ->
if constraints[:sort] do
@ -659,8 +659,9 @@ defmodule Ash.EmbeddableType do
defp find_duplicates([], _), do: nil
defp find_duplicates([_item], _), do: nil
defp find_duplicates(list, unique_keys) do
Enum.find(unique_keys, fn unique_key ->
defp find_duplicates(list, unique_key_configs) do
Enum.find(unique_key_configs, fn unique_key_config ->
unique_key = unique_key_config[:keys]
attributes = Enum.map(unique_key, &Ash.Resource.Info.attribute(__MODULE__, &1))
if Enum.all?(attributes, &Ash.Type.simple_equality?(&1.type)) do
@ -671,7 +672,7 @@ defmodule Ash.EmbeddableType do
|> Map.take(unique_key)
|> Map.values()
if Enum.any?(value, &is_nil/1) do
if unique_key_config.nils_distinct? && Enum.any?(value, &is_nil/1) do
[]
else
[value]
@ -697,8 +698,12 @@ defmodule Ash.EmbeddableType do
this_value = Map.get(this, attribute.name)
other_value = Map.get(other, attribute.name)
not is_nil(this_value) and not is_nil(other_value) and
if unique_key_config.nils_distinct? do
not is_nil(this_value) and not is_nil(other_value) and
Ash.Type.equal?(attribute.type, this_value, other_value)
else
Ash.Type.equal?(attribute.type, this_value, other_value)
end
end)
end)

View file

@ -482,7 +482,7 @@ defmodule Ash.Filter do
end
end
defp get_keys(value, fields, resource) do
defp get_keys(value, fields, resource, nils_distinct? \\ true) do
original_value = value
Enum.reduce_while(fields, {:ok, %{}}, fn field, {:ok, vals} ->
@ -490,10 +490,14 @@ defmodule Ash.Filter do
{:ok, value} ->
case cast_value(resource, field, value, original_value) do
{:ok, value} ->
{:cont, {:ok, Map.put(vals, field, value)}}
if value == nil && nils_distinct? do
{:halt, :error}
else
{:cont, {:ok, Map.put(vals, field, value)}}
end
{:error, error} ->
{:halt, {:error, error}}
{:error, _error} ->
{:halt, :error}
end
:error ->
@ -501,10 +505,14 @@ defmodule Ash.Filter do
{:ok, value} ->
case cast_value(resource, field, value, original_value) do
{:ok, value} ->
{:cont, {:ok, Map.put(vals, field, value)}}
if value == nil && nils_distinct? do
{:halt, :error}
else
{:cont, {:ok, Map.put(vals, field, value)}}
end
{:error, error} ->
{:error, error}
{:error, _error} ->
{:halt, :error}
end
:error ->
@ -512,6 +520,23 @@ defmodule Ash.Filter do
end
end
end)
|> case do
{:ok, values} ->
{:ok,
Map.new(values, fn
{key, nil} ->
{key, [is_nil: true]}
{key, value} ->
{key, value}
end)}
:error ->
:error
{:error, error} ->
{:error, error}
end
end
defp cast_value(resource, field, value, id) do
@ -540,7 +565,7 @@ defmodule Ash.Filter do
|> Enum.find_value(
:error,
fn identity ->
case get_keys(id, identity.keys, resource) do
case get_keys(id, identity.keys, resource, identity.nils_distinct?) do
{:ok, key} ->
{:ok, key}

View file

@ -769,6 +769,7 @@ defmodule Ash.Resource.Dsl do
describe: """
Unique identifiers for the resource
""",
imports: [Ash.Expr],
examples: [
"""
identities do

View file

@ -16,9 +16,11 @@ defmodule Ash.Resource.Identity do
:message,
:eager_check?,
:eager_check_with,
:where,
:pre_check?,
:pre_check_with,
:all_tenants?
:all_tenants?,
nils_distinct?: false
]
@schema [
@ -32,6 +34,17 @@ defmodule Ash.Resource.Identity do
required: true,
doc: "The names of the attributes that uniquely identify this resource."
],
where: [
type: :any,
doc:
"A filter that expresses only matching records are unique on the provided keys. Ignored on embedded resources."
],
nils_distinct?: [
type: :boolean,
doc:
"Whether or not `nil` values are considered always distinct from eachother. `nil` values won't conflict with eachother unless you set this option to `false`.",
default: true
],
eager_check?: [
type: :boolean,
default: false,

View file

@ -283,7 +283,8 @@ defmodule Ash.Resource.Info do
end
@doc "A list of unique keys and information for a resource"
@spec unique_keys(Spark.Dsl.t() | Ash.Resource.t()) :: list(%{type: atom, keys: list(atom)})
@spec unique_keys(Spark.Dsl.t() | Ash.Resource.t()) ::
list(%{type: atom, keys: list(atom), nils_distinct?: boolean()})
def unique_keys(resource) do
Spark.Dsl.Extension.get_persisted(resource, :unique_keys, [])
end

View file

@ -8,7 +8,7 @@ defmodule Ash.Resource.Transformers.CacheUniqueKeys do
unique_keys =
dsl_state
|> Ash.Resource.Info.identities()
|> Enum.map(&%{keys: &1.keys, type: :identity})
|> Enum.map(&%{keys: &1.keys, type: :identity, nils_distinct?: &1.nils_distinct?})
pkey = Ash.Resource.Info.primary_key(dsl_state)
@ -17,7 +17,7 @@ defmodule Ash.Resource.Transformers.CacheUniqueKeys do
Enum.all?(pkey, fn pkey ->
Ash.Resource.Info.attribute(dsl_state, pkey).public?
end) do
[%{keys: pkey, type: :primary_key} | unique_keys]
[%{keys: pkey, type: :primary_key, nils_distinct?: false} | unique_keys]
else
unique_keys
end

View file

@ -9,10 +9,10 @@ defmodule Ash.Resource.Verifiers.VerifyIdentityFields do
for identity <- identities do
for key <- identity.keys do
if !Ash.Resource.Info.attribute(dsl, key) do
unless Ash.Resource.Info.attribute(dsl, key) || Ash.Resource.Info.calculation(dsl, key) do
raise Spark.Error.DslError,
module: Spark.Dsl.Verifier.get_persisted(dsl, :module),
message: "All identity keys must be attributes. Got: #{inspect(key)}",
message: "All identity keys must be attributes or calculations. Got: #{inspect(key)}",
path: [:identities, identity.name]
end
end

View file

@ -22,6 +22,11 @@ defmodule Ash.Test.Actions.IdentityTest do
identity :unique_url, [:url] do
pre_check_with(Domain)
end
identity :unique_uniq_nil, [:uniq_nil] do
eager_check_with(Domain)
nils_distinct?(false)
end
end
actions do
@ -33,17 +38,22 @@ defmodule Ash.Test.Actions.IdentityTest do
end
end
calculations do
calculate :lower_title, :string, expr(string_downcase(title))
end
attributes do
uuid_primary_key :id
attribute(:title, :string, allow_nil?: false, public?: true)
attribute(:url, :string, public?: true)
attribute(:uniq_nil, :string, public?: true)
end
end
describe "eager_check_with" do
test "will check for an identity mismatch at validation" do
Post
|> Ash.Changeset.for_create(:create, %{title: "fred"}, domain: Domain)
|> Ash.Changeset.for_create(:create, %{title: "fred", uniq_nil: "foo"}, domain: Domain)
|> Ash.create!()
assert %{
@ -56,12 +66,28 @@ defmodule Ash.Test.Actions.IdentityTest do
]
} = Ash.Changeset.for_create(Post, :create, %{title: "fred"})
end
test "honors nils_distinct?" do
Post
|> Ash.Changeset.for_create(:create, %{title: "a"}, domain: Domain)
|> Ash.create!()
assert %{
valid?: false,
errors: [
%Ash.Error.Changes.InvalidChanges{
fields: [:uniq_nil],
message: "has already been taken"
}
]
} = Ash.Changeset.for_create(Post, :create, %{title: "fred"})
end
end
describe "pre_check?" do
test "will check for an identity mismatch prior to submission" do
Post
|> Ash.Changeset.for_create(:create, %{title: "fred", url: "google.com"}, domain: Domain)
|> Ash.Changeset.for_create(:create, %{title: "fred", url: "google.com", uniq_nil: "foo"}, domain: Domain)
|> Ash.create!()
assert_raise Ash.Error.Invalid, ~r/url: has already been taken/, fn ->

View file

@ -127,7 +127,7 @@ defmodule Ash.Test.Resource.IdentitiesTest do
test "enforce identity constraints on attributes" do
assert_raise Spark.Error.DslError,
~r/All identity keys must be attributes. Got: :naem/,
~r/All identity keys must be attributes or calculations. Got: :naem/,
fn ->
defmodule Site do
@moduledoc false