From 09e8b2933e9cf7a49e07df3999d8dee336489c87 Mon Sep 17 00:00:00 2001 From: Lucas Mendelowski Date: Thu, 6 Feb 2025 21:20:21 +0100 Subject: [PATCH 1/4] Add support for field names in idenitity constraints --- lib/data_layer.ex | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 274919e3..eb96c216 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -2310,16 +2310,18 @@ defmodule AshPostgres.DataLayer do %Postgrex.Error{} = error, stacktrace, {:bulk_create, fake_changeset}, - _resource + resource ) do case Ecto.Adapters.Postgres.Connection.to_constraints(error, []) do [] -> {:error, Ash.Error.to_ash_error(error, stacktrace)} constraints -> + identities = Ash.Resource.Info.identities(resource) + {:error, fake_changeset - |> constraints_to_errors(:insert, constraints) + |> constraints_to_errors(:insert, constraints, identities) |> Ash.Error.to_ash_error()} end end @@ -2372,7 +2374,7 @@ defmodule AshPostgres.DataLayer do {:error, Ash.Error.to_ash_error(error, stacktrace)} end - defp constraints_to_errors(%{constraints: user_constraints} = changeset, action, constraints) do + defp constraints_to_errors(%{constraints: user_constraints} = changeset, action, constraints, identities) do Enum.map(constraints, fn {type, constraint} -> user_constraint = Enum.find(user_constraints, fn c -> @@ -2387,14 +2389,22 @@ defmodule AshPostgres.DataLayer do case user_constraint do %{field: field, error_message: error_message, type: type, constraint: constraint} -> - Ash.Error.Changes.InvalidAttribute.exception( - field: field, - message: error_message, - private_vars: [ - constraint: constraint, - constraint_type: type - ] - ) + identity = Enum.find(identities, fn identity -> + String.contains?(constraint, to_string(identity.name)) + end) + + field_names = if identity, do: identity.field_names, else: [field] + + Enum.map(field_names, fn field_name -> + Ash.Error.Changes.InvalidAttribute.exception( + field: field_name, + message: error_message, + private_vars: [ + constraint: constraint, + constraint_type: type + ] + ) + end) nil -> Ecto.ConstraintError.exception( From 69c0099912aa546ef79d5cf1d2d55e48240ced4c Mon Sep 17 00:00:00 2001 From: Lucas Mendelowski Date: Sun, 9 Feb 2025 20:29:31 +0100 Subject: [PATCH 2/4] Update logic and try to add tests --- lib/data_layer.ex | 11 ++++++----- mix.exs | 2 +- test/support/resources/post.ex | 1 + 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/data_layer.ex b/lib/data_layer.ex index eb96c216..a073e5ea 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -2317,11 +2317,9 @@ defmodule AshPostgres.DataLayer do {:error, Ash.Error.to_ash_error(error, stacktrace)} constraints -> - identities = Ash.Resource.Info.identities(resource) - {:error, fake_changeset - |> constraints_to_errors(:insert, constraints, identities) + |> constraints_to_errors(:insert, constraints, resource) |> Ash.Error.to_ash_error()} end end @@ -2374,7 +2372,7 @@ defmodule AshPostgres.DataLayer do {:error, Ash.Error.to_ash_error(error, stacktrace)} end - defp constraints_to_errors(%{constraints: user_constraints} = changeset, action, constraints, identities) do + defp constraints_to_errors(%{constraints: user_constraints} = changeset, action, constraints, resource) do Enum.map(constraints, fn {type, constraint} -> user_constraint = Enum.find(user_constraints, fn c -> @@ -2389,8 +2387,11 @@ defmodule AshPostgres.DataLayer do case user_constraint do %{field: field, error_message: error_message, type: type, constraint: constraint} -> + identities = Ash.Resource.Info.identities(resource) + table = AshPostgres.DataLayer.Info.table(resource) + identity = Enum.find(identities, fn identity -> - String.contains?(constraint, to_string(identity.name)) + "#{table}_#{identity.name}_index" == constraint end) field_names = if identity, do: identity.field_names, else: [field] diff --git a/mix.exs b/mix.exs index bb70edca..bac982d9 100644 --- a/mix.exs +++ b/mix.exs @@ -197,7 +197,7 @@ defmodule AshPostgres.MixProject do [path: "../ash", override: true] "main" -> - [git: "https://github.com/ash-project/ash.git"] + [git: "https://github.com/ash-project/ash.git", override: true] version when is_binary(version) -> "~> #{version}" diff --git a/test/support/resources/post.ex b/test/support/resources/post.ex index 3fb6e103..dbabaee2 100644 --- a/test/support/resources/post.ex +++ b/test/support/resources/post.ex @@ -379,6 +379,7 @@ defmodule AshPostgres.Test.Post do identities do identity(:uniq_one_and_two, [:uniq_one, :uniq_two]) identity(:uniq_on_upper, [:upper_thing]) + identity(:uniq_on_upper_title, [:upper_title], field_names: [:title]) identity(:uniq_if_contains_foo, [:uniq_if_contains_foo]) do where expr(contains(uniq_if_contains_foo, "foo")) From 034428488fa31bcb8d5c77ba82a225972f06a54a Mon Sep 17 00:00:00 2001 From: Lucas Mendelowski Date: Mon, 10 Feb 2025 20:14:28 +0100 Subject: [PATCH 3/4] Add tests --- .../test_repo/orgs/20250210191116.json | 64 +++++++++++++++++++ .../20250210191116_migrate_resources49.exs | 25 ++++++++ test/support/resources/organization.ex | 11 ++++ test/support/resources/post.ex | 1 - test/unique_identity_test.exs | 14 ++++ 5 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 priv/resource_snapshots/test_repo/orgs/20250210191116.json create mode 100644 priv/test_repo/migrations/20250210191116_migrate_resources49.exs diff --git a/priv/resource_snapshots/test_repo/orgs/20250210191116.json b/priv/resource_snapshots/test_repo/orgs/20250210191116.json new file mode 100644 index 00000000..61cc82a1 --- /dev/null +++ b/priv/resource_snapshots/test_repo/orgs/20250210191116.json @@ -0,0 +1,64 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "primary_key?": true, + "references": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "name", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "department", + "type": "text" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "1D1BA9E1E272238D80C9861CAA67C4A85F675E3B052A15F4D5AC272551B820A7", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "orgs_department_index", + "keys": [ + { + "type": "string", + "value": "(LOWER(department))" + } + ], + "name": "department", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.AshPostgres.TestRepo", + "schema": null, + "table": "orgs" +} \ No newline at end of file diff --git a/priv/test_repo/migrations/20250210191116_migrate_resources49.exs b/priv/test_repo/migrations/20250210191116_migrate_resources49.exs new file mode 100644 index 00000000..622dddc6 --- /dev/null +++ b/priv/test_repo/migrations/20250210191116_migrate_resources49.exs @@ -0,0 +1,25 @@ +defmodule AshPostgres.TestRepo.Migrations.MigrateResources49 do + @moduledoc """ + Updates resources based on their most recent snapshots. + + This file was autogenerated with `mix ash_postgres.generate_migrations` + """ + + use Ecto.Migration + + def up do + alter table(:orgs) do + add(:department, :text) + end + + create(unique_index(:orgs, ["(LOWER(department))"], name: "orgs_department_index")) + end + + def down do + drop_if_exists(unique_index(:orgs, ["(LOWER(department))"], name: "orgs_department_index")) + + alter table(:orgs) do + remove(:department) + end + end +end diff --git a/test/support/resources/organization.ex b/test/support/resources/organization.ex index d5a22d65..0013f909 100644 --- a/test/support/resources/organization.ex +++ b/test/support/resources/organization.ex @@ -10,6 +10,8 @@ defmodule AshPostgres.Test.Organization do postgres do table("orgs") repo(AshPostgres.TestRepo) + + calculations_to_sql(lower_department: "LOWER(department)") end policies do @@ -39,6 +41,15 @@ defmodule AshPostgres.Test.Organization do attributes do uuid_primary_key(:id, writable?: true) attribute(:name, :string, public?: true) + attribute(:department, :string, public?: true) + end + + calculations do + calculate(:lower_department, :string, expr(fragment("LOWER(?)", department))) + end + + identities do + identity(:department, [:lower_department], field_names: [:department_slug]) end relationships do diff --git a/test/support/resources/post.ex b/test/support/resources/post.ex index dbabaee2..3fb6e103 100644 --- a/test/support/resources/post.ex +++ b/test/support/resources/post.ex @@ -379,7 +379,6 @@ defmodule AshPostgres.Test.Post do identities do identity(:uniq_one_and_two, [:uniq_one, :uniq_two]) identity(:uniq_on_upper, [:upper_thing]) - identity(:uniq_on_upper_title, [:upper_title], field_names: [:title]) identity(:uniq_if_contains_foo, [:uniq_if_contains_foo]) do where expr(contains(uniq_if_contains_foo, "foo")) diff --git a/test/unique_identity_test.exs b/test/unique_identity_test.exs index e210ac77..8fae1b98 100644 --- a/test/unique_identity_test.exs +++ b/test/unique_identity_test.exs @@ -1,6 +1,7 @@ defmodule AshPostgres.Test.UniqueIdentityTest do use AshPostgres.RepoCase, async: false alias AshPostgres.Test.Post + alias AshPostgres.Test.Organization require Ash.Query @@ -19,6 +20,19 @@ defmodule AshPostgres.Test.UniqueIdentityTest do end end + test "unique constraint field names are property set" do + Organization + |> Ash.Changeset.for_create(:create, %{name: "Acme", department: "Sales"}) + |> Ash.create!() + + assert {:error, %Ash.Error.Invalid{errors: [invalid_attribute]}} = + Organization + |> Ash.Changeset.for_create(:create, %{name: "Acme", department: "SALES"}) + |> Ash.create() + + assert %Ash.Error.Changes.InvalidAttribute{field: :department_slug} = invalid_attribute + end + test "a unique constraint can be used to upsert when the resource has a base filter" do post = Post From 8d2887313dbb8640b69f9c9849e5d5b6eec84303 Mon Sep 17 00:00:00 2001 From: Lucas Mendelowski Date: Mon, 17 Feb 2025 16:23:30 +0100 Subject: [PATCH 4/4] Update version requirement for Ash --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index bac982d9..b814f9a5 100644 --- a/mix.exs +++ b/mix.exs @@ -165,7 +165,7 @@ defmodule AshPostgres.MixProject do # Run "mix help deps" to learn about dependencies. defp deps do [ - {:ash, ash_version("~> 3.4 and >= 3.4.48")}, + {:ash, ash_version("~> 3.4 and >= 3.4.64")}, {:ash_sql, ash_sql_version("~> 0.2 and >= 0.2.43")}, {:igniter, "~> 0.5 and >= 0.5.16", optional: true}, {:ecto_sql, "~> 3.12"},