Proper separation between the serializer class vs. the serializer instance

[christophersansone]

This issue is an extraction of the conversations in #49 and #67.

Right now, it is difficult to differentiate between what functionality belongs on the class and what belongs on the instance. Currently they both kinda act as singletons. It poses potential problems, currently and most notably when trying to implement AMS-style custom attribute methods, e.g.:

class MovieSerializer
  include FastJsonapi::ObjectSerializer

  attributes :name, :year, :name_with_year

  def name_with_year
    "#{object.name} (#{object.year})"
  end
end

In what context is name_with_year executed? What is its scope? How is the object property available within this method? And how do we avoid the performance pitfalls of AMS, that instantiates new serializers for every object being serialized?

I believe an ideal solution would be the following:

• The serializer class is responsible for containing the definition of the serializer: attributes, relationships, key transformation, etc. Basically, anything built into the serializer DSL would be stored at the class.

• The serializer instance is responsible for serializing a single object at one time. Its job is to take an object as an input and output the object's serialized representation. This way, additional methods defined on the serializer (custom attributes, custom relationships, etc.) will naturally have an object within the context of the serialization process.

• The serializer class will have methods to serialize one object or a collection of objects. The class will then create the necessary instance(s). In most cases, developers will be calling class methods to perform the serialization, and they will not be instantiating serializers themselves.

• We can achieve performance levels comparable to today by reusing the serializer instances within the serialization process, as opposed to generating new instances for every object. Prior to each iteration, we simply set the new object and clear any instance variables that custom methods may have set. From the developer's point of view, it will feel like the serializer is new each time, but without the performance cost.

Examples:

class MovieSerializer
  include FastJsonapi::ObjectSerializer

  attributes :name, :year, :name_with_year

  def name_with_year
    "#{object.name} (#{object.year})"
  end
end

MovieSerializer.to_hash(movie, options)
MovieSerializer.to_hash([ movie1, movie2 ], options)

MovieSerializer.to_json(movie, options)
MovieSerializer.to_json([ movie1, movie2 ], options)

Making some decisions along these lines will help set us up for success as we continue to build support for the entire JSON:API 1.0 spec.

[shishirmk]

@christophersansone I like the idea of having a clear guideline about what goes into the serializer class and what goes into the serializer instance. I wonder if we can provide backward compatibility to the current way of doing things so that users of the gem can upgrade without changing things around a lot.

[christophersansone]

@shishirmk Yeah I was thinking about the upgrade path too. I think we can potentially offer backwards compatibility for the short-term and provide deprecation warnings. But it seems like the sooner we implement this, the fewer people will have to change.

I'm thinking we wait until after next week's release so we don't hold it up, and then tackle this right afterwards. Does that seem reasonable?

[youroff]

I attempted to refactor fast_jsonapi because of this exact reason: I was confused with abstraction distribution across instance-class levels. But it turned out to be a complete rewrite, the result can be checked out here: https://github.com/youroff/alt_jsonapi

Core idea is that class is the place where serializer is configured and then using this configuration and fields and include params, it builds a tree of serializer instances that in its turn is used for serialization itself. It already supports custom attributes, polymorphic relationships etc.

Eventually it can be pulled in to fast_jsonapi if maintainers decide to do so. However that are number of architectural decisions that are different with fast_jsonapi and might be incompatible.

• Inheritance of serializers is explicitly forbidden, because it's reserved for PolymorphicSerializer. Though it's technically possible to make so that regular serializers were supporting inheritance.
• It doesn't know anything about ActiveRecord and its conventions (like relation_id / _ids), so even without inclusion of relationships, they must be preloaded to build id hashes. The reasons are described in documentation.
• There is no internal caching, and honestly I don't see the right place and way to put it there.

So please try it and let me know if you find any issues and/or get ideas how to make it better! :)

[christophersansone]

@shishirmk I feel like we are starting to be blocked by the amount of divergence between dev and master. I see there is a v1.1 milestone, but I'm wondering if we can help speed up development and issue resolution if we release a smaller version bump like 1.0.18. If there are no breaking changes between dev and master, can we do this? If there are breaking changes, can we consider reducing the scope of v1.1 and push some of the less complete features to a future version?

[shishirmk]

@christophersansone yes we are. I think we can release v1.1 with what we have in dev now. It is not ideal but i think it's a good next step. I would like to document some known issues before we do the release.

There is a minor breaking change hence i would like to bump it to v1.1.0. We dont support use_hyphen anymore. It is being replaced by key_transform.

@mrryanjohnston is testing parts of the dev branch. I would like to test the branch on some of our internal projects. Once it looks good. I am planning to release v1.1.0

[christophersansone]

@shishirmk Awesome, thanks. If use_hyphen is the only breaking change, would it be better to deprecate it rather than remove it all together? The implementation can just call the new key transform method and serve a deprecation warning. It would be simple to do, and we can remove it in a future release that will have more significant breaking changes that we can't easily deprecate.

For upcoming releases, it will help to tag PRs with "breaking change" when appropriate, and perhaps require the author to update a CHANGELOG to document the code changes the users need to implement upon release.

[shishirmk]

Ya i agree lets just add support for use_hyphen and we can just release 1.0.18

[christophersansone]

@shishirmk Thanks for getting 1.1 out the door! It seems like this is probably the next big blocker to address before some of the other features can be implemented, right? Want me to give this a shot this week?

[shishirmk]

@christophersansone ya i think after we address #79. This would be a good thing to to tackle.