Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Extra Ecto Mix tasks for migrations #131

Merged
merged 6 commits into from

4 participants

@christhekeele

Updating the integrations tests.

@ericmj
Collaborator

To fix the warnings we need to purge the modules before running migrations on the same module again. You can add a function like this: https://github.com/elixir-lang/elixir/blob/master/lib/mix/test/test_helper.exs#L52 to one of the test helper files in test/support/.

Don't add questions to commits, rather post them as comments on the PR or ask on IRC. So please rebase the two last comments so the commit messages are nicer.

The rest looks really good, great work!

@ericmj
Collaborator

Im not sure of the usefulness of running or reverting a single migration, maybe we should have a parameter that allows you to run/revert up until a given migration version/name.

@christhekeele

I have to agree migrate.next is a little useless. I'm a sucker for parallelism.

I based rollback on the active_record equivalent, which reverts the latest migration. I get a lot of milage out of it when forging a new migration pre-commit: it allows me to easily go back and modify it before it gets pushed out there, saving me multiple 'corrective' migrations in a single commit.

I'm open to renaming rollback:all into something top-level and expressive, but can't think of a great name that wouldn't get confused with rollback.

I like the idea of extending the 'single' versions to accept a version/name or cardinal integer of how far to go, with the latest/pending as a default, but I'm not sure how to reconcile the syntax for to version_number with migrate/rollback n migrations. Thoughts? Which one would be more useful? Could we fit both into the same invocation?

I'll rebase, purge, and drop migrate.next here in a bit.

@kotedo
@ericmj
Collaborator

What do you think of the following tasks:

  • migrate - run up migrations until we are the latest version
  • migrate --version version - run up or down migrations until we are at given version
  • migrate --step steps - run steps number of up migrations
  • rollback - run a single down migration
  • rollback --all - run all down migrations
  • rollback --step steps - run steps number of down migrations

If we want to be able to run a single up migration, we can make migrate do that (like rollback) and make running all up migrations explicit with migrate --all.

This is just a proposal, I would love your inputs.

/cc @josevalim

@kotedo
@christhekeele

I like it. My only objection would be with

migrate --version version - run up or down migrations until we are at given version
migrate --step steps - run steps number of up migrations

That means the migrate task isn't consistent about always moving the schema up. If we're going to allow migrate to take you both directions, I'd rather:

  • migrate - run up migrations until we are the latest version
  • rollback - run a single down migration
  • rollback --all - run all down migrations
  • migrate --to version - run up or down migrations until we are at given version
  • migrate --step n - run number of migrations, positive for up, negative for down, zero for contentedness

migrate, rollback, and rollback --all cover the most common migration scenarios. migrate --version version and migrate --step n provide simple mechanisms to handle any other usecase and empower scripts.

@josevalim
Owner

Here is another proposal to get the best of both so far:

  • migrate - run up migrations until we are the latest version
  • migrate --all - same as migrate
  • migrate --to version - run up guaranteeing we are at the given version (no-op if we already have it)
  • migrate --step steps - run steps number of up migrations
  • rollback - won't work (ask user to use --all or --step 1)
  • rollback --all - run all down migrations
  • rollback --to version - run down guaranteeing we are at the given version (no-op if we already have it)
  • rollback --step steps - run steps number of down migrations

My only question is if rollback should 1) abort or 2) should do --step 1 (different from migrate) or 3) rollback all (a very bad idea).

@ericmj
Collaborator

@josevalim's proposal is good. I think just rollback can rollback a single step (why would you write rollback if not to rollback at least one step?).

Should we support "inexact" versions? For example say i have pending migrations 1, 3 and 5. If i do migrate --to 4 should we run migrations 1 and 3 or just error?

@josevalim
Owner

Should we support "inexact" versions? For example say i have pending migrations 1, 3 and 5. If i do migrate --to 4 should we run migrations 1 and 3 or just error?

I would say that is fine. It could just work.

@christhekeele

^ This is a force push rebase of my feature branch; there was enough churn to merit it.

@christhekeele

The main remaining issue is that my integration tests are still stomping on existing migrations.

I've tried a number of things to try and resolve this, to little avail.

The naive bumping version numbers + changing migration names doesn't work, for some reason no matter what the migration module is called 5 particular create_migrations always report a conflict when loaded (namely the ones on lines 125, 139, 140, 154, and 155).

I also tried the purge reccommended by @ericmj, wherein create_migration becomes a macro that accumulates the migration's module name into an attribute, and calling purge_migrations in a teardown that tries to delete and purge any registered loaded migrations. Interestingly enough, while that worked great for most of the migrations, it failed because the same 5 mentioned above still reported being duplicated, and the erlang code server reported:

=ERROR REPORT==== 13-Dec-2013::19:22:03 ===
Module 'Elixir.Ecto.MigrationTest.GoodMigration43' must be purged before loading

I'd love help with this: I have no idea what's going on, I'd like to see this out the door, and my brain is a little fried tonight. :grin:

@ericmj
Collaborator

In some tests you are running the same migration multiple times. Every time you run a migration it will be loaded. Purging in a teardown doesn't help much because you are already naming the migrations differently. If you run the same migration multiple times you need to purge between runs.

@josevalim
Owner

Note, you can use this syntax to purge:

test "..." do
  # actual test
after
  purge [Migration1, Migration2]
end

I plan to review this pull request later today and give more insight. Thanks a lot @christhekeele! :D

@christhekeele

you need to purge between runs

Of course, I see it now.

after
  purge [Migration1, Migration2]
end

I didn't know that syntax existed! Cool.

@josevalim josevalim commented on the diff
lib/ecto/migrator.ex
((124 lines not shown))
versions = repo.adapter.migrated_versions(repo)
- Enum.filter(migrations, fn { version, _file } ->
- not (version in versions)
+ migrations_for(directory) |>
+ Enum.filter(fn { version, _file } ->
+ not (version in versions)
+ end)
+ end
+
+ defp pending_in_direction(repo, directory, :down) do
+ versions = repo.adapter.migrated_versions(repo)
+ migrations_for(directory) |>
+ Enum.filter(fn { version, _file } ->
+ version in versions
+ end)
+ |> :lists.reverse
@josevalim Owner

Enum.reverse/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@josevalim josevalim commented on the diff
lib/ecto/migrator.ex
@@ -31,69 +32,161 @@ defmodule Ecto.Migrator do
@doc """
Runs a down migration on the given repository.
"""
+ @spec down(Ecto.Repo.t, integer, Module.t) :: :ok | :missing_up | no_return
@josevalim Owner

Thanks for adding the missing typespecs. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/ecto/migrator.ex
((5 lines not shown))
def down(repo, version, module) do
commands = List.wrap(module.down)
repo.adapter.migrate_down(repo, version, commands)
end
+ @doc false
+ def run_all(repo, directory) do
@josevalim Owner

We can just get rid of this. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/ecto/migrator.ex
((16 lines not shown))
@doc """
- Runs all migrations in the given directory.
+ Apply migrations in a directory to a repository.
+
+ Available options are:
+ - `direction:` `:up` or `:down`
@josevalim Owner

Markdown requires * for listing, iirc. And we usually document options in the atom format, for example: :direction

Markdown supports either, and also +. I personally prefer - since *italics* and **bold** use * already, but consistency is more important.

@josevalim Owner

I didn't know this. Someone needs to extend IEx.DocsFormatter to handle - too then. :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/ecto/migrator.ex
((16 lines not shown))
@doc """
- Runs all migrations in the given directory.
+ Apply migrations in a directory to a repository.
+
+ Available options are:
+ - `direction:` `:up` or `:down`
+ defaults to :up
+
+ - `all:` runs all available if `true`
+ - `step:` runs a specific number of migrations
+ - `to:` runs all until supplied version is reached
+
+ If `:all`, `:step`, or `:to` are not provided, the direction
@josevalim Owner

It feels this "weird" default is a property of the mix task, it feels weird to let it creep in into the internal API. I have two suggestions:

  1. Make the direction explicit and required as argument
  2. Require one of :all, :step or :to to be given, raises ArgumentError otherwise. Each task can set the appropriate default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@josevalim
Owner

This looks fantastic, excellent code, docs and tests! I have added minor notes, thank you @christhekeele!

@christhekeele

I've moved all option munging into the task level in a series of 4 commits.

The reason why I didn't start off with the logic there is because (linguistically) we have certain expectations when talking about running migrations:

  • A 'migration' is a vector script (it can be run in two directions).
  • 'Running migrations', means running migrations in some direction.
  • 'To migrate' means running migrations upwards.
  • 'Migrating' means running all available migrations upwards.
  • 'To revert' means running migrations downwards.
  • 'Rolling back' means running one migration downwards.

This is clearly insane. The inconsistency is an artifact of what operations we perform with migrations most often.

Each of the four commits moves responsibility for deciding how to run migrations further into the tasks. The result, as can be seen by browsing updated tests, is the Migrator.run's API becomes increasingly straightforward, but requires further and further code to achieve what linguistically we consider to be the 'expected' result.

I'm quite fine with that, but wasn't sure how you felt about the trade-off. You should be able to merge any of the four commits to suite your taste. I'll push them one-by-one to my feature branch so that travis makes sure each one is green.

christhekeele added some commits
@christhekeele christhekeele Rewrite Ecto.Migrator to accept different directions and strategies.
Exposes new task `ecto.rollback` for reverting migrations, and
adds options `--all`, `--step n`, and `--to version` to both
`ecto.rollback` and `ecto.migrate`.

Deprecates calls to Migrator.run_all, which will for now warn, delegate to
Migrator.run, and behave identically to the old implementation.
cff936d
@christhekeele christhekeele Purge migrations between runs in integration tests. 6e258c4
@christhekeele christhekeele Cleans up private Migrator API.
- Direction is an explicit parameter
- Defaults are now handled by function clauses
- Allows non-strategy options to be consumed
- Deprecation warning removed
- Docs follow conventions
- 'strategies' vs 'strategy_types' no longer conflated
d257af2
@christhekeele christhekeele Require direction to be specified in Migrator.run/4. 1a81b56
@christhekeele christhekeele Move strategy selection out of Migrator and into related tasks. fd8aba7
@christhekeele christhekeele Make mix migration tasks responsible for furnishing defaults to Migra…
…tor.
10c61ce
@christhekeele

Whoops, travis went dark because elixir got updated! Bad timing. On the other hand, I got to know about the release 18 minutes before anyone else! I rebased off of master trying to figure out if the update to deps.lock would fix it before realizing Travis was privvy to the release before I was, sorry for the noise.

@josevalim
Owner

@christhekeele superb work. Thanks for splitting it in commits! This is on my plate now, I will merge it soon. :) :heart: :green_heart: :blue_heart: :yellow_heart: :purple_heart:

@josevalim josevalim merged commit 10c61ce into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 15, 2013
  1. @christhekeele

    Rewrite Ecto.Migrator to accept different directions and strategies.

    christhekeele authored
    Exposes new task `ecto.rollback` for reverting migrations, and
    adds options `--all`, `--step n`, and `--to version` to both
    `ecto.rollback` and `ecto.migrate`.
    
    Deprecates calls to Migrator.run_all, which will for now warn, delegate to
    Migrator.run, and behave identically to the old implementation.
  2. @christhekeele
  3. @christhekeele

    Cleans up private Migrator API.

    christhekeele authored
    - Direction is an explicit parameter
    - Defaults are now handled by function clauses
    - Allows non-strategy options to be consumed
    - Deprecation warning removed
    - Docs follow conventions
    - 'strategies' vs 'strategy_types' no longer conflated
  4. @christhekeele
  5. @christhekeele
  6. @christhekeele
This page is out of date. Refresh to see the latest.
View
115 integration_test/pg/migrations_test.exs
@@ -29,7 +29,7 @@ defmodule Ecto.Integration.MigrationsTest do
end
@good_migration """
- defmodule Ecto.MigrationTest.GoodMigration~B do
+ defmodule ~s do
use Ecto.Migration
def up do
@@ -38,13 +38,13 @@ defmodule Ecto.Integration.MigrationsTest do
end
def down do
- []
+ [ "DELETE FROM migrations_test WHERE id IN ( SELECT id FROM migrations_test LIMIT 1 )" ]
end
end
"""
@bad_migration """
- defmodule Ecto.MigrationTest.BadMigration~B do
+ defmodule ~s do
use Ecto.Migration
def up do
@@ -77,27 +77,111 @@ defmodule Ecto.Integration.MigrationsTest do
end
end
- test "run all migrations" do
+ test "run up all migrations" do
in_tmp fn path ->
create_migration(42, @good_migration)
create_migration(43, @good_migration)
- assert [42, 43] = run_up(TestRepo, path)
+ assert [42, 43] = run(TestRepo, path, :up, { :all, true })
create_migration(44, @good_migration)
- assert [44] = run_up(TestRepo, path)
+ assert [44] = run(TestRepo, path, :up, { :all, true })
- assert [] = run_up(TestRepo, path)
+ assert [] = run(TestRepo, path, :up, { :all, true })
assert Postgrex.Result[num_rows: 3] =
Postgres.query(TestRepo, "SELECT * FROM migrations_test")
end
end
+ test "run up to migration" do
+ in_tmp fn path ->
+ create_migration(45, @good_migration)
+ create_migration(46, @good_migration)
+ assert [45] = run(TestRepo, path, :up, { :to, 45 })
+
+ assert Postgrex.Result[num_rows: 1] =
+ Postgres.query(TestRepo, "SELECT * FROM migrations_test")
+
+ assert [46] = run(TestRepo, path, :up, { :to, 46 })
+ end
+ end
+
+ test "run up 1 migration" do
+ in_tmp fn path ->
+ create_migration(47, @good_migration)
+ create_migration(48, @good_migration)
+ assert [47] = run(TestRepo, path, :up, { :step, 1 })
+
+ assert Postgrex.Result[num_rows: 1] =
+ Postgres.query(TestRepo, "SELECT * FROM migrations_test")
+
+ assert [48] = run(TestRepo, path, :up, { :to, 48 })
+ end
+ end
+
+ test "run down 1 migration" do
+ in_tmp fn path ->
+ migrations = [
+ create_migration(49, @good_migration),
+ create_migration(50, @good_migration),
+ ]
+ assert [49, 50] = run(TestRepo, path, :up, { :all, true })
+ purge migrations
+
+ assert [50] = run(TestRepo, path, :down, { :step, 1 })
+ purge migrations
+
+ assert Postgrex.Result[num_rows: 1] =
+ Postgres.query(TestRepo, "SELECT * FROM migrations_test")
+
+ assert [50] = run(TestRepo, path, :up, { :to, 50 })
+ end
+ end
+
+ test "run down to migration" do
+ in_tmp fn path ->
+ migrations = [
+ create_migration(51, @good_migration),
+ create_migration(52, @good_migration),
+ ]
+
+ assert [51, 52] = run(TestRepo, path, :up, { :all, true })
+ purge migrations
+
+ assert [52] = run(TestRepo, path, :down, { :to, 52 })
+ purge migrations
+
+ assert Postgrex.Result[num_rows: 1] =
+ Postgres.query(TestRepo, "SELECT * FROM migrations_test")
+
+ assert [52] = run(TestRepo, path, :up, { :to, 52 })
+ end
+ end
+
+ test "run down all migrations" do
+ in_tmp fn path ->
+ migrations = [
+ create_migration(53, @good_migration),
+ create_migration(54, @good_migration),
+ ]
+ assert [53, 54] = run(TestRepo, path, :up, { :all, true })
+ purge migrations
+
+ assert [54, 53] = run(TestRepo, path, :down, { :all, true })
+ purge migrations
+
+ assert Postgrex.Result[num_rows: 0] =
+ Postgres.query(TestRepo, "SELECT * FROM migrations_test")
+
+ assert [53, 54] = run(TestRepo, path, :up, { :all, true })
+ end
+ end
+
test "bad migration raises" do
in_tmp fn path ->
- create_migration(42, @bad_migration)
+ create_migration(55, @bad_migration)
assert_raise Postgrex.Error, fn ->
- run_up(TestRepo, path)
+ run(TestRepo, path, :up, { :all, true })
end
end
end
@@ -107,6 +191,17 @@ defmodule Ecto.Integration.MigrationsTest do
end
defp create_migration(num, contents) do
- File.write!("#{num}_migration.exs", :io_lib.format(contents, [num]))
+ migration = Module.concat(__MODULE__, "Migration#{num}")
+ File.write!("#{num}_migration.exs", :io_lib.format(contents, [migration]))
+ migration
+ end
+
+ defp purge(modules) do
+ modules
+ |> List.wrap
+ |> Enum.each( fn m ->
+ :code.delete m
+ :code.purge m
+ end )
end
end
View
117 lib/ecto/migrator.ex
@@ -20,9 +20,12 @@ defmodule Ecto.Migrator do
"""
+ @type strategy :: { atom, any }
+
@doc """
Runs an up migration on the given repository.
"""
+ @spec up(Ecto.Repo.t, integer, Module.t) :: :ok | :already_up | no_return
def up(repo, version, module) do
commands = List.wrap(module.up)
repo.adapter.migrate_up(repo, version, commands)
@@ -31,69 +34,115 @@ defmodule Ecto.Migrator do
@doc """
Runs a down migration on the given repository.
"""
+ @spec down(Ecto.Repo.t, integer, Module.t) :: :ok | :missing_up | no_return
@josevalim Owner

Thanks for adding the missing typespecs. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
def down(repo, version, module) do
commands = List.wrap(module.down)
repo.adapter.migrate_down(repo, version, commands)
end
+ @spec strategy_types :: list(atom)
+ def strategy_types, do: [:to, :step, :all]
+
@doc """
- Runs all migrations in the given directory.
+ Apply migrations in a directory to a repository with given strategy.
+
+ Strategies are two-tuples of a type and a parameter.
+
+ Available strategy types are:
+ * `:all` runs all available if `true`
+ * `:step` runs the specific number of migrations
+ * `:to` runs all until the supplied version is reached
"""
- @spec run_up(Ecto.Repo.t, binary) :: [integer] | no_return
- def run_up(repo, directory) do
- migrations = Path.join(directory, "*")
- |> Path.wildcard
- |> Enum.filter(&Regex.match?(%r"\d+_.+\.exs$", &1))
- |> attach_versions
- ensure_no_duplication(migrations)
+ # To extend Migrator.run with different strategies,
+ # define a `run` clause that matches on it and insert
+ # the strategy type into the `strategies` function above.
+
+ @spec run(Ecto.Repo.t, binary, atom, strategy) :: [integer]
+ def run(repo, directory, direction, strategy)
- migrations
- |> filter_migrated(repo)
- |> execute_migrations(repo)
+ def run(repo, directory, direction, {:to, target_version}) do
+ within_target_version? = fn
+ { version, _ }, target, :up ->
+ version <= target
+ { version, _ }, target, :down ->
+ version >= target
+ end
+ pending_in_direction(repo, directory, direction)
+ |> Enum.take_while(&(within_target_version?.(&1, target_version, direction)))
+ |> migrate(direction, repo)
end
- defp attach_versions(files) do
- Enum.map(files, fn(file) ->
- { integer, _ } = Integer.parse(Path.basename(file))
- { integer, file }
- end)
+ def run(repo, directory, direction, {:step, count}) do
+ pending_in_direction(repo, directory, direction)
+ |> Enum.take(count)
+ |> migrate(direction, repo)
end
- defp ensure_no_duplication([{ version, _ } | t]) do
- if List.keyfind(t, version, 0) do
- raise Ecto.MigrationError, message: "migrations can't be executed, version #{version} is duplicated"
- else
- ensure_no_duplication(t)
- end
+ def run(repo, directory, direction, {:all, true}) do
+ pending_in_direction(repo, directory, direction)
+ |> migrate(direction, repo)
end
- defp ensure_no_duplication([]), do: :ok
+ defp pending_in_direction(repo, directory, :up) do
+ versions = repo.adapter.migrated_versions(repo)
+ migrations_for(directory) |>
+ Enum.filter(fn { version, _file } ->
+ not (version in versions)
+ end)
+ end
- defp filter_migrated(migrations, repo) do
+ defp pending_in_direction(repo, directory, :down) do
versions = repo.adapter.migrated_versions(repo)
- Enum.filter(migrations, fn { version, _file } ->
- not (version in versions)
+ migrations_for(directory) |>
+ Enum.filter(fn { version, _file } ->
+ version in versions
+ end)
+ |> :lists.reverse
@josevalim Owner

Enum.reverse/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
+ defp migrations_for(directory) do
+ Path.join(directory, "*")
+ |> Path.wildcard
+ |> Enum.filter(&Regex.match?(%r"\d+_.+\.exs$", &1))
+ |> attach_versions
+ end
+
+ defp attach_versions(files) do
+ Enum.map(files, fn(file) ->
+ { integer, _ } = Integer.parse(Path.basename(file))
+ { integer, file }
end)
end
- defp execute_migrations(migrations, repo) do
+ defp migrate(migrations, direction, repo) do
+ ensure_no_duplication(migrations)
+ migrator = case direction do
+ :up -> &up/3
+ :down -> &down/3
+ end
+
Enum.map migrations, fn { version, file } ->
{ mod, _bin } =
Enum.find(Code.load_file(file), fn { mod, _bin } ->
function_exported?(mod, :__migration__, 0)
end) || raise_no_migration_in_file(file)
- commands = List.wrap(mod.up)
- case repo.adapter.migrate_up(repo, version, commands) do
- :already_up ->
- version
- :ok ->
- version
- end
+ migrator.(repo, version, mod)
+ version
end
end
+ defp ensure_no_duplication([{ version, _ } | t]) do
+ if List.keyfind(t, version, 0) do
+ raise Ecto.MigrationError, message: "migrations can't be executed, version #{version} is duplicated"
+ else
+ ensure_no_duplication(t)
+ end
+ end
+
+ defp ensure_no_duplication([]), do: :ok
+
defp raise_no_migration_in_file(file) do
raise Ecto.MigrationError, message: "file #{Path.relative_to_cwd(file)} does not contain any Ecto.Migration"
end
View
23 lib/mix/tasks/ecto.ex
@@ -66,6 +66,7 @@ defmodule Mix.Tasks.Ecto do
@doc """
Asks if the user wants to open a file based on ECTO_EDITOR.
"""
+ @spec open?(binary) :: boolean
def open?(file) do
editor = System.get_env("ECTO_EDITOR") || ""
if editor != "" do
@@ -75,4 +76,26 @@ defmodule Mix.Tasks.Ecto do
false
end
end
+
+ @spec parse_strategy(Keyword.t) :: { Migrator.strategy, Keyword.t }
+ def parse_strategy(opts) do
+ { strategies, opts } = Enum.partition(opts, &(valid_strategy?(&1)))
+ { select_strategy(strategies), opts }
+ end
+
+ defp valid_strategy?({ type, _ }), do: type in Ecto.Migrator.strategy_types
+
+ defp select_strategy([]), do: nil
+ defp select_strategy([strategy]), do: strategy
+ defp select_strategy(strategies) do
+ strategies
+ |> Enum.sort(&(strategy_precedence(&1) > strategy_precedence(&2)))
+ |> Enum.first
+ end
+
+ defp strategy_precedence({ type, _ }) do
+ Ecto.Migrator.strategy_types
+ |> Enum.reverse
+ |> Enum.find_index(&(&1 == type))
+ end
end
View
21 lib/mix/tasks/ecto.migrate.ex
@@ -2,23 +2,38 @@ defmodule Mix.Tasks.Ecto.Migrate do
use Mix.Task
import Mix.Tasks.Ecto
- @shortdoc "Runs the given repo migrations"
+ @shortdoc "Runs migrations up on a repo"
@moduledoc """
Runs the pending migrations for the given repository.
+
Migrations are expected to be found inside the migrations
directory returned by the priv function defined in the
repository.
+ Runs all pending migrations by default. To migrate up
+ to a version number, supply `--to version_number`.
+ To migrate up a specific number of times, use `--step n`.
+
## Examples
mix ecto.migrate MyApp.Repo
+ mix ecto.migrate MyApp.Repo -n 3
+ mix ecto.migrate MyApp.Repo --step 3
+
+ mix ecto.migrate MyApp.Repo -v 20080906120000
+ mix ecto.migrate MyApp.Repo --to 20080906120000
+
"""
- def run(args, migrator // &Ecto.Migrator.run_up/2) do
+ def run(args, migrator // &Ecto.Migrator.run/4) do
+ { opts, args, _ } = OptionParser.parse args,
+ switches: [all: :boolean, step: :integer, version: :integer],
+ aliases: [n: :step, v: :to]
{ repo, _ } = parse_repo(args)
ensure_repo(repo)
ensure_started(repo)
- migrator.(repo, migrations_path(repo))
+ { strategy, _ } = parse_strategy(opts) # We don't use other opts atm
+ migrator.(repo, migrations_path(repo), :up, strategy || { :all, true })
end
end
View
40 lib/mix/tasks/ecto.rollback.ex
@@ -0,0 +1,40 @@
+defmodule Mix.Tasks.Ecto.Rollback do
+ use Mix.Task
+ import Mix.Tasks.Ecto
+
+ @shortdoc "Reverts migrations down on a repo"
+
+ @moduledoc """
+ Reverts applied migrations in the given repository.
+
+ Migrations are expected to be found inside the migrations
+ directory returned by the priv function defined in the
+ repository.
+
+ Runs the latest applied migration by default. To roll back to
+ to a version number, supply `--to version_number`.
+ To roll back a specific number of times, use `--step n`.
+ To undo all applied migrations, provide `--all`.
+
+ ## Examples
+
+ mix ecto.rollback MyApp.Repo
+
+ mix ecto.rollback MyApp.Repo -n 3
+ mix ecto.rollback MyApp.Repo --step 3
+
+ mix ecto.rollback MyApp.Repo -v 20080906120000
+ mix ecto.rollback MyApp.Repo --to 20080906120000
+
+ """
+ def run(args, migrator // &Ecto.Migrator.run/4) do
+ { opts, args, _ } = OptionParser.parse args,
+ switches: [all: :boolean, step: :integer, version: :integer],
+ aliases: [n: :step, v: :to]
+ { repo, _ } = parse_repo(args)
+ ensure_repo(repo)
+ ensure_started(repo)
+ { strategy, _ } = parse_strategy(opts) # We don't use other opts atm
+ migrator.(repo, migrations_path(repo), :down, strategy || { :step, 1 })
+ end
+end
View
93 test/ecto/migrator_test.exs
@@ -3,6 +3,8 @@ defmodule Ecto.MigratorTest do
import Support.FileHelpers
+ import Ecto.Migrator
+
defmodule ProcessRepo do
@behaviour Ecto.Adapter.Migrations
@@ -41,33 +43,19 @@ defmodule Ecto.MigratorTest do
end
test "up invokes the repository adapter with up commands" do
- assert Ecto.Migrator.up(ProcessRepo, 0, Migration) == :ok
- assert Ecto.Migrator.up(ProcessRepo, 1, Migration) == :already_up
+ assert up(ProcessRepo, 0, Migration) == :ok
+ assert up(ProcessRepo, 1, Migration) == :already_up
end
test "down invokes the repository adapter with down commands" do
- assert Ecto.Migrator.down(ProcessRepo, 0, Migration) == :already_down
- assert Ecto.Migrator.down(ProcessRepo, 1, Migration) == :ok
- end
-
- test "run_up runs all migrations inside a directory" do
- in_tmp fn path ->
- create_migration "13_sample.exs"
- assert Ecto.Migrator.run_up(ProcessRepo, path) == [13]
- end
- end
-
- test "run_up skip migrations that are already up" do
- in_tmp fn path ->
- create_migration "1_sample.exs"
- assert Ecto.Migrator.run_up(ProcessRepo, path) == []
- end
+ assert down(ProcessRepo, 0, Migration) == :already_down
+ assert down(ProcessRepo, 1, Migration) == :ok
end
test "expects files starting with an integer" do
in_tmp fn path ->
create_migration "a_sample.exs"
- assert Ecto.Migrator.run_up(ProcessRepo, path) == []
+ assert run(ProcessRepo, path, :up, { :all, true }) == []
end
end
@@ -75,7 +63,7 @@ defmodule Ecto.MigratorTest do
in_tmp fn path ->
File.write! "13_sample.exs", ":ok"
assert_raise Ecto.MigrationError, "file 13_sample.exs does not contain any Ecto.Migration", fn ->
- Ecto.Migrator.run_up(ProcessRepo, path)
+ run(ProcessRepo, path, :up, { :all, true })
end
end
end
@@ -85,11 +73,74 @@ defmodule Ecto.MigratorTest do
create_migration "13_hello.exs"
create_migration "13_other.exs"
assert_raise Ecto.MigrationError, "migrations can't be executed, version 13 is duplicated", fn ->
- Ecto.Migrator.run_up(ProcessRepo, path)
+ run(ProcessRepo, path, :up, { :all, true })
end
end
end
+ test "upwards migrations skips migrations that are already up" do
+ in_tmp fn path ->
+ create_migration "1_sample.exs"
+ assert run(ProcessRepo, path, :up, { :all, true }) == []
+ end
+ end
+
+ test "downwards migrations skips migrations that are already down" do
+ in_tmp fn path ->
+ create_migration "1_sample.exs"
+ create_migration "4_sample.exs"
+ assert run(ProcessRepo, path, :down, { :all, true }) == [1]
+ end
+ end
+
+ test "stepwise migrations stop before all have been run" do
+ in_tmp fn path ->
+ create_migration "13_step_premature_end.exs"
+ create_migration "14_step_premature_end.exs"
+ assert run(ProcessRepo, path, :up, { :step, 1 }) == [13]
+ end
+ end
+
+ test "stepwise migrations stop at the number of available migrations" do
+ in_tmp fn path ->
+ create_migration "13_step_to_the_end.exs"
+ create_migration "14_step_to_the_end.exs"
+ assert run(ProcessRepo, path, :up, { :step, 2 }) == [13, 14]
+ end
+ end
+
+ test "stepwise migrations stop even if asked to exceed available" do
+ in_tmp fn path ->
+ create_migration "13_step_past_the_end.exs"
+ create_migration "14_step_past_the_end.exs"
+ assert run(ProcessRepo, path, :up, { :step, 3 }) == [13, 14]
+ end
+ end
+
+ test "version migrations stop before all have been run" do
+ in_tmp fn path ->
+ create_migration "13_version_premature_end.exs"
+ create_migration "14_version_premature_end.exs"
+ assert run(ProcessRepo, path, :up, { :to, 13 }) == [13]
+ end
+ end
+
+ test "version migrations stop at the number of available migrations" do
+ in_tmp fn path ->
+ create_migration "13_version_to_the_end.exs"
+ create_migration "14_version_to_the_end.exs"
+ assert run(ProcessRepo, path, :up, { :to, 14 }) == [13, 14]
+ end
+ end
+
+ test "version migrations stop even if asked to exceed available" do
+ in_tmp fn path ->
+ create_migration "13_version_past_the_end.exs"
+ create_migration "14_version_past_the_end.exs"
+ assert run(ProcessRepo, path, :up, { :to, 15 }) == [13, 14]
+ end
+ end
+
defp create_migration(name) do
File.write! name, """
defmodule Ecto.MigrationTest.S#{Path.rootname(name)} do
View
6 test/mix/tasks/ecto.migrate_test.exs
@@ -19,7 +19,7 @@ defmodule Mix.Tasks.Ecto.MigrateTest do
end
test "runs the migrator" do
- run [to_string(Repo)], fn _, _ ->
+ run [to_string(Repo)], fn _, _, _, _ ->
Process.put(:migrated, true)
end
assert Process.get(:migrated)
@@ -27,9 +27,11 @@ defmodule Mix.Tasks.Ecto.MigrateTest do
end
test "runs the migrator yielding the repository and migrations path" do
- run [to_string(Repo)], fn repo, path ->
+ run [to_string(Repo)], fn repo, path, direction, strategy ->
assert repo == Repo
assert path == "hello/migrations"
+ assert direction == :up
+ assert strategy == { :all, true }
end
end
end
View
37 test/mix/tasks/ecto.rollback_test.exs
@@ -0,0 +1,37 @@
+defmodule Mix.Tasks.Ecto.RollbackTest do
+ use ExUnit.Case, async: true
+
+ import Mix.Tasks.Ecto.Rollback, only: [run: 2]
+
+ defmodule Repo do
+ def start_link do
+ Process.put(:started, true)
+ :ok
+ end
+
+ def priv do
+ "hello"
+ end
+
+ def __repo__ do
+ true
+ end
+ end
+
+ test "runs the migrator" do
+ run [to_string(Repo)], fn _, _, _, _ ->
+ Process.put(:migrated, true)
+ end
+ assert Process.get(:migrated)
+ assert Process.get(:started)
+ end
+
+ test "runs the migrator yielding the repository and migrations path" do
+ run [to_string(Repo)], fn repo, path, direction, strategy ->
+ assert repo == Repo
+ assert path == "hello/migrations"
+ assert direction == :down
+ assert strategy == { :step, 1 }
+ end
+ end
+end
View
12 test/mix/tasks/ecto_test.exs
@@ -50,4 +50,16 @@ defmodule Mix.Tasks.EctoTest do
assert migrations_path(Repo) == "hello/migrations"
assert_raise Mix.Error, fn -> migrations_path(String) end
end
+
+ test :parse_strategy do
+ assert parse_strategy([]) == { nil, [] }
+ assert parse_strategy([to: 1]) == { {:to, 1}, [] }
+ assert parse_strategy([step: 1]) == { {:step, 1}, [] }
+ assert parse_strategy([all: true]) == { {:all, true}, [] }
+ assert parse_strategy([foo: :bar]) == { nil, [foo: :bar] }
+ assert parse_strategy([step: 1, foo: :bar]) == { {:step, 1}, [foo: :bar] }
+ assert parse_strategy([all: true, step: 1]) == { {:step, 1}, [] }
+ assert parse_strategy([all: true, to: 1]) == { {:to, 1}, [] }
+ assert parse_strategy([step: 1, to: 1]) == { {:to, 1}, [] }
+ end
end
Something went wrong with that request. Please try again.