Skip to content

Commit

Permalink
un-deprecate stream-like objects
Browse files Browse the repository at this point in the history
- this splits the purpose of "stream" and "file" inside_route methods.
- file will now always setup headers for streaming.
- stream is purely for stream-like objects. This allows you to have streaming responses of generated data
  • Loading branch information
urkle committed Jun 2, 2020
1 parent 2bf2c1a commit c7da23b
Show file tree
Hide file tree
Showing 11 changed files with 170 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#### Features

* [#1520](https://github.com/ruby-grape/grape/pull/1520): Un-deprecate stream-like objects - [@urkle](https://github.com/urkle).
* [#2060](https://github.com/ruby-grape/grape/pull/2060): Drop support for Ruby 2.4 - [@dblock](https://github.com/dblock).
* [#2060](https://github.com/ruby-grape/grape/pull/2060): Upgraded Rubocop to 0.84.0 - [@dblock](https://github.com/dblock).
* Your contribution here.
Expand Down
16 changes: 13 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3168,7 +3168,7 @@ end
Use `body false` to return `204 No Content` without any data or content-type.
You can also set the response to a file with `file`.
You can also set the response to a file with `file` and it will be streamed using Rack::Chunked.
```ruby
class API < Grape::API
Expand All @@ -3178,12 +3178,22 @@ class API < Grape::API
end
```
If you want a file to be streamed using Rack::Chunked, use `stream`.
If you want to stream non-file data you use the stream method and a Stream object.
This is simply an object that responds to each and yields for each chunk to send to the client.
Each chunk will be sent as it is yielded instead of waiting for all of the content to be available.
```ruby
class MyStream
def each
yield 'part 1'
yield 'part 2'
yield 'part 3'
end
end
class API < Grape::API
get '/' do
stream '/path/to/file'
stream MyStream.new
end
end
```
Expand Down
36 changes: 36 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,42 @@
Upgrading Grape
===============

### Upgrading to >= 1.3.4

#### Reworking stream and file and un-deprecating stream like-objects

Previously in 0.16 stream-like objects were deprecated. This release restores their functionality for use-cases other than file streaming.

For streaming files, simply use file always.

```ruby
class API < Grape::API
get '/' do
file '/path/to/file'
end
end
```

If you want to stream other kinds of content from a streamer object you may.
An example would be a streamer class that fetches several pages of data from a database and streams the formatted responses back.

```ruby
class MyObject
def each
yield '['
# maybe do some paginated DB fetches and return each page
yield {}.to_json
yield ']'
end
end

class API < Grape::API
get '/' do
stream MyObject.new
end
end
```

### Upgrading to >= 1.3.3

#### Nil values for structures
Expand Down
4 changes: 2 additions & 2 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,12 @@ module Presenters
end
end

module ServeFile
module ServeStream
extend ::ActiveSupport::Autoload
eager_autoload do
autoload :FileResponse
autoload :FileBody
autoload :SendfileResponse
autoload :StreamResponse
end
end
end
Expand Down
21 changes: 15 additions & 6 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,13 @@ def return_no_content
# GET /file # => "contents of file"
def file(value = nil)
if value.is_a?(String)
file_body = Grape::ServeFile::FileBody.new(value)
@file = Grape::ServeFile::FileResponse.new(file_body)
file_body = Grape::ServeStream::FileBody.new(value)
stream(file_body)
elsif !value.is_a?(NilClass)
warn '[DEPRECATION] Argument as FileStreamer-like object is deprecated. Use path to file instead.'
@file = Grape::ServeFile::FileResponse.new(value)
warn '[DEPRECATION] Argument as FileStreamer-like object is deprecated. Use path to file instead or stream to use a Stream object.'
stream(value)
else
instance_variable_defined?(:@file) ? @file : nil
stream
end
end

Expand All @@ -318,7 +318,16 @@ def stream(value = nil)
header 'Content-Length', nil
header 'Transfer-Encoding', nil
header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front)
file(value)
if value.is_a?(String)
warn '[DEPRECATION] Use `file file_path` to stream a file instead.'
file(value)
elsif value.respond_to?(:each)
@stream = Grape::ServeStream::StreamResponse.new(value)
elsif !value.is_a?(NilClass)
raise ArgumentError, 'Stream object must respond to :each.'
else
instance_variable_defined?(:@stream) ? @stream : nil
end
end

# Allows you to make use of Grape Entities by setting
Expand Down
6 changes: 3 additions & 3 deletions lib/grape/middleware/formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ def after
def build_formatted_response(status, headers, bodies)
headers = ensure_content_type(headers)

if bodies.is_a?(Grape::ServeFile::FileResponse)
Grape::ServeFile::SendfileResponse.new([], status, headers) do |resp|
resp.body = bodies.file
if bodies.is_a?(Grape::ServeStream::StreamResponse)
Grape::ServeStream::SendfileResponse.new([], status, headers) do |resp|
resp.body = bodies.stream
end
else
# Allow content-type to be explicitly overwritten
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Grape
module ServeFile
module ServeStream
CHUNK_SIZE = 16_384

# Class helps send file through API
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Grape
module ServeFile
module ServeStream
# Response should respond to to_path method
# for using Rack::SendFile middleware
class SendfileResponse < Rack::Response
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
# frozen_string_literal: true

module Grape
module ServeFile
# A simple class used to identify responses which represent files and do not
module ServeStream
# A simple class used to identify responses which represent streams (or files) and do not
# need to be formatted or pre-read by Rack::Response
class FileResponse
attr_reader :file
class StreamResponse
attr_reader :stream

# @param file [Object]
def initialize(file)
@file = file
# @param stream [Object]
def initialize(stream)
@stream = stream
end

# Equality provided mostly for tests.
#
# @return [Boolean]
def ==(other)
file == other.file
stream == other.stream
end
end
end
Expand Down
115 changes: 89 additions & 26 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,31 +208,55 @@ def initialize
let(:file_path) { '/some/file/path' }

let(:file_response) do
file_body = Grape::ServeFile::FileBody.new(file_path)
Grape::ServeFile::FileResponse.new(file_body)
file_body = Grape::ServeStream::FileBody.new(file_path)
Grape::ServeStream::StreamResponse.new(file_body)
end

before do
subject.header 'Cache-Control', 'cache'
subject.header 'Content-Length', 123
subject.header 'Transfer-Encoding', 'base64'

subject.file file_path
end

it 'returns value wrapped in FileResponse' do
it 'returns value wrapped in StreamResponse' do
expect(subject.file).to eq file_response
end

it 'sets Cache-Control header to no-cache' do
expect(subject.header['Cache-Control']).to eq 'no-cache'
end

it 'sets Content-Length header to nil' do
expect(subject.header['Content-Length']).to eq nil
end

it 'sets Transfer-Encoding header to nil' do
expect(subject.header['Transfer-Encoding']).to eq nil
end
end

context 'as object (backward compatibility)' do
let(:file_object) { Class.new }
let(:file_object) { double('StreamerObject', each: nil) }

let(:file_response) do
Grape::ServeFile::FileResponse.new(file_object)
Grape::ServeStream::StreamResponse.new(file_object)
end

before do
allow(subject).to receive(:warn)
end

it 'emits a warning that a stream object should be sent to the stream method' do
expect(subject).to receive(:warn).with(/Argument as FileStreamer-like/)

subject.file file_object
end

it 'returns value wrapped in FileResponse' do
it 'returns value wrapped in StreamResponse' do
subject.file file_object

expect(subject.file).to eq file_response
end
end
Expand All @@ -245,38 +269,77 @@ def initialize

describe '#stream' do
describe 'set' do
let(:file_object) { Class.new }
context 'as a file path (backward compatibility)' do
let(:file_path) { '/some/file/path' }

before do
subject.header 'Cache-Control', 'cache'
subject.header 'Content-Length', 123
subject.header 'Transfer-Encoding', 'base64'
subject.stream file_object
end
let(:file_response) do
file_body = Grape::ServeStream::FileBody.new(file_path)
Grape::ServeStream::StreamResponse.new(file_body)
end

it 'returns value wrapped in FileResponse' do
expect(subject.stream).to eq Grape::ServeFile::FileResponse.new(file_object)
end
before do
allow(subject).to receive(:warn)
end

it 'also sets result of file to value wrapped in FileResponse' do
expect(subject.file).to eq Grape::ServeFile::FileResponse.new(file_object)
end
it 'emits a warning to use file method to stream a file' do
expect(subject).to receive(:warn).with(/file file_path/)

subject.stream file_path
end

it 'returns value wrapped in StreamResponse' do
subject.stream file_path

it 'sets Cache-Control header to no-cache' do
expect(subject.header['Cache-Control']).to eq 'no-cache'
expect(subject.file).to eq file_response
end
end

it 'sets Content-Length header to nil' do
expect(subject.header['Content-Length']).to eq nil
context 'as a stream object' do
let(:stream_object) { double('StreamerObject', each: nil) }

let(:stream_response) do
Grape::ServeStream::StreamResponse.new(stream_object)
end

before do
subject.header 'Cache-Control', 'cache'
subject.header 'Content-Length', 123
subject.header 'Transfer-Encoding', 'base64'
subject.stream stream_object
end

it 'returns value wrapped in StreamResponse' do
expect(subject.stream).to eq stream_response
end

it 'also sets result of file to value wrapped in StreamResponse' do
expect(subject.file).to eq stream_response
end

it 'sets Cache-Control header to no-cache' do
expect(subject.header['Cache-Control']).to eq 'no-cache'
end

it 'sets Content-Length header to nil' do
expect(subject.header['Content-Length']).to eq nil
end

it 'sets Transfer-Encoding header to nil' do
expect(subject.header['Transfer-Encoding']).to eq nil
end
end

it 'sets Transfer-Encoding header to nil' do
expect(subject.header['Transfer-Encoding']).to eq nil
context 'as a non-stream object' do
let(:non_stream_object) { double('NonStreamerObject') }

it 'raises an error that the object must implement :each' do
expect { subject.stream non_stream_object }.to raise_error(ArgumentError, /:each/)
end
end
end

it 'returns default' do
expect(subject.file).to be nil
expect(subject.stream).to be nil
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ def to_xml

context 'send file' do
let(:file) { double(File) }
let(:file_body) { Grape::ServeFile::FileResponse.new(file) }
let(:file_body) { Grape::ServeStream::StreamResponse.new(file) }
let(:app) { ->(_env) { [200, {}, file_body] } }

it 'returns a file response' do
Expand Down

0 comments on commit c7da23b

Please sign in to comment.