From 2dd0c334616e208116f8dd5d047d9d23c8b1bc83 Mon Sep 17 00:00:00 2001 From: Roman Kapitonov Date: Wed, 24 Feb 2016 21:03:36 +0300 Subject: [PATCH 1/2] [FIX] Fetch json key from item serializer if empty collection is passed to collection serializer and each_searializer is specified. --- .../serializer/collection_serializer.rb | 21 +++++++++++++++---- test/collection_serializer_test.rb | 6 ++++++ test/fixtures/poro.rb | 6 ++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/active_model/serializer/collection_serializer.rb b/lib/active_model/serializer/collection_serializer.rb index 7862e994..ebaded3d 100644 --- a/lib/active_model/serializer/collection_serializer.rb +++ b/lib/active_model/serializer/collection_serializer.rb @@ -10,8 +10,14 @@ module ActiveModel def initialize(resources, options = {}) @root = options[:root] @object = resources + + serializer_context_class = options.fetch(:serializer_context_class, ActiveModel::Serializer) + + if resources.blank? && options[:serializer] + @each_serializer = options[:serializer] + end + @serializers = resources.map do |resource| - serializer_context_class = options.fetch(:serializer_context_class, ActiveModel::Serializer) serializer_class = options.fetch(:serializer) { serializer_context_class.serializer_for(resource) } if serializer_class.nil? # rubocop:disable Style/GuardClause @@ -27,7 +33,7 @@ module ActiveModel end def json_key - root || derived_root + root || derived_root || guess_root || default_root end def paginated? @@ -43,8 +49,15 @@ module ActiveModel private def derived_root - key = serializers.first.try(:json_key) || object.try(:name).try(:underscore) - key.try(:pluralize) + serializers.first.try(:json_key).try(:pluralize) + end + + def default_root + object.try(:name).try(:underscore).try(:pluralize) + end + + def guess_root + @each_serializer.try(:allocate).try(:json_key).try(:pluralize) end end end diff --git a/test/collection_serializer_test.rb b/test/collection_serializer_test.rb index a7c0fa02..833867b0 100644 --- a/test/collection_serializer_test.rb +++ b/test/collection_serializer_test.rb @@ -84,6 +84,12 @@ module ActiveModel assert_nil serializer.json_key end + def test_json_key_with_empty_resources_with_serializer + resource = [] + serializer = collection_serializer.new(resource, serializer: MessagesSerializer) + assert_equal 'messages', serializer.json_key + end + def test_json_key_with_root expected = 'custom_root' serializer = collection_serializer.new(@resource, root: expected) diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index c7fb831c..7fc59ffa 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -183,6 +183,12 @@ PaginatedSerializer = Class.new(ActiveModel::Serializer::CollectionSerializer) d end end +MessagesSerializer = Class.new(ActiveModel::Serializer) do + def json_key + 'messages' + end +end + AlternateBlogSerializer = Class.new(ActiveModel::Serializer) do attribute :id attribute :name, key: :title From a74d17442022135bd7a973e58511bbe1afde3c5c Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 24 Mar 2016 23:56:53 -0500 Subject: [PATCH 2/2] Include Serializer._type in collection serializer json_key cascade --- CHANGELOG.md | 2 + lib/active_model/serializer.rb | 2 +- .../serializer/collection_serializer.rb | 48 +++++++++---------- test/collection_serializer_test.rb | 4 ++ test/fixtures/poro.rb | 6 --- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06534b3d..304707bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ Breaking changes: Features: +- [#1618](https://github.com/rails-api/active_model_serializers/issues/1618) Get collection root key for + empty collection from explicit serializer option, when possible. (@bf4) - [#1574](https://github.com/rails-api/active_model_serializers/pull/1574) Provide key translation. (@remear) - [#1494](https://github.com/rails-api/active_model_serializers/pull/1494) Make serializers serializalbe (using the Attributes adapter by default). (@bf4) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index d78b873f..79478abb 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -182,7 +182,7 @@ module ActiveModel # Used by adapter as resource root. def json_key - root || object.class.model_name.to_s.underscore + root || _type || object.class.model_name.to_s.underscore end def read_attribute_for_serialization(attr) diff --git a/lib/active_model/serializer/collection_serializer.rb b/lib/active_model/serializer/collection_serializer.rb index ebaded3d..f026ebfe 100644 --- a/lib/active_model/serializer/collection_serializer.rb +++ b/lib/active_model/serializer/collection_serializer.rb @@ -8,15 +8,10 @@ module ActiveModel attr_reader :object, :root def initialize(resources, options = {}) - @root = options[:root] - @object = resources - + @object = resources + @options = options + @root = options[:root] serializer_context_class = options.fetch(:serializer_context_class, ActiveModel::Serializer) - - if resources.blank? && options[:serializer] - @each_serializer = options[:serializer] - end - @serializers = resources.map do |resource| serializer_class = options.fetch(:serializer) { serializer_context_class.serializer_for(resource) } @@ -32,9 +27,28 @@ module ActiveModel true end + # TODO: unify naming of root, json_key, and _type. Right now, a serializer's + # json_key comes from the root option or the object's model name, by default. + # But, if a dev defines a custom `json_key` method with an explicit value, + # we have no simple way to know that it is safe to call that instance method. + # (which is really a class property at this point, anyhow). + # rubocop:disable Metrics/CyclomaticComplexity + # Disabling cop since it's good to highlight the complexity of this method by + # including all the logic right here. def json_key - root || derived_root || guess_root || default_root + return root if root + # 1. get from options[:serializer] for empty resource collection + key = object.empty? && + (explicit_serializer_class = options[:serializer]) && + explicit_serializer_class._type + # 2. get from first serializer instance in collection + key ||= (serializer = serializers.first) && serializer.json_key + # 3. get from collection name, if a named collection + key ||= object.respond_to?(:name) ? object.name && object.name.underscore : nil + # 4. key may be nil for empty collection and no serializer option + key && key.pluralize end + # rubocop:enable Metrics/CyclomaticComplexity def paginated? object.respond_to?(:current_page) && @@ -44,21 +58,7 @@ module ActiveModel protected - attr_reader :serializers - - private - - def derived_root - serializers.first.try(:json_key).try(:pluralize) - end - - def default_root - object.try(:name).try(:underscore).try(:pluralize) - end - - def guess_root - @each_serializer.try(:allocate).try(:json_key).try(:pluralize) - end + attr_reader :serializers, :options end end end diff --git a/test/collection_serializer_test.rb b/test/collection_serializer_test.rb index 833867b0..5e5267e4 100644 --- a/test/collection_serializer_test.rb +++ b/test/collection_serializer_test.rb @@ -3,6 +3,10 @@ require 'test_helper' module ActiveModel class Serializer class CollectionSerializerTest < ActiveSupport::TestCase + MessagesSerializer = Class.new(ActiveModel::Serializer) do + type 'messages' + end + def setup @comment = Comment.new @post = Post.new diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 7fc59ffa..c7fb831c 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -183,12 +183,6 @@ PaginatedSerializer = Class.new(ActiveModel::Serializer::CollectionSerializer) d end end -MessagesSerializer = Class.new(ActiveModel::Serializer) do - def json_key - 'messages' - end -end - AlternateBlogSerializer = Class.new(ActiveModel::Serializer) do attribute :id attribute :name, key: :title