fix: deduplicate identity keys

fixes #602

Author: zachdaniel

lib/migration_generator/operation.ex

@@ -130,7 +130,7 @@ defmodule AshPostgres.MigrationGenerator.Operation do
 
     def index_keys(keys, all_tenants?, multitenancy) do
       if multitenancy.strategy == :attribute and not all_tenants? do
-        [multitenancy.attribute | keys]
+        Enum.uniq([multitenancy.attribute | keys])
       else
         keys
       end

mix.lock

@@ -1,6 +1,6 @@
 %{
   "ash": {:hex, :ash, "3.5.32", "ee717c49744374be7abe8997011115a4997917535e02d36146937fb6f89c19fe", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ets, "~> 0.8", [hex: :ets, repo: "hexpm", optional: false]}, {:igniter, ">= 0.6.4 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: false]}, {:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:plug, ">= 0.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:reactor, "~> 0.11", [hex: :reactor, repo: "hexpm", optional: false]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:spark, ">= 2.2.65 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, ">= 0.2.6 and < 1.0.0-0", [hex: :splode, repo: "hexpm", optional: false]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.1", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "4704f536682538ee3ae0e8f99ff3cef6af53df09ef3e7e550e202da9d5ce685c"},
-  "ash_sql": {:hex, :ash_sql, "0.2.87", "17197c643918cdaee657946a1998860402dcf53a980f7665bb81d1fa53c224e7", [:mix], [{:ash, ">= 3.5.25 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, "~> 3.9", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "f82d6bf78f08bd9040af3adc28676965421598c88866074d8b1ccca65978d774"},
+  "ash_sql": {:hex, :ash_sql, "0.2.89", "ad4ad497263b586a7f3949ceea5d44620a36cb99a1ef0ff5f58f13a77d9b99ef", [:mix], [{:ash, ">= 3.5.25 and < 4.0.0-0", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, "~> 3.9", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "bd957aee95bbdf6326fc7a9212f9a2ab87329b99ee3646c373a87bb3c9968566"},
   "benchee": {:hex, :benchee, "1.4.0", "9f1f96a30ac80bab94faad644b39a9031d5632e517416a8ab0a6b0ac4df124ce", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.0", [hex: :statistex, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "299cd10dd8ce51c9ea3ddb74bb150f93d25e968f93e4c1fa31698a8e4fa5d715"},
   "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
   "credo": {:hex, :credo, "1.7.12", "9e3c20463de4b5f3f23721527fcaf16722ec815e70ff6c60b86412c695d426c1", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8493d45c656c5427d9c729235b99d498bd133421f3e0a683e5c1b561471291e5"},

test/migration_generator_test.exs

@@ -2793,6 +2793,68 @@ defmodule AshPostgres.MigrationGeneratorTest do
     end
   end
 
+  describe "multitenancy identity with tenant attribute" do
+    setup do
+      on_exit(fn ->
+        File.rm_rf!("test_snapshots_path")
+        File.rm_rf!("test_migration_path")
+      end)
+    end
+
+    test "identity including tenant attribute does not duplicate columns in index" do
+      defresource Channel, "channels" do
+        postgres do
+          table "channels"
+          repo(AshPostgres.TestRepo)
+        end
+
+        actions do
+          defaults([:create, :read, :update, :destroy])
+        end
+
+        multitenancy do
+          strategy(:attribute)
+          attribute(:project_id)
+        end
+
+        identities do
+          identity(:unique_type_per_project, [:project_id, :type])
+        end
+
+        attributes do
+          uuid_primary_key(:id)
+          attribute(:project_id, :uuid, allow_nil?: false, public?: true)
+          attribute(:type, :string, allow_nil?: false, public?: true)
+          attribute(:name, :string, public?: true)
+        end
+      end
+
+      defdomain([Channel])
+
+      AshPostgres.MigrationGenerator.generate(Domain,
+        snapshot_path: "test_snapshots_path",
+        migration_path: "test_migration_path",
+        quiet: true,
+        format: false,
+        auto_name: true
+      )
+
+      assert [file] =
+               Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")
+               |> Enum.reject(&String.contains?(&1, "extensions"))
+
+      file_content = File.read!(file)
+
+      # The index should only have project_id and type, not project_id twice
+      assert file_content =~
+               ~S{create unique_index(:channels, [:project_id, :type], name: "channels_unique_type_per_project_index")}
+
+      # Make sure it doesn't have duplicate columns
+      refute file_content =~
+               ~S{create unique_index(:channels, [:project_id, :project_id, :type]}
+    end
+  end
+
   describe "decimal precision and scale" do
     setup do
       on_exit(fn ->