Refactor determination of whether the field has changed

The types know more about what is going on than the dirty module. Let's
ask them!

6 files changed, 39 insertions, 36 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index ad01b5bf25..6cd4e43ddd 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -94,33 +94,8 @@ module ActiveRecord
       end
 
       def _field_changed?(attr, old, value)
-        if column = column_for_attribute(attr)
-          if column.number? && (changes_from_nil_to_empty_string?(column, old, value) ||
-                                changes_from_zero_to_string?(old, value))
-            value = nil
-          else
-            value = column.type_cast(value)
-          end
-        end
-
-        old != value
-      end
-
-      def changes_from_nil_to_empty_string?(column, old, value)
-        # For nullable numeric columns, NULL gets stored in database for blank (i.e. '') values.
-        # Hence we don't record it as a change if the value changes from nil to ''.
-        # If an old value of 0 is set to '' we want this to get changed to nil as otherwise it'll
-        # be typecast back to 0 (''.to_i => 0)
-        column.null && (old.nil? || old == 0) && value.blank?
-      end
-
-      def changes_from_zero_to_string?(old, value)
-        # For columns with old 0 and value non-empty string
-        old == 0 && value.is_a?(String) && value.present? && non_zero?(value)
-      end
-
-      def non_zero?(value)
-        value !~ /\A0+(\.0+)?\z/
+        column = column_for_attribute(attr) || Type::Value.new
+        column.changed?(old, value)
       end
     end
   end
diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb
index 425c33f2c6..148fc9eae5 100644
--- a/activerecord/lib/active_record/attribute_methods/serialization.rb
+++ b/activerecord/lib/active_record/attribute_methods/serialization.rb
@@ -83,14 +83,6 @@ module ActiveRecord
         def keys_for_partial_write
           super | (attributes.keys & self.class.serialized_attributes.keys)
         end
-
-        def _field_changed?(attr, old, value)
-          if self.class.serialized_attributes.include?(attr)
-            old != value
-          else
-            super
-          end
-        end
       end
     end
   end
diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index 60da541e3d..34054589d0 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -16,7 +16,7 @@ module ActiveRecord
       attr_reader :name, :default, :cast_type, :null, :sql_type, :default_function
 
       delegate :type, :precision, :scale, :limit, :klass, :accessor,
-        :text?, :number?, :binary?, :serialized?,
+        :text?, :number?, :binary?, :serialized?, :changed?,
         :type_cast, :type_cast_for_write, :type_cast_for_database,
         :type_cast_for_schema,
         to: :cast_type
diff --git a/activerecord/lib/active_record/type/numeric.rb b/activerecord/lib/active_record/type/numeric.rb
index 464d631d80..9cc6411e77 100644
--- a/activerecord/lib/active_record/type/numeric.rb
+++ b/activerecord/lib/active_record/type/numeric.rb
@@ -13,6 +13,29 @@ module ActiveRecord
         else super
         end
       end
+
+      def changed?(old_value, new_value) # :nodoc:
+        # 0 => 'wibble' should mark as changed so numericality validations run
+        if nil_or_zero?(old_value) && non_numeric_string?(new_value)
+          # nil => '' should not mark as changed
+          old_value != new_value.presence
+        else
+          super
+        end
+      end
+
+      private
+
+      def non_numeric_string?(value)
+        # 'wibble'.to_i will give zero, we want to make sure
+        # that we aren't marking int zero to string zero as
+        # changed.
+        value !~ /\A\d+\.?\d*\z/
+      end
+
+      def nil_or_zero?(value)
+        value.nil? || value == 0
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/type/serialized.rb b/activerecord/lib/active_record/type/serialized.rb
index eac31f6cc3..78a6d31e26 100644
--- a/activerecord/lib/active_record/type/serialized.rb
+++ b/activerecord/lib/active_record/type/serialized.rb
@@ -36,6 +36,10 @@ module ActiveRecord
 
       private
 
+      def changed?(old_value, new_value) # :nodoc:
+        old_value != new_value
+      end
+
       def is_default_value?(value)
         value == coder.load(nil)
       end
diff --git a/activerecord/lib/active_record/type/value.rb b/activerecord/lib/active_record/type/value.rb
index 1f7d4e20b2..c072c1e2b6 100644
--- a/activerecord/lib/active_record/type/value.rb
+++ b/activerecord/lib/active_record/type/value.rb
@@ -55,6 +55,15 @@ module ActiveRecord
         value
       end
 
+      # +old_value+ will always be type-cast.
+      # +new_value+ will come straight from the database
+      # or from assignment, so it could be anything. Types
+      # which cannot typecast arbitrary values should override
+      # this method.
+      def changed?(old_value, new_value) # :nodoc:
+        old_value != type_cast(new_value)
+      end
+
       private
 
       # Responsible for casting values from external sources to the appropriate