Handle serializer namespacing more robustly

[eprothro]

Continuing from #144 at the request of @joaomdmoura.

@MarkMurphy and others suggest automatically handling lookup context similar to ActionView. This solves two distinct issues that would provide a good solution to serializer versioning:

1. We want namespaced controllers to use namespaced serializers, without explicitly specifying a serializer or namespace in the controller
2. We want namespaced serializers to use namespaced serializers for association serialization, without explicitly specifying a serializer or namespace with the association

More info from Mark's proposal, copied:

---

If the get_serializer method here functioned in the same manner as rails does template lookups for a controller's views, then versioning would be as easy as namespacing your controllers.

For example:

# app/controllers/api/v1/users_controller.rb
module API::V1
  class UsersController < APIController
    def show
       @user = User.find(params[:id])
       render json: @user
    end
  end
end

Based on the above controllers' namespace, AMS would look for a User serializer at the following locations in order of priority:

app/serializers/api/v1/user_serializer.rb
app/serializers/api/user_serializer.rb
app/serializers/user_serializer.rb

So when api version 2 comes around you'll have the following:

# app/controllers/api/v2/users_controller.rb
module API::V2
  class UsersController < APIController
    def show
       @user = User.find(params[:id])
       render json: @user
    end
  end
end

Based on the above controllers' namespace, AMS would look for a User serializer at the following locations in order of priority:

app/serializers/api/v2/user_serializer.rb
app/serializers/api/user_serializer.rb
app/serializers/user_serializer.rb

Routes would look something like this:

# config/routes.rb
constraints subdomain: 'api' do
  namespace :api, path: nil, except: [:new, :edit], defaults: { format: 'json' } do
    # API V1
    constraints API::VersionConstraint.new(version: 1) do
      scope module: 'v1' do
        resources :users
      end
    end

    # API V2
    constraints API::VersionConstraint.new(version: 2, default: true) do
      scope module: 'v2' do
        resources :users
      end
    end
  end
end

---

Reference:

• Action View LookupContext
• Action View ViewPaths

[skalee]

I strongly disagree with this proposal.

Serializers are meant to present objects as JSONs. This includes but is not limited to rendering responses in controllers. Wider namespaces support is required.

Consider elasticsearch-model gem and how does it serialize records. You can customize it by defining #as_indexed_json in model. It is very beneficial to use serializers there and to keep them in separated namespace.

It is very important to set lookup namespace with parameter. And when you can do that, then no ActionView's goodies are necessary because specifying #default_serializer_options in base controller is so simple. You can even infer that namespace from controller class using ActiveSupport: self.class.parent.

[MarkMurphy]

@skalee

Serializers are meant to present objects as JSONs. This includes but is not limited to rendering responses in controllers.

That's what this does.

Wider namespaces support is required.

Yes. Agreed.

Consider elasticsearch-model gem and how does it serialize records. You can customize it by defining #as_indexed_json in model. It is very beneficial to use serializers there and to keep them in separated namespace.

I disagree with this approach entirely and I'm sure the community will agree that it's because of that sort of implementation that AMS was created. People were previously overriding #to_json so this approach would be no different.

[skalee]

Wow, I wanted to convict myself of premature comment but you were faster. In fact I am still pretty 0.8-minded.

When it comes to #as_indexed_json, it is elasticsearch-model's standard method, not my suggestion of how it should be used. What I want to say is that namespaces support is crippled unless it works outside controllers too.

To give you more concrete example, this is a line of code I would expect to work outside any controller:

self.as_json serialization_namespace: ::Indexing

Or at least:

"Indexing::#{self.class.name}Serializer".safe_constantize.new.as_json

Or anything more less similar, I'm not sure if #as_json or #serializable_hash still do work as they used to do in 0.8 (I think they don't).

[MarkMurphy]

@skalee

What I want to say is that namespaces support is crippled unless it works outside controllers too.

Fair point. Can you provide an example where the model would be serialized to json outside the context of a controller?

[eprothro]

@MarkMurphy I can: a worker delivering a json representation of a model object to an external service (e.g. mobile device notification).

However, the proposal works fine here, unless I'm missing something. The worker has the responsibility of determining which namespace to use for the serializer. Once that is done, association serializers are chosen with the correct namespaces.

[skalee]

@MarkMurphy Following comes from the project I was working on:

module Concerns
  module Indexable
    extend ActiveSupport::Concern

    # …

    # elasticsearch-model expects method of that name
    def as_indexed_json options={}
      options.reverse_merge! root: false, serialization_namespace: ::Indexing
      self.active_model_serializer.new(self, options).as_json
    end
  end
end

I agree this shouldn't be in models, but elasticsearch-model is pretty constraining. Actually, it worked with AMS 0.8.3 with my patches which I shared with you a little too late, apparently ;)

@eprothro As you wrote, it should preserve namespace when serializing associations. Another important thing is serializing arrays, preferrably without defining yet another array serializer identical with the stock one.

[MarkMurphy]

@eprothro thanks!

However, the proposal works fine here, unless I'm missing something. The worker has the responsibility of determining which namespace to use for the serializer. Once that is done, association serializers are chosen with the correct namespaces.

Exactly. And perhaps it's possible that the calling instance (i.e. the worker or any class for that matter) could be passed through the same serializer resolver. Who said it has to only work for Controllers?

[MarkMurphy]

@skalee Looks like what you need would just be to manually specify the namespace or serializer. AMS already allows you to do the latter, sort of...(it needs work). In any case, I agree that the developer should be allowed to be explicit in specifying what namespace to use for resolving the serializer. I would consider this to be an amendment to my initial proposal.

Example:

module Concerns
  module Indexable
    extend ActiveSupport::Concern

    # …

    # elasticsearch-model expects method of that name
    def as_indexed_json options={}
      options.reverse_merge! root: false, namespace: 'elastic_search/index/v1'
      ActiveModel::Serializer.new(self, options).as_json
    end
  end
end

In the example above lets assume that a User model includes this concern. The serializer which gets loaded would be: ElasticSearch::Index::V1::UserSerializer found in app/serializers/elastic_search/index/v1/user_serializer.rb

If not found it would then check in the following order:

1. ElasticSearch::Index::UserSerializer via app/serializers/elastic_search/index/user_serializer.rb
2. ElasticSearch::UserSerializer via app/serializers/elastic_search/user_serializer.rb
3. UserSerializer via app/serializers/user_serializer.rb

[skalee]

@MarkMurphy I don't think it should be a directory path. Firstly, serializers are regular classes, so specifying parent module seems more natural. Secondly, AMS depends on ActiveModel, not whole Rails, therefore enforcing Rails application layout does not seem appropriate.

Example:

options.reverse_merge! root: false, namespace: Elasticsearch::Index::V1

[skalee]

@MarkMurphy Is ActiveModel::Serializer.new(self, options).as_json supposed to work with current master? I mean, without namespaces support. If yes, then there's a bug.

[MarkMurphy]

@skalee no I don't think it does. That was more or less just pseudo code. A syntax I'd like to see because it seems intuitive.