From fdf539851a8de56e12d5e040ad8a4cdfe3a28952 Mon Sep 17 00:00:00 2001 From: Edward Rudd Date: Mon, 14 Nov 2016 17:06:15 -0500 Subject: [PATCH] un-deprecate stream-like objects - 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 --- CHANGELOG.md | 1 + lib/grape.rb | 4 ++-- lib/grape/dsl/inside_route.rb | 19 +++++++++++++----- lib/grape/middleware/formatter.rb | 6 +++--- .../{serve_file => serve_stream}/file_body.rb | 2 +- .../sendfile_response.rb | 2 +- .../stream_response.rb} | 16 +++++++-------- spec/grape/dsl/inside_route_spec.rb | 20 +++++++++---------- spec/grape/middleware/formatter_spec.rb | 2 +- 9 files changed, 41 insertions(+), 31 deletions(-) rename lib/grape/{serve_file => serve_stream}/file_body.rb (96%) rename lib/grape/{serve_file => serve_stream}/sendfile_response.rb (95%) rename lib/grape/{serve_file/file_response.rb => serve_stream/stream_response.rb} (58%) diff --git a/CHANGELOG.md b/CHANGELOG.md index eec65e074..fed63be7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/lib/grape.rb b/lib/grape.rb index 3ba532fbc..e2a0b9808 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -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 diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 7739db845..3e1385b39 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -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 @@ -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 diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 7ea7e57af..bb9888c2a 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -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 diff --git a/lib/grape/serve_file/file_body.rb b/lib/grape/serve_stream/file_body.rb similarity index 96% rename from lib/grape/serve_file/file_body.rb rename to lib/grape/serve_stream/file_body.rb index da4dd9e3f..b842af661 100644 --- a/lib/grape/serve_file/file_body.rb +++ b/lib/grape/serve_stream/file_body.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Grape - module ServeFile + module ServeStream CHUNK_SIZE = 16_384 # Class helps send file through API diff --git a/lib/grape/serve_file/sendfile_response.rb b/lib/grape/serve_stream/sendfile_response.rb similarity index 95% rename from lib/grape/serve_file/sendfile_response.rb rename to lib/grape/serve_stream/sendfile_response.rb index 5ff723b96..b46fc102a 100644 --- a/lib/grape/serve_file/sendfile_response.rb +++ b/lib/grape/serve_stream/sendfile_response.rb @@ -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 diff --git a/lib/grape/serve_file/file_response.rb b/lib/grape/serve_stream/stream_response.rb similarity index 58% rename from lib/grape/serve_file/file_response.rb rename to lib/grape/serve_stream/stream_response.rb index 88928d33c..0705fbf7b 100644 --- a/lib/grape/serve_file/file_response.rb +++ b/lib/grape/serve_stream/stream_response.rb @@ -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 diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 51da952fc..75fb357e0 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -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 @@ -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 @@ -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' @@ -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 diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 7c73663b5..9f6333cd0 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -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