aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/attribute_methods/dirty.rb
Commit message (Collapse)AuthorAgeFilesLines
* Correct deprecation warnings in `ActiveRecord::Dirty`Sean Griffin2016-11-301-6/+6
| | | | I had pointed the messages at the new behavior, not the old.
* Deprecate the behavior of AR::Dirty inside of after_(create|update|save) ↵Sean Griffin2016-11-011-6/+173
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* normalizes indentation and whitespace across the projectXavier Noria2016-08-061-36/+36
|
* applies new string literal convention in activerecord/libXavier Noria2016-08-061-2/+2
| | | | | The current code base is not uniform. After some discussion, we have chosen to go with double quotes by default.
* Add Singleton in NullMutationTracker classacapilleri2015-10-031-1/+1
| | | | to reduce allocation of same object
* Build the `AttributeMutationTracker` lazilySean Griffin2015-10-021-13/+15
| | | | | For reads, we never need to construct this object. The double `defined?` check is to avoid errors in tests
* Fix test failures on MySQLSean Griffin2015-10-021-1/+3
| | | | | There were a few places where I missed a `create` vs `new` before_type_cast check, and the semantics of `reload` became wrong.
* Further encapsulate dirty checking on `Attribute`Sean Griffin2015-10-021-4/+7
| | | | | | | | | | | | | | | | | | | 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.
* Separate `dup` from `deep_dup` in the attributes hashSean Griffin2015-09-281-1/+1
| | | | | | | | | | | I'm looking to move towards a tree-like structure for dirty checking that involves an attribute holding onto the attribute that it was created from. This means that `changed?` can be fully encapsulated on that object. Since the objects are immutable, in `changes_applied`, we can simply perform a shallow dup, instead of a deep one. I'm not sure if that will actually end up in a performance boost, but I'd like to semantically separate these concepts regardless
* We still need to reset `@changed_attributes` in `changes_applied`Sean Griffin2015-09-241-0/+2
| | | | | | When I removed the call to `super` to avoid the setting of `@previous_changes`, I forgot to duplicate the other part of that behavior, which led to failing tests
* Improve the performance of `save` and friendsSean Griffin2015-09-241-2/+10
| | | | | | | | | | | | | | | | | | | | | | 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
* Encapsulate a lot of the logic from `Dirty` in an objectSean Griffin2015-09-241-24/+9
| | | | | | | | | 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.
* Clean up the implementation of AR::DirtySean Griffin2015-09-241-78/+39
| | | | | | | | | | | | | This moves a bit more of the logic required for dirty checking into the attribute objects. I had hoped to remove the `with_value_from_database` stuff, but unfortunately just calling `dup` on the attribute objects isn't enough, since the values might contain deeply nested data structures. I think this can be cleaned up further. This makes most dirty checking become lazy, and reduces the number of object allocations and amount of CPU time when assigning a value. This opens the door (but doesn't quite finish) to improving the performance of writes to a place comparable to 4.1
* Don't crash when mutating attributes in a getterSean Griffin2015-06-121-1/+6
| | | | | | | | | | | If a getter has side effects on the DB, `changes_applied` will be called twice. The second time will try and remove the changed attributes cache, and will crash because it's already been unset. This also demonstrates that we shouldn't assume that calling getters won't change the value of `changed_attributes`, and we need to clear the cache if an attribute is modified. Fixes #20531.
* Attribute assignment and type casting has nothing to do with columnsSean Griffin2015-01-311-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's finally finished!!!!!!! The reason the Attributes API was kept private in 4.2 was due to some publicly visible implementation details. It was previously implemented by overloading `columns` and `columns_hash`, to make them return column objects which were modified with the attribute information. This meant that those methods LIED! We didn't change the database schema. We changed the attribute information on the class. That is wrong! It should be the other way around, where schema loading just calls the attributes API for you. And now it does! Yes, this means that there is nothing that happens in automatic schema loading that you couldn't manually do yourself. (There's still some funky cases where we hit the connection adapter that I need to handle, before we can turn off automatic schema detection entirely.) There were a few weird test failures caused by this that had to be fixed. The main source came from the fact that the attribute methods are now defined in terms of `attribute_names`, which has a clause like `return [] unless table_exists?`. I don't *think* this is an issue, since the only place this caused failures were in a fake adapter which didn't override `table_exists?`. Additionally, there were a few cases where tests were failing because a migration was run, but the model was not reloaded. I'm not sure why these started failing from this change, I might need to clear an additional cache in `reload_schema_from_cache`. Again, since this is not normal usage, and it's expected that `reset_column_information` will be called after the table is modified, I don't think it's a problem. Still, test failures that were unrelated to the change are worrying, and I need to dig into them further. Finally, I spent a lot of time debugging issues with the mutex used in `define_attribute_methods`. I think we can just remove that method entirely, and define the attribute methods *manually* in the call to `define_attribute`, which would simplify the code *tremendously*. Ok. now to make this damn thing public, and work on moving it up to Active Model.
* Errors raised in `type_cast_for_database` no longer raise on assignmentSean Griffin2015-01-231-1/+1
| | | | Fixes #18580.
* Don't calculate in-place changes on attribute assignmentSean Griffin2015-01-181-1/+1
| | | | | | | | | | | When an attribute is assigned, we determine if it was already marked as changed so we can determine if we need to clear the changes, or mark it as changed. Since this only affects the `attributes_changed_by_setter` hash, in-place changes are irrelevant to this process. Since calculating in-place changes can be expensive, we can just skip it here. I also added a test for the only edge case I could think of that would be affected by this change.
* Don't attempt to save dirty attributes which are not persistableSean Griffin2015-01-101-1/+1
| | | | | | | | | | | | | | | | This sets a precident for how we handle `attribute` calls, which aren't backed by a database column. We should not take this as a conscious decision on how to handle them, and this can change when we make `attribute` public if we have better ideas in the future. As the composed attributes API gets fleshed out, I expect the `persistable_attributes` method to change to `@attributes.select(&:persistable).keys`, or some more performant variant there-of. This can probably go away completely once we fully move dirty checking into the attribute objects once it gets moved up to Active Model. Fixes #18407
* Don't calculate all in-place changes to determine if attribute_changed?Sean Griffin2014-12-221-0/+4
| | | | | | | | | Calling `changed_attributes` will ultimately check if every mutable attribute has changed in place. Since this gets called whenever an attribute is assigned, it's extremely slow. Instead, we can avoid this calculation until we actually need it. Fixes #18029
* Ensure numericality validations work with mutationSean Griffin2014-12-011-6/+6
| | | | | | | | | | | | | | | | | | | | | The detection of in-place changes caused a weird unexpected issue with numericality validations. That validator (out of necessity) works on the `_before_type_cast` version of the attribute, since on an `:integer` type column, a non-numeric string would type cast to 0. However, strings are mutable, and we changed strings to ensure that the post type cast version of the attribute was a different instance than the before type cast version (so the mutation detection can work properly). Even though strings are the only mutable type for which a numericality validation makes sense, special casing strings would feel like a strange change to make here. Instead, we can make the assumption that for all mutable types, we should work on the post-type-cast version of the attribute, since all cases which would return 0 for non-numeric strings are immutable. Fixes #17852
* Improve the performance of reading attributesSean Griffin2014-11-181-1/+1
| | | | | | | We added a comparison to "id", and call to `self.class.primary_key` a *lot*. We also have performance hits from `&block` all over the place. We skip the check in a new method, in order to avoid breaking the behavior of `read_attribute`
* Cache the value of `changed_attributes` when calling `changes_applied`Sean Griffin2014-08-221-1/+20
| | | | | | | `changes_applied` calles `changes`, which will call `changed_attributes` multiple times in a loop. This method actually performs work now, so we should cache the results while looping over it when we know it cannot change.
* Don't calculate in-place changes twiceSean Griffin2014-08-221-8/+0
| | | | | | Now that `changed_attributes` includes in place changes, we don't need to override these methods in Active Record. Partially fixes the performance regression caused by #16189
* Implement `_was` and `changes` for in-place mutations of AR attributesSean Griffin2014-08-161-11/+14
|
* Deprecate ActiveModel::Dirty#reset_changes in favor of ↵Rafael Mendonça França2014-07-151-2/+2
| | | | | | | | | #clear_changes_information This method name is causing confusion with the `reset_#{attribute}` methods. While `reset_name` set the value of the name attribute for the previous value the `reset_changes` only discard the changes and previous changes.
* Return a null object from `AttributeSet#[]`Sean Griffin2014-06-201-3/+3
|
* Use `column_defaults` in dirty for checking changed defaultsSean Griffin2014-06-171-11/+3
| | | | | | We no longer need to "init changed attributes" from the initializer, either, as there is no longer a case where a given value would differ from the default, but would not already be marked as changed.
* Refactor in-place dirty checking to use the attribute objectSean Griffin2014-06-161-9/+3
|
* Detect in-place changes on mutable AR attributesSean Griffin2014-06-131-6/+77
| | | | | | We have several mutable types on Active Record now. (Serialized, JSON, HStore). We need to be able to detect if these have been modified in place.
* Do not type cast twice on attribute assignmentSean Griffin2014-06-071-11/+20
| | | | | | | | | | | | | The definition of `write_attribute` in dirty checking ultimately leads to the columns calling `type_cast` on the value to perform the comparison. However, this is a potentially expensive computation that we cache when it occurs in `read_attribute`. The only case that we need the non-type-cast form is for numeric, so we pass that through as well (something I'm looking to remove in the future). This also reduces the number of places that manually access various stages in an attribute's type casting lifecycle, which will aid in one of the larger refactorings that I'm working on.
* Return a null column when no column exists for an attributeSean Griffin2014-06-031-2/+1
|
* Refactor determination of whether the field has changedSean Griffin2014-06-031-27/+2
| | | | | The types know more about what is going on than the dirty module. Let's ask them!
* Rename attribute related instance variables to better express intentSean Griffin2014-05-301-1/+1
| | | | | | | | | `@attributes` was actually used for `_before_type_cast` and friends, while `@attributes_cache` is the type cast version (and caching is the wrong word there, but I'm working on removing the conditionals around that). I opted for `@raw_attributes`, because `_before_type_cast` is also semantically misleading. The values in said hash are in the state given by the form builder or database, so raw seemed to be a good word.
* [Active Record] Renamed private methods create_record and update_recordPrathamesh Sonpatki2014-02-201-2/+2
| | | | | | This is to ensure that they are not accidentally called by the app code. They are renamed to _create_record and _update_record respectively. Closes #11645
* Merge pull request #13799 from kbrock/better_dirtyJeremy Kemper2014-01-221-1/+21
|\ | | | | Better ActiveRecord hierarchy for Dirty and others
| * Move changed_attributes into dirty.rbKeenan Brock2014-01-221-1/+21
| | | | | | Move serialization dirty into serialization.rb
* | Add more tests for the dirty feature for enumsRafael Mendonça França2014-01-211-1/+0
| |
* | Extract all attribute changed work to its own methodRafael Mendonça França2014-01-211-3/+7
|/ | | | This will make easier to hook on this feature to customize the behavior
* Merge pull request #10816 from bogdan/less-dirty-dirtyRafael Mendonça França2013-09-231-9/+6
| | | | Make AM::Dirty less dirty to plugin into AR or other library
* Removed deprecated methods partial_updates and familyNeeraj Singh2013-07-021-11/+0
| | | | | Removed deprecated methods `partial_updates`, `partial_updates?` and `partial_updates=`
* assigning '0.0' to a nullable numeric column does not make it dirtyYves Senn2013-03-051-1/+5
|
* These are already required through AS/railsAkira Matsuda2013-01-071-1/+0
| | | | | | * dependencies/autoload * concern * deprecation
* Rename update_attributes method to update, keep update_attributes as an aliasAmparo Luna + Guillermo Iguaran2013-01-031-2/+2
|
* Remove ActiveRecord::ModelJon Leighton2012-10-261-6/+2
| | | | | | | | | | In the end I think the pain of implementing this seamlessly was not worth the gain provided. The intention was that it would allow plain ruby objects that might not live in your main application to be subclassed and have persistence mixed in. But I've decided that the benefit of doing that is not worth the amount of complexity that the implementation introduced.
* Revert "Get rid of the ActiveRecord::Model::DeprecationProxy thing."Jeremy Kemper2012-10-201-1/+1
| | | | This reverts commit 83846838252397b3781eed165ca301e05db39293.
* Get rid of the ActiveRecord::Model::DeprecationProxy thing.Jon Leighton2012-10-191-1/+1
| | | | | | | | | | | | | | | | | I think it's going to be too much pain to try to transition the :active_record load hook from executing against Base to executing against Model. For example, after Model is included in Base, and modules included in Model will no longer get added to the ancestors of Base. So plugins which wish to be compatible with both Model and Base should use the :active_record_model load hook which executes *before* Base gets loaded. In general, ActiveRecord::Model is an advanced feature at the moment and probably most people will continue to inherit from ActiveRecord::Base for the time being.
* Rename the partial_updates config to partial_writesJon Leighton2012-10-191-6/+18
| | | | This reflects the fact that it now impact inserts as well as updates.
* The default value of a text/blob in mysql strict mode should be nilJon Leighton2012-10-191-11/+1
| | | | | | | | | In non-strict mode it is '', but if someone is in strict mode then we should honour the strict semantics. Also, this removes the need for a completely horrible hack in dirty.rb. Closes #7780
* Merge branch 'master' of github.com:lifo/docrailsVijay Dev2012-09-281-4/+4
|\ | | | | | | | | Conflicts: actionpack/lib/action_view/helpers/asset_tag_helper.rb
| * fix AR::AttributeMethods::Dirty :nodoc: [ci skip]Francesco Rodriguez2012-09-211-4/+4
| |
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-03 07:46:32 +0000