Merge pull request #1465 from jsteinberg/better-fix-method-not-allowed

Fix filters being called multiple times, consistent intialization for…

Author: dblock

CHANGELOG.md

@@ -14,6 +14,7 @@
 
 #### Fixes
 
+* [#1465](https://github.com/ruby-grape/grape/pull/1465): Fix 'before' being called twice when using not allowed method - [@jsteinberg](https://github.com/jsteinberg).
 * [#1446](https://github.com/ruby-grape/grape/pull/1446): Fix for `env` inside `before` when using not allowed method - [@leifg](https://github.com/leifg).
 * [#1438](https://github.com/ruby-grape/grape/pull/1439): Try to dup non-frozen default params with each use - [@jlfaber](https://github.com/jlfaber).
 * [#1430](https://github.com/ruby-grape/grape/pull/1430): Fix for `declared(params)` inside `route_param` - [@Arkanain](https://github.com/Arkanain).

lib/grape/endpoint.rb

@@ -248,9 +248,12 @@ def run
 
         run_filters befores, :before
 
+        allowed_methods = env[Grape::Env::GRAPE_METHOD_NOT_ALLOWED]
+        raise Grape::Exceptions::MethodNotAllowed, header.merge('Allow' => allowed_methods) if allowed_methods
+
         run_filters before_validations, :before_validation
 
-        run_validators validations, request unless env[Grape::Env::GRAPE_METHOD_NOT_ALLOWED]
+        run_validators validations, request
 
         run_filters after_validations, :after_validation
 

lib/grape/router.rb

@@ -133,24 +133,8 @@ def greedy_match?(input)
     end
 
     def method_not_allowed(env, methods, endpoint)
-      env[Grape::Env::GRAPE_METHOD_NOT_ALLOWED] = true
-      current = endpoint.dup
-      current.instance_eval do
-        @env = env
-        @header = {}
-
-        @request = Grape::Request.new(env)
-        @params = @request.params
-        @headers = @request.headers
-
-        @lazy_initialized = false
-        lazy_initialize!
-        run_filters befores, :before
-        @block = proc do
-          raise Grape::Exceptions::MethodNotAllowed, header.merge('Allow' => methods)
-        end
-      end
-      current.call(env)
+      env[Grape::Env::GRAPE_METHOD_NOT_ALLOWED] = methods
+      endpoint.call(env)
     end
 
     def cascade?(response)

spec/grape/api_spec.rb

@@ -543,6 +543,36 @@ class DummyFormatClass
       expect(last_response.headers['X-Custom-Header']).to eql 'foo'
     end
 
+    it 'runs only the before filter on 405 bad method' do
+      subject.namespace :example do
+        before            { header 'X-Custom-Header', 'foo' }
+        before_validation { raise 'before_validation filter should not run' }
+        after_validation  { raise 'after_validation filter should not run' }
+        after             { raise 'after filter should not run' }
+        get
+      end
+
+      post '/example'
+      expect(last_response.status).to eql 405
+      expect(last_response.headers['X-Custom-Header']).to eql 'foo'
+    end
+
+    it 'runs before filter exactly once on 405 bad method' do
+      already_run = false
+      subject.namespace :example do
+        before do
+          raise 'before filter ran twice' if already_run
+          already_run = true
+          header 'X-Custom-Header', 'foo'
+        end
+        get
+      end
+
+      post '/example'
+      expect(last_response.status).to eql 405
+      expect(last_response.headers['X-Custom-Header']).to eql 'foo'
+    end
+
     context 'when format is xml' do
       it 'returns a 405 for an unsupported method' do
         subject.format :xml