| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The test added in 12b0b26df7560ab5199ba830586864085441508f passes even
without this code since 9b8c7796a9c2048208aa843ad3dc477dffa8bdee, as the
call to `id` in `remember_transaction_record_state` now triggers a
`sync_with_transaction_state` which discards the leftover state from the
previous transaction.
This issue had already been fixed for `save!`, `destroy` and `touch` in
caae79a385ce112245262a17414bcd96bea013c2, but continued to affect `save`
because the call to `rollback_active_record_state!` in that method would
increment the transaction level before `add_to_transaction` could clear
it, preventing the fix from working correctly.
As `rollback_active_record_state!` was removed entirely in
48007d5390db47fc1223f57c8e7ab3ebb7c3a3d7, this code is no longer needed.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since a record is already persisted in `after_create_commit`, so `save`
should invoke only `after_update_commit`.
This bug is caused by depending on `@_start_transaction_state` for
rollback to consider whether it was `new_record` before being committed.
If after commit callbacks caused another commit, the state before
last commit is no longer `new_record`.
Fixes #32831.
Closes #18367.
Closes #31106.
|
| |
|
|
| |
Fixes #32806.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |\
| |
| | |
Remove ActiveRecord::Transactions#rollback_active_record_state!
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`rollback_active_record_state!` was removed from `save!` but not `save`
in da840d13da865331297d5287391231b1ed39721b. I believe that leaving it
in `save` was a mistake, since that commit was intended to move the
rollback logic from the `save`/`save!` call to the transaction stack.
As of 67d8bb963d5d51fc644d6b1ca20164efb4cee6d7 the record's original
state is lazily restored the first time it's accessed after the
transaction, instead of when a rollback occurs. This means that the call
to `restore_transaction_record_state` here has no effect: the record's
transaction level is incremented twice (in rollback_active_record_state!
and `with_transaction_returning_status`), isn't decremented again until
the the `ensure` block runs, and won't hit zero until the next time
`sync_with_transaction_state` is called.
|
| |/
|
|
|
|
|
| |
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.
|
| |
|
|
| |
Commit callbacks are intentionally disabled when errors occur when calling the callback chain in order to reset the internal record state. However, the implicit order of operations on the logic for checking if callbacks are disabled is wrong. The result is that callbacks can be unexpectedly when errors occur in transactions.
|
| | |
|
| | |
|
| |
|
|
| |
[ci skip]
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
refs: https://github.com/rails/rails/pull/30161
```
$ echo "+@size+" | rdoc --pipe
<p>+@size+</p>
$ echo "<tt>@size</tt>" | rdoc --pipe
<p><code>@size</code></p>
```
[ci skip]
|
| |\
| |
| | |
Sync transaction state when accessing primary key
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |/ |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | |
|
| |
|
|
|
| |
The old top level classes PGconn, PGresult and PGError were deprecated
since pg-0.13.0: https://github.com/ged/ruby-pg/blob/master/History.rdoc#v0130-2012-02-09-michael-granger-gedfaeriemudorg
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
| |
Race conditions can occur when an ActiveRecord is destroyed
twice or destroyed and updated. The callbacks should only be
triggered once, similar to a SQL database trigger.
|
| |
|
|
|
|
| |
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.
|
| | |
|
| |
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| | |
|
| |
|
|
|
|
|
|
| |
Just noticed this on the [edge API].
[ci skip]
[edge API]: http://edgeapi.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Those are actually shortcuts for `after_commit`.
Before:
after_commit :add_to_index_later, on: :create
after_commit :update_in_index_later, on: :update
after_commit :remove_from_index_later, on: :destroy
After:
after_create_commit :add_to_index_later
after_update_commit :update_in_index_later
after_destroy_commit :remove_from_index_later
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The focus of this change is to make the API more accessible.
References to method and classes should be linked to make it easy to
navigate around.
This patch makes exzessiv use of `rdoc-ref:` to provide more readable
docs. This makes it possible to document `ActiveRecord::Base#save` even
though the method is within a separate module
`ActiveRecord::Persistence`. The goal here is to bring the API closer to
the actual code that you would write.
This commit only deals with Active Record. The other gems will be
updated accordingly but in different commits. The pass through Active
Record is not completely finished yet. A follow up commit will change
the spots I haven't yet had the time to update.
/cc @fxn
|
| |
|
|
|
|
| |
Rails 4.2.3 AS::Callbacks will not halt chain if `false` is returned.
That is the behavior of specific callbacks like AR::Callbacks and
AM::Callbacks.
|
| |
|
|
|
| |
skip]
Bumps from `5.6` to `5.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
```
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This reverts commit bdc1d329d4eea823d07cf010064bd19c07099ff3.
Before:
Calculating -------------------------------------
22.000 i/100ms
-------------------------------------------------
229.700 (± 0.4%) i/s - 1.166k
Total Allocated Object: 9939
After:
Calculating -------------------------------------
24.000 i/100ms
-------------------------------------------------
246.443 (± 0.8%) i/s - 1.248k
Total Allocated Object: 7939
```
begin
require 'bundler/inline'
rescue LoadError => e
$stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
raise e
end
gemfile(true) do
source 'https://rubygems.org'
# gem 'rails', github: 'rails/rails', ref: 'bdc1d329d4eea823d07cf010064bd19c07099ff3'
gem 'rails', github: 'rails/rails', ref: 'd2876141d08341ec67cf6a11a073d1acfb920de7'
gem 'arel', github: 'rails/arel'
gem 'sqlite3'
gem 'benchmark-ips'
end
require 'active_record'
require 'benchmark/ips'
ActiveRecord::Base.establish_connection('sqlite3::memory:')
ActiveRecord::Migration.verbose = false
ActiveRecord::Schema.define do
create_table :users, force: true do |t|
t.string :name, :email
t.boolean :admin
t.timestamps null: false
end
end
class User < ActiveRecord::Base
default_scope { where(admin: true) }
end
admin = true
1000.times do
attributes = {
name: "Lorem ipsum dolor sit amet, consectetur adipiscing elit.",
email: "foobar@email.com",
admin: admin
}
User.create!(attributes)
admin = !admin
end
GC.disable
Benchmark.ips(5, 3) do |x|
x.report { User.all.to_a }
end
key =
if RUBY_VERSION < '2.2'
:total_allocated_object
else
:total_allocated_objects
end
before = GC.stat[key]
User.all.to_a
after = GC.stat[key]
puts "Total Allocated Object: #{after - before}"
```
|
| | |
|
| |
|
|
|
| |
See commit 890da514, this is not intended.
So fix indent.
|
| |
|
|
| |
This reverts commit 796cab45561fce268aa74e6587cdb9cae3bb243e.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
| |
This reverts commit 393e65b4170608593ad82377a9eadc918e85698d and
ec51c3fedd16b561d096dcc1a6705fdc02ab7666
We don't want the records to hold hard references to transactions
because they point at records that have callbacks.
|
| |
|
|
|
| |
this way we don't have to mutate a state object, we can just change the
state of the txn
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We only set the state on the record if that condition is `false` in the
first place, so we dont need to call that again. Also that call is
expensive, follow benchmark with before and after this change:
```
Calculating -------------------------------------
persisted? 15.272k i/100ms
-------------------------------------------------
persisted? 350.119k (± 4.6%) i/s - 1.756M
```
```
Calculating -------------------------------------
persisted? 25.988k i/100ms
-------------------------------------------------
persisted? 1.294M (± 5.3%) i/s - 6.445M
```
(benchmark borrowed from 57d35b2bf9e48173a5f97ccff5e6897f0c46411f)
|
| |
|
|
|
| |
depth is always 0, so the index will always be false. No reason to
create the instance variable if it isn't used
|
| | |
|
