diff --git a/CHANGELOG.md b/CHANGELOG.md index 70b91a4cb..21fe02eee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#1776](https://github.com/ruby-grape/grape/pull/1776): Validate response returned by the exception handler - [@darren987469](https://github.com/darren987469). * [#1787](https://github.com/ruby-grape/grape/pull/1787): Add documented but not implemented ability to `.insert` a middleware in the stack - [@michaellennox](https://github.com/michaellennox). * [#1788](https://github.com/ruby-grape/grape/pull/1788): Fix route requirements bug - [@darren987469](https://github.com/darren987469), [@darrellnash](https://github.com/darrellnash). +* [#1774](https://github.com/ruby-grape/grape/pull/1774): Change HTTP delete status code and response body logic. Fix WEBrick compatibility - [@basjanssen](https://github.com/basjanssen). ### 1.1.0 (8/4/2018) @@ -667,4 +668,4 @@ ### 0.1.0 (11/13/2010) -* Initial public release - [@mbleigh](https://github.com/mbleigh). +* Initial public release - [@mbleigh](https://github.com/mbleigh). \ No newline at end of file diff --git a/UPGRADING.md b/UPGRADING.md index 6edbf2ad1..49b98100b 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,5 +1,9 @@ Upgrading Grape =============== +### Upgrading to >= ? +#### Changes in HTTP Delete Response Body and HTTP status code +Previously, using `body false` or returning an empty Hash/Array would result in a empty response body and HTTP 204 (No content). +Currently, returning an empty Hash or Array will return the corresponding json/xml string and HTTP 200. Calling `body nil` or setting the response body to `nil` or using `return_no_content` will return an empty response body and HTTP 204. ### Upgrading to >= 1.1.1 diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index b4672bb97..c3ebfeb09 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -178,10 +178,10 @@ def status(status = nil) when Grape::Http::Headers::POST 201 when Grape::Http::Headers::DELETE - if @body.present? - 200 - else + if @body.nil? 204 + else + 200 end else 200 @@ -222,12 +222,17 @@ def cookies # end # # GET /body # => "Body" - def body(value = nil) - if value - @body = value - elsif value == false - @body = '' - status 204 + def body(*value) + raise ArgumentError 'you can only set a body with one argument' if value.length > 1 + + if value.length == 1 + value = value.first + if value.nil? + @body = '' + status 204 + else + @body = value + end else @body end @@ -244,7 +249,7 @@ def body(value = nil) # DELETE /12 # => 204 No Content, "" def return_no_content status 204 - body false + body nil end # Allows you to define the response as a file-like object. diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index df58f88e0..eb63aa30d 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -3585,10 +3585,10 @@ def before end context 'body' do - context 'false' do + context 'nil' do before do subject.get '/blank' do - body false + body nil end end it 'returns blank body' do diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 928510f75..90dad08ff 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -111,11 +111,40 @@ def initialize expect(subject.status).to eq 204 end - it 'defaults to 200 on DELETE with a body present' do - request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) - subject.body 'content here' - expect(subject).to receive(:request).and_return(request) - expect(subject.status).to eq 200 + describe 'what is regarded as content on DELETE' do + it 'regards a string as content' do + request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) + subject.body 'content here' + expect(subject).to receive(:request).and_return(request) + expect(subject.status).to eq 200 + end + + it 'regards an empty string as content' do + request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) + subject.body '' + expect(subject).to receive(:request).and_return(request) + expect(subject.status).to eq 200 + end + + it 'regards an empty hash as content' do + request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) + subject.body({}) # Use brackets, because it's a block otherwise + expect(subject).to receive(:request).and_return(request) + expect(subject.status).to eq 200 + end + + it 'regards an empty array as content' do + request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) + subject.body [] + expect(subject).to receive(:request).and_return(request) + expect(subject.status).to eq 200 + end + + it 'regards a nil as no content' do + Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE')) + subject.body nil + expect(subject.status).to eq 204 + end end it 'returns status set' do @@ -184,9 +213,9 @@ def initialize end end - describe 'false' do + describe 'nil' do before do - subject.body false + subject.body nil end it 'sets status to 204' do diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 5614eea76..c7e3b3f85 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -1320,18 +1320,18 @@ def memoized end end - describe 'delete 204, with nil has return value (no explicit body)' do + describe 'delete 200, with empty string as return value' do it 'responds to /example delete method' do - subject.delete(:example) { nil } + subject.delete(:example) { '' } delete '/example' - expect(last_response.status).to eql 204 + expect(last_response.status).to eql 200 expect(last_response.body).to be_empty end end - describe 'delete 204, with empty array has return value (no explicit body)' do + describe 'delete 204, with nil as return value (no explicit body)' do it 'responds to /example delete method' do - subject.delete(:example) { '' } + subject.delete(:example) { nil } delete '/example' expect(last_response.status).to eql 204 expect(last_response.body).to be_empty