improvement: support all_tenants? option for identities

improvement: support `all_tenants?` option for custom indexes
ash-project · Jan 12, 2024 · b93b1d7 · b93b1d7
1 parent 40c1a13
commit b93b1d7
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 15 deletions.
diff --git a/.formatter.exs b/.formatter.exs
@@ -1,4 +1,5 @@
 spark_locals_without_parens = [
+  all_tenants?: 1,
   base_filter_sql: 1,
   check: 1,
   check_constraint: 2,

diff --git a/documentation/dsls/DSL:-AshPostgres.DataLayer.md b/documentation/dsls/DSL:-AshPostgres.DataLayer.md
@@ -112,6 +112,7 @@ index ["column", "column2"], unique: true, where: "thing = TRUE"
 | [`where`](#postgres-custom_indexes-index-where){: #postgres-custom_indexes-index-where } | `String.t` |  | specify conditions for a partial index. |
 | [`message`](#postgres-custom_indexes-index-message){: #postgres-custom_indexes-index-message } | `String.t` |  | A custom message to use for unique indexes that have been violated |
 | [`include`](#postgres-custom_indexes-index-include){: #postgres-custom_indexes-index-include } | `list(String.t)` |  | specify fields for a covering index. This is not supported by all databases. For more information on PostgreSQL support, please read the official docs. |
+| [`all_tenants?`](#postgres-custom_indexes-index-all_tenants?){: #postgres-custom_indexes-index-all_tenants? } | `boolean` | `false` | Whether or not the index should factor in the multitenancy attribute or not. |
 
 
 

diff --git a/lib/custom_index.ex b/lib/custom_index.ex
@@ -10,7 +10,8 @@ defmodule AshPostgres.CustomIndex do
     :prefix,
     :where,
     :include,
-    :message
+    :message,
+    :all_tenants?
   ]
 
   defstruct @fields
@@ -56,6 +57,11 @@ defmodule AshPostgres.CustomIndex do
       type: {:list, :string},
       doc:
         "specify fields for a covering index. This is not supported by all databases. For more information on PostgreSQL support, please read the official docs."
+    ],
+    all_tenants?: [
+      type: :boolean,
+      default: false,
+      doc: "Whether or not the index should factor in the multitenancy attribute or not."
     ]
   ]
 

diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex
@@ -1697,7 +1697,7 @@ defmodule AshPostgres.MigrationGenerator do
 
     custom_indexes_to_remove =
       Enum.filter(old_snapshot.custom_indexes, fn old_custom_index ->
-        rewrite_all_identities? ||
+        (rewrite_all_identities? && !old_custom_index.all_tenants?) ||
           !Enum.find(snapshot.custom_indexes, fn index ->
             indexes_match?(snapshot.table, old_custom_index, index)
           end)
@@ -1714,13 +1714,14 @@ defmodule AshPostgres.MigrationGenerator do
 
     unique_indexes_to_remove =
       if rewrite_all_identities? do
-        old_snapshot.identities
+        Enum.reject(old_snapshot.identities, & &1.all_tenants?)
       else
         Enum.reject(old_snapshot.identities, fn old_identity ->
           Enum.find(snapshot.identities, fn identity ->
             identity.name == old_identity.name &&
               Enum.sort(old_identity.keys) == Enum.sort(identity.keys) &&
-              old_identity.base_filter == identity.base_filter
+              old_identity.base_filter == identity.base_filter &&
+              old_identity.all_tenants? == identity.all_tenants?
           end)
         end)
       end
@@ -1734,7 +1735,17 @@ defmodule AshPostgres.MigrationGenerator do
 
     unique_indexes_to_rename =
       if rewrite_all_identities? do
-        []
+        snapshot.identities
+        |> Enum.filter(& &1.all_tenants?)
+        |> Enum.map(fn identity ->
+          Enum.find_value(old_snapshot.identities, fn old_identity ->
+            if old_identity.name == identity.name &&
+                 old_identity.index_name != identity.index_name do
+              {old_identity, identity}
+            end
+          end)
+        end)
+        |> Enum.filter(& &1)
       else
         snapshot.identities
         |> Enum.map(fn identity ->
@@ -1759,12 +1770,25 @@ defmodule AshPostgres.MigrationGenerator do
     unique_indexes_to_add =
       if rewrite_all_identities? do
         snapshot.identities
+        |> Enum.reject(fn identity ->
+          if identity.all_tenants? do
+            Enum.find(old_snapshot.identities, fn old_identity ->
+              old_identity.name == identity.name &&
+                Enum.sort(old_identity.keys) == Enum.sort(identity.keys) &&
+                old_identity.base_filter == identity.base_filter &&
+                old_identity.all_tenants? == identity.all_tenants?
+            end)
+          else
+            false
+          end
+        end)
       else
         Enum.reject(snapshot.identities, fn identity ->
           Enum.find(old_snapshot.identities, fn old_identity ->
             old_identity.name == identity.name &&
               Enum.sort(old_identity.keys) == Enum.sort(identity.keys) &&
-              old_identity.base_filter == identity.base_filter
+              old_identity.base_filter == identity.base_filter &&
+              old_identity.all_tenants? == identity.all_tenants?
           end)
         end)
       end
@@ -2684,7 +2708,7 @@ defmodule AshPostgres.MigrationGenerator do
       end)
     end)
     |> Enum.sort_by(& &1.name)
-    |> Enum.map(&Map.take(&1, [:name, :keys]))
+    |> Enum.map(&Map.take(&1, [:name, :keys, :all_tenants?]))
     |> Enum.map(fn %{keys: keys} = identity ->
       %{
         identity
@@ -2864,6 +2888,7 @@ defmodule AshPostgres.MigrationGenerator do
       |> Map.put_new(:fields, [])
       |> Map.put_new(:include, [])
       |> Map.put_new(:message, nil)
+      |> Map.put_new(:all_tenants?, false)
     end)
   end
 
@@ -3010,6 +3035,7 @@ defmodule AshPostgres.MigrationGenerator do
     end)
     |> add_index_name(table)
     |> Map.put_new(:base_filter, nil)
+    |> Map.put_new(:all_tenants?, false)
   end
 
   defp add_index_name(%{name: name} = index, table) do

diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex
@@ -769,18 +769,28 @@ defmodule AshPostgres.MigrationGenerator.Operation do
     import Helper
 
     def up(%{
-          identity: %{name: name, keys: keys, base_filter: base_filter, index_name: index_name},
+          identity: %{
+            name: name,
+            keys: keys,
+            base_filter: base_filter,
+            index_name: index_name,
+            all_tenants?: all_tenants?
+          },
           table: table,
           schema: schema,
           multitenancy: multitenancy
         }) do
       keys =
-        case multitenancy.strategy do
-          :attribute ->
-            [multitenancy.attribute | keys]
+        if all_tenants? do
+          keys
+        else
+          case multitenancy.strategy do
+            :attribute ->
+              [multitenancy.attribute | keys]
 
-          _ ->
-            keys
+            _ ->
+              keys
+          end
         end
 
       index_name = index_name || "#{table}_#{name}_index"

diff --git a/mix.exs b/mix.exs
@@ -204,7 +204,7 @@ defmodule AshPostgres.MixProject do
       {:ecto, "~> 3.9"},
       {:jason, "~> 1.0"},
       {:postgrex, ">= 0.0.0"},
-      {:ash, ash_version("~> 2.17 and >= 2.17.24")},
+      {:ash, ash_version("~> 2.18")},
       {:benchee, "~> 1.1", only: [:dev, :test]},
       {:git_ops, "~> 2.5", only: [:dev, :test]},
       {:ex_doc, github: "elixir-lang/ex_doc", only: [:dev, :test], runtime: false},

diff --git a/mix.lock b/mix.lock
@@ -1,5 +1,5 @@
 %{
-  "ash": {:hex, :ash, "2.17.24", "af3785c966f6141b92bb4c9826ef46c60234b0f2f4081d978e6d871f787e88cc", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:spark, ">= 1.1.50 and < 2.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.6", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a2e8f472a900cbade23c1ec3add4ddd3e60fbff8dc4c9d6b2a184aad01ab919b"},
+  "ash": {:hex, :ash, "2.18.0", "498f3c1766d2343530035a3bae40dffb89eb721531c4b3375b51cdd175dfb4db", [:mix], [{:comparable, "~> 1.0", [hex: :comparable, repo: "hexpm", optional: false]}, {:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:earmark, "~> 1.4", [hex: :earmark, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: false]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:spark, ">= 1.1.50 and < 2.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:stream_data, "~> 0.6", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "edb61da47f0e98ea6cb3dd3e3e81a91bfb4c189afbc69c44e2c4b41dea8d1f6f"},
   "benchee": {:hex, :benchee, "1.1.0", "f3a43817209a92a1fade36ef36b86e1052627fd8934a8b937ac9ab3a76c43062", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}], "hexpm", "7da57d545003165a012b587077f6ba90b89210fd88074ce3c60ce239eb5e6d93"},
   "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"},
   "certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},

diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs
@@ -942,6 +942,57 @@ defmodule AshPostgres.MigrationGeneratorTest do
                ~S{references(:users, column: :secondary_id, with: [related_key_id: :key_id, org_id: :org_id], match: :full, name: "user_things_user_id_fkey", type: :uuid, prefix: "public")}
     end
 
+    test "identities using `all_tenants?: true` will not have the condition on multitenancy attribtue added" do
+      defresource Org, "orgs" do
+        attributes do
+          uuid_primary_key(:id, writable?: true)
+          attribute(:name, :string)
+        end
+
+        multitenancy do
+          strategy(:attribute)
+          attribute(:id)
+        end
+      end
+
+      defresource User, "users" do
+        attributes do
+          uuid_primary_key(:id, writable?: true)
+          attribute(:secondary_id, :uuid)
+          attribute(:name, :string)
+          attribute(:org_id, :uuid)
+          attribute(:key_id, :uuid)
+        end
+
+        multitenancy do
+          strategy(:attribute)
+          attribute(:org_id)
+        end
+
+        identities do
+          identity(:unique_name, [:name], all_tenants?: true)
+        end
+
+        relationships do
+          belongs_to(:org, Org)
+        end
+      end
+
+      defapi([Org, User])
+
+      AshPostgres.MigrationGenerator.generate(Api,
+        snapshot_path: "test_snapshots_path",
+        migration_path: "test_migration_path",
+        quiet: true,
+        format: false
+      )
+
+      assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")
+
+      assert File.read!(file) =~
+               ~S{create unique_index(:users, [:name], name: "users_unique_name_index")}
+    end
+
     test "when modified, the foreign key is dropped before modification" do
       defposts do
         attributes do