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 Jun 2, 2020
1 parent 2bf2c1a commit 9e0329e
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@

* [#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 @@ -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
19 changes: 14 additions & 5 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 = Grape::ServeStream::StreamResponse.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
instance_variable_defined?(:@file) ? @file : nil
instance_variable_defined?(:@stream) ? @stream : nil
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
@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 @@ -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
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 @@ -208,15 +208,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 @@ -225,14 +225,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 @@ -245,7 +245,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 @@ -254,12 +254,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
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 9e0329e

Please sign in to comment.