Reusing the "books" one could cause interferences when fixtures are
loaded in a very specific order such as:
https://buildkite.com/rails/rails/builds/76217#ee4ce591-e6c1-4a0d-a7db-1f83647d141e
Reproduction script:
```
activerecord $ bin/test -v --seed 23607 -n "/^(?:EagerAssociationTest#(?:test_preloading_a_regular_association_with_a_typo_through_a_polymorphic_association_still_raises)|ActiveRecord::Encryption::EncryptableFixtureTest#(?:test_fixtures_get_encrypted_automatically)|ViewWithoutPrimaryKeyTest#(?:test_attributes|test_reading))$/"
```
This implements several changes to encourage deterministic encryption to
remain unchanged. The main motivation is letting you define unique
indexes on deterministically-encrypted columns:
- By default, deterministic encryption will always use the oldest
encryption scheme to encrypt new data, when there are many.
- You can skip this default behavior and make it always use the current
encryption scheme with:
```ruby
deterministic: { fixed: false } # using this should be a rare need
```
- Deterministic encryption still supports previous encryption schemes
normally. So they will be used to add additional values to queries, for
example.
- You can't rotate deterministic encryption keys anymore. We can add
support for that in the future.
This makes for reasonable defaults:
- People using "deterministic: true" will get unique indexes working out
of the box.
- The system will encourage keeping deterministic encryption stable:
- By always using oldest encryption schemes
- By forbidding configuring multiple keys
But you can still opt-out of the default if you need to.
We are not encrypting attributes when loading models with the table
missing. This way we make sure we only load the encrypted models when
necessary during the encryption tests and prevent the problem of missing
encrypted attributes due to having cached the class without them encrypted.
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.
