Skip to content

Use 200 as default status for deletes that reply with content #1550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#### Fixes

* [#1548](https://github.com/ruby-grape/grape/pull/1548): Avoid failing even if given path does not match with prefix - [@thomas-peyric](https://github.com/thomas-peyric), [@namusyaka](https://github.com/namusyaka).
* [#1550](https://github.com/ruby-grape/grape/pull/1550): Use 200 as default status for deletes that reply with content - [@jthornec](https://github.com/jthornec).
* Your contribution here.

### 0.19.0 (12/18/2016)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1764,7 +1764,7 @@ cookies.delete :status_count, path: '/'

## HTTP Status Code

By default Grape returns a 201 for `POST`-Requests, 204 for `DELETE`-Requests and 200 status code for all other Requests.
By default Grape returns a 201 for `POST`-Requests, 204 for `DELETE`-Requests that don't return any content, and 200 status code for all other Requests.
You can use `status` to query and set the actual HTTP Status Code

```ruby
Expand Down
32 changes: 32 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
Upgrading Grape
===============

### Upgrading to >= 0.19.1 (next)

#### DELETE now defaults to status code 200 for responses with a body, or 204 otherwise

Prior to this version, DELETE requests defaulted to a status code of 204 No Content, even when the response included content. This behavior confused some clients and prevented the formatter middleware from running properly. As of this version, DELETE requests will only default to a 204 No Content status code if no response body is provided, and will default to 200 OK otherwise.

Specifically, DELETE behaviour has changed as follows:

- In versions < 0.19.0, all DELETE requests defaulted to a 200 OK status code.
- In version 0.19.0, all DELETE requests defaulted to a 204 No Content status code, even when content was included in the response.
- As of version 0.19.1, DELETE requests default to a 204 No Content status code, unless content is supplied, in which case they default to a 200 OK status code.

To achieve the old behavior, one can specify the status code explicitly:

```ruby
delete :id do
status 204 # or 200, for < 0.19.0 behavior
'foo successfully deleted'
end
```

One can also use the new `return_no_content` helper to explicitly return a 204 status code and an empty body for any request type:

```ruby
delete :id do
return_no_content
'this will not be returned'
end
```

See [#1550](https://github.com/ruby-grape/grape/pull/1550) for more information.

### Upgrading to >= 0.18.1

#### Changes in priority of :any routes
Expand Down
20 changes: 19 additions & 1 deletion lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ def status(status = nil)
when Grape::Http::Headers::POST
201
when Grape::Http::Headers::DELETE
204
if @body.present?
200
else
204
end
else
200
end
Expand Down Expand Up @@ -181,6 +185,20 @@ def body(value = nil)
end
end

# Allows you to explicitly return no content.
#
# @example
# delete :id do
# return_no_content
# "not returned"
# end
#
# DELETE /12 # => 204 No Content, ""
def return_no_content
status 204
body false
end

# Allows you to define the response as a file-like object.
#
# @example
Expand Down
15 changes: 15 additions & 0 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,13 @@ 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
end

it 'returns status set' do
subject.status 501
expect(subject.status).to eq 501
Expand All @@ -136,6 +143,14 @@ def initialize
end
end

describe '#return_no_content' do
it 'sets the status code and body' do
subject.return_no_content
expect(subject.status).to eq 204
expect(subject.body).to eq ''
end
end

describe '#content_type' do
describe 'set' do
before do
Expand Down