diff --git a/.gitignore b/.gitignore index e79954f..15657de 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,6 @@ test_snapshots_path test/test.db test/test.db-shm test/test.db-wal +test/dev_test.db +test/dev_test.db-shm +test/dev_test.db-wal diff --git a/config/config.exs b/config/config.exs index 7fd1cae..f433068 100644 --- a/config/config.exs +++ b/config/config.exs @@ -28,8 +28,15 @@ if Mix.env() == :test do pool: Ecto.Adapters.SQL.Sandbox, migration_primary_key: [name: :id, type: :binary_id] + config :ash_sqlite, AshSqlite.DevTestRepo, + database: Path.join(__DIR__, "../test/dev_test.db"), + pool_size: 1, + migration_lock: false, + pool: Ecto.Adapters.SQL.Sandbox, + migration_primary_key: [name: :id, type: :binary_id] + config :ash_sqlite, - ecto_repos: [AshSqlite.TestRepo], + ecto_repos: [AshSqlite.TestRepo, AshSqlite.DevTestRepo], ash_domains: [ AshSqlite.Test.Domain ] diff --git a/documentation/topics/development/migrations-and-tasks.md b/documentation/topics/development/migrations-and-tasks.md index 6d08580..6d9ffcb 100644 --- a/documentation/topics/development/migrations-and-tasks.md +++ b/documentation/topics/development/migrations-and-tasks.md @@ -6,11 +6,30 @@ Ash comes with its own tasks, and AshSqlite exposes lower level tasks that you c ## Basic Workflow -- Make resource changes -- Run `mix ash.codegen --name add_a_combobulator` to generate migrations and resource snapshots -- Run `mix ash.migrate` to run those migrations +### Development Workflow (Recommended) -For more information on generating migrations, run `mix help ash_sqlite.generate_migrations` (the underlying task that is called by `mix ash.migrate`) +For development iterations, use the dev workflow to avoid naming migrations prematurely: + +1. Make resource changes +2. Run `mix ash.codegen --dev` to generate dev migrations +3. Review the migrations and run `mix ash.migrate` to run them +4. Continue making changes and running `mix ash.codegen --dev` as needed +5. When your feature is complete, run `mix ash.codegen add_feature_name` to generate final named migrations (this will remove dev migrations and squash them) +6. Review the migrations and run `mix ash.migrate` to run them + +### Traditional Migration Generation + +For single-step changes or when you know the final feature name: + +1. Make resource changes +2. Run `mix ash.codegen --name add_a_combobulator` to generate migrations and resource snapshots +3. Run `mix ash.migrate` to run those migrations + +> **Tip**: The dev workflow (`--dev` flag) is preferred during development as it allows you to iterate without thinking of migration names and provides better development ergonomics. + +> **Warning**: Always review migrations before applying them to ensure they are correct and safe. + +For more information on generating migrations, run `mix help ash_sqlite.generate_migrations` (the underlying task that is called by `mix ash.codegen`) ### Regenerating Migrations diff --git a/documentation/tutorials/getting-started-with-ash-sqlite.md b/documentation/tutorials/getting-started-with-ash-sqlite.md index 11e60c1..8321e5f 100644 --- a/documentation/tutorials/getting-started-with-ash-sqlite.md +++ b/documentation/tutorials/getting-started-with-ash-sqlite.md @@ -170,6 +170,8 @@ Then we will generate database migrations. This is one of the many ways that Ash mix ash_sqlite.generate_migrations --name add_tickets_and_representatives ``` +> **Development Tip**: For iterative development, you can use `mix ash_sqlite.generate_migrations --dev` to create dev migrations without needing to name them immediately. When you're ready to finalize your changes, run the command with a proper name to consolidate all dev migrations into a single, well-named migration. + If you are unfamiliar with database migrations, it is a good idea to get a rough idea of what they are and how they work. See the links at the bottom of this guide for more. A rough overview of how migrations work is that each time you need to make changes to your database, they are saved as small, reproducible scripts that can be applied in order. This is necessary both for clean deploys as well as working with multiple developers making changes to the structure of a single database. Typically, you need to write these by hand. AshSqlite, however, will store snapshots each time you run the command to generate migrations and will figure out what migrations need to be created. diff --git a/lib/migration_generator/migration_generator.ex b/lib/migration_generator/migration_generator.ex index d406d5e..2421c28 100644 --- a/lib/migration_generator/migration_generator.ex +++ b/lib/migration_generator/migration_generator.ex @@ -17,6 +17,8 @@ defmodule AshSqlite.MigrationGenerator do format: true, dry_run: false, check: false, + dev: false, + auto_name: false, drop_columns: false def generate(domains, opts \\ []) do @@ -213,7 +215,7 @@ defmodule AshSqlite.MigrationGenerator do migration_file = opts |> migration_path(repo) - |> Path.join(migration_name <> ".exs") + |> Path.join(migration_name <> "#{if opts.dev, do: "_dev"}.exs") sanitized_module = module @@ -332,6 +334,22 @@ defmodule AshSqlite.MigrationGenerator do :ok operations -> + dev_migrations = get_dev_migrations(opts, repo) + + if !opts.dev and dev_migrations != [] 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(dev_migrations, repo, opts, snapshots) + end + end + if opts.check do IO.puts(""" Migrations would have been generated, but the --check flag was provided. @@ -353,6 +371,46 @@ defmodule AshSqlite.MigrationGenerator do end) end + defp get_dev_migrations(opts, repo) do + opts + |> migration_path(repo) + |> File.ls() + |> case do + {:error, _error} -> [] + {:ok, migrations} -> Enum.filter(migrations, &String.contains?(&1, "_dev.exs")) + end + end + + defp remove_dev_migrations_and_snapshots(dev_migrations, repo, opts, snapshots) do + # Remove dev migration files + Enum.each(dev_migrations, fn migration_name -> + opts + |> migration_path(repo) + |> Path.join(migration_name) + |> File.rm!() + end) + + # Remove dev snapshots + Enum.each(snapshots, fn snapshot -> + snapshot_folder = + opts + |> snapshot_path(snapshot.repo) + |> Path.join(repo_name(snapshot.repo)) + |> Path.join(snapshot.table) + + if File.exists?(snapshot_folder) do + snapshot_folder + |> File.ls!() + |> Enum.filter(&String.contains?(&1, "_dev.json")) + |> Enum.each(fn snapshot_name -> + snapshot_folder + |> Path.join(snapshot_name) + |> File.rm!() + end) + end + end) + end + defp add_order_to_operations({snapshot, operations}) do operations_with_order = Enum.map(operations, &add_order_to_operation(&1, snapshot.attributes)) @@ -712,6 +770,8 @@ defmodule AshSqlite.MigrationGenerator do defp write_migration!({up, down}, repo, opts) do migration_path = migration_path(opts, repo) + require_name!(opts) + {migration_name, last_part} = if opts.name do {"#{timestamp(true)}_#{opts.name}", "#{opts.name}"} @@ -742,7 +802,7 @@ defmodule AshSqlite.MigrationGenerator do migration_file = migration_path - |> Path.join(migration_name <> ".exs") + |> Path.join(migration_name <> "#{if opts.dev, do: "_dev"}.exs") module_name = Module.concat([repo, Migrations, Macro.camelize(last_part)]) @@ -815,6 +875,20 @@ defmodule AshSqlite.MigrationGenerator do end end + defp require_name!(opts) do + if !opts.name && !opts.dry_run && !opts.check && !opts.dev && !opts.auto_name do + raise """ + Name must be provided when generating migrations, unless `--dry-run` or `--check` or `--dev` is also provided. + + Please provide a name. for example: + + mix ash_sqlite.generate_migrations ...args + """ + end + + :ok + end + defp add_line_numbers(contents) do lines = String.split(contents, "\n") @@ -837,15 +911,16 @@ defmodule AshSqlite.MigrationGenerator do |> snapshot_path(snapshot.repo) |> Path.join(repo_name) - snapshot_file = Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}.json") + dev = if opts.dev, do: "_dev" + snapshot_file = Path.join(snapshot_folder, "#{snapshot.table}/#{timestamp()}#{dev}.json") File.mkdir_p(Path.dirname(snapshot_file)) File.write!(snapshot_file, snapshot_binary, []) - 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) diff --git a/lib/mix/tasks/ash_sqlite.generate_migrations.ex b/lib/mix/tasks/ash_sqlite.generate_migrations.ex index 10bb054..d8959c8 100644 --- a/lib/mix/tasks/ash_sqlite.generate_migrations.ex +++ b/lib/mix/tasks/ash_sqlite.generate_migrations.ex @@ -20,6 +20,7 @@ defmodule Mix.Tasks.AshSqlite.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 (see Development Workflow section below) #### Snapshots @@ -58,6 +59,25 @@ defmodule Mix.Tasks.AshSqlite.GenerateMigrations do Non-function default values will be dumped to their native type and inspected. This may not work for some types, and may require manual intervention/patches to the migration generator code. + #### Development Workflow + + The `--dev` flag enables a development-focused migration workflow that allows you to iterate + on resource changes without committing to migration names prematurely: + + 1. Make resource changes + 2. Run `mix ash_sqlite.generate_migrations --dev` to generate dev migrations + - Creates migration files with `_dev.exs` suffix + - Creates snapshot files with `_dev.json` suffix + - No migration name required + 3. Continue making changes and running `--dev` as needed + 4. When ready, run `mix ash_sqlite.generate_migrations my_feature_name` to: + - Remove all dev migrations and snapshots + - Generate final named migrations that consolidate all changes + - Create clean snapshots + + This workflow prevents migration history pollution during development while maintaining + the ability to generate clean, well-named migrations for production. + #### Identities Identities will cause the migration generator to generate unique constraints. If multiple @@ -79,6 +99,8 @@ defmodule Mix.Tasks.AshSqlite.GenerateMigrations do no_format: :boolean, dry_run: :boolean, check: :boolean, + dev: :boolean, + auto_name: :boolean, drop_columns: :boolean ] ) diff --git a/priv/dev_test_repo/migrations/.gitkeep b/priv/dev_test_repo/migrations/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/test/dev_migrations_test.exs b/test/dev_migrations_test.exs new file mode 100644 index 0000000..57bad48 --- /dev/null +++ b/test/dev_migrations_test.exs @@ -0,0 +1,205 @@ +defmodule AshSqlite.DevMigrationsTest do + use AshSqlite.RepoCase, async: false + @moduletag :migration + + alias Ecto.Adapters.SQL.Sandbox + + setup do + current_shell = Mix.shell() + + :ok = Mix.shell(Mix.Shell.Process) + + on_exit(fn -> + Mix.shell(current_shell) + end) + + Sandbox.checkout(AshSqlite.DevTestRepo) + Sandbox.mode(AshSqlite.DevTestRepo, {:shared, self()}) + end + + defmacrop defresource(mod, do: body) do + quote do + Code.compiler_options(ignore_module_conflict: true) + + defmodule unquote(mod) do + use Ash.Resource, + domain: nil, + data_layer: AshSqlite.DataLayer + + unquote(body) + end + + Code.compiler_options(ignore_module_conflict: false) + end + end + + defmacrop defposts(do: body) do + quote do + defresource Post do + sqlite do + table "posts" + repo(AshSqlite.DevTestRepo) + + custom_indexes do + # need one without any opts + index(["id"]) + index(["id"], unique: true, name: "test_unique_index") + end + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + unquote(body) + end + end + end + + defmacrop defdomain(resources) do + quote do + Code.compiler_options(ignore_module_conflict: true) + + defmodule Domain do + use Ash.Domain + + resources do + for resource <- unquote(resources) do + resource(resource) + end + end + end + + Code.compiler_options(ignore_module_conflict: false) + end + end + + setup do + File.mkdir_p!("priv/dev_test_repo/migrations") + 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 = + if File.exists?(migrations_dev_path), do: File.ls!(migrations_dev_path), else: [] + + on_exit(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 + Enum.each(new_migration_files, &File.rm!(Path.join(migrations_dev_path, &1))) + end + + # Clean up test directories + File.rm_rf!("test_snapshots_path") + File.rm_rf!("test_migration_path") + + try do + AshSqlite.DevTestRepo.query!("DROP TABLE IF EXISTS posts") + rescue + _ -> :ok + end + end) + end + + describe "--dev option" do + test "generates dev migration" do + defposts do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, public?: true) + end + end + + defdomain([Post]) + + AshSqlite.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + dev: true + ) + + assert [dev_file] = + Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + + assert String.contains?(dev_file, "_dev.exs") + contents = File.read!(dev_file) + + AshSqlite.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + auto_name: true + ) + + assert [file] = + Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + + refute String.contains?(file, "_dev.exs") + + assert contents == File.read!(file) + end + + test "removes dev migrations when generating regular migrations" do + defposts do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, public?: true) + end + end + + defdomain([Post]) + + # Generate dev migration first + AshSqlite.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + dev: true + ) + + assert [dev_file] = + Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + + assert String.contains?(dev_file, "_dev.exs") + + # Generate regular migration - should remove dev migration + AshSqlite.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + auto_name: true + ) + + # Should only have regular migration now + files = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") + assert length(files) == 1 + assert [regular_file] = files + refute String.contains?(regular_file, "_dev.exs") + end + + test "requires name when not using dev option" do + defposts do + attributes do + uuid_primary_key(:id) + attribute(:title, :string, public?: true) + end + end + + defdomain([Post]) + + assert_raise RuntimeError, ~r/Name must be provided/, fn -> + AshSqlite.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path" + ) + end + end + end +end diff --git a/test/dev_test.db b/test/dev_test.db new file mode 100644 index 0000000..28564ef Binary files /dev/null and b/test/dev_test.db differ diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index ee21de0..42a5414 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -90,7 +90,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) :ok @@ -170,7 +171,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) :ok @@ -198,7 +200,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -227,7 +230,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -253,7 +257,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -278,7 +283,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -306,7 +312,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -336,7 +343,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -367,7 +375,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = @@ -402,7 +411,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) :ok @@ -438,7 +448,8 @@ defmodule AshSqlite.MigrationGeneratorTest do AshSqlite.MigrationGenerator.generate(domain, snapshot_path: "test_snapshot_path", migration_path: "test_migration_path", - check: true + check: true, + auto_name: true ) ) == {:shutdown, 1} @@ -481,7 +492,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") @@ -516,7 +528,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [file] = Path.wildcard("test_migration_path/**/*_migrate_resources*.exs") @@ -551,7 +564,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) defposts Post2 do @@ -575,7 +589,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert file = @@ -663,7 +678,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) [domain: Domain] @@ -700,7 +716,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) end) @@ -754,7 +771,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) :ok @@ -794,7 +812,8 @@ defmodule AshSqlite.MigrationGeneratorTest do snapshot_path: "test_snapshots_path", migration_path: "test_migration_path", quiet: true, - format: false + format: false, + auto_name: true ) assert [_file1, file2] = diff --git a/test/support/dev_test_repo.ex b/test/support/dev_test_repo.ex new file mode 100644 index 0000000..43b3c1e --- /dev/null +++ b/test/support/dev_test_repo.ex @@ -0,0 +1,13 @@ +defmodule AshSqlite.DevTestRepo do + @moduledoc false + use AshSqlite.Repo, + otp_app: :ash_sqlite + + def on_transaction_begin(data) do + send(self(), data) + end + + def prefer_transaction?, do: false + + def prefer_transaction_for_atomic_updates?, do: false +end diff --git a/test/test_helper.exs b/test/test_helper.exs index 5329339..7a3c703 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -2,5 +2,7 @@ ExUnit.start() ExUnit.configure(stacktrace_depth: 100) AshSqlite.TestRepo.start_link() +AshSqlite.DevTestRepo.start_link() Ecto.Adapters.SQL.Sandbox.mode(AshSqlite.TestRepo, :manual) +Ecto.Adapters.SQL.Sandbox.mode(AshSqlite.DevTestRepo, :manual)