mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Deprecate the behavior of AR::Dirty inside of after_(create|update|save) 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.
This commit is contained in:
Sean Griffin
parent
6b5ad4979e
commit
16ae3db5a5
18 changed files with 362 additions and 53 deletions
|
|
@ -166,7 +166,7 @@ module ActiveRecord
|
|||
def initialize_attributes(record, except_from_scope_attributes = nil) #:nodoc:
|
||||
except_from_scope_attributes ||= {}
|
||||
skip_assign = [reflection.foreign_key, reflection.type].compact
|
||||
assigned_keys = record.changed
|
||||
assigned_keys = record.changed_attribute_names_to_save
|
||||
assigned_keys += except_from_scope_attributes.keys.map(&:to_s)
|
||||
attributes = create_scope.except(*(assigned_keys - skip_assign))
|
||||
record.assign_attributes(attributes)
|
||||
|
|
|
|||
|
|
@ -35,17 +35,17 @@ module ActiveRecord::Associations::Builder # :nodoc:
|
|||
@_after_create_counter_called = false
|
||||
elsif (@_after_replace_counter_called ||= false)
|
||||
@_after_replace_counter_called = false
|
||||
elsif attribute_changed?(foreign_key) && !new_record?
|
||||
elsif saved_change_to_attribute?(foreign_key) && !new_record?
|
||||
if reflection.polymorphic?
|
||||
model = attribute(reflection.foreign_type).try(:constantize)
|
||||
model_was = attribute_was(reflection.foreign_type).try(:constantize)
|
||||
model = attribute_in_database(reflection.foreign_type).try(:constantize)
|
||||
model_was = attribute_before_last_save(reflection.foreign_type).try(:constantize)
|
||||
else
|
||||
model = reflection.klass
|
||||
model_was = reflection.klass
|
||||
end
|
||||
|
||||
foreign_key_was = attribute_was foreign_key
|
||||
foreign_key = attribute foreign_key
|
||||
foreign_key_was = attribute_before_last_save foreign_key
|
||||
foreign_key = attribute_in_database foreign_key
|
||||
|
||||
if foreign_key && model.respond_to?(:increment_counter)
|
||||
model.increment_counter(cache_column, foreign_key)
|
||||
|
|
@ -70,14 +70,16 @@ module ActiveRecord::Associations::Builder # :nodoc:
|
|||
klass.attr_readonly cache_column if klass && klass.respond_to?(:attr_readonly)
|
||||
end
|
||||
|
||||
def self.touch_record(o, foreign_key, name, touch, touch_method) # :nodoc:
|
||||
old_foreign_id = o.changed_attributes[foreign_key]
|
||||
def self.touch_record(o, changes, foreign_key, name, touch, touch_method) # :nodoc:
|
||||
old_foreign_id = changes[foreign_key] && changes[foreign_key].first
|
||||
|
||||
if old_foreign_id
|
||||
association = o.association(name)
|
||||
reflection = association.reflection
|
||||
if reflection.polymorphic?
|
||||
klass = o.public_send("#{reflection.foreign_type}_was").constantize
|
||||
foreign_type = reflection.foreign_type
|
||||
klass = changes[foreign_type] && changes[foreign_type].first || o.public_send(foreign_type)
|
||||
klass = klass.constantize
|
||||
else
|
||||
klass = association.klass
|
||||
end
|
||||
|
|
@ -107,13 +109,13 @@ module ActiveRecord::Associations::Builder # :nodoc:
|
|||
n = reflection.name
|
||||
touch = reflection.options[:touch]
|
||||
|
||||
callback = lambda { |record|
|
||||
BelongsTo.touch_record(record, foreign_key, n, touch, belongs_to_touch_method)
|
||||
}
|
||||
callback = lambda { |changes_method| lambda { |record|
|
||||
BelongsTo.touch_record(record, record.send(changes_method), foreign_key, n, touch, belongs_to_touch_method)
|
||||
}}
|
||||
|
||||
model.after_save callback, if: :changed?
|
||||
model.after_touch callback
|
||||
model.after_destroy callback
|
||||
model.after_save callback.(:saved_changes), if: :saved_changes?
|
||||
model.after_touch callback.(:changes_to_save)
|
||||
model.after_destroy callback.(:changes_to_save)
|
||||
end
|
||||
|
||||
def self.add_destroy_callbacks(model, reflection)
|
||||
|
|
|
|||
|
|
@ -375,7 +375,7 @@ module ActiveRecord
|
|||
persisted.map! do |record|
|
||||
if mem_record = memory.delete(record)
|
||||
|
||||
((record.attribute_names & mem_record.attribute_names) - mem_record.changes.keys).each do |name|
|
||||
((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save).each do |name|
|
||||
mem_record[name] = record[name]
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ module ActiveRecord
|
|||
return target unless target || record
|
||||
|
||||
assigning_another_record = target != record
|
||||
if assigning_another_record || record.changed?
|
||||
if assigning_another_record || record.has_changes_to_save?
|
||||
save &&= owner.persisted?
|
||||
|
||||
transaction_if(save) do
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
# frozen_string_literal: true
|
||||
require "active_support/core_ext/module/attribute_accessors"
|
||||
require "active_record/attribute_mutation_tracker"
|
||||
|
||||
|
|
@ -15,6 +16,18 @@ module ActiveRecord
|
|||
|
||||
class_attribute :partial_writes, instance_writer: false
|
||||
self.partial_writes = true
|
||||
|
||||
after_create { changes_internally_applied }
|
||||
after_update { changes_internally_applied }
|
||||
|
||||
# Attribute methods for "changed in last call to save?"
|
||||
attribute_method_affix(prefix: "saved_change_to_", suffix: "?")
|
||||
attribute_method_prefix("saved_change_to_")
|
||||
attribute_method_suffix("_before_last_save")
|
||||
|
||||
# Attribute methods for "will change if I call save?"
|
||||
attribute_method_affix(prefix: "will_save_change_to_", suffix: "?")
|
||||
attribute_method_suffix("_change_to_be_saved", "_in_database")
|
||||
end
|
||||
|
||||
# Attempts to +save+ the record and clears changed attributes if successful.
|
||||
|
|
@ -35,8 +48,8 @@ module ActiveRecord
|
|||
# <tt>reload</tt> the record and clears changed attributes.
|
||||
def reload(*)
|
||||
super.tap do
|
||||
@mutation_tracker = nil
|
||||
@previous_mutation_tracker = nil
|
||||
clear_mutation_trackers
|
||||
@changed_attributes = HashWithIndifferentAccess.new
|
||||
end
|
||||
end
|
||||
|
|
@ -46,19 +59,26 @@ module ActiveRecord
|
|||
@attributes = self.class._default_attributes.map do |attr|
|
||||
attr.with_value_from_user(@attributes.fetch_value(attr.name))
|
||||
end
|
||||
@mutation_tracker = nil
|
||||
clear_mutation_trackers
|
||||
end
|
||||
|
||||
def changes_internally_applied # :nodoc:
|
||||
@mutations_before_last_save = mutation_tracker
|
||||
forget_attribute_assignments
|
||||
@mutations_from_database = AttributeMutationTracker.new(@attributes)
|
||||
end
|
||||
|
||||
def changes_applied
|
||||
@previous_mutation_tracker = mutation_tracker
|
||||
@changed_attributes = HashWithIndifferentAccess.new
|
||||
store_original_attributes
|
||||
clear_mutation_trackers
|
||||
end
|
||||
|
||||
def clear_changes_information
|
||||
@previous_mutation_tracker = nil
|
||||
@changed_attributes = HashWithIndifferentAccess.new
|
||||
store_original_attributes
|
||||
forget_attribute_assignments
|
||||
clear_mutation_trackers
|
||||
end
|
||||
|
||||
def raw_write_attribute(attr_name, *)
|
||||
|
|
@ -80,17 +100,27 @@ module ActiveRecord
|
|||
if defined?(@cached_changed_attributes)
|
||||
@cached_changed_attributes
|
||||
else
|
||||
emit_warning_if_needed("changed_attributes", "attributes_in_database")
|
||||
super.reverse_merge(mutation_tracker.changed_values).freeze
|
||||
end
|
||||
end
|
||||
|
||||
def changes
|
||||
cache_changed_attributes do
|
||||
emit_warning_if_needed("changes", "changes_to_save")
|
||||
super
|
||||
end
|
||||
end
|
||||
|
||||
def previous_changes
|
||||
unless previous_mutation_tracker.equal?(mutations_before_last_save)
|
||||
ActiveSupport::Deprecation.warn(<<-EOW.strip_heredoc)
|
||||
The behavior of `previous_changes` inside of after callbacks is
|
||||
deprecated without replacement. In the next release of Rails,
|
||||
this method inside of `after_save` will return the changes that
|
||||
were just saved.
|
||||
EOW
|
||||
end
|
||||
previous_mutation_tracker.changes
|
||||
end
|
||||
|
||||
|
|
@ -98,6 +128,109 @@ module ActiveRecord
|
|||
mutation_tracker.changed_in_place?(attr_name)
|
||||
end
|
||||
|
||||
# Did this attribute change when we last saved? This method can be invoked
|
||||
# as `saved_change_to_name?` instead of `saved_change_to_attribute?("name")`.
|
||||
# Behaves similarly to +attribute_changed?+. This method is useful in
|
||||
# after callbacks to determine if the call to save changed a certain
|
||||
# attribute.
|
||||
#
|
||||
# ==== Options
|
||||
#
|
||||
# +from+ When passed, this method will return false unless the original
|
||||
# value is equal to the given option
|
||||
#
|
||||
# +to+ When passed, this method will return false unless the value was
|
||||
# changed to the given value
|
||||
def saved_change_to_attribute?(attr_name, **options)
|
||||
mutations_before_last_save.changed?(attr_name, **options)
|
||||
end
|
||||
|
||||
# Returns the change to an attribute during the last save. If the
|
||||
# attribute was changed, the result will be an array containing the
|
||||
# original value and the saved value.
|
||||
#
|
||||
# Behaves similarly to +attribute_change+. This method is useful in after
|
||||
# callbacks, to see the change in an attribute that just occurred
|
||||
#
|
||||
# This method can be invoked as `saved_change_to_name` in instead of
|
||||
# `saved_change_to_attribute("name")`
|
||||
def saved_change_to_attribute(attr_name)
|
||||
mutations_before_last_save.change_to_attribute(attr_name)
|
||||
end
|
||||
|
||||
# Returns the original value of an attribute before the last save.
|
||||
# Behaves similarly to +attribute_was+. This method is useful in after
|
||||
# callbacks to get the original value of an attribute before the save that
|
||||
# just occurred
|
||||
def attribute_before_last_save(attr_name)
|
||||
mutations_before_last_save.original_value(attr_name)
|
||||
end
|
||||
|
||||
# Did the last call to `save` have any changes to change?
|
||||
def saved_changes?
|
||||
mutations_before_last_save.any_changes?
|
||||
end
|
||||
|
||||
# Returns a hash containing all the changes that were just saved.
|
||||
def saved_changes
|
||||
mutations_before_last_save.changes
|
||||
end
|
||||
|
||||
# Alias for `attribute_changed?`
|
||||
def will_save_change_to_attribute?(attr_name, **options)
|
||||
mutations_from_database.changed?(attr_name, **options)
|
||||
end
|
||||
|
||||
# Alias for `attribute_change`
|
||||
def attribute_change_to_be_saved(attr_name)
|
||||
mutations_from_database.change_to_attribute(attr_name)
|
||||
end
|
||||
|
||||
# Alias for `attribute_was`
|
||||
def attribute_in_database(attr_name)
|
||||
mutations_from_database.original_value(attr_name)
|
||||
end
|
||||
|
||||
# Alias for `changed?`
|
||||
def has_changes_to_save?
|
||||
mutations_from_database.any_changes?
|
||||
end
|
||||
|
||||
# Alias for `changes`
|
||||
def changes_to_save
|
||||
mutations_from_database.changes
|
||||
end
|
||||
|
||||
# Alias for `changed`
|
||||
def changed_attribute_names_to_save
|
||||
changes_to_save.keys
|
||||
end
|
||||
|
||||
# Alias for `changed_attributes`
|
||||
def attributes_in_database
|
||||
changes_to_save.transform_values(&:first)
|
||||
end
|
||||
|
||||
def attribute_was(*)
|
||||
emit_warning_if_needed("attribute_was", "attribute_in_database")
|
||||
super
|
||||
end
|
||||
|
||||
def attribute_change(*)
|
||||
emit_warning_if_needed("attribute_change", "attribute_change_to_be_saved")
|
||||
super
|
||||
end
|
||||
|
||||
def attribute_changed?(*)
|
||||
emit_warning_if_needed("attribute_changed?", "will_save_change_to_attribute?")
|
||||
super
|
||||
end
|
||||
|
||||
def changed(*)
|
||||
emit_warning_if_needed("changed", "changed_attribute_names_to_save")
|
||||
super
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def mutation_tracker
|
||||
|
|
@ -107,12 +240,37 @@ module ActiveRecord
|
|||
@mutation_tracker ||= AttributeMutationTracker.new(@attributes)
|
||||
end
|
||||
|
||||
def emit_warning_if_needed(method_name, new_method_name)
|
||||
unless mutation_tracker.equal?(mutations_from_database)
|
||||
ActiveSupport::Deprecation.warn(<<-EOW.squish)
|
||||
The behavior of `#{method_name}` inside of after callbacks will
|
||||
be changing in the next version of Rails. The new return value will reflect the
|
||||
behavior of calling the method after `save` returned (e.g. the opposite of what
|
||||
it returns now). To maintain the current behavior, use `#{new_method_name}`
|
||||
instead.
|
||||
EOW
|
||||
end
|
||||
end
|
||||
|
||||
def mutations_from_database
|
||||
unless defined?(@mutations_from_database)
|
||||
@mutations_from_database = nil
|
||||
end
|
||||
@mutations_from_database ||= mutation_tracker
|
||||
end
|
||||
|
||||
def changes_include?(attr_name)
|
||||
super || mutation_tracker.changed?(attr_name)
|
||||
end
|
||||
|
||||
def clear_attribute_change(attr_name)
|
||||
mutation_tracker.forget_change(attr_name)
|
||||
mutations_from_database.forget_change(attr_name)
|
||||
end
|
||||
|
||||
def attribute_will_change!(attr_name)
|
||||
super
|
||||
mutations_from_database.force_change(attr_name)
|
||||
end
|
||||
|
||||
def _update_record(*)
|
||||
|
|
@ -124,18 +282,27 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def keys_for_partial_write
|
||||
changed & self.class.column_names
|
||||
changed_attribute_names_to_save & self.class.column_names
|
||||
end
|
||||
|
||||
def store_original_attributes
|
||||
def forget_attribute_assignments
|
||||
@attributes = @attributes.map(&:forgetting_assignment)
|
||||
end
|
||||
|
||||
def clear_mutation_trackers
|
||||
@mutation_tracker = nil
|
||||
@mutations_from_database = nil
|
||||
@mutations_before_last_save = nil
|
||||
end
|
||||
|
||||
def previous_mutation_tracker
|
||||
@previous_mutation_tracker ||= NullMutationTracker.instance
|
||||
end
|
||||
|
||||
def mutations_before_last_save
|
||||
@mutations_before_last_save ||= previous_mutation_tracker
|
||||
end
|
||||
|
||||
def cache_changed_attributes
|
||||
@cached_changed_attributes = changed_attributes
|
||||
yield
|
||||
|
|
|
|||
|
|
@ -45,6 +45,11 @@ module ActiveRecord
|
|||
attribute_was(self.class.primary_key)
|
||||
end
|
||||
|
||||
def id_in_database
|
||||
sync_with_transaction_state
|
||||
attribute_in_database(self.class.primary_key)
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
def attribute_method?(attr_name)
|
||||
|
|
@ -60,7 +65,7 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was).to_set
|
||||
ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set
|
||||
|
||||
def dangerous_attribute_method?(method_name)
|
||||
super && !ID_ATTRIBUTE_METHODS.include?(method_name)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,10 @@
|
|||
module ActiveRecord
|
||||
class AttributeMutationTracker # :nodoc:
|
||||
OPTION_NOT_GIVEN = Object.new
|
||||
|
||||
def initialize(attributes)
|
||||
@attributes = attributes
|
||||
@forced_changes = Set.new
|
||||
end
|
||||
|
||||
def changed_values
|
||||
|
|
@ -14,15 +17,29 @@ module ActiveRecord
|
|||
|
||||
def changes
|
||||
attr_names.each_with_object({}.with_indifferent_access) do |attr_name, result|
|
||||
if changed?(attr_name)
|
||||
result[attr_name] = [attributes[attr_name].original_value, attributes.fetch_value(attr_name)]
|
||||
change = change_to_attribute(attr_name)
|
||||
if change
|
||||
result[attr_name] = change
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def changed?(attr_name)
|
||||
def change_to_attribute(attr_name)
|
||||
if changed?(attr_name)
|
||||
[attributes[attr_name].original_value, attributes.fetch_value(attr_name)]
|
||||
end
|
||||
end
|
||||
|
||||
def any_changes?
|
||||
attr_names.any? { |attr| changed?(attr) }
|
||||
end
|
||||
|
||||
def changed?(attr_name, from: OPTION_NOT_GIVEN, to: OPTION_NOT_GIVEN)
|
||||
attr_name = attr_name.to_s
|
||||
attributes[attr_name].changed?
|
||||
forced_changes.include?(attr_name) ||
|
||||
attributes[attr_name].changed? &&
|
||||
(OPTION_NOT_GIVEN == from || attributes[attr_name].original_value == from) &&
|
||||
(OPTION_NOT_GIVEN == to || attributes[attr_name].value == to)
|
||||
end
|
||||
|
||||
def changed_in_place?(attr_name)
|
||||
|
|
@ -32,11 +49,20 @@ module ActiveRecord
|
|||
def forget_change(attr_name)
|
||||
attr_name = attr_name.to_s
|
||||
attributes[attr_name] = attributes[attr_name].forgetting_assignment
|
||||
forced_changes.delete(attr_name)
|
||||
end
|
||||
|
||||
def original_value(attr_name)
|
||||
attributes[attr_name].original_value
|
||||
end
|
||||
|
||||
def force_change(attr_name)
|
||||
forced_changes << attr_name.to_s
|
||||
end
|
||||
|
||||
protected
|
||||
|
||||
attr_reader :attributes
|
||||
attr_reader :attributes, :forced_changes
|
||||
|
||||
private
|
||||
|
||||
|
|
@ -48,14 +74,21 @@ module ActiveRecord
|
|||
class NullMutationTracker # :nodoc:
|
||||
include Singleton
|
||||
|
||||
def changed_values
|
||||
def changed_values(*)
|
||||
{}
|
||||
end
|
||||
|
||||
def changes
|
||||
def changes(*)
|
||||
{}
|
||||
end
|
||||
|
||||
def change_to_attribute(attr_name)
|
||||
end
|
||||
|
||||
def any_changes?(*)
|
||||
false
|
||||
end
|
||||
|
||||
def changed?(*)
|
||||
false
|
||||
end
|
||||
|
|
@ -66,5 +99,8 @@ module ActiveRecord
|
|||
|
||||
def forget_change(*)
|
||||
end
|
||||
|
||||
def original_value(*)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -267,7 +267,7 @@ module ActiveRecord
|
|||
# Returns whether or not this record has been changed in any way (including whether
|
||||
# any of its nested autosave associations are likewise changed)
|
||||
def changed_for_autosave?
|
||||
new_record? || changed? || marked_for_destruction? || nested_records_changed_for_autosave?
|
||||
new_record? || has_changes_to_save? || marked_for_destruction? || nested_records_changed_for_autosave?
|
||||
end
|
||||
|
||||
private
|
||||
|
|
@ -451,7 +451,7 @@ module ActiveRecord
|
|||
def record_changed?(reflection, record, key)
|
||||
record.new_record? ||
|
||||
(record.has_attribute?(reflection.foreign_key) && record[reflection.foreign_key] != key) ||
|
||||
record.attribute_changed?(reflection.foreign_key)
|
||||
record.will_save_change_to_attribute?(reflection.foreign_key)
|
||||
end
|
||||
|
||||
# Saves the associated record if it's new or <tt>:autosave</tt> is enabled.
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ require "active_support/core_ext/module/introspection"
|
|||
require "active_support/core_ext/object/duplicable"
|
||||
require "active_support/core_ext/class/subclasses"
|
||||
require "active_record/attribute_decorators"
|
||||
require "active_record/define_callbacks"
|
||||
require "active_record/errors"
|
||||
require "active_record/log_subscriber"
|
||||
require "active_record/explain_subscriber"
|
||||
|
|
@ -303,6 +304,7 @@ module ActiveRecord #:nodoc:
|
|||
include AttributeDecorators
|
||||
include Locking::Optimistic
|
||||
include Locking::Pessimistic
|
||||
include DefineCallbacks
|
||||
include AttributeMethods
|
||||
include Callbacks
|
||||
include Timestamp
|
||||
|
|
|
|||
|
|
@ -265,17 +265,6 @@ module ActiveRecord
|
|||
:before_destroy, :around_destroy, :after_destroy, :after_commit, :after_rollback
|
||||
]
|
||||
|
||||
module ClassMethods # :nodoc:
|
||||
include ActiveModel::Callbacks
|
||||
end
|
||||
|
||||
included do
|
||||
include ActiveModel::Validations::Callbacks
|
||||
|
||||
define_model_callbacks :initialize, :find, :touch, only: :after
|
||||
define_model_callbacks :save, :create, :update, :destroy
|
||||
end
|
||||
|
||||
def destroy #:nodoc:
|
||||
@_destroy_callback_already_called ||= false
|
||||
return if @_destroy_callback_already_called
|
||||
|
|
|
|||
20
activerecord/lib/active_record/define_callbacks.rb
Normal file
20
activerecord/lib/active_record/define_callbacks.rb
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
module ActiveRecord
|
||||
# This module exists because `ActiveRecord::AttributeMethods::Dirty` needs to
|
||||
# define callbacks, but continue to have its version of `save` be the super
|
||||
# method of `ActiveRecord::Callbacks`. This will be removed when the removal
|
||||
# of deprecated code removes this need.
|
||||
module DefineCallbacks
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
module ClassMethods # :nodoc:
|
||||
include ActiveModel::Callbacks
|
||||
end
|
||||
|
||||
included do
|
||||
include ActiveModel::Validations::Callbacks
|
||||
|
||||
define_model_callbacks :initialize, :find, :touch, :only => :after
|
||||
define_model_callbacks :save, :create, :update, :destroy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -253,7 +253,11 @@ module ActiveRecord
|
|||
verify_readonly_attribute(name)
|
||||
public_send("#{name}=", value)
|
||||
|
||||
changed? ? save(validate: false) : true
|
||||
if has_changes_to_save?
|
||||
save(validate: false)
|
||||
else
|
||||
true
|
||||
end
|
||||
end
|
||||
|
||||
# Updates the attributes of the model from the passed-in hash and saves the
|
||||
|
|
@ -336,7 +340,7 @@ module ActiveRecord
|
|||
# record could be saved.
|
||||
def increment!(attribute, by = 1)
|
||||
increment(attribute, by)
|
||||
change = public_send(attribute) - (attribute_was(attribute.to_s) || 0)
|
||||
change = public_send(attribute) - (attribute_in_database(attribute.to_s) || 0)
|
||||
self.class.update_counters(id, attribute => change)
|
||||
clear_attribute_change(attribute) # eww
|
||||
self
|
||||
|
|
@ -548,7 +552,7 @@ module ActiveRecord
|
|||
if attributes_values.empty?
|
||||
0
|
||||
else
|
||||
self.class.unscoped._update_record attributes_values, id, id_was
|
||||
self.class.unscoped._update_record attributes_values, id, id_in_database
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -74,7 +74,7 @@ module ActiveRecord
|
|||
|
||||
timestamp_attributes_for_update_in_model.each do |column|
|
||||
column = column.to_s
|
||||
next if attribute_changed?(column)
|
||||
next if will_save_change_to_attribute?(column)
|
||||
write_attribute(column, current_time)
|
||||
end
|
||||
end
|
||||
|
|
@ -82,7 +82,7 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def should_record_timestamps?
|
||||
record_timestamps && (!partial_writes? || changed?)
|
||||
record_timestamps && (!partial_writes? || has_changes_to_save?)
|
||||
end
|
||||
|
||||
def timestamp_attributes_for_create_in_model
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ module ActiveRecord
|
|||
# touch the parents as we are not calling the after_save callbacks
|
||||
self.class.reflect_on_all_associations(:belongs_to).each do |r|
|
||||
if touch = r.options[:touch]
|
||||
ActiveRecord::Associations::Builder::BelongsTo.touch_record(self, r.foreign_key, r.name, touch, :touch_later)
|
||||
ActiveRecord::Associations::Builder::BelongsTo.touch_record(self, changes_to_save, r.foreign_key, r.name, touch, :touch_later)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ module ActiveRecord
|
|||
relation = build_relation(finder_class, attribute, value)
|
||||
if record.persisted?
|
||||
if finder_class.primary_key
|
||||
relation = relation.where.not(finder_class.primary_key => record.id_was || record.id)
|
||||
relation = relation.where.not(finder_class.primary_key => record.id_in_database || record.id)
|
||||
else
|
||||
raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.")
|
||||
end
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ module ActiveRecord
|
|||
def self.decorate_matching_attribute_types(*); end
|
||||
def self.initialize_generated_modules; end
|
||||
|
||||
include ActiveRecord::DefineCallbacks
|
||||
include ActiveRecord::AttributeMethods
|
||||
|
||||
def self.attribute_names
|
||||
|
|
|
|||
|
|
@ -726,6 +726,89 @@ class DirtyTest < ActiveRecord::TestCase
|
|||
assert person.changed?
|
||||
end
|
||||
|
||||
test "saved_change_to_attribute? returns whether a change occurred in the last save" do
|
||||
person = Person.create!(first_name: "Sean")
|
||||
|
||||
assert person.saved_change_to_first_name?
|
||||
refute person.saved_change_to_gender?
|
||||
assert person.saved_change_to_first_name?(from: nil, to: "Sean")
|
||||
assert person.saved_change_to_first_name?(from: nil)
|
||||
assert person.saved_change_to_first_name?(to: "Sean")
|
||||
refute person.saved_change_to_first_name?(from: "Jim", to: "Sean")
|
||||
refute person.saved_change_to_first_name?(from: "Jim")
|
||||
refute person.saved_change_to_first_name?(to: "Jim")
|
||||
end
|
||||
|
||||
test "saved_change_to_attribute returns the change that occurred in the last save" do
|
||||
person = Person.create!(first_name: "Sean", gender: "M")
|
||||
|
||||
assert_equal [nil, "Sean"], person.saved_change_to_first_name
|
||||
assert_equal [nil, "M"], person.saved_change_to_gender
|
||||
|
||||
person.update(first_name: "Jim")
|
||||
|
||||
assert_equal ["Sean", "Jim"], person.saved_change_to_first_name
|
||||
assert_nil person.saved_change_to_gender
|
||||
end
|
||||
|
||||
test "attribute_before_last_save returns the original value before saving" do
|
||||
person = Person.create!(first_name: "Sean", gender: "M")
|
||||
|
||||
assert_nil person.first_name_before_last_save
|
||||
assert_nil person.gender_before_last_save
|
||||
|
||||
person.first_name = "Jim"
|
||||
|
||||
assert_nil person.first_name_before_last_save
|
||||
assert_nil person.gender_before_last_save
|
||||
|
||||
person.save
|
||||
|
||||
assert_equal "Sean", person.first_name_before_last_save
|
||||
assert_equal "M", person.gender_before_last_save
|
||||
end
|
||||
|
||||
test "saved_changes? returns whether the last call to save changed anything" do
|
||||
person = Person.create!(first_name: "Sean")
|
||||
|
||||
assert person.saved_changes?
|
||||
|
||||
person.save
|
||||
|
||||
refute person.saved_changes?
|
||||
end
|
||||
|
||||
test "saved_changes returns a hash of all the changes that occurred" do
|
||||
person = Person.create!(first_name: "Sean", gender: "M")
|
||||
|
||||
assert_equal [nil, "Sean"], person.saved_changes[:first_name]
|
||||
assert_equal [nil, "M"], person.saved_changes[:gender]
|
||||
assert_equal %w(id first_name gender created_at updated_at).sort, person.saved_changes.keys.sort
|
||||
|
||||
travel(1.second) do
|
||||
person.update(first_name: "Jim")
|
||||
end
|
||||
|
||||
assert_equal ["Sean", "Jim"], person.saved_changes[:first_name]
|
||||
assert_equal %w(first_name lock_version updated_at).sort, person.saved_changes.keys.sort
|
||||
end
|
||||
|
||||
test "changed? in after callbacks returns true but is deprecated" do
|
||||
klass = Class.new(ActiveRecord::Base) do
|
||||
self.table_name = "people"
|
||||
|
||||
after_save do
|
||||
ActiveSupport::Deprecation.silence do
|
||||
raise "changed? should be true" unless changed?
|
||||
end
|
||||
raise "has_changes_to_save? should be false" if has_changes_to_save?
|
||||
end
|
||||
end
|
||||
|
||||
person = klass.create!(first_name: "Sean")
|
||||
refute person.changed?
|
||||
end
|
||||
|
||||
private
|
||||
def with_partial_writes(klass, on = true)
|
||||
old = klass.partial_writes?
|
||||
|
|
|
|||
|
|
@ -22,12 +22,12 @@ class Eye < ActiveRecord::Base
|
|||
alias trace_after_create2 trace_after_create
|
||||
|
||||
def trace_after_update
|
||||
(@after_update_callbacks_stack ||= []) << iris.changed?
|
||||
(@after_update_callbacks_stack ||= []) << iris.has_changes_to_save?
|
||||
end
|
||||
alias trace_after_update2 trace_after_update
|
||||
|
||||
def trace_after_save
|
||||
(@after_save_callbacks_stack ||= []) << iris.changed?
|
||||
(@after_save_callbacks_stack ||= []) << iris.has_changes_to_save?
|
||||
end
|
||||
alias trace_after_save2 trace_after_save
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in a new issue