Provide a clear way to include nested associations and fields

[iMacTia]

The Situation

I encourage to have a read to #1333 and #1845.

Let's assume we have the current serializers:

class BlogSerializer < ActiveModel::Serializer
  attributes :id
  attribute :name, key: :title

  has_many :posts
end

class PostSerializer < ActiveModel::Serializer
  attributes :id, :title

  has_one :author
  has_one :category
end

class AuthorSerializer < ActiveModel::Serializer
  attributes :id, :name

  has_one :address 
end

class CategorySerializer < ActiveModel::Serializer
  attributes :id, :name
end

The issue

The only way to specify nested associations inclusion, at the moment, is through the controller's render include option, as shown in the current examples:

# blogs_controller.rb
render json: @blog, include: 'posts.category, posts.author.address', fields: { posts: { category: [:name], author: [:id, :name] } }

# posts_controller.rb
render json: @posts, include: 'category, author.address', fields: { category: [:name], author: [:id, :name] }

The issue is that in this case (I admit is probably a stupid example, but stay with me), when rendering a post, we always want to render also its category name, its author attributes (except for the bio) and its author address.
(@bf4, I would like to specify the fields for author's address as well, but I'm not sure about how to do it in the controller's include.)

The proposed solution

The same result could be achieved without filling controller's with common serialisation logics, moving them into serialisers. For example, the above issue could have a more elegant solution like:

class BlogSerializer < ActiveModel::Serializer
  attributes :id
  attribute :name, key: :title

  has_many :posts, include: :all
end

class PostSerializer < ActiveModel::Serializer
  attributes :id, :title

  has_one :author, exclude: :bio
  has_one :category, include: :name
end

class AuthorSerializer < ActiveModel::Serializer
  attributes :id, :name, :bio

  has_one :address, include: :all
end

class CategorySerializer < ActiveModel::Serializer
  attributes :id, :name
end

# blogs_controller.rb
render json: @blog

# posts_controller.rb
render json: @posts

To be decided

We'll probably need some team discussion together with @bf4 and @NullVoxPopuli
Open questions:

1. When should include be taken into consideration? Should we use only the ones in the class we're using to serialise? Does the above example make sense or should the Author's and Post's includes be ignored when serialising the Blog because they are on a deeper level?
2. Should we have some instance-level options as suggested by @tommiller1 here?

[NullVoxPopuli]

Some thoughts:

re: 1:
Where this gets complicated is when you want to stray from your default (say, when you want to render an aggregate index view, and you only want limited amounts of data).

the render include should for sure have the final say in what gets included. But with default include setups, how do specify that we don't want something (like if we're aggregating things for a large amount of data)?

if using a render include means that default includes are ignored, I think that traversing the tree of default includes would be viable.

re: 2:
I like the idea of having access to the instance, but I think the api could be different -- more similar to our other apis, such as

class PostSerializer < AM::S
  default_include do |object, serializer_instance|
    # whatever logic in here
  end
end

[tommiller1]

The problem that we started off with was how to avoid 'repetitive includes' in the controllers. Ex. if you have a 'Product' serializer with a has_one :price and has_one :image, you might |always| want to include those two associations since the product wont be complete without them. By default only the associations on the top-most object are serialized (to avoid cyclic dependencies) so if you are trying to include a Product through another serializer ex. "render @line_item, include: [:product]" then you would still want the product's price and image to be serialized but that wont be the case (since its nested), so you have to rewrite the render call as "render @line_item, include: ['product.price', 'product.image']" which causes unnecessary repetition wherever you want to serialize @line_item. You can bypass this issue by declaring the associations as attributes (which are always serialized even if nested) but thats a hack that we are trying to avoid. A simple solution would be to have an 'always_include' field on the associations. This field determines if the association should be included when the object in question is nested.

The solution bf4 presented with the include/exclude is solving a different problem; where you only want to include/serialize specific fields on the target object instead of serializing the whole thing. Its a great feature but its not the same as 'always_include'. If you were to add both features to AMS then an association could look like this: "has_one :price, include: [:cost, :currency], always_include: true" i.e. both features can be used separately or in tandem.

[tommiller1]

always_include: associations that should always be serialized even if the object is deeply nested.
include/exclude: specific fields on the target association that you want to include/exclude (when you dont want to serialize the whole thing).
default_include: associations that you want to include by default (without default_includes the adapter decides which associations to include, which (at least for the json adapter) is to include all top-level associations)

[NullVoxPopuli]

since acceptable values for include are pretty limited, maybe we need to be looking at fields functionality? fields will wihtelist both attributes and relationships.

[iMacTia]

I agree with both of you and I also think that we're getting some confusion here in order to allow too many different possibilities.

@tommiller1 actually got straight to the point, the issue we want to solve is that sometimes an object really makes sense only when serialised with some associations. Or simply we need those associations to always be included when that object is serialised. To get this done, the always_include/defalt_include proposed in #1845 would made the trick. (we can keep discussing if that should be a class method like me and @NullVoxPopuli thought, or an association option like @tommiller1 proposed, but that's just semantic and the class method was chosen for simplicity. We can still switch to an instance method to allow greater customization).

On the other side, @bf4 have a good point on limiting the included attributes. Because maybe we do want to always include an association, but we don't need all its attributes. In this case, you can get the behaviour @tommiller1 wants with include: :all, but you can also specify a subset of the attributes with include/exclude. This gives more flexibility tot he final user.

Another thing @bf4 doesn't probably like is the cascade-inclusion, which I think in this case would be preferable. On this matter, I would ask @NullVoxPopuli what he means exactly by "traversing the tree of default includes":

if using a render include means that default includes are ignored, I think that traversing the tree of default includes would be viable.

[NullVoxPopuli]

what he means exactly by "traversing the tree of default includes":

an example:

class PostSerializer < AMS
  default_include 'author,comments.author'
  has_many :comments
  belongs_to :author
end

class CommentSerializer < AMS
  default_include 'replies'

  belongs_to :post
  belongs_to :author
  has_many :replies
end

I'll use include notation to keep things short,

render json: post

implies: include: 'author,comments.author,comments.replies'
by traversing the tree, I mean that all the way down relationships tree, default_include is merged

render json: post, include: 'comments'

since we only specified a default, this would mean your result would simply be comments, and none of the specified default included stuff.

but if we change the default_include to some form of always_include

render json: post, include: 'comments'

would be include: 'author,comments.author,comments.replies' (unchanged)

but then if we use include_always, how do we say we don't want it? using the except option for fields?

render json: post, except: 'comments'

yielding: include: 'author'

does that help?

[iMacTia]

Yes thanks! That's a lot clearer now :)
Unfortunately I don't think there's a correct answer to this, whichever implementation we'll go for will never cover all possible cases.

I think that giving something powerful like always_include should also rely on users understanding that they simply can't override that. You'll need to use a different serialiser for that (which is not even that much difficult since you can always inherit from the standard one and override the always_include directive).

I also like much more the default_include/always_include syntax proposed discussed with @NullVoxPopuli in #1845, but would be better with @tommiller1 instance context and @bf4 fields addition.

So, trying to make everyone happy and foreseeing user cases, what do you think about:

class PostSerializer < AMS
  default_include '...'
  always_include '...'

  has_many :comments, only/include: [...]
  belongs_to :author, except/exclude: [...]
end

default_include: associations that you want to include by default (if you use json/attributes adapter, this defaults to all first-level associations). This value will be merged/overriden by the controller's render include.
always_include: associations that should always be serialized even if the object is deeply nested. This CAN'T BE OVERRIDDEN from the controller.
include/exclude: I still can't see a big difference between these and only/except. And even if there's any difference as @bf4 explained, I don't think that's clear enough. Maybe is just naming, maybe it's the api, can't tell for sure.

Both default_include and always_include can accept an optional block syntax to be evaluated at instance level as @NullVoxPopuli suggested.

class PostSerializer < AM::S
  default_include do |object, serializer_instance|
    # whatever logic in here, must return a value to be parsed as JSONAPI::IncludeDirective
  end
end

[tommiller1]

Looks good to me, PR plx.

[iMacTia]

Would like to agree with @bf4 and @NullVoxPopuli as well before coming back to #1845 and adding the association options.

[bf4]

It's okay for solutions to exist in userland :).

B mobile phone

On Jul 18, 2016, at 2:00 PM, Mattia notifications@github.com wrote:

So, trying to make everyone happy

[iMacTia]

@bf4 your thoughts on the above?

include/exclude: I still can't see a big difference between these and only/except. And even if there's any difference as @bf4 explained, I don't think that's clear enough. Maybe is just naming, maybe it's the api, can't tell for sure.

Does the difference lie on the position in the includes-tree of the serialiser?

[iMacTia]

@NullVoxPopuli @bf4 sorry for chasing you, would really like to come back working on this asap 😄

[NullVoxPopuli]

I think my preference would be have it both ways.
both the default_include and always_include, and the relationship-level options.

[bf4]

I'd like to come back too

B mobile phone

On Jul 21, 2016, at 3:51 AM, Mattia notifications@github.com wrote:

@NullVoxPopuli @bf4 sorry for chasing you, would really like to come back working on this asap 😄

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[iMacTia]

It's fine for me, we can probably split it into 2 separate PR.
#1845 can provide default_include and always_include, and then we can have a separate one for the relationship-level include.
I'm not sure I'll have the time/will to work on the latter tough, I'm not even sure about its complexity at the moment.

[bf4]

@iMacTia I need a few things clarified here to understand the needs in AMS

1. Is this for JSON API, only?
2. Why is having explicit render_options in the controller inferior to having implicit render options in the serializer?
3. Arguably the serializer's job is to map attributes and associations, not provide default rendering options
4. include is a JSON API thing. exclude would be the AMS-specific convenience api for an inverse declaration. only/except come from Rails.. and yes, it's confusing and needs discussion, see #as_json option :only Not working #1851 Add ActiveModel::Serializer#default_include #1845 (comment)

I'm trying to understand to what extent the problem is that AMS is hard to customize vs. AMS isn't doing the right thing

[iMacTia]

1. No, the idea is to have this feature on the serialiser level, so completely adapter-agnostic. In fact, I personally need it on the standard Attributes serialiser.
2. Not sure what do you mean by "inferior" here. The idea in case of conflict between controller and serialiser include directives is that controller takes precedence.
3. Totally agree on this one, but don't you have scopes in active_record to decide which associations needs to be included? Think about this as the same thing, more or less. It saves you copy-and-pasting all those record.includes(...) in controllers, just defining it in the serialiser.
4. Thanks for the clarification, I agree that's not clear enough and would like to help making it clearer. Or to just drop one of the 2, I think in most cases (especially if you use always_include) you can use only/except to get the desired result.

There's really no issue with AMS's customisation on any error with its implementation.
I just keep reading of people defining associations as attributes (that hurts), just to get them automatically included in the result. I don't think that's the right way to do it, so before doing the same I thought I could provide a new feature to correctly get the same result.

[bf4]

@iMacTia I was writing an awesome response when github magically cleared it

[bf4]

Related, I think, to #1857

@iMacTia I owe you a longer response.

[iMacTia]

@bf4 I totally understand that, it's frustrating when it happens.
If the comment is going to be too long, then is probably better to write it on a word processor and paste it at the end.

[bf4]

@iMacTia yeah.. well GitHub is good at preserving comments in progress, but I had something that must have been in the flash cache that killed it... I'll try to come back to this at lunch today

[antonioparisi]

hi guys, any update on this?

[goravbhootra]

subscribed

[iMacTia]

Update is that unfortunately default_include option has been discarded and #1845 closed.
I'm waiting for @bf4 or anyone else for a new direction on this, but I'm not sure we'll ever have one as the core team clearly stated they don't think association's inclusion should be managed at serialiser level.
So if you find yourself in this situation, you can use other approaches to get around the issue.
I strongly suggest you have a look at @robinsingh-bw comment

[nicooga]

IMHO supporting something like render json: collection, include: 'assoca, assoca.assocb' should be a core feature of the default adapter. Sure, I could switch to JSONAPI, but that also means losing the possibility of creating resources with nested assocs via accepts_nested_attributes_for which most apps [I wrote and I will probably write] rely on heavily.

[bf4]

I believe there's an adapter unification rfc that hasn't yet been implemented You can still do accepts_nested w jsonapi. Just deserialize first

…