squash! un-deprecate stream-like objects

- fully deprecate file
- add sendfile as replacement

Author: urkle

README.md

@@ -3168,12 +3168,22 @@ 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` and it will be streamed using `Rack::Chunked`.
+You can also set the response to a file with `sendfile`
 
 ```ruby
 class API < Grape::API
   get '/' do
-    file '/path/to/file'
+    sendfile '/path/to/file'
+  end
+end
+```
+
+To stream a file use `stream`
+
+```ruby
+class API < Grape::API
+  get '/' do
+    stream '/path/to/file'
   end
 end
 ```

UPGRADING.md

@@ -7,17 +7,28 @@ Upgrading Grape
 
 Previously in 0.16 stream-like objects were deprecated. This release restores their functionality for use-cases other than file streaming.
 
-For streaming files, use `file` always.
+This release also renamed `file` to `sendfile` to better document it's purpose.
 
+To deliver a file via `sendfile` if available do this.
 ```ruby
 class API < Grape::API
   get '/' do
-    file '/path/to/file'
+    sendfile '/path/to/file'
+  end
+end
+```
+
+Use `stream` to stream files
+
+```ruby
+class API < Grape::API
+  get '/' do
+    stream '/path/to/file'
   end
 end
 ```
 
-Use `stream` to stream other kinds of content. In the following example a streamer class 
+Or use `stream` to stream other kinds of content. In the following example a streamer class 
 streams paginated data from a database.
 
 ```ruby

lib/grape/dsl/inside_route.rb

@@ -279,21 +279,34 @@ def return_no_content
         body false
       end
 
-      # Allows you to define the response as a file-like object.
+      # Deprecated method to send files to the client. Use `sendfile` or `stream`
+      def file(value = nil)
+        if value.is_a?(String)
+          warn '[DEPRECATION] Use sendfile or stream to send files.'
+          sendfile(value)
+        elsif !value.is_a?(NilClass)
+          warn '[DEPRECATION] Use stream to use a Stream object.'
+          stream(value)
+        else
+          warn '[DEPRECATION] Use sendfile or stream to send files.'
+          sendfile
+        end
+      end
+
+      # Allows you to send a file to the client via sendfile.
       #
       # @example
       #   get '/file' do
-      #     file FileStreamer.new(...)
+      #     sendfile FileStreamer.new(...)
       #   end
       #
       #   GET /file # => "contents of file"
-      def file(value = nil)
+      def sendfile(value = nil)
         if value.is_a?(String)
           file_body = Grape::ServeStream::FileBody.new(value)
-          stream(file_body)
+          @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 or stream to use a Stream object.'
-          stream(value)
+          raise ArgumentError, 'Argument must be a file path'
         else
           stream
         end
@@ -319,8 +332,8 @@ def stream(value = nil)
         header 'Transfer-Encoding', nil
         header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front)
         if value.is_a?(String)
-          warn '[DEPRECATION] Use `file file_path` to stream a file instead.'
-          file(value)
+          file_body = Grape::ServeStream::FileBody.new(value)
+          @stream = Grape::ServeStream::StreamResponse.new(file_body)
         elsif value.respond_to?(:each)
           @stream = Grape::ServeStream::StreamResponse.new(value)
         elsif !value.is_a?(NilClass)

lib/grape/endpoint.rb

@@ -99,7 +99,7 @@ def initialize(new_settings, options = {}, &block)
       @block = nil
 
       @status = nil
-      @file = nil
+      @stream = nil
       @body = nil
       @proc = nil
 
@@ -271,8 +271,8 @@ def run
           # status verifies body presence when DELETE
           @body ||= response_object
 
-          # The body commonly is an Array of Strings, the application instance itself, or a File-like object
-          response_object = file || [body]
+          # The body commonly is an Array of Strings, the application instance itself, or a Stream-like object
+          response_object = stream || [body]
 
           [status, header, response_object]
         ensure

spec/grape/dsl/inside_route_spec.rb

@@ -203,6 +203,60 @@ def initialize
   end
 
   describe '#file' do
+    before do
+      allow(subject).to receive(:warn)
+    end
+
+    describe 'set' do
+      context 'as file path' do
+        let(:file_path) { '/some/file/path' }
+
+        it 'emits a warning that this method is deprecated' do
+          expect(subject).to receive(:warn).with(/Use sendfile or stream/)
+
+          subject.file file_path
+        end
+
+        it 'forwards the call to sendfile' do
+          expect(subject).to receive(:sendfile).with(file_path)
+
+          subject.file file_path
+        end
+      end
+
+      context 'as object (backward compatibility)' do
+        let(:file_object) { double('StreamerObject', each: nil) }
+
+        it 'emits a warning that this method is deprecated' do
+          expect(subject).to receive(:warn).with(/Use stream to use a Stream object/)
+
+          subject.file file_object
+        end
+
+        it 'forwards the call to stream' do
+          expect(subject).to receive(:stream).with(file_object)
+
+          subject.file file_object
+        end
+      end
+    end
+
+    describe 'get' do
+      it 'emits a warning that this method is deprecated' do
+        expect(subject).to receive(:warn).with(/Use sendfile or stream/)
+
+        subject.file
+      end
+
+      it 'fowards call to sendfile' do
+        expect(subject).to receive(:sendfile)
+
+        subject.file
+      end
+    end
+  end
+
+  describe '#sendfile' do
     describe 'set' do
       context 'as file path' do
         let(:file_path) { '/some/file/path' }
@@ -216,60 +270,56 @@ def initialize
           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 StreamResponse' do
-          expect(subject.file).to eq file_response
-        end
+        it 'sends no deprecation warnings' do
+          expect(subject).to_not receive(:warn)
 
-        it 'sets Cache-Control header to no-cache' do
-          expect(subject.header['Cache-Control']).to eq 'no-cache'
+          subject.sendfile file_path
         end
 
-        it 'sets Content-Length header to nil' do
-          expect(subject.header['Content-Length']).to eq nil
-        end
+        it 'returns value wrapped in StreamResponse' do
+          subject.sendfile file_path
 
-        it 'sets Transfer-Encoding header to nil' do
-          expect(subject.header['Transfer-Encoding']).to eq nil
+          expect(subject.sendfile).to eq file_response
         end
-      end
 
-      context 'as object (backward compatibility)' do
-        let(:file_object) { double('StreamerObject', each: nil) }
+        it 'does not change the Cache-Control header' do
+          subject.sendfile file_path
 
-        let(:file_response) do
-          Grape::ServeStream::StreamResponse.new(file_object)
+          expect(subject.header['Cache-Control']).to eq 'cache'
         end
 
-        before do
-          allow(subject).to receive(:warn)
+        it 'does not change the Content-Length header' do
+          subject.sendfile file_path
+
+          expect(subject.header['Content-Length']).to eq 123
         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/)
+        it 'does not change the Transfer-Encoding header' do
+          subject.sendfile file_path
 
-          subject.file file_object
+          expect(subject.header['Transfer-Encoding']).to eq 'base64'
         end
+      end
 
-        it 'returns value wrapped in StreamResponse' do
-          subject.file file_object
+      context 'as object' do
+        let(:file_object) { double('StreamerObject', each: nil) }
 
-          expect(subject.file).to eq file_response
+        it 'raises an error that only a file path is supported' do
+          expect { subject.sendfile file_object }.to raise_error(ArgumentError, /Argument must be a file path/)
         end
       end
     end
 
     it 'returns default' do
-      expect(subject.file).to be nil
+      expect(subject.sendfile).to be nil
     end
   end
 
   describe '#stream' do
     describe 'set' do
-      context 'as a file path (backward compatibility)' do
+      context 'as a file path' do
         let(:file_path) { '/some/file/path' }
 
         let(:file_response) do
@@ -278,19 +328,39 @@ def initialize
         end
 
         before do
-          allow(subject).to receive(:warn)
+          subject.header 'Cache-Control', 'cache'
+          subject.header 'Content-Length', 123
+          subject.header 'Transfer-Encoding', 'base64'
         end
 
-        it 'emits a warning to use file method to stream a file' do
-          expect(subject).to receive(:warn).with(/file file_path/)
+        it 'emits no deprecation warnings' do
+          expect(subject).to_not receive(:warn)
 
           subject.stream file_path
         end
 
-        it 'returns value wrapped in StreamResponse' do
+        it 'returns file body wrapped in StreamResponse' do
+          subject.stream file_path
+
+          expect(subject.stream).to eq file_response
+        end
+
+        it 'sets Cache-Control header to no-cache' do
           subject.stream file_path
 
-          expect(subject.file).to eq file_response
+          expect(subject.header['Cache-Control']).to eq 'no-cache'
+        end
+
+        it 'sets Content-Length header to nil' do
+          subject.stream file_path
+
+          expect(subject.header['Content-Length']).to eq nil
+        end
+
+        it 'sets Transfer-Encoding header to nil' do
+          subject.stream file_path
+
+          expect(subject.header['Transfer-Encoding']).to eq nil
         end
       end
 
@@ -305,26 +375,35 @@ def initialize
           subject.header 'Cache-Control', 'cache'
           subject.header 'Content-Length', 123
           subject.header 'Transfer-Encoding', 'base64'
+        end
+
+        it 'emits no deprecation warnings' do
+          expect(subject).to_not receive(:warn)
+
           subject.stream stream_object
         end
 
         it 'returns value wrapped in StreamResponse' do
-          expect(subject.stream).to eq stream_response
-        end
+          subject.stream stream_object
 
-        it 'also sets result of file to value wrapped in StreamResponse' do
-          expect(subject.file).to eq stream_response
+          expect(subject.stream).to eq stream_response
         end
 
         it 'sets Cache-Control header to no-cache' do
+          subject.stream stream_object
+
           expect(subject.header['Cache-Control']).to eq 'no-cache'
         end
 
         it 'sets Content-Length header to nil' do
+          subject.stream stream_object
+
           expect(subject.header['Content-Length']).to eq nil
         end
 
         it 'sets Transfer-Encoding header to nil' do
+          subject.stream stream_object
+
           expect(subject.header['Transfer-Encoding']).to eq nil
         end
       end