Allow to tag serialized attributes as comparable

Fix: rails#28416

A not rare issue when working with serialized attributes is that
the serialized representation of an object can change over time.
Either because you are migrating from one serializer to the other
(e.g. YAML to JSON or to msgpack), or because the serializer used
subtly changed it's output.

One example is `libyaml` that used to have some extra trailing
whitespaces, and recently fixed that: yaml/libyaml#186

When this sorts of thing happen, you end up with lots of records that
report being changed even though they aren't, which in the best case
leads to a lot more writes to the database and in the worst case lead to
nasty bugs.

So ideally rather than to compare serialized representations, we'd
compare the deserialized objects, however we can't assume the deserialized
type has a proper working `==` method that deeply compare objects.

So the best thing we can do is to only do this if the type is declared
as being comparable.

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,22 @@
+*   Serialized attributes can now be marked as comparable.
+
+    A not rare issue when working with serialized attributes is that the serialized representation of an object
+    can change over time. Either because you are migrating from one serializer to the other (e.g. YAML to JSON or to msgpack),
+    or because the serializer used subtly changed it's output.
+
+    One example is libyaml that used to have some extra trailing whitespaces, and recently fixed that.
+    When this sorts of thing happen, you end up with lots of records that report being changed even though
+    they aren't, which in the best case leads to a lot more writes to the database and in the worst case lead to nasty bugs.
+
+    The solution is to instead compare the deserialized representation of the object, however Active Record
+    can't assume the deserialized object has a working `==` method. Hence why this new functionality is opt-in.
+
+    ```ruby
+    serialized :config, type: Hash, coder: JSON, comparable: true
+    ```
+
+    *Jean Boussier*
+
 *   Fix MySQL default functions getting dropped when changing a column's nullability.
 
     *Bastian Bartmann*

activerecord/lib/active_record/attribute_methods/serialization.rb

@@ -51,6 +51,16 @@ module ClassMethods
         #     ActiveRecord::SerializationTypeMismatch error.
         #   * If the column is +NULL+ or starting from a new record, the default value
         #     will set to +type.new+
+        # * +comparable+ - Specify whether the deserialized object is safely comparable
+        #   for the purpose of detecting changes. Defaults to +false+
+        #   When set to +false+ the old and new values will be compared by their serialized
+        #   representation (e.g. JSON or YAML), which can sometimes cause two objects that are
+        #   semantically equal to be considered different.
+        #   For instance two hashes with the same keys and values but a different order have a
+        #   different serialized representation, but are semantically equal once deserialized.
+        #   If set to +true+ the comparison will be done on the deserialized object. This options
+        #   should only be enabled if the +type+ is known to have a proper +==+ method that deeply
+        #   compare the objects.
         # * +yaml+ - Optional. Yaml specific options. The allowed config is:
         #   * +:permitted_classes+ - +Array+ with the permitted classes.
         #   * +:unsafe_load+ - Unsafely load YAML blobs, allow YAML to load any class.
@@ -180,7 +190,7 @@ module ClassMethods
         #     serialize :preferences, coder: Rot13JSON
         #   end
         #
-        def serialize(attr_name, coder: nil, type: Object, yaml: {}, **options)
+        def serialize(attr_name, coder: nil, type: Object, comparable: false, yaml: {}, **options)
           coder ||= default_column_serializer
           unless coder
             raise ArgumentError, <<~MSG.squish
@@ -200,7 +210,7 @@ def serialize(attr_name, coder: nil, type: Object, yaml: {}, **options)
             end
 
             cast_type = cast_type.subtype if Type::Serialized === cast_type
-            Type::Serialized.new(cast_type, column_serializer)
+            Type::Serialized.new(cast_type, column_serializer, comparable: comparable)
           end
         end
 

activerecord/lib/active_record/type/serialized.rb

@@ -9,9 +9,10 @@ class Serialized < DelegateClass(ActiveModel::Type::Value) # :nodoc:
 
       attr_reader :subtype, :coder
 
-      def initialize(subtype, coder)
+      def initialize(subtype, coder, comparable: false)
         @subtype = subtype
         @coder = coder
+        @comparable = comparable
         super(subtype)
       end
 
@@ -34,9 +35,15 @@ def serialize(value)
 
       def changed_in_place?(raw_old_value, value)
         return false if value.nil?
-        raw_new_value = encoded(value)
-        raw_old_value.nil? != raw_new_value.nil? ||
-          subtype.changed_in_place?(raw_old_value, raw_new_value)
+
+        if @comparable
+          old_value = deserialize(raw_old_value)
+          old_value != value
+        else
+          raw_new_value = encoded(value)
+          raw_old_value.nil? != raw_new_value.nil? ||
+            subtype.changed_in_place?(raw_old_value, raw_new_value)
+        end
       end
 
       def accessor

activerecord/test/cases/serialized_attribute_test.rb

@@ -697,4 +697,29 @@ def test_supports_permitted_classes_for_default_column_serializer
     topic = Topic.new(content: Time.now)
     assert topic.save
   end
+
+  def test_changed_in_place_compare_serialized_representation
+    Topic.serialize :content, type: Hash
+    topic = Topic.create!(content: { "a" => 1, "b" => 2 })
+
+    topic.content = { "a" => 1, "b" => 2 }
+    assert_not_predicate topic, :content_changed?
+
+    topic.content = { "b" => 2, "a" => 1 }
+    assert_predicate topic, :content_changed?
+  end
+
+  def test_changed_in_place_compare_deserialized_representation_when_comparable_is_set
+    Topic.serialize :content, type: Hash, comparable: true
+    topic = Topic.create!(content: { "a" => 1, "b" => 2 })
+
+    topic.content = { "a" => 1, "b" => 2 }
+    assert_not_predicate topic, :content_changed?
+
+    topic.content = { "b" => 2, "a" => 1 }
+    assert_not_predicate topic, :content_changed?
+
+    topic.content = {}
+    assert_predicate topic, :content_changed?
+  end
 end