Merge pull request #1678 from bf4/serialization_interface

Restrict serializable_hash to accepted options

lib/active_model/serializer.rb

@@ -18,6 +18,8 @@ require 'active_model/serializer/type'
 # reified when subclassed to decorate a resource.
 module ActiveModel
   class Serializer
+    # @see #serializable_hash for more details on these valid keys.
+    SERIALIZABLE_HASH_VALID_KEYS = [:only, :except, :methods, :include, :root].freeze
     extend ActiveSupport::Autoload
     autoload :Adapter
     autoload :Null

lib/active_model_serializers/adapter/attributes.rb

@@ -8,7 +8,7 @@ module ActiveModelSerializers
       end
 
       def serializable_hash(options = nil)
-        options ||= {}
+        options = serialization_options(options)
 
         if serializer.respond_to?(:each)
           serializable_hash_for_collection(options)

lib/active_model_serializers/adapter/base.rb

@@ -19,6 +19,8 @@ module ActiveModelSerializers
         @cached_name ||= self.class.name.demodulize.underscore
       end
 
+      # Subclasses that implement this method must first call
+      #   options = serialization_options(options)
       def serializable_hash(_options = nil)
         fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
       end
@@ -41,6 +43,12 @@ module ActiveModelSerializers
 
       private
 
+      # see https://github.com/rails-api/active_model_serializers/pull/965
+      # When <tt>options</tt> is +nil+, sets it to +{}+
+      def serialization_options(options)
+        options ||= {} # rubocop:disable Lint/UselessAssignment
+      end
+
       def meta
         instance_options.fetch(:meta, nil)
       end

lib/active_model_serializers/adapter/json.rb

@@ -2,7 +2,7 @@ module ActiveModelSerializers
   module Adapter
     class Json < Base
       def serializable_hash(options = nil)
-        options ||= {}
+        options = serialization_options(options)
         serialized_hash = { root => Attributes.new(serializer, instance_options).serializable_hash(options) }
         self.class.transform_key_casing!(serialized_hash, instance_options)
       end

test/action_controller/json_api/transform_test.rb

@@ -60,10 +60,11 @@ module ActionController
           end
 
           def render_resource_with_transform_with_global_config
-            setup_post
             old_transform = ActiveModelSerializers.config.key_transform
+            setup_post
             ActiveModelSerializers.config.key_transform = :camel_lower
             render json: @post, serializer: PostSerializer, adapter: :json_api
+          ensure
             ActiveModelSerializers.config.key_transform = old_transform
           end
         end

test/adapter_test.rb

@@ -14,6 +14,33 @@ module ActiveModelSerializers
       end
     end
 
+    def test_serialization_options_ensures_option_is_a_hash
+      adapter = Class.new(ActiveModelSerializers::Adapter::Base) do
+        def serializable_hash(options = nil)
+          serialization_options(options)
+        end
+      end.new(@serializer)
+      assert_equal({}, adapter.serializable_hash(nil))
+      assert_equal({}, adapter.serializable_hash({}))
+    ensure
+      ActiveModelSerializers::Adapter.adapter_map.delete_if { |k, _| k =~ /class/ }
+    end
+
+    def test_serialization_options_ensures_option_is_one_of_valid_options
+      adapter = Class.new(ActiveModelSerializers::Adapter::Base) do
+        def serializable_hash(options = nil)
+          serialization_options(options)
+        end
+      end.new(@serializer)
+      filtered_options = { now: :see_me, then: :not }
+      valid_options = ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS.each_with_object({}) do |option, result|
+        result[option] = option
+      end
+      assert_equal(valid_options, adapter.serializable_hash(filtered_options.merge(valid_options)))
+    ensure
+      ActiveModelSerializers::Adapter.adapter_map.delete_if { |k, _| k =~ /class/ }
+    end
+
     def test_serializer
       assert_equal @serializer, @adapter.serializer
     end

test/test_helper.rb

@@ -15,6 +15,22 @@ require 'action_controller'
 require 'action_controller/test_case'
 require 'action_controller/railtie'
 require 'active_model_serializers'
+# For now, we only restrict the options to serializable_hash/as_json/to_json
+# in tests, to ensure developers don't add any unsupported options.
+# There's no known benefit, at this time, to having the filtering run in
+# production when the excluded options would simply not be used.
+#
+# However, for documentation purposes, the constant
+# ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS is defined
+# in the Serializer.
+ActiveModelSerializers::Adapter::Base.class_eval do
+  alias_method :original_serialization_options, :serialization_options
+
+  def serialization_options(options)
+    original_serialization_options(options)
+      .slice(*ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS)
+  end
+end
 require 'fileutils'
 FileUtils.mkdir_p(File.expand_path('../../tmp/cache', __FILE__))