Only add one more custom key in the serialized hash

Now custom serialziers can register itself in the serialized hash using
the "_aj_serialized" key that constains the serializer name.

This way we can avoid poluting the hash with many reserved keys.

6 files changed, 59 insertions, 51 deletions

diff --git a/activejob/lib/active_job/serializers.rb b/activejob/lib/active_job/serializers.rb
index 41113c521c..12458ea572 100644
--- a/activejob/lib/active_job/serializers.rb
+++ b/activejob/lib/active_job/serializers.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "set"
+
 module ActiveJob
   # Raised when an exception is raised during job arguments deserialization.
   #
@@ -34,7 +36,7 @@ module ActiveJob
     autoload :StandardTypeSerializer
 
     mattr_accessor :_additional_serializers
-    self._additional_serializers = []
+    self._additional_serializers = Set.new
 
     class << self
       # Returns serialized representative of the passed object.
@@ -62,27 +64,32 @@ module ActiveJob
 
       # Adds a new serializer to a list of known serializers
       def add_serializers(*new_serializers)
-        check_duplicate_serializer_keys!(new_serializers)
-
-        self._additional_serializers = new_serializers + self._additional_serializers
+        self._additional_serializers += new_serializers
       end
 
       # Returns a list of reserved keys, which cannot be used as keys for a hash
       def reserved_serializers_keys
-        serializers.select { |s| s.respond_to?(:key) }.map(&:key)
+        RESERVED_KEYS
       end
-
-      private
-
-        def check_duplicate_serializer_keys!(serializers)
-          keys_to_add = serializers.select { |s| s.respond_to?(:key) }.map(&:key)
-
-          duplicate_keys = reserved_serializers_keys & keys_to_add
-
-          raise ArgumentError.new("Can't add serializers because of keys duplication: #{duplicate_keys}") if duplicate_keys.any?
-        end
     end
 
+    # :nodoc:
+    GLOBALID_KEY = "_aj_globalid".freeze
+    # :nodoc:
+    SYMBOL_KEYS_KEY = "_aj_symbol_keys".freeze
+    # :nodoc:
+    WITH_INDIFFERENT_ACCESS_KEY = "_aj_hash_with_indifferent_access".freeze
+    # :nodoc:
+    OBJECT_SERIALIZER_KEY = "_aj_serialized"
+
+    # :nodoc:
+    RESERVED_KEYS = [
+      GLOBALID_KEY, GLOBALID_KEY.to_sym,
+      SYMBOL_KEYS_KEY, SYMBOL_KEYS_KEY.to_sym,
+      WITH_INDIFFERENT_ACCESS_KEY, WITH_INDIFFERENT_ACCESS_KEY.to_sym,
+    ]
+    private_constant :RESERVED_KEYS
+
     add_serializers GlobalIDSerializer,
       StandardTypeSerializer,
       HashWithIndifferentAccessSerializer,
diff --git a/activejob/lib/active_job/serializers/global_id_serializer.rb b/activejob/lib/active_job/serializers/global_id_serializer.rb
index ec20cf04f7..84ed33ef99 100644
--- a/activejob/lib/active_job/serializers/global_id_serializer.rb
+++ b/activejob/lib/active_job/serializers/global_id_serializer.rb
@@ -4,21 +4,21 @@ module ActiveJob
   module Serializers
     # Provides methods to serialize and deserialize objects which mixes `GlobalID::Identification`,
     # including `ActiveRecord::Base` models
-    class GlobalIDSerializer < ObjectSerializer
+    class GlobalIDSerializer < BaseSerializer
       class << self
         def serialize(object)
-          { key => object.to_global_id.to_s }
+          { GLOBALID_KEY => object.to_global_id.to_s }
         rescue URI::GID::MissingModelIdError
           raise SerializationError, "Unable to serialize #{object.class} " \
             "without an id. (Maybe you forgot to call save?)"
         end
 
         def deserialize(hash)
-          GlobalID::Locator.locate(hash[key])
+          GlobalID::Locator.locate(hash[GLOBALID_KEY])
         end
 
-        def key
-          "_aj_globalid"
+        def deserialize?(argument)
+          argument.is_a?(Hash) && argument[GLOBALID_KEY]
         end
 
         private
diff --git a/activejob/lib/active_job/serializers/hash_serializer.rb b/activejob/lib/active_job/serializers/hash_serializer.rb
index ca39a81ae9..2bbb31946d 100644
--- a/activejob/lib/active_job/serializers/hash_serializer.rb
+++ b/activejob/lib/active_job/serializers/hash_serializer.rb
@@ -23,12 +23,12 @@ module ActiveJob
           transform_symbol_keys(result, symbol_keys)
         end
 
+        private
+
         def key
-          "_aj_symbol_keys"
+          SYMBOL_KEYS_KEY
         end
 
-        private
-
         def serialize_hash(hash)
           hash.each_with_object({}) do |(key, value), result|
             result[serialize_hash_key(key)] = Serializers.serialize(value)
diff --git a/activejob/lib/active_job/serializers/hash_with_indifferent_access_serializer.rb b/activejob/lib/active_job/serializers/hash_with_indifferent_access_serializer.rb
index b0fb29d58b..af3576dd57 100644
--- a/activejob/lib/active_job/serializers/hash_with_indifferent_access_serializer.rb
+++ b/activejob/lib/active_job/serializers/hash_with_indifferent_access_serializer.rb
@@ -12,22 +12,18 @@ module ActiveJob
           result
         end
 
-        def deserialize?(argument)
-          argument.is_a?(Hash) && argument[key]
-        end
-
         def deserialize(hash)
           result = hash.transform_values { |v| Serializers.deserialize(v) }
           result.delete(key)
           result.with_indifferent_access
         end
 
+        private
+
         def key
-          "_aj_hash_with_indifferent_access"
+          WITH_INDIFFERENT_ACCESS_KEY
         end
 
-        private
-
         def klass
           ActiveSupport::HashWithIndifferentAccess
         end
diff --git a/activejob/lib/active_job/serializers/object_serializer.rb b/activejob/lib/active_job/serializers/object_serializer.rb
index 075360b26e..d5ff8c91f1 100644
--- a/activejob/lib/active_job/serializers/object_serializer.rb
+++ b/activejob/lib/active_job/serializers/object_serializer.rb
@@ -4,22 +4,12 @@ module ActiveJob
   module Serializers
     class ObjectSerializer < BaseSerializer
       class << self
-        def serialize(object)
-          { key => object.class.name }
+        def serialize(hash)
+          { OBJECT_SERIALIZER_KEY => self.name }.merge!(hash)
         end
 
         def deserialize?(argument)
-          argument.respond_to?(:keys) && argument.keys == keys
-        end
-
-        def deserialize(hash)
-          hash[key].constantize
-        end
-
-        private
-
-        def keys
-          [key]
+          argument.is_a?(Hash) && argument[OBJECT_SERIALIZER_KEY] == self.name
         end
       end
     end
diff --git a/activejob/test/cases/serializers_test.rb b/activejob/test/cases/serializers_test.rb
index 3b526c932b..fb0e6ecae6 100644
--- a/activejob/test/cases/serializers_test.rb
+++ b/activejob/test/cases/serializers_test.rb
@@ -10,20 +10,20 @@ class SerializersTest < ActiveSupport::TestCase
     def initialize(value)
       @value = value
     end
+
+    def ==(other)
+      self.value == other.value
+    end
   end
 
   class DummySerializer < ActiveJob::Serializers::ObjectSerializer
     class << self
       def serialize(object)
-        { key => object.value }
+        super({ "value" => object.value })
       end
 
       def deserialize(hash)
-        DummyValueObject.new(hash[key])
-      end
-
-      def key
-        "_dummy_serializer"
+        DummyValueObject.new(hash["value"])
       end
 
       private
@@ -49,9 +49,24 @@ class SerializersTest < ActiveSupport::TestCase
     end
   end
 
+  test "will serialize objects with serializers registered" do
+    ActiveJob::Serializers.add_serializers DummySerializer
+
+    assert_equal(
+      { "_aj_serialized" => "SerializersTest::DummySerializer", "value" => 123 },
+      ActiveJob::Serializers.serialize(@value_object)
+    )
+  end
+
   test "won't deserialize unknown hash" do
     hash = { "_dummy_serializer" => 123, "_aj_symbol_keys" => [] }
-    assert ActiveJob::Serializers.deserialize(hash), hash.except("_aj_symbol_keys")
+    assert_equal({ "_dummy_serializer" => 123 }, ActiveJob::Serializers.deserialize(hash))
+  end
+
+  test "will deserialize know serialized objects" do
+    ActiveJob::Serializers.add_serializers DummySerializer
+    hash = { "_aj_serialized" => "SerializersTest::DummySerializer", "value" => 123 }
+    assert_equal DummyValueObject.new(123), ActiveJob::Serializers.deserialize(hash)
   end
 
   test "adds new serializer" do
@@ -61,7 +76,7 @@ class SerializersTest < ActiveSupport::TestCase
 
   test "can't add serializer with the same key twice" do
     ActiveJob::Serializers.add_serializers DummySerializer
-    assert_raises ArgumentError do
+    assert_no_difference(-> { ActiveJob::Serializers.serializers.size } ) do
       ActiveJob::Serializers.add_serializers DummySerializer
     end
   end