Merge pull request #1359 from ruby-grape/fix-router-bugs

Fix router bugs

CHANGELOG.md

@@ -10,6 +10,8 @@
 #### Fixes
 
 * [#1357](https://github.com/ruby-grape/grape/pull/1357): Don't include fixed named captures as route params - [@namusyaka](https://github.com/namusyaka).
+* [#1359](https://github.com/ruby-grape/grape/pull/1359): Return 404 even if version is using as header - [@namusyaka](https://github.com/namusyaka).
+* [#1359](https://github.com/ruby-grape/grape/pull/1359): Avoid evaluating the same route twice - [@namusyaka](https://github.com/namusyaka), [@dblock](https://github.com/dblock).
 
 0.16.1 (4/3/2016)
 =================

lib/grape/router.rb

@@ -41,7 +41,8 @@ def associate_routes(pattern, options = {})
 
     def call(env)
       with_optimization do
-        identity(env) || rotation(env) { |route| route.exec(env) }
+        response, route = identity(env)
+        response || rotation(env, route)
       end
     end
 
@@ -54,26 +55,28 @@ def recognize_path(input)
     private
 
     def identity(env)
-      transaction(env) do |input, method, routing_args|
+      route = nil
+      response = transaction(env) do |input, method, routing_args|
         route = match?(input, method)
         if route
           env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
           route.exec(env)
         end
       end
+      [response, route]
     end
 
-    def rotation(env)
-      transaction(env) do |input, method, routing_args|
-        response = nil
-        routes_for(method).each do |route|
-          next unless route.match?(input)
-          env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
-          response = yield(route)
-          break unless cascade?(response)
-        end
-        response
+    def rotation(env, exact_route = nil)
+      response = nil
+      input, method, routing_args = *extract_required_args(env)
+      routes_for(method).each do |route|
+        next if exact_route == route
+        next unless route.match?(input)
+        env[Grape::Env::GRAPE_ROUTING_ARGS] = make_routing_args(routing_args, route, input)
+        response = route.exec(env)
+        break unless cascade?(response)
       end
+      response
     end
 
     def transaction(env)
@@ -124,6 +127,8 @@ def greedy_match?(input)
     def method_not_allowed(env, methods, endpoint)
       current = endpoint.dup
       current.instance_eval do
+        namespace_inheritable_to_nil(:version)
+        @lazy_initialized = false
         run_filters befores, :before
         @method_not_allowed = true
         @block = proc do

spec/grape/integration/version_header_spec.rb

@@ -0,0 +1,80 @@
+require 'spec_helper'
+
+describe Grape::API::Helpers do
+  module PatchHelpersSpec
+    class PatchPublic < Grape::API
+      format :json
+      version 'public-v1', using: :header, vendor: 'grape'
+
+      get do
+        { ok: 'public' }
+      end
+    end
+
+    module AuthMethods
+      def authenticate!
+      end
+    end
+
+    class PatchPrivate < Grape::API
+      format :json
+      version 'private-v1', using: :header, vendor: 'grape'
+
+      helpers AuthMethods
+
+      before do
+        authenticate!
+      end
+
+      get do
+        { ok: 'private' }
+      end
+    end
+
+    class Main < Grape::API
+      mount PatchPublic
+      mount PatchPrivate
+    end
+  end
+
+  def app
+    PatchHelpersSpec::Main
+  end
+
+  context 'default' do
+    it 'public' do
+      get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
+      expect(last_response.status).to eq 200
+      expect(last_response.body).to eq({ ok: 'public' }.to_json)
+    end
+
+    it 'private' do
+      get '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
+      expect(last_response.status).to eq 200
+      expect(last_response.body).to eq({ ok: 'private' }.to_json)
+    end
+
+    it 'default' do
+      get '/'
+      expect(last_response.status).to eq 200
+      expect(last_response.body).to eq({ ok: 'public' }.to_json)
+    end
+  end
+
+  context 'patch' do
+    it 'public' do
+      patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-public-v1+json'
+      expect(last_response.status).to eq 405
+    end
+
+    it 'private' do
+      patch '/', {}, 'HTTP_ACCEPT' => 'application/vnd.grape-private-v1+json'
+      expect(last_response.status).to eq 405
+    end
+
+    it 'default' do
+      patch '/'
+      expect(last_response.status).to eq 405
+    end
+  end
+end