Skip to content

Commit

Permalink
Reverting ruby-grape#1263, closes ruby-grape#1307.
Browse files Browse the repository at this point in the history
  • Loading branch information
dblock authored and przbadu committed Apr 5, 2016
1 parent ab3999a commit b4ff94d
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 79 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
8 changes: 2 additions & 6 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
6 changes: 0 additions & 6 deletions lib/grape/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
38 changes: 38 additions & 0 deletions spec/grape/api/namespace_parameters_in_route_spec.rb
Original file line number Diff line number Diff line change
@@ -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
66 changes: 0 additions & 66 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b4ff94d

Please sign in to comment.