fix: ensure that context multitenancy is properly applied to lateral many-to-many joins

Author: zachdaniel

.formatter.exs

@@ -19,6 +19,7 @@ spark_locals_without_parens = [
   include: 1,
   index: 1,
   index: 2,
+  index?: 1,
   match_type: 1,
   match_with: 1,
   message: 1,

documentation/dsls/DSL:-AshPostgres.DataLayer.md

@@ -301,10 +301,11 @@ reference :post, on_delete: :delete, on_update: :update, name: "comments_to_post
 | [`ignore?`](#postgres-references-reference-ignore?){: #postgres-references-reference-ignore? } | `boolean` |  | If set to true, no reference is created for the given relationship. This is useful if you need to define it in some custom way |
 | [`on_delete`](#postgres-references-reference-on_delete){: #postgres-references-reference-on_delete } | `:delete \| :nilify \| :nothing \| :restrict \| {:nilify, atom \| list(atom)}` |  | What should happen to records of this resource when the referenced record of the *destination* resource is deleted. |
 | [`on_update`](#postgres-references-reference-on_update){: #postgres-references-reference-on_update } | `:update \| :nilify \| :nothing \| :restrict` |  | What should happen to records of this resource when the referenced destination_attribute of the *destination* record is update. |
-| [`deferrable`](#postgres-references-reference-deferrable){: #postgres-references-reference-deferrable } | `false \| true \| :initially` | `false` | Wether or not the constraint is deferrable. This only affects the migration generator. |
+| [`deferrable`](#postgres-references-reference-deferrable){: #postgres-references-reference-deferrable } | `false \| true \| :initially` | `false` | Whether or not the constraint is deferrable. This only affects the migration generator. |
 | [`name`](#postgres-references-reference-name){: #postgres-references-reference-name } | `String.t` |  | The name of the foreign key to generate in the database. Defaults to <table>_<source_attribute>_fkey |
 | [`match_with`](#postgres-references-reference-match_with){: #postgres-references-reference-match_with } | `keyword` |  | Defines additional keys to the foreign key in order to build a composite foreign key. The key should be the name of the source attribute (in the current resource), the value the name of the destination attribute. |
 | [`match_type`](#postgres-references-reference-match_type){: #postgres-references-reference-match_type } | `:simple \| :partial \| :full` |  | select if the match is `:simple`, `:partial`, or `:full` |
+| [`index?`](#postgres-references-reference-index?){: #postgres-references-reference-index? } | `boolean` | `false` | Whether to create or not a corresponding index |
 
 
 

lib/data_layer.ex

@@ -1270,11 +1270,17 @@ defmodule AshPostgres.DataLayer do
   end
 
   defp lateral_join_source_query(
-         %{__ash_bindings__: %{lateral_join_source_query: lateral_join_source_query}},
-         _
+         %{
+           __ash_bindings__: %{
+             lateral_join_source_query: lateral_join_source_query
+           }
+         },
+         source_query
        )
        when not is_nil(lateral_join_source_query) do
-    {:ok, lateral_join_source_query}
+    {:ok,
+     lateral_join_source_query
+     |> set_subquery_prefix(source_query, lateral_join_source_query.__ash_bindings__.resource)}
   end
 
   defp lateral_join_source_query(query, source_query) do

lib/migration_generator/migration_generator.ex

@@ -1777,8 +1777,10 @@ defmodule AshPostgres.MigrationGenerator do
             attribute.source == old_attribute.source
           end)
 
-        has_removed_index? = attribute && !attribute[:index?] && old_attribute[:index?]
-        attribute_doesnt_exist? = !attribute && old_attribute[:index?]
+        has_removed_index? =
+          attribute && !attribute[:references][:index?] && old_attribute[:references][:index?]
+
+        attribute_doesnt_exist? = !attribute && old_attribute[:references][:index?]
 
         has_removed_index? || attribute_doesnt_exist?
       end)
@@ -3173,6 +3175,7 @@ defmodule AshPostgres.MigrationGenerator do
         |> Map.put_new(:on_update, nil)
         |> Map.update!(:on_delete, &(&1 && load_references_on_delete(&1)))
         |> Map.update!(:on_update, &(&1 && maybe_to_atom(&1)))
+        |> Map.put_new(:index?, false)
         |> Map.put_new(:match_with, nil)
         |> Map.put_new(:match_type, nil)
         |> Map.update!(

priv/resource_snapshots/test_repo/tenants/friend_links/20240610162043.json

@@ -0,0 +1,77 @@
+{
+  "attributes": [
+    {
+      "default": "nil",
+      "size": null,
+      "type": "uuid",
+      "source": "source_id",
+      "references": {
+        "name": "friend_links_source_id_fkey",
+        "table": "multitenant_posts",
+        "multitenancy": {
+          "global": false,
+          "attribute": null,
+          "strategy": "context"
+        },
+        "destination_attribute": "id",
+        "primary_key?": true,
+        "schema": "public",
+        "on_delete": null,
+        "on_update": null,
+        "deferrable": false,
+        "match_with": null,
+        "match_type": null,
+        "index?": false,
+        "destination_attribute_default": null,
+        "destination_attribute_generated": null
+      },
+      "primary_key?": true,
+      "allow_nil?": false,
+      "generated?": false
+    },
+    {
+      "default": "nil",
+      "size": null,
+      "type": "uuid",
+      "source": "dest_id",
+      "references": {
+        "name": "friend_links_dest_id_fkey",
+        "table": "multitenant_posts",
+        "multitenancy": {
+          "global": false,
+          "attribute": null,
+          "strategy": "context"
+        },
+        "destination_attribute": "id",
+        "primary_key?": true,
+        "schema": "public",
+        "on_delete": null,
+        "on_update": null,
+        "deferrable": false,
+        "match_with": null,
+        "match_type": null,
+        "index?": false,
+        "destination_attribute_default": null,
+        "destination_attribute_generated": null
+      },
+      "primary_key?": true,
+      "allow_nil?": false,
+      "generated?": false
+    }
+  ],
+  "table": "friend_links",
+  "hash": "880BED202EB36FA2543D5DCC25DE1373676CE38CECFA2C8B5651757FFF3817EF",
+  "repo": "Elixir.AshPostgres.TestRepo",
+  "multitenancy": {
+    "global": false,
+    "attribute": null,
+    "strategy": "context"
+  },
+  "schema": null,
+  "check_constraints": [],
+  "identities": [],
+  "custom_indexes": [],
+  "base_filter": null,
+  "custom_statements": [],
+  "has_create_action": true
+}

priv/test_repo/tenant_migrations/20240610162043_migrate_resources3.exs

@@ -0,0 +1,45 @@
+defmodule AshPostgres.TestRepo.TenantMigrations.MigrateResources3 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
+    create table(:friend_links, primary_key: false, prefix: prefix()) do
+      add(
+        :source_id,
+        references(:multitenant_posts,
+          column: :id,
+          name: "friend_links_source_id_fkey",
+          type: :uuid,
+          prefix: prefix()
+        ),
+        primary_key: true,
+        null: false
+      )
+
+      add(
+        :dest_id,
+        references(:multitenant_posts,
+          column: :id,
+          name: "friend_links_dest_id_fkey",
+          type: :uuid,
+          prefix: prefix()
+        ),
+        primary_key: true,
+        null: false
+      )
+    end
+  end
+
+  def down do
+    drop(constraint(:friend_links, "friend_links_source_id_fkey"))
+
+    drop(constraint(:friend_links, "friend_links_dest_id_fkey"))
+
+    drop(table(:friend_links, prefix: prefix()))
+  end
+end

test/multitenancy_test.exs

@@ -109,23 +109,74 @@ defmodule AshPostgres.Test.MultitenancyTest do
       |> Ash.Changeset.new()
       |> Ash.create!()
 
-    user1 =
+    user =
       User
-      |> Ash.Changeset.for_create(:create, %{name: "a"})
+      |> Ash.Changeset.for_create(:create, %{name: "a"}, tenant: "org_#{org.id}")
       |> Ash.Changeset.manage_relationship(:org, org, type: :append_and_remove)
       |> Ash.create!()
 
     user2 =
       User
-      |> Ash.Changeset.for_create(:create, %{name: "b"})
+      |> Ash.Changeset.for_create(:create, %{name: "a"}, tenant: "org_#{org.id}")
       |> Ash.Changeset.manage_relationship(:org, org, type: :append_and_remove)
       |> Ash.create!()
 
-    user1_id = user1.id
-    user2_id = user2.id
+    post =
+      Post
+      |> Ash.Changeset.for_create(:create, %{name: "foobar"},
+        authorize?: false,
+        tenant: "org_#{org.id}"
+      )
+      |> Ash.Changeset.manage_relationship(:user, user, type: :append_and_remove)
+      |> Ash.create!()
 
-    assert [%{id: ^user1_id}, %{id: ^user2_id}] =
-             Ash.load!(org, users: Ash.Query.sort(User, :name)).users
+    post_id = post.id
+
+    assert [%{posts: [%{id: ^post_id}]}, _] =
+             Ash.load!([user, user2], [posts: Ash.Query.limit(Post, 2)],
+               tenant: "org_#{org.id}",
+               authorize?: false
+             )
+  end
+
+  test "loading context multitenant resources across a many-to-many with a limit works" do
+    org =
+      Org
+      |> Ash.Changeset.new()
+      |> Ash.create!()
+
+    user =
+      User
+      |> Ash.Changeset.for_create(:create, %{name: "a"}, tenant: "org_#{org.id}")
+      |> Ash.Changeset.manage_relationship(:org, org, type: :append_and_remove)
+      |> Ash.create!()
+
+    post =
+      Post
+      |> Ash.Changeset.for_create(:create, %{name: "foobar"},
+        authorize?: false,
+        tenant: "org_#{org.id}"
+      )
+      |> Ash.Changeset.manage_relationship(:user, user, type: :append_and_remove)
+      |> Ash.create!()
+
+    post2 =
+      Post
+      |> Ash.Changeset.for_create(:create, %{name: "foobar"},
+        authorize?: false,
+        tenant: "org_#{org.id}"
+      )
+      |> Ash.Changeset.manage_relationship(:user, user, type: :append_and_remove)
+      |> Ash.Changeset.manage_relationship(:linked_posts, post, type: :append_and_remove)
+      |> Ash.create!()
+
+    post_id = post.id
+
+    assert [%{linked_posts: [%{id: ^post_id}]}, _] =
+             Ash.load!([post2, post], [linked_posts: Ash.Query.limit(Post, 2)],
+               tenant: "org_#{org.id}",
+               authorize?: false
+             )
   end
 
   test "manage_relationship from context multitenant resource to attribute multitenant resource doesn't raise an error" do

test/support/multitenancy/domain.ex

@@ -6,5 +6,6 @@ defmodule AshPostgres.MultitenancyTest.Domain do
     resource(AshPostgres.MultitenancyTest.Org)
     resource(AshPostgres.MultitenancyTest.User)
     resource(AshPostgres.MultitenancyTest.Post)
+    resource(AshPostgres.MultitenancyTest.PostLink)
   end
 end

test/support/multitenancy/resources/post.ex

@@ -50,6 +50,12 @@ defmodule AshPostgres.MultitenancyTest.Post do
     end
 
     has_one(:self, __MODULE__, destination_attribute: :id, source_attribute: :id, public?: true)
+
+    many_to_many :linked_posts, __MODULE__ do
+      through(AshPostgres.MultitenancyTest.PostLink)
+      source_attribute_on_join_resource(:source_id)
+      destination_attribute_on_join_resource(:dest_id)
+    end
   end
 
   calculations do

test/support/multitenancy/resources/post_link.ex

@@ -0,0 +1,31 @@
+defmodule AshPostgres.MultitenancyTest.PostLink do
+  @moduledoc false
+  use Ash.Resource,
+    domain: AshPostgres.MultitenancyTest.Domain,
+    data_layer: AshPostgres.DataLayer
+
+  postgres do
+    table "friend_links"
+    repo AshPostgres.TestRepo
+  end
+
+  multitenancy do
+    strategy(:context)
+  end
+
+  actions do
+    defaults([:read, :destroy, create: :*, update: :*])
+  end
+
+  relationships do
+    belongs_to(:source, AshPostgres.MultitenancyTest.Post,
+      primary_key?: true,
+      allow_nil?: false
+    )
+
+    belongs_to(:dest, AshPostgres.MultitenancyTest.Post,
+      primary_key?: true,
+      allow_nil?: false
+    )
+  end
+end