Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Versioning Serializers #144

Closed
enrico opened this issue Nov 4, 2012 · 35 comments
Closed

Question: Versioning Serializers #144

enrico opened this issue Nov 4, 2012 · 35 comments

Comments

@enrico
Copy link

enrico commented Nov 4, 2012

Hello,

I just started playing with active model serializers.

Although my API is v1, I am trying to understand what the scenario looks like when I have to release v2.

What's the recommended way to handle different serialized attributes depending on version in active model serializers?

Right now, I'm planning to have 2 sets of namespaced API controllers, but wanted to know if it makes sense to also namespacing the serializer classes....

If I do this, will the has_many and has_one methods respect the namespace?

Does anyone have any input on what the recommended way is?

Thanks in advance.

@steveklabnik
Copy link
Contributor

Hey @enrico !

This is a complex issue with many different answers. If I were you, I'd create a version attribute and include it in your serializable_hash.

Since this is not a bug, and nobody's answered in a month, I'm giving it a close.

@juljimm
Copy link

juljimm commented Nov 20, 2012

Hi,

Sorry for not reply until now, just forgot this thread I subscribed before resolving my similar case.

I just created a custom responder who makes the right choice when selecting serializer to use:

Include in your application_controller.rb:

require 'app_responder'

/lib/app_responders.rb

require 'responders/serialized_responder'

class AppResponder < ActionController::Responder
  include Responders::SerializedResponder
end

/lib/responders/serialized_responder.rb

module Responders
  module SerializedResponder

    def initialize(controller, resources, options={})
      super
      serializer = (controller.class.name.gsub("Controller","").singularize + "Serializer").constantize
      if resource.kind_of?(ActiveRecord::Relation)
        options.merge!(arrayserializer: serializer)
      else
        options.merge!(serializer: serializer)
      end
    end

  end
end

With this you will get 2 things:

  1. Now you can have a /controllers/v1/items_controller.rb paired with /app/serailizers/v1/item_serializer.rb
  2. You don't need to add serializer: V1::ItemSerializer or arrayserializer: V1::ItemSerializer options in your respond_with sentences (DRY!)

Maybe this addition could be added to https://github.com/plataformatec/responders

Hope it helps

@blakewatters
Copy link
Contributor

Hey late to the party on this one, but I think its worth baking in some support for (optional) versioning. What I was thinking is something like this:

class WhateverSerializer < ActiveModel::Serializer
  root :whatever

  version 1 do
    attributes :id, :name, :body
  end

  # We've added some attributes here, we're just inheriting from the previous version
  version 2 do
    attributes :url, :created_at
  end

  # We've substantially changed the format, declarations here are made in an exclusive scope
  version 3, :exclusive => true do
    attributes :id, :name, :text, :something_else

    def helper_method
      # Override for v3
    end
  end

  def helper_method
    # do stuff
  end
end

You could then pass in the :version argument to the serializer and it would select the highest version greater than or equal to the version specified. You could then use a custom responder or whatever to implement the versioning determination by whatever makes sense for you app (URL param, Content-Type header, whatever).

I'd be happy to champion this work if there's interest and consensus on the API direction -- we have a similar system brewing within our application that could be generalized and polished if there's a mergeable path forward.

@vikksr
Copy link

vikksr commented Mar 20, 2013

+1

@arelenglish
Copy link

I like @blakewatters idea. Any progress on this?

@glooko
Copy link

glooko commented Jul 1, 2013

I like @blakewatters idea too. Maybe no one is reading this since the issue was closed a while back?

@elfassy
Copy link

elfassy commented Oct 25, 2013

https://github.com/hookercookerman/active_model_version_serializers
haven't used it myself, but could be what you are looking for

@elfassy
Copy link

elfassy commented Oct 25, 2013

actually, what @blakewatters is describing is much better

@plehoux
Copy link

plehoux commented Jan 23, 2014

application_controller.rb:

def default_serializer_options(options = {})
  name   = controller_name.classify
  object = "V#{params[:api_version]}::#{name}Serializer"
  options.merge(serializer: object.constantize)
end

routes.rb:

Rails.application.routes.draw do
  constraints subdomain: 'api', format: 'json' do
    scope '/v1', api_version: 1 do
      resources :messages
    end
  end
end

@elfassy
Copy link

elfassy commented Jan 23, 2014

@plehoux that won't work for has_many and has_one relationships between serializers

@gabehoffman
Copy link

I like @blakewatters idea. That looks like I want it to look. It's going to be needed for many people soon enough. I think this is a great topic to be open, not closed. You can only find this when you google the topic. It's an open issue in a very real way.

@blakewatters
Copy link
Contributor

Unfortunately I am no longer responsible for any Rails applications so its unlikely that I’ll be implementing this myself.

I still think the API design is what you really want though :-)

@MarkMurphy
Copy link

I agree this should be re-opened. Not sure I agree with @blakewatters solution. What if I had a serializer that took 100 lines of code or more to define, and what if each new version added another large block of code. What if it gets up to 15 or 20 versions long?

I'd prefer to keep things separated.

Leaning more towards @juljimm's proposal

@sethherr
Copy link

+1

@mehulkar
Copy link

mehulkar commented Sep 4, 2014

I also like @juljimm's approach. Follows the same pattern as scoping controllers.

@chadwtaylor
Copy link

+1

@kurko
Copy link
Member

kurko commented Feb 1, 2015

I do believe working with versions should be easier, but I don't think it should be in the serializer for some reasons:

  1. putting versions inside of the serializer kills any future endeavour for a HATEOAS adapter.
  2. if we totally disregard HATEOAS, a serializer should, in my opinion, just serialize, not handle versions. PostSerializer's v1 is a different beast than PostSerializer's v2. Each version should have its own class, such as V1::PostSerializer and V2::PostSerializer.
  3. Version handling should happen at the controller level. render json: @post, serializer: V1::PostSerializer. How you'd automatically load this class is open for discussion.

@MarkMurphy
Copy link

I think this should work the same way rails's controller and views work using the action view lookup context. It makes the most sense and was sort of what I expected to begin with.

https://github.com/rails/rails/blob/ec28c4fb242a9bf0632bb4dac0d0a2d949eab1b3/actionpack/lib/abstract_controller/view_paths.rb#L43

Except instead of looking up templates we'd be looking up serializers

@MarkMurphy
Copy link

@steveklabnik This should really be re-opened as a feature request/enhancement. This is pretty much why I'm still using jBuilder.

@chadwtaylor
Copy link

@MarkMurphy Agreed.

@steveklabnik My v1 is using AMS but dealing with AMS for v2 was painful, we retreated to jBuilder (just for v2 though).

+1 on reopening as a feature request/enhancement.

@saneshark
Copy link

👎 I'm of the opinion that versioning should happen with namespaces.

Controllers

class Api::V1::ApiController < ActionController::Base
end

class Api::V2::ApiController < Api::V1::ApiController
end

Routes

scope 'apis', module: 'api', defaults: {format: :json} do
   namespace 'v1' do
     resource ...
  end

  #or 

   scope 'v2/:api_key', module: 'v2' do
      resources ...
   end
end

Serializers

class V1::MySerializer < ActiveModel::Serializer
end

class V2::MySerializer < V1::MySerializer
#slightly different serializer attributes perhaps
end

@chadwtaylor
Copy link

@saneshark I tried the namespacing route, and things are not very consistent. I've posted this issue at #804.

@MarkMurphy
Copy link

@saneshark

I'm of the opinion that versioning should happen with namespaces.

I agree with you. I've set my routes and controllers up in a similar fashion. My suggestion still stands I'm not sure what your thumbs down was for.

@MarkMurphy
Copy link

I'd like to try and be more clear with what I'm suggesting. I'm saying that 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:

  1. app/serializers/api/v1/user_serializer.rb
  2. app/serializers/api/user_serializer.rb
  3. 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:

  1. app/serializers/api/v2/user_serializer.rb
  2. app/serializers/api/user_serializer.rb
  3. 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

@saneshark
Copy link

@MarkMurphy I didn't realize there was this issue when you have the associations within a serializer. It does look like it should be easier to implement in that regard. My downvote was particularly in regard to having versions designated within a serializer it self in the fashion described by @blakewatters and others as that flies in the face of how most people implement version controllers and routes. One would think serializers should work similarly, indeed what I love about this gem is that it's not acts_as_api or the myriad other gems out there that are inferior.

I had to hack my collection serializers to support mixed classes for STI. I also had to hack my standard Serializer for a few cases in which I needed a has_one relationship dynamically. I rather like the flexibility of being able to hack together solutions like these personally.

class PaginationSerializer < ActiveModel::ArraySerializer

  def initialize(object, options={})
    meta_key = options[:meta_key] || :meta
    options[meta_key] ||= {}

    options[meta_key][:total_count] = object.total_entries.try(:to_i)
    options[meta_key][:pagination] = {
        previous:     object.previous_page.try(:to_i),
        next:         object.next_page.try(:to_i),
        current:      object.current_page.try(:to_i),
        per_page:     object.per_page.try(:to_i),
        pages:        object.total_pages.try(:to_i)
    }

    #need to do this hack to make sure it works for STI subclasses as well.
    options[:each_serializer] = options[:each_serializer] || get_serializer_for(object)

    super(object, options)
  end

  private
  def get_serializer_for(klass)
    serializer_class_name = "#{klass.name}Serializer"
    serializer_class = serializer_class_name.safe_constantize

    if serializer_class
      serializer_class
    elsif klass.superclass
      get_serializer_for(klass.superclass)
    end
  end
end

So I'm thinking one could expand on that get_serializer_for(klass) to better support version namespaces, and I would agree with you as well that it should certainly be easier than overriding or hacking together a solution like this.

class ExampleSerializer < ActiveModel::Serializer
  attributes :child_id, :created_at, :updated_at, :associated_class_name, :type

  def initialize(object, options={})
    self.class.has_one(:sibling) if object.try(:sibling)
    self.class.has_one(:parent) if object.try(:parent)
    super
  end

  def filter(keys)
    keys.delete :sibling unless object.try(:reference_class) == :sibling && object.reference_id.present?
    keys.delete :parent unless object.parent_id.present?
    keys
  end

  def associated_class_name
    object.parent_id.present? ? :parent : object.reference_class
  end

end

Similarly, here I had to dynamically add in has_one relationships based on the presence of associations so as to not have to write a serializer for every single STI class and its various potential associations. Haven't ran into an issue with versioning on this one per se, but would most likely specify the serializer with the appropriate namespace required should that be necessary.

Not entirely sure if this helps you or not, but it illustrates how flexible I think AMS is to adapt for some interesting scenarios. I'd personally hate for that to change.

@chadwtaylor
Copy link

@MarkMurphy Your suggested implementation is exactly what I did.

My Api::V2::PeopleController was able to call V2::PersonSerializer successfully. Yes, there is a but...

Inside my V2::PersonSerializer, I have an association has_many :assignments, serializer: V2::AssignmentSerializer. It looked at AssignmentSerializer instead of V2::AssignmentSerializer.

That is where the problem is for me, associations are defaulted to the bottom order of priority, at least it is for me.

@MarkMurphy
Copy link

@chadwtaylor It sounds like there are two different but related issues at play. When you say you were able to call V2::PersonSerializer successfully. Do you mean that in your controller you explicitly tell it to use that serializer? If so, that is the first issue I'd liked solved. Having to specify the serializer everywhere is a large and tedious pain just to solve versioning and/or namespacing use cases.

The secondary issue but also related is the fact that your V2::PersonSerializer didn't maintain the namespace when it did a lookup for your AssignmentSerializer.

I think both of these issues could be solved the same way or similarly using what I've previously outlined.

Doing a bit more digging into how view paths are found in rails it looks like we need to build an AMS version of this:

https://github.com/rails/rails/blob/6d72485b6977804a872dcfa5d7ade2cec74c768e/actionview/lib/action_view/lookup_context.rb
also related:
https://github.com/rails/rails/blob/6d72485b6977804a872dcfa5d7ade2cec74c768e/actionview/lib/action_view/view_paths.rb

@chadwtaylor
Copy link

@MarkMurphy The only reason I explicitly tell the controller to use a certain serializer was one of my troubleshooting processes. I agree that we shouldn't need to explicitly tell it if we follow the namespacing route as it would be large and tedious as you mentioned.

I will continue to troubleshoot things and if I continue to have no luck with getting things to work with all the namespacing implementations, I'll report back here.

I appreciate your patience and support @MarkMurphy!

@eprothro
Copy link

@steveklabnik this seems to be an open issue, by most accounts; thoughts on re-opening?

@chadwtaylor @MarkMurphy appreciate the efforts and agree with y'alls direction here. Are y'all working off master (e.g. 0.10.x) or other (e.g. 0.8.x)? I'm transitioning from jbuilder to AMS and concerned about this versioning issue as well. Any update on progress, or any way I can assist?

@skalee
Copy link

skalee commented Apr 21, 2015

…so I wrote this gem: https://github.com/skalee/active_model_serializers-namespaces

It defines ::active_model_serializer for all models. However this method does not return serializer class (as it would do in stock Active Model Serializers) but a proxy object. Whenever it receives #new (as you would normally call ::new on serializer class), this proxy performs serializer class lookup and passes the call to proper class.

It integrates seamlessly without any Rails hacking. Works across associations and collections. Just one drawback: you can no longer override ::active_model_serializer in models (actually you can, but the return value must be a finder proxy, not serializer class).

Works with Active Model Serializers 0.8.x only.

@MarkMurphy
Copy link

@eprothro I haven't put forth any other effort other than outlining the potential solution. It's tough to find the time. I'm still using jbuilder and will continue to until this is resolved.

@joaomdmoura
Copy link
Member

@eprothro Indeed the best way of versioning your serializers right now would be using namespaces.
There is no easy solution here, we could try to implement something by default but we haven't given enough attention on this yet. Would be awesome if some of you propose some implementation on a new issue, would be a great point for us to start discussing it 😄 cc/ @MarkMurphy @joshsmith

@MarkMurphy
Copy link

@joaomdmoura I've already proposed an implementation in my previous comments. Essentially the same implementation rails controllers use to lookup views. Feel free to extract those into a new issue.

@eprothro
Copy link

@joaomdmoura I agree with @MarkMurphy architecturally, serializers handling lookup context to more robustly support namespaced serializers feels like the right way forward.

Opened #886 to continue any discussion in that context.

@sbonami
Copy link

sbonami commented Mar 11, 2016

@eprothro Can you update your tag to #886 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests