From 0f0ef2baf5494ed505a336407530982f1e07af2f Mon Sep 17 00:00:00 2001 From: Edward Loveall Date: Wed, 10 Jun 2015 18:20:34 -0400 Subject: [PATCH 1/3] Don't pass serializer option to associated serializers Fixes #870 Commit af81a40 introduced passing a serializer's 'options' along to its associated model serializers. Thus, an explicit 'each_serializer' passed to render for a singular resource would be passed on as the implicit 'serializer' for its associations. With @bf4 --- lib/active_model/serializer.rb | 2 +- .../explicit_serializer_test.rb | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 00eed5bc..0da1755b 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -217,7 +217,7 @@ module ActiveModel if serializer_class serializer = serializer_class.new( association_value, - options.merge(serializer_from_options(association_options)) + options.except(:serializer).merge(serializer_from_options(association_options)) ) elsif !association_value.nil? && !association_value.instance_of?(Object) association_options[:association_options][:virtual_value] = association_value diff --git a/test/action_controller/explicit_serializer_test.rb b/test/action_controller/explicit_serializer_test.rb index ff0a07e6..5fafafa5 100644 --- a/test/action_controller/explicit_serializer_test.rb +++ b/test/action_controller/explicit_serializer_test.rb @@ -55,6 +55,14 @@ module ActionController render json: [@post], each_serializer: PostPreviewSerializer end + + def render_using_explicit_each_serializer + @comment = Comment.new({ id: 1, body: 'ZOMG A COMMENT' }) + @author = Author.new(id: 1, name: 'Joao Moura.') + @post = Post.new({ id: 1, title: 'New Post', body: 'Body', comments: [@comment], author: @author }) + + render json: @post, each_serializer: PostSerializer + end end tests MyController @@ -105,6 +113,31 @@ module ActionController assert_equal expected.to_json, @response.body end + + def test_render_using_explicit_each_serializer + get :render_using_explicit_each_serializer + + expected = { + id: 1, + title: 'New Post', + body: 'Body', + comments: [ + { + id: 1, + body: 'ZOMG A COMMENT' } + ], + blog: { + id: 999, + name: 'Custom blog' + }, + author: { + id: 1, + name: 'Joao Moura.' + } + } + + assert_equal expected.to_json, @response.body + end end end end From a5554e0d9f425afc3dd7242aa6941830e9b7fd2d Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Fri, 12 Jun 2015 10:54:57 -0500 Subject: [PATCH 2/3] Use a different controller in different tests A number of test were defining and using the same controller MyController = Class.new(ActionController::Base) which was causing some state to leak across tests. --- test/action_controller/adapter_selector_test.rb | 4 ++-- test/action_controller/explicit_serializer_test.rb | 4 ++-- test/action_controller/json_api_linked_test.rb | 4 ++-- test/action_controller/rescue_from_test.rb | 4 ++-- test/action_controller/serialization_test.rb | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/action_controller/adapter_selector_test.rb b/test/action_controller/adapter_selector_test.rb index ce90daf8..88c0ce6b 100644 --- a/test/action_controller/adapter_selector_test.rb +++ b/test/action_controller/adapter_selector_test.rb @@ -3,7 +3,7 @@ require 'test_helper' module ActionController module Serialization class AdapterSelectorTest < ActionController::TestCase - class MyController < ActionController::Base + class AdapterSelectorTestController < ActionController::Base def render_using_default_adapter @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) render json: @profile @@ -20,7 +20,7 @@ module ActionController end end - tests MyController + tests AdapterSelectorTestController def test_render_using_default_adapter get :render_using_default_adapter diff --git a/test/action_controller/explicit_serializer_test.rb b/test/action_controller/explicit_serializer_test.rb index 5fafafa5..35138cf4 100644 --- a/test/action_controller/explicit_serializer_test.rb +++ b/test/action_controller/explicit_serializer_test.rb @@ -3,7 +3,7 @@ require 'test_helper' module ActionController module Serialization class ExplicitSerializerTest < ActionController::TestCase - class MyController < ActionController::Base + class ExplicitSerializerTestController < ActionController::Base def render_using_explicit_serializer @profile = Profile.new(name: 'Name 1', description: 'Description 1', @@ -65,7 +65,7 @@ module ActionController end end - tests MyController + tests ExplicitSerializerTestController def test_render_using_explicit_serializer get :render_using_explicit_serializer diff --git a/test/action_controller/json_api_linked_test.rb b/test/action_controller/json_api_linked_test.rb index 559b2dd9..dab35c56 100644 --- a/test/action_controller/json_api_linked_test.rb +++ b/test/action_controller/json_api_linked_test.rb @@ -3,7 +3,7 @@ require 'test_helper' module ActionController module Serialization class JsonApiLinkedTest < ActionController::TestCase - class MyController < ActionController::Base + class JsonApiLinkedTestController < ActionController::Base def setup_post ActionController::Base.cache_store.clear @role1 = Role.new(id: 1, name: 'admin') @@ -78,7 +78,7 @@ module ActionController end end - tests MyController + tests JsonApiLinkedTestController def test_render_resource_without_include get :render_resource_without_include diff --git a/test/action_controller/rescue_from_test.rb b/test/action_controller/rescue_from_test.rb index d52ea8e6..82689178 100644 --- a/test/action_controller/rescue_from_test.rb +++ b/test/action_controller/rescue_from_test.rb @@ -3,7 +3,7 @@ require 'test_helper' module ActionController module Serialization class RescueFromTest < ActionController::TestCase - class MyController < ActionController::Base + class RescueFromTestController < ActionController::Base rescue_from Exception, with: :handle_error def render_using_raise_error_serializer @@ -16,7 +16,7 @@ module ActionController end end - tests MyController + tests RescueFromTestController def test_rescue_from get :render_using_raise_error_serializer diff --git a/test/action_controller/serialization_test.rb b/test/action_controller/serialization_test.rb index 240ba93c..ff646a97 100644 --- a/test/action_controller/serialization_test.rb +++ b/test/action_controller/serialization_test.rb @@ -3,7 +3,7 @@ require 'test_helper' module ActionController module Serialization class ImplicitSerializerTest < ActionController::TestCase - class MyController < ActionController::Base + class ImplicitSerializationTestController < ActionController::Base def render_using_implicit_serializer @profile = Profile.new({ name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }) render json: @profile @@ -152,7 +152,7 @@ module ActionController end end - tests MyController + tests ImplicitSerializationTestController # We just have Null for now, this will change def test_render_using_implicit_serializer From 14439aada49f9444098521a0163ea24cc6e5ba7b Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Fri, 12 Jun 2015 11:12:26 -0500 Subject: [PATCH 3/3] Use model that doesn't fail with race condition For some reason, the post would sometimes be serialized as "{\"id\":\"1\", + \"type\":\"posts\", \"attributes\":{\"title\":\"New Post\",\"body\":\"Body\"}, \"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}], \"blog\":{\"id\":999,\"name\":\"Custom blog\"}, \"author\":{\"id\":1,\"name\":\"Joao Moura.\"}}" instead of: "{\"id\":1, - \"title\":\"New Post\",\"body\":\"Body\", \"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}], \"blog\":{\"id\":999,\"name\":\"Custom blog\"},\ "author\":{\"id\":1,\"name\":\"Joao Moura.\"}}" To reproduce prior to this PR: SEED=55284 rake 1) Failure: ActionController::Serialization::ExplicitSerializerTest#test_render_using_explicit_each_serializer [active_model_serializers/test/action_controller/explicit_serializer_test.rb:139]: --- expected +++ actual @@ -1 +1 @@ -"{\"id\":1,\"title\":\"New Post\",\"body\":\"Body\",\"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}],\"blog\":{\"id\":999,\"name\":\"Custom blog\"},\"author\":{\"id\":1,\"name\":\"Joao Moura.\"}}" +"{\"id\":\"1\",\"type\":\"posts\",\"attributes\":{\"title\":\"New Post\",\"body\":\"Body\"},\"comments\":[{\"id\":1,\"body\":\"ZOMG A COMMENT\"}],\"blog\":{\"id\":999,\"name\":\"Custom blog\"},\"author\":{\"id\":1,\"name\":\"Joao Moura.\"}}" 137 runs, 211 assertions, 1 failures, 0 errors, 0 skips rake aborted! Command failed with status (1): [ruby -I"lib:test" -r./test/test_helper.rb "/$HOME/.rvm/rubies/ruby-2.2.2/lib/ruby/2.2.0/rake/rake_test_loader.rb" "test/action_controller/adapter_selector_test.rb" "test/action_controller/explicit_serializer_test.rb" "test/action_controller/json_api_linked_test.rb" "test/action_controller/rescue_from_test.rb" "test/action_controller/serialization_scope_name_test.rb" "test/action_controller/serialization_test.rb" "test/adapter/fragment_cache_test.rb" "test/adapter/json/belongs_to_test.rb" "test/adapter/json/collection_test.rb" "test/adapter/json/has_many_test.rb" "test/adapter/json_api/belongs_to_test.rb" "test/adapter/json_api/collection_test.rb" "test/adapter/json_api/has_many_embed_ids_test.rb" "test/adapter/json_api/has_many_explicit_serializer_test.rb" "test/adapter/json_api/has_many_test.rb" "test/adapter/json_api/has_one_test.rb" "test/adapter/json_api/linked_test.rb" "test/adapter/json_test.rb" "test/adapter/null_test.rb" "test/adapter_test.rb" "test/array_serializer_test.rb" "test/serializers/adapter_for_test.rb" "test/serializers/associations_test.rb" "test/serializers/attribute_test.rb" "test/serializers/attributes_test.rb" "test/serializers/cache_test.rb" "test/serializers/configuration_test.rb" "test/serializers/fieldset_test.rb" "test/serializers/generators_test.rb" "test/serializers/meta_test.rb" "test/serializers/options_test.rb" "test/serializers/serializer_for_test.rb" "test/serializers/urls_test.rb" ] /$HOME/.rvm/gems/ruby-2.2.2/bin/ruby_executable_hooks:15:in `eval' /$HOME/.rvm/gems/ruby-2.2.2/bin/ruby_executable_hooks:15:in `
' Tasks: TOP => default => test (See full trace by running task with --trace) --- .../explicit_serializer_test.rb | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/test/action_controller/explicit_serializer_test.rb b/test/action_controller/explicit_serializer_test.rb index 35138cf4..d4cbdb4d 100644 --- a/test/action_controller/explicit_serializer_test.rb +++ b/test/action_controller/explicit_serializer_test.rb @@ -57,11 +57,10 @@ module ActionController end def render_using_explicit_each_serializer - @comment = Comment.new({ id: 1, body: 'ZOMG A COMMENT' }) - @author = Author.new(id: 1, name: 'Joao Moura.') - @post = Post.new({ id: 1, title: 'New Post', body: 'Body', comments: [@comment], author: @author }) + location = Location.new(id: 42, lat: '-23.550520', lng: '-46.633309') + place = Place.new(id: 1337, name: 'Amazing Place', locations: [location]) - render json: @post, each_serializer: PostSerializer + render json: place, each_serializer: PlaceSerializer end end @@ -118,25 +117,19 @@ module ActionController get :render_using_explicit_each_serializer expected = { - id: 1, - title: 'New Post', - body: 'Body', - comments: [ + id: 1337, + name: "Amazing Place", + locations: [ { - id: 1, - body: 'ZOMG A COMMENT' } - ], - blog: { - id: 999, - name: 'Custom blog' - }, - author: { - id: 1, - name: 'Joao Moura.' - } + id: 42, + lat: "-23.550520", + lng: "-46.633309", + place: "Nowhere" # is a virtual attribute on LocationSerializer + } + ] } - assert_equal expected.to_json, @response.body + assert_equal expected.to_json, response.body end end end