Merge pull request #2088 from stanhu/sh-fix-issue-2087

Set Cache-Control only for streamed responses

Author: dblock

CHANGELOG.md

@@ -6,7 +6,7 @@
 
 #### Fixes
 
-* Your contribution here.
+* [#2083](https://github.com/ruby-grape/grape/pull/2083): Set Cache-Control only for streamed responses - [@stanhu](https://github.com/stanhu).
 
 ### 1.4.0 (2020/07/10)
 

lib/grape/dsl/inside_route.rb

@@ -328,6 +328,8 @@ def sendfile(value = nil)
       # * https://github.com/rack/rack/blob/99293fa13d86cd48021630fcc4bd5acc9de5bdc3/lib/rack/chunked.rb
       # * https://github.com/rack/rack/blob/99293fa13d86cd48021630fcc4bd5acc9de5bdc3/lib/rack/etag.rb
       def stream(value = nil)
+        return if value.nil? && @stream.nil?
+
         header 'Content-Length', nil
         header 'Transfer-Encoding', nil
         header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front)
@@ -339,7 +341,7 @@ def stream(value = nil)
         elsif !value.is_a?(NilClass)
           raise ArgumentError, 'Stream object must respond to :each.'
         else
-          instance_variable_defined?(:@stream) ? @stream : nil
+          @stream
         end
       end
 

spec/grape/api_spec.rb

@@ -1149,6 +1149,11 @@ class DummyFormatClass
       expect(last_response.headers['Content-Type']).to eq('text/plain')
     end
 
+    it 'does not set Cache-Control' do
+      get '/foo'
+      expect(last_response.headers['Cache-Control']).to eq(nil)
+    end
+
     it 'sets content type for xml' do
       get '/foo.xml'
       expect(last_response.headers['Content-Type']).to eq('application/xml')

spec/grape/dsl/inside_route_spec.rb

@@ -351,6 +351,12 @@ def initialize
           expect(subject.header['Cache-Control']).to eq 'no-cache'
         end
 
+        it 'does not change Cache-Control header' do
+          subject.stream
+
+          expect(subject.header['Cache-Control']).to eq 'cache'
+        end
+
         it 'sets Content-Length header to nil' do
           subject.stream file_path
 
@@ -419,6 +425,7 @@ def initialize
 
     it 'returns default' do
       expect(subject.stream).to be nil
+      expect(subject.header['Cache-Control']).to eq nil
     end
   end