| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
`attributes` (`values` in `LazyAttributeHash` in `AttributeSet`) has
string keys, not symbols.
Fixes #29665.
|
| |
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This was never really intended to work (at least not without calling
`define_attribute_methods`, which is less common with Active Record). As
we move forward the intention is to require the use of `attribute` over
`attr_accessor` for more complex model behavior both on Active Record
and Active Model, so this behavior is deprecated.
Fixes #27956.
Close #27963.
[Alex Serban & Sean Griffin]
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
callbacks
We pretty frequently get bug reports that "dirty is broken inside of
after callbacks". Intuitively they are correct. You'd expect
`Model.after_save { puts changed? }; model.save` to do the same thing as
`model.save; puts model.changed?`, but it does not.
However, changing this goes much farther than just making the behavior
more intuitive. There are a _ton_ of places inside of AR that can be
drastically simplified with this change. Specifically, autosave
associations, timestamps, touch, counter cache, and just about anything
else in AR that works with callbacks have code to try to avoid "double
save" bugs which we will be able to flat out remove with this change.
We introduce two new sets of methods, both with names that are meant to
be more explicit than dirty. The first set maintains the old behavior,
and their names are meant to center that they are about changes that
occurred during the save that just happened. They are equivalent to
`previous_changes` when called outside of after callbacks, or once the
deprecation cycle moves.
The second set is the new behavior. Their names imply that they are
talking about changes from the database representation. The fact that
this is what we really care about became clear when looking at
`BelongsTo.touch_record` when tests were failing. I'm unsure that this
set of methods should be in the public API. Outside of after callbacks,
they are equivalent to the existing methods on dirty.
Dirty itself is not deprecated, nor are the methods inside of it. They
will only emit the warning when called inside of after callbacks. The
scope of this breakage is pretty large, but the migration path is
simple. Given how much this can improve our codebase, and considering
that it makes our API more intuitive, I think it's worth doing.
|
| | |
|
| |
|
|
| |
to reduce allocation of same object
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We can skip the allocation of a full `AttributeSet` by changing the
semantics of how we structure things. Instead of comparing two separate
`AttributeSet` objects, and `Attribute` is now a singly linked list of
every change that has happened to it. Since the attribute objects are
immutable, to apply the changes we simply need to copy the head of the
list.
It's worth noting that this causes one subtle change in the behavior of
AR. When a record is saved successfully, the `before_type_cast` version
of everything will be what was sent to the database. I honestly think
these semantics make more sense, as we could have just as easily had the
DB do `RETURNING *` and updated the record with those if we had things
like timestamps implemented at the DB layer.
This brings our performance closer to 4.2, but we're still not quite
there.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The biggest source of the performance regression in these methods
occurred because dirty tracking required eagerly materializing and type
casting the assigned values. In the previous commits, I've changed dirty
tracking to perform the comparisons lazily. However, all of this is moot
when calling `save`, since `changes_applied` will be called, which just
ends up eagerly materializing everything, anyway. With the new mutation
tracker, it's easy to just compare the previous two hashes in the same
lazy fashion.
We will not have aliasing issues with this setup, which is proven by the
fact that we're able to detect nested mutation.
Before:
User.create! 2.007k (± 7.1%) i/s - 10.098k
After:
User.create! 2.557k (± 3.5%) i/s - 12.789k
Fixes #19859
|
|
|
In order to improve the performance of dirty checking, we're going to
need to duplicate all of the `previous_` methods in Active Model.
However, these methods are basically the same as their non-previous
counterparts, but comparing `@original_attributes` to
`@previous_original_attributes` instead of `@attributes` and
`@original_attributes`. This will help reduce that duplication.
|
