Nested serializers/attributes

[beauby]

tl;dr

We need better serializer lookup in order to handle namespaces, which brings the opportunity to handle nested serializers, and introduce a friendly syntax to declare them.

---

As you may have noticed, there is currently some work being done on nested serialization. Different needs have been identified:

1. nesting some or all of the relationships (this is taken care of by Add support for wildcards in nested includes #1158 and Support nested associations for Json and Attributes adapters + Refactor Attributes adapter #1127) via the include adapter option (which allows to specify a potentially infinite (via wildcards) inclusion tree)
2. nesting some or all of the relationships, and contextually (depending on the serializer) choosing which attributes to serialize (which is taken care of by Add support for nested serializers. #1157, although not currently perfect: specifying a nested serializer forces you to redundantly specify the relationships in the include adapter option, unless you set it to **, in which case only the nested serializers are used)
3. conditionally include/exclude attributes from a serializer (for instance depending on some authorization primitive), which is being discussed in Attribute filtering for 0.10.x #1141

I think the state of 1. is satisfying.
For 2., we could either extend the include option to handle attributes as well, but this would lead to implicitly defining nested serializers, which is IMO more confusing than actually defining nested serializers (as in #1157). With #1157, nesting serializers works as follows:

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

  belongs_to :author
  AuthorSerializer < ActiveModel::Serializer
    attributes :name

  has_many :comments
  CommentSerializer < ActiveModel::Serializer
    attributes :body, :date
    belongs_to :author # This would be serialized by PostSerializer::AuthorSerializer, unless we define a PostSerializer::CommentSerializer::AuthorSerializer
  end
end

I would be in favor of introducing the following syntax:

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

  belongs_to :author do
    attributes :name
  end

  has_many :comments do
    attributes :body, :date
    belongs_to :author # This would be serialized by PostSerializer::AuthorSerializer, unless we define a PostSerializer::CommentSerializer::AuthorSerializer
  end
end

in conjunction with a default behavior of either including all nested relationships, or at least all nested relationships for which a nested serializer has been provided.

For 3., I think we should either have something like

attribute :secret_stuff, include_if: { object == current_user || current_user.admin? }

or something more like 0.8.x

def include_attributes(included) # I purposely did not use "filter" because it has a specific sense in the scope of JSON API
  if current_user.admin?
     included
  else
     included.except([:secret_stuff1, :secret_stuff2])
  end
end

or maybe both.

Questions? Suggestions? PRs?

UPDATE:
#1225 brings support for nested serializers (and automatically uses serializers in the same namespace as the resource)
#1226 brings support for the block syntax

[NullVoxPopuli]

I really like that block syntax. omg.

[bf4]

  has_many :comments do
    attributes :body, :date
    belongs_to :author # This would be serialized by PostSerializer::AuthorSerializer, unless we define a PostSerializer::CommentSerializer::AuthorSerializer
  end

anonymous class or dynamic constant set here are fine with me. I think this interface is most intuitive.

I do like always being able to hook into a method or template method like:

def include_attributes(included) # I purposely did not use "filter" because it has a specific sense in the scope of JSON API
  if current_user.admin?
     included
  else
     included.except([:secret_stuff1, :secret_stuff2])
  end
end

it's possible to use both of those...

I'm unsure about having conditional attributes include_if as it could get very complicated to use and maintain.. might be nice to use, but ultimately lead to spaghetti code that is hard to test and be a big bug-attractor. But I like some of the Rails callback options like only, except.

Perhaps we could just make ActiveModel::Callbacks
available, keep the internal interface super simple, and wish you luck

All in all, I think nesting more than two levels is an anti-pattern, so I prefer to make doing that possible, but uncomfortable. Sometimes easy is a bad thing :)

I'm happy with the current state of making it possible to nest, but not as easy as the 'golden path'.

In terms of encapsulation and maintenance, the block form is probably the easiest to implement and maintain, and nicely encapsulates one-off nested associations in an intentionally non-reusable way. It only worries me if people start putting blocks in the blocks because it looks so easy.

Much of this can also be fudged in the present state by overriding existing methods like define title; object.something_complicated; end or even def attributes(*), so I'd be curious how far we can get with just that.

Maybe we can make a test case of a models and a desired output hash, and try different implementations that get the test to pass?

[beauby]

Update: nested implicit serializers are here (#1193).

[joaomdmoura]

Nice, also in love with the block implementation.
Way nicer and easier to use!

Just one question:

 has_many :comments do
    attributes :body, :date
    belongs_to :author # This would be serialized by PostSerializer::AuthorSerializer, unless we define a PostSerializer::CommentSerializer::AuthorSerializer
end

How this look up logic works? If it didn't found PostSerializer::AuthorSerializer would it look for AuthorSerializer?

I tried to catch up with the referenced PR and Issues, but this whole nested thing seems to have chunks spread across everywhere 😄

About 3. I don't think include_if is way to go, I think we should keep the filter implementation, or some implementation similar as you proposed, referencing to 0.8.

[beauby]

@joaomdmoura It would look first for the nested serializer, then for the namespaced serializer (if the resource is inside a namespace), then for a global serializer. Possibly, we can add a lookup in the controller's namespace as well.

[beauby]

After giving it more thoughts, I'm also against include_if or similar attribute-level, as I believe this logic does not belong in AMS. Instead, I'm more in favor of a way to explicitly specify to the adapter what fields to include (and potentially exclude) for each type at the adapter level (JSON API fields style). However, we should also think about a clever way to have different flavors of serializers (PostSerializer, PostSerializerForAdmin or such) for each resource, in a way that there is as little duplication between the definitions

[joaomdmoura]

Nice @beauby. IMO no need for controller namespace look up, i think we have done enough to make it easy to use already 😄 thank you btw, great work with all this.

[NullVoxPopuli]

Controller namespace lookesup is handy for versioning serializers. Like Api::v1::___

[joaomdmoura]

Yup, it could but we can put on the end os the list, if we end up having some extra-time maybe 😄

[joaomdmoura]

This is done. I'm closing the issue, congratz to @beauby for the hard work on this.