JSON adapter does not handle nested associations

[frahugo]

It only supports the associations in the model being serialized. For instance if you serialize Blog, it will serialize the posts, but not the comments of each post.

This line https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model/serializer/adapter/json.rb#L14 should probably be changed to get a serializable_hash instead of just attributes.

This was discovered while trying existing 0.9.x code on the master branch. Is this something that should be supported? If so, I can submit a pull request for it.

[joaomdmoura]

I actually thought about it yesterday!
I even tried to make some tests with it locally, but it didn't seem to be the right direction.

1. Defining serializable_hash as the default behaviour would result in bigger payload.
   • It's a not a good convention.
   • Taking into account Mobile First (it isn't just about screen size, network is part of it too)
2. It would make the AMS a little bit slower.
   • It could make some cache integrations easier for me but would be a lazy choice 😝

It could be implemented as an option but Imho it isn't the way to go.
Would be cool to hear some other thoughts about it @joshsmith 😄

[a5sk4s]

+1

dealing with the very same issue at the moment with an inherited app - i agree that our request is humongous, but it's cached on the mobile client as i've been told

i was expecting that I could define a nested includes and pass the object to the serializer and it would use it, but as far as i can tell now, it stops at level one, even though the active record object has everything loaded already

[joshsmith]

I think in general this is not a good convention because of the bigger payload, as you said.

Maybe as an option, but I don't think allowing for all this configuration is a good idea, especially when it runs counter to a convention.

[frahugo]

John, when you say that it runs counter to a convention, why do you mean by that? If people want to follow a convention, they could possibly go with JSONAPI instead. The issue here is more of a backward compatibility issue.

We could use the same option as JSONAPI:

render @posts, include: ['authors', 'comments']

[joshsmith]

@frahugo is right in this case.

To reduce the number of HTTP requests, servers MAY allow responses that include related resources along with the requested primary resources. Such responses are called "compound documents".

In a compound document, all included resources MUST be represented as an array of resource objects in a top-level "included" member.

[joshsmith]

@joaomdmoura is this something that's already handled by the ActiveModel::Serializer::Adapter::JsonApi?

[joshsmith]

Here's a link to reference my above quote http://jsonapi.org/format/#document-structure-compound-documents

[joshsmith]

@frahugo I think this should be handled by the JsonApi adapter, and not the Json adapter.

[joshsmith]

This adapter, according to the documentation, does the following:

render @posts, include: ['authors', 'comments']

Am I misunderstanding the issue you're having? Do you want this to also be handled by the Json adapter itself?

[joaomdmoura]

@frahugo What @joshsmith and I meant was that it is not usual and not a convention to choose big payloads including ever object related to the resource you are retrieving.

Indeed the JsonApi adapter already enables you to retrieve included resources, but it's just one level deep.

There is a work-around to this that would be define a method at your serializer.
Imo if decide to move forward on this it should be some kind of option as you said.
Would be great to hear more toughts, @kurko I choose you! Thunder Shock! 😝

[frahugo]

A work-around would be awesome. We have existing apps that expect a JSON structure as in 0.9.x. So as long as we can provide the same JSON and that caching works, that would be awesome.

Le 2015-03-16 à 21:00, João Moura notifications@github.com a écrit :

@frahugo What @joshsmith and I meant was that it is not usual and not a convention to choose big payloads including ever object related to the resource you are retrieving.

Indeed the JsonApi adapter already enables you to retrieve included resources, but it's just one level deep.

There is a work-around to this that would be define a method at your serializer.
Imo if decide to move forward on this it should be some kind of option as you said.
Would be great to hear more toughts, @kurko I choose you! Thunder Shock!

—
Reply to this email directly or view it on GitHub.

[joaomdmoura]

@frahugo the work around would be something like bellow:

Instead of use has_many :comments, you can add it as an attribute and define a method that will overlap it's value, inside this method you can mount any kind of object you want. In the example bellow I'm returning also the user that is related with the comments. do this solve your problem?

class PostSerializer < ActiveModel::Serializer
  attributes :title, :body, :comments

  def comments
    comments = []
    object.comments.each do |comment|
      comments << { content: comment.content, user: comment.user }
    end
  end

end

[frahugo]

Would it support caching? Caching is the main reason why we want to go with the latest code.

[joaomdmoura]

Hell yeah! 😄
You will find everything you need at #693
All you will need to do is add cache method:

[iMacTia]

Sorry to re-open (even if was never closed) this discussion, but I agree with @frahugo.
I just realised testing my application that nested associations serialisation stops after first level.
I Completely understand your concerns about payload size, but as @joshsmith also pointed out sometimes you just need to send a lot of information to avoid even slower multiple API calls.

That said, @joaomdmoura solution could work, but is not DRY in my opinion. First-level nested association are serialised using their own serialisers, if found, with this solution I'm actually defining how to serialise comments inside the posts serialiser...
And the situation is even worse if we have 3 or more level nested associations.

It would be great to have this as an option in the serialiser class, something like:

class PostSerializer < ActiveModel::Serializer
    has_many :comments, include: true # (or 'include_when_included', or 'always_include', or 'nested_include', ...)
end

so that if I have has_many :posts somewhere, I'll also get all comments with my post.
This would allow everyone to choose the best implementation, because we can also have 2 different serialisers for post, one with inclusion and one without inclusion.

Looking forward to hear your thoughts :)

[iMacTia]

However, for the time being, I created a new adapter called JsonCustom.
It's a copy-and-paste of the standard Json Adapter, but changing line L22 from

item.attributes(opts)

to

self.class.new(item).serializable_hash

and it actually seems to be working. No option, it ALWAYS call recursively. That would be the best approach

[iMacTia]

Line 29 also need to change into

self.class.new(association).serializable_hash

[joaomdmoura]

Hey @iMacTia, first of all thank you for contributing 😄
Indeed, the "solution" I provided, as I said, was just a workaround. I'm not in favor of always call relationships recursively but your proposal of optionally including it sounds great to me. I'd be willing to review a PR implementing it. I'm not able to implement it myself right now, I'm already busy working on other new features, but in case you can't too I'll put in on my todo list 😄

[iMacTia]

Just submitted PR #952 on the master.

Great timing for an rc2 with at least my commit and PR #936 (I REALLY need this), what do you think, @joaomdmoura ;)?

[joaomdmoura]

Tks @iMacTia! Have just checked the PR.
hahah yeah, Rails folks also are waiting for a new RC.
I'm planning a new one to the next week 😄, I just need to finish a feature that wanted to include into it. 😝

[gastonmorixe]

Needed this too.
I added a check for nested associations because I was getting multiple root keys on them.

unless options[:root_key] == false 
    { root => @result }
else
    @result
 end

https://gist.github.com/imton/a1e1ee93d469fbd9f2fd

[malroc]

+1 for nested associations.
Also, I don't think that increased payload is a problem at all: if you don't need associations somewhere, you can always create a serializer that don't have them (i.e. use different serializer classes for different cases).
I.e. there IS always a good option to exclude unnecessary associations from the output: define multiple serializer classes + uses inheritance to stay DRY.
And with the current implementation there is NO sane option to include nested associations (defining attributes instead of associations is a terrible hack).

[jiajiawang]

+1 for option to include nested associations

[trezona-lecomte]

+1 for supporting nested associations deeper than 1 level

[elliotlarson]

👍 I both agree that it's a bad idea for the default, but the option to configure it to allow nested associations would be very useful

[richmolj]

+1 Another scenario here is when you have a recursive tree structure. Looking at #968 you can actually make this work with dot-notation, though @joaomdmoura I'm not sure if this is officially supported or something that just happens to work?

render json: tree, include: ['children', 'children.children', 'children.children.children']

[bilby91]

+1

[joaomdmoura]

Yeap @richmolj it's officially supported, but I think it only work with json-API adapter.

[flybayer]

Is there anything specific to to get this working as mentioned by @richmolj? It's not working for me using the latest on master and with the json-api adapter.

render json: tree, include: ['children', 'children.children', 'children.children.children']

Do you need to define the nested relationship in the serializer?

[joaomdmoura]

@feedthebayer Nops, all you would need the the usual relationship declaration, if it does not work, could you open a new issue if more info, it might be a little details because it is supposed to work.

[NullVoxPopuli]

hey, I think #1127 resolves this.

[beauby]

So #1158 and #1127 solve this problem. If you want to serialize all defined relationships (which might be dangerous if you have cycles in your data graph), you would just provide the include: '**' option to the adapter. You could also manually specify the include tree, or just specify a level of nesting (like include: '*.*.*' for 3 levels).

Closing this for now, feel free to reopen if needed.

[nicolas-besnard]

Sorry to comment on this old thread, but this feature include: '**', doesn't seem to be documented. I can make a PR for this. Do you have a nice spot to put documentation on this ?

[bf4]

@nicolas-besnard It's actually documented in https://github.com/rails-api/active_model_serializers/blob/db6083af2fb9932f9e8591e1d964f1787aacdb37/docs/general/adapters.md#included

Do you think you can improve the docs so that you would have found it?

[nicolas-besnard]

Well, firstly, it's on the JSON API section, but it works for the JSON adapter :)

[bf4]

@nicolas-besnard Looking forward a PR :)

[dan-corneanu]

Does this actually work with the JSON API adapter ?

[bf4]

@dan-corneanu Please open a new issue. (And clarify what 'this' and 'actually' refer to)