Merge pull request #13799 from kbrock/better_dirty

Better ActiveRecord hierarchy for Dirty and others

7 files changed, 49 insertions, 18 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index 68168bb729..8a1b199997 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -38,7 +38,27 @@ module ActiveRecord
         end
       end
 
+    def initialize_dup(other) # :nodoc:
+      super
+      init_changed_attributes
+    end
+
     private
+      def initialize_internals_callback
+        super
+        init_changed_attributes
+      end
+
+      def init_changed_attributes
+        @changed_attributes = nil
+        # Intentionally avoid using #column_defaults since overridden defaults (as is done in
+        # optimistic locking) won't get written unless they get marked as changed
+        self.class.columns.each do |c|
+          attr, orig_value = c.name, c.default
+          changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr])
+        end
+      end
+
       # Wrap write_attribute to remember original attribute value.
       def write_attribute(attr, value)
         attr = attr.to_s
@@ -70,7 +90,7 @@ module ActiveRecord
       # Serialized attributes should always be written in case they've been
       # changed in place.
       def keys_for_partial_write
-        changed | (attributes.keys & self.class.serialized_attributes.keys)
+        changed
       end
 
       def _field_changed?(attr, old, value)
diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb
index d484659190..3227464032 100644
--- a/activerecord/lib/active_record/attribute_methods/serialization.rb
+++ b/activerecord/lib/active_record/attribute_methods/serialization.rb
@@ -115,6 +115,14 @@ module ActiveRecord
           end
         end
 
+        def should_record_timestamps?
+          super || (self.record_timestamps && (attributes.keys & self.class.serialized_attributes.keys).present?)
+        end
+
+        def keys_for_partial_write
+          super | (attributes.keys & self.class.serialized_attributes.keys)
+        end
+
         def type_cast_attribute_for_write(column, value)
           if column && coder = self.class.serialized_attributes[column.name]
             Attribute.new(coder, value, :unserialized)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 1d3ec75aa1..9ec1feea97 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -294,6 +294,7 @@ module ActiveRecord #:nodoc:
     extend Enum
     extend Delegation::DelegateCache
 
+    include Core
     include Persistence
     include NoTouching
     include ReadonlyAttributes
@@ -320,7 +321,6 @@ module ActiveRecord #:nodoc:
     include Reflection
     include Serialization
     include Store
-    include Core
   end
 
   ActiveSupport.run_load_hooks(:active_record, Base)
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index a4fe1efd33..6f02c763fe 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -182,9 +182,7 @@ module ActiveRecord
       @column_types = self.class.column_types
 
       init_internals
-      init_changed_attributes
-      ensure_proper_type
-      populate_with_current_scope_attributes
+      initialize_internals_callback
 
       # +options+ argument is only needed to make protected_attributes gem easier to hook.
       # Remove it when we drop support to this gem.
@@ -255,16 +253,12 @@ module ActiveRecord
 
       run_callbacks(:initialize) unless _initialize_callbacks.empty?
 
-      @changed_attributes = {}
-      init_changed_attributes
-
       @aggregation_cache = {}
       @association_cache = {}
       @attributes_cache  = {}
 
       @new_record  = true
 
-      ensure_proper_type
       super
     end
 
@@ -440,13 +434,7 @@ module ActiveRecord
       @reflects_state           = [false]
     end
 
-    def init_changed_attributes
-      # Intentionally avoid using #column_defaults since overridden defaults (as is done in
-      # optimistic locking) won't get written unless they get marked as changed
-      self.class.columns.each do |c|
-        attr, orig_value = c.name, c.default
-        changed_attributes[attr] = orig_value if _field_changed?(attr, orig_value, @attributes[attr])
-      end
+    def initialize_internals_callback
     end
 
     # This method is needed to make protected_attributes gem easier to hook.
diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb
index da73112e90..08fc91c9df 100644
--- a/activerecord/lib/active_record/inheritance.rb
+++ b/activerecord/lib/active_record/inheritance.rb
@@ -195,8 +195,18 @@ module ActiveRecord
       end
     end
 
+    def initialize_dup(other)
+      super
+      ensure_proper_type
+    end
+
     private
 
+    def initialize_internals_callback
+      super
+      ensure_proper_type
+    end
+
     # Sets the attribute used for single table inheritance to this class name if this is not the
     # ActiveRecord::Base descendant.
     # Considering the hierarchy Reply < Message < ActiveRecord::Base, this makes it possible to
diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb
index 0cf3d59985..3e43591672 100644
--- a/activerecord/lib/active_record/scoping.rb
+++ b/activerecord/lib/active_record/scoping.rb
@@ -27,6 +27,11 @@ module ActiveRecord
       end
     end
 
+    def initialize_internals_callback
+      super
+      populate_with_current_scope_attributes
+    end
+
     # This class stores the +:current_scope+ and +:ignore_default_scope+ values
     # for different classes. The registry is stored as a thread local, which is
     # accessed through +ScopeRegistry.current+.
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index e0541b7681..7178bed560 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -37,8 +37,8 @@ module ActiveRecord
     end
 
     def initialize_dup(other) # :nodoc:
-      clear_timestamp_attributes
       super
+      clear_timestamp_attributes
     end
 
   private
@@ -71,7 +71,7 @@ module ActiveRecord
     end
 
     def should_record_timestamps?
-      self.record_timestamps && (!partial_writes? || changed? || (attributes.keys & self.class.serialized_attributes.keys).present?)
+      self.record_timestamps && (!partial_writes? || changed?)
     end
 
     def timestamp_attributes_for_create_in_model