diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f3cf62b8..c9a58846 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -71,7 +71,7 @@ Style/BracesAroundHashParameters: - 'test/adapter/json_api/pagination_links_test.rb' - 'test/adapter/null_test.rb' - 'test/adapter_test.rb' - - 'test/array_serializer_test.rb' + - 'test/collection_serializer_test.rb' - 'test/serializable_resource_test.rb' - 'test/serializers/associations_test.rb' - 'test/serializers/attribute_test.rb' diff --git a/CHANGELOG.md b/CHANGELOG.md index ea0d772e..0f7b24bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ Features: - [#1127](https://github.com/rails-api/active_model_serializers/pull/1127) Add support for nested associations for JSON and Attributes adapters via the `include` option (@NullVoxPopuli, @beauby). - [#1050](https://github.com/rails-api/active_model_serializers/pull/1050) Add support for toplevel jsonapi member (@beauby, @bf4) +- [#tbd](https://github.com/rails-api/active_model_serializers/pull/tbd) Rename ArraySerializer to + CollectionSerializer for clarity, add ActiveModelSerializers.config.collection_serializer (@bf4) Fixes: - [#1239](https://github.com/rails-api/active_model_serializers/pull/1239) Fix duplicates in JSON API compound documents (@beauby) diff --git a/README.md b/README.md index c4e9c6e9..ec5c90e1 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ If you wish to use a serializer other than the default, you can explicitly pass #### 2. For an array resource: ```ruby -# Use the default `ArraySerializer`, which will use `each_serializer` to +# Use the default `CollectionSerializer`, which will use `each_serializer` to # serialize each element render json: @posts, each_serializer: PostPreviewSerializer diff --git a/docs/howto/add_pagination_links.md b/docs/howto/add_pagination_links.md index 11ce9b9c..0e784ec9 100644 --- a/docs/howto/add_pagination_links.md +++ b/docs/howto/add_pagination_links.md @@ -75,7 +75,7 @@ render json: @posts, serializer: PaginatedSerializer, each_serializer: PostPrevi And then, you could do something like the following class. ```ruby -class PaginatedSerializer < ActiveModel::Serializer::ArraySerializer +class PaginatedSerializer < ActiveModel::Serializer::CollectionSerializer def initialize(object, options={}) meta_key = options[:meta_key] || :meta options[meta_key] ||= {} diff --git a/lib/action_controller/serialization.rb b/lib/action_controller/serialization.rb index 42ad220c..1158e975 100644 --- a/lib/action_controller/serialization.rb +++ b/lib/action_controller/serialization.rb @@ -31,7 +31,7 @@ module ActionController serializable_resource.serialization_scope_name = _serialization_scope begin serializable_resource.adapter - rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError + rescue ActiveModel::Serializer::CollectionSerializer::NoSerializerError resource end else diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 27068b62..fd357630 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -1,4 +1,5 @@ require 'thread_safe' +require 'active_model/serializer/collection_serializer' require 'active_model/serializer/array_serializer' require 'active_model/serializer/include_tree' require 'active_model/serializer/associations' @@ -105,7 +106,7 @@ module ActiveModel if resource.respond_to?(:serializer_class) resource.serializer_class elsif resource.respond_to?(:to_ary) - config.array_serializer + config.collection_serializer else options.fetch(:serializer) { get_serializer_for(resource.class) } end diff --git a/lib/active_model/serializer/array_serializer.rb b/lib/active_model/serializer/array_serializer.rb index f9c91ea9..dc95d941 100644 --- a/lib/active_model/serializer/array_serializer.rb +++ b/lib/active_model/serializer/array_serializer.rb @@ -1,41 +1,9 @@ -module ActiveModel - class Serializer - class ArraySerializer - NoSerializerError = Class.new(StandardError) - include Enumerable - delegate :each, to: :@serializers - - attr_reader :object, :root - - def initialize(resources, options = {}) - @root = options[:root] - @object = resources - @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? - fail NoSerializerError, "No serializer found for resource: #{resource.inspect}" - else - serializer_class.new(resource, options.except(:serializer)) - end - end - end - - def json_key - key = root || serializers.first.try(:json_key) || object.try(:name).try(:underscore) - key.try(:pluralize) - end - - def paginated? - object.respond_to?(:current_page) && - object.respond_to?(:total_pages) && - object.respond_to?(:size) - end - - protected - - attr_reader :serializers +require 'active_model/serializer/collection_serializer' +class ActiveModel::Serializer + class ArraySerializer < CollectionSerializer + def initialize(*) + warn "Calling deprecated ArraySerializer in #{caller[0]}. Please use CollectionSerializer" + super end end end diff --git a/lib/active_model/serializer/collection_serializer.rb b/lib/active_model/serializer/collection_serializer.rb new file mode 100644 index 00000000..a3c9dc47 --- /dev/null +++ b/lib/active_model/serializer/collection_serializer.rb @@ -0,0 +1,41 @@ +module ActiveModel + class Serializer + class CollectionSerializer + NoSerializerError = Class.new(StandardError) + include Enumerable + delegate :each, to: :@serializers + + attr_reader :object, :root + + def initialize(resources, options = {}) + @root = options[:root] + @object = resources + @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? + fail NoSerializerError, "No serializer found for resource: #{resource.inspect}" + else + serializer_class.new(resource, options.except(:serializer)) + end + end + end + + def json_key + key = root || serializers.first.try(:json_key) || object.try(:name).try(:underscore) + key.try(:pluralize) + end + + def paginated? + object.respond_to?(:current_page) && + object.respond_to?(:total_pages) && + object.respond_to?(:size) + end + + protected + + attr_reader :serializers + end + end +end diff --git a/lib/active_model/serializer/configuration.rb b/lib/active_model/serializer/configuration.rb index 56427780..9e33633e 100644 --- a/lib/active_model/serializer/configuration.rb +++ b/lib/active_model/serializer/configuration.rb @@ -7,9 +7,19 @@ module ActiveModel # Configuration options may also be set in # Serializers and Adapters included do |base| - base.config.array_serializer = ActiveModel::Serializer::ArraySerializer - base.config.adapter = :attributes - base.config.jsonapi_resource_type = :plural + config = base.config + config.collection_serializer = ActiveModel::Serializer::CollectionSerializer + + def config.array_serializer=(collection_serializer) + self.collection_serializer = collection_serializer + end + + def config.array_serializer + collection_serializer + end + + config.adapter = :attributes + config.jsonapi_resource_type = :plural end end end diff --git a/lib/active_model/serializer/reflection.rb b/lib/active_model/serializer/reflection.rb index e2333303..18850abe 100644 --- a/lib/active_model/serializer/reflection.rb +++ b/lib/active_model/serializer/reflection.rb @@ -50,7 +50,7 @@ module ActiveModel association_value, serializer_options(subject, parent_serializer_options, reflection_options) ) - rescue ActiveModel::Serializer::ArraySerializer::NoSerializerError + rescue ActiveModel::Serializer::CollectionSerializer::NoSerializerError reflection_options[:virtual_value] = association_value.try(:as_json) || association_value end elsif !association_value.nil? && !association_value.instance_of?(Object) diff --git a/test/adapter/json/collection_test.rb b/test/adapter/json/collection_test.rb index 3b9e4b01..f60f262d 100644 --- a/test/adapter/json/collection_test.rb +++ b/test/adapter/json/collection_test.rb @@ -23,7 +23,7 @@ module ActiveModel def test_with_serializer_option @blog.special_attribute = 'Special' @blog.articles = [@first_post, @second_post] - serializer = ArraySerializer.new([@blog], serializer: CustomBlogSerializer) + serializer = CollectionSerializer.new([@blog], serializer: CustomBlogSerializer) adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) expected = { blogs: [{ @@ -35,7 +35,7 @@ module ActiveModel end def test_include_multiple_posts - serializer = ArraySerializer.new([@first_post, @second_post]) + serializer = CollectionSerializer.new([@first_post, @second_post]) adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) expected = { posts: [{ @@ -70,14 +70,14 @@ module ActiveModel def test_root_is_underscored virtual_value = VirtualValue.new(id: 1) - serializer = ArraySerializer.new([virtual_value]) + serializer = CollectionSerializer.new([virtual_value]) adapter = ActiveModel::Serializer::Adapter::Json.new(serializer) assert_equal 1, adapter.serializable_hash[:virtual_values].length end def test_include_option - serializer = ArraySerializer.new([@first_post, @second_post]) + serializer = CollectionSerializer.new([@first_post, @second_post]) adapter = ActiveModel::Serializer::Adapter::Json.new(serializer, include: '') actual = adapter.serializable_hash expected = { posts: [{ id: 1, title: 'Hello!!', body: 'Hello, world!!' }, diff --git a/test/adapter/json_api/collection_test.rb b/test/adapter/json_api/collection_test.rb index d1e70cf8..d5946ee5 100644 --- a/test/adapter/json_api/collection_test.rb +++ b/test/adapter/json_api/collection_test.rb @@ -19,7 +19,7 @@ module ActiveModel @second_post.author = @author @author.posts = [@first_post, @second_post] - @serializer = ArraySerializer.new([@first_post, @second_post]) + @serializer = CollectionSerializer.new([@first_post, @second_post]) @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer) ActionController::Base.cache_store.clear end diff --git a/test/adapter/json_api/linked_test.rb b/test/adapter/json_api/linked_test.rb index ead5d576..cf3aa21f 100644 --- a/test/adapter/json_api/linked_test.rb +++ b/test/adapter/json_api/linked_test.rb @@ -45,7 +45,7 @@ module ActiveModel end def test_include_multiple_posts_and_linked_array - serializer = ArraySerializer.new([@first_post, @second_post]) + serializer = CollectionSerializer.new([@first_post, @second_post]) adapter = ActiveModel::Serializer::Adapter::JsonApi.new( serializer, include: [:comments, author: [:bio]] @@ -226,7 +226,7 @@ module ActiveModel end def test_multiple_references_to_same_resource - serializer = ArraySerializer.new([@first_comment, @second_comment]) + serializer = CollectionSerializer.new([@first_comment, @second_comment]) adapter = ActiveModel::Serializer::Adapter::JsonApi.new( serializer, include: [:post] diff --git a/test/array_serializer_test.rb b/test/array_serializer_test.rb index 3141f713..233756c4 100644 --- a/test/array_serializer_test.rb +++ b/test/array_serializer_test.rb @@ -1,92 +1,21 @@ require 'test_helper' +require_relative 'collection_serializer_test' module ActiveModel class Serializer - class ArraySerializerTest < Minitest::Test - def setup - @comment = Comment.new - @post = Post.new - @resource = build_named_collection @comment, @post - @serializer = ArraySerializer.new(@resource, { some: :options }) + class ArraySerializerTest < CollectionSerializerTest + extend ActiveSupport::Testing::Stream + def self.run_one_method(*) + stderr = (capture(:stderr) do + super + end) + if stderr !~ /Calling deprecated ArraySerializer/ + fail Minitest::Assertion, stderr + end end - def build_named_collection(*resource) - resource.define_singleton_method(:name) { 'MeResource' } - resource - end - - def test_has_object_reader_serializer_interface - assert_equal @serializer.object, @resource - end - - def test_respond_to_each - assert_respond_to @serializer, :each - end - - def test_each_object_should_be_serialized_with_appropriate_serializer - serializers = @serializer.to_a - - assert_kind_of CommentSerializer, serializers.first - assert_kind_of Comment, serializers.first.object - - assert_kind_of PostSerializer, serializers.last - assert_kind_of Post, serializers.last.object - - assert_equal serializers.last.custom_options[:some], :options - end - - def test_serializer_option_not_passed_to_each_serializer - serializers = ArraySerializer.new([@post], { serializer: PostSerializer }).to_a - - refute serializers.first.custom_options.key?(:serializer) - end - - def test_root_default - @serializer = ArraySerializer.new([@comment, @post]) - assert_equal @serializer.root, nil - end - - def test_root - expected = 'custom_root' - @serializer = ArraySerializer.new([@comment, @post], root: expected) - assert_equal @serializer.root, expected - end - - def test_root_with_no_serializers - expected = 'custom_root' - @serializer = ArraySerializer.new([], root: expected) - assert_equal @serializer.root, expected - end - - def test_json_key - assert_equal @serializer.json_key, 'comments' - end - - def test_json_key_with_resource_with_name_and_no_serializers - serializer = ArraySerializer.new(build_named_collection) - assert_equal serializer.json_key, 'me_resources' - end - - def test_json_key_with_resource_with_nil_name_and_no_serializers - resource = [] - resource.define_singleton_method(:name) { nil } - serializer = ArraySerializer.new(resource) - assert_equal serializer.json_key, nil - end - - def test_json_key_with_resource_without_name_and_no_serializers - serializer = ArraySerializer.new([]) - assert_equal serializer.json_key, nil - end - - def test_json_key_with_root - serializer = ArraySerializer.new(@resource, root: 'custom_root') - assert_equal serializer.json_key, 'custom_roots' - end - - def test_json_key_with_root_and_no_serializers - serializer = ArraySerializer.new(build_named_collection, root: 'custom_root') - assert_equal serializer.json_key, 'custom_roots' + def collection_serializer + ArraySerializer end end end diff --git a/test/collection_serializer_test.rb b/test/collection_serializer_test.rb new file mode 100644 index 00000000..d874cec2 --- /dev/null +++ b/test/collection_serializer_test.rb @@ -0,0 +1,97 @@ +require 'test_helper' + +module ActiveModel + class Serializer + class CollectionSerializerTest < Minitest::Test + def setup + @comment = Comment.new + @post = Post.new + @resource = build_named_collection @comment, @post + @serializer = collection_serializer.new(@resource, { some: :options }) + end + + def collection_serializer + CollectionSerializer + end + + def build_named_collection(*resource) + resource.define_singleton_method(:name) { 'MeResource' } + resource + end + + def test_has_object_reader_serializer_interface + assert_equal @serializer.object, @resource + end + + def test_respond_to_each + assert_respond_to @serializer, :each + end + + def test_each_object_should_be_serialized_with_appropriate_serializer + serializers = @serializer.to_a + + assert_kind_of CommentSerializer, serializers.first + assert_kind_of Comment, serializers.first.object + + assert_kind_of PostSerializer, serializers.last + assert_kind_of Post, serializers.last.object + + assert_equal serializers.last.custom_options[:some], :options + end + + def test_serializer_option_not_passed_to_each_serializer + serializers = collection_serializer.new([@post], { serializer: PostSerializer }).to_a + + refute serializers.first.custom_options.key?(:serializer) + end + + def test_root_default + @serializer = collection_serializer.new([@comment, @post]) + assert_equal @serializer.root, nil + end + + def test_root + expected = 'custom_root' + @serializer = collection_serializer.new([@comment, @post], root: expected) + assert_equal @serializer.root, expected + end + + def test_root_with_no_serializers + expected = 'custom_root' + @serializer = collection_serializer.new([], root: expected) + assert_equal @serializer.root, expected + end + + def test_json_key + assert_equal @serializer.json_key, 'comments' + end + + def test_json_key_with_resource_with_name_and_no_serializers + serializer = collection_serializer.new(build_named_collection) + assert_equal serializer.json_key, 'me_resources' + end + + def test_json_key_with_resource_with_nil_name_and_no_serializers + resource = [] + resource.define_singleton_method(:name) { nil } + serializer = collection_serializer.new(resource) + assert_equal serializer.json_key, nil + end + + def test_json_key_with_resource_without_name_and_no_serializers + serializer = collection_serializer.new([]) + assert_equal serializer.json_key, nil + end + + def test_json_key_with_root + serializer = collection_serializer.new(@resource, root: 'custom_root') + assert_equal serializer.json_key, 'custom_roots' + end + + def test_json_key_with_root_and_no_serializers + serializer = collection_serializer.new(build_named_collection, root: 'custom_root') + assert_equal serializer.json_key, 'custom_roots' + end + end + end +end diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index ca4a8b45..28ee08d5 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -154,7 +154,7 @@ BlogSerializer = Class.new(ActiveModel::Serializer) do has_many :articles end -PaginatedSerializer = Class.new(ActiveModel::Serializer::ArraySerializer) do +PaginatedSerializer = Class.new(ActiveModel::Serializer::CollectionSerializer) do def json_key 'paginated' end diff --git a/test/serializers/associations_test.rb b/test/serializers/associations_test.rb index 10646907..205cfcc8 100644 --- a/test/serializers/associations_test.rb +++ b/test/serializers/associations_test.rb @@ -33,13 +33,13 @@ module ActiveModel case key when :posts assert_equal({}, options) - assert_kind_of(ActiveModel::Serializer.config.array_serializer, serializer) + assert_kind_of(ActiveModel::Serializer.config.collection_serializer, serializer) when :bio assert_equal({}, options) assert_nil serializer when :roles assert_equal({}, options) - assert_kind_of(ActiveModel::Serializer.config.array_serializer, serializer) + assert_kind_of(ActiveModel::Serializer.config.collection_serializer, serializer) else flunk "Unknown association: #{key}" end diff --git a/test/serializers/configuration_test.rb b/test/serializers/configuration_test.rb index 24e02836..d9667b9e 100644 --- a/test/serializers/configuration_test.rb +++ b/test/serializers/configuration_test.rb @@ -3,8 +3,25 @@ require 'test_helper' module ActiveModel class Serializer class ConfigurationTest < Minitest::Test + def test_collection_serializer + assert_equal ActiveModel::Serializer::CollectionSerializer, ActiveModel::Serializer.config.collection_serializer + end + def test_array_serializer - assert_equal ActiveModel::Serializer::ArraySerializer, ActiveModel::Serializer.config.array_serializer + assert_equal ActiveModel::Serializer::CollectionSerializer, ActiveModel::Serializer.config.array_serializer + end + + def test_setting_array_serializer_sets_collection_serializer + config = ActiveModel::Serializer.config + old_config = config.dup + begin + assert_equal ActiveModel::Serializer::CollectionSerializer, config.collection_serializer + config.array_serializer = :foo + assert_equal config.array_serializer, :foo + assert_equal config.collection_serializer, :foo + ensure + ActiveModel::Serializer.config.replace(old_config) + end end def test_default_adapter diff --git a/test/serializers/serializer_for_test.rb b/test/serializers/serializer_for_test.rb index 7dc18935..fe0acef9 100644 --- a/test/serializers/serializer_for_test.rb +++ b/test/serializers/serializer_for_test.rb @@ -3,26 +3,26 @@ require 'test_helper' module ActiveModel class Serializer class SerializerForTest < Minitest::Test - class ArraySerializerTest < Minitest::Test + class CollectionSerializerTest < Minitest::Test def setup @array = [1, 2, 3] - @previous_array_serializer = ActiveModel::Serializer.config.array_serializer + @previous_collection_serializer = ActiveModel::Serializer.config.collection_serializer end def teardown - ActiveModel::Serializer.config.array_serializer = @previous_array_serializer + ActiveModel::Serializer.config.collection_serializer = @previous_collection_serializer end def test_serializer_for_array serializer = ActiveModel::Serializer.serializer_for(@array) - assert_equal ActiveModel::Serializer.config.array_serializer, serializer + assert_equal ActiveModel::Serializer.config.collection_serializer, serializer end def test_overwritten_serializer_for_array - new_array_serializer = Class.new - ActiveModel::Serializer.config.array_serializer = new_array_serializer + new_collection_serializer = Class.new + ActiveModel::Serializer.config.collection_serializer = new_collection_serializer serializer = ActiveModel::Serializer.serializer_for(@array) - assert_equal new_array_serializer, serializer + assert_equal new_collection_serializer, serializer end end