Merge pull request #6447 from samvera/update_downloads_controller_spec

Valkyrize downloads_controller_spec; Improved Range and Cache-Control features

app/controllers/concerns/hyrax/local_file_downloads_controller_behavior.rb

@@ -6,9 +6,7 @@ module LocalFileDownloadsControllerBehavior
     # Handle the HTTP show request
     def send_local_content
       response.headers['Accept-Ranges'] = 'bytes'
-      if request.head?
-        local_content_head
-      elsif request.headers['Range']
+      if request.headers['Range']
         send_range_for_local_file
       else
         send_local_file_contents
@@ -26,15 +24,23 @@ def send_range_for_local_file
       self.status = 206
       prepare_local_file_headers
       # For derivatives stored on the local file system
-      send_data IO.binread(file, length, from), local_derivative_download_options.merge(status: status)
+      if request.head?
+        head status
+      else
+        send_data IO.binread(file, length, from), local_derivative_download_options.merge(status: status)
+      end
     end
 
     def send_local_file_contents
       return unless stale?(last_modified: local_file_last_modified, template: false)
       self.status = 200
       prepare_local_file_headers
       # For derivatives stored on the local file system
-      send_file file, local_derivative_download_options
+      if request.head?
+        head status
+      else
+        send_file file, local_derivative_download_options
+      end
     end
 
     def local_file_size
@@ -54,16 +60,9 @@ def local_file_last_modified
       File.mtime(file) if file.is_a? String
     end
 
-    # Override
-    # render an HTTP HEAD response
-    def local_content_head
-      response.headers['Content-Length'] = local_file_size.to_s
-      head :ok, content_type: local_file_mime_type
-    end
-
     # Override
     def prepare_local_file_headers
-      send_file_headers! local_content_options
+      send_file_headers! local_derivative_download_options
       response.headers['Content-Type'] = local_file_mime_type
       response.headers['Content-Length'] ||= local_file_size.to_s
       # Prevent Rack::ETag from calculating a digest over body
@@ -73,12 +72,6 @@ def prepare_local_file_headers
 
     private
 
-    # Override the Hydra::Controller::DownloadBehavior#content_options so that
-    # we have an attachement rather than 'inline'
-    def local_content_options
-      { type: local_file_mime_type, filename: local_file_name, disposition: 'attachment' }
-    end
-
     # Override this method if you want to change the options sent when downloading
     # a derivative file
     def local_derivative_download_options

app/controllers/concerns/hyrax/stream_file_downloads_controller_behavior.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+module Hyrax
+  # Overrides Hydra::Controller:DownloadBehavior handing of HEAD requests to
+  # respond with same headers as a GET request would receive.
+  module StreamFileDownloadsControllerBehavior
+    protected
+
+    # Handle the HTTP show request
+    def send_content
+      response.headers['Accept-Ranges'] = 'bytes'
+      if request.headers['HTTP_RANGE']
+        send_range
+      else
+        send_file_contents
+      end
+    end
+
+    # rubocop:disable Metrics/AbcSize
+    def send_range
+      _, range = request.headers['HTTP_RANGE'].split('bytes=')
+      from, to = range.split('-').map(&:to_i)
+      to = file.size - 1 unless to
+      length = to - from + 1
+      response.headers['Content-Range'] = "bytes #{from}-#{to}/#{file.size}"
+      response.headers['Content-Length'] = length.to_s
+      self.status = 206
+      prepare_file_headers
+
+      if request.head?
+        head status
+      else
+        stream_body file.stream(request.headers['HTTP_RANGE'])
+      end
+    end
+    # rubocop:enable Metrics/AbcSize
+
+    def send_file_contents
+      return unless stale?(last_modified: file_last_modified, template: false)
+
+      self.status = 200
+      prepare_file_headers
+
+      if request.head?
+        head status
+      else
+        stream_body file.stream
+      end
+    end
+
+    def file_last_modified
+      file.modified_date
+    end
+  end
+end

app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb

@@ -9,10 +9,10 @@ def show_valkyrie
 
     private
 
+    # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     def send_file_contents_valkyrie(file_set)
       response.headers["Accept-Ranges"] = "bytes"
       self.status = 200
-      use = params.fetch(:file, :original_file).to_sym
       mime_type = params[:mime_type]
       file_metadata = find_file_metadata(file_set: file_set, use: use, mime_type: mime_type)
       return unless stale?(last_modified: file_metadata.updated_at, template: false)
@@ -23,43 +23,72 @@ def send_file_contents_valkyrie(file_set)
       # Warning - using the range header will load the range selection in to memory
       # this can cause memory bloat
       if request.headers['Range']
-        file.rewind
-        send_data send_range_valkyrie(file: file), data_options(file_metadata)
+        if request.head?
+          prepare_range_headers_valkyrie(file: file)
+          head status
+        else
+          send_data send_range_valkyrie(file: file), data_options(file_metadata)
+        end
+      elsif request.head?
+        head status
       else
         send_file file.disk_path, data_options(file_metadata).except(:status)
       end
     end
+    # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
 
     def data_options(file_metadata)
       {
         type: file_metadata.mime_type,
         filename: file_metadata.original_filename,
-        disposition: "inline",
+        disposition: disposition,
         status: status
       }
     end
 
+    def use
+      params.fetch(:file, :original_file).to_sym
+    end
+
+    def disposition
+      if ActiveRecord::Type::Boolean.new.cast(params.fetch(:inline, use != :original_file))
+        'inline'
+      else
+        'attachment'
+      end
+    end
+
     def send_range_valkyrie(file:)
+      from, length = prepare_range_headers_valkyrie(file: file)
+      file.rewind
+      file.read from # Seek to start of requested range
+      file.read length
+    end
+
+    def prepare_range_headers_valkyrie(file:)
       _, range = request.headers['Range'].split('bytes=')
       from, to = range.split('-').map(&:to_i)
       to = file.size - 1 unless to
       length = to - from + 1
       response.headers['Content-Range'] = "bytes #{from}-#{to}/#{file.size}"
       response.headers['Content-Length'] = length.to_s
       self.status = 206
-      file.read from # Seek to start of requested range
-      file.read length
+      [from, length]
     end
 
-    def prepare_file_headers_valkyrie(metadata:, file:, inline: false)
-      inline_display = ActiveRecord::Type::Boolean.new.cast(params.fetch(:inline, inline))
-      response.headers["Content-Disposition"] = "#{inline_display ? 'inline' : 'attachment'}; filename=#{metadata.original_filename}"
+    # rubocop:disable Metrics/AbcSize
+    def prepare_file_headers_valkyrie(metadata:, file:)
+      response.headers["Content-Disposition"] =
+        ActionDispatch::Http::ContentDisposition.format(disposition: disposition, filename: metadata.original_filename)
       response.headers["Content-Type"] = metadata.mime_type
       response.headers["Content-Length"] ||= (file.try(:size) || metadata.size.first).to_s
+      headers["Content-Transfer-Encoding"] = "binary"
       # Prevent Rack::ETag from calculating a digest over body
       response.headers["Last-Modified"] = metadata.updated_at.utc.strftime("%a, %d %b %Y %T GMT")
       self.content_type = metadata.mime_type
+      response.cache_control[:public] ||= false
     end
+    # rubocop:enable Metrics/AbcSize
 
     def find_file_metadata(file_set:, use: :original_file, mime_type: nil)
       if mime_type.nil?

app/controllers/hyrax/downloads_controller.rb

@@ -2,6 +2,7 @@
 module Hyrax
   class DownloadsController < ApplicationController
     include Hydra::Controller::DownloadBehavior
+    include Hyrax::StreamFileDownloadsControllerBehavior
     include Hyrax::LocalFileDownloadsControllerBehavior
     include Hyrax::ValkyrieDownloadsControllerBehavior
     include Hyrax::WorkflowsHelper # Provides #workflow_restriction?

app/models/hyrax/file_metadata.rb

@@ -52,6 +52,8 @@ def uri_for(use:)
           EXTRACTED_TEXT
         when :thumbnail_file
           THUMBNAIL
+        when :service_file
+          SERVICE_FILE
         else
           raise ArgumentError, "No PCDM use is recognized for #{use}"
         end