Merge pull request #498 from karlfreeman/fix-options-headers

Fix options headers

CHANGELOG.md

@@ -7,6 +7,7 @@ Next Release
 
 * [#492](/~https://github.com/intridea/grape/pull/492): Don't allow to have nil value when a param is required and has a list of allowed values. - [@Antti](/~https://github.com/Antti)
 * [#495](/~https://github.com/intridea/grape/pull/495): Fix `ParamsScope#params` for parameters nested inside arrays - [@asross](/~https://github.com/asross).
+* [#498](/~https://github.com/intridea/grape/pull/498): Dry up options and headers logic, allow headers to be passed to OPTIONS requests  - [@karlfreeman](/~https://github.com/karlfreeman).
 
 0.6.1
 =====

lib/grape/api.rb

@@ -514,10 +514,10 @@ def inherit_settings(other_stack)
 
     def initialize
       @route_set = Rack::Mount::RouteSet.new
+      add_head_not_allowed_methods_and_options_methods
       self.class.endpoints.each do |endpoint|
         endpoint.mount_in(@route_set)
       end
-      add_head_not_allowed_methods
       @route_set.freeze
     end
 
@@ -549,7 +549,7 @@ def cascade?
     # with a list of HTTP methods that can be called. Also add a route that
     # will return an HTTP 405 response for any HTTP method that the resource
     # cannot handle.
-    def add_head_not_allowed_methods
+    def add_head_not_allowed_methods_and_options_methods
       allowed_methods = Hash.new { |h, k| h[k] = [] }
       resources = self.class.endpoints.map do |endpoint|
         if endpoint.options[:app] && endpoint.options[:app].respond_to?(:endpoints)
@@ -559,27 +559,27 @@ def add_head_not_allowed_methods
         end
       end
       resources.flatten.each do |route|
-        allowed_methods[route.route_compiled] << route.route_method
+        allowed_methods[route.route_path] << route.route_method
       end
-      allowed_methods.each do |path_info, methods|
-        if methods.include?('GET') && !methods.include?("HEAD") && !self.class.settings[:do_not_route_head]
+      allowed_methods.each do |path, methods|
+        if methods.include?('GET') && !methods.include?('HEAD') && !self.class.settings[:do_not_route_head]
           methods = methods | ['HEAD']
         end
-        allow_header = (["OPTIONS"] | methods).join(", ")
-        unless methods.include?("OPTIONS") || self.class.settings[:do_not_route_options]
-          @route_set.add_route(proc { [204, { 'Allow' => allow_header }, []] }, {
-            path_info: path_info,
-            request_method: "OPTIONS"
-          })
+        allow_header = (['OPTIONS'] | methods).join(', ')
+        if methods.include?('OPTIONS') || !self.class.settings[:do_not_route_options]
+          self.class.options(path, {}) {
+            header 'Allow', allow_header
+            status 204
+            ''
+          }
         end
         not_allowed_methods = %w(GET PUT POST DELETE PATCH HEAD) - methods
-        not_allowed_methods << "OPTIONS" if self.class.settings[:do_not_route_options]
-        not_allowed_methods.each do |bad_method|
-          @route_set.add_route(proc { [405, { 'Allow' => allow_header, 'Content-Type' => 'text/plain' }, []] }, {
-            path_info: path_info,
-            request_method: bad_method
-          })
-        end
+        not_allowed_methods << 'OPTIONS' if self.class.settings[:do_not_route_options]
+        self.class.route(not_allowed_methods, path) {
+          header 'Allow', allow_header
+          status 405
+          ''
+        }
       end
     end
 

spec/grape/api_spec.rb

@@ -473,13 +473,15 @@ def subject.enable_root_route!
       last_response.body.should eql 'Created'
     end
 
-    it 'returns a 405 for an unsupported method' do
+    it 'returns a 405 for an unsupported method with an X-Custom-Header' do
+      subject.before { header 'X-Custom-Header', 'foo' }
       subject.get 'example' do
         "example"
       end
       put '/example'
       last_response.status.should eql 405
       last_response.body.should eql ''
+      last_response.headers['X-Custom-Header'].should eql 'foo'
     end
 
     specify '405 responses includes an Allow header specifying supported methods' do
@@ -504,14 +506,16 @@ def subject.enable_root_route!
       last_response.headers['Content-Type'].should eql 'text/plain'
     end
 
-    it 'adds an OPTIONS route that returns a 204 and an Allow header' do
+    it 'adds an OPTIONS route that returns a 204, an Allow header and a X-Custom-Header' do
+      subject.before { header 'X-Custom-Header', 'foo' }
       subject.get 'example' do
         "example"
       end
       options '/example'
       last_response.status.should eql 204
       last_response.body.should eql ''
       last_response.headers['Allow'].should eql 'OPTIONS, GET, HEAD'
+      last_response.headers['X-Custom-Header'].should eql 'foo'
     end
 
     it 'allows HEAD on a GET request' do