From 658810e6a0417d3c4222a448bf3166c60757f25a Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Mon, 5 Oct 2015 06:08:11 +0200 Subject: [PATCH] Extract attributes filtering from serializer into adapter. --- CHANGELOG.md | 1 + lib/active_model/serializer.rb | 9 ++---- .../serializer/adapter/attributes.rb | 5 +++- .../serializer/adapter/json_api.rb | 30 +++++++++---------- lib/active_model/serializer/fieldset.rb | 17 +++-------- test/adapter/json_api/collection_test.rb | 8 +++-- test/serializers/attributes_test.rb | 5 ---- test/serializers/fieldset_test.rb | 19 +++--------- 8 files changed, 35 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ecc4189..baa15d103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Fixes: - [#1214](https://github.com/rails-api/active_model_serializers/pull/1214) retrieve the key from the reflection options when building associations (@NullVoxPopuli, @hut8) Misc: +- [#1232](https://github.com/rails-api/active_model_serializers/pull/1232) fields option no longer handled at serializer level (@beauby) - [#1178](https://github.com/rails-api/active_model_serializers/pull/1178) env CAPTURE_STDERR=false lets devs see hard failures (@bf4) - [#1177](https://github.com/rails-api/active_model_serializers/pull/1177) Remove Adapter autoloads in favor of require (@bf4) - [#1117](https://github.com/rails-api/active_model_serializers/pull/1117) FlattenJson adapter no longer inherits Json adapter, renamed to Attributes (@bf4) diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 1e3b0b724..eafa4942d 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -148,13 +148,8 @@ def json_key root || object.class.model_name.to_s.underscore end - def attributes(options = {}) - attributes = - if options[:fields] - self.class._attributes & options[:fields] - else - self.class._attributes.dup - end + def attributes + attributes = self.class._attributes.dup attributes.each_with_object({}) do |name, hash| unless self.class._fragmented diff --git a/lib/active_model/serializer/adapter/attributes.rb b/lib/active_model/serializer/adapter/attributes.rb index 2d2da6ebb..15577eb8e 100644 --- a/lib/active_model/serializer/adapter/attributes.rb +++ b/lib/active_model/serializer/adapter/attributes.rb @@ -57,7 +57,10 @@ def include_meta(json) def resource_object_for(options) cache_check(serializer) do - serializer.attributes(options) + attributes = serializer.attributes + attributes.slice!(*options[:fields]) if options[:fields] + + attributes end end end diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index a6977877f..2792af094 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -47,7 +47,7 @@ def initialize(serializer, options = {}) fields = options.delete(:fields) if fields - @fieldset = ActiveModel::Serializer::Fieldset.new(fields, serializer.json_key) + @fieldset = ActiveModel::Serializer::Fieldset.new(fields) else @fieldset = options[:fieldset] end @@ -60,7 +60,7 @@ def serializable_hash(options = nil) if serializer.respond_to?(:each) serializable_hash_for_collection(options) else - serializable_hash_for_single_resource(options) + serializable_hash_for_single_resource end ApiObjects::JsonApi.add!(hash) @@ -99,8 +99,8 @@ def serializable_hash_for_collection(options) hash end - def serializable_hash_for_single_resource(options) - primary_data = primary_data_for(serializer, options) + def serializable_hash_for_single_resource + primary_data = primary_data_for(serializer) relationships = relationships_for(serializer) included = included_resources(@include_tree) hash = { data: primary_data } @@ -134,22 +134,22 @@ def resource_identifier_for(serializer) { id: id.to_s, type: type } end - def resource_object_for(serializer, options = {}) - options[:fields] = fieldset && fieldset.fields_for(serializer) - + def resource_object_for(serializer) cache_check(serializer) do - result = resource_identifier_for(serializer) - attributes = serializer.attributes(options).except(:id) - result[:attributes] = attributes if attributes.any? - result + resource_object = resource_identifier_for(serializer) + requested_fields = fieldset && fieldset.fields_for(resource_object[:type]) + attributes = serializer.attributes.except(:id) + attributes.slice!(*requested_fields) if requested_fields + resource_object[:attributes] = attributes if attributes.any? + resource_object end end - def primary_data_for(serializer, options) + def primary_data_for(serializer) if serializer.respond_to?(:each) - serializer.map { |s| resource_object_for(s, options) } + serializer.map { |s| resource_object_for(s) } else - resource_object_for(serializer, options) + resource_object_for(serializer) end end @@ -187,7 +187,7 @@ def add_included_resources_for(serializer, include_tree, included) else return unless serializer && serializer.object - primary_data = primary_data_for(serializer, instance_options) + primary_data = primary_data_for(serializer) relationships = relationships_for(serializer) primary_data[:relationships] = relationships if relationships.any? diff --git a/lib/active_model/serializer/fieldset.rb b/lib/active_model/serializer/fieldset.rb index 55dad6346..eaa47ec8d 100644 --- a/lib/active_model/serializer/fieldset.rb +++ b/lib/active_model/serializer/fieldset.rb @@ -1,8 +1,7 @@ module ActiveModel class Serializer class Fieldset - def initialize(fields, root = nil) - @root = root + def initialize(fields) @raw_fields = fields end @@ -10,27 +9,19 @@ def fields @fields ||= parsed_fields end - def fields_for(serializer) - key = serializer.json_key - fields[key.to_sym] || fields[key.pluralize.to_sym] + def fields_for(type) + fields[type.singularize.to_sym] || fields[type.pluralize.to_sym] end private ActiveModelSerializers.silence_warnings do - attr_reader :raw_fields, :root + attr_reader :raw_fields end def parsed_fields if raw_fields.is_a?(Hash) raw_fields.inject({}) { |h, (k, v)| h[k.to_sym] = v.map(&:to_sym); h } - elsif raw_fields.is_a?(Array) - if root.nil? - raise ArgumentError, 'The root argument must be specified if the fields argument is an array.'.freeze - end - hash = {} - hash[root.to_sym] = raw_fields.map(&:to_sym) - hash else {} end diff --git a/test/adapter/json_api/collection_test.rb b/test/adapter/json_api/collection_test.rb index 7f42719e6..d1e70cf87 100644 --- a/test/adapter/json_api/collection_test.rb +++ b/test/adapter/json_api/collection_test.rb @@ -58,8 +58,10 @@ def test_include_multiple_posts end def test_limiting_fields - @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, fields: ['title']) - + actual = ActiveModel::SerializableResource.new( + [@first_post, @second_post], adapter: :json_api, + fields: { posts: ['title'] }) + .serializable_hash expected = [ { id: '1', @@ -86,7 +88,7 @@ def test_limiting_fields } } ] - assert_equal(expected, @adapter.serializable_hash[:data]) + assert_equal(expected, actual[:data]) end end end diff --git a/test/serializers/attributes_test.rb b/test/serializers/attributes_test.rb index 735de3f97..684500d99 100644 --- a/test/serializers/attributes_test.rb +++ b/test/serializers/attributes_test.rb @@ -18,11 +18,6 @@ def test_attributes_definition @profile_serializer.class._attributes) end - def test_attributes_with_fields_option - assert_equal({ name: 'Name 1' }, - @profile_serializer.attributes(fields: [:name])) - end - def test_attributes_inheritance_definition assert_equal([:id, :body], @serializer_klass._attributes) end diff --git a/test/serializers/fieldset_test.rb b/test/serializers/fieldset_test.rb index ca613045d..e042f36fe 100644 --- a/test/serializers/fieldset_test.rb +++ b/test/serializers/fieldset_test.rb @@ -4,22 +4,11 @@ module ActiveModel class Serializer class FieldsetTest < Minitest::Test def test_fieldset_with_hash - fieldset = ActiveModel::Serializer::Fieldset.new({ 'post' => %w(id title), 'coment' => ['body'] }) + fieldset = ActiveModel::Serializer::Fieldset.new('post' => %w(id title), 'comment' => ['body']) + expected = { :post => [:id, :title], :comment => [:body] } - assert_equal( - { :post => [:id, :title], :coment => [:body] }, - fieldset.fields - ) - end - - def test_fieldset_with_array_of_fields_and_root_name - fieldset = ActiveModel::Serializer::Fieldset.new(['title'], 'post') - - assert_equal( - { :post => [:title] }, - fieldset.fields - ) + assert_equal(expected, fieldset.fields) end end end -end \ No newline at end of file +end