Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
17 changes: 12 additions & 5 deletions lib/oat/adapters/json_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def meta(key, value)

def entity(name, obj, serializer_class = nil, context_options = {}, &block)
ent = serializer_from_block_or_class(obj, serializer_class, context_options, &block)
if ent
if ent && !serializer.serialized?(name, ent.item.id)
ent_hash = ent.to_hash
_name = entity_name(name)
entity_hash[_name.to_s.pluralize.to_sym] ||= []
Expand All @@ -80,16 +80,17 @@ def entity(name, obj, serializer_class = nil, context_options = {}, &block)
end
end

def entities(name, collection, serializer_class = nil, context_options = {}, &block)
def entities(name, collection, serializer_class = nil, context_options = nil, &block)
return if collection.nil? || collection.empty?
context_options ||= {}
Copy link
Owner

Choose a reason for hiding this comment

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

If the content_options argument default value is nil, this line will always resolve to {}. So keeping the default value as {} means this line is not necessary.

Copy link
Author

Choose a reason for hiding this comment

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

The default only takes effect when the arg is not supplied. The intention is for context_options to be {} even if the argument is passed in explicitly as nil.

Copy link
Owner

Choose a reason for hiding this comment

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

I see, that makes sense. Thanks for clarifying.

_name = entity_name(name)
link_name = _name.to_s.pluralize.to_sym
data[:links][link_name] = []

collection.each do |obj|
entity_hash[link_name] ||= []
ent = serializer_from_block_or_class(obj, serializer_class, context_options, &block)
if ent
if ent && !serializer.serialized?(link_name, ent.item.id)
ent_hash = ent.to_hash
data[:links][link_name] << ent_hash[:id]
entity_hash[link_name] << ent_hash
Expand All @@ -104,13 +105,19 @@ def entity_name(name)

private :entity_name

def collection(name, collection, serializer_class = nil, context_options = {}, &block)
def collection(name, collection, serializer_class = nil, context_options = nil, &block)
context_options ||= {}
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

@treat_as_resource_collection = true
data[:resource_collection] = [] unless data[:resource_collection].is_a?(Array)

collection.each do |obj|
ent = serializer_from_block_or_class(obj, serializer_class, context_options, &block)
data[:resource_collection] << ent.to_hash if ent
if ent
unless serializer.serialized?(root_name, ent.item.id)
# serializer.set_serialized(root_name, ent.item.id)
data[:resource_collection] << ent.to_hash
end
end
end
end

Expand Down
27 changes: 25 additions & 2 deletions lib/oat/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ class Serializer

class_attribute :_adapter, :logger

class << self
attr_accessor :type
end

def self.schema(&block)
@schema = block if block_given?
@schema || Proc.new{}
Expand All @@ -20,11 +24,16 @@ def self.warn(msg)

attr_reader :item, :context, :adapter_class, :adapter

def initialize(item, context = {}, _adapter_class = nil, parent_serializer = nil)
@item, @context = item, context
def initialize(item, context = nil, _adapter_class = nil, parent_serializer = nil)
@item = item
@context = context || {}
Copy link
Owner

Choose a reason for hiding this comment

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

Same: keeping context = {} makes this line redundant.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry in this case it makes the conditional assignment redundant.

@context = context

@parent_serializer = parent_serializer
@adapter_class = _adapter_class || self.class.adapter
@adapter = @adapter_class.new(self)
if self.class.type
type(self.class.type)
end
@context[:_serialized_entities] ||= {}
end

def top
Expand All @@ -51,6 +60,9 @@ def respond_to_missing?(method_name, include_private = false)

def to_hash
@to_hash ||= (
if self.class.type
Array(item).each { |i| set_serialized(self.class.type, i.id) }
Copy link
Owner

Choose a reason for hiding this comment

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

Why is it needed to wrap item in an array? Is item ever an array itself?

end
instance_eval(&self.class.schema)
adapter.to_hash
)
Expand All @@ -65,5 +77,16 @@ def map_property(name)
property name, value
end

def set_serialized(type, id)
@context[:_serialized_entities][type] ||= {}
@context[:_serialized_entities][type][id] = true
end

def serialized?(type, id)
h = @context[:_serialized_entities]
h = h[type] if h
Copy link
Owner

Choose a reason for hiding this comment

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

@context[:serialized_entities] is always assigned on initialize, so is this if h needed?

h[id] if h
Copy link
Owner

Choose a reason for hiding this comment

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

This method should always return true or false, so maybe the last line could be:

h && h[id]

end

end
end