From 41612980507959f657f706dac9efe623e78db5f7 Mon Sep 17 00:00:00 2001 From: Aaron Collier Date: Thu, 1 Aug 2024 09:18:59 -0700 Subject: [PATCH] Use file controller for v2 file path --- app/controllers/file_controller.rb | 8 +- app/controllers/v2/versions_controller.rb | 84 ------------------- config/routes.rb | 6 +- .../v2/versions_controller_spec.rb | 56 ------------- spec/requests/versioned_file_spec.rb | 6 +- 5 files changed, 12 insertions(+), 148 deletions(-) delete mode 100644 app/controllers/v2/versions_controller.rb delete mode 100644 spec/controllers/v2/versions_controller_spec.rb diff --git a/app/controllers/file_controller.rb b/app/controllers/file_controller.rb index d8130c4d..40ea3b72 100644 --- a/app/controllers/file_controller.rb +++ b/app/controllers/file_controller.rb @@ -45,7 +45,7 @@ def disposition end def file_params - params.permit(:id, :file_name, :download) + params.permit(:id, :file_name, :download, :version_id) end # called when CanCan::AccessDenied error is raised, typically by authorize! @@ -74,6 +74,10 @@ def current_file end def cocina - @cocina ||= Cocina.find(params[:id]) + @cocina ||= Cocina.find(params[:id], version) + end + + def version + params[:version_id] || :head end end diff --git a/app/controllers/v2/versions_controller.rb b/app/controllers/v2/versions_controller.rb deleted file mode 100644 index c8c9d99b..00000000 --- a/app/controllers/v2/versions_controller.rb +++ /dev/null @@ -1,84 +0,0 @@ -# frozen_string_literal: true - -module V2 - # API for delivering files from stacks by version - class VersionsController < ApplicationController - rescue_from ActionController::MissingFile do - render plain: 'File not found', status: :not_found - end - - # rubocop:disable Metrics/AbcSize - def show - return unless stale?(**cache_headers) - - authorize! :download, current_file - expires_in 10.minutes - response.headers['Accept-Ranges'] = 'bytes' - response.headers['Content-Length'] = current_file.content_length - response.headers.delete('X-Frame-Options') - - TrackDownloadJob.perform_later( - druid: current_file.id, - file: current_file.file_name, - user_agent: request.user_agent, - ip: request.remote_ip - ) - - send_file current_file.path, disposition: - end - # rubocop:enable Metrics/AbcSize - - def options - response.headers['Access-Control-Allow-Methods'] = 'GET, OPTIONS' - response.headers['Access-Control-Allow-Headers'] = 'Range' - response.headers['Access-Control-Max-Age'] = 1.day.to_i - - head :ok - end - - private - - def disposition - return :attachment if file_params[:download] - - :inline - end - - def file_params - params.permit(:id, :file_name, :download) - end - - # called when CanCan::AccessDenied error is raised, typically by authorize! - # Should only be here if - # a) access not allowed (send to super) OR - # b) need user to login to determine if access allowed - def rescue_can_can(exception) - if User.stanford_generic_user.ability.can?(:access, current_file) && !current_user.webauth_user? - redirect_to auth_file_url(file_params.to_h.symbolize_keys) - else - super - end - end - - def cache_headers - { - etag: [current_file.etag, current_user.try(:etag)], - last_modified: current_file.mtime, - public: anonymous_ability.can?(:download, current_file), - template: false - } - end - - def current_file - @file ||= StacksFile.new(file_name: params[:file_name], cocina:) - end - - def cocina - @cocina ||= Cocina.find(params[:id], version) - end - - def version - params[:version_id] || :head - end - end -end diff --git a/config/routes.rb b/config/routes.rb index 513be2fd..ff276eb6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,7 +6,9 @@ constraints id: druid_regex do get '/file/:id/*file_name' => 'file#show', format: false, as: :file + get '/v2/file/:id/:version_id/*file_name', to: 'file#show', format: false, constraints: { version_id: /v\d+/ } options '/file/:id/*file_name', to: 'file#options', format: false + options '/v2/file/:id/:version_id/*file_name', to: 'file#options', format: false, constraints: { version_id: /v\d+/ } get '/file/app/:id/*file_name' => 'webauth#login_file', format: false get '/file/auth/:id/*file_name' => 'webauth#login_file', format: false, as: :auth_file get '/file/auth/:id' => 'webauth#login_object', format: false, as: :auth_object @@ -16,10 +18,6 @@ get '/file/app/druid::id/*file_name' => 'webauth#login_file', format: false get '/file/auth/druid::id/*file_name' => 'webauth#login_file', format: false get '/file/auth/druid::id' => 'webauth#login_object', format: false - - namespace 'v2' do - get '/file/:id/:version_id/*file_name', to: 'versions#show', format: false, constraints: { version_id: /v\d+/ } - end end if Settings.features.streaming_media diff --git a/spec/controllers/v2/versions_controller_spec.rb b/spec/controllers/v2/versions_controller_spec.rb deleted file mode 100644 index 6f40df42..00000000 --- a/spec/controllers/v2/versions_controller_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe V2::VersionsController do - before do - allow(Cocina).to receive(:find).and_return(Cocina.new(public_json)) - end - - let(:public_json) do - { - 'externalIdentifier' => druid, - 'structural' => { - 'contains' => [ - { - 'structural' => { - 'contains' => [ - { - 'filename' => 'image.jp2', - 'access' => { - 'view' => 'world', - 'download' => 'world' - } - } - ] - } - } - ] - } - } - end - - describe '#show' do - subject { get :show, params: { id: druid, file_name: 'image.jp2', version_id: } } - - let(:druid) { 'nr349ct7889' } - let(:version_id) { 'v1' } - let(:path) { 'spec/fixtures/nr/349/ct/7889/image.jp2' } - - it 'sends the file to the user' do - expect(controller).to receive(:send_file).with(path, disposition: :inline).and_call_original - subject - end - - it 'sends headers for content' do - expect(controller).to receive(:send_file).with(path, disposition: :attachment).and_call_original - get :show, params: { id: druid, file_name: 'image.jp2', download: 'any', version_id: } - expect(response.headers.to_h).to include 'content-length' => 11_043, 'accept-ranges' => 'bytes' - end - - it 'missing file returns 404 Not Found' do - expect(controller).to receive(:send_file).and_raise ActionController::MissingFile - expect(subject.status).to eq 404 - end - end -end diff --git a/spec/requests/versioned_file_spec.rb b/spec/requests/versioned_file_spec.rb index a67ae836..bc9fbc4f 100644 --- a/spec/requests/versioned_file_spec.rb +++ b/spec/requests/versioned_file_spec.rb @@ -35,7 +35,7 @@ describe 'OPTIONS options' do it 'permits Range headers for all origins' do - options "/file/#{druid}/#{version_id}/#{file_name}" + options "/v2/file/#{druid}/#{version_id}/#{file_name}" expect(response).to be_successful expect(response.headers['Access-Control-Allow-Origin']).to eq '*' expect(response.headers['Access-Control-Allow-Headers']).to include 'Range' @@ -69,13 +69,14 @@ end before do - allow_any_instance_of(V2::VersionsController).to receive(:send_file) + allow_any_instance_of(FileController).to receive(:send_file) .with('spec/fixtures/nr/349/ct/7889/path/to/image.jp2', disposition: :inline) end it 'returns a successful HTTP response' do get "/v2/file/#{druid}/#{version_id}/#{file_name}" expect(response).to be_successful + expect(Cocina).to have_received(:find).with(druid, version_id) end end @@ -83,6 +84,7 @@ it 'returns a 400 HTTP response' do get '/v2/file/xf680rd3068/v1/path/to/99999.jp2' expect(response).to have_http_status(:not_found) + expect(Cocina).to have_received(:find).with('xf680rd3068', 'v1') end end end