Skip to content

sparse fieldsets #700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 6, 2015
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/action_controller/serialization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Serialization

include ActionController::Renderers

ADAPTER_OPTION_KEYS = [:include, :root]
ADAPTER_OPTION_KEYS = [:include, :fields, :root]

[:_render_option_json, :_render_with_renderer_json].each do |renderer_method|
define_method renderer_method do |resource, options|
Expand Down
9 changes: 8 additions & 1 deletion lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,14 @@ def json_key
end

def attributes(options = {})
self.class._attributes.dup.each_with_object({}) do |name, hash|
attributes =
if options[:fields]
self.class._attributes & options[:fields]
else
self.class._attributes.dup
end

attributes.each_with_object({}) do |name, hash|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change belongs into the serializer. The serializer should just return all attributes, where jsonapi adapter would then intersect the ones it needs. I think this should go inside JsonApiAdapter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree If it just returned the attribute names but it calls send for each attribute. It would be terribly inefficient to call the method just to filter it out later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurko any suggestions?

hash[name] = send(name)
end
end
Expand Down
12 changes: 11 additions & 1 deletion lib/active_model/serializer/adapter/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,20 @@ def initialize(serializer, options = {})
serializer.root = true
@hash = {}
@top = @options.fetch(:top) { @hash }

if fields = options.delete(:fields)
@fieldset = ActiveModel::Serializer::Fieldset.new(fields, serializer.json_key)
else
@fieldset = options[:fieldset]
end
end

def serializable_hash(options = {})
@root = (@options[:root] || serializer.json_key.to_s.pluralize).to_sym

if serializer.respond_to?(:each)
@hash[@root] = serializer.map do |s|
self.class.new(s, @options.merge(top: @top)).serializable_hash[@root]
self.class.new(s, @options.merge(top: @top, fieldset: @fieldset)).serializable_hash[@root]
end
else
@hash[@root] = attributes_for_serializer(serializer, @options)
Expand Down Expand Up @@ -93,6 +99,10 @@ def add_linked(resource, serializer, parent = nil)
private

def attributes_for_serializer(serializer, options)
if fields = @fieldset && @fieldset.fields_for(serializer)
options[:fields] = fields
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just

options[:fields] = @fields if @fieldset.fields_for(serializer)

will do. No need to set this variable.

attributes = serializer.attributes(options)
attributes[:id] = attributes[:id].to_s if attributes[:id]
attributes
Expand Down
36 changes: 36 additions & 0 deletions lib/active_model/serializer/fieldset.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module ActiveModel
class Serializer
class Fieldset

attr_reader :fields, :root
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used by anyone outside this class? If not, could you move it to below the private line? Just so we're aware of what's public and what's not for easiness of refactoring.


def initialize(fields, root = nil)
@root = root
@fields = parse(fields)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind moving this parse() into another method for lazy evaluation sake? Evaluating things on instantiation can lead to hard to debug scenarios later.

...
  @fields = fields
...

def fields
  @parsed_fields ||= parse(fields)
end

end

def fields_for(serializer)
key = serializer.json_key || serializer.class.root_name
fields[key.to_sym]
end

private

def parse(fields)
if fields.is_a?(Hash)
fields.inject({}) { |h,(k,v)| h[k.to_sym] = v.map(&:to_sym); h}
elsif fields.is_a?(Array)
if root.nil?
raise ArgumentError, 'The root argument must be specified if the fileds argument is an array.'
end
hash = {}
hash[root.to_sym] = fields.map(&:to_sym)
hash
else
{}
end
end

end
end
end
1 change: 1 addition & 0 deletions lib/active_model_serializers.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require "active_model"
require "active_model/serializer/version"
require "active_model/serializer"
require "active_model/serializer/fieldset"

begin
require 'action_controller'
Expand Down
5 changes: 5 additions & 0 deletions test/adapter/json_api/belongs_to_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ def test_includes_linked_post
assert_equal([{id: "42", title: 'New Post', body: 'Body'}], @adapter.serializable_hash[:linked][:posts])
end

def test_limiting_linked_post_fields
@adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'post', fields: {post: [:title]})
assert_equal([{title: 'New Post'}], @adapter.serializable_hash[:linked][:posts])
end

def test_include_nil_author
serializer = PostSerializer.new(@anonymous_post)
adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer)
Expand Down
9 changes: 9 additions & 0 deletions test/adapter/json_api/collection_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ def test_include_multiple_posts
{ title: "New Post", body: "Body", id: "2", links: { comments: [], author: "1" } }
], @adapter.serializable_hash[:posts])
end

def test_limiting_fields
@adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, fields: ['title'])
assert_equal([
{ title: "Hello!!", links: { comments: [], author: "1" } },
{ title: "New Post", links: { comments: [], author: "1" } }
], @adapter.serializable_hash[:posts])
end

end
end
end
Expand Down
8 changes: 8 additions & 0 deletions test/adapter/json_api/has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ def test_includes_linked_comments
], @adapter.serializable_hash[:linked][:comments])
end

def test_limit_fields_of_linked_comments
@adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, include: 'comments', fields: {comment: [:body]})
assert_equal([
{body: 'ZOMG A COMMENT'},
{body: 'ZOMG ANOTHER COMMENT'}
], @adapter.serializable_hash[:linked][:comments])
end

def test_no_include_linked_if_comments_is_empty
serializer = PostSerializer.new(@post_without_comments)
adapter = ActiveModel::Serializer::Adapter::JsonApi.new(serializer)
Expand Down
6 changes: 5 additions & 1 deletion test/serializers/attributes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ def test_attributes_definition
assert_equal([:name, :description],
@profile_serializer.class._attributes)
end

def test_attributes_with_fields_option
assert_equal({name: 'Name 1'},
@profile_serializer.attributes( { fields: [:name] } ) )
end
end
end
end

26 changes: 26 additions & 0 deletions test/serializers/fieldset_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
require 'test_helper'

module ActiveModel
class Serializer
class FieldsetTest < Minitest::Test

def test_fieldset_with_hash
fieldset = ActiveModel::Serializer::Fieldset.new({'post' => ['id', 'title'], 'coment' => ['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
)
end
end
end
end