Calling `self.class` multiple times is not cheap.
```ruby
class A
def self.foo
end
def foo1
self.class.foo
self.class.foo
self.class.foo
self.class.foo
end
def foo2
klass = self.class
klass.foo
klass.foo
klass.foo
klass.foo
end
end
a = A.new
Benchmark.ips do |x|
x.report("foo1") { a.foo1 }
x.report("foo2") { a.foo2 }
end
```
```
Warming up --------------------------------------
foo1 341.701k i/100ms
foo2 414.000k i/100ms
Calculating -------------------------------------
foo1 3.194M (± 5.4%) i/s - 16.060M in 5.044653s
foo2 4.276M (± 3.8%) i/s - 21.528M in 5.041999s
```
Similar with #36052.
Commit a1a5d37749964b1e1a23914ef13da327403e34cb doesn't properly set the
foreign key on associations. Instead of trying to patch the change
revert it for now, so we can investigate a better solution.
This reverts commit a1a5d37749964b1e1a23914ef13da327403e34cb
It also reverts the follow-up commits:
* Revert "Rename internal `@saving` state to `@_saving`"
This reverts commit 2eb5458978f3f993ccc414b321b35fb1aef1efd2
* Revert "Add `_` prefix for the internal methods"
This reverts commit 12c0bec15275fa458bc0ddd6b57f7a0ae7881bd5.
* Revert "protected :can_save?"
This reverts commit 8cd3b657f8795aedb9bc97e725642cbd04dc09b8.
* Revert "Exclude #saving? from API docs"
This reverts commit 35d3923ea0c51b3a628e913e3b35f85124de8ac8.
Add an optional mode argument to
Core#strict_loading! to support n_plus_one_only
mode. Currently, when we turn on strict_loading
for a single record, it will raise even if we are
loading an association that is relatively safe to
lazy load like a belongs_to. This can be helpful
for some use cases, but prevents us from using
strict_loading to identify only problematic
instances of lazy loading.
The n_plus_one_only argument allows us to turn
strict_loading on for a single record, and only
raise when a N+1 query is likely to be executed.
When loading associations on a single record,
this only happens when we go through a has_many
association type. Note that the has_many
association itself is not problematic as it only
requires one query. We do this by turning
strict_loading on for each record that is loaded
through the has_many. This ensures that any
subsequent lazy loads on these records will raise
a StrictLoadingViolationError.
For example, where a developer belongs_to a ship
and each ship has_many parts, we expect the
following behaviour:
developer.strict_loading!(mode: :n_plus_one_only)
# Do not raise when a belongs_to association
# (:ship) loads its has_many association (:parts)
assert_nothing_raised do
developer.ship.parts.to_a
end
refute developer.ship.strict_loading?
assert developer.ship.parts.all?(&:strict_loading?)
assert_raises ActiveRecord::StrictLoadingViolationError do
developer.ship.parts.first.trinkets.to_a
end
Usually we add `_` prefix for newly added short term living (used)
internal state (e.g. ae02898, d1107f4, dcb8259), and also `@saving`
might be already used in users' code.
Add `_` prefix for the internal methods since `saving?`, `can_save?`,
and `saving` are too shorter naming for the internal methods, it can
easily be overridden by the user defined methods.
Also, I've removed `saving?` since the internal method is used only for
testing.
Autosave will double save records for some cyclic associations.
For example a child record with a parent association.
If save is called on the child before the parent has been saved,
the child will be saved twice.
The double save of the child will clear the mutation tracker on
the child record resulting in an incorrect dirty state.
```
# pirate has_one ship
ship = Ship.new(name: "Nights Dirty Lightning")
pirate = ship.build_pirate(catchphrase: "Aye")
ship.save!
ship.previous_changes # => returns {} but this should contain the changes.
```
When saving ship the following happens:
1. the `before_save` callbacks of the ship are called
2. the callbacks call `autosave_associated_records_for_pirate`
3. `autosave_associated_records_for_pirate` saves the pirate
4. the `after_save` callbacks of the pirate are called
5. the callbacks call `autosave_associated_records_for_ship`
6. `autosave_associated_records_for_ship` saves the ship
7. the ship is saved again by the original save
`autosave_associated_records_for_ship` saves the ship because the ship
association is set by inverse_of in a `has_one` on the pirate. This does
not happen with a `has_many` by default because the inverse is not set.
If setting the inverse on the `has_many` the problem occurs as well.
----------------------------
This commit adds a @saving state which tracks if a record is currently being saved.
If @saving is set to true, the record won't be saved by the autosave callbacks.
With this commit the following happens when saving a ship:
1. @saving is set to true
2. the `before_save` callbacks of the ship are called
3. the callbacks call `autosave_associated_records_for_pirate`
5. `autosave_associated_records_for_pirate` saves the pirate
6. the `after_save` callbacks of the pirate are called
6. `autosave_associated_records_for_ship` skip saving the ship
8. the ship is saved.
9. @saving is set to false
One disadvantage of this approach is the following...
While the child is no longer saved in the autosave, similar children
could be autosaved. This will result in unexpected order when creating
new records, as similar children will be commited first.
If an association scope would generate the same query but have different
values for `extending`, `skip_query_cache` or `strict_loading` we should
consider them the same for the purpose of generating the preload
batches.
I've found the test doesn't catch a future regression since it doesn't
assert the result of the preload.
Also, add extra assertions to assert the difference of the value type.
This reverts commit 8da6ba9cae21beae1ee3c379db7b7113d2731c9b.
The type is not standard to make sure preload work when the types don't
match.
See #14855.
After assigning string "infinity" to datetime attribute with postgresql adapter, reading it back gets Float::INFINITY.
But assigning Float::INFINITY to datetime attribute results in getting nil value when ActiveRecord::Base.time_zone_aware_attributes is true.
This is due to TimeZoneConverter not handling Float::INFINITY appropriately.