improvement: add eager validate identities

improvement: percolate `nil` values in operators in ash expresion language (like SQL)
chore: more docs work

.formatter.exs

@@ -51,6 +51,7 @@ locals_without_parens = [
   destroy: 3,
   destroy: 4,
   dispatcher: 1,
+  eager_check?: 1,
   entry: 1,
   entry: 2,
   error_handler: 1,
@@ -99,6 +100,7 @@ locals_without_parens = [
   output: 1,
   pagination: 1,
   parse_attribute: 1,
+  pre_check?: 1,
   prefix: 1,
   prepare: 1,
   prepare: 2,

.github/workflows/elixir.yml

@@ -53,7 +53,7 @@ jobs:
     strategy:
       matrix:
         otp: ["23"]
-        elixir: ["1.11.0"]
+        elixir: ["1.13.1"]
     env:
       GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     steps:
@@ -79,7 +79,7 @@ jobs:
     name: Release
     strategy:
       matrix:
-        otp: ["23"]
+        otp: ["24.1.2"]
         elixir: ["1.11.0"]
     env:
       GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

documentation/concepts/expressions.md

@@ -0,0 +1,2 @@
+# Expressions
+

lib/ash/actions/create.ex

@@ -21,10 +21,13 @@ defmodule Ash.Actions.Create do
           opts
       end
 
-    upsert? = opts[:upsert?] || false
+    upsert? = opts[:upsert?] || get_in(changeset.context, [:private, :upsert?]) || false
     authorize? = authorize?(opts)
     upsert_keys = opts[:upsert_keys]
-    upsert_identity = opts[:upsert_identity]
+
+    upsert_identity =
+      opts[:upsert_identity] || get_in(changeset.context, [:private, :upsert_identity])
+
     return_notifications? = opts[:return_notifications?]
     actor = opts[:actor]
     verbose? = opts[:verbose?]

lib/ash/changeset/changeset.ex

@@ -169,6 +169,16 @@ defmodule Ash.Changeset do
 
   require Ash.Query
 
+  # Used for eager validating identities
+  defmodule ShadowApi do
+    @moduledoc false
+    use Ash.Api
+
+    resources do
+      allow_unregistered? true
+    end
+  end
+
   @doc """
   Return a changeset over a resource or a record. `params` can be either attributes, relationship values or arguments.
 
@@ -419,7 +429,11 @@ defmodule Ash.Changeset do
             """
       end
 
-    for_action(changeset, action, params, opts)
+    changeset
+    |> set_context(%{
+      private: %{upsert?: opts[:upsert?] || false, upsert_identity: opts[:upsert_identity]}
+    })
+    |> for_action(action, params, opts)
   end
 
   @doc """
@@ -535,6 +549,7 @@ defmodule Ash.Changeset do
           |> add_validations()
           |> mark_validated(action.name)
           |> require_arguments(action)
+          |> eager_validate_identities()
 
         if Keyword.get(opts, :require?, true) do
           require_values(changeset, action.type)
@@ -549,6 +564,97 @@ defmodule Ash.Changeset do
     end
   end
 
+  defp eager_validate_identities(changeset) do
+    identities =
+      changeset.resource
+      |> Ash.Resource.Info.identities()
+
+    case identities do
+      [] ->
+        changeset
+
+      identities ->
+        Enum.reduce(identities, changeset, fn identity, changeset ->
+          changeset =
+            if identity.eager_check_with do
+              validate_identity(changeset, identity)
+            else
+              changeset
+            end
+
+          if identity.pre_check_with do
+            before_action(changeset, &validate_identity(&1, identity))
+          else
+            changeset
+          end
+        end)
+    end
+  end
+
+  defp validate_identity(
+         %{context: %{private: %{upsert?: true, upsert_identity: name}}} = changeset,
+         %{name: name}
+       ) do
+    changeset
+  end
+
+  defp validate_identity(
+         %{action: %{soft?: true}} = changeset,
+         identity
+       ) do
+    do_validate_identity(changeset, identity)
+  end
+
+  defp validate_identity(
+         %{action: %{type: type}} = changeset,
+         identity
+       )
+       when type in [:create, :update] do
+    do_validate_identity(changeset, identity)
+  end
+
+  defp validate_identity(
+         %{action: %{type: type}} = changeset,
+         identity
+       )
+       when type in [:create, :update] do
+    do_validate_identity(changeset, identity)
+  end
+
+  defp validate_identity(changeset, _), do: changeset
+
+  defp do_validate_identity(changeset, identity) do
+    if Enum.any?(identity.keys, &changing_attribute?(changeset, &1)) do
+      action = Ash.Resource.Info.primary_action(changeset.resource, :read).name
+
+      values =
+        Enum.map(identity.keys, fn key ->
+          {key, Ash.Changeset.get_attribute(changeset, key)}
+        end)
+
+      changeset.resource
+      |> Ash.Query.for_read(action, %{}, tenant: changeset.tenant)
+      |> Ash.Query.do_filter(values)
+      |> Ash.Query.limit(1)
+      |> ShadowApi.read_one()
+      |> 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)
+      end
+    else
+      changeset
+    end
+  end
+
   defp require_arguments(changeset, action) do
     changeset
     |> do_require_arguments(action)

lib/ash/data_layer/ets.ex

@@ -335,12 +335,53 @@ defmodule Ash.DataLayer.Ets do
   end
 
   @impl true
-  def upsert(resource, changeset, _) do
-    update(resource, changeset)
+  def upsert(resource, changeset, keys) do
+    if Enum.sort(keys) == Enum.sort(Ash.Resource.Info.primary_key(resource)) do
+      create(resource, changeset, upsert?: true)
+    else
+      key_filters =
+        Enum.map(keys, fn key ->
+          {key, Ash.Changeset.get_attribute(changeset, key)}
+        end)
+
+      if Enum.any?(key_filters, fn {_, val} -> is_nil(val) end) do
+        update(resource, changeset)
+      else
+        query = Ash.Query.do_filter(resource, and: [key_filters])
+
+        resource
+        |> resource_to_query(changeset.api)
+        |> Map.put(:filter, query.filter)
+        |> Map.put(:tenant, changeset.tenant)
+        |> run_query(resource)
+        |> case do
+          {:ok, []} ->
+            update(resource, changeset)
+
+          {:ok, results} ->
+            Enum.reduce_while(results, :ok, fn result, :ok ->
+              case do_destroy(changeset.resource, result, changeset.tenant) do
+                :ok ->
+                  {:cont, :ok}
+
+                {:error, error} ->
+                  {:halt, {:error, error}}
+              end
+            end)
+            |> case do
+              :ok ->
+                update(resource, changeset)
+
+              {:error, error} ->
+                {:error, error}
+            end
+        end
+      end
+    end
   end
 
   @impl true
-  def create(resource, changeset) do
+  def create(resource, changeset, opts \\ []) do
     pkey =
       resource
       |> Ash.Resource.Info.primary_key()
@@ -351,18 +392,31 @@ defmodule Ash.DataLayer.Ets do
     with {:ok, table} <- wrap_or_create_table(resource, changeset.tenant),
          {:ok, record} <- Ash.Changeset.apply_attributes(changeset),
          record <- unload_relationships(resource, record),
-         {:ok, _} <- ETS.Set.put(table, {pkey, record}) do
+         {:ok, _} <-
+           put_or_insert_new(table, {pkey, record}, opts) do
       {:ok, record}
     else
       {:error, error} -> {:error, error}
     end
   end
 
+  defp put_or_insert_new(table, {pkey, record}, opts) do
+    if opts[:upsert?] do
+      ETS.Set.put(table, {pkey, record})
+    else
+      ETS.Set.put_new(table, {pkey, record})
+    end
+  end
+
   @impl true
   def destroy(resource, %{data: record} = changeset) do
+    do_destroy(resource, record, changeset.tenant)
+  end
+
+  defp do_destroy(resource, record, tenant) do
     pkey = Map.take(record, Ash.Resource.Info.primary_key(resource))
 
-    with {:ok, table} <- wrap_or_create_table(resource, changeset.tenant),
+    with {:ok, table} <- wrap_or_create_table(resource, tenant),
          {:ok, _} <- ETS.Set.delete(table, pkey) do
       :ok
     else
@@ -372,7 +426,7 @@ defmodule Ash.DataLayer.Ets do
 
   @impl true
   def update(resource, changeset) do
-    create(resource, changeset)
+    create(resource, changeset, upsert?: true)
   end
 
   defp unload_relationships(resource, record) do

lib/ash/query/operator/basic.ex

@@ -46,6 +46,9 @@ defmodule Ash.Query.Operator.Basic do
           end
         end
 
+        defp do_evaluate(_, nil, _), do: :unknown
+        defp do_evaluate(_, _, nil), do: :unknown
+
         defp do_evaluate(:<>, left, right) do
           {:known, to_string(left) <> to_string(right)}
         end

lib/ash/query/operator/eq.ex

@@ -13,6 +13,9 @@ defmodule Ash.Query.Operator.Eq do
     predicate?: true,
     types: [:same, :any]
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Comp.equal?(left, right)}
   end

lib/ash/query/operator/greater_than.ex

@@ -10,6 +10,9 @@ defmodule Ash.Query.Operator.GreaterThan do
     predicate?: true,
     types: [:same, :any]
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Comp.greater_than?(left, right)}
   end

lib/ash/query/operator/greater_than_or_equal.ex

@@ -10,6 +10,9 @@ defmodule Ash.Query.Operator.GreaterThanOrEqual do
     predicate?: true,
     types: [:same, :any]
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}),
     do: {:known, Comp.greater_or_equal?(left, right)}
 

lib/ash/query/operator/in.ex

@@ -21,6 +21,9 @@ defmodule Ash.Query.Operator.In do
 
   def new(left, right), do: {:ok, %__MODULE__{left: left, right: right}}
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Enum.any?(right, &Comp.equal?(&1, left))}
   end

lib/ash/query/operator/is_nil.ex

@@ -30,6 +30,8 @@ defmodule Ash.Query.Operator.IsNil do
     end
   end
 
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: is_nil?}) do
     {:known, is_nil(left) == is_nil?}
   end

lib/ash/query/operator/less_than.ex

@@ -21,6 +21,9 @@ defmodule Ash.Query.Operator.LessThan do
 
   alias Ash.Query.Operator.{Eq, IsNil}
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Comp.less_than?(left, right)}
   end

lib/ash/query/operator/less_than_or_equal.ex

@@ -10,6 +10,9 @@ defmodule Ash.Query.Operator.LessThanOrEqual do
     predicate?: true,
     types: [:same, :any]
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Comp.less_or_equal?(left, right)}
   end

lib/ash/query/operator/not_eq.ex

@@ -13,6 +13,9 @@ defmodule Ash.Query.Operator.NotEq do
   alias Ash.Query.Not
   alias Ash.Query.Operator.Eq
 
+  def evaluate(%{left: nil}), do: :unknown
+  def evaluate(%{right: nil}), do: :unknown
+
   def evaluate(%{left: left, right: right}) do
     {:known, Comp.not_equal?(left, right)}
   end

lib/ash/resource/dsl.ex

@@ -628,6 +628,7 @@ defmodule Ash.Resource.Dsl do
     ],
     target: Ash.Resource.Identity,
     schema: Ash.Resource.Identity.schema(),
+    modules: [:pre_check_with, :eager_check_with],
     args: [:name, :keys]
   }
 
@@ -1077,7 +1078,8 @@ defmodule Ash.Resource.Dsl do
     Ash.Resource.Transformers.DefaultAccept,
     Ash.Resource.Transformers.SetTypes,
     Ash.Resource.Transformers.RequireUniqueFieldNames,
-    Ash.Resource.Transformers.ValidateRelationshipAttributes
+    Ash.Resource.Transformers.ValidateRelationshipAttributes,
+    Ash.Resource.Transformers.ValidateEagerIdentities
   ]
 
   @moduledoc """

lib/ash/resource/identity.ex

@@ -1,6 +1,15 @@
 defmodule Ash.Resource.Identity do
-  @moduledoc "Represents a unique constraint on a resource"
-  defstruct [:name, :keys, :description, :message]
+  @moduledoc """
+  Represents a unique constraint on a resource
+
+  Data layers should (and all built in ones do), discount `nil` or `null` (in the case of postgres) values
+  when determining if a unique constraint matches. This often means that you should
+  prefer to use identities with non-nullable columns.
+
+  Eventually, features could be added to support including `nil` or `null` values, but they would
+  need to include a corresponding feature for data layers.
+  """
+  defstruct [:name, :keys, :description, :message, :eager_check_with, :pre_check_with]
 
   @schema [
     name: [
@@ -15,6 +24,33 @@ defmodule Ash.Resource.Identity do
       doc:
         "The names of attributes, aggregates or calculations that uniquely identify this resource."
     ],
+    eager_check_with: [
+      type: {:behaviour, Ash.Api},
+      doc: """
+      Validates that the unique identity provided is unique at validation time, using the api module provided.
+
+      The identity is checked on each validation of the changeset. For example, if you are using
+      `AshPhoenix.Form`, this looks for a conflicting record on each call to `Form.validate/2`.
+      For updates, it is only checked if one of the involved fields is being changed.
+
+      For creates, The identity is checked unless your are performing an `upsert`, and the
+      `upsert_identity` is this identity. Keep in mind that for this to work properly, you will need
+      to pass the `upsert?: true, upsert_identity: :identity_name` *when creating the changeset* instead of
+      passing it to the Api when creating.
+
+      The `primary?` action is used to search for a record. This will error if you have not
+      configured one.
+      """
+    ],
+    pre_check_with: [
+      type: {:behaviour, Ash.Api},
+      doc: """
+      Validates that the unique identity provided is unique *just prior* to enacting the resource action, using the Api provided.
+
+      Behaves the same as `eager_check?`, but it runs just prior to the action being committed. Useful for
+      data layers that don't support transactions/unique constraints, or manual resources with identities.
+      """
+    ],
     description: [
       type: :string,
       doc: "An optional description for the identity"

lib/ash/resource/transformers/validate_eager_identities.ex

@@ -0,0 +1,79 @@
+defmodule Ash.Resource.Transformers.ValidateEagerIdentities do
+  @moduledoc """
+  Confirms that eager identities are not declared on a resource with no primary read.
+  """
+  use Ash.Dsl.Transformer
+
+  alias Ash.Dsl.Transformer
+  alias Ash.Error.Dsl.DslError
+
+  def after_compile?, do: true
+
+  def transform(resource, dsl_state) do
+    primary_read =
+      dsl_state
+      |> Transformer.get_entities([:actions])
+      |> Enum.find(&(&1.type == :read && &1.primary?))
+
+    dsl_state
+    |> Transformer.get_entities([:identities])
+    |> Enum.filter(&(&1.eager_check_with || &1.pre_check_with))
+    |> case do
+      [] ->
+        {:ok, dsl_state}
+
+      eager ->
+        if primary_read do
+          non_attributes =
+            Enum.filter(eager, fn identity ->
+              Enum.any?(identity.keys, &(!Ash.Resource.Info.attribute(resource, &1)))
+            end)
+
+          case non_attributes do
+            [] ->
+              {:ok, dsl_state}
+
+            [identity] ->
+              {:error,
+               DslError.exception(
+                 module: resource,
+                 path: [:identities, identity.name],
+                 message:
+                   "Identity #{identity.name} is declared with `eager_check?` but not all of the `keys` are attributes."
+               )}
+
+            identities ->
+              {:error,
+               DslError.exception(
+                 module: resource,
+                 path: [:identities],
+                 message:
+                   "Identities #{Enum.map_join(identities, ",", & &1.name)} are declared with `eager_check?` but not all of the `keys` are attributes."
+               )}
+          end
+        else
+          names = Enum.map(eager, & &1.name)
+
+          case names do
+            [name] ->
+              {:error,
+               DslError.exception(
+                 module: resource,
+                 path: [:identities, name],
+                 message:
+                   "Identity #{name} is declared with `eager_check?` but the resource has no primary read action."
+               )}
+
+            names ->
+              {:error,
+               DslError.exception(
+                 module: resource,
+                 path: [:identities],
+                 message:
+                   "Identities #{Enum.join(names, ",")} are declared with `eager_check?` but the resource has no primary read action."
+               )}
+          end
+        end
+    end
+  end
+end

test/actions/create_test.exs

@@ -282,6 +282,11 @@ defmodule Ash.Test.Actions.CreateTest do
     end
   end
 
+  describe "upserts" do
+    test "allows upserting a record using an identity" do
+    end
+  end
+
   describe "simple creates" do
     test "allows creating a record with valid attributes" do
       assert %Post{title: "foo", contents: "bar"} =

test/actions/identity_test.exs

@@ -0,0 +1,87 @@
+defmodule Ash.Test.Actions.IdentityTest do
+  @moduledoc false
+  use ExUnit.Case, async: true
+
+  defmodule Post do
+    @moduledoc false
+    use Ash.Resource, data_layer: Ash.DataLayer.Ets
+
+    ets do
+      private?(true)
+    end
+
+    identities do
+      identity :unique_title, [:title] do
+        eager_check_with(Ash.Test.Actions.IdentityTest.Api)
+      end
+
+      identity :unique_url, [:url] do
+        pre_check_with(Ash.Test.Actions.IdentityTest.Api)
+      end
+    end
+
+    actions do
+      defaults [:create, :read, :update, :destroy]
+
+      create :create_with_required do
+        require_attributes [:tag]
+      end
+    end
+
+    attributes do
+      uuid_primary_key :id
+      attribute(:title, :string, allow_nil?: false)
+      attribute(:url, :string)
+    end
+  end
+
+  defmodule Registry do
+    @moduledoc false
+    use Ash.Registry
+
+    entries do
+      entry(Post)
+    end
+  end
+
+  defmodule Api do
+    @moduledoc false
+    use Ash.Api
+
+    resources do
+      registry Registry
+    end
+  end
+
+  describe "eager_check?" do
+    test "will check for an identity mismatch at validation" do
+      Post
+      |> Ash.Changeset.for_create(:create, %{title: "fred"}, api: Api)
+      |> Api.create!()
+
+      assert %{
+               valid?: false,
+               errors: [
+                 %Ash.Error.Changes.InvalidChanges{
+                   fields: [:title],
+                   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"}, api: Api)
+      |> Api.create!()
+
+      assert_raise Ash.Error.Invalid, ~r/url: has already been taken/, fn ->
+        Post
+        |> Ash.Changeset.for_create(:create, %{title: "george", url: "google.com"})
+        |> Api.create!()
+      end
+    end
+  end
+end

test/ash/data_layer/ets_test.exs

@@ -31,6 +31,10 @@ defmodule Ash.DataLayer.EtsTest do
       destroy(:destroy)
     end
 
+    identities do
+      identity :unique_name, [:name]
+    end
+
     attributes do
       uuid_primary_key :id, writable?: true
       attribute :name, :string

test/resource/identities_test.exs

@@ -32,6 +32,18 @@ defmodule Ash.Test.Resource.IdentitiesTest do
                Ash.Resource.Info.identities(Post)
     end
 
+    test "eager_check? requires a primary read action" do
+      defposts do
+        identities do
+          identity :foobar, [:name], eager_check?: true
+        end
+
+        actions do
+          defaults [:read]
+        end
+      end
+    end
+
     test "Identity descriptions are allowed" do
       defposts do
         identities do