fix: sort AddUniqueIndex after CreateTable and all AddAttribute ops

When an explicit FK attribute preceded the identity key attribute,
AddUniqueIndex was sorted before AddAttribute ops for the same table,
breaking Phase.Create grouping and producing alter instead of create.

Also fixes dev_migrations_test to use a tmp dir for snapshots so it
no longer deletes orphan snapshots from priv/resource_snapshots/.

Author: zachdaniel

lib/migration_generator/migration_generator.ex

@@ -1693,12 +1693,12 @@ defmodule AshPostgres.MigrationGenerator do
        ),
        do: false
 
-  # Do not place AddUniqueIndex after CreateTable (must be after columns are added).
+  # AddUniqueIndex must come after CreateTable (table must exist first).
   defp after?(
          %Operation.AddUniqueIndex{table: table, schema: schema},
          %Operation.CreateTable{table: table, schema: schema}
        ),
-       do: false
+       do: true
 
   # Place AddUniqueIndex after a specific attribute (by source) for the same
   # table so it appears before AlterAttributes (issue #236).
@@ -1729,7 +1729,7 @@ defmodule AshPostgres.MigrationGenerator do
          }
        )
        when not is_nil(source),
-       do: false
+       do: true
 
   defp after?(
          %Operation.AddUniqueIndex{

mix.lock

@@ -1,5 +1,5 @@
 %{
-  "ash": {:hex, :ash, "3.19.3", "58b1bb3aea3d1d45d1c990059ffd0753409cc92fc4afe387376cb155e2a8c2a0", [:mix], [{:crux, ">= 0.1.2 and < 1.0.0-0", [hex: :crux, repo: "hexpm", optional: false]}, {: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.29 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, "~> 1.0", [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.3.14 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.3", [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", "94b628319f2e144affaf1f8008277bad3340a198d48e6d2ed372990ac1643f9b"},
+  "ash": {:hex, :ash, "3.20.0", "1837fd4af980a67cf723e48612fe48d86d410c5ec86a3bef2c0a7ba5815fe371", [:mix], [{:crux, ">= 0.1.2 and < 1.0.0-0", [hex: :crux, repo: "hexpm", optional: false]}, {: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.29 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, "~> 1.0", [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.3.14 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.3", [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", "29300a6dbbe38ea33941b2bb479e9826acf8aa46590c94ebfca22102a54d569e"},
   "ash_sql": {:hex, :ash_sql, "0.5.0", "06cf976f2cca3c16542b9c3103220ed1358903c6a83bd0ee541432d371a87a9e", [:mix], [{:ash, "~> 3.7", [hex: :ash, repo: "hexpm", optional: false]}, {:ecto, ">= 3.13.4 and < 4.0.0-0", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.9", [hex: :ecto_sql, repo: "hexpm", optional: false]}], "hexpm", "b78865699cd706db7d8e07f366a5bd61ea06dde19f649870ff895d293ecc42a1"},
   "benchee": {:hex, :benchee, "1.5.0", "4d812c31d54b0ec0167e91278e7de3f596324a78a096fd3d0bea68bb0c513b10", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}, {:statistex, "~> 1.1", [hex: :statistex, repo: "hexpm", optional: false]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "5b075393aea81b8ae74eadd1c28b1d87e8a63696c649d8293db7c4df3eb67535"},
   "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"},
@@ -24,7 +24,7 @@
   "git_ops": {:hex, :git_ops, "2.9.2", "1e6de106e57cc8d10850511f12921fb9a98960fd7a8f0d0d1dc334b816f7f13b", [:mix], [{:git_cli, "~> 0.2", [hex: :git_cli, repo: "hexpm", optional: false]}, {:igniter, ">= 0.5.27 and < 1.0.0-0", [hex: :igniter, repo: "hexpm", optional: true]}, {:nimble_parsec, "~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}], "hexpm", "1a1b49197da6b1551fd7a170d82c98980f8e7fab4727b6ce2db6672aefea5b96"},
   "glob_ex": {:hex, :glob_ex, "0.1.11", "cb50d3f1ef53f6ca04d6252c7fde09fd7a1cf63387714fe96f340a1349e62c93", [:mix], [], "hexpm", "342729363056e3145e61766b416769984c329e4378f1d558b63e341020525de4"},
   "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"},
-  "igniter": {:hex, :igniter, "0.7.4", "b5f9dd512eb1e672f1c141b523142b5b4602fcca231df5b4e362999df4b88e14", [:mix], [{:glob_ex, "~> 0.1.7", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:phx_new, "~> 1.7", [hex: :phx_new, repo: "hexpm", optional: true]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}, {:rewrite, ">= 1.1.1 and < 2.0.0-0", [hex: :rewrite, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.4", [hex: :sourceror, repo: "hexpm", optional: false]}, {:spitfire, ">= 0.1.3 and < 1.0.0-0", [hex: :spitfire, repo: "hexpm", optional: false]}], "hexpm", "971b240ee916a06b1af56381a262d9eeaff9610eddc299d61a213cd7a9d79efd"},
+  "igniter": {:hex, :igniter, "0.7.6", "687d622c735e020f13cf480c83d0fce1cc899f4fbed547f5254b960ea82d3525", [:mix], [{:glob_ex, "~> 0.1.7", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}, {:owl, "~> 0.11", [hex: :owl, repo: "hexpm", optional: false]}, {:phx_new, "~> 1.7", [hex: :phx_new, repo: "hexpm", optional: true]}, {:req, "~> 0.5", [hex: :req, repo: "hexpm", optional: false]}, {:rewrite, ">= 1.1.1 and < 2.0.0-0", [hex: :rewrite, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.4", [hex: :sourceror, repo: "hexpm", optional: false]}, {:spitfire, ">= 0.1.3 and < 1.0.0-0", [hex: :spitfire, repo: "hexpm", optional: false]}], "hexpm", "424f41a41273fce0f7424008405ee073b5bd06359ca9396e841f83a669c01619"},
   "iterex": {:hex, :iterex, "0.1.2", "58f9b9b9a22a55cbfc7b5234a9c9c63eaac26d276b3db80936c0e1c60355a5a6", [:mix], [], "hexpm", "2e103b8bcc81757a9af121f6dc0df312c9a17220f302b1193ef720460d03029d"},
   "jason": {:hex, :jason, "1.4.4", "b9226785a9aa77b6857ca22832cffa5d5011a667207eb2a0ad56adb5db443b8a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "c5eb0cab91f094599f94d55bc63409236a8ec69a21a67814529e8d5f6cc90b3b"},
   "libgraph": {:hex, :libgraph, "0.16.0", "3936f3eca6ef826e08880230f806bfea13193e49bf153f93edcf0239d4fd1d07", [:mix], [], "hexpm", "41ca92240e8a4138c30a7e06466acc709b0cbb795c643e9e17174a178982d6bf"},
@@ -58,5 +58,5 @@
   "tz": {:hex, :tz, "0.28.1", "717f5ffddfd1e475e2a233e221dc0b4b76c35c4b3650b060c8e3ba29dd6632e9", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:mint, "~> 1.6", [hex: :mint, repo: "hexpm", optional: true]}], "hexpm", "bfdca1aa1902643c6c43b77c1fb0cb3d744fd2f09a8a98405468afdee0848c8a"},
   "yamerl": {:hex, :yamerl, "0.10.0", "4ff81fee2f1f6a46f1700c0d880b24d193ddb74bd14ef42cb0bcf46e81ef2f8e", [:rebar3], [], "hexpm", "346adb2963f1051dc837a2364e4acf6eb7d80097c0f53cbdc3046ec8ec4b4e6e"},
   "yaml_elixir": {:hex, :yaml_elixir, "2.12.1", "d74f2d82294651b58dac849c45a82aaea639766797359baff834b64439f6b3f4", [:mix], [{:yamerl, "~> 0.10", [hex: :yamerl, repo: "hexpm", optional: false]}], "hexpm", "d9ac16563c737d55f9bfeed7627489156b91268a3a21cd55c54eb2e335207fed"},
-  "ymlr": {:hex, :ymlr, "5.1.4", "b924d61e1fc1ec371cde6ab3ccd9311110b1e052fc5c2460fb322e8380e7712a", [:mix], [], "hexpm", "75f16cf0709fbd911b30311a0359a7aa4b5476346c01882addefd5f2b1cfaa51"},
+  "ymlr": {:hex, :ymlr, "5.1.5", "0b9207c7940be3f2bc29b77cd55109d5aa2f4dcde6575942017335769e6f5628", [:mix], [], "hexpm", "7030cb240c46850caeb3b01be745307632be319b15f03083136f6251f49b516d"},
 }

test/dev_migrations_test.exs

@@ -5,12 +5,13 @@
 defmodule AshPostgres.DevMigrationsTest do
   use AshPostgres.RepoCase, async: false
   @moduletag :migration
+  @moduletag :tmp_dir
 
   require Logger
 
   alias Ecto.Adapters.SQL.Sandbox
 
-  setup do
+  setup %{tmp_dir: tmp_dir} do
     current_shell = Mix.shell()
 
     :ok = Mix.shell(Mix.Shell.Process)
@@ -20,6 +21,17 @@ defmodule AshPostgres.DevMigrationsTest do
     end)
 
     Sandbox.checkout(AshPostgres.DevTestRepo)
+
+    # Copy existing snapshots to tmp dir so the generator doesn't
+    # re-generate extensions or delete orphan snapshots from priv/
+    snapshot_path = Path.join(tmp_dir, "snapshots")
+    source = "priv/resource_snapshots"
+
+    if File.exists?(source) do
+      File.cp_r!(source, snapshot_path)
+    end
+
+    %{snapshot_path: snapshot_path}
   end
 
   defmacrop defresource(mod, do: body) do
@@ -80,11 +92,6 @@ defmodule AshPostgres.DevMigrationsTest do
   end
 
   setup do
-    resource_dev_path = "priv/resource_snapshots/dev_test_repo"
-
-    initial_resource_files =
-      if File.exists?(resource_dev_path), do: File.ls!(resource_dev_path), else: []
-
     migrations_dev_path = "priv/dev_test_repo/migrations"
 
     initial_migration_files =
@@ -98,12 +105,6 @@ defmodule AshPostgres.DevMigrationsTest do
         else: []
 
     clean = fn ->
-      if File.exists?(resource_dev_path) do
-        current_resource_files = File.ls!(resource_dev_path)
-        new_resource_files = current_resource_files -- initial_resource_files
-        Enum.each(new_resource_files, &File.rm_rf!(Path.join(resource_dev_path, &1)))
-      end
-
       if File.exists?(migrations_dev_path) do
         current_migration_files = File.ls!(migrations_dev_path)
         new_migration_files = current_migration_files -- initial_migration_files
@@ -131,7 +132,7 @@ defmodule AshPostgres.DevMigrationsTest do
   end
 
   describe "--dev option" do
-    test "rolls back dev migrations before deleting" do
+    test "rolls back dev migrations before deleting", %{snapshot_path: snapshot_path} do
       defposts do
         attributes do
           uuid_primary_key(:id)
@@ -142,7 +143,7 @@ defmodule AshPostgres.DevMigrationsTest do
       defdomain([Post])
 
       AshPostgres.MigrationGenerator.generate(Domain,
-        snapshot_path: "priv/resource_snapshots",
+        snapshot_path: snapshot_path,
         migration_path: "priv/dev_test_repo/migrations",
         dev: true,
         auto_name: true
@@ -157,7 +158,7 @@ defmodule AshPostgres.DevMigrationsTest do
       # Generating without dev: true rolls back the dev migration (dropping the table)
       # and creates a permanent migration in its place
       AshPostgres.MigrationGenerator.generate(Domain,
-        snapshot_path: "priv/resource_snapshots",
+        snapshot_path: snapshot_path,
         migration_path: "priv/dev_test_repo/migrations",
         auto_name: true
       )
@@ -170,7 +171,7 @@ defmodule AshPostgres.DevMigrationsTest do
   end
 
   describe "--dev option tenant" do
-    test "rolls back dev migrations before deleting" do
+    test "rolls back dev migrations before deleting", %{snapshot_path: snapshot_path} do
       defposts do
         attributes do
           uuid_primary_key(:id)
@@ -185,7 +186,7 @@ defmodule AshPostgres.DevMigrationsTest do
       defdomain([Post])
 
       AshPostgres.MigrationGenerator.generate(Domain,
-        snapshot_path: "priv/resource_snapshots",
+        snapshot_path: snapshot_path,
         migration_path: "priv/dev_test_repo/migrations",
         tenant_migration_path: "priv/dev_test_repo/tenant_migrations",
         dev: true,
@@ -210,7 +211,7 @@ defmodule AshPostgres.DevMigrationsTest do
                |> Enum.reject(&String.contains?(&1, "extensions"))
 
       AshPostgres.MigrationGenerator.generate(Domain,
-        snapshot_path: "priv/resource_snapshots",
+        snapshot_path: snapshot_path,
         migration_path: "priv/dev_test_repo/migrations",
         tenant_migration_path: "priv/dev_test_repo/tenant_migrations",
         auto_name: true