[RFC] Automatic serializer lookup

[beauby]

TL;DR

What should be the lookup mechanism when inferring a serializer for a resource?

Proposal

What seems to make most sense is the following:

# resource.class == ::ResNamespace::Resource
# controller.class == ::Api::V1::ResourcesController
lookup_chain = prefixes(controller.class.to_s).map { |p| "#{p}::#{resource.class}Serializer" }
# => [ "::Api::V1::ResourcesController::ResNamespace::ResourceSerializer", 
# "::Api::V1::ResNamespace::ResourceSerializer",
# "::Api::ResNamespace::ResourceSerializer",
# "::ResNamespace::ResourceSerializer" ]

# When the resource is serialized as an association of an other serializer, say SerNamespace::OtherResourceSerializer,
# add "SerNamespace::OtherResourceSerializer::ResNamespace::ResourceSerializer" at the beginning of the lookup chain 

Implemented in #1436.

Current situation

Work has been done to get us to a more flexible situation in this regard.

The whole automatic lookup can be disabled by setting ActiveModelSerializers.config.automatic_lookup = false from within an initializer.

Currently, for a resource CoolNamespace::CoolResource, AMS will look for a CoolResourceSerializer in the following namespaces:

• If it is being serialized as an association of some other resource using OtherNamespace::OtherResourceSerializer, first look for OtherNamespace::OtherResourceSerializer::CoolResourceSerializer (this is designed so that defining single-purpose serializers for associations is less cumbersome),
• otherwise, look for CoolNamespace::CoolResourceSerializer (this is kind of the "normal" lookup),
• if none is found, recursively do the same for the superclass of CoolResource.

Other possible places we might want to look at:

• the calling controller's namespace (Controller namespace lookup. #1436), i.e. when doing render json: @posts inside Api::V1::PostsController#index, look for Api::V1::PostSerializer,
• the calling controller's namespace, concatenated with the resource's namespace, i.e. when doing render json: @cool_resources inside Api::V1::CoolResourcesController#index, look for Api::V1::CoolNamespace::CoolResourceSerializer (@natematykiewicz),
• all of the calling controller's prefix namespaces (in the previous case it would be ::Api::V1, ::Api, and ::,
• the parent serializer's namespace, i.e. when serializing CoolResource as an association through OtherNamespace::OtherResourceSerializer, look for OtherNamespace::CoolResourceSerializer (so that when you do render @post, serializer: Experimental::CoolFeature::PostSerializer, the post's author can be inferred to be Experimental::CoolFeature::AuthorSerializer),
• a user-specified list of namespaces,
• possibly many other things.

Possible API for user-provided paths

• add callbacks at the beginning and end of the lookup chain

Please chip in with your ideas, use-cases and strong opinions!

[beauby]

(cc @bf4, @joaomdmoura)

[bf4]

Change to pr?

[bf4]

Mention this is controller can be disabled by a global option or by supplying adapter:nil to the render call.

[bf4]

Mention 0.9 behavior w serializer_nanespace option to limit the cases ams had to handle. Could ve an option to serialization context

[natematykiewicz]

I believe this is related to my ticket: #1337

I've had a use-case where I had CoolNamespace::CoolResource and the serializer I needed to use was Api::V1::CoolNamespace::CoolResourceSerializer because the request came from Api::V1::CoolNamespaceController. Furthermore, I wanted my resource to have a root_node of cool_resource instead of cool_namespace/cool_resource. I don't expect this to become default behavior, but it is something I've actually done.

In my case, CoolNamespace was actually the name of a third-party vendor I was interfacing with, and CoolResource was the name of one of the few resources I needed to get from/create with them. This app has a few functions, one of which being an API for dealing with third-party vendors, so that when they randomly decide to change their spec on us, we can change it in one place company-wide instead of many apps that need to talk to these vendors.

[beauby]

@natematykiewicz It is indeed (and I haven't forgotted #1337, but thought I'd make an issue to broaden the scope a bit).
I agree that Api::V1::CoolNamespace::CoolResourceSerializer would make sense as well in many situations.

[beauby]

I just edited the issue to add your suggestion. If this RFC gets a bit of traction, I'll turn it into a PR so that everyone can chip in easily.

[natematykiewicz]

@beauby my app actually isn't Api::V1::CoolResourceController#index but instead Api::V1::CoolNamespaceController#cool_resource. We chose to make a controller per vendor. Though, I could see being more RESTful and making a Api::V1::CoolNamespace::CoolResourceController instead. The thing is, these vendors are not RESTful, and don't have many actions to them, so making a single controller for each vendor felt like it made more sense than trying to break their actions into resources, which felt like overkill. But, that's why I felt like I didn't necessarily expect this to be any sort of default behavior.

Thanks for acknowledging #1337!

[beauby]

@natematykiewicz I see. Note that in the scenario I described, it does not make a difference what your controller/action is called, since only its namespace is used.

[natematykiewicz]

Oh, right. 👍🏻

[bdmac]

I think the line mentioning "all of the calling controller's prefix namespaces" addresses my use-case in #1338. Just to bring that inline here:

#1338 adds support for modifying the lookup chain for a serializer's relationships based on its own namespace. Concretely, given:

class Public::V1::PostSerializer
  belongs_to :author
end

class Public::V1::AuthorSerializer
end

we will wind up with the following items in the lookup chain for an
author when serializing a Post with Public::V1::PostSerializer:

['Public::V1::PostSerializer::AuthorSerializer',
'Public::V1::AuthorSerializer', 'Public::AuthorSerializer',
'::AuthorSerializer']

This preserves the functionality of the single-purpose serializers by leaving them highest in the chain but allows for sibling serializers that are not nested below the active serializer to also be utilized.

[natematykiewicz]

@bdmac++ Sibling lookup is actually something I would love to see as well (#1337), though your PR seems to cover even more cases that I hadn't thought of.

[beauby]

Updated my initial post with a proposed mechanism (taking into account the recommendations of those who chipped in), and updated #1436 to implement such mechanism.
In particular, it incorporates @bdmac's suggestion of prefix controller namespaces.

[bdmac]

@beauby correct me if I'm wrong but the proposal still seems to rely on the controller's namespace instead of the serializer's own namespace. Also it does not sound like sibling lookups are handled when serializing an association.

The last part of your proposal says

# When the resource is serialized as an association of an other serializer, say SerNamespace::OtherResourceSerializer,
# add "SerNamespace::OtherResourceSerializer::ResNamespace::ResourceSerializer" at the beginning of the lookup chain 

and I would expect this instead:

# When the resource is serialized as an association of an other serializer, say Public::V1::OtherResourceSerializer,
# add "Public::V1::OtherResourceSerializer::ResNamespace::ResourceSerializer" at the beginning of the lookup chain ***AND "Public::V1::ResNamespace::ResourceSerializer"***

My request/proposal also accounts for walking further up the namespaces.

[groyoh]

I'd very much appreciate it if we would make the serializer lookup configurable e.g.:

module CustomLookup
  module_function
  def self.lookup(resource, options= {})
    # some custom lookup
  end
end
ActiveModelSerializers.config.automatic_lookup = CustomLookup

As described at #879 (comment), in our code base we almost don't use Rails and our serializers have the following namespacing:

module Serializer
  module V1
    class Comment < ActiveModel::Serializer; end
    class Post < ActiveModel::Serializer
      has_many :comments, serializer: Comment
    end
  end
  module V2
    class Comment < ActiveModel::Serializer; end
    class Post < ActiveModel::Serializer
      has_many :comments, serializer: Comment
    end
  end
end

so the normal lookup doesn't work. This might completely solve the issue I described by simply creating our own custom lookup module:

module CustomLookup
  module_function
  def self.lookup(resource, options= {})
    namespace = options[:namespace]
    "#{namespace}::#{resource.class}".classify
  end
end

and then simply specify the namespace option when serializing.

[bf4]

ActiveModelSerializers.config.automatic_lookup = CustomLookup

Nice, also see Trek's issue re implicit lookup disabling

On Sun, Feb 28, 2016 at 8:31 AM Yohan Robert notifications@github.com
wrote:

I'd very much appreciate it if we would make the serializer lookup
configurable e.g.:

module CustomLookup
module_function
def self.lookup(resource, options= {})
# some custom lookup
endendActiveModelSerializers.config.automatic_lookup = CustomLookup

As described at #879 (comment)
#879 (comment),
in our code base we almost don't use Rails and our serializers have the
following namespacing:

module Serializer
module V1
class Comment < ActiveModel::Serializer; end
class Post < ActiveModel::Serializer
has_many :comments, serializer: Comment
end
end
module V2
class Comment < ActiveModel::Serializer; end
class Post < ActiveModel::Serializer
has_many :comments, serializer: Comment
end
endend

so the normal lookup doesn't work. This might completely solve the issue I
described by simply creating our own custom lookup module:

module CustomLookup
module_function
def self.lookup(resource, options= {})
namespace = options[:namespace]
"#{namespace}::#{resource.class}".classify
endend

and then simply specify the namespace option when serializing.

—
Reply to this email directly or view it on GitHub
#1442 (comment)
.

[NullVoxPopuli]

@groyoh I really like that idea. If @rails-api/ams is unable to justify implementing lookup logic by default, maybe if we provided some un-official examples in the docs of how to customize for common scenarios, that would be sufficient?

[bf4]

@groyoh I like allowing passing in namespace: as it was an option in earlier versions of AMS #1436 (comment) and is less work to add and maintain.

[NullVoxPopuli]

@bf4 that's also a good idea.

Someone could just write a mixin that retrieved the namespace and added it to the render call. ez

[prusswan]

For CoolResource not under any specific namespace (since it is the same model), should the lookup take the namespace(s) of the controllers into account? Like Api::CoolResourceController should look for Api::CoolResourceSerializer ahead of CoolResourceSerializer. I know this is not how it works right now, but I might not want to create Api::CoolResource if there is no need for it.

[natematykiewicz]

I don't know what others think, but it makes sense to me that the lookup chain would include ['Api::CoolResourceSerializer', 'CoolResourceSerializer'] (in that order).

[KendallPark]

I'm running into the automatic lookup issue as well.

I have a Api::V1::CoolResourceSerializer and a Api::V1::CoolResourceController but the resource is a plain old CoolResource.

[bdmac]

@bf4 is there any update on serializer lookup support? As things stand today on 0.10.2 I'm still in need of my monkey patched version of serializer_lookup_chain_for. I see this RFC is still open and there are seemingly many other interrelated Issues/PRs and I'm having trouble figuring out where this stands.

[NullVoxPopuli]

@bdmac there is this: #1757

[bdmac]

@NullVoxPopuli is that the final result/direction from this RFC? I notice that is not merged yet either so still unclear what the ultimate direction will be.

[NullVoxPopuli]

I'm not sure. We've had a hard time deciding on a proper API for this.

I'm in favor of one that's configured via the initializer (like in #1757).

[NullVoxPopuli]

if you have any opinions, please share! :-)

[bf4]

My own solution is rather simple: I have a method in the controller that drrives the record class and passes that in to render, so it's not a pressure I'm feeling. It's def on the todo list, just needs a simpl impl

B mobile phone

On Jul 11, 2016, at 7:11 PM, L. Preston Sego III notifications@github.com wrote:

if you have any opinions, please share! :-)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[NullVoxPopuli]

I thought my implementation was pretty simple :-\

[bdmac]

Ben that works for the top level but unless something else is wrong for me this fails to carry through to associations. The top level serializer gets the namespace fine but associations on that serializer don't respect the namespacing without my monkeypatch (which I submitted as a pr ages ago).

~ Brian McManus

On Jul 11, 2016 at 7:19 PM, <Benjamin Fleischer (mailto:notifications@github.com)> wrote:

My own solution is rather simple: I have a method in the controller that drrives the record class and passes that in to render, so it's not a pressure I'm feeling. It's def on the todo list, just needs a simpl impl

B mobile phone

On Jul 11, 2016, at 7:11 PM, L. Preston Sego III <notifications@github.com (mailto:notifications@github.com)> wrote:

if you have any opinions, please share! :-)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (#1442 (comment)), or mute the thread (https://github.com/notifications/unsubscribe/AAC43n06gDYY2_LL0yKeZLI3FhaV3bUyks5qUvnAgaJpZM4HHF5R).

[bdmac]

@NullVoxPopuli i believe that works for my case as I just wound up monkeypatching serializer_lookup_chain_for to add:

chain.push(*name.deconstantize.split('::').each_with_object([]) { |element, array| array.push((array.last ? array.last + '::' : '') + element) }.reverse.map { |k| k + "::#{serializer_class_name}" })

Pretty sure your implementation allows me to still do that but w/out monkeypatching.

[rossholdway]

Any update on this? It would be great to namespace with Api::V1::CoolResourceSerializer and Api::V1::CoolResourceController and have it work via auto lookup.

[NullVoxPopuli]

@rossholdway there is this: #1757