[Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Benjamin Fleischer]

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

-- 
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.

[Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Benjamin Fleischer]

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)-- 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)- else- super(resource, options)- end+ serializable_resource = 
get_serializer(resource, options)+ super(serializable_resource, options) end 
end

Thanks for any eyeballs on this.  I want to help!

-Benjamin


On Thursday, June 11, 2015 at 12:34:55 AM UTC-5, Benjamin Fleischer wrote:
>
> 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
>

-- 
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.

Re: [Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Rafael Mendonça França]

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 
wrote:

> 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)-- 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.

Re: [Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Godfrey Chan]

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 :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 
> wrote:
>
>> 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)-- if @_serializer_opts.key?(:each_serializer)-
>> @_serializer_opts[:serializer] = @_serializer_opts.delete(
>> :each_serializer)+ def get

Re: [Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Benjamin Fleischer]

On Wed, Jun 24, 2015 at 12:42 PM, Godfrey Chan  wrote:

> 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.


Thanks Godfrey, I agree with you 100% here.  I was thinking of refining
this to something that took the render used into account.  I'm not exactly
proposing the bolded line there, but something like that would extend the
current logic.  In any case, sounds like a PR to document this would at
least be useful.  I'll take a look at your code and see what ideas are in
there

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

-- 
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.

Re: [Rails-core] Re: RFC: Rails-API: Proposed better interface for registering serializers with the controller

[Benjamin Fleischer]

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

-- 
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.