diff --git a/lib/ecto/adapter/migration.ex b/lib/ecto/adapter/migration.ex index e7870c58..f1c0a712 100644 --- a/lib/ecto/adapter/migration.ex +++ b/lib/ecto/adapter/migration.ex @@ -32,9 +32,12 @@ defmodule Ecto.Adapter.Migration do @typedoc "All commands allowed within the block passed to `table/2`" @type table_subcommand :: {:add, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), Keyword.t()} - | {:add_if_not_exists, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), Keyword.t()} - | {:modify, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), Keyword.t()} - | {:remove, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), Keyword.t()} + | {:add_if_not_exists, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), + Keyword.t()} + | {:modify, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), + Keyword.t()} + | {:remove, field :: atom, type :: Ecto.Type.t() | Reference.t() | binary(), + Keyword.t()} | {:remove, field :: atom} | {:remove_if_exists, type :: Ecto.Type.t() | Reference.t() | binary()} @@ -55,7 +58,7 @@ defmodule Ecto.Adapter.Migration do Executes migration commands. """ @callback execute_ddl(adapter_meta, command, options :: Keyword.t()) :: - {:ok, [{Logger.level, Logger.message, Logger.metadata}]} + {:ok, [{Logger.level(), Logger.message(), Logger.metadata()}]} @doc """ Locks the migrations table and emits the locked versions for callback execution. diff --git a/lib/ecto/adapter/structure.ex b/lib/ecto/adapter/structure.ex index 4c172f5c..1cbf6cc0 100644 --- a/lib/ecto/adapter/structure.ex +++ b/lib/ecto/adapter/structure.ex @@ -18,8 +18,8 @@ defmodule Ecto.Adapter.Structure do hostname: "localhost") """ - @callback structure_dump(default :: String.t, config :: Keyword.t) :: - {:ok, String.t} | {:error, term} + @callback structure_dump(default :: String.t(), config :: Keyword.t()) :: + {:ok, String.t()} | {:error, term} @doc """ Loads the given structure. @@ -36,8 +36,8 @@ defmodule Ecto.Adapter.Structure do hostname: "localhost") """ - @callback structure_load(default :: String.t, config :: Keyword.t) :: - {:ok, String.t} | {:error, term} + @callback structure_load(default :: String.t(), config :: Keyword.t()) :: + {:ok, String.t()} | {:error, term} @doc """ Runs the dump command for the given repo / config. diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex index 904546e6..25092ba0 100644 --- a/lib/ecto/migration/runner.ex +++ b/lib/ecto/migration/runner.ex @@ -16,12 +16,12 @@ defmodule Ecto.Migration.Runner do level = Keyword.get(opts, :log, :info) sql = Keyword.get(opts, :log_migrations_sql, false) log = %{level: level, sql: sql} - args = {self(), repo, config, module, direction, migrator_direction, log} + args = {self(), repo, config, module, direction, migrator_direction, log} {:ok, runner} = DynamicSupervisor.start_child(Ecto.MigratorSupervisor, {__MODULE__, args}) metadata(runner, opts) - log(level, "== Running #{version} #{inspect module}.#{operation}/0 #{direction}") + log(level, "== Running #{version} #{inspect(module)}.#{operation}/0 #{direction}") {time, _} = :timer.tc(fn -> perform_operation(repo, module, operation) end) log(level, "== Migrated #{version} in #{inspect(div(time, 100_000) / 10)}s") @@ -97,7 +97,7 @@ defmodule Ecto.Migration.Runner do def prefix do case Process.get(:ecto_migration) do %{prefix: prefix} -> prefix - _ -> raise "could not find migration runner process for #{inspect self()}" + _ -> raise "could not find migration runner process for #{inspect(self())}" end end @@ -128,7 +128,8 @@ defmodule Ecto.Migration.Runner do reply = Agent.get_and_update(runner(), fn %{command: nil} = state -> - {:ok, %{state | subcommands: [], commands: [command|state.commands]}} + {:ok, %{state | subcommands: [], commands: [command | state.commands]}} + %{command: _} = state -> {:error, %{state | command: nil}} end) @@ -136,6 +137,7 @@ defmodule Ecto.Migration.Runner do case reply do :ok -> :ok + :error -> raise Ecto.MigrationError, "cannot execute nested commands" end @@ -149,6 +151,7 @@ defmodule Ecto.Migration.Runner do Agent.get_and_update(runner(), fn %{command: nil} = state -> {:ok, %{state | command: command}} + %{command: _} = state -> {:error, %{state | command: command}} end) @@ -156,6 +159,7 @@ defmodule Ecto.Migration.Runner do case reply do :ok -> :ok + :error -> raise Ecto.MigrationError, "cannot execute nested commands" end @@ -165,11 +169,11 @@ defmodule Ecto.Migration.Runner do Queues and clears current command. Must call `start_command/1` first. """ def end_command do - Agent.update runner(), fn state -> + Agent.update(runner(), fn state -> {operation, object} = state.command command = {operation, object, Enum.reverse(state.subcommands)} - %{state | command: nil, subcommands: [], commands: [command|state.commands]} - end + %{state | command: nil, subcommands: [], commands: [command | state.commands]} + end) end @doc """ @@ -180,13 +184,15 @@ defmodule Ecto.Migration.Runner do Agent.get_and_update(runner(), fn %{command: nil} = state -> {:error, state} + state -> - {:ok, update_in(state.subcommands, &[subcommand|&1])} + {:ok, update_in(state.subcommands, &[subcommand | &1])} end) case reply do :ok -> :ok + :error -> raise Ecto.MigrationError, message: "cannot execute command outside of block" end @@ -210,31 +216,41 @@ defmodule Ecto.Migration.Runner do if reversed = reverse(command) do log_and_execute_ddl(repo, migration, log, reversed) else - raise Ecto.MigrationError, message: - "cannot reverse migration command: #{command command}. " <> - "You will need to explicitly define up/0 and down/0 in your migration" + raise Ecto.MigrationError, + message: + "cannot reverse migration command: #{command(command)}. " <> + "You will need to explicitly define up/0 and down/0 in your migration" end end defp reverse({:create, %Index{} = index}), do: {:drop, index, :restrict} + defp reverse({:create_if_not_exists, %Index{} = index}), do: {:drop_if_exists, index, :restrict} + defp reverse({:drop, %Index{} = index, _}), do: {:create, index} + defp reverse({:drop_if_exists, %Index{} = index, _}), do: {:create_if_not_exists, index} + defp reverse({:rename, %Index{} = index, new_name}), do: {:rename, %{index | name: new_name}, index.name} + defp reverse({:create, %Table{} = table, _columns}), do: {:drop, table, :restrict} + defp reverse({:create_if_not_exists, %Table{} = table, _columns}), do: {:drop_if_exists, table, :restrict} + defp reverse({:rename, %Table{} = table_current, %Table{} = table_new}), do: {:rename, table_new, table_current} + defp reverse({:rename, %Table{} = table, current_column, new_column}), do: {:rename, table, new_column, current_column} - defp reverse({:alter, %Table{} = table, changes}) do + + defp reverse({:alter, %Table{} = table, changes}) do if reversed = table_reverse(changes, []) do {:alter, table, reversed} end @@ -244,14 +260,16 @@ defmodule Ecto.Migration.Runner do # we can't guarantee data integrity when applying them back. defp reverse({:create_if_not_exists, %Constraint{} = constraint}), do: {:drop_if_exists, constraint, :restrict} + defp reverse({:create, %Constraint{} = constraint}), do: {:drop, constraint, :restrict} defp reverse(_command), do: false - defp table_reverse([{:remove, name, type, opts}| t], acc) do + defp table_reverse([{:remove, name, type, opts} | t], acc) do table_reverse(t, [{:add, name, type, opts} | acc]) end + defp table_reverse([{:modify, name, type, opts} | t], acc) do case opts[:from] do nil -> @@ -267,12 +285,15 @@ defmodule Ecto.Migration.Runner do table_reverse(t, [{:modify, name, reverse_type, reverse_opts} | acc]) end end + defp table_reverse([{:add, name, type, _opts} | t], acc) do table_reverse(t, [{:remove, name, type, []} | acc]) end + defp table_reverse([_ | _], _acc) do false end + defp table_reverse([], acc) do acc end @@ -302,7 +323,7 @@ defmodule Ecto.Migration.Runner do defp runner do case Process.get(:ecto_migration) do %{runner: runner} -> runner - _ -> raise "could not find migration runner process for #{inspect self()}" + _ -> raise "could not find migration runner process for #{inspect(self())}" end end @@ -339,22 +360,26 @@ defmodule Ecto.Migration.Runner do defp log(level, msg, metadata \\ []) defp log(false, _msg, _metadata), do: :ok defp log(true, msg, metadata), do: Logger.log(:info, msg, metadata) - defp log(level, msg, metadata), do: Logger.log(level, msg, metadata) + defp log(level, msg, metadata), do: Logger.log(level, msg, metadata) defp maybe_warn_index_ddl_transaction(%{concurrently: true} = index, migration) do migration_config = migration.__migration__() if not migration_config[:disable_ddl_transaction] do - IO.warn """ - Migration #{inspect(migration)} has set index `#{index.name}` on table \ - `#{index.table}` to concurrently but did not disable ddl transaction. \ - Please set: - - use Ecto.Migration - @disable_ddl_transaction true - """, [] + IO.warn( + """ + Migration #{inspect(migration)} has set index `#{index.name}` on table \ + `#{index.table}` to concurrently but did not disable ddl transaction. \ + Please set: + + use Ecto.Migration + @disable_ddl_transaction true + """, + [] + ) end end + defp maybe_warn_index_ddl_transaction(_index, _migration), do: :ok defp maybe_warn_index_migration_lock(%{concurrently: true} = index, repo, migration) do @@ -367,77 +392,106 @@ defmodule Ecto.Migration.Runner do :ok {false, Ecto.Adapters.Postgres, _} -> - IO.warn """ - Migration #{inspect(migration)} has set index `#{index.name}` on table \ - `#{index.table}` to concurrently but did not disable migration lock. \ - Please set: + IO.warn( + """ + Migration #{inspect(migration)} has set index `#{index.name}` on table \ + `#{index.table}` to concurrently but did not disable migration lock. \ + Please set: - use Ecto.Migration - @disable_migration_lock true + use Ecto.Migration + @disable_migration_lock true - Alternatively, consider using advisory locks during migrations in the \ - repo configuration: + Alternatively, consider using advisory locks during migrations in the \ + repo configuration: - config #{inspect(repo)}, migration_lock: :pg_advisory_lock - """, [] + config #{inspect(repo)}, migration_lock: :pg_advisory_lock + """, + [] + ) {false, _adapter, _migration_lock} -> - IO.warn """ - Migration #{inspect(migration)} has set index `#{index.name}` on table \ - `#{index.table}` to concurrently but did not disable migration lock. \ - Please set: - - use Ecto.Migration - @disable_migration_lock true - """, [] + IO.warn( + """ + Migration #{inspect(migration)} has set index `#{index.name}` on table \ + `#{index.table}` to concurrently but did not disable migration lock. \ + Please set: + + use Ecto.Migration + @disable_migration_lock true + """, + [] + ) _ -> :ok end end + defp maybe_warn_index_migration_lock(_index, _repo, _migration), do: :ok defp command(ddl) when is_binary(ddl) or is_list(ddl), - do: "execute #{inspect ddl}" + do: "execute #{inspect(ddl)}" defp command({:create, %Table{} = table, _}), do: "create table #{quote_name(table.prefix, table.name)}" + defp command({:create_if_not_exists, %Table{} = table, _}), do: "create table if not exists #{quote_name(table.prefix, table.name)}" + defp command({:alter, %Table{} = table, _}), do: "alter table #{quote_name(table.prefix, table.name)}" + defp command({:drop, %Table{} = table, mode}), do: "drop table #{quote_name(table.prefix, table.name)}#{drop_mode(mode)}" + defp command({:drop_if_exists, %Table{} = table, mode}), do: "drop table if exists #{quote_name(table.prefix, table.name)}#{drop_mode(mode)}" defp command({:create, %Index{} = index}), do: "create index #{quote_name(index.prefix, index.name)}" + defp command({:create_if_not_exists, %Index{} = index}), do: "create index if not exists #{quote_name(index.prefix, index.name)}" + defp command({:drop, %Index{} = index, mode}), do: "drop index #{quote_name(index.prefix, index.name)}#{drop_mode(mode)}" + defp command({:drop_if_exists, %Index{} = index, mode}), do: "drop index if exists #{quote_name(index.prefix, index.name)}#{drop_mode(mode)}" + defp command({:rename, %Index{} = index_current, new_name}), do: "rename index #{quote_name(index_current.name)} to #{new_name}" + defp command({:rename, %Table{} = current_table, %Table{} = new_table}), - do: "rename table #{quote_name(current_table.prefix, current_table.name)} to #{quote_name(new_table.prefix, new_table.name)}" + do: + "rename table #{quote_name(current_table.prefix, current_table.name)} to #{quote_name(new_table.prefix, new_table.name)}" + defp command({:rename, %Table{} = table, current_column, new_column}), - do: "rename column #{current_column} to #{new_column} on table #{quote_name(table.prefix, table.name)}" + do: + "rename column #{current_column} to #{new_column} on table #{quote_name(table.prefix, table.name)}" defp command({:create, %Constraint{check: nil, exclude: nil}}), do: raise(ArgumentError, "a constraint must have either a check or exclude option") - defp command({:create, %Constraint{check: check, exclude: exclude}}) when is_binary(check) and is_binary(exclude), - do: raise(ArgumentError, "a constraint must not have both check and exclude options") + + defp command({:create, %Constraint{check: check, exclude: exclude}}) + when is_binary(check) and is_binary(exclude), + do: raise(ArgumentError, "a constraint must not have both check and exclude options") + defp command({:create, %Constraint{check: check} = constraint}) when is_binary(check), - do: "create check constraint #{constraint.name} on table #{quote_name(constraint.prefix, constraint.table)}" + do: + "create check constraint #{constraint.name} on table #{quote_name(constraint.prefix, constraint.table)}" + defp command({:create, %Constraint{exclude: exclude} = constraint}) when is_binary(exclude), - do: "create exclude constraint #{constraint.name} on table #{quote_name(constraint.prefix, constraint.table)}" + do: + "create exclude constraint #{constraint.name} on table #{quote_name(constraint.prefix, constraint.table)}" + defp command({:drop, %Constraint{} = constraint, _}), - do: "drop constraint #{constraint.name} from table #{quote_name(constraint.prefix, constraint.table)}" + do: + "drop constraint #{constraint.name} from table #{quote_name(constraint.prefix, constraint.table)}" + defp command({:drop_if_exists, %Constraint{} = constraint, _}), - do: "drop constraint if exists #{constraint.name} from table #{quote_name(constraint.prefix, constraint.table)}" + do: + "drop constraint if exists #{constraint.name} from table #{quote_name(constraint.prefix, constraint.table)}" defp drop_mode(:restrict), do: "" defp drop_mode(:cascade), do: " cascade" diff --git a/lib/ecto/migration/schema_migration.ex b/lib/ecto/migration/schema_migration.ex index ab9a85e2..1ebc3d4a 100644 --- a/lib/ecto/migration/schema_migration.ex +++ b/lib/ecto/migration/schema_migration.ex @@ -72,7 +72,8 @@ defmodule Ecto.Migration.SchemaMigration do defp default_opts(opts) do Keyword.merge( @default_opts, - [prefix: opts[:prefix], log: Keyword.get(opts, :log_migrator_sql, false)] + prefix: opts[:prefix], + log: Keyword.get(opts, :log_migrator_sql, false) ) end end