Auto OPTIONS should not check params or call _validation hooks

CHANGELOG.md

@@ -1,10 +1,16 @@
 Next Release
 ============
 
+#### Features
+
 * [#1503](https://github.com/ruby-grape/grape/pull/1503): Allow to use regexp validator with arrays - [@akoltun](https://github.com/akoltun).
 * [#1507](https://github.com/ruby-grape/grape/pull/1507): Add group attributes for parameter definitions - [@304](https://github.com/304).
 * Your contribution here.
 
+#### Fixes
+
+* [#1505](https://github.com/ruby-grape/grape/pull/1505): Run only before hook for automatic OPTIONS - [@jlfaber](https://github.com/jlfaber).
+
 0.18.0 (10/7/2016)
 ==================
 

lib/grape/endpoint.rb

@@ -124,8 +124,8 @@ def method_name
       [options[:method],
        Namespace.joined_space(namespace_stackable(:namespace)),
        (namespace_stackable(:mount_path) || []).join('/'),
-       options[:path].join('/')
-      ].join(' ')
+       options[:path].join('/')]
+        .join(' ')
     end
 
     def routes
@@ -248,21 +248,20 @@ def run
 
         run_filters befores, :before
 
-        allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS]
-        raise Grape::Exceptions::MethodNotAllowed, header.merge('Allow' => allowed_methods) if !options? && allowed_methods
+        if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS])
+          raise Grape::Exceptions::MethodNotAllowed, header.merge('Allow' => allowed_methods) unless options?
+          header 'Allow', allowed_methods
+          cookies.write(header)
+          status 204
+          return [status, header, '']
+        end
 
         run_filters before_validations, :before_validation
         run_validators validations, request
         run_filters after_validations, :after_validation
 
-        response_object =
-          if options?
-            header 'Allow', allowed_methods
-            status 204
-            ''
-          else
-            @block ? @block.call(self) : nil
-          end
+        response_object = @block ? @block.call(self) : nil
+
         run_filters afters, :after
         cookies.write(header)
 

spec/grape/api_spec.rb

@@ -516,9 +516,15 @@ class DummyFormatClass
         end
         send(verb, '/example')
         expect(last_response.body).to eql verb == 'head' ? '' : verb
-        # Call it with a method other than the properly constrained one.
-        send(used_verb = verbs[(verbs.index(verb) + 2) % verbs.size], '/example')
-        expect(last_response.status).to eql used_verb == 'options' ? 204 : 405
+        # Call it with all methods other than the properly constrained one.
+        (verbs - [verb]).each do |other_verb|
+          send(other_verb, '/example')
+          expected_rc = if other_verb == 'options' then 204
+                        elsif other_verb == 'head' && verb == 'get' then 200
+                        else 405
+                        end
+          expect(last_response.status).to eql expected_rc
+        end
       end
     end
 
@@ -549,6 +555,7 @@ class DummyFormatClass
         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' }
+        params { requires :only_for_get }
         get
       end
 
@@ -573,6 +580,27 @@ class DummyFormatClass
       expect(last_response.headers['X-Custom-Header']).to eql 'foo'
     end
 
+    it 'runs all filters and body with a custom OPTIONS method' do
+      subject.namespace :example do
+        before            { header 'X-Custom-Header-1', 'foo' }
+        before_validation { header 'X-Custom-Header-2', 'foo' }
+        after_validation  { header 'X-Custom-Header-3', 'foo' }
+        after             { header 'X-Custom-Header-4', 'foo' }
+        options { 'yup' }
+        get
+      end
+
+      options '/example'
+      expect(last_response.status).to eql 200
+      expect(last_response.body).to eql 'yup'
+      expect(last_response.headers['Allow']).to be_nil
+      expect(last_response.headers['X-Custom-Header-1']).to eql 'foo'
+      expect(last_response.headers['X-Custom-Header-2']).to eql 'foo'
+      expect(last_response.headers['X-Custom-Header-3']).to eql 'foo'
+      expect(last_response.headers['X-Custom-Header-4']).to eql 'foo'
+      expect(last_response.headers['X-Custom-Header-4']).to eql 'foo'
+    end
+
     context 'when format is xml' do
       it 'returns a 405 for an unsupported method' do
         subject.format :xml
@@ -594,8 +622,8 @@ class DummyFormatClass
     context 'when accessing env' do
       it 'returns a 405 for an unsupported method' do
         subject.before do
-          _custom_header_1 = headers['X-Custom-Header']
-          _custom_header_2 = env['HTTP_X_CUSTOM_HEADER']
+          _customheader1 = headers['X-Custom-Header']
+          _customheader2 = env['HTTP_X_CUSTOM_HEADER']
         end
         subject.get 'example' do
           'example'
@@ -631,6 +659,9 @@ class DummyFormatClass
     describe 'adds an OPTIONS route that' do
       before do
         subject.before { header 'X-Custom-Header', 'foo' }
+        subject.before_validation { header 'X-Custom-Header-2', 'bar' }
+        subject.after_validation { header 'X-Custom-Header-3', 'baz' }
+        subject.params { requires :only_for_get }
         subject.get 'example' do
           'example'
         end
@@ -656,6 +687,14 @@ class DummyFormatClass
         expect(last_response.headers['X-Custom-Header']).to eql 'foo'
       end
 
+      it 'does not call before_validation hook' do
+        expect(last_response.headers.key?('X-Custom-Header-2')).to be false
+      end
+
+      it 'does not call after_validation hook' do
+        expect(last_response.headers.key?('X-Custom-Header-3')).to be false
+      end
+
       it 'has no Content-Type' do
         expect(last_response.content_type).to be_nil
       end
@@ -2555,13 +2594,11 @@ def static
           params: {
             'param1' => { required: true },
             'param2' => { required: false }
-          }
-        },
+          } },
         { description: 'global description',
           params: {
             'param2' => { required: false }
-          }
-        }
+          } }
       ]
     end
     it 'merges the parameters of the namespace with the parameters of the method' do
@@ -2585,8 +2622,7 @@ def static
           params: {
             'ns_param' => { required: true, desc: 'namespace parameter' },
             'method_param' => { required: false, desc: 'method parameter' }
-          }
-        }
+          } }
       ]
     end
     it 'merges the parameters of nested namespaces' do
@@ -2618,8 +2654,7 @@ def static
             'ns1_param' => { required: true, desc: 'ns1 param' },
             'ns2_param' => { required: true, desc: 'ns2 param' },
             'method_param' => { required: false, desc: 'method param' }
-          }
-        }
+          } }
       ]
     end
     it 'groups nested params and prevents overwriting of params with same name in different groups' do
@@ -2662,8 +2697,7 @@ def static
             'root_param' => { required: true, desc: 'root param' },
             'nested' => { required: true, type: 'Array' },
             'nested[nested_param]' => { required: true, desc: 'nested param' }
-          }
-        }
+          } }
       ]
     end
     it 'allows to set the type attribute on :group element' do