RFC - Refactor (Part 1) to get all the adapters using the same API

[NullVoxPopuli]

• Start Date: (2016-07-13)
• RFC PR: n/a (yet)

Summary

The goal here is to redesign the serializer <-> adapter interaction such that there is minimum effort in formatting the output. Adapters would then only really be responisble for formatting, and it may be more appropriate to name them Formatters

Motivation

There is inconsistency with what adapters implement what features. I feel like this is confusing for people.

Current Issues with adapters:

• repeated code:
  • include logic
  • building relationships
  • building the attributes hash (from fields whitelisting, for example)
• all the logic for retrieving fields from the cache is different per adapter
• JSON and Attributes are missing features that JSON API offers
  • If adapters are what we intend them to be, they'll all be just formatters... sort of...

Detailed design

in-progress, needs discussion.

Adapters should have an API modeled after JSON API

• data
  • primary serializer / resource(s)
• included
  • flat list -- these objects would have the AR integration that doesn't make DB queries to belongs_to records... but in here is where the include logic would be handled, so we don't have to deal with it in the individual adapters.
• meta
• links
• errors

A lot of these are already accessible to the serializer, but we've been relying on the adapter to traverse all of it.

All of the relationships and everything should already be traversed before getting to the adapter, so the adapter can just focus on how to format the output

An adapter would be responsible for converting each of these structures into whatever format it wishes.

Drawbacks

a lot of work, would invalidate / cause major conflicts for nearly all outstanding PRs.

Alternatives

deal with things as they are.

Unresolved questions

• what needs to happen first?
• why is caching so much of a problem?
• disadvatages of not evaluating values of anything in the serializer (lazy evaluation of attributes, relationships, etc)?

[bf4]

? https://github.com/rails-api/active_model_serializers/blob/master/docs/rfcs/template.md

[NullVoxPopuli]

I didn't know that existed, thanks!

[bf4]

repeated code

• serializers have an attributes methods. It executes read_attribute_for_serialization on each attribute in self.class._attributes.
  • the list of serialized attributes can be limited by passing in an argument options[:fields]
• serializers have a list of defined associations
  • for each associated, a serializer is found, and we call attributes on it.
    • we pass in to attributes options[:includes][association_name], see serializer for what this does, like fields

If adapters are what we intend them to be, they'll all be just formatters... sort of...

yup, basically

• Attributes Adapter
  • given a serializer, passes options into attributes and then into associations. Associations use a key. This is basically the behavior of an active model /active record object's serializable_hash method it gets from ActiveModel::Serializers::JSON and ActiveModel::Serialization.
• JSON adapter
  • is basically exactly the same as attributes adapter when ActiveModel::Serializers::JSON.include_root_in_json is true or when root is passed in. See https://github.com/rails/rails/blob/4-2-stable/activemodel/lib/active_model/serializers/json.rb#L88-L93
• JSON API adapter
  • by default associations, called relationships are not serialized unless included.
  • two defaults are available for attributes when no relationships are included: return just resource_object_identifier or full resource_object.
• JSON API provides other words that other adapters may use: filter, sort and some recommendations for pagination
• Words from ActiveModel::Serialization, which admittedly predates AMS and is all that remains of it in Rails (it was removed before 3.2 was released), are :only, :except, :methods, :include, and :root
  • These words should be supported as first class citizens in JSON/Attributes adapters, but not in JSON API, I think

all the logic for retrieving fields from the cache is different per adapter

caching should be its own layer. This is a bug. See my incomplete starts to fix this 9477fb2^...bf4:remove_caching 9477fb2^...bf4:remove_caching_only_knuckles 155033e^...bf4:collection_caching

[bf4]

We'll also want to discuss other concerns, not totally related, but on my mind:

• Using a serializer on it's own
• Trying to serialize a serializer
• The default serializer like in 0.8 and in 0.9 the default_serializer, serializable module, and serializer support
• Look at what options/design/words was in 0.8/0.9 and in other libs, just like in JSON API Resources and how this relates to active record sql queries
• To what extent JSON API concerns should exist in serializers, like type see root confusion Exclude namespace in root key in 0.10? #1756 (comment) Add documentation for #root #1794
• How this might relate to re-adding schema Re-add Serializer#schema #1430
• namespacing issues? [RFC] Automatic serializer lookup #1442 (comment)

[NullVoxPopuli]

@bf4, updated with the RFC template.

[NullVoxPopuli]

So that it's recorded, I'm going to start with some PRs of refactoring some of the smaller things, such as include, and fields, and see where that takes me

[bf4]

@NullVoxPopuli can you turn this into a PR?

[NullVoxPopuli]

I think so. Should I wait on your caching removal PR / any other PRs?

[bf4]

@NullVoxPopuli no, and the caching removal pr is really mean to refactor caching. I'll push the tests to it to make that more clear

[richmolj]

Spent some time looking at the JSON API adapter yesterday and here is an issue I see.

Attributes adapter has a serializer-calls-serializers pattern. One serializer serializes itself and its relationships, turtles all the way down.

JSON API adapter recurses over each relationship instead. The parent serializer doesn't serialize his children, the adapter contains this logic.

This is confusing code, but actually makes a kind of sense. The Attributes adapter returns relations as a nested payload - so each serializer can just keep nesting. But JSON API sideloads everything in included - it's not nested data. This means there has to be a memo somewhere keeping track of all the relationships so they can eventually be output to included.

Currently that memo is on the adapter. The alternative would be sticking it on the parent serializer, but then you could make the case A) adapter (formatter) logic is leaking into the serializers, B) JSONAPI-specific logic is leaking into areas not specific to JSONAPI.

You could also follow the JSONAPI adapter's pattern in every adapter, it just means the additional complexity that comes with that pattern. You'd more or less be taking something simple, converting it to something more complex, then formatting it back as something simple again.

[bf4]

@richmolj

Attributes adapter has a serializer-calls-serializers pattern. One serializer serializes itself and its relationships, turtles all the way down.

JSON API adapter recurses over each relationship instead. The parent serializer doesn't serialize his children, the adapter contains this logic.

I kind of consider this a bug in the divergence of how serializers are used. However, to be fair-ish to history, it was my work that pushed most of the 'Attributes' code back into the serializer.