From 81cf0fbce025cd54b97acf1135adabe7ddaee2fc Mon Sep 17 00:00:00 2001 From: James Thorne Date: Wed, 4 Jan 2017 12:35:15 -0700 Subject: [PATCH 1/4] Use 200 as default status for deletes that reply with content --- CHANGELOG.md | 1 + README.md | 2 +- lib/grape/dsl/inside_route.rb | 6 +++++- spec/grape/dsl/inside_route_spec.rb | 7 +++++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05926b91f..5b96a66da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/README.md b/README.md index f68df4a22..f3282cb41 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 28e37873e..72db57b66 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -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 diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 5cbd4e7c3..6e0e77fc6 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -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 From d71a7b0275e92bec971630ed7062ab7189261318 Mon Sep 17 00:00:00 2001 From: James Thorne Date: Wed, 4 Jan 2017 14:23:47 -0700 Subject: [PATCH 2/4] =?UTF-8?q?Add=20=E2=80=9Creturn=5Fno=5Fcontent?= =?UTF-8?q?=E2=80=9D=20helper=20to=20explicitly=20respond=20with=20a=20204?= =?UTF-8?q?=20status=20code=20and=20an=20empty=20body?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/grape/dsl/inside_route.rb | 14 ++++++++++++++ spec/grape/dsl/inside_route_spec.rb | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 72db57b66..3f0b6640d 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -185,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 diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 6e0e77fc6..f48ba0d8b 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -143,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 From a758d45d70bf82f5a5a0bac1a3341c35a4049dab Mon Sep 17 00:00:00 2001 From: James Thorne Date: Wed, 4 Jan 2017 14:28:26 -0700 Subject: [PATCH 3/4] =?UTF-8?q?Add=20a=20section=20to=20UPGRADING.md=20des?= =?UTF-8?q?cribing=20the=20new=20DELETE=20status=20code=20behaviour=20and?= =?UTF-8?q?=20how=20it=E2=80=99s=20changed=20over=20the=20past=20few=20ver?= =?UTF-8?q?sions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UPGRADING.md | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/UPGRADING.md b/UPGRADING.md index 89a6832c2..1b24127d2 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,36 @@ Upgrading Grape =============== +### Upgrading to >= 0.19.1 (next) + +#### DELETE now defaults to status code 200 for responses with a body, or 204 otherwise + +DELETE responses now use a 200 status code by default when a body is present, or 204 otherwise. + +- 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 204 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 From 9164650bfb6f0ad88788bf7ce2021844a80c65d6 Mon Sep 17 00:00:00 2001 From: James Thorne Date: Wed, 4 Jan 2017 14:41:04 -0700 Subject: [PATCH 4/4] Improve wording of UPGRADING.md DELETE status code changes --- UPGRADING.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/UPGRADING.md b/UPGRADING.md index 1b24127d2..1ac31f201 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -5,7 +5,9 @@ Upgrading Grape #### DELETE now defaults to status code 200 for responses with a body, or 204 otherwise -DELETE responses now use a 200 status code by default when a body is present, 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. @@ -20,7 +22,7 @@ delete :id do end ``` -One can also use the new `return_no_content` helper to explicitly return 204 and an empty body for any request type: +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