aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test/cases/transactions_test.rb
Commit message (Collapse)AuthorAgeFilesLines
* Fix dirty tracking after rollback.Ryuta Kamizono2019-04-161-0/+59
| | | | | | | | | | | | | | | | | Currently the rollback only restores primary key value, `new_record?`, `destroyed?`, and `frozen?`. Since the `save` clears current dirty attribute states, retrying save after rollback will causes no change saved if partial writes is enabled (by default). This makes `remember_transaction_record_state` remembers original values then restores dirty attribute states after rollback. Fixes #15018. Fixes #30167. Fixes #33868. Fixes #33443. Closes #33444. Closes #34504.
* Don't expose `add_to_transaction`Ryuta Kamizono2019-04-141-1/+1
| | | | | | | | | | | | | | `add_to_transaction` was added at da840d1, but it should not be called by except internal, since `remember_transaction_record_state` should be called only once before saving. And also, currently `add_to_transaction` doesn't always add the record to transaction since da8de91, that is the reason hard to use that even in internal. Even if `add_to_transaction` ensure to add the record to transaction, that is an internal concern, people don't need to explicitly call `add_to_transaction`.
* Lazy sync with transaction state on destroyRyuta Kamizono2019-04-101-6/+8
| | | | | | | | | | | | | | | | This reverts commit 58410b3d566e6b93c7b71c0eec0fc11ec906b68e. If we have any implicit commit/rollback callbacks, it is necessary to add record to transaction explicitly like `:touch_deferred_attributes`. https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L9 https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L25 But I can't find any other implicit commit/rollback callbacks in our code base at least now. I think the `self.class.connection.add_transaction_record(self)` line doesn't cover any behavior.
* Add assertions for lazy sync transaction stateRyuta Kamizono2019-04-101-36/+49
|
* Remove deprecated `#set_state` from the transaction objectRafael Mendonça França2019-01-171-11/+0
|
* No need to handle if FrozenError is availableYasuo Honda2018-12-231-1/+1
| | | | | | | Rails 6 requires Ruby 2.5, which introduces `FrozenError` https://docs.ruby-lang.org/en/2.5.0/NEWS.html Related to #31520
* Change the empty block style to have space inside of the blockRafael Mendonça França2018-09-251-5/+5
|
* Omit BEGIN/COMMIT statements for empty transactionsEugene Kenny2018-08-131-1/+71
| | | | | | | | | | | | | | | | | | | | | | If a transaction is opened and closed without any queries being run, we can safely omit the `BEGIN` and `COMMIT` statements, as they only exist to modify the connection's behaviour inside the transaction. This removes the overhead of those statements when saving a record with no changes, which makes workarounds like `save if changed?` unnecessary. This implementation buffers transactions inside the transaction manager and materializes them the next time the connection is used. For this to work, the adapter needs to guard all connection use with a call to `materialize_transactions`. Because of this, adapters must opt in to get this new behaviour by implementing `supports_lazy_transactions?`. If `raw_connection` is used to get a reference to the underlying database connection, the behaviour is disabled and transactions are opened eagerly, as we can't know how the connection will be used. However when the connection is checked back into the pool, we can assume that the application won't use the reference again and reenable lazy transactions. This prevents a single `raw_connection` call from disabling lazy transactions for the lifetime of the connection.
* Enable `Layout/EmptyLinesAroundBlockBody` to reduce review cost in the futureRyuta Kamizono2018-07-121-1/+0
| | | | | | | We sometimes ask "✂️ extra blank lines" to a contributor in reviews like https://github.com/rails/rails/pull/33337#discussion_r201509738. It is preferable to deal automatically without depending on manpower.
* Rollback correctly restore initial record id after double saveRyuta Kamizono2018-05-191-0/+1
|
* Finalize transaction record state after real transactionEugene Kenny2018-05-191-0/+29
| | | | | | | | | | | | | | | | | | After a real (non-savepoint) transaction has committed or rolled back, the original persistence-related state for all records modified in that transaction is discarded or restored, respectively. When the model has transactional callbacks, this happens synchronously in the `committed!` or `rolled_back!` methods; otherwise, it happens lazily the next time the record's persistence-related state is accessed. The synchronous code path always finalizes the state of the record, but the lazy code path only pops one "level" from the transaction counter, assuming it will always reach zero immediately after a real transaction. As the test cases included here demonstrate, that isn't always the case. By using the same logic as the synchronous code path, we ensure that the record's state is always updated after a real transaction has finished.
* Fix `CustomCops/AssertNot` to allow it to have failure messageRyuta Kamizono2018-05-131-18/+18
| | | | Follow up of #32605.
* Don't clear transaction state after manual rollbackEugene Kenny2018-05-101-0/+12
| | | | | | | If an `ActiveRecord::Rollback` error was raised by a persistence method (e.g. in an `after_save` callback), this logic would potentially discard the original state of the record from before the transaction, preventing it from being restored later when the transaction was rolled back.
* Replace `assert !` with `assert_not`Daniel Colson2018-04-191-4/+4
| | | | | This autocorrects the violations after adding a custom cop in 3305c78dcd.
* Autocorrect `refute` RuboCop violationsDaniel Colson2018-04-031-7/+7
| | | | | | 73e7aab behaved as expected on codeship, failing the build with exactly these RuboCop violations. Hopefully `rubocop -a` will have been enough to get a passing build!
* Use assert_predicate and assert_not_predicateDaniel Colson2018-01-251-28/+28
|
* Handle `FrozenError` if it is availableYasuo Honda2017-12-201-1/+1
| | | | | | | | | | | | | | | This pull request handles `FrozenError` introduced by Ruby 2.5. Refer https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/61131 Since `FrozenError` is a subclass of `RuntimeError` minitest used by master branch can handle it, though it would be better to handle `FrozenError` explicitly if possible. `FrozenError` does not exist in Ruby 2.4 or lower, `frozen_error_class` handles which exception is expected to be raised. This pull request is intended to be merged to master, then backported to `5-1-stable` to address #31508
* Run `ConcurrentTransactionTest` if `supports_transaction_isolation?`Yasuo Honda2017-11-081-15/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | returns true Not only postgresql or mysql2 adapter, Oracle enhanced adapter whose default isolation level is read commited, passes these two test cases. `ConcurrentTransactionTest#test_transaction_per_thread` `ConcurrentTransactionTest#test_transaction_isolation__read_committed` ```ruby $ ARCONN=oracle bin/test test/cases/transactions_test.rb:961 -v Using oracle Run options: -v --seed 18865 ConcurrentTransactionTest#test_transaction_per_thread = 0.98 s = . Finished in 1.061036s, 0.9425 runs/s, 5.6549 assertions/s. 1 runs, 6 assertions, 0 failures, 0 errors, 0 skips ``` ```ruby $ ARCONN=oracle bin/test test/cases/transactions_test.rb:979 -v Using oracle Run options: -v --seed 13341 ConcurrentTransactionTest#test_transaction_isolation__read_committed = 1.85 s = . Finished in 1.928637s, 0.5185 runs/s, 10.3700 assertions/s. 1 runs, 20 assertions, 0 failures, 0 errors, 0 skips $ ``` Also, regardless it is a file based or memory based these tests could fail with SQLite3Adapter. (Extra CR added to make lines shorter) ```ruby $ ARCONN=sqlite3 bin/test test/cases/transactions_test.rb:961 -v Using sqlite3 Run options: -v --seed 18815 ConcurrentTransactionTest#test_transaction_per_thread = /home/yahonda/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sqlite3-1.3.13/lib/sqlite3/statement.rb:108:in `step': SQLite3::BusyException: database is locked: UPDATE "topics" SET "approved" = ?, "updated_at" = ? WHERE "topics"."id" = ? (ActiveRecord::StatementInvalid) ``` ```ruby $ ARCONN=sqlite3 bin/test test/cases/transactions_test.rb:979 -v Using sqlite3 Run options: -v --seed 25520 ConcurrentTransactionTest#test_transaction_isolation__read_committed = 0.12 s = E /home/yahonda/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/sqlite3-1.3.13/lib/sqlite3/statement.rb:108:in `step': SQLite3::BusyException: database is locked: UPDATE "developers" SET "salary" = ?, "updated_at" = ?, "updated_on" = ? WHERE "developers"."id" = ? (ActiveRecord::StatementInvalid) ```
* `Mysql2Adapter` should pass `ConcurrentTransactionTest`Ryuta Kamizono2017-11-091-1/+1
|
* Merge pull request #29724 from eugeneius/sync_primary_keyMatthew Draper2017-08-021-3/+49
|\ | | | | Sync transaction state when accessing primary key
| * Sync transaction state when accessing primary keyEugene Kenny2017-07-091-3/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a record is modified inside a transaction, it must check the outcome of that transaction before accessing any state which would no longer be valid if it was rolled back. For example, consider a new record that was saved inside a transaction which was later rolled back: it should be restored to its previous state so that saving it again inserts a new row into the database instead of trying to update a row that no longer exists. The `id` and `id=` methods defined on the PrimaryKey module implement this correctly, but when a model uses a custom primary key, the reader and writer methods for that attribute must check the transaction state too. The `read_attribute` and `write_attribute` methods also need to check the transaction state when accessing the primary key.
* | Use frozen-string-literal in ActiveRecordKir Shatrov2017-07-191-0/+2
|/
* :warning: "Use assert_nil if expecting nil. This will fail in Minitest 6."Akira Matsuda2017-07-021-1/+1
|
* Apply record state based on parent transaction stateeileencodes2017-07-011-0/+70
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's say you have a nested transaction and both records are saved. Before the outer transaction closes, a rollback is performed. Previously the record in the outer transaction would get marked as not persisted but the inner transaction would get persisted. ```ruby Post.transaction do post_one.save # will get rolled back Post.transaction(requires_new: true) do post_two.save # incorrectly remains marked as persisted end raise ActiveRecord::Rollback end ``` To fix this the PR changes transaction handling to have the child transaction ask the parent how the records should be marked. When there are child transactions, it will always be a SavpointTransaction because the stack isn't empty. From there we pass the parent_transaction to the child SavepointTransaction where we add the children to the parent so the parent can mark the inner transaction as rolledback and thus mark the record as not persisted. `update_attributes_from_transaction_state` uses the `completed?` check to correctly mark all the transactions as rolledback and the inner record as not persisted. ```ruby Post.transaction do post_one.save # will get rolled back Post.transaction(requires_new: true) do post_two.save # with new behavior, correctly marked as not persisted on rollback end raise ActiveRecord::Rollback end ``` Fixes #29320
* Deprecate and replace `set_state` methodeileencodes2017-07-011-0/+38
| | | | | | | | | | | | | `set_state` was directly setting the transaction state instance variable. It's better to set the state via specific methods (`rollback!` and `commit!` respectively. While undocumented and untested, it's possible someone is using `set_state` in their app or gem so I've added a deprecation notice to it. No where in the app do we use `nullify!` but I wanted to keep existing behavior while replacing the method with a better pattern.
* Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"Matthew Draper2017-07-021-1/+0
| | | | | This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
* Enforce frozen string in RubocopKir Shatrov2017-07-011-0/+1
|
* Add test cases for #28274Ryuta Kamizono2017-06-151-0/+46
| | | | | | | | `object.id` is correctly restored since #29378 has merged. Closes #28274, Closes #28395. [Ryuta Kamizono & Eugene Kenny]
* Restore `fixtures :author_addresses`Ryuta Kamizono2017-04-271-1/+1
| | | | | This change reverted in eac6f369 but it is needed for data integrity. See #25328.
* Revert "Merge pull request #27636 from ↵Rafael Mendonça França2017-04-261-1/+1
| | | | | | | | | mtsmfm/disable-referential-integrity-without-superuser-privilege-take-2" This reverts commit c1faca6333abe4b938b98fedc8d1f47b88209ecf, reversing changes made to 8c658a0ecc7f2b5fc015d424baf9edf6f3eb2b0b. See https://github.com/rails/rails/pull/27636#issuecomment-297534129
* Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser ↵Fumiaki MATSUSHIMA2017-03-261-1/+1
| | | | | | | | | | | | | | | privileges (take 2) Re-create https://github.com/rails/rails/pull/21233 eeac6151a5 was reverted (127509c071b4) because it breaks tests. ---------------- ref: 72c1557254 - We must use `authors` fixture with `author_addresses` because of its foreign key constraint. - Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.
* Remove deprecated behavior that halts callbacks when the return is falseRafael Mendonça França2017-02-071-10/+0
|
* :warning: "Use assert_nil if expecting nil. This will fail in MT6."Akira Matsuda2017-01-181-1/+5
| | | | | | These are followups for 307065f959f2b34bdad16487bae906eb3bfeaf28, but TBH I'm personally not very much confortable with this style. Maybe we could override assert_equal in our test_helper not to warn?
* Revert "Merge pull request #21233 from ↵Rafael Mendonça França2017-01-031-1/+1
| | | | | | | | | | mtsmfm/disable-referential-integrity-without-superuser-privileges" This reverts commit eeac6151a55cb7d5f799e1ae33aa64a839cbc3aa, reversing changes made to 5c40239d3104543e70508360d27584a3e4dc5baf. Reason: Broke the isolated tests. https://travis-ci.org/rails/rails/builds/188721346
* Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser ↵Fumiaki MATSUSHIMA2016-12-031-1/+1
| | | | | | | | | privileges ref: 72c1557254 - We must use `authors` fixture with `author_addresses` because of its foreign key constraint. - Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.
* Add more rubocop rules about whitespacesRafael Mendonça França2016-10-291-5/+5
|
* improve error message when include assertions failMichael Grosser2016-09-161-2/+2
| | | | | | assert [1, 3].includes?(2) fails with unhelpful "Asserting failed" message assert_includes [1, 3], 2 fails with "Expected [1, 3] to include 2" which makes it easier to debug and more obvious what went wrong
* Fix broken comments indentation caused by rubocop auto-correct [ci skip]Ryuta Kamizono2016-09-141-3/+4
| | | | | | All indentation was normalized by rubocop auto-correct at 80e66cc4d90bf8c15d1a5f6e3152e90147f00772. But comments was still kept absolute position. This commit aligns comments with method definitions for consistency.
* Add three new rubocop rulesRafael Mendonça França2016-08-161-2/+2
| | | | | | | | Style/SpaceBeforeBlockBraces Style/SpaceInsideBlockBraces Style/SpaceInsideHashLiteralBraces Fix all violations in the repository.
* normalizes indentation and whitespace across the projectXavier Noria2016-08-061-8/+8
|
* modernizes hash syntax in activerecordXavier Noria2016-08-061-19/+19
|
* applies new string literal convention in activerecord/testXavier Noria2016-08-061-44/+44
| | | | | The current code base is not uniform. After some discussion, we have chosen to go with double quotes by default.
* Allow add_to_transaction with null transactionEmanuel Evans2015-12-281-0/+5
| | | | Fixes https://github.com/rails/rails/issues/22819
* Removed mocha from Active Record Part 2Ronak Jangir2015-09-161-6/+10
|
* Fix state being carried over from previous transactionRoque Pinel2015-07-201-0/+7
| | | | | | | | | | | | | | | This clears the transaction record state when the transaction finishes with a `:committed` status. Considering the following example where `name` is a required attribute. Before we had `new_record?` returning `true` for a persisted record: ```ruby author = Author.create! name: 'foo' author.name = nil author.save # => false author.new_record? # => true ```
* Silence deprecation warning from force reloadPrem Sichanugrist2015-07-161-2/+2
| | | | | | | | | | | | We deprecate the support for passing an argument to force reload in 6eae366d0d2e5d5211eeaf955f56bd1dc6836758. That led to several deprecation warning when running Active Record test suite. This commit silence the warnings by properly calling `#reload` on the association proxy or on the association object instead. However, there are several places that `ActiveSupport::Deprecation.silence` are used as those tests actually tests the force reload functionality and will be removed once `master` is targeted next minor release (5.1).
* Closes rails/rails#18864: Renaming transactional fixtures to transactional testsBrandon Weiss2015-03-161-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | I’m renaming all instances of `use_transcational_fixtures` to `use_transactional_tests` and “transactional fixtures” to “transactional tests”. I’m deprecating `use_transactional_fixtures=`. So anyone who is explicitly setting this will get a warning telling them to use `use_transactional_tests=` instead. I’m maintaining backwards compatibility—both forms will work. `use_transactional_tests` will check to see if `use_transactional_fixtures` is set and use that, otherwise it will use itself. But because `use_transactional_tests` is a class attribute (created with `class_attribute`) this requires a little bit of hoop jumping. The writer method that `class_attribute` generates defines a new reader method that return the value being set. Which means we can’t set the default of `true` using `use_transactional_tests=` as was done previously because that won’t take into account anyone using `use_transactional_fixtures`. Instead I defined the reader method manually and it checks `use_transactional_fixtures`. If it was set then it should be used, otherwise it should return the default, which is `true`. If someone uses `use_transactional_tests=` then it will overwrite the backwards-compatible method with whatever they set.
* Fix transaction state for unsynced records when entering transactionbrainopia2015-03-041-0/+9
|
* Fix rollback of frozen recordsbrainopia2015-03-041-0/+9
|
* prefer `drop_table if_exists: true` over raw SQL.Yves Senn2015-02-181-1/+1
| | | | | | | /cc @yahonda This makes it easier for third party adapters to run our tests, even if that database does not support IF EXISTS.