fast_jsonapi icon indicating copy to clipboard operation
fast_jsonapi copied to clipboard

Serializer validates only first argument of `include` option

Open AlexTatarnikov opened this issue 5 years ago • 2 comments

Hi,

I've noticed that #validate_includes! validates only first element of the include option array. The reason for this behavior is #detect method that was used in order to iterate through the array, it stops iterating after a first successful block or exception obviously.

class Movie
  attr_accessor :id, :name, :actor_ids, :actors
end

class Actor
  attr_accessor :id
end

class ActorSerializer < ApplicationSerializer
  attributes :id
end

class MovieSerializer < ApplicationSerializer
  attributes :id, :name
  has_many :actors
end

actor = Actor.new
actor.id = 1

movie = Movie.new
movie.id = 1
movie.name = 'Movie #1'
movie.actor_ids = [1]
movie.actors = [actor]

#include: %i(actors blah_blah)

irb(main):001:0> MovieSerializer.new(movie, include: %i(actors blah_blah)).serializable_hash
=> {:data=>{:id=>"1", :type=>:movie, :attributes=>{:id=>1, :name=>"Movie #1"}, :relationships=>{:actors=>{:data=>[{:id=>"1", :type=>:actor}]}}}, :included=>[{:id=>"1", :type=>:actor, :attributes=>{:id=>1}}]}

#include: %i(blah_blah actors)

irb(main):002:0> MovieSerializer.new(movie, include: %i(blah_blah actors)).serializable_hash
Traceback (most recent call last):
        2: from (irb):28
        1: from (irb):28:in `new'
ArgumentError (blah_blah is not specified as a relationship on MovieSerializer)

As you can see, only the first relation in array truly validated.

I've tried something like this in order to handle that behavior:

def validate_includes!(includes)
  return if includes.blank?
  invalid_include_items = []

  includes.each do |include_item|
    klass = self
    
    parse_include_item(include_item).each do |parsed_include|
      relationships_to_serialize = klass.relationships_to_serialize || {}
      relationship_to_include = relationships_to_serialize[parsed_include]
      invalid_include_items << parsed_include unless relationship_to_include
      klass = relationship_to_include.serializer.to_s.constantize unless relationship_to_include.polymorphic.is_a?(Hash)
    end
  end

  raise ArgumentError, "#{invalid_include_items} are not specified as relationships on #{klass.name}" unless invalid_include_items.blank?
end
irb(main):001:0> MovieSerializer.new(movie, include: %i(blah_blah actors weird_thing)).serializable_hash
Traceback (most recent call last):
        2: from (irb):28
        1: from (irb):28:in `new'
ArgumentError (['blah_blah', 'weird_thing'] are not specified as relationships on MovieSerializer)

All rspec tests and performance checks are passed with this solution but I'm still not sure that it will not affect overall performance in comparing to single element check as it does right now.

AlexTatarnikov avatar Mar 12 '19 15:03 AlexTatarnikov

it stops iterating after a first successful block or exception obviously

A bit off-topic, but if there's any work planned on this one, it would be great to add a specific exception type instead of the generic ArgumentError.

stas avatar Mar 25 '19 16:03 stas

@stas there is planned work on this. However i see some PRs addressing this issue. I will take a look at them see if i can merge and release a new version

shishirmk avatar Mar 29 '19 04:03 shishirmk