From b4ff94da949181b3802f7a66ffa3ac1fa05144b5 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 8 Mar 2016 11:15:26 -0500 Subject: [PATCH] Reverting #1263, closes #1307. --- CHANGELOG.md | 1 - UPGRADING.md | 8 +-- lib/grape/api.rb | 6 -- .../api/namespace_parameters_in_route_spec.rb | 38 +++++++++++ spec/grape/api_spec.rb | 66 ------------------- 5 files changed, 40 insertions(+), 79 deletions(-) create mode 100644 spec/grape/api/namespace_parameters_in_route_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index d22e235e4b..16e17837dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ * [#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`, Method Not Allowed routes - [@arempe93](https://github.com/arempe93). * [#1266](https://github.com/ruby-grape/grape/pull/1266): Fix `Allow` header including `OPTIONS` when `do_not_route_options!` is active - [@arempe93](https://github.com/arempe93). * [#1270](https://github.com/ruby-grape/grape/pull/1270): Fix `param` versioning with a custom parameter - [@wshatch](https://github.com/wshatch). * [#1282](https://github.com/ruby-grape/grape/pull/1282): Fix specs circular dependency - [@304](https://github.com/304). diff --git a/UPGRADING.md b/UPGRADING.md index 607be9f1f0..b2f00a8ed4 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -38,13 +38,9 @@ caught error of type NoMethodError in after callback inside Api::Middleware::Som See [#1285](https://github.com/ruby-grape/grape/pull/1285) for more information. -#### Changes to generated OPTIONS and Method Not Allowed routes +#### Changes to Method Not Allowed routes -Using `route :any, '*path'` no longer blocks generated `OPTIONS` and Method Not Allowed methods from other routes. Use `do_not_route_options!` to prevent `OPTIONS` routes from being created. - -See [#1263](https://github.com/ruby-grape/grape/pull/1263) for more information. - -Furthermore, a `405 Method Not Allowed` error now causes `Grape::Exceptions::MethodNotAllowed` to be raised, which will be rescued via `rescue_from :all`. Restore old behavior with the following error handler. +A `405 Method Not Allowed` error now causes `Grape::Exceptions::MethodNotAllowed` to be raised, which will be rescued via `rescue_from :all`. Restore old behavior with the following error handler. ```ruby rescue_from Grape::Exceptions::MethodNotAllowed do |e| diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 13bda747c7..a6f0c8b8de 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -175,9 +175,6 @@ def generate_options_method(path, 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 @@ -191,9 +188,6 @@ def generate_not_allowed_method(path, allowed_methods, allow_header) self.class.route(not_allowed_methods, path) do fail Grape::Exceptions::MethodNotAllowed, header.merge('Allow' => allow_header) 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 diff --git a/spec/grape/api/namespace_parameters_in_route_spec.rb b/spec/grape/api/namespace_parameters_in_route_spec.rb new file mode 100644 index 0000000000..0baa341baa --- /dev/null +++ b/spec/grape/api/namespace_parameters_in_route_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Grape::Endpoint do + subject { Class.new(Grape::API) } + + def app + subject + end + + before do + subject.namespace :me do + namespace :pending do + get '/' do + 'banana' + end + end + put ':id' do + params[:id] + end + end + end + + context 'get' do + it 'responds without ext' do + get '/me/pending' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'banana' + end + end + + context 'put' do + it 'responds' do + put '/me/foo' + expect(last_response.status).to eq 200 + expect(last_response.body).to eq 'foo' + end + end +end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 8fee9aab62..7934f222ae 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -567,72 +567,6 @@ def subject.enable_root_route! expect(last_response.headers['Content-Type']).to eql 'text/plain' end - describe 'adds an OPTIONS route that' do - before do - subject.before { header 'X-Custom-Header', 'foo' } - subject.get 'example' do - 'example' - end - subject.route :any, '*path' do - error! :not_found, 404 - end - options '/example' - end - - it 'returns a 204' do - expect(last_response.status).to eql 204 - 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, GET, HEAD' - end - - it 'has a X-Custom-Header' do - expect(last_response.headers['X-Custom-Header']).to eql 'foo' - end - - it 'has no Content-Type' do - expect(last_response.content_type).to be_nil - end - - it 'has no Content-Length' do - expect(last_response.content_length).to be_nil - end - end - - 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 'contains error message in body' do - expect(last_response.body).to eq '405 Not Allowed' - 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