From 037b1054694c50695e1264645c9b7e6ce1b5c966 Mon Sep 17 00:00:00 2001 From: ken-kost Date: Sun, 2 Mar 2025 16:07:48 +0100 Subject: [PATCH 1/9] Add dev opt to generate migrations task --- lib/mix/tasks/ash_postgres.generate_migrations.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mix/tasks/ash_postgres.generate_migrations.ex b/lib/mix/tasks/ash_postgres.generate_migrations.ex index 065942c8..d83fc4df 100644 --- a/lib/mix/tasks/ash_postgres.generate_migrations.ex +++ b/lib/mix/tasks/ash_postgres.generate_migrations.ex @@ -21,6 +21,7 @@ defmodule Mix.Tasks.AshPostgres.GenerateMigrations do * `no-format` - files that are created will not be formatted with the code formatter * `dry-run` - no files are created, instead the new migration is printed * `check` - no files are created, returns an exit(1) code if the current snapshots and resources don't fit + * 'dev' - dev files are created * `snapshots-only` - no migrations are generated, only snapshots are stored * `concurrent-indexes` - new identities will be run concurrently and in a separate migration (like concurrent custom indexes) @@ -97,6 +98,7 @@ defmodule Mix.Tasks.AshPostgres.GenerateMigrations do no_format: :boolean, dry_run: :boolean, check: :boolean, + dev: :boolean, dont_drop_columns: :boolean, concurrent_indexes: :boolean ] @@ -110,7 +112,7 @@ defmodule Mix.Tasks.AshPostgres.GenerateMigrations do |> Keyword.delete(:no_format) |> Keyword.put_new(:name, name) - if !opts[:name] && !opts[:dry_run] && !opts[:check] && !opts[:snapshots_only] do + if !opts[:name] && !opts[:dry_run] && !opts[:check] && !opts[:snapshots_only] && !opts[:dev] do IO.warn(""" Name must be provided when generating migrations, unless `--dry-run` or `--check` is also provided. Using an autogenerated name will be deprecated in a future release. From 6ee9afac441306220ef2ff43b2aec83aea80123a Mon Sep 17 00:00:00 2001 From: ken-kost Date: Sun, 2 Mar 2025 16:08:23 +0100 Subject: [PATCH 2/9] Handle dev flag in migration generator --- .../migration_generator.ex | 79 +++++++++++++++++-- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index 1c35b6b0..bc2d6502 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -19,6 +19,7 @@ defmodule AshPostgres.MigrationGenerator do format: true, dry_run: false, check: false, + dev: false, snapshots_only: false, dont_drop_columns: false @@ -452,6 +453,20 @@ defmodule AshPostgres.MigrationGenerator do :ok operations -> + if !opts.dev and any_dev_migrations?(opts, repo) do + if opts.check do + Mix.shell().error(""" + Generated migrations are from dev mode. + + Generate migrations without `--dev` flag. + """) + + exit({:shutdown, 1}) + else + remove_dev_migrations_and_snapshots(opts, tenant?, repo, snapshots) + end + end + if opts.check do Mix.shell().error(""" Migrations would have been generated, but the --check flag was provided. @@ -491,6 +506,50 @@ defmodule AshPostgres.MigrationGenerator do end) end + defp any_dev_migrations?(opts, repo) do + opts + |> migration_path(repo) + |> File.ls!() + |> Enum.any?(&String.contains?(&1, "_dev.exs")) + end + + defp remove_dev_migrations_and_snapshots(opts, tenant?, repo, snapshots) do + opts + |> migration_path(repo) + |> File.ls!() + |> Enum.filter(&String.contains?(&1, "_dev.exs")) + |> Enum.each(fn migration_name -> + opts + |> migration_path(repo) + |> Path.join(migration_name) + |> File.rm!() + end) + + Enum.each(snapshots, fn snapshot -> + snapshot_folder = + if tenant? do + opts + |> snapshot_path(snapshot.repo) + |> Path.join(repo_name(repo)) + |> Path.join("tenants") + |> Path.join(snapshot.table) + else + opts + |> snapshot_path(snapshot.repo) + |> Path.join(repo_name(snapshot.repo)) + |> Path.join(snapshot.table) + end + + File.ls!(snapshot_folder) + |> Enum.filter(&String.contains?(&1, "_dev.json")) + |> Enum.each(fn snapshot_name -> + snapshot_folder + |> Path.join(snapshot_name) + |> File.rm!() + end) + end) + end + defp split_into_migrations(operations) do operations |> Enum.split_with(fn @@ -960,7 +1019,7 @@ defmodule AshPostgres.MigrationGenerator do migration_file = migration_path - |> Path.join(migration_name <> ".exs") + |> Path.join(migration_name <> "#{if opts.dev, do: "_dev"}.exs") module_name = if tenant? do @@ -1054,20 +1113,25 @@ defmodule AshPostgres.MigrationGenerator do |> Path.join(repo_name) end + dev = if opts.dev, do: "_dev" + snapshot_file = if snapshot.schema do - Path.join(snapshot_folder, "#{snapshot.schema}.#{snapshot.table}/#{timestamp()}.json") + Path.join( + snapshot_folder, + "#{snapshot.schema}.#{snapshot.table}/#{timestamp()}#{dev}.json" + ) else - Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}.json") + Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}#{dev}.json") end File.mkdir_p(Path.dirname(snapshot_file)) create_file(snapshot_file, snapshot_binary, force: true) - old_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}.json") + old_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}#{dev}.json") if File.exists?(old_snapshot_folder) do - new_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}/initial.json") + new_snapshot_folder = Path.join(snapshot_folder, "#{snapshot.table}/initial#{dev}.json") File.rename(old_snapshot_folder, new_snapshot_folder) end end) @@ -2640,7 +2704,10 @@ defmodule AshPostgres.MigrationGenerator do if File.exists?(snapshot_folder) do snapshot_folder |> File.ls!() - |> Enum.filter(&String.match?(&1, ~r/^\d{14}\.json$/)) + |> Enum.filter( + &(String.match?(&1, ~r/^\d{14}\.json$/) or + (opts.dev and String.match?(&1, ~r/^\d{14}\_dev.json$/))) + ) |> case do [] -> get_old_snapshot(folder, snapshot) From 23e1c425e0a049121c83f0fa458227d6116bcdfd Mon Sep 17 00:00:00 2001 From: ken-kost Date: Mon, 3 Mar 2025 18:53:44 +0100 Subject: [PATCH 3/9] Expand warn message with dev option --- lib/mix/tasks/ash_postgres.generate_migrations.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/tasks/ash_postgres.generate_migrations.ex b/lib/mix/tasks/ash_postgres.generate_migrations.ex index d83fc4df..846c0333 100644 --- a/lib/mix/tasks/ash_postgres.generate_migrations.ex +++ b/lib/mix/tasks/ash_postgres.generate_migrations.ex @@ -114,7 +114,7 @@ defmodule Mix.Tasks.AshPostgres.GenerateMigrations do if !opts[:name] && !opts[:dry_run] && !opts[:check] && !opts[:snapshots_only] && !opts[:dev] do IO.warn(""" - Name must be provided when generating migrations, unless `--dry-run` or `--check` is also provided. + Name must be provided when generating migrations, unless `--dry-run` or `--check` or `--dev` is also provided. Using an autogenerated name will be deprecated in a future release. Please provide a name. for example: From af4f4b6bb2b41013403ce765f256eab9bea4577c Mon Sep 17 00:00:00 2001 From: ken-kost Date: Mon, 3 Mar 2025 18:54:05 +0100 Subject: [PATCH 4/9] Add test for dev option --- test/migration_generator_test.exs | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 88f76f67..7cd837c2 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -1250,6 +1250,53 @@ defmodule AshPostgres.MigrationGeneratorTest do end end + describe "--dev option" do + setup do + on_exit(fn -> + File.rm_rf!("test_snapshots_path") + File.rm_rf!("test_migration_path") + end) + end + + test "generates dev migration" do + defposts do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, public?: true) + end + end + + defdomain([Post]) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + snapshots_only: false, + migration_path: "test_migration_path", + dev: true + ) + + assert [dev_file] = + Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert String.contains?(dev_file, "_dev.exs") + contents = File.read!(dev_file) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path" + ) + + assert [file] = + Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + |> Enum.reject(&String.contains?(&1, "extensions")) + + refute String.contains?(file, "_dev.exs") + + assert contents == File.read!(file) + end + end + describe "references" do setup do on_exit(fn -> From efb62b706f4a4ce03130048f73c62756533125ca Mon Sep 17 00:00:00 2001 From: ken-kost Date: Fri, 7 Mar 2025 08:21:16 +0100 Subject: [PATCH 5/9] Rollback migrations before file removal --- lib/migration_generator/migration_generator.ex | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index bc2d6502..c896c41f 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -518,6 +518,12 @@ defmodule AshPostgres.MigrationGenerator do |> migration_path(repo) |> File.ls!() |> Enum.filter(&String.contains?(&1, "_dev.exs")) + |> then(fn dev_migrations -> + version = dev_migrations |> Enum.min() |> String.split("_") |> hd() + System.cmd("mix", ["ash_postgres.rollback", "--to", version]) + System.cmd("mix", ["ash_postgres.rollback", "--to", version], env: [{"MIX_ENV", "test"}]) + dev_migrations + end) |> Enum.each(fn migration_name -> opts |> migration_path(repo) From ad981816c523b62151d8f025c8bd55daa9c2abc4 Mon Sep 17 00:00:00 2001 From: ken-kost Date: Fri, 7 Mar 2025 09:07:15 +0100 Subject: [PATCH 6/9] Rollback tenant migrations --- lib/migration_generator/migration_generator.ex | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index c896c41f..ffa3a1bd 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -520,8 +520,17 @@ defmodule AshPostgres.MigrationGenerator do |> Enum.filter(&String.contains?(&1, "_dev.exs")) |> then(fn dev_migrations -> version = dev_migrations |> Enum.min() |> String.split("_") |> hd() + opts = [env: [{"MIX_ENV", "test"}]] System.cmd("mix", ["ash_postgres.rollback", "--to", version]) - System.cmd("mix", ["ash_postgres.rollback", "--to", version], env: [{"MIX_ENV", "test"}]) + System.cmd("mix", ["ash_postgres.rollback", "--to", version], opts) + + if tenant? do + for tenant <- AshPostgres.Mix.Helpers.tenants(repo, opts) do + System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant]) + System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant], opts) + end + end + dev_migrations end) |> Enum.each(fn migration_name -> From 50f5b6f79f8a3b981efc2b54c6f91dde0243690b Mon Sep 17 00:00:00 2001 From: ken-kost Date: Fri, 7 Mar 2025 13:22:47 +0100 Subject: [PATCH 7/9] Check dev for tenants, use helper functions to split up logic --- .../migration_generator.ex | 136 +++++++++--------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index ffa3a1bd..f0308cf0 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -453,7 +453,9 @@ defmodule AshPostgres.MigrationGenerator do :ok operations -> - if !opts.dev and any_dev_migrations?(opts, repo) do + dev_migrations = get_dev_migrations(opts, tenant?, repo) + + if !opts.dev and dev_migrations != [] do if opts.check do Mix.shell().error(""" Generated migrations are from dev mode. @@ -463,7 +465,8 @@ defmodule AshPostgres.MigrationGenerator do exit({:shutdown, 1}) else - remove_dev_migrations_and_snapshots(opts, tenant?, repo, snapshots) + remove_dev_migrations(dev_migrations, tenant?, repo, opts) + remove_dev_snapshots(snapshots, opts) end end @@ -506,59 +509,53 @@ defmodule AshPostgres.MigrationGenerator do end) end - defp any_dev_migrations?(opts, repo) do + defp get_dev_migrations(opts, tenant?, repo) do opts - |> migration_path(repo) - |> File.ls!() - |> Enum.any?(&String.contains?(&1, "_dev.exs")) + |> migration_path(repo, tenant?) + |> File.ls() + |> case do + {:error, _error} -> [] + {:ok, migrations} -> Enum.filter(migrations, &String.contains?(&1, "_dev.exs")) + end end - defp remove_dev_migrations_and_snapshots(opts, tenant?, repo, snapshots) do - opts - |> migration_path(repo) - |> File.ls!() - |> Enum.filter(&String.contains?(&1, "_dev.exs")) - |> then(fn dev_migrations -> - version = dev_migrations |> Enum.min() |> String.split("_") |> hd() - opts = [env: [{"MIX_ENV", "test"}]] - System.cmd("mix", ["ash_postgres.rollback", "--to", version]) - System.cmd("mix", ["ash_postgres.rollback", "--to", version], opts) + defp remove_dev_migrations(dev_migrations, tenant?, repo, opts) do + version = dev_migrations |> Enum.min() |> String.split("_") |> hd() + test_env = [env: [{"MIX_ENV", "test"}]] + System.cmd("mix", ["ash_postgres.rollback", "--to", version]) + System.cmd("mix", ["ash_postgres.rollback", "--to", version], test_env) - if tenant? do - for tenant <- AshPostgres.Mix.Helpers.tenants(repo, opts) do - System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant]) - System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant], opts) - end + if tenant? do + for tenant <- repo.all_tenants() do + System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant]) + + System.cmd( + "mix", + ["ash_postgres.rollback", "--to", version, "--prefix", tenant], + opts + ) end + end - dev_migrations - end) + dev_migrations |> Enum.each(fn migration_name -> opts - |> migration_path(repo) + |> migration_path(repo, tenant?) |> Path.join(migration_name) |> File.rm!() end) + end + def remove_dev_snapshots(snapshots, opts) do Enum.each(snapshots, fn snapshot -> - snapshot_folder = - if tenant? do - opts - |> snapshot_path(snapshot.repo) - |> Path.join(repo_name(repo)) - |> Path.join("tenants") - |> Path.join(snapshot.table) - else - opts - |> snapshot_path(snapshot.repo) - |> Path.join(repo_name(snapshot.repo)) - |> Path.join(snapshot.table) - end + folder = get_snapshot_folder(snapshot, opts) + snapshot_path = get_snapshot_path(snapshot, folder) - File.ls!(snapshot_folder) + snapshot_path + |> File.ls!() |> Enum.filter(&String.contains?(&1, "_dev.json")) |> Enum.each(fn snapshot_name -> - snapshot_folder + snapshot_path |> Path.join(snapshot_name) |> File.rm!() end) @@ -2689,35 +2686,11 @@ defmodule AshPostgres.MigrationGenerator do end def get_existing_snapshot(snapshot, opts) do - repo_name = snapshot.repo |> Module.split() |> List.last() |> Macro.underscore() - - folder = - if snapshot.multitenancy.strategy == :context do - opts - |> snapshot_path(snapshot.repo) - |> Path.join(repo_name) - |> Path.join("tenants") - else - opts - |> snapshot_path(snapshot.repo) - |> Path.join(repo_name) - end - - snapshot_folder = - if snapshot.schema do - schema_dir = Path.join(folder, "#{snapshot.schema}.#{snapshot.table}") - - if File.dir?(schema_dir) do - schema_dir - else - Path.join(folder, snapshot.table) - end - else - Path.join(folder, snapshot.table) - end + folder = get_snapshot_folder(snapshot, opts) + snapshot_path = get_snapshot_path(snapshot, folder) - if File.exists?(snapshot_folder) do - snapshot_folder + if File.exists?(snapshot_path) do + snapshot_path |> File.ls!() |> Enum.filter( &(String.match?(&1, ~r/^\d{14}\.json$/) or @@ -2728,7 +2701,7 @@ defmodule AshPostgres.MigrationGenerator do get_old_snapshot(folder, snapshot) snapshot_files -> - snapshot_folder + snapshot_path |> Path.join(Enum.max(snapshot_files)) |> File.read!() |> load_snapshot() @@ -2738,6 +2711,33 @@ defmodule AshPostgres.MigrationGenerator do end end + defp get_snapshot_folder(snapshot, opts) do + if snapshot.multitenancy.strategy == :context do + opts + |> snapshot_path(snapshot.repo) + |> Path.join(repo_name(snapshot.repo)) + |> Path.join("tenants") + else + opts + |> snapshot_path(snapshot.repo) + |> Path.join(repo_name(snapshot.repo)) + end + end + + defp get_snapshot_path(snapshot, folder) do + if snapshot.schema do + schema_dir = Path.join(folder, "#{snapshot.schema}.#{snapshot.table}") + + if File.dir?(schema_dir) do + schema_dir + else + Path.join(folder, snapshot.table) + end + else + Path.join(folder, snapshot.table) + end + end + defp get_old_snapshot(folder, snapshot) do schema_file = if snapshot.schema do From bd090b52bc5c5c0a4f1659e65ffb88c0e40b8aa4 Mon Sep 17 00:00:00 2001 From: ken-kost Date: Fri, 7 Mar 2025 13:23:10 +0100 Subject: [PATCH 8/9] Add dev option test for tenant --- test/migration_generator_test.exs | 51 ++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 7cd837c2..c0f53a01 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -1255,6 +1255,7 @@ defmodule AshPostgres.MigrationGeneratorTest do on_exit(fn -> File.rm_rf!("test_snapshots_path") File.rm_rf!("test_migration_path") + File.rm_rf!("test_tenant_migration_path") end) end @@ -1270,7 +1271,6 @@ defmodule AshPostgres.MigrationGeneratorTest do AshPostgres.MigrationGenerator.generate(Domain, snapshot_path: "test_snapshots_path", - snapshots_only: false, migration_path: "test_migration_path", dev: true ) @@ -1295,6 +1295,55 @@ defmodule AshPostgres.MigrationGeneratorTest do assert contents == File.read!(file) end + + test "generates dev migration for tenant" do + defposts do + postgres do + schema("example") + end + + attributes do + uuid_primary_key(:id) + attribute(:title, :string, public?: true, primary_key?: true, allow_nil?: false) + end + + multitenancy do + strategy(:context) + end + end + + defdomain([Post]) + + send(self(), {:mix_shell_input, :yes?, true}) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + tenant_migration_path: "test_tenant_migration_path", + dev: true + ) + + assert [dev_file] = + Enum.sort(Path.wildcard("test_tenant_migration_path/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert String.contains?(dev_file, "_dev.exs") + contents = File.read!(dev_file) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + tenant_migration_path: "test_tenant_migration_path" + ) + + assert [file] = + Path.wildcard("test_tenant_migration_path/**/*_migrate_resources*.exs") + |> Enum.reject(&String.contains?(&1, "extensions")) + + refute String.contains?(file, "_dev.exs") + + assert contents == File.read!(file) + end end describe "references" do From de5d3eec7ee30a8959d2b70428af51fdb26feafd Mon Sep 17 00:00:00 2001 From: ken-kost Date: Sun, 9 Mar 2025 18:02:28 +0100 Subject: [PATCH 9/9] Fix wrong opts pass in tenant rollback --- lib/migration_generator/migration_generator.ex | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index f0308cf0..039edc95 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -526,15 +526,11 @@ defmodule AshPostgres.MigrationGenerator do System.cmd("mix", ["ash_postgres.rollback", "--to", version], test_env) if tenant? do - for tenant <- repo.all_tenants() do - System.cmd("mix", ["ash_postgres.rollback", "--to", version, "--prefix", tenant]) - - System.cmd( - "mix", - ["ash_postgres.rollback", "--to", version, "--prefix", tenant], - opts - ) - end + Enum.each(repo.all_tenants(), fn tenant -> + args = ["ash_postgres.rollback", "--to", version, "--prefix", tenant] + System.cmd("mix", args) + System.cmd("mix", args, test_env) + end) end dev_migrations