From f290ae47b83aa2c1148978327d970fe38ec5000b Mon Sep 17 00:00:00 2001 From: Alan Heywood Date: Sun, 29 Jan 2023 16:05:34 +1000 Subject: [PATCH] Add failing test for policy + aggregate issue The conditions for this issue to occur seem to be: - DataLayer is Postgres - Resource has a relates_to_actor_via policy on read - The relates_to_actor_via path includes a has_many relationship - An aggregate is loaded The following error is produced: 1) test relates to actor via has_many and with an aggregate (AshPostgres.AggregateTest) test/aggregate_test.exs:8 ** (Ash.Error.Unknown.UnknownError) ** (ArgumentError) No such entity nil found. code: |> Api.read_one!(actor: user) stacktrace: nil.spark_dsl_config() (spark 0.3.8) lib/spark/dsl/extension.ex:129: Spark.Dsl.Extension.dsl!/1 (spark 0.3.8) lib/spark/dsl/extension.ex:158: Spark.Dsl.Extension.get_persisted/3 (ash 2.5.10) lib/ash/filter/filter.ex:2986: Ash.Filter.do_hydrate_refs/2 (ash 2.5.10) lib/ash/policy/check/relates_to_actor_via.ex:3: Ash.Policy.Check.RelatesToActorVia.try_eval/2 (ash 2.5.10) lib/ash/policy/check/relates_to_actor_via.ex:3: Ash.Policy.Check.RelatesToActorVia.try_strict_check/3 (ash 2.5.10) lib/ash/policy/checker.ex:63: Ash.Policy.Checker.do_strict_check_facts/3 (ash 2.5.10) lib/ash/policy/checker.ex:88: anonymous fn/2 in Ash.Policy.Checker.strict_check_policies/3 (elixir 1.14.2) lib/enum.ex:4751: Enumerable.List.reduce/3 (elixir 1.14.2) lib/enum.ex:2514: Enum.reduce_while/3 (ash 2.5.10) lib/ash/policy/checker.ex:9: anonymous fn/2 in Ash.Policy.Checker.strict_check_facts/1 (elixir 1.14.2) lib/enum.ex:4751: Enumerable.List.reduce/3 (elixir 1.14.2) lib/enum.ex:2514: Enum.reduce_while/3 (ash 2.5.10) lib/ash/policy/authorizer.ex:790: Ash.Policy.Authorizer.do_strict_check_facts/1 (ash 2.5.10) lib/ash/policy/authorizer.ex:372: Ash.Policy.Authorizer.strict_check/2 (ash 2.5.10) lib/ash/engine/request.ex:550: Ash.Engine.Request.do_strict_check/3 (ash 2.5.10) lib/ash/engine/request.ex:518: anonymous fn/2 in Ash.Engine.Request.strict_check/2 (elixir 1.14.2) lib/enum.ex:4751: Enumerable.List.reduce/3 (elixir 1.14.2) lib/enum.ex:2514: Enum.reduce_while/3 (ash 2.5.10) lib/ash/engine/request.ex:255: Ash.Engine.Request.do_next/1 (ash 2.5.10) lib/ash/engine/request.ex:211: Ash.Engine.Request.next/1 (ash 2.5.10) lib/ash/engine/engine.ex:650: Ash.Engine.advance_request/2 (ash 2.5.10) lib/ash/engine/engine.ex:556: Ash.Engine.fully_advance_request/2 (ash 2.5.10) lib/ash/engine/engine.ex:497: Ash.Engine.do_run_iteration/2 (elixir 1.14.2) lib/enum.ex:2468: Enum."-reduce/3-lists^foldl/2-0-"/3 (ash 2.5.10) lib/ash/engine/engine.ex:440: Ash.Engine.run_iteration/1 (ash 2.5.10) lib/ash/engine/engine.ex:257: Ash.Engine.run_to_completion/1 (ash 2.5.10) lib/ash/engine/engine.ex:202: Ash.Engine.do_run/2 (ash 2.5.10) lib/ash/engine/engine.ex:141: Ash.Engine.run/2 (ash 2.5.10) lib/ash/actions/read.ex:170: Ash.Actions.Read.do_run/3 (ash 2.5.10) lib/ash/actions/read.ex:90: Ash.Actions.Read.run/3 (ash 2.5.10) lib/ash/api/api.ex:1005: Ash.Api.read_one/3 (ash 2.5.10) lib/ash/api/api.ex:998: Ash.Api.read_one!/3 test/aggregate_test.exs:44: (test) --- .../test_repo/orgs/20230129050950.json | 39 +++ .../test_repo/posts/20230129050950.json | 275 ++++++++++++++++++ .../test_repo/users/20230129050950.json | 97 ++++++ .../20230129050950_migrate_resources8.exs | 75 +++++ test/aggregate_test.exs | 44 ++- test/support/registry.ex | 1 + test/support/resources/organization.ex | 24 ++ test/support/resources/post.ex | 14 +- test/support/resources/user.ex | 1 + 9 files changed, 568 insertions(+), 2 deletions(-) create mode 100644 priv/resource_snapshots/test_repo/orgs/20230129050950.json create mode 100644 priv/resource_snapshots/test_repo/posts/20230129050950.json create mode 100644 priv/resource_snapshots/test_repo/users/20230129050950.json create mode 100644 priv/test_repo/migrations/20230129050950_migrate_resources8.exs create mode 100644 test/support/resources/organization.ex diff --git a/priv/resource_snapshots/test_repo/orgs/20230129050950.json b/priv/resource_snapshots/test_repo/orgs/20230129050950.json new file mode 100644 index 0000000..1e15b07 --- /dev/null +++ b/priv/resource_snapshots/test_repo/orgs/20230129050950.json @@ -0,0 +1,39 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"uuid_generate_v4()\")", + "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" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "B14556A2079B06D3ED1BF1D557B7FD1DA2D859BBB25B702352DD4D28680580D7", + "identities": [], + "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/resource_snapshots/test_repo/posts/20230129050950.json b/priv/resource_snapshots/test_repo/posts/20230129050950.json new file mode 100644 index 0000000..22366b6 --- /dev/null +++ b/priv/resource_snapshots/test_repo/posts/20230129050950.json @@ -0,0 +1,275 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"uuid_generate_v4()\")", + "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": "title", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "score", + "type": "bigint" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "public", + "type": "boolean" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "category", + "type": "citext" + }, + { + "allow_nil?": true, + "default": "\"sponsored\"", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "type", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "price", + "type": "bigint" + }, + { + "allow_nil?": true, + "default": "\"0\"", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "decimal", + "type": "decimal" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "status", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "status_enum", + "type": "status" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "point", + "type": [ + "array", + "float" + ] + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "uniq_one", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "uniq_two", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "uniq_custom_one", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "uniq_custom_two", + "type": "text" + }, + { + "allow_nil?": false, + "default": "fragment(\"now()\")", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "created_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "fragment(\"now()\")", + "generated?": false, + "primary_key?": false, + "references": null, + "size": null, + "source": "updated_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": { + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "posts_organization_id_fkey", + "on_delete": null, + "on_update": null, + "schema": "public", + "table": "orgs" + }, + "size": null, + "source": "organization_id", + "type": "uuid" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": { + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "posts_author_id_fkey", + "on_delete": null, + "on_update": null, + "schema": "public", + "table": "authors" + }, + "size": null, + "source": "author_id", + "type": "uuid" + } + ], + "base_filter": "type = 'sponsored'", + "check_constraints": [ + { + "attribute": [ + "price" + ], + "base_filter": "type = 'sponsored'", + "check": "price > 0", + "name": "price_must_be_positive" + } + ], + "custom_indexes": [ + { + "concurrently": true, + "fields": [ + "uniq_custom_one", + "uniq_custom_two" + ], + "include": null, + "message": "dude what the heck", + "name": null, + "prefix": null, + "table": null, + "unique": true, + "using": null, + "where": null + } + ], + "custom_statements": [], + "has_create_action": true, + "hash": "0009D3FF4056FBE84946C468F41E34B1B6B7ED9F65D64FD9F6B91D32F74A6AD8", + "identities": [ + { + "base_filter": "type = 'sponsored'", + "index_name": "posts_uniq_one_and_two_index", + "keys": [ + "uniq_one", + "uniq_two" + ], + "name": "uniq_one_and_two" + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.AshPostgres.TestRepo", + "schema": null, + "table": "posts" +} \ No newline at end of file diff --git a/priv/resource_snapshots/test_repo/users/20230129050950.json b/priv/resource_snapshots/test_repo/users/20230129050950.json new file mode 100644 index 0000000..868cec4 --- /dev/null +++ b/priv/resource_snapshots/test_repo/users/20230129050950.json @@ -0,0 +1,97 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"uuid_generate_v4()\")", + "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": "is_active", + "type": "boolean" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "primary_key?": false, + "references": { + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "users_organization_id_fkey", + "on_delete": null, + "on_update": null, + "schema": "public", + "table": "orgs" + }, + "size": null, + "source": "organization_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": { + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "multitenancy": { + "attribute": "id", + "global": true, + "strategy": "attribute" + }, + "name": "users_org_id_fkey", + "on_delete": null, + "on_update": null, + "schema": "public", + "table": "multitenant_orgs" + }, + "size": null, + "source": "org_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "D8E17F78D925AF35AC9D21FF6D606B82AB249A4E0A2BD6B6B83DD486A0F2264D", + "identities": [], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.AshPostgres.TestRepo", + "schema": null, + "table": "users" +} \ No newline at end of file diff --git a/priv/test_repo/migrations/20230129050950_migrate_resources8.exs b/priv/test_repo/migrations/20230129050950_migrate_resources8.exs new file mode 100644 index 0000000..4cfaa7d --- /dev/null +++ b/priv/test_repo/migrations/20230129050950_migrate_resources8.exs @@ -0,0 +1,75 @@ +defmodule AshPostgres.TestRepo.Migrations.MigrateResources8 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(:users) do + add :organization_id, :uuid + end + + alter table(:posts) do + add :organization_id, :uuid + end + + create table(:orgs, primary_key: false) do + add :id, :uuid, null: false, default: fragment("uuid_generate_v4()"), primary_key: true + end + + alter table(:users) do + modify :organization_id, + references(:orgs, + column: :id, + prefix: "public", + name: "users_organization_id_fkey", + type: :uuid + ) + end + + alter table(:posts) do + modify :organization_id, + references(:orgs, + column: :id, + prefix: "public", + name: "posts_organization_id_fkey", + type: :uuid + ) + end + + alter table(:orgs) do + add :name, :text + end + end + + def down do + alter table(:orgs) do + remove :name + end + + drop constraint(:posts, "posts_organization_id_fkey") + + alter table(:posts) do + modify :organization_id, :uuid + end + + drop constraint(:users, "users_organization_id_fkey") + + alter table(:users) do + modify :organization_id, :uuid + end + + drop table(:orgs) + + alter table(:posts) do + remove :organization_id + end + + alter table(:users) do + remove :organization_id + end + end +end \ No newline at end of file diff --git a/test/aggregate_test.exs b/test/aggregate_test.exs index 51348bb..2412eca 100644 --- a/test/aggregate_test.exs +++ b/test/aggregate_test.exs @@ -1,9 +1,51 @@ defmodule AshPostgres.AggregateTest do use AshPostgres.RepoCase, async: false - alias AshPostgres.Test.{Api, Comment, Post, Rating} + alias AshPostgres.Test.{Api, Comment, Post, Rating, Organization, User} require Ash.Query + test "relates to actor via has_many and with an aggregate" do + org = + Organization + |> Ash.Changeset.new(%{name: "The Org"}) + |> Api.create!() + + post = + Post + |> Ash.Changeset.for_create(:create, %{title: "title"}) + |> Ash.Changeset.manage_relationship(:organization, org, type: :append_and_remove) + |> Api.create!() + + user = + User + |> Ash.Changeset.for_create(:create, %{}) + |> Ash.Changeset.manage_relationship(:organization, org, type: :append_and_remove) + |> Api.create!() + + Comment + |> Ash.Changeset.new(%{title: "match"}) + |> Ash.Changeset.manage_relationship(:post, post, type: :append_and_remove) + |> Api.create!() + + read_post = + Post + |> Ash.Query.filter(id == ^post.id) + |> Api.read_one!(actor: user) + + # The policy works fine in this case and we can read the post, + # since the post and the actor are in the same org + assert read_post.id == post.id + + read_post = + Post + |> Ash.Query.filter(id == ^post.id) + |> Ash.Query.load(:count_of_comments) + |> Api.read_one!(actor: user) + + # Loading the :count_of_comments aggregate produces the error + assert read_post.count_of_comments == 1 + end + describe "count" do test "with no related data it returns 0" do post = diff --git a/test/support/registry.ex b/test/support/registry.ex index 8648891..5068bd1 100644 --- a/test/support/registry.ex +++ b/test/support/registry.ex @@ -12,5 +12,6 @@ defmodule AshPostgres.Test.Registry do entry(AshPostgres.Test.Profile) entry(AshPostgres.Test.User) entry(AshPostgres.Test.Account) + entry(AshPostgres.Test.Organization) end end diff --git a/test/support/resources/organization.ex b/test/support/resources/organization.ex new file mode 100644 index 0000000..caef07a --- /dev/null +++ b/test/support/resources/organization.ex @@ -0,0 +1,24 @@ +defmodule AshPostgres.Test.Organization do + @moduledoc false + use Ash.Resource, + data_layer: AshPostgres.DataLayer + + postgres do + table("orgs") + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:name, :string) + end + + relationships do + has_many(:users, AshPostgres.Test.User) + has_many(:posts, AshPostgres.Test.Post) + end +end diff --git a/test/support/resources/post.ex b/test/support/resources/post.ex index c4addec..d4dc176 100644 --- a/test/support/resources/post.ex +++ b/test/support/resources/post.ex @@ -1,7 +1,17 @@ defmodule AshPostgres.Test.Post do @moduledoc false use Ash.Resource, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [ + Ash.Policy.Authorizer + ] + + policies do + bypass action_type(:read) do + # Check that the post is in the same org as actor + authorize_if(relates_to_actor_via([:organization, :users])) + end + end postgres do table("posts") @@ -83,6 +93,8 @@ defmodule AshPostgres.Test.Post do end relationships do + belongs_to(:organization, AshPostgres.Test.Organization) + belongs_to(:author, AshPostgres.Test.Author) has_many(:comments, AshPostgres.Test.Comment, destination_attribute: :post_id) diff --git a/test/support/resources/user.ex b/test/support/resources/user.ex index b047c9b..bdb7f7a 100644 --- a/test/support/resources/user.ex +++ b/test/support/resources/user.ex @@ -17,6 +17,7 @@ defmodule AshPostgres.Test.User do end relationships do + belongs_to(:organization, AshPostgres.Test.Organization) has_many(:accounts, AshPostgres.Test.Account) end end