Rails API Versioning, Routing issue

Question

I'm trying to implement a simple rails api app with versioning, based on Railscasts #350 episode. I wanted to access a specific API version of the app by providing the version through Accept header in this format: application/vnd.rails5_api_test.v1. And when no Accept header is provided the request will be routed to the current default version of the app. To handle this I have created an api_constraints file in my lib directory which is to be required in routes.

I have created two versions of the app v1 and v2 in which, v1 has the Users resource and v2 has Users and Comments resources. Everything was working as expected, except for when I request URL localhost:3000/comments by passing version 1 through the headers using Postman, I'm getting the response from the comments resource, displaying all the comments. But I'm expecting the response to be status: 404 Not Found, as the comments resource was in version 2 and the requested version is 1.

This is the response from the server:

Started GET "/comments" for 127.0.0.1 at 2016-04-01 20:57:53 +0530
Processing by Api::V2::CommentsController#index as application/vnd.rails5_api_test.v1
  Comment Load (0.6ms)  SELECT "comments".* FROM "comments"
[active_model_serializers]   User Load (0.9ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
[active_model_serializers] Rendered ActiveModel::Serializer::CollectionSerializer with ActiveModelSerializers::Adapter::JsonApi (4.32ms)
Completed 200 OK in 7ms (Views: 5.0ms | ActiveRecord: 1.5ms)

Here are my working files:
Constraints file, lib/api_constraints.rb:

class APIConstraints
  def initialize(options)
    @version = options[:version]
    @default = options[:default]
  end

  def matches?(req)
    req.headers["Accept"].include?(media_type) || @default
  end

  private

  def media_type
    "application/vnd.rails5_api_test.v#{@version}"
  end
end

Routes file, config/routes.rb:

Rails.application.routes.draw do
  require "api_constraints"

  scope module: 'api/v1', constraints: APIConstraints.new(version: 1) do
    resources :users
  end

  scope module: 'api/v2', constraints: APIConstraints.new(version: 2, default: true) do
    resources :users
    resources :comments
  end
end

Users controller for v1, api/v1/users_controller.rb:

class Api::V1::UsersController < ApplicationController

  def index
    @users = User.all

    render json: @users, each_serializer: ::V1::UserSerializer
  end
end

Users controller for v2, api/v2/users_controller.rb:

class Api::V2::UsersController < Api::V1::UsersController

  def index
    @users = User.all

    render json: @users, each_serializer: ::V2::UserSerializer
  end

end

Comments controller for v2, api/v2/comments_controller.rb:

class Api::V2::CommentsController < ApplicationController

  def index
    @comments = Comment.all

    render json: @comments, each_serializer: ::V2::CommentSerializer
  end
end

user serializer for v1, user_serializer.rb:

class V1::UserSerializer < ActiveModel::Serializer
  attributes :id, :name, :email
end

user serializer for v2, user_serializer.rb:

class V2::UserSerializer < V1::UserSerializer

  has_many :comments
end

comments serializer for v2, comment_serializer.rb:

class V2::CommentSerializer < ActiveModel::Serializer
  attributes :id, :description

  belongs_to :user
end

I have tried removing the default: true option in the routes and then it is working as expected. But I want it to work with default option.

Could anyone please let me know where I'm getting this wrong and also share your thoughts on this approach. If this one is not the best way to do, then guide me through the correct way of implementing it. Thanks in advance for anyone who takes time in helping me out. :) Cheers!

Answer 1

I don't think this can be solved easily as the v1 doesn't have comments the v2 will be matched regardless.

Is you APIConstraints using this method?

  def matches?(req)
    @default || req.headers['Accept'].include?("application/vnd.example.v#{@version}")
  end

I think the method is a bit too loose here and should look like this to ignore requests that do have a version.

  def matches?(req)
    (@default &&
         req.headers['Accept'].grep(/^application/vnd.example.v\d+/$).empty?
    ) || req.headers['Accept'].include?("application/vnd.example.v#{@version}")
  end