GeezerVison is showing github.com — Add before_transaction/1 and after_transaction/1 migration callbacks … · elixir-ecto/ecto_sql@bf126d6
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/ecto/migration.ex +55-1Lines changed: 55 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -405,6 +405,37 @@ defmodule Ecto.Migration do | 405 | 405 | Then in your migrations you can `use MyApp.Migration` to share this behavior | 406 | 406 | among all your migrations. | 407 | 407 |
| | 408 | + ### Before and after the transaction | | 409 | +
| | 410 | + Some commands only work outside of a transaction. For example, SQLite | | 411 | + ignores `PRAGMA foreign_keys` inside one, but its | | 412 | + [procedure for changing a table](https://sqlite.org/lang_altertable.html#making_other_kinds_of_table_schema_changes) | | 413 | + requires it to be off. `c:before_transaction/1` and `c:after_transaction/1` | | 414 | + run around the migration transaction, on the same connection, so a setting | | 415 | + made in the first applies to the transaction and the second can restore it: | | 416 | +
| | 417 | + defmodule MyApp.Migration do | | 418 | + defmacro __using__(_) do | | 419 | + quote do | | 420 | + use Ecto.Migration | | 421 | +
| | 422 | + def before_transaction(repo) do | | 423 | + repo.query!("PRAGMA foreign_keys = OFF") | | 424 | + end | | 425 | +
| | 426 | + def after_transaction(repo) do | | 427 | + # Restore the configured value, do not turn it on unconditionally | | 428 | + if Keyword.get(repo.config(), :foreign_keys, :on) == :on do | | 429 | + repo.query!("PRAGMA foreign_keys = ON") | | 430 | + end | | 431 | + end | | 432 | + end | | 433 | + end | | 434 | + end | | 435 | +
| | 436 | + Like `c:after_begin/0` and `c:before_commit/0`, they are not run when the | | 437 | + migration does not run in a transaction. | | 438 | +
| 408 | 439 | ## Additional resources | 409 | 440 |
| 410 | 441 | * The [Safe Ecto Migrations guide](safe_migrations.md) | @@ -426,7 +457,30 @@ defmodule Ecto.Migration do | 426 | 457 | consider both the up *and* down cases of the migration. | 427 | 458 | """ | 428 | 459 | @callback before_commit() :: term | 429 | | - @optional_callbacks after_begin: 0, before_commit: 0 | | 460 | + | | 461 | + @doc """ | | 462 | + Code to run before the migration transaction is opened. | | 463 | +
| | 464 | + It runs on the same connection as the transaction and receives the repo. | | 465 | + `repo/0`, `prefix/0`, `direction/0`, `execute/1` and `flush/0` are not | | 466 | + available. | | 467 | + """ | | 468 | + @callback before_transaction(repo :: Ecto.Repo.t()) :: term | | 469 | + | | 470 | + @doc """ | | 471 | + Code to run after the migration transaction is closed. | | 472 | +
| | 473 | + It runs on the same connection as the transaction, whether the transaction | | 474 | + was committed or rolled back and even if `c:before_transaction/1` raised, | | 475 | + so that what it changed can always be restored. `repo/0`, `prefix/0`, | | 476 | + `direction/0`, `execute/1` and `flush/0` are not available. | | 477 | + """ | | 478 | + @callback after_transaction(repo :: Ecto.Repo.t()) :: term | | 479 | + | | 480 | + @optional_callbacks after_begin: 0, | | 481 | + before_commit: 0, | | 482 | + before_transaction: 1, | | 483 | + after_transaction: 1 | 430 | 484 |
| 431 | 485 | defmodule Index do | 432 | 486 | @moduledoc """ | |
Collapse file Copy file name to clipboardExpand all lines: lib/ecto/migrator.ex +28-3Lines changed: 28 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -353,15 +353,40 @@ defmodule Ecto.Migrator do | 353 | 353 | not repo.__adapter__().supports_ddl_transaction?() do | 354 | 354 | fun.() | 355 | 355 | else | 356 | | - {:ok, result} = repo.transaction(fun, log: migrator_log(opts), timeout: :infinity) | 357 | | - | 358 | | - result | | 356 | + run_in_transaction(repo, module, fun, opts) | 359 | 357 | end | 360 | 358 | catch | 361 | 359 | kind, reason -> | 362 | 360 | {kind, reason, __STACKTRACE__} | 363 | 361 | end | 364 | 362 |
| | 363 | + defp run_in_transaction(repo, module, fun, opts) do | | 364 | + transaction = fn -> | | 365 | + {:ok, result} = repo.transaction(fun, log: migrator_log(opts), timeout: :infinity) | | 366 | + result | | 367 | + end | | 368 | + | | 369 | + if function_exported?(module, :before_transaction, 1) or | | 370 | + function_exported?(module, :after_transaction, 1) do | | 371 | + repo.checkout( | | 372 | + fn -> | | 373 | + try do | | 374 | + if function_exported?(module, :before_transaction, 1), | | 375 | + do: module.before_transaction(repo) | | 376 | + | | 377 | + transaction.() | | 378 | + after | | 379 | + if function_exported?(module, :after_transaction, 1), | | 380 | + do: module.after_transaction(repo) | | 381 | + end | | 382 | + end, | | 383 | + timeout: :infinity | | 384 | + ) | | 385 | + else | | 386 | + transaction.() | | 387 | + end | | 388 | + end | | 389 | + | 365 | 390 | defp attempt(repo, config, version, module, direction, operation, reference, opts) do | 366 | 391 | if Code.ensure_loaded?(module) and function_exported?(module, operation, 0) do | 367 | 392 | Runner.run(repo, config, version, module, direction, operation, reference, opts) | |
Collapse file Copy file name to clipboardExpand all lines: test/ecto/migrator_test.exs +172Lines changed: 172 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -130,6 +130,77 @@ defmodule Ecto.MigratorTest do | 130 | 130 | end | 131 | 131 | end | 132 | 132 |
| | 133 | + defmodule Events do | | 134 | + def record(name, repo \\ nil) do | | 135 | + notify( | | 136 | + {:migration_event, {name, repo, TestRepo.in_transaction?(), TestRepo.checked_out?()}} | | 137 | + ) | | 138 | + end | | 139 | + | | 140 | + def notify(message) do | | 141 | + config = Application.get_env(:ecto_sql, EctoSQL.TestAdapter, []) | | 142 | + send(Keyword.fetch!(config, :test_process), message) | | 143 | + end | | 144 | + end | | 145 | + | | 146 | + defmodule MigrationWithBeforeAndAfterTransaction do | | 147 | + use Ecto.Migration | | 148 | + | | 149 | + def before_transaction(repo), do: Events.record(:before_transaction, repo) | | 150 | + def after_begin, do: Events.record(:after_begin) | | 151 | + def up, do: Events.record(:up) | | 152 | + def down, do: Events.record(:down) | | 153 | + def before_commit, do: Events.record(:before_commit) | | 154 | + def after_transaction(repo), do: Events.record(:after_transaction, repo) | | 155 | + end | | 156 | + | | 157 | + defmodule MigrationWithBeforeAndAfterTransactionAndNoTransaction do | | 158 | + use Ecto.Migration | | 159 | + | | 160 | + @disable_ddl_transaction true | | 161 | + | | 162 | + def before_transaction(repo), do: Events.record(:before_transaction, repo) | | 163 | + def up, do: Events.record(:up) | | 164 | + def after_transaction(repo), do: Events.record(:after_transaction, repo) | | 165 | + end | | 166 | + | | 167 | + defmodule MigrationWithoutBeforeAndAfterTransaction do | | 168 | + use Ecto.Migration | | 169 | + | | 170 | + def up, do: Events.record(:up) | | 171 | + end | | 172 | + | | 173 | + defmodule MigrationWithAfterTransactionOnly do | | 174 | + use Ecto.Migration | | 175 | + | | 176 | + def up, do: Events.record(:up) | | 177 | + def after_transaction(repo), do: Events.record(:after_transaction, repo) | | 178 | + end | | 179 | + | | 180 | + defmodule MigrationWithFailingUp do | | 181 | + use Ecto.Migration | | 182 | + | | 183 | + def before_transaction(repo), do: Events.record(:before_transaction, repo) | | 184 | + def up, do: raise("up failed") | | 185 | + def after_transaction(repo), do: Events.record(:after_transaction, repo) | | 186 | + end | | 187 | + | | 188 | + defmodule MigrationWithFailingBeforeTransaction do | | 189 | + use Ecto.Migration | | 190 | + | | 191 | + def before_transaction(_repo), do: raise("before_transaction failed") | | 192 | + def up, do: Events.record(:up) | | 193 | + def after_transaction(repo), do: Events.record(:after_transaction, repo) | | 194 | + end | | 195 | + | | 196 | + defmodule MigrationWithDynamicRepoCallbacks do | | 197 | + use Ecto.Migration | | 198 | + | | 199 | + def before_transaction(repo), do: Events.notify({:dynamic_repo, repo.get_dynamic_repo()}) | | 200 | + | | 201 | + def up, do: :ok | | 202 | + end | | 203 | + | 133 | 204 | defmodule ExecuteOneAnonymousFunctionMigration do | 134 | 205 | use Ecto.Migration | 135 | 206 |
| @@ -920,6 +991,107 @@ defmodule Ecto.MigratorTest do | 920 | 991 | end | 921 | 992 | end | 922 | 993 |
| | 994 | + describe "migration callbacks around the transaction" do | | 995 | + setup do | | 996 | + put_test_adapter_config(supports_ddl_transaction?: true, test_process: self()) | | 997 | + end | | 998 | + | | 999 | + defp events(acc \\ []) do | | 1000 | + receive do | | 1001 | + {:migration_event, event} -> events([event | acc]) | | 1002 | + after | | 1003 | + 0 -> Enum.reverse(acc) | | 1004 | + end | | 1005 | + end | | 1006 | + | | 1007 | + test "run around the transaction, on a checked out connection, going up" do | | 1008 | + assert up(TestRepo, 10, MigrationWithBeforeAndAfterTransaction, log: false) == :ok | | 1009 | + | | 1010 | + assert events() == [ | | 1011 | + {:before_transaction, TestRepo, false, true}, | | 1012 | + {:after_begin, nil, true, true}, | | 1013 | + {:up, nil, true, true}, | | 1014 | + {:before_commit, nil, true, true}, | | 1015 | + {:after_transaction, TestRepo, false, true} | | 1016 | + ] | | 1017 | + | | 1018 | + assert {10, nil} in MigrationsAgent.get() | | 1019 | + end | | 1020 | + | | 1021 | + test "run around the transaction, on a checked out connection, going down" do | | 1022 | + assert up(TestRepo, 10, MigrationWithBeforeAndAfterTransaction, log: false) == :ok | | 1023 | + events() | | 1024 | + | | 1025 | + assert down(TestRepo, 10, MigrationWithBeforeAndAfterTransaction, log: false) == :ok | | 1026 | + | | 1027 | + assert events() == [ | | 1028 | + {:before_transaction, TestRepo, false, true}, | | 1029 | + {:after_begin, nil, true, true}, | | 1030 | + {:down, nil, true, true}, | | 1031 | + {:before_commit, nil, true, true}, | | 1032 | + {:after_transaction, TestRepo, false, true} | | 1033 | + ] | | 1034 | + | | 1035 | + refute {10, nil} in MigrationsAgent.get() | | 1036 | + end | | 1037 | + | | 1038 | + test "run for the dynamic repo" do | | 1039 | + assert up(TestRepo, 10, MigrationWithDynamicRepoCallbacks, | | 1040 | + log: false, | | 1041 | + dynamic_repo: :tenant_db | | 1042 | + ) == :ok | | 1043 | + | | 1044 | + assert_received {:dynamic_repo, :tenant_db} | | 1045 | + end | | 1046 | + | | 1047 | + test "do not check out a connection when there are none" do | | 1048 | + assert up(TestRepo, 10, MigrationWithoutBeforeAndAfterTransaction, log: false) == :ok | | 1049 | + assert events() == [{:up, nil, true, false}] | | 1050 | + end | | 1051 | + | | 1052 | + test "run when only after_transaction is defined" do | | 1053 | + assert up(TestRepo, 10, MigrationWithAfterTransactionOnly, log: false) == :ok | | 1054 | + | | 1055 | + assert events() == [ | | 1056 | + {:up, nil, true, true}, | | 1057 | + {:after_transaction, TestRepo, false, true} | | 1058 | + ] | | 1059 | + end | | 1060 | + | | 1061 | + test "are not run when the transaction is disabled" do | | 1062 | + assert up(TestRepo, 10, MigrationWithBeforeAndAfterTransactionAndNoTransaction, log: false) == | | 1063 | + :ok | | 1064 | + | | 1065 | + assert events() == [{:up, nil, false, false}] | | 1066 | + end | | 1067 | + | | 1068 | + test "are not run when the adapter does not support transactions" do | | 1069 | + put_test_adapter_config(supports_ddl_transaction?: false, test_process: self()) | | 1070 | + | | 1071 | + assert up(TestRepo, 10, MigrationWithBeforeAndAfterTransaction, log: false) == :ok | | 1072 | + assert events() == [{:up, nil, false, false}] | | 1073 | + end | | 1074 | + | | 1075 | + test "still run after_transaction when the migration fails" do | | 1076 | + assert_raise RuntimeError, "up failed", fn -> | | 1077 | + up(TestRepo, 10, MigrationWithFailingUp, log: false) | | 1078 | + end | | 1079 | + | | 1080 | + assert events() == [ | | 1081 | + {:before_transaction, TestRepo, false, true}, | | 1082 | + {:after_transaction, TestRepo, false, true} | | 1083 | + ] | | 1084 | + end | | 1085 | + | | 1086 | + test "run after_transaction but not the migration when before_transaction fails" do | | 1087 | + assert_raise RuntimeError, "before_transaction failed", fn -> | | 1088 | + up(TestRepo, 10, MigrationWithFailingBeforeTransaction, log: false) | | 1089 | + end | | 1090 | + | | 1091 | + assert events() == [{:after_transaction, TestRepo, false, true}] | | 1092 | + end | | 1093 | + end | | 1094 | + | 923 | 1095 | describe "migrations_path" do | 924 | 1096 | test "is inferred from the repository name" do | 925 | 1097 | path = migrations_path(TestRepo) | |
Collapse file Copy file name to clipboardExpand all lines: test/support/test_repo.exs +9-2Lines changed: 9 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -33,8 +33,6 @@ defmodule EctoSQL.TestAdapter do | 33 | 33 | {:ok, child_spec, %{meta: :meta}} | 34 | 34 | end | 35 | 35 |
| 36 | | - def checkout(_, _, _), do: raise("not implemented") | 37 | | - def checked_out?(_), do: raise("not implemented") | 38 | 36 | def delete(_, _, _, _, _), do: raise("not implemented") | 39 | 37 | def insert_all(_, _, _, _, _, _, _, _), do: raise("not implemented") | 40 | 38 | def rollback(_, _), do: raise("not implemented") | @@ -75,6 +73,15 @@ defmodule EctoSQL.TestAdapter do | 75 | 73 | {:ok, []} | 76 | 74 | end | 77 | 75 |
| | 76 | + def checked_out?(_), do: Process.get(:checked_out?) || false | | 77 | + | | 78 | + def checkout(_, _opts, fun) do | | 79 | + Process.put(:checked_out?, true) | | 80 | + fun.() | | 81 | + after | | 82 | + Process.put(:checked_out?, false) | | 83 | + end | | 84 | + | 78 | 85 | def in_transaction?(_), do: Process.get(:in_transaction?) || false | 79 | 86 |
| 80 | 87 | def transaction(mod, _opts, fun) do | |
|