From 1dafcc228aee150a3f0a81de2e3f6f4b9f74f5c9 Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Sun, 20 Oct 2024 14:07:20 +1100 Subject: [PATCH 1/2] Add prefix option to CheckRepoStatus When using schema based multi-tenancy, prefix can be used to check pending migrations for a "dev" tenant --- lib/phoenix_ecto/check_repo_status.ex | 16 +++++-- lib/phoenix_ecto/exceptions.ex | 4 +- lib/phoenix_ecto/plug.ex | 8 ++-- test/phoenix_ecto/check_repo_status_test.exs | 49 ++++++++++++++++---- 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/lib/phoenix_ecto/check_repo_status.ex b/lib/phoenix_ecto/check_repo_status.ex index 32e93c2..79f288d 100644 --- a/lib/phoenix_ecto/check_repo_status.ex +++ b/lib/phoenix_ecto/check_repo_status.ex @@ -10,13 +10,14 @@ defmodule Phoenix.Ecto.CheckRepoStatus do * `:otp_app` - name of the application which the repos are fetched from * `:migration_paths` - a function that accepts a repo and returns a migration directory, or a list of migration directories, that is used to check for pending migrations * `:migration_lock` - the locking strategy used by the Ecto Adapter when checking for pending migrations. Set to `false` to disable migration locks. + * `:prefix` - the prefix used to check for pending migrations. """ @behaviour Plug alias Plug.Conn - @migration_opts [:migration_lock] + @migration_opts [:migration_lock, :prefix] @compile {:no_warn_undefined, Ecto.Migrator} def init(opts) do @@ -60,16 +61,23 @@ defmodule Phoenix.Ecto.CheckRepoStatus do end) true = is_function(migrations_fun, 3) + migration_opts = Keyword.take(opts, @migration_opts) try do repo - |> migrations_fun.(dirs, Keyword.take(opts, @migration_opts)) + |> migrations_fun.(dirs, migration_opts) |> Enum.any?(fn {status, _version, _migration} -> status == :down end) rescue _ -> false else - true -> raise Phoenix.Ecto.PendingMigrationError, repo: repo, directories: dirs - false -> true + true -> + raise Phoenix.Ecto.PendingMigrationError, + repo: repo, + directories: dirs, + migration_opts: migration_opts + + false -> + true end end diff --git a/lib/phoenix_ecto/exceptions.ex b/lib/phoenix_ecto/exceptions.ex index f03e2bf..1284c6f 100644 --- a/lib/phoenix_ecto/exceptions.ex +++ b/lib/phoenix_ecto/exceptions.ex @@ -8,8 +8,8 @@ defmodule Phoenix.Ecto.StorageNotCreatedError do end defmodule Phoenix.Ecto.PendingMigrationError do - @enforce_keys [:repo, :directories] - defexception [:repo, :directories] + @enforce_keys [:repo, :directories, :migration_opts] + defexception [:repo, :directories, :migration_opts] def message(%__MODULE__{repo: repo}) do "there are pending migrations for repo: #{inspect(repo)}. " <> diff --git a/lib/phoenix_ecto/plug.ex b/lib/phoenix_ecto/plug.ex index daff2c1..de38088 100644 --- a/lib/phoenix_ecto/plug.ex +++ b/lib/phoenix_ecto/plug.ex @@ -30,15 +30,17 @@ unless Phoenix.Ecto.PendingMigrationError in excluded_exceptions do defimpl Plug.Exception, for: Phoenix.Ecto.PendingMigrationError do def status(_error), do: 503 - def actions(%{repo: repo, directories: directories}), + def actions(%{repo: repo, directories: directories, migration_opts: migration_opts}), do: [ %{ label: "Run migrations for repo", - handler: {__MODULE__, :migrate, [repo, directories]} + handler: {__MODULE__, :migrate, [repo, directories, migration_opts]} } ] - def migrate(repo, directories), do: Ecto.Migrator.run(repo, directories, :up, all: true) + def migrate(repo, directories, migration_opts) do + Ecto.Migrator.run(repo, directories, :up, Keyword.merge(migration_opts, all: true)) + end end end diff --git a/test/phoenix_ecto/check_repo_status_test.exs b/test/phoenix_ecto/check_repo_status_test.exs index 4c47122..e6bd2dc 100644 --- a/test/phoenix_ecto/check_repo_status_test.exs +++ b/test/phoenix_ecto/check_repo_status_test.exs @@ -103,14 +103,47 @@ defmodule Phoenix.Ecto.CheckRepoStatusTest do conn = conn(:get, "/") - assert_raise(Phoenix.Ecto.PendingMigrationError, fn -> - CheckRepoStatus.call( - conn, - otp_app: :check_repo_ready, - mock_migrations_fn: mock_migrations_fn, - migration_lock: false - ) - end) + exception = + assert_raise(Phoenix.Ecto.PendingMigrationError, fn -> + CheckRepoStatus.call( + conn, + otp_app: :check_repo_ready, + mock_migrations_fn: mock_migrations_fn, + migration_lock: false + ) + end) + + assert exception.migration_opts == [migration_lock: false] + after + Application.delete_env(:check_repo_ready, :ecto_repos) + Process.unregister(StorageUpRepo) + end + + test "supports Ecto's prefix option" do + Process.register(self(), StorageUpRepo) + Application.put_env(:check_repo_ready, :ecto_repos, [StorageUpRepo]) + + mock_migrations_fn = fn _repo, _directories, opts -> + if opts[:prefix] == "tenant_1" do + [{:down, 1, "migration"}] + else + [] + end + end + + conn = conn(:get, "/") + + exception = + assert_raise(Phoenix.Ecto.PendingMigrationError, fn -> + CheckRepoStatus.call( + conn, + otp_app: :check_repo_ready, + mock_migrations_fn: mock_migrations_fn, + prefix: "tenant_1" + ) + end) + + assert exception.migration_opts == [prefix: "tenant_1"] after Application.delete_env(:check_repo_ready, :ecto_repos) Process.unregister(StorageUpRepo) From 6c36d02e8702a34884cd540b16b01ff12f9a54ab Mon Sep 17 00:00:00 2001 From: Steve Hodgkiss Date: Mon, 21 Oct 2024 13:37:09 +1100 Subject: [PATCH 2/2] Handle the case where error is raised without migration_opts --- lib/phoenix_ecto/plug.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/phoenix_ecto/plug.ex b/lib/phoenix_ecto/plug.ex index de38088..6291d01 100644 --- a/lib/phoenix_ecto/plug.ex +++ b/lib/phoenix_ecto/plug.ex @@ -39,7 +39,7 @@ unless Phoenix.Ecto.PendingMigrationError in excluded_exceptions do ] def migrate(repo, directories, migration_opts) do - Ecto.Migrator.run(repo, directories, :up, Keyword.merge(migration_opts, all: true)) + Ecto.Migrator.run(repo, directories, :up, Keyword.merge(migration_opts || [], all: true)) end end end