diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex index c36dcfdd..c0dddb48 100644 --- a/lib/ecto/migration.ex +++ b/lib/ecto/migration.ex @@ -260,6 +260,19 @@ defmodule Ecto.Migration do such as migrations. For a repository named `MyApp.FooRepo`, `:priv` defaults to "priv/foo_repo" and migrations should be placed at "priv/foo_repo/migrations" + * `:migrations_paths` - a list of paths where migrations are located. This option + allows you to specify multiple migration directories that will all be used when + running migrations. Relative paths are considered relative to the application root + (the directory containing `mix.exs`). If this option is not set, the default path + is derived from the `:priv` configuration. For example: + + config :app, App.Repo, + migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"] + + When using this option, all specified paths will be checked for migrations, and + migrations will be sorted by version across all directories as if they were in + a single directory. + * `:start_apps_before_migration` - A list of applications to be started before running migrations. Used by `Ecto.Migrator.with_repo/3` and the migration tasks: diff --git a/lib/ecto/migrator.ex b/lib/ecto/migrator.ex index 20ae7bc4..e7c5a6bf 100644 --- a/lib/ecto/migrator.ex +++ b/lib/ecto/migrator.ex @@ -189,13 +189,78 @@ defmodule Ecto.Migrator do This function accepts an optional second parameter to customize the migrations directory. This can be used to specify a custom migrations path. + + > #### Discouraged {: .warning} + > + > If your repository is configured with multiple migration paths via + > `:migrations_paths`, this function will raise an error. Use + > `migrations_paths/1` instead. + """ @spec migrations_path(Ecto.Repo.t(), String.t()) :: String.t() def migrations_path(repo, directory \\ "migrations") do config = repo.config() - priv = config[:priv] || "priv/#{repo |> Module.split() |> List.last() |> Macro.underscore()}" - app = Keyword.fetch!(config, :otp_app) - Application.app_dir(app, Path.join(priv, directory)) + + case config[:migrations_paths] do + [_first, _second | _rest] -> + raise ArgumentError, """ + cannot use migrations_path/1 when multiple migration paths are configured. + + The repository #{inspect(repo)} has #{length(config[:migrations_paths])} migration paths configured + via :migrations_paths. Please use migrations_paths/1 instead to get all configured paths. + """ + + _other -> + hd(migrations_paths(repo, directory: directory)) + end + end + + @doc """ + Gets the migrations paths from a repository configuration. + + This function checks the repository configuration for the `:migrations_paths` + option. If found, it returns a list of absolute paths by resolving any relative + paths against the application directory. If not found, it returns a single-element + list containing the default migrations path. + + Relative paths in the `:migrations_paths` configuration are considered relative + to the root of the application (the directory containing `mix.exs`). + + ## Examples + + # In config/config.exs + config :my_app, MyApp.Repo, + migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"] + + """ + @spec migrations_paths(Ecto.Repo.t(), Keyword.t()) :: [String.t()] + def migrations_paths(repo, opts \\ []) do + config = repo.config() + + case config[:migrations_paths] do + nil -> + priv = + config[:priv] || "priv/#{repo |> Module.split() |> List.last() |> Macro.underscore()}" + + app = Keyword.fetch!(config, :otp_app) + directory = Keyword.get(opts, :directory, "migrations") + [Application.app_dir(app, Path.join(priv, directory))] + + paths when is_list(paths) -> + app = Keyword.fetch!(config, :otp_app) + + Enum.map(paths, fn path -> + if Path.type(path) == :absolute do + path + else + Application.app_dir(app, path) + end + end) + + other -> + raise ArgumentError, + ":migrations_paths must be a list of paths, got: #{inspect(other)}" + end end @doc """ @@ -372,13 +437,13 @@ defmodule Ecto.Migrator do Equivalent to: - Ecto.Migrator.run(repo, [Ecto.Migrator.migrations_path(repo)], direction, opts) + Ecto.Migrator.run(repo, Ecto.Migrator.migrations_paths(repo), direction, opts) See `run/4` for more information. """ @spec run(Ecto.Repo.t(), atom, Keyword.t()) :: [integer] def run(repo, direction, opts) do - run(repo, [migrations_path(repo)], direction, opts) + run(repo, migrations_paths(repo), direction, opts) end @doc ~S""" @@ -464,12 +529,12 @@ defmodule Ecto.Migrator do Equivalent to: - Ecto.Migrator.migrations(repo, [Ecto.Migrator.migrations_path(repo)]) + Ecto.Migrator.migrations(repo, Ecto.Migrator.migrations_paths(repo)) """ @spec migrations(Ecto.Repo.t()) :: [{:up | :down, id :: integer(), name :: String.t()}] def migrations(repo) do - migrations(repo, [migrations_path(repo)]) + migrations(repo, migrations_paths(repo)) end @doc """ diff --git a/lib/mix/ecto_sql.ex b/lib/mix/ecto_sql.ex index bf76566d..88bc948e 100644 --- a/lib/mix/ecto_sql.ex +++ b/lib/mix/ecto_sql.ex @@ -3,11 +3,45 @@ defmodule Mix.EctoSQL do @doc """ Ensures the given repository's migrations paths exists on the file system. + + This function checks for migrations paths in the following order: + 1. Command-line options (`--migrations_path`) + 2. Repository configuration (`:migrations_paths`) + 3. Default path based on `:priv` configuration or "priv/repo/migrations" """ @spec ensure_migrations_paths(Ecto.Repo.t(), Keyword.t()) :: [String.t()] def ensure_migrations_paths(repo, opts) do paths = Keyword.get_values(opts, :migrations_path) - paths = if paths == [], do: [Path.join(source_repo_priv(repo), "migrations")], else: paths + + paths = + if paths == [] do + # Use repo config if available, otherwise fall back to default + config = repo.config() + + case config[:migrations_paths] do + nil -> + [Path.join(source_repo_priv(repo), "migrations")] + + config_paths when is_list(config_paths) -> + app = Keyword.fetch!(config, :otp_app) + # In Mix context, we use deps_paths or cwd for path resolution + base_dir = Mix.Project.deps_paths()[app] || File.cwd!() + + Enum.map(config_paths, fn path -> + if Path.type(path) == :absolute do + path + else + Path.join(base_dir, path) + end + end) + + other -> + raise ArgumentError, + ":migrations_paths must be a list of paths, got: #{inspect(other)}" + end + else + paths + end if not Mix.Project.umbrella?() do for path <- paths, not File.dir?(path) do diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs index a33d6530..87f1df62 100644 --- a/test/ecto/migrator_test.exs +++ b/test/ecto/migrator_test.exs @@ -907,6 +907,237 @@ defmodule Ecto.MigratorTest do expected = "priv/test_repo/custom" assert path == Application.app_dir(TestRepo.config()[:otp_app], expected) end + + test "works with single migrations_paths configured" do + defmodule RepoWithSinglePath do + def config do + [ + otp_app: :ecto_sql, + migrations_paths: ["priv/repo/migrations"] + ] + end + end + + path = migrations_path(RepoWithSinglePath) + app_dir = Application.app_dir(:ecto_sql) + assert path == Path.join(app_dir, "priv/repo/migrations") + end + + test "raises when multiple migrations_paths are configured" do + defmodule RepoWithMultiplePaths do + def config do + [ + otp_app: :ecto_sql, + migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"] + ] + end + end + + assert_raise ArgumentError, + ~r/cannot use migrations_path\/1 when multiple migration paths are configured/, + fn -> + migrations_path(RepoWithMultiplePaths) + end + end + end + + describe "migrations_paths" do + defmodule RepoWithMigrationsPaths do + def config do + [ + otp_app: :ecto_sql, + migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"] + ] + end + end + + defmodule RepoWithAbsoluteMigrationsPaths do + def config do + [ + otp_app: :ecto_sql, + migrations_paths: ["/absolute/path/migrations", "relative/path/migrations"] + ] + end + end + + defmodule RepoWithInvalidMigrationsPaths do + def config do + [ + otp_app: :ecto_sql, + migrations_paths: "not_a_list" + ] + end + end + + test "returns default path when migrations_paths is not configured" do + paths = migrations_paths(TestRepo) + expected = [migrations_path(TestRepo)] + assert paths == expected + end + + test "returns configured paths with relative paths resolved" do + paths = migrations_paths(RepoWithMigrationsPaths) + app_dir = Application.app_dir(:ecto_sql) + + assert paths == [ + Path.join(app_dir, "priv/repo/migrations"), + Path.join(app_dir, "priv/repo/tenant_migrations") + ] + end + + test "handles absolute paths correctly" do + paths = migrations_paths(RepoWithAbsoluteMigrationsPaths) + app_dir = Application.app_dir(:ecto_sql) + + assert paths == [ + "/absolute/path/migrations", + Path.join(app_dir, "relative/path/migrations") + ] + end + + test "raises error when migrations_paths is not a list" do + assert_raise ArgumentError, + ":migrations_paths must be a list of paths, got: \"not_a_list\"", + fn -> + migrations_paths(RepoWithInvalidMigrationsPaths) + end + end + end + + describe "migrations with migrations_paths set" do + @describetag migrated_versions: [] + + test "runs migrations from multiple configured paths in order" do + in_tmp(fn path -> + # Create two migration directories + File.mkdir_p!("migrations_a") + File.mkdir_p!("migrations_b") + + # Create migrations in different directories + create_migration("migrations_a/15_migration_a.exs") + create_migration("migrations_b/16_migration_b.exs") + create_migration("migrations_a/17_migration_c.exs") + + # Use explicit paths since we're in a tmp directory + paths = [ + Path.join(path, "migrations_a"), + Path.join(path, "migrations_b") + ] + + # Run all migrations + assert run(TestRepo, paths, :up, all: true, log: false) == [15, 16, 17] + end) + end + + test "migrations/1 reports status from multiple configured paths" do + in_tmp(fn path -> + File.mkdir_p!("migrations_a") + File.mkdir_p!("migrations_b") + + create_migration("migrations_a/18_migration_a.exs") + create_migration("migrations_b/19_migration_b.exs") + create_migration("migrations_a/20_migration_c.exs") + + paths = [ + Path.join(path, "migrations_a"), + Path.join(path, "migrations_b") + ] + + # All should be pending (down) + expected = [ + {:down, 18, "migration_a"}, + {:down, 19, "migration_b"}, + {:down, 20, "migration_c"} + ] + + assert migrations(TestRepo, paths) == expected + end) + end + + test "can rollback migrations from multiple paths" do + in_tmp(fn path -> + File.mkdir_p!("migrations_a") + File.mkdir_p!("migrations_b") + + create_migration("migrations_a/21_migration_a.exs") + create_migration("migrations_b/22_migration_b.exs") + create_migration("migrations_a/23_migration_c.exs") + + paths = [ + Path.join(path, "migrations_a"), + Path.join(path, "migrations_b") + ] + + # Run all migrations + assert run(TestRepo, paths, :up, all: true, log: false) == [21, 22, 23] + + # Rollback step by step + capture_io(:stderr, fn -> + assert run(TestRepo, paths, :down, step: 1, log: false) == [23] + assert run(TestRepo, paths, :down, step: 2, log: false) == [22, 21] + end) + end) + end + + test "migrations from multiple paths are sorted by version" do + in_tmp(fn path -> + File.mkdir_p!("migrations_a") + File.mkdir_p!("migrations_b") + + # Create migrations with versions that would be out of order if grouped by directory + create_migration("migrations_b/24_first.exs") + create_migration("migrations_a/25_second.exs") + create_migration("migrations_b/26_third.exs") + create_migration("migrations_a/27_fourth.exs") + + paths = [ + Path.join(path, "migrations_a"), + Path.join(path, "migrations_b") + ] + + # They should run in version order, not directory order + assert run(TestRepo, paths, :up, all: true, log: false) == [24, 25, 26, 27] + + # Verify migrations status is also in correct order + expected = [ + {:up, 24, "first"}, + {:up, 25, "second"}, + {:up, 26, "third"}, + {:up, 27, "fourth"} + ] + + assert migrations(TestRepo, paths) == expected + end) + end + + test "handles missing migration files from one path" do + in_tmp(fn path -> + File.mkdir_p!("migrations_a") + File.mkdir_p!("migrations_b") + + create_migration("migrations_a/28_migration_a.exs") + create_migration("migrations_b/29_migration_b.exs") + + paths = [ + Path.join(path, "migrations_a"), + Path.join(path, "migrations_b") + ] + + # Run migrations + assert run(TestRepo, paths, :up, all: true, log: false) == [28, 29] + + # Delete a migration file from one directory + File.rm("migrations_b/29_migration_b.exs") + + # Should show file not found for the deleted migration + expected = [ + {:up, 28, "migration_a"}, + {:up, 29, "** FILE NOT FOUND **"} + ] + + assert migrations(TestRepo, paths) == expected + end) + end end describe "with_repo" do diff --git a/test/mix/tasks/ecto.migrate_test.exs b/test/mix/tasks/ecto.migrate_test.exs index 43435b49..f6c130d9 100644 --- a/test/mix/tasks/ecto.migrate_test.exs +++ b/test/mix/tasks/ecto.migrate_test.exs @@ -160,4 +160,86 @@ defmodule Mix.Tasks.Ecto.MigrateTest do assert Process.get(:started) end + + describe "migrations_paths config" do + defmodule RepoWithMigrationsPaths do + def start_link(_) do + Process.put(:started, true) + + Task.start_link(fn -> + Process.flag(:trap_exit, true) + + receive do + {:EXIT, _, :normal} -> :ok + end + end) + end + + def stop do + :ok + end + + def __adapter__ do + EctoSQL.TestAdapter + end + + def config do + migrations_path_1 = + Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + + migrations_path_2 = + Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + + [ + priv: "tmp/#{inspect(Ecto.Migrate)}", + otp_app: :ecto_sql, + migrations_paths: [ + Path.relative_to(migrations_path_1, File.cwd!()), + Path.relative_to(migrations_path_2, File.cwd!()) + ] + ] + end + end + + setup do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + File.mkdir_p!(path1) + File.mkdir_p!(path2) + :ok + end + + test "uses migrations_paths from repo config when no --migrations-path flag" do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + + run(["-r", to_string(RepoWithMigrationsPaths)], fn repo, paths, direction, _opts -> + assert repo == RepoWithMigrationsPaths + assert length(paths) == 2 + assert Path.expand(Enum.at(paths, 0)) == Path.expand(path1) + assert Path.expand(Enum.at(paths, 1)) == Path.expand(path2) + assert direction == :up + [] + end) + + assert Process.get(:started) + end + + test "command-line --migrations-path takes precedence over repo config" do + custom_path = Path.join([tmp_path(), inspect(Ecto.Migrate), "cli_migrations"]) + File.mkdir_p!(custom_path) + + run( + ["-r", to_string(RepoWithMigrationsPaths), "--migrations-path", custom_path], + fn repo, [path], direction, _opts -> + assert repo == RepoWithMigrationsPaths + assert path == custom_path + assert direction == :up + [] + end + ) + + assert Process.get(:started) + end + end end diff --git a/test/mix/tasks/ecto.migrations_test.exs b/test/mix/tasks/ecto.migrations_test.exs index 11468ece..91135ded 100644 --- a/test/mix/tasks/ecto.migrations_test.exs +++ b/test/mix/tasks/ecto.migrations_test.exs @@ -113,4 +113,121 @@ defmodule Mix.Tasks.Ecto.MigrationsTest do fn _ -> :ok end ) end + + describe "migrations_paths config" do + defmodule RepoWithMigrationsPaths do + def start_link(_) do + Process.put(:started, true) + + Task.start_link(fn -> + Process.flag(:trap_exit, true) + + receive do + {:EXIT, _, :normal} -> :ok + end + end) + end + + def stop do + :ok + end + + def __adapter__ do + EctoSQL.TestAdapter + end + + def config do + migrations_path_1 = + Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_1"]) + + migrations_path_2 = + Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_2"]) + + [ + priv: "tmp/#{inspect(Ecto.Migrations)}", + otp_app: :ecto_sql, + migrations_paths: [ + Path.relative_to(migrations_path_1, File.cwd!()), + Path.relative_to(migrations_path_2, File.cwd!()) + ] + ] + end + end + + setup do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_2"]) + File.mkdir_p!(path1) + File.mkdir_p!(path2) + :ok + end + + test "uses migrations_paths from repo config when no --migrations-path flag" do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrations), "configured_migrations_2"]) + + migrations = fn repo, paths, _opts -> + assert repo == RepoWithMigrationsPaths + assert length(paths) == 2 + assert Path.expand(Enum.at(paths, 0)) == Path.expand(path1) + assert Path.expand(Enum.at(paths, 1)) == Path.expand(path2) + + [ + {:up, 20_230_000_000_001, "migration_from_path_1"}, + {:down, 20_230_000_000_002, "migration_from_path_2"} + ] + end + + expected_output = """ + + Repo: Mix.Tasks.Ecto.MigrationsTest.RepoWithMigrationsPaths + + Status Migration ID Migration Name + -------------------------------------------------- + up 20230000000001 migration_from_path_1 + down 20230000000002 migration_from_path_2 + """ + + run(["-r", to_string(RepoWithMigrationsPaths)], migrations, fn output -> + assert output == expected_output + end) + + assert Process.get(:started) + end + + test "command-line --migrations-path takes precedence over repo config" do + custom_path = Path.join([tmp_path(), inspect(Ecto.Migrations), "cli_migrations"]) + File.mkdir_p!(custom_path) + + migrations = fn repo, [path], _opts -> + assert repo == RepoWithMigrationsPaths + assert path == custom_path + + [ + {:up, 20_230_000_000_003, "cli_migration_1"}, + {:down, 20_230_000_000_004, "cli_migration_2"} + ] + end + + expected_output = """ + + Repo: Mix.Tasks.Ecto.MigrationsTest.RepoWithMigrationsPaths + + Status Migration ID Migration Name + -------------------------------------------------- + up 20230000000003 cli_migration_1 + down 20230000000004 cli_migration_2 + """ + + run( + ["-r", to_string(RepoWithMigrationsPaths), "--migrations-path", custom_path], + migrations, + fn output -> + assert output == expected_output + end + ) + + assert Process.get(:started) + end + end end diff --git a/test/mix/tasks/ecto.rollback_test.exs b/test/mix/tasks/ecto.rollback_test.exs index ce2ebebd..d1691512 100644 --- a/test/mix/tasks/ecto.rollback_test.exs +++ b/test/mix/tasks/ecto.rollback_test.exs @@ -120,4 +120,86 @@ defmodule Mix.Tasks.Ecto.RollbackTest do assert Process.get(:started) end + + describe "migrations_paths config" do + defmodule RepoWithMigrationsPaths do + def start_link(_) do + Process.put(:started, true) + + Task.start_link(fn -> + Process.flag(:trap_exit, true) + + receive do + {:EXIT, _, :normal} -> :ok + end + end) + end + + def stop do + :ok + end + + def __adapter__ do + EctoSQL.TestAdapter + end + + def config do + migrations_path_1 = + Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + + migrations_path_2 = + Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + + [ + priv: "tmp/#{inspect(Ecto.Migrate)}", + otp_app: :ecto_sql, + migrations_paths: [ + Path.relative_to(migrations_path_1, File.cwd!()), + Path.relative_to(migrations_path_2, File.cwd!()) + ] + ] + end + end + + setup do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + File.mkdir_p!(path1) + File.mkdir_p!(path2) + :ok + end + + test "uses migrations_paths from repo config when no --migrations-path flag" do + path1 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_1"]) + path2 = Path.join([tmp_path(), inspect(Ecto.Migrate), "configured_migrations_2"]) + + run(["-r", to_string(RepoWithMigrationsPaths)], fn repo, paths, direction, _opts -> + assert repo == RepoWithMigrationsPaths + assert length(paths) == 2 + assert Path.expand(Enum.at(paths, 0)) == Path.expand(path1) + assert Path.expand(Enum.at(paths, 1)) == Path.expand(path2) + assert direction == :down + [] + end) + + assert Process.get(:started) + end + + test "command-line --migrations-path takes precedence over repo config" do + custom_path = Path.join([tmp_path(), inspect(Ecto.Migrate), "cli_migrations"]) + File.mkdir_p!(custom_path) + + run( + ["-r", to_string(RepoWithMigrationsPaths), "--migrations-path", custom_path], + fn repo, [path], direction, _opts -> + assert repo == RepoWithMigrationsPaths + assert path == custom_path + assert direction == :down + [] + end + ) + + assert Process.get(:started) + end + end end