RFC: Rails-API: Proposed better interface for registering serializers with the controller

Adapted from original comment at https://github.com/rails/rails/pull/19832#discussion_r30468412

Since this post is long, I’ve divided it into sections:

  1. Summary
  2. Background
  3. How Serializers Work Now
  4. How Renderers Work Now
  5. Example code changing the renderer
  6. Example code changing the serializer
  7. Proposal

Summary:

I propose there be a first-class serializer interface in Rails much like there is for renderers.

Proposed interfaces are: serializer_for and ActionController::Serializers.register

Background:

How serializers work now:

Right now, that interface is a private-looking method :_render_with_renderer_json or:_render_option_json, (e.g. see ActionController:: Serialization)

Such that creating a serializer is complicated and non-obvious

# When Rails renders JSON, it calls the json renderer method
# "_render_with_renderer_#{key = json}"
# https://github.com/rails/rails/blob/8c2c1bccccb3ea1f22503a184429d94193a1d9aa/actionpack/lib/action_controller/metal/renderers.rb#L44-L45
# which is defined in AMS https://github.com/rails-api/active_model_serializers/blob/1577969cb76309d1f48c68facc73e44c49489744/lib/action_controller/serialization.rb#L36-L37
    [:_render_option_json, :_render_with_renderer_json].each do |renderer_method      |
define_method renderer_method do |resource, options          |
super      (adapter, options)
end
    end

which is really hard to find in the source code… since it calls "_render_with_renderer_#{key}" where the key is json…

# https://github.com/rails/rails/blob/8c2c1bccccb3ea1f22503a184429d94193a1d9aa/actionpack/lib/action_controller/metal/renderers.rb#L44-L45
   def _render_to_body_with_renderer(options      )
_renderers.each do |name        |
if options.key?(name)
_process_options(options)
method_name = Renderers          ._render_with_renderer_method_name(name)
return send(method_name, options.delete(name), options)
end
      end
      nil
    end

    # A Set containing renderer names that correspond to available renderer procs.
    # Default values are <tt>:json</tt>, <tt>:js</tt>, <tt>:xml</tt>.
    RENDERERS = Set.new

    def self._render_with_renderer_method_name(key      )
"_render_with_renderer_#{key}"
    end

that is, it calls “send(:render_with_renderer_json, @model, options)” which is not greppable in the code base

(@model is the argument to json: in the controller, for example render json: @model)

How renderers work now:

How does the renderer fit into this? Well, when a controller has

render json: @model

the @model is passed to the JSON Renderer (see ActionController::Renderers default)

which basically calls json = @model.to_json(options)

Example code changing the renderer:

If, for example, I wanted to change how the JSON was rendered, to pretty print it, I would just redefine the :json renderer as below

# https://github.com/rails/rails/blob/4-2-stable/actionpack/lib/action_controller/metal/renderers.rb#L66-L128
# https://github.com/rails/rails/blob/4-2-stable//actionview/lib/action_view/renderer/renderer.rb#L32
 ActionController::Renderers.remove :json
 ActionController::Renderers.add :json do |json, options    |
if !json.is_a?(String      )
# changed from
      # json = json.to_json(options)
      # changed to
      json = json.as_json(options) if json.respond_to?(:as_json      )
json = JSON    .pretty_generate(json, options)
end

    if options[:callback      ].present?
if content_type.nil? || content_type == Mime::JSON
        self.content_type = Mime::JS
      end

      "/**/#{options[:callback]}(#{json})"
    else
      self.content_type ||= Mime::JSON
      json
end
  end

Example code changing the serializer:

But, to change how the @model is serialized, a library such as what ActiveModelSerializers overrides :_render_option_json, :_render_with_renderer_json]

to basically change @model = ModelSerializer.new(@model) so that the renderer is calling to_json/as_json on the serializer

I think this could be way better:

PROPOSAL:

  1. Renderer could have a method serializer_for that can by default returns its argument, but can be overridden in the controller
add :json do |json, options|
- json = json.to_json(options) unless json.kind_of?(String)
+ json = serializer_for(json).to_json(options) unless json.kind_of?(String)

example controller code:

def serializer_for(model  )
ActiveModel::Serializer::Adapter    .create(
ActiveModel::Serializer.serializer_for(resource).new    (resource, serializer_opts),
adapter_opts
) end

or

  1. have a serializer registry (like renderers and mime-types have), that may be called in a method just as in #1
ActionController::Serializers.register :user, UserSerializer, only: [:json]

Benefits:

  • Simple, clear, extendable interface for model serialization

  • Less meta-programming

  • I wouldn’t have needed to spend hours in the debugger and rails source code trying to find out the path from render json: @model to _render_with_renderer_json(@model, options),

Thanks for your thoughts!

-Benjamin

I know y’all on core are busy… I thought this was a reasonable proposal to improve the usability or Rails, especially as rails-api gets merged in https://github.com/rails/rails/pull/19832

Besides JBuilder and https://github.com/rails-api/active_model_serializers, I know of https://github.com/cerebris/jsonapi-resources ( which basically forces you to use their render call that wraps the model in a ‘resource serializer’ ) and https://github.com/RestPack/restpack_serializer ( which requires you wrap your model in its serializer in the controller yourself ). I think they’d all benefit from having a nice place in the request lifecycle where the how the rendered resource is serialized can be configured.

Anyhow, basically justing bumping to see if there’s interest in a PR

I’m a little bummed no one responded to this. Here’s some new info: ActiveModelSerializers already mixes in get_serializer to controllers.

In AMS PR #954 I’ve moved the serialization logic there from _render_with_renderer_json

  • def get_serializer(resource)
  • @_serializer ||= @_serializer_opts.delete(:serializer)
  • @_serializer ||= ActiveModel::Serializer.serializer_for(resource)

Go for it, this will improve the way different serializers hook inside Rails so I don’t see any reason to not do so.

I had a related user case at some point that led me down to path of the passive_model_serializers experiment.

Basically, the observation is that serializers:models is a N:1 relationship, not 1:1. So imo asking the model what its serializer should be (or as proposed, register one serializer per model type) is fundamentally problematic/flawed.

In theory, you might want a serializer (not necessarily a AM::S, but the concept in general) for exporting the “transactions” table in CSV; at the same time, you might have two different “transactions” serializers for the merchant dashboard and the admin dashboard, maybe another one for the legacy mobile app API.

Therefore, which serializer to use (in the case where you have more than one) depends on the context of the serialization, so you really should be asking the thing that is trying to serialize the model (in most case, the controller) rather than the model that is being serialized.

That’s basically the spirit of the passive_model_serializers experiment. There are more nuances to that - inside your AdminTransactionsSerializer, if you want to serialize a user, chances are you probably want to use an AdminUserSerializer as well, but that serializer in turns want to serialize another object, you need to set that up properly too, which, in the then-current AM::S API required a lot of boilerplate.

I stopped working on the project that caused my pain, so I stopped working on the experiment too :stuck_out_tongue: I’m not super happy with the state I left it at, so I never took the time to write up a proposal… (so thank you for doing that :D)

So… In the context of this proposal, between the two things you proposed, my personal preference is to keep this decision inside the controller, instead of a 1:1 model-serializer map. (Also, the “User” class -> “UserSerializer” thing worked well for simple cases, and we should keep that working; but I guess that’s an AM::S concern.)

Maybe something along the lines of…

class ActionController::Base

default implementation that ships with Rails

class ToJsonSerializer < Struct.new(:object, :options)

def serialize

object.to_json(options)

end

end

default implementation that ships with Rails

def serializer_for(object, options)

ToJsonSerializer.new(object, options)

end

end

…which can be overridden by AM::S or by yourself in your controller to do the right thing.


But at the end of the day, I’m not sure if this is really /that/ much different from what we have today.

At the high level, you are proposing that (correct me if I am wrong), when you call render format: model, some_options, we should look up an object that knows how to serialize model into “format” and get a string back. That sounds like… basically the same architecture as the current renderer set up, but with a different name than what you expected.

The renderer is basically a “serializer” that takes an object and an options hash and returns a string. The reason it is not greppable and uses meta-programming is that we allow for a huge list of formats other than json, and this is a generic set up that works well for most formats (including, imo, json). Do we only try to look up a serializer object for render json: ...? What about render xml: ...? render text: ...? So long as we don’t special-case json or a handful of things (imo, that seems like a bad idea), I think we will still end up with the same problem with metaprogramming and greppability.

So, I wonder if we should just better document this plugin API and fix any flaws and annoyances you find along the way.

What do you think?

(Thanks again for taking the time to write a detailed proposal, really appreciate it!)

Godfrey

def _render_to_body_with_renderer(options)
_renderers.each do |name|

if options.key?(name)

_process_options(options)

method_name = Renderers._render_with_renderer_method_name(name)

object = Serializers._serialize_with_serializer_json(options.delete(name))

return send(method_name, object, options)

end

end

nil

end

or a controller method that takes in the renderer name somehow (again, just for illustration of that idea)

def get_serializer(resource, options)

renderer_method_name = options.delete(:renderer_method_name)

fail if renderer_method_name.blank?

case renderer_method_name

when :json then resource.as_json

end

end

-Benjamin

Well, I’ve updated to PR to a state that I think improves on the status quo and is tested. It should be a good point for discussion.

See https://github.com/rails/rails/pull/21496 and previous discussion in this thread.

-Benjamin