From 4d89ec84bc462a4d84b91f5158b7b488da66277e Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 10 Apr 2016 21:29:41 -0500 Subject: [PATCH] Restrict tests/impl from passing AMS options into serializable_hash --- CHANGELOG.md | 2 + lib/active_model_serializers/adapter/json.rb | 2 +- .../adapter/json_api.rb | 6 +-- .../adapter/json_api/pagination_links.rb | 13 +++++-- lib/active_model_serializers/adapter/null.rb | 3 +- .../serializable_resource.rb | 2 +- test/adapter/json/transform_test.rb | 28 +++++++------- .../adapter/json_api/pagination_links_test.rb | 37 ++++++++----------- 8 files changed, 47 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e8225c5f..34eb2298a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ Breaking changes: - [#1574](https://github.com/rails-api/active_model_serializers/pull/1574) Default key case for the JsonApi adapter changed to dashed. (@remear) Features: +- [#1647](https://github.com/rails-api/active_model_serializers/pull/1647) Restrict usage of `serializable_hash` options + to the ActiveModel::Serialization and ActiveModel::Serializers::JSON interface. (@bf4) - [#1645](https://github.com/rails-api/active_model_serializers/pull/1645) Transform keys referenced in values. (@remear) - [#1650](https://github.com/rails-api/active_model_serializers/pull/1650) Fix serialization scope options `scope`, `scope_name` take precedence over `serialization_scope` in the controller. diff --git a/lib/active_model_serializers/adapter/json.rb b/lib/active_model_serializers/adapter/json.rb index 5d182a515..aa2d8bb0e 100644 --- a/lib/active_model_serializers/adapter/json.rb +++ b/lib/active_model_serializers/adapter/json.rb @@ -4,7 +4,7 @@ class Json < Base def serializable_hash(options = nil) options ||= {} serialized_hash = { root => Attributes.new(serializer, instance_options).serializable_hash(options) } - self.class.transform_key_casing!(serialized_hash, options) + self.class.transform_key_casing!(serialized_hash, instance_options) end end end diff --git a/lib/active_model_serializers/adapter/json_api.rb b/lib/active_model_serializers/adapter/json_api.rb index c324798ca..1ba731632 100644 --- a/lib/active_model_serializers/adapter/json_api.rb +++ b/lib/active_model_serializers/adapter/json_api.rb @@ -128,7 +128,7 @@ def success_document(options) if is_collection && serializer.paginated? hash[:links] ||= {} - hash[:links].update(pagination_links_for(serializer, options)) + hash[:links].update(pagination_links_for(serializer)) end hash @@ -499,8 +499,8 @@ def links_for(serializer) # end # prs: # https://github.com/rails-api/active_model_serializers/pull/1041 - def pagination_links_for(serializer, options) - PaginationLinks.new(serializer.object, options[:serialization_context]).serializable_hash(options) + def pagination_links_for(serializer) + PaginationLinks.new(serializer.object, instance_options).as_json end # {http://jsonapi.org/format/#document-meta Docment Meta} diff --git a/lib/active_model_serializers/adapter/json_api/pagination_links.rb b/lib/active_model_serializers/adapter/json_api/pagination_links.rb index cb3ed6ba0..58c6f1df1 100644 --- a/lib/active_model_serializers/adapter/json_api/pagination_links.rb +++ b/lib/active_model_serializers/adapter/json_api/pagination_links.rb @@ -6,20 +6,25 @@ class PaginationLinks attr_reader :collection, :context - def initialize(collection, context) + def initialize(collection, adapter_options) @collection = collection - @context = context + @adapter_options = adapter_options + @context = adapter_options.fetch(:serialization_context) end - def serializable_hash(options = {}) + def as_json per_page = collection.try(:per_page) || collection.try(:limit_value) || collection.size pages_from.each_with_object({}) do |(key, value), hash| params = query_parameters.merge(page: { number: value, size: per_page }).to_query - hash[key] = "#{url(options)}?#{params}" + hash[key] = "#{url(adapter_options)}?#{params}" end end + protected + + attr_reader :adapter_options + private def pages_from diff --git a/lib/active_model_serializers/adapter/null.rb b/lib/active_model_serializers/adapter/null.rb index 6e690b1b0..9e5faf5cb 100644 --- a/lib/active_model_serializers/adapter/null.rb +++ b/lib/active_model_serializers/adapter/null.rb @@ -1,8 +1,7 @@ module ActiveModelSerializers module Adapter class Null < Base - # Since options param is not being used, underscored naming of the param - def serializable_hash(_options = nil) + def serializable_hash(*) {} end end diff --git a/lib/active_model_serializers/serializable_resource.rb b/lib/active_model_serializers/serializable_resource.rb index 6040c79d9..c226953c1 100644 --- a/lib/active_model_serializers/serializable_resource.rb +++ b/lib/active_model_serializers/serializable_resource.rb @@ -2,7 +2,7 @@ module ActiveModelSerializers class SerializableResource - ADAPTER_OPTION_KEYS = Set.new([:include, :fields, :adapter, :meta, :meta_key, :links]) + ADAPTER_OPTION_KEYS = Set.new([:include, :fields, :adapter, :meta, :meta_key, :links, :serialization_context]) include ActiveModelSerializers::Logging delegate :serializable_hash, :as_json, :to_json, to: :adapter diff --git a/test/adapter/json/transform_test.rb b/test/adapter/json/transform_test.rb index 30e058a50..4a18746de 100644 --- a/test/adapter/json/transform_test.rb +++ b/test/adapter/json/transform_test.rb @@ -8,9 +8,11 @@ def mock_request(key_transform = nil) context = Minitest::Mock.new context.expect(:request_url, URI) context.expect(:query_parameters, {}) - @options = {} - @options[:key_transform] = key_transform if key_transform - @options[:serialization_context] = context + options = {} + options[:key_transform] = key_transform if key_transform + options[:serialization_context] = context + serializer = CustomBlogSerializer.new(@blog) + @adapter = ActiveModelSerializers::Adapter::Json.new(serializer, options) end Post = Class.new(::Model) @@ -18,24 +20,22 @@ class PostSerializer < ActiveModel::Serializer attributes :id, :title, :body, :publish_at end - def setup + setup do ActionController::Base.cache_store.clear @blog = Blog.new(id: 1, name: 'My Blog!!', special_attribute: 'neat') - serializer = CustomBlogSerializer.new(@blog) - @adapter = ActiveModelSerializers::Adapter::Json.new(serializer) end def test_transform_default mock_request assert_equal({ blog: { id: 1, special_attribute: 'neat', articles: nil } - }, @adapter.serializable_hash(@options)) + }, @adapter.serializable_hash) end def test_transform_global_config mock_request result = with_config(key_transform: :camel_lower) do - @adapter.serializable_hash(@options) + @adapter.serializable_hash end assert_equal({ blog: { id: 1, specialAttribute: 'neat', articles: nil } @@ -45,7 +45,7 @@ def test_transform_global_config def test_transform_serialization_ctx_overrides_global_config mock_request(:camel) result = with_config(key_transform: :camel_lower) do - @adapter.serializable_hash(@options) + @adapter.serializable_hash end assert_equal({ Blog: { Id: 1, SpecialAttribute: 'neat', Articles: nil } @@ -56,7 +56,7 @@ def test_transform_undefined mock_request(:blam) result = nil assert_raises NoMethodError do - result = @adapter.serializable_hash(@options) + result = @adapter.serializable_hash end end @@ -64,28 +64,28 @@ def test_transform_dash mock_request(:dash) assert_equal({ blog: { id: 1, :"special-attribute" => 'neat', articles: nil } - }, @adapter.serializable_hash(@options)) + }, @adapter.serializable_hash) end def test_transform_unaltered mock_request(:unaltered) assert_equal({ blog: { id: 1, special_attribute: 'neat', articles: nil } - }, @adapter.serializable_hash(@options)) + }, @adapter.serializable_hash) end def test_transform_camel mock_request(:camel) assert_equal({ Blog: { Id: 1, SpecialAttribute: 'neat', Articles: nil } - }, @adapter.serializable_hash(@options)) + }, @adapter.serializable_hash) end def test_transform_camel_lower mock_request(:camel_lower) assert_equal({ blog: { id: 1, specialAttribute: 'neat', articles: nil } - }, @adapter.serializable_hash(@options)) + }, @adapter.serializable_hash) end end end diff --git a/test/adapter/json_api/pagination_links_test.rb b/test/adapter/json_api/pagination_links_test.rb index 244f8108c..406d0a6e4 100644 --- a/test/adapter/json_api/pagination_links_test.rb +++ b/test/adapter/json_api/pagination_links_test.rb @@ -25,13 +25,13 @@ def mock_request(query_parameters = {}, original_url = URI) context = Minitest::Mock.new context.expect(:request_url, original_url) context.expect(:query_parameters, query_parameters) - @options = {} - @options[:serialization_context] = context + context.expect(:key_transform, nil) end - def load_adapter(paginated_collection, options = {}) - options = options.merge(adapter: :json_api) - ActiveModelSerializers::SerializableResource.new(paginated_collection, options) + def load_adapter(paginated_collection, mock_request = nil) + render_options = { adapter: :json_api } + render_options[:serialization_context] = mock_request if mock_request + serializable(paginated_collection, render_options) end def using_kaminari(page = 2) @@ -102,43 +102,38 @@ def expected_response_with_last_page_pagination_links end def test_pagination_links_using_kaminari - adapter = load_adapter(using_kaminari) + adapter = load_adapter(using_kaminari, mock_request) - mock_request - assert_equal expected_response_with_pagination_links, adapter.serializable_hash(@options) + assert_equal expected_response_with_pagination_links, adapter.serializable_hash end def test_pagination_links_using_will_paginate - adapter = load_adapter(using_will_paginate) + adapter = load_adapter(using_will_paginate, mock_request) - mock_request - assert_equal expected_response_with_pagination_links, adapter.serializable_hash(@options) + assert_equal expected_response_with_pagination_links, adapter.serializable_hash end def test_pagination_links_with_additional_params - adapter = load_adapter(using_will_paginate) + adapter = load_adapter(using_will_paginate, mock_request({ test: 'test' })) - mock_request({ test: 'test' }) assert_equal expected_response_with_pagination_links_and_additional_params, - adapter.serializable_hash(@options) + adapter.serializable_hash end def test_last_page_pagination_links_using_kaminari - adapter = load_adapter(using_kaminari(3)) + adapter = load_adapter(using_kaminari(3), mock_request) - mock_request - assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash(@options) + assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash end def test_last_page_pagination_links_using_will_paginate - adapter = load_adapter(using_will_paginate(3)) + adapter = load_adapter(using_will_paginate(3), mock_request) - mock_request - assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash(@options) + assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash end def test_not_showing_pagination_links - adapter = load_adapter(@array) + adapter = load_adapter(@array, mock_request) assert_equal expected_response_without_pagination_links, adapter.serializable_hash end