Place generated OPTIONS ahead of user defined routes

CHANGELOG.md

@@ -18,6 +18,7 @@
 * [#1216](https://github.com/ruby-grape/grape/pull/1142): Fix JSON error response when calling `error!` with non-Strings - [@jrforrest](https://github.com/jrforrest).
 * [#1225](https://github.com/ruby-grape/grape/pull/1225): Fix `given` with nested params not returning correct declared params - [@JanStevens](https://github.com/JanStevens).
 * [#1249](https://github.com/ruby-grape/grape/pull/1249): Don't fail even if invalid type value is passed to default validator - [@namusyaka](https://github.com/namusyaka).
+* [#1263](https://github.com/ruby-grape/grape/pull/1263): Fix `route :any, '*path'` breaking generated OPTIONS, Not Allowed routes - [@arempe93](https://github.com/arempe93)
 
 0.14.0 (12/07/2015)
 ===================

UPGRADING.md

@@ -32,6 +32,10 @@ end
 
 See [#1147](https://github.com/ruby-grape/grape/issues/1147) and [#1240](https://github.com/ruby-grape/grape/issues/1240) for discussion of the issues.
 
+#### Changes to generated OPTIONS and Not Allowed routes
+
+Using `route :any, '*path'` no longer blocks generated OPTIONS and Not Allowed methods from other routes. Use `do_not_route_options!` to prevent OPTIONS routes from being created.
+
 ### Upgrading to >= 0.14.0
 
 #### Changes to availability of DSL methods in filters

lib/grape/api.rb

@@ -146,6 +146,9 @@ def add_head_not_allowed_methods_and_options_methods
         routes.each do |route|
           methods_per_path[route.route_path] ||= []
           methods_per_path[route.route_path] << route.route_method
+
+          # using the :any shorthand produces [nil] for route methods, substitute all manually
+          methods_per_path[route.route_path] = %w(GET PUT POST DELETE PATCH HEAD OPTIONS) if methods_per_path[route.route_path].compact.empty?
         end
       end
 
@@ -157,33 +160,53 @@ def add_head_not_allowed_methods_and_options_methods
         without_versioning do
           methods_per_path.each do |path, methods|
             allowed_methods = methods.dup
+
             unless self.class.namespace_inheritable(:do_not_route_head)
               allowed_methods |= [Grape::Http::Headers::HEAD] if allowed_methods.include?(Grape::Http::Headers::GET)
             end
 
             allow_header = ([Grape::Http::Headers::OPTIONS] | allowed_methods).join(', ')
+
             unless self.class.namespace_inheritable(:do_not_route_options)
-              unless allowed_methods.include?(Grape::Http::Headers::OPTIONS)
-                self.class.options(path, {}) do
-                  header 'Allow', allow_header
-                  status 204
-                  ''
-                end
-              end
+              generate_options_method(path, allow_header) unless allowed_methods.include?(Grape::Http::Headers::OPTIONS)
             end
 
-            not_allowed_methods = %w(GET PUT POST DELETE PATCH HEAD) - allowed_methods
-            not_allowed_methods << Grape::Http::Headers::OPTIONS if self.class.namespace_inheritable(:do_not_route_options)
-            self.class.route(not_allowed_methods, path) do
-              header 'Allow', allow_header
-              status 405
-              ''
-            end
+            generate_not_allowed_method(path, allowed_methods, allow_header)
           end
         end
       end
     end
 
+    # Generate an 'OPTIONS' route for a pre-exisiting user defined route
+    def generate_options_method(path, allow_header)
+      self.class.options(path, {}) do
+        header 'Allow', allow_header
+        status 204
+        ''
+      end
+
+      # move options endpoint to top of defined endpoints
+      self.class.endpoints.unshift self.class.endpoints.pop
+    end
+
+    # Generate a route that returns an HTTP 405 response for a user defined
+    # path on methods not specified
+    def generate_not_allowed_method(path, allowed_methods, allow_header)
+      not_allowed_methods = %w(GET PUT POST DELETE PATCH HEAD) - allowed_methods
+      not_allowed_methods << Grape::Http::Headers::OPTIONS if self.class.namespace_inheritable(:do_not_route_options)
+
+      return if not_allowed_methods.empty?
+
+      self.class.route(not_allowed_methods, path) do
+        header 'Allow', allow_header
+        status 405
+        ''
+      end
+
+      # move options endpoint to top of defined endpoints
+      self.class.endpoints.unshift self.class.endpoints.pop
+    end
+
     # Allows definition of endpoints that ignore the versioning configuration
     # used by the rest of your API.
     def without_versioning(&_block)

spec/grape/api_spec.rb

@@ -555,6 +555,9 @@ def subject.enable_root_route!
         subject.get 'example' do
           'example'
         end
+        subject.route :any, '*path' do
+          error! :not_found, 404
+        end
         options '/example'
       end
 
@@ -583,13 +586,53 @@ def subject.enable_root_route!
       end
     end
 
-    it 'allows HEAD on a GET request' do
-      subject.get 'example' do
-        'example'
+    describe 'adds a 405 Not Allowed route that' do
+      before do
+        subject.before { header 'X-Custom-Header', 'foo' }
+        subject.post :example do
+          'example'
+        end
+        subject.route :any, '*path' do
+          error! :not_found, 404
+        end
+        get '/example'
+      end
+
+      it 'returns a 405' do
+        expect(last_response.status).to eql 405
+      end
+
+      it 'has an empty body' do
+        expect(last_response.body).to be_blank
+      end
+
+      it 'has an Allow header' do
+        expect(last_response.headers['Allow']).to eql 'OPTIONS, POST'
+      end
+
+      it 'has a X-Custom-Header' do
+        expect(last_response.headers['X-Custom-Header']).to eql 'foo'
+      end
+    end
+
+    context 'allows HEAD on a GET request that' do
+      before do
+        subject.get 'example' do
+          'example'
+        end
+        subject.route :any, '*path' do
+          error! :not_found, 404
+        end
+        head '/example'
+      end
+
+      it 'returns a 200' do
+        expect(last_response.status).to eql 200
+      end
+
+      it 'has an empty body' do
+        expect(last_response.body).to eql ''
       end
-      head '/example'
-      expect(last_response.status).to eql 200
-      expect(last_response.body).to eql ''
     end
 
     it 'overwrites the default HEAD request' do