I had a related user case at some point that led me down to path of the
passive_model_serializers
<https://github.com/chancancode/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 :P 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


On Wed, Jun 24, 2015 at 9:11 AM, Rafael Mendonça França <
rafaelmfra...@gmail.com> wrote:

> Go for it, this will improve the way different serializers hook inside
> Rails so I don't see any reason to not do so.
>
> On Wed, Jun 24, 2015 at 1:07 PM Benjamin Fleischer <bfleisc...@gmail.com>
> wrote:
>
>> I'm a little bummed no one responded to this.  Here's some new info:
>> ActiveModelSerializers already mixes in `get_serializer` to controllers.
>> <https://github.com/rails-api/active_model_serializers/blob/f67fd976ecbe41305c484e4689003758e8631998/lib/action_controller/serialization.rb#L21-L30>
>>
>> In AMS PR #954
>> <https://github.com/rails-api/active_model_serializers/pull/954> I've moved
>> the serialization logic there from _render_with_renderer_json
>> <https://github.com/rails-api/active_model_serializers/blob/728b939521321f0bbe6680d4d070a4b63a1ca404/lib/action_controller/serialization.rb#L22-L49>
>>
>>
>> - def get_serializer(resource)- @_serializer ||= @_serializer_opts
>> .delete(:serializer)- @_serializer ||= ActiveModel::Serializer
>> .serializer_for(resource)-- if @_serializer_opts.key?(:each_serializer)-
>> @_serializer_opts[:serializer] = @_serializer_opts.delete(
>> :each_serializer)+ def get_serializer(resource, options = {})+
>> serializable_resource = ActiveModel::Serializer::build(resource,
>> options) do |builder|+ if builder.serializer?+
>> builder.serialization_scope ||= serialization_scope+
>> builder.serialization_scope_name = _serialization_scope+ builder.adapter+
>> else+ resource+ end end-- @_serializer end
>>  [:_render_option_json, :_render_with_renderer_json].each do |
>> renderer_method| define_method renderer_method do |resource, options|-
>> @_adapter_opts, @_serializer_opts =- options.partition { |k, _|
>> ADAPTER_OPTION_KEYS.include? k }.map { |h| Hash[h] }-- if use_adapter? &&
>> (serializer = get_serializer(resource))-- @_serializer_opts[:scope] ||=
>> serialization_scope- @_serializer_opts[:scope_name] =
>> _serialization_scope-- # omg hax- object = serializer.new(resource,
>> @_serializer_opts)- adapter = ActiveModel::Serializer::Adapter.create(object,
>> @_adapter_opts)- super(adapter, options)
>>
>  --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Reply via email to