From 94c80663388de266de2a0a7938a030aeb3b40172 Mon Sep 17 00:00:00 2001
From: hackartisan <845363+hackartisan@users.noreply.github.com>
Date: Thu, 7 Sep 2023 14:03:53 -0400
Subject: [PATCH] Fix audio and video playback through valkyrie codepaths
(#6297)
* Save derivative pcdm_use when creating derivatives via valkyrie
closes #6294
* allowed mime_type to be passed to find file metadata
* Correctly pass mime_type
---
.../valkyrie_downloads_controller_behavior.rb | 19 +++++++++-----
app/models/hyrax/file_metadata.rb | 1 +
.../hyrax/file_set_derivatives_service.rb | 8 +++---
app/services/hyrax/valkyrie_upload.rb | 2 ++
.../file_sets/media_display/_audio.html.erb | 8 +++---
.../file_sets/media_display/_video.html.erb | 4 +--
.../hyrax/downloads_controller_spec.rb | 26 +++++++++++++++++++
.../file_set_derivatives_service_spec.rb | 14 +++++++---
.../valkyrie_persist_derivatives_spec.rb | 9 ++++---
9 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb b/app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb
index feedd4a1ca..4186161c00 100644
--- a/app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb
+++ b/app/controllers/concerns/hyrax/valkyrie_downloads_controller_behavior.rb
@@ -13,7 +13,8 @@ def send_file_contents_valkyrie(file_set)
response.headers["Accept-Ranges"] = "bytes"
self.status = 200
use = params.fetch(:file, :original_file).to_sym
- file_metadata = find_file_metadata(file_set: file_set, use: use)
+ 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)
file = Hyrax.storage_adapter.find_by(id: file_metadata.file_identifier)
@@ -60,15 +61,19 @@ def prepare_file_headers_valkyrie(metadata:, file:, inline: false)
self.content_type = metadata.mime_type
end
- def find_file_metadata(file_set:, use: :original_file)
- use = :thumbnail_file if use == :thumbnail
- begin
+ def find_file_metadata(file_set:, use: :original_file, mime_type: nil)
+ if mime_type.nil?
+ use = :thumbnail_file if use == :thumbnail
use = Hyrax::FileMetadata::Use.uri_for(use: use)
- rescue ArgumentError
- raise Hyrax::ObjectNotFoundError
+ results = Hyrax.custom_queries.find_many_file_metadata_by_use(resource: file_set, use: use)
+ else
+ files = Hyrax.custom_queries.find_files(file_set: file_set)
+ results = [files.find { |f| f.mime_type == mime_type }]
end
- results = Hyrax.custom_queries.find_many_file_metadata_by_use(resource: file_set, use: use)
+
results.first || raise(Hyrax::ObjectNotFoundError)
+ rescue ArgumentError
+ raise(Hyrax::ObjectNotFoundError)
end
end
end
diff --git a/app/models/hyrax/file_metadata.rb b/app/models/hyrax/file_metadata.rb
index cebfc34db8..69e24d3703 100644
--- a/app/models/hyrax/file_metadata.rb
+++ b/app/models/hyrax/file_metadata.rb
@@ -35,6 +35,7 @@ module Use
ORIGINAL_FILE = ::Valkyrie::Vocab::PCDMUse.OriginalFile
EXTRACTED_TEXT = ::Valkyrie::Vocab::PCDMUse.ExtractedText
THUMBNAIL = ::Valkyrie::Vocab::PCDMUse.ThumbnailImage
+ SERVICE_FILE = ::Valkyrie::Vocab::PCDMUse.ServiceFile
##
# @param use [RDF::URI, Symbol]
diff --git a/app/services/hyrax/file_set_derivatives_service.rb b/app/services/hyrax/file_set_derivatives_service.rb
index b6eaad9e46..e3874fd4c4 100644
--- a/app/services/hyrax/file_set_derivatives_service.rb
+++ b/app/services/hyrax/file_set_derivatives_service.rb
@@ -91,15 +91,15 @@ def create_office_document_derivatives(filename)
def create_audio_derivatives(filename)
Hydra::Derivatives::AudioDerivatives.create(filename,
- outputs: [{ label: 'mp3', format: 'mp3', url: derivative_url('mp3'), mime_type: 'audio/mpeg' },
- { label: 'ogg', format: 'ogg', url: derivative_url('ogg'), mime_type: 'audio/ogg' }])
+ outputs: [{ label: 'mp3', format: 'mp3', url: derivative_url('mp3'), mime_type: 'audio/mpeg', container: 'service_file' },
+ { label: 'ogg', format: 'ogg', url: derivative_url('ogg'), mime_type: 'audio/ogg', container: 'service_file' }])
end
def create_video_derivatives(filename)
Hydra::Derivatives::VideoDerivatives.create(filename,
outputs: [{ label: :thumbnail, format: 'jpg', url: derivative_url('thumbnail'), mime_type: 'image/jpeg' },
- { label: 'webm', format: 'webm', url: derivative_url('webm'), mime_type: 'video/webm' },
- { label: 'mp4', format: 'mp4', url: derivative_url('mp4'), mime_type: 'video/mp4' }])
+ { label: 'webm', format: 'webm', url: derivative_url('webm'), mime_type: 'video/webm', container: 'service_file' },
+ { label: 'mp4', format: 'mp4', url: derivative_url('mp4'), mime_type: 'video/mp4', container: 'service_file' }])
end
def create_image_derivatives(filename)
diff --git a/app/services/hyrax/valkyrie_upload.rb b/app/services/hyrax/valkyrie_upload.rb
index 0b599fdc5c..ad699a88f1 100644
--- a/app/services/hyrax/valkyrie_upload.rb
+++ b/app/services/hyrax/valkyrie_upload.rb
@@ -94,6 +94,8 @@ def set_file_use_ids(file_set, file_metadata)
file_set.thumbnail_id = file_metadata.id
when Hyrax::FileMetadata::Use::EXTRACTED_TEXT
file_set.extracted_text_id = file_metadata.id
+ when Hyrax::FileMetadata::Use::SERVICE_FILE
+ # do nothing
else
Hyrax.logger.warn "Unknown file use #{file_metadata.type} specified for #{file_metadata.file_identifier}"
end
diff --git a/app/views/hyrax/file_sets/media_display/_audio.html.erb b/app/views/hyrax/file_sets/media_display/_audio.html.erb
index c1f7fcc051..27b830013d 100644
--- a/app/views/hyrax/file_sets/media_display/_audio.html.erb
+++ b/app/views/hyrax/file_sets/media_display/_audio.html.erb
@@ -2,8 +2,8 @@
<div>
<h2 class="sr-only"><%= t('hyrax.file_set.show.downloadable_content.heading') %></h2>
<audio controls="controls" class="audiojs" style="width:100%" controlsList="nodownload" preload="auto">
- <source src="<%= hyrax.download_path(file_set, file: 'ogg') %>" type="audio/ogg" />
- <source src="<%= hyrax.download_path(file_set, file: 'mp3') %>" type="audio/mpeg" />
+ <source src="<%= hyrax.download_path(file_set, file: 'ogg', mime_type: 'audio/ogg') %>" type="audio/ogg" />
+ <source src="<%= hyrax.download_path(file_set, file: 'mp3', mime_type: 'audio/mpeg') %>" type="audio/mpeg" />
<%= t('hyrax.file_set.show.downloadable_content.audio_tag_not_supported') %>
</audio>
<%= link_to t('hyrax.file_set.show.downloadable_content.audio_link'),
@@ -15,8 +15,8 @@
<% else %>
<div>
<audio controls="controls" class="audiojs" style="width:100%" controlsList="nodownload" preload="auto">
- <source src="<%= hyrax.download_path(file_set, file: 'ogg') %>" type="audio/ogg" />
- <source src="<%= hyrax.download_path(file_set, file: 'mp3') %>" type="audio/mpeg" />
+ <source src="<%= hyrax.download_path(file_set, file: 'ogg', mime_type: 'audio/ogg') %>" type="audio/ogg" />
+ <source src="<%= hyrax.download_path(file_set, file: 'mp3', mime_type: 'audio/mpeg') %>" type="audio/mpeg" />
<%= t('hyrax.file_set.show.downloadable_content.audio_tag_not_supported') %>
</audio>
</div>
diff --git a/app/views/hyrax/file_sets/media_display/_video.html.erb b/app/views/hyrax/file_sets/media_display/_video.html.erb
index c4c4c3832f..77c859d25f 100644
--- a/app/views/hyrax/file_sets/media_display/_video.html.erb
+++ b/app/views/hyrax/file_sets/media_display/_video.html.erb
@@ -2,8 +2,8 @@
<div>
<h2 class="sr-only"><%= t('hyrax.file_set.show.downloadable_content.heading') %></h2>
<video controls="controls" class="video-js vjs-default-skin" style="width:100%" data-setup="{}" controlsList="nodownload" preload="auto">
- <source src="<%= hyrax.download_path(file_set, file: 'webm') %>" type="video/webm" />
- <source src="<%= hyrax.download_path(file_set, file: 'mp4') %>" type="video/mp4" />
+ <source src="<%= hyrax.download_path(file_set, file: 'webm', mime_type: 'video/webm') %>" type="video/webm" />
+ <source src="<%= hyrax.download_path(file_set, file: 'mp4', mime_type: 'video/mp4') %>" type="video/mp4" />
<%= t('hyrax.file_set.show.downloadable_content.video_tag_not_supported') %>
</video>
<%= link_to t('hyrax.file_set.show.downloadable_content.video_link'),
diff --git a/spec/controllers/hyrax/downloads_controller_spec.rb b/spec/controllers/hyrax/downloads_controller_spec.rb
index 5ef377e9aa..dc0edd462e 100644
--- a/spec/controllers/hyrax/downloads_controller_spec.rb
+++ b/spec/controllers/hyrax/downloads_controller_spec.rb
@@ -95,6 +95,32 @@
end
end
+ context 'with video file' do
+ let(:service_file_use) { Hyrax::FileMetadata::Use::SERVICE_FILE }
+ let(:file_path) { fixture_path + '/sample_mpeg4.mp4' }
+ let(:service_file_metadata) { FactoryBot.valkyrie_create(:hyrax_file_metadata, use: service_file_use, mime_type: 'video/webm', file_identifier: "disk://#{file_path}") }
+ let(:file_set) do
+ if Hyrax.config.use_valkyrie?
+ FactoryBot.valkyrie_create(:hyrax_file_set,
+ :in_work,
+ files: [original_file_metadata, service_file_metadata],
+ edit_users: [user],
+ visibility_setting: Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED)
+ else
+ create(:file_with_work, user: user, content: original_file)
+ end
+ end
+ before do
+ allow(subject).to receive(:authorize!).and_return(true)
+ allow(subject).to receive(:workflow_restriction?).and_return(false)
+ end
+
+ it 'accepts a mime_type param' do
+ get :show, params: { id: file_set, file: "webm", mime_type: 'video/webm' }
+ expect(response.body).to eq IO.binread(file_path)
+ end
+ end
+
context 'when restricted by workflow' do
before do
allow(subject).to receive(:authorize!).and_return(true)
diff --git a/spec/services/hyrax/file_set_derivatives_service_spec.rb b/spec/services/hyrax/file_set_derivatives_service_spec.rb
index de03c9fa78..e4dde98f0e 100644
--- a/spec/services/hyrax/file_set_derivatives_service_spec.rb
+++ b/spec/services/hyrax/file_set_derivatives_service_spec.rb
@@ -29,10 +29,16 @@
FactoryBot.valkyrie_create(:hyrax_file_metadata, :audio_file, file_set_id: SecureRandom.uuid)
end
- it "passes a mime-type to the audio derivatives service" do
+ it "passes a mime-type and container to the audio derivatives service" do
allow(Hydra::Derivatives::AudioDerivatives).to receive(:create)
described_class.new(valid_file_set).create_derivatives('foo')
- expect(Hydra::Derivatives::AudioDerivatives).to have_received(:create).with('foo', outputs: contain_exactly(hash_including(mime_type: 'audio/mpeg'), hash_including(mime_type: 'audio/ogg')))
+ expect(Hydra::Derivatives::AudioDerivatives).to have_received(:create).with(
+ 'foo',
+ outputs: contain_exactly(
+ hash_including(mime_type: 'audio/mpeg', container: 'service_file'),
+ hash_including(mime_type: 'audio/ogg', container: 'service_file')
+ )
+ )
end
end
@@ -47,8 +53,8 @@
expect(Hydra::Derivatives::VideoDerivatives).to have_received(:create).with(
'foo',
outputs: contain_exactly(
- hash_including(mime_type: 'video/mp4'),
- hash_including(mime_type: 'video/webm'),
+ hash_including(mime_type: 'video/mp4', container: 'service_file'),
+ hash_including(mime_type: 'video/webm', container: 'service_file'),
hash_including(mime_type: 'image/jpeg')
)
)
diff --git a/spec/services/hyrax/valkyrie_persist_derivatives_spec.rb b/spec/services/hyrax/valkyrie_persist_derivatives_spec.rb
index 0b132c85cf..f91ff013c8 100644
--- a/spec/services/hyrax/valkyrie_persist_derivatives_spec.rb
+++ b/spec/services/hyrax/valkyrie_persist_derivatives_spec.rb
@@ -28,14 +28,17 @@
context "when given a mime_type directive" do
let(:directives) do
- { url: "file:///app/samvera/hyrax-webapp/derivatives/#{id}-webm.webm",
- mime_type: 'video/webm' }
+ { url: "file:///app/samvera/hyrax-webapp/derivatives/#{id}-mp4.mp4",
+ mime_type: 'video/webm',
+ container: 'service_file' }
end
it 'adds the mime_type to the file metadata' do
described_class.call(stream, directives)
files = Hyrax.custom_queries.find_files(file_set: file_set)
- expect(files.first.mime_type).to eq 'video/webm'
+ file = files.first
+ expect(file.mime_type).to eq 'video/webm'
+ expect(file.pcdm_use).to eq [Hyrax::FileMetadata::Use::SERVICE_FILE]
end
end
end