Skip to content

Commit

Permalink
un-deprecate stream-like objects
Browse files Browse the repository at this point in the history
- this splits the "stream" and "file" inside_route methods
- file is purely for specifying a file (and utilizing sendfile when it can)
- stream is purely for stream-like objects.. This allows you to have streaming responses of generated data
  • Loading branch information
urkle committed Jan 3, 2019
1 parent b645fb4 commit 88372d3
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Your contribution here.
* [#1850](https://github.com/ruby-grape/grape/pull/1850): Adds `same_as` validator - [@glaucocustodio](https://github.com/glaucocustodio).
* [#1833](https://github.com/ruby-grape/grape/pull/1833): Allows to set the `ParamBuilder` globally - [@myxoh](https://github.com/myxoh).
* [#1520](https://github.com/ruby-grape/grape/pull/1520): un-deprecate stream-like objects - [@urkle](https://github.com/urkle).

#### Fixes

Expand Down
4 changes: 2 additions & 2 deletions lib/grape.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,11 @@ module Presenters
autoload :Presenter
end

module ServeFile
module ServeStream
extend ::ActiveSupport::Autoload
autoload :FileResponse
autoload :FileBody
autoload :SendfileResponse
autoload :StreamResponse
end
end

Expand Down
19 changes: 14 additions & 5 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,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 = Grape::ServeStream::SteamResponse.new(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)
@stream = Grape::ServeStream::StreamResponse.new(value)
else
@file
@stream
end
end

Expand All @@ -286,7 +286,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
@stream
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 @@ -34,9 +34,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,5 +1,5 @@
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,5 +1,5 @@
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,20 +1,20 @@
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
20 changes: 10 additions & 10 deletions spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,15 @@ 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.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
end
Expand All @@ -223,14 +223,14 @@ def initialize
let(:file_object) { Class.new }

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

before do
subject.file file_object
end

it 'returns value wrapped in FileResponse' do
it 'returns value wrapped in StreamResponse' do
expect(subject.file).to eq file_response
end
end
Expand All @@ -243,7 +243,7 @@ def initialize

describe '#stream' do
describe 'set' do
let(:file_object) { Class.new }
let(:file_object) { double('StreamerObject', each: nil) }

before do
subject.header 'Cache-Control', 'cache'
Expand All @@ -252,12 +252,12 @@ def initialize
subject.stream file_object
end

it 'returns value wrapped in FileResponse' do
expect(subject.stream).to eq Grape::ServeFile::FileResponse.new(file_object)
it 'returns value wrapped in StreamResponse' do
expect(subject.stream).to eq Grape::ServeStream::StreamResponse.new(file_object)
end

it 'also sets result of file to value wrapped in FileResponse' do
expect(subject.file).to eq Grape::ServeFile::FileResponse.new(file_object)
it 'also sets result of file to value wrapped in StreamResponse' do
expect(subject.file).to eq Grape::ServeStream::StreamResponse.new(file_object)
end

it 'sets Cache-Control header to no-cache' do
Expand Down
6 changes: 3 additions & 3 deletions spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,12 @@ def to_xml
end

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

it 'returns Grape::Uril::SendFileReponse' do
it 'returns Grape::ServerStream::SendFileReponse' do
env = { 'PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json' }
expect(subject.call(env)).to be_a(Grape::ServeFile::SendfileResponse)
expect(subject.call(env)).to be_a(Grape::ServeStream::SendfileResponse)
end
end

Expand Down

0 comments on commit 88372d3

Please sign in to comment.