Getter of has_many or has_one relationship gets called twice

[wuarmin]

Hello,
following test code illustrates the behaviour:

class Post
	attr_accessor :id, :title, :content, :likes

	def initialize(params)
		@id = params[:id]
		@title = params[:title]
		@content = params[:content]
		@likes = params[:likes]
	end

	def likes
		puts "likes called" # getter gets called twice.
		@likes
	end
end

class Like
	attr_accessor :id, :content

	def initialize(params)
		@id = params[:id]
		@content = params[:content]
	end
end

require 'jsonapi-serializers'

class PostSerializer
  include JSONAPI::Serializer

  attribute :title
  attribute :content
  has_many :likes, include_data: false 
end

class LikeSerializer
  include JSONAPI::Serializer

  attribute :content
end

post = Post.new({
	id: 1, 
	title: 'title', 
	content: 'content', 
	likes: [
		Like.new(id: 2, content: 'test'),
		Like.new(id: 3, content: 'test2')
	]
})

JSONAPI::Serializer.serialize(post, is_collection: false, include: ['likes'])

Is this the expected behaviour? Maybe an if-statement? I did not look deep in code yet.
Our problem is, that we have some time-consuming "getter"-functions and don't want to use/aren't able to use members.

Best regards!

[fotinakis]

That definitely could be a problem and one I'd love to get fixed. Let me know if you have some time to dig into it, would love some help identifying it.

[wuarmin]

Ok, currently I have less time, but I forked it already and will dig into it if I have some.

[wuarmin]

I have read through code.
There's an evaluation-call at serialize_primary(serializer.relationships) and another at find_recursive_relationships(serializer.has_one/has_many_relationship). The receiving Serializer-instances are certainly not the same. So memoizing on Serializer-instances are out of question and I think Serializeres should not be responsible for caching data anyway. Their single responsibility is the creating of JSON API-corresponding json.

In my opinion the data providers should take care of how they provide data. Currently the data-providers are the domain-objects itself and they can't be changed.

So I implemented a thin proxy-object-class(JSONAPI::ProxyObject i.e.) and such a proxy-object-instance delegates the getter-methods to the domain-objects and take care of memoizing return values for following calls. This works quite good at first sight. So I want to know what you think about my approach. If you are interested I could provide a PR.

[wuarmin]

@fotinakis I see you released a v1.0.0 👍 with some huge perf-improvements.
What do you think about the approach mentioned above. Fixing this would be also an improvement.

[fotinakis]

@wuarmin so that sounds ok, just two assumptions that it makes: 1) the proxy object will always return data correctly, ie. it's ok to memoize values, and 2) it assumes that asking objects for values is currently a performance problem.

On the second point, here is a ruby-prof profiler recording of a big API serialization response:
https://gist.github.com/timhaines/4c4df35311ad4ab365f38d56001f77fc — it does seem like things like ActiveRecord::Attribute#value, which would be called by the serializer when getting a value, are actually being called a LOT and could benefit from caching.

Someone else (@JonasBorchelt) forked this gem recently to add a "store" layer, which I'm not sure if it does exactly what you're talking about but I think it's also close: etventure@63a45a3

My big concern is backwards-compatibility, so as long as PR takes that into account I'm very interested in perf fixes.