Add Ecto.Migrator.with_repo/2 to start/stop apps for migrations (#113)

This will be useful to create minimum release scripts
that migrate or rollback the repository, such as:

    defmodule MyApp.Release do
      @app :my_app

      def migrate do
        for repo <- repos() do
          Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :up, all: true))
        end
      end

      def rollback(repo, version) do
        Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version))
      end

      defp repos do
        Application.get_env(@app, :ecto_repos, [])
      end
    end

There is also a configuration called :start_apps_before_migration
that will help guarantee any auxiliary app is started before
migrations run.

lib/ecto/migration.ex

@@ -204,6 +204,11 @@ defmodule Ecto.Migration do
 
           config :app, App.Repo, migration_default_prefix: "my_prefix"
 
+    * `: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:
+
+          config :app, App.Repo, start_apps_before_migration: [:ssl, :some_custom_logger]
+
   """
 
   @doc """

lib/ecto/migrator.ex

@@ -25,6 +25,69 @@ defmodule Ecto.Migrator do
   alias Ecto.Migration.Runner
   alias Ecto.Migration.SchemaMigration
 
+  @doc """
+  Ensures the repo is started to perform migration operations.
+
+  All of the application reqeuired to run the repo will be started
+  before hand with chosen mode. If the repo has not yet been started,
+  it is manually started  before, with a `:pool_size` of 2, the given
+  function is executed, and the repo is terminated. If the repo was
+  already started, then the function is directly executed, without
+  terminating the repo afterwards.
+
+  The repo may also configure a `:start_apps_before_migration` option
+  which is a list of applications to be started before the migration
+  runs.
+
+  It returns `{:ok, fun_return, apps}`, with all apps that have been
+  started, or `{:error, term}`.
+
+  ## Options
+
+    * `:pool_size` - The pool size to start the repo for migrations.
+      Defaults to 2.
+    * `:mode` - The mode to start all applications.
+      Defaults to `:permanent`.
+
+  ## Examples
+
+      {:ok, _, _} =
+        Ecto.Migrator.with_repo(repo, fn repo ->
+          Ecto.Migrator.run(repo, :up, all: true)
+        end)
+
+  """
+  def with_repo(repo, fun, opts \\ []) do
+    config = repo.config()
+    mode = Keyword.get(opts, :mode, :permanent)
+    apps = [:ecto_sql | config[:start_apps_before_migration] || []]
+
+    extra_started =
+      Enum.flat_map(apps, fn app ->
+        {:ok, started} = Application.ensure_all_started(app, mode)
+        started
+      end)
+
+    {:ok, repo_started} = repo.__adapter__.ensure_all_started(config, mode)
+    started = extra_started ++ repo_started
+    pool_size = Keyword.get(opts, :pool_size, 2)
+
+    case repo.start_link(pool_size: pool_size) do
+      {:ok, _} ->
+        try do
+          {:ok, fun.(repo), started}
+        after
+          repo.stop()
+        end
+
+      {:error, {:already_started, _pid}} ->
+        {:ok, fun.(repo), started}
+
+      {:error, _} = error ->
+        error
+    end
+  end
+
   @doc """
   Gets the migrations path from a repository.
   """

lib/mix/ecto_sql.ex

@@ -1,41 +1,6 @@
 defmodule Mix.EctoSQL do
   @moduledoc false
 
-  @doc """
-  Ensures the given repository is started and running.
-  """
-  @spec ensure_started(Ecto.Repo.t, Keyword.t) :: {:ok, pid | nil, [atom]}
-  def ensure_started(repo, opts) do
-    {:ok, started} = Application.ensure_all_started(:ecto_sql)
-
-    # If we starting EctoSQL just now, assume
-    # logger has not been properly booted yet.
-    if :ecto_sql in started && Process.whereis(Logger) do
-      backends = Application.get_env(:logger, :backends, [])
-      try do
-        Logger.App.stop
-        Application.put_env(:logger, :backends, [:console])
-        :ok = Logger.App.start
-      after
-        Application.put_env(:logger, :backends, backends)
-      end
-    end
-
-    {:ok, apps} = repo.__adapter__.ensure_all_started(repo.config(), :temporary)
-    pool_size = Keyword.get(opts, :pool_size, 2)
-
-    case repo.start_link(pool_size: pool_size) do
-      {:ok, pid} ->
-        {:ok, pid, apps}
-
-      {:error, {:already_started, _pid}} ->
-        {:ok, nil, apps}
-
-      {:error, error} ->
-        Mix.raise "Could not start repo #{inspect repo}, error: #{inspect error}"
-    end
-  end
-
   @doc """
   Ensures the given repository's migrations path exists on the file system.
   """

lib/mix/tasks/ecto.migrate.ex

@@ -74,7 +74,7 @@ defmodule Mix.Tasks.Ecto.Migrate do
     * `--to` - run all migrations up to and including version
     * `--quiet` - do not log migration commands
     * `--prefix` - the prefix to run migrations on
-    * `--pool-size` - the pool size if the repository is started only for the task (defaults to 1)
+    * `--pool-size` - the pool size if the repository is started only for the task (defaults to 2)
     * `--log-sql` - log the raw sql migrations are running
     * `--strict-version-order` - abort when applying a migration with old timestamp
     * `--no-compile` - does not compile applications before migrating
@@ -97,21 +97,28 @@ defmodule Mix.Tasks.Ecto.Migrate do
         do: Keyword.merge(opts, [log: false, log_sql: false]),
         else: opts
 
-    Enum.each repos, fn repo ->
+    # Start ecto_sql explicitly before as we don't need
+    # to restart those apps if migrated.
+    {:ok, _} = Application.ensure_all_started(:ecto_sql)
+
+    for repo <- repos do
       ensure_repo(repo, args)
       path = ensure_migrations_path(repo)
-      {:ok, pid, apps} = ensure_started(repo, opts)
-
       pool = repo.config[:pool]
-      migrated =
+
+      fun =
         if function_exported?(pool, :unboxed_run, 2) do
-          pool.unboxed_run(repo, fn -> migrator.(repo, path, :up, opts) end)
+          &pool.unboxed_run(&1, fn -> migrator.(&1, path, :up, opts) end)
         else
-          migrator.(repo, path, :up, opts)
+          &migrator.(&1, path, :up, opts)
         end
 
-      pid && repo.stop()
-      restart_apps_if_migrated(apps, migrated)
+      case Ecto.Migrator.with_repo(repo, fun, [mode: :temporary] ++ opts) do
+        {:ok, migrated, apps} -> restart_apps_if_migrated(apps, migrated)
+        {:error, error} -> Mix.raise "Could not start repo #{inspect repo}, error: #{inspect error}"
+      end
     end
+
+    :ok
   end
 end

lib/mix/tasks/ecto.migrations.ex

@@ -36,27 +36,31 @@ defmodule Mix.Tasks.Ecto.Migrations do
   def run(args, migrations \\ &Ecto.Migrator.migrations/2, puts \\ &IO.puts/1) do
     repos = parse_repo(args)
 
-    result =
-      Enum.map(repos, fn repo ->
-        ensure_repo(repo, args)
-        path = ensure_migrations_path(repo)
-        {:ok, pid, _} = ensure_started(repo, all: true)
-        repo_status = migrations.(repo, path)
-        pid && repo.stop()
-
-        """
-
-        Repo: #{inspect(repo)}
-
-          Status    Migration ID    Migration Name
-        --------------------------------------------------
-        """ <>
-          Enum.map_join(repo_status, "\n", fn {status, number, description} ->
-            "  #{format(status, 10)}#{format(number, 16)}#{description}"
-          end) <> "\n"
-      end)
-
-    puts.(Enum.join(result, "\n"))
+    for repo <- repos do
+      ensure_repo(repo, args)
+      path = ensure_migrations_path(repo)
+
+      case Ecto.Migrator.with_repo(repo, &migrations.(&1, path), [mode: :temporary]) do
+        {:ok, repo_status, _} ->
+          puts.(
+            """
+
+            Repo: #{inspect(repo)}
+
+              Status    Migration ID    Migration Name
+            --------------------------------------------------
+            """ <>
+              Enum.map_join(repo_status, "\n", fn {status, number, description} ->
+                "  #{format(status, 10)}#{format(number, 16)}#{description}"
+              end) <> "\n"
+          )
+
+        {:error, error} ->
+          Mix.raise "Could not start repo #{inspect repo}, error: #{inspect error}"
+      end
+    end
+
+    :ok
   end
 
   defp format(content, pad) do

lib/mix/tasks/ecto.rollback.ex

@@ -71,7 +71,7 @@ defmodule Mix.Tasks.Ecto.Rollback do
     * `--to` - revert all migrations down to and including version
     * `--quiet` - do not log migration commands
     * `--prefix` - the prefix to run migrations on
-    * `--pool-size` - the pool size if the repository is started only for the task (defaults to 1)
+    * `--pool-size` - the pool size if the repository is started only for the task (defaults to 2)
     * `--log-sql` - log the raw sql migrations are running
     * `--no-compile` - does not compile applications before rolling back
     * `--no-deps-check` - does not check depedendencies before rolling back
@@ -93,21 +93,28 @@ defmodule Mix.Tasks.Ecto.Rollback do
         do: Keyword.merge(opts, [log: false, log_sql: false]),
         else: opts
 
-    Enum.each repos, fn repo ->
+    # Start ecto_sql explicitly before as we don't need
+    # to restart those apps if migrated.
+    {:ok, _} = Application.ensure_all_started(:ecto_sql)
+
+    for repo <- repos do
       ensure_repo(repo, args)
       path = ensure_migrations_path(repo)
-      {:ok, pid, apps} = ensure_started(repo, opts)
-
       pool = repo.config[:pool]
-      migrated =
+
+      fun =
         if function_exported?(pool, :unboxed_run, 2) do
-          pool.unboxed_run(repo, fn -> migrator.(repo, path, :down, opts) end)
+          &pool.unboxed_run(&1, fn -> migrator.(&1, path, :down, opts) end)
         else
-          migrator.(repo, path, :down, opts)
+          &migrator.(&1, path, :down, opts)
         end
 
-      pid && repo.stop()
-      restart_apps_if_migrated(apps, migrated)
+      case Ecto.Migrator.with_repo(repo, fun, [mode: :temporary] ++ opts) do
+        {:ok, migrated, apps} -> restart_apps_if_migrated(apps, migrated)
+        {:error, error} -> Mix.raise "Could not start repo #{inspect repo}, error: #{inspect error}"
+      end
     end
+
+    :ok
   end
 end

test/ecto/migrator_test.exs

@@ -512,4 +512,43 @@ defmodule Ecto.MigratorTest do
       refute log =~ "before_commit"
     end
   end
+
+  describe "with_repo" do
+    defmodule Repo do
+      def start_link(opts) do
+        assert opts[:pool_size] == 2
+        Process.get(:start_link)
+      end
+
+      def stop() do
+        Process.put(:stopped, true)
+      end
+
+      def __adapter__ do
+        EctoSQL.TestAdapter
+      end
+
+      def config do
+        [priv: Process.get(:priv), otp_app: :ecto_sql]
+      end
+    end
+
+    test "starts and stops repo" do
+      Process.put(:start_link, {:ok, self()})
+      assert with_repo(Repo, fn Repo -> :one end) == {:ok, :one, []}
+      assert Process.get(:stopped)
+    end
+
+    test "runs with existing repo" do
+      Process.put(:start_link, {:error, {:already_started, self()}})
+      assert with_repo(Repo, fn Repo -> :two end) == {:ok, :two, []}
+      refute Process.get(:stopped)
+    end
+
+    test "handles errors" do
+      Process.put(:start_link, {:error, :oops})
+      assert with_repo(Repo, fn Repo -> raise "never invoked" end) == {:error, :oops}
+      refute Process.get(:stopped)
+    end
+  end
 end

test/mix/ecto_sql_test.exs

@@ -3,31 +3,11 @@ defmodule Mix.EctoSQLTest do
   import Mix.EctoSQL
 
   defmodule Repo do
-    def start_link(opts) do
-      assert opts[:pool_size] == 2
-      Process.get(:start_link)
-    end
-
-    def __adapter__ do
-      EctoSQL.TestAdapter
-    end
-
     def config do
       [priv: Process.get(:priv), otp_app: :ecto_sql]
     end
   end
 
-  test "ensure_started" do
-    Process.put(:start_link, {:ok, self()})
-    assert ensure_started(Repo, []) == {:ok, self(), []}
-
-    Process.put(:start_link, {:error, {:already_started, self()}})
-    assert ensure_started(Repo, []) == {:ok, nil, []}
-
-    Process.put(:start_link, {:error, self()})
-    assert_raise Mix.Error, fn -> ensure_started(Repo, []) end
-  end
-
   test "source_priv_repo" do
     Process.put(:priv, nil)
     assert source_repo_priv(Repo) == Path.expand("priv/repo", File.cwd!)