Skip to content

Commit

Permalink
Allow the serializer to return sparse fieldsets
Browse files Browse the repository at this point in the history
  • Loading branch information
Erol authored and shishirmk committed Jul 17, 2018
1 parent 41c1e0a commit a363c90
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 17 deletions.
7 changes: 7 additions & 0 deletions lib/fast_jsonapi/fieldset.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module FastJsonapi
class Fieldset
def initialize(fields)
@fields = fields
end
end
end
25 changes: 21 additions & 4 deletions lib/fast_jsonapi/object_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require 'fast_jsonapi/relationship'
require 'fast_jsonapi/link'
require 'fast_jsonapi/serialization_core'
require 'fast_jsonapi/fieldset'

module FastJsonapi
module ObjectSerializer
Expand Down Expand Up @@ -41,8 +42,8 @@ def hash_for_one_record

return serializable_hash unless @resource

serializable_hash[:data] = self.class.record_hash(@resource, @params)
serializable_hash[:included] = self.class.get_included_records(@resource, @includes, @known_included_objects, @params) if @includes.present?
serializable_hash[:data] = self.class.record_hash(@resource, @fieldsets[self.class.reflected_record_type.to_sym], @params)
serializable_hash[:included] = self.class.get_included_records(@resource, @includes, @known_included_objects, @fieldsets, @params) if @includes.present?
serializable_hash
end

Expand All @@ -51,9 +52,10 @@ def hash_for_collection

data = []
included = []
fieldset = @fieldsets[self.class.reflected_record_type.to_sym]
@resource.each do |record|
data << self.class.record_hash(record, @params)
included.concat self.class.get_included_records(record, @includes, @known_included_objects, @params) if @includes.present?
data << self.class.record_hash(record, fieldset, @params)
included.concat self.class.get_included_records(record, @includes, @known_included_objects, @fieldsets, @params) if @includes.present?
end

serializable_hash[:data] = data
Expand All @@ -70,6 +72,8 @@ def serialized_json
private

def process_options(options)
@fieldsets = deep_symbolize(options[:fields].presence || {})

return if options.blank?

@known_included_objects = {}
Expand All @@ -85,6 +89,18 @@ def process_options(options)
end
end

def deep_symbolize(collection)
if collection.is_a? Hash
Hash[collection.map do |k, v|
[k.to_sym, deep_symbolize(v)]
end]
elsif collection.is_a? Array
collection.map { |i| deep_symbolize(i) }
else
collection.to_sym
end
end

def is_collection?(resource, force_is_collection = nil)
return force_is_collection unless force_is_collection.nil?

Expand All @@ -104,6 +120,7 @@ def inherited(subclass)
subclass.race_condition_ttl = race_condition_ttl
subclass.data_links = data_links
subclass.cached = cached
subclass.fieldset = fieldset.dup if fieldset.present?
end

def reflected_record_type
Expand Down
29 changes: 17 additions & 12 deletions lib/fast_jsonapi/serialization_core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class << self
:cache_length,
:race_condition_ttl,
:cached,
:data_links
:data_links,
:fieldset
end
end

Expand All @@ -40,36 +41,39 @@ def links_hash(record, params = {})
end
end

def attributes_hash(record, params = {})
attributes_to_serialize.each_with_object({}) do |(_k, attribute), hash|
def attributes_hash(record, fieldset = nil, params = {})
attributes = attributes_to_serialize
attributes = attributes.slice(*fieldset) if fieldset.present?
attributes.each_with_object({}) do |(_k, attribute), hash|
attribute.serialize(record, params, hash)
end
end

def relationships_hash(record, relationships = nil, params = {})
def relationships_hash(record, relationships = nil, fieldset = nil, params = {})
relationships = relationships_to_serialize if relationships.nil?
relationships = relationships.slice(*fieldset) if fieldset.present?

This comment has been minimized.

Copy link
@everlast240

everlast240 Aug 8, 2019

What's this needed for here? And what do relationships have to do with sparse fieldsets?
In fact, I've identified this line as causing a bug (pretty critical actually): if you have sparse fieldsets (fields in the request), relationships key remains empty and when returning multiple results, you have no way how to link included resources (include clause) to the proper root resource in a compound document.
Commenting it out seems to resolve this issue...

This comment has been minimized.

Copy link
@stas

stas Aug 8, 2019

Contributor

@everlast240 do you think you can file an issue please 🙏

This comment has been minimized.

Copy link
@everlast240

everlast240 Aug 12, 2019

Done. : #439
When submitting I found a related issue with a way to work around the problem: #304
So, I'm not sure if we're supposed to use that way or it's a bug

This comment has been minimized.

Copy link
@stas

stas Aug 12, 2019

Contributor

Thanks Borislav!

This comment has been minimized.

Copy link
@everlast240

everlast240 Aug 12, 2019

Btw it seems the jsonapi-resources gem has similar behavior - you need to specify the relationship name in the fields (sparse fieldset) if you want it to get included...
E.g.:
included=address,address.country&fields[ROOT_RESOUCE]=attr1,attr2,address&fields[addresses]=latitude,longitude,country
That's really confusing, but it seems it's the expected way of working. So maybe just a documentation update to make this clear would be nice.


relationships.each_with_object({}) do |(_k, relationship), hash|
relationship.serialize(record, params, hash)
end
end

def record_hash(record, params = {})
def record_hash(record, fieldset, params = {})
if cached
record_hash = Rails.cache.fetch(record.cache_key, expires_in: cache_length, race_condition_ttl: race_condition_ttl) do
temp_hash = id_hash(id_from_record(record), record_type, true)
temp_hash[:attributes] = attributes_hash(record, params) if attributes_to_serialize.present?
temp_hash[:attributes] = attributes_hash(record, fieldset, params) if attributes_to_serialize.present?
temp_hash[:relationships] = {}
temp_hash[:relationships] = relationships_hash(record, cachable_relationships_to_serialize, params) if cachable_relationships_to_serialize.present?
temp_hash[:relationships] = relationships_hash(record, cachable_relationships_to_serialize, fieldset, params) if cachable_relationships_to_serialize.present?
temp_hash[:links] = links_hash(record, params) if data_links.present?
temp_hash
end
record_hash[:relationships] = record_hash[:relationships].merge(relationships_hash(record, uncachable_relationships_to_serialize, params)) if uncachable_relationships_to_serialize.present?
record_hash
else
record_hash = id_hash(id_from_record(record), record_type, true)
record_hash[:attributes] = attributes_hash(record, params) if attributes_to_serialize.present?
record_hash[:relationships] = relationships_hash(record, nil, params) if relationships_to_serialize.present?
record_hash[:attributes] = attributes_hash(record, fieldset, params) if attributes_to_serialize.present?
record_hash[:relationships] = relationships_hash(record, nil, fieldset, params) if relationships_to_serialize.present?
record_hash[:links] = links_hash(record, params) if data_links.present?
record_hash
end
Expand Down Expand Up @@ -100,7 +104,7 @@ def remaining_items(items)
end

# includes handler
def get_included_records(record, includes_list, known_included_objects, params = {})
def get_included_records(record, includes_list, known_included_objects, fieldsets, params = {})
return unless includes_list.present?

includes_list.sort.each_with_object([]) do |include_item, included_records|
Expand All @@ -120,15 +124,16 @@ def get_included_records(record, includes_list, known_included_objects, params =

included_objects.each do |inc_obj|
if remaining_items(items)
serializer_records = serializer.get_included_records(inc_obj, remaining_items(items), known_included_objects)
serializer_records = serializer.get_included_records(inc_obj, remaining_items(items), known_included_objects, fieldsets)
included_records.concat(serializer_records) unless serializer_records.empty?
end

code = "#{record_type}_#{inc_obj.id}"
next if known_included_objects.key?(code)

known_included_objects[code] = inc_obj
included_records << serializer.record_hash(inc_obj, params)

included_records << serializer.record_hash(inc_obj, fieldsets[serializer.reflected_record_type], params)
end
end
end
Expand Down
36 changes: 36 additions & 0 deletions spec/lib/object_serializer_fields_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
require 'spec_helper'

describe FastJsonapi::ObjectSerializer do
include_context 'movie class'

let(:fields) do
{
movie: %i[name actors],
actor: %i[name agency]
}
end

it 'only returns specified fields' do
hash = MovieSerializer.new(movie, fields: fields).serializable_hash

expect(hash[:data][:attributes].keys.sort).to eq %i[name]
end

it 'only returns specified relationships' do
hash = MovieSerializer.new(movie, fields: fields).serializable_hash

expect(hash[:data][:relationships].keys.sort).to eq %i[actors]
end

it 'only returns specified fields for included relationships' do
hash = MovieSerializer.new(movie, fields: fields, include: %i[actors]).serializable_hash

expect(hash[:included].first[:attributes].keys.sort).to eq %i[name]
end

it 'only returns specified relationships for included relationships' do
hash = MovieSerializer.new(movie, fields: fields, include: %i[actors]).serializable_hash

expect(hash[:included].first[:relationships].keys.sort).to eq %i[agency]
end
end
2 changes: 1 addition & 1 deletion spec/lib/serialization_core_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
known_included_objects = {}
included_records = []
[movie, movie].each do |record|
included_records.concat MovieSerializer.send(:get_included_records, record, includes_list, known_included_objects, nil)
included_records.concat MovieSerializer.send(:get_included_records, record, includes_list, known_included_objects, {}, nil)
end
expect(included_records.size).to eq 3
end
Expand Down

0 comments on commit a363c90

Please sign in to comment.