Skip to content

Feature/add octet stream content type #745

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
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
============

* Your contribution here.
* [#745](https://github.com/intridea/grape/pull/745): Removed `atom+xml`, `rss+xml`, and `jsonapi` content-types - [@akabraham](https://github.com/akabraham).
* [#745](https://github.com/intridea/grape/pull/745): Added `:binary, application/octet-stream` content-type - [@akabraham](https://github.com/akabraham).

0.9.0 (8/27/2014)
=================
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1178,7 +1178,7 @@ end

## API Formats

By default, Grape supports _XML_, _JSON_, and _TXT_ content-types. The default format is `:txt`.
By default, Grape supports _XML_, _JSON_, _BINARY_, and _TXT_ content-types. The default format is `:txt`.

Serialization takes place automatically. For example, you do not have to call `to_json` in each JSON API implementation.

Expand Down Expand Up @@ -1242,10 +1242,11 @@ end

Built-in formats are the following.

* `:json` and `:jsonapi`: use object's `to_json` when available, otherwise call `MultiJson.dump`
* `:json`: use object's `to_json` when available, otherwise call `MultiJson.dump`
* `:xml`: use object's `to_xml` when available, usually via `MultiXml`, otherwise call `to_s`
* `:txt`: use object's `to_txt` when available, otherwise `to_s`
* `:serializable_hash`: use object's `serializable_hash` when available, otherwise fallback to `:json`
* `:binary`

Use `default_format` to set the fallback format when the format could not be determined from the `Accept` header.
See below for the order for choosing the API format.
Expand Down
12 changes: 12 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
Upgrading Grape
===============

### Upgrading to >= 0.9.1

#### Changes to content-types

The following content-types have been removed:

* atom (application/atom+xml)
* rss (application/rss+xml)
* jsonapi (application/jsonapi)

This is because they have never been properly supported.

### Upgrading to >= 0.9.0

#### Changes in Authentication
Expand Down
6 changes: 2 additions & 4 deletions lib/grape/util/content_types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ module ContentTypes
:xml, 'application/xml',
:serializable_hash, 'application/json',
:json, 'application/json',
:jsonapi, 'application/vnd.api+json',
:atom, 'application/atom+xml',
:rss, 'application/rss+xml',
:txt, 'text/plain',
:binary, 'application/octet-stream',
:txt, 'text/plain'
]

def self.content_types_for(from_settings)
Expand Down
33 changes: 29 additions & 4 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,37 @@ def subject.enable_root_route!

it 'sets content type for txt format' do
get '/foo'
expect(last_response.headers['Content-Type']).to eql 'text/plain'
expect(last_response.headers['Content-Type']).to eq('text/plain')
end

it 'sets content type for xml' do
get '/foo.xml'
expect(last_response.headers['Content-Type']).to eq('application/xml')
end

it 'sets content type for json' do
get '/foo.json'
expect(last_response.headers['Content-Type']).to eql 'application/json'
expect(last_response.headers['Content-Type']).to eq('application/json')
end

it 'sets content type for serializable hash format' do
get '/foo.serializable_hash'
expect(last_response.headers['Content-Type']).to eq('application/json')
end

it 'sets content type for binary format' do
get '/foo.binary'
expect(last_response.headers['Content-Type']).to eq('application/octet-stream')
end

it 'returns raw data when content type binary' do
image_filename = 'grape.png'
file = File.open(image_filename, 'rb') { |io| io.read }
subject.format :binary
subject.get('/binary_file') { File.binread(image_filename) }
get '/binary_file'
expect(last_response.headers['Content-Type']).to eq('application/octet-stream')
expect(last_response.body).to eq(file)
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So thanks for adding the tests, however they don't actually work as intended. If we want to properly support atom+xml, rss+xml and jsonapi we need a formatter for these. I think they should be removed for now.

it 'sets content type for error' do
Expand All @@ -756,14 +781,14 @@ def subject.enable_root_route!
expect(last_response.headers['Content-Type']).to eql 'text/plain'
end

it 'sets content type for error' do
it 'sets content type for json error' do
subject.format :json
subject.get('/error') { error!('error in json', 500) }
get '/error.json'
expect(last_response.headers['Content-Type']).to eql 'application/json'
end

it 'sets content type for xml' do
it 'sets content type for xml error' do
subject.format :xml
subject.get('/error') { error!('error in xml', 500) }
get '/error.xml'
Expand Down
6 changes: 2 additions & 4 deletions spec/grape/dsl/request_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,8 @@ def self.imbue(key, value)
expect(subject.content_types).to eq(xml: "application/xml",
serializable_hash: "application/json",
json: "application/json",
jsonapi: "application/vnd.api+json",
atom: "application/atom+xml",
rss: "application/rss+xml",
txt: "text/plain")
txt: "text/plain",
binary: "application/octet-stream")
end
end

Expand Down