| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`test_update_does_not_run_sql_if_record_has_not_changed` would pass
without #18501 since `assert_queries` ignores BEGIN/COMMIT unless
`ignore_none: true` is given.
Since #32647, empty BEGIN/COMMIT is ommited. So we no longer need to use
`assert_queries(0)` to ignore BEGIN/COMMIT in the queries.
|
| |
| |
| |
| | |
cherry-picked from https://github.com/rails/rails/pull/33763
|
| |
| |
| |
| | |
missing records
|
| |
| |
| |
| | |
Since counter cache handles touch option too.
|
|\ \
| | |
| | | |
If association is a hash-like object preloading fails
|
| | |
| | |
| | |
| | |
| | |
| | | |
If you pass a hash-like object to preload associations (for example ActionController::Parameters)
preloader will fail with the ArgumentError.
This change allows passing objects that may be converted to a Hash or String into a preloader
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
`association.increment_counters` and `association.decrement_counters`
works regardless of parent target is loaded or not.
Related 52e11e462f6114a4d12225c639c5f501f0ffec7a.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This reverts commit 376ffe0ea2e59dc51461122210729c05a10fb443.
Since 38fae1f, `association.increment_counters` is called without
inflated parent target if inverse_of is disabled.
In that case, that commit would cause extra queries to inflate parent.
|
|/ /
| |
| |
| | |
Fixes #19550.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This is a 4th attempt to make counter cache transactional completely.
Past attempts: #9236, #14849, #23357.
All existing counter cache issues (increment/decrement twice, lost
increment) are caused due to updating counter cache on the outside of
the record saving transaction by assigning belongs_to record, even
though assigning that doesn't cause the record saving.
We have the `@_after_replace_counter_called` guard condition to mitigate
double increment/decrement issues, but we can't completely prevent that
inconsistency as long as updating counter cache on the outside of the
transaction, since saving the record is not always happened after that.
We already have handling counter cache after create/update/destroy,
https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/counter_cache.rb#L162-L189
https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/associations/builder/belongs_to.rb#L33-L59
so just removing assigning logic on the belongs_to association makes
counter cache transactional completely.
Closes #14849.
Closes #23357.
Closes #31493.
Closes #31494.
Closes #32372.
Closes #33113.
Closes #33117
Closes #33129.
Closes #33458.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`topic` and `reply` belongs_to associations on `SillyReply` are defined
with the same `foreign_key` (`parent_id`) and `counter_cache`
(`replies_count`) columns.
This would cause unintentional side-effect (e.g. saving `SillyReply`
object would cause double increment `replies_count`), so it is better to
avoid that side-effect.
|
| |
| |
| |
| | |
same record
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Since 213796f, bind params are used for IN clause if enabled prepared
statements.
Unfortunately, most adapter modules have a limitation for # of bind
params (mysql2 65535, pg 65535, sqlite3 250000). So if eager loading
large number of records at once, that query couldn't be sent to the
database.
Since eager loading/preloading queries are auto-generated by Active
Record itself, so it should be worked regardless of large number of
records like as before.
Fixes #33702.
|
| |
| |
| |
| |
| | |
Without this, `inverse_associations_test.rb` breaks when running in isolation.
https://travis-ci.org/rails/rails/jobs/422266840#L1894-L1899
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously ActiveRecord couldn't find inverse associations if they were plural, which is a pretty standard use case. This commit changes the behavior to first attempt to find the singular version, then attempt to find the plural version. That makes it work and find plural associations as in the example below:
```
class Post
has_many :comments
end
class Comment
belongs_to :post
end
```
Previously the `:post` association reflection would only attempt to find a `comment` inverse, as opposed to both a `comment` and `comments` inverse.
|
| | |
|
| |
| |
| |
| |
| |
| | |
There is test in has_one to test enum, but there is no for has_many.
[Rich Chen]
|
|\ \
| | |
| | | |
Stop using Mocha
|
| |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Six Mocha calls prove quite resistant to Minitestification. For example,
if we replace
```
ActiveRecord::Associations::HasManyAssociation
.any_instance
.expects(:reader)
.never
```
with `assert_not_called`, Minitest wisely raises
```
NameError: undefined method `reader' for class `ActiveRecord::Associations::HasManyAssociation'
```
as `:reader` comes from a deeply embedded abstract class,
`ActiveRecord::Associations::CollectionAssociation`.
This patch tackles this difficulty by adding
`ActiveSupport::Testing::MethodCallAsserts#assert_called_on_instance_of`
which injects a stubbed method into `klass`, and verifies the number of
times it is called, similar to `assert_called`. It also adds a convenience
method, `assert_not_called_on_instance_of`, mirroring
`assert_not_called`.
It uses the new method_call_assertions to replace the remaining Mocha
calls in `ActiveRecord` tests.
[utilum + bogdanvlviv + kspath]
|
| |
| |
| |
| |
| |
| | |
without define the enum in class “SpecialBook”, any string status will be casted to integer 0. Then the test have no meaning.
[Rich Chen]
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since fe9547b6, numericality validator would parse raw value only when a
value came from user to work type casting to a value from database.
But that was caused a regression that the validator would work against
getter value instead of parsed raw value, a getter is sometimes
customized by people. #33550
There we never guarantees that the value before type cast was going to
the used in this validation (actually here is only place that getter
value might not be used), but we should not change the behavior unless
there is some particular reason.
The purpose of fe9547b6 is to work type casting to a value from
database. We could achieve the purpose by using `read_attribute`,
without using getter value.
Fixes #33550.
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, the seen object cache is shared if join nodes have the same
target class. But it is a wrong assumption, we can't share the seen
object cache between different join nodes (e.g. `:readonly_account` and
`:accounts` have the same target class `Account`, but the instances
have the different state `readonly`).
Fixes #26805.
Closes #27737.
|
|
|
|
| |
hash condition
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, column aliases which is used for eager loading are calculated
before constructing all table aliases in FROM clause.
`JoinDependency#join_constraints` constructs table aliases for `joins`
first, and then always re-constructs table aliases for eager loading.
If both `joins` and eager loading are given a same table association,
the re-construction would cause the discrepancy between column aliases
and table aliases.
To avoid the discrepancy, the column aliases should be calculated after
all table aliases are constructed.
Fixes #30603.
|
|
|
|
|
| |
It mark the association as loaded and this can cause the object to be in
an stale state.
|
|
|
|
|
| |
This also mark the association as loaded given we changed it in memory
and avoid the next access to the reader to make a query to the databse.
|
|
|
|
|
|
|
|
|
|
| |
left_joins
This regression was caused by #30995 due to `Hash#fetch` won't invoke
default proc. Just revert the change since #30995 is completely fixed by
e9c1653.
Fixes #33048.
|
|
|
|
|
|
|
|
|
|
|
| |
In `_create_record`, explicit `transaction` block requires rollback
handling manually when `insert_record` is failed.
We need to handle it in `_create_record`, not in `insert_record`, since
our test cases expect a record added to target and returned even if
`insert_record` is failed,
Closes #31488.
|
|
|
|
| |
Should be done before `before_add` callbacks.
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #31405, using `#increment!` with touch option instead of `#touch`
to touch belongs_to association if counter cache is enabled. It caused
the regression since `#increment!` won't invoke after_touch callbacks
even if touch option is given.
To fix the regression, make `#increment!` invokes after_touch callbacks
if touch option is given.
Fixes #31559.
Fixes #32408.
|
|
|
|
|
|
|
|
|
|
|
| |
On belongs_to with `touch: true` association, unassigned object is
caused touching, but assigned object is not touched.
And also, if primary key is customized, it will touch against the wrong
target looked up by the customized key as primary key.
This change ensures correctly touching consistently between assigning
and unassigning.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
counter
If belongs_to primary key is customized, the callback will update
counters against the wrong target looked up by the customized key as
primary key.
We need to convert the customized key into an object that can be
referred to as primary key.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #31575, `BelongsToAssociation#target=` replaces owner record's
foreign key to fix an inverse association bug.
But the method is not only used for inverse association but also used
for eager loading/preloading, it caused some public behavior changes
(#32338, #32375).
To avoid any side-effect in loading associations, I reverted the
overriding `#target=`, then introduced `#inversed_from` to replace
foreign key in `set_inverse_instance`.
Closes #32375.
|
|
|
|
| |
Follow up of #32605.
|
|
|
|
| |
If a 'has one' object is created from a new record, an ActiveRecord::RecordNotSaved error is raised but this behavior was also applied to the reverse scenario.
|
|\
| |
| | |
Use MethodCallAssertions instead of mocha expects
|
| | |
|
| | |
|
| |
| |
| |
| |
| |
| |
| | |
Currently `ids_reader` doesn't respect dirty target when the target is
not loaded yet unlike `collection.size`. I believe the inconsistency is
a bug, fixes the `ids_reader` to behave consistently regardless of
whether target is loaded or not.
|
|\ \
| | |
| | | |
Loaded associations should not run a new query when size is called
|
| |/
| |
| |
| |
| |
| |
| | |
Already loaded associations were running an extra query when `size` was called on the association.
This fix ensures that an extra query is no longer run.
Update tests to use proper methods
|
|/
|
|
|
| |
This is to ensure that the behavior has not changed before and after
#31575.
|
|
|
|
| |
Follow up of #32514.
|
|\
| |
| |
| |
| | |
samdec/multiple-has-one-through-associations-build-bug
Fix .new with multiple through associations
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This fixes a bug with building an object that has multiple
`has_many :through` associations through the same object.
Previously, when building the object via .new, the intermediate
object would be created instead of just being built.
Here's an example:
Given a GameBoard, that has_one Owner and Collection through Game.
The following line would cause a game object to be created in the
database.
GameBoard.new(owner: some_owner, collection: some_collection)
Whereas, if passing only one of those associations into `.new` would
cause the Game object to be built and not created in the database.
Now the above code will only build the Game object, and not save it.
|
| | |
|
| | |
|