Skip to content

Commit

Permalink
Merge pull request #5525 from avalonmediasystem/speedy_playlists_redux
Browse files Browse the repository at this point in the history
Speed up playlist manifest generation
  • Loading branch information
cjcolvar authored Dec 15, 2023
2 parents edc8c68 + 2141d74 commit c0d8cba
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 28 deletions.
2 changes: 1 addition & 1 deletion app/controllers/media_objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ def manifest
@media_object = SpeedyAF::Proxy::MediaObject.find(params[:id])
authorize! :read, @media_object

stream_info_hash = secure_stream_infos(master_file_presenters, @media_object.id)
stream_info_hash = secure_stream_infos(master_file_presenters, [@media_object])
canvas_presenters = master_file_presenters.collect { |mf| IiifCanvasPresenter.new(master_file: mf, stream_info: stream_info_hash[mf.id]) }
presenter = IiifManifestPresenter.new(media_object: @media_object, master_files: canvas_presenters, lending_enabled: lending_enabled?(@media_object))

Expand Down
24 changes: 16 additions & 8 deletions app/controllers/playlists_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,23 @@ def destroy
def manifest
authorize! :read, @playlist

# Fetch all master files related to the playlist items in a single SpeedyAF::Base.where
master_file_ids = @playlist.items.collect { |item| item.clip.master_file_id }
master_files = SpeedyAF::Proxy::MasterFile.where("id:#{master_file_ids.join(' id:')}", load_reflections: true)
media_objects = master_files.collect(&:media_object).uniq(&:id)

# This small optimization relies on the assumption that can? :read, master_file is the same as can? :read, master_file.media_object
# This only optimizes the case where multiple playlist items come from the same media object
cannot_read_hash = {}
media_objects.each { |mo| cannot_read_hash[mo.id] = cannot?(:read, mo) }

# Condense secure_streams into single call using master_files
stream_info_hash = secure_stream_infos(master_files, media_objects)

canvas_presenters = @playlist.items.collect do |item|
@master_file = item.clip.master_file
cannot_read_item = cannot? :read, @master_file
stream_info = if @master_file.nil?
nil
else
secure_streams(@master_file.stream_details, @master_file.media_object_id)
end
IiifPlaylistCanvasPresenter.new(playlist_item: item, stream_info: stream_info, cannot_read_item: cannot_read_item)
master_file = master_files.find { |mf| mf.id == item.clip.master_file_id }
cannot_read_item = master_file.nil? || cannot_read_hash[master_file.media_object_id]
IiifPlaylistCanvasPresenter.new(playlist_item: item, stream_info: stream_info_hash[master_file&.id], cannot_read_item: cannot_read_item, master_file: master_file)
end

can_edit_playlist = can? :edit, @playlist
Expand Down
26 changes: 16 additions & 10 deletions app/helpers/security_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,21 @@ def secure_streams(stream_info, media_object_id)
# media_object CDL check only happens once
# session tokens retrieved in batch then passed into add_stream_url
# Returns Hash[MasterFile.id, stream_info]
def secure_stream_infos(master_files, media_object_id)
def secure_stream_infos(master_files, media_objects)
stream_info_hash = {}
if not_checked_out?(media_object_id)
master_files.each { |mf| stream_info_hash[mf.id] = mf.stream_details }
else
stream_tokens = StreamToken.get_session_tokens_for(session: session, targets: master_files.map(&:id))
stream_token_hash = stream_tokens.pluck(:target, :token).to_h
master_files.each { |mf| stream_info_hash[mf.id] = secure_stream_info(mf.stream_details, stream_token_hash[mf.id]) }
end

not_checked_out_hash = {}
mo_ids = master_files.collect(&:media_object_id)
mo_ids.each { |mo_id| not_checked_out_hash[mo_id] ||= not_checked_out?(mo_id, media_object: media_objects.find {|mo| mo.id == mo_id}) }
not_checked_out_master_files = master_files.select { |mf| not_checked_out_hash[mf.media_object_id] }
checked_out_master_files = master_files - not_checked_out_master_files

not_checked_out_master_files.each { |mf| stream_info_hash[mf.id] = mf.stream_details }

stream_tokens = StreamToken.get_session_tokens_for(session: session, targets: checked_out_master_files.map(&:id))
stream_token_hash = stream_tokens.pluck(:target, :token).to_h
checked_out_master_files.each { |mf| stream_info_hash[mf.id] = secure_stream_info(mf.stream_details, stream_token_hash[mf.id]) }

stream_info_hash
end

Expand All @@ -60,7 +66,7 @@ def add_stream_url(stream_info, token: nil)

private

def not_checked_out?(media_object_id)
lending_enabled?(SpeedyAF::Proxy::MediaObject.find(media_object_id)) && Checkout.checked_out_to_user(media_object_id, current_user&.id).empty?
def not_checked_out?(media_object_id, media_object: nil)
lending_enabled?(media_object || SpeedyAF::Proxy::MediaObject.find(media_object_id)) && Checkout.checked_out_to_user(media_object_id, current_user&.id).empty?
end
end
7 changes: 5 additions & 2 deletions app/models/avalon_annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,13 @@ def title_default!
self.title = master_file.embed_title if self.title.nil? && master_file.present?
end

def master_file_id
@master_file_id ||= CGI::unescape(self.source.split('/').last) if self.source
end

# Sets the class variable @master_file by finding the master referenced in the source uri
def master_file
@master_file ||= SpeedyAF::Proxy::MasterFile.find(CGI::unescape(self.source.split('/').last)) if self.source
@master_file ||= SpeedyAF::Proxy::MasterFile.find(master_file_id) if master_file_id
end

def master_file=(value)
Expand All @@ -74,5 +78,4 @@ def mediafragment_uri
rescue
"#{master_file&.rdf_uri}?t=#{start_time},#{end_time}"
end

end
7 changes: 4 additions & 3 deletions app/models/iiif_playlist_canvas_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ class IiifPlaylistCanvasPresenter
attr_reader :playlist_item, :stream_info, :cannot_read_item
attr_accessor :media_fragment

def initialize(playlist_item:, stream_info:, cannot_read_item: false, media_fragment: nil)
def initialize(playlist_item:, stream_info:, cannot_read_item: false, media_fragment: nil, master_file: nil)
@playlist_item = playlist_item
@stream_info = stream_info
@cannot_read_item = cannot_read_item
@media_fragment = media_fragment
@master_file = master_file
end

delegate :id, to: :playlist_item
Expand All @@ -36,7 +37,7 @@ def to_s
end

def master_file
playlist_item.clip.master_file
@master_file ||= playlist_item.clip.master_file
end

def part_of
Expand Down Expand Up @@ -177,7 +178,7 @@ def simple_iiif_range(label = stream_info[:embed_title])
IiifManifestRange.new(
label: { "none" => [label] },
items: [
IiifPlaylistCanvasPresenter.new(playlist_item: playlist_item, stream_info: stream_info, media_fragment: fragment_identifier)
IiifPlaylistCanvasPresenter.new(playlist_item: playlist_item, stream_info: stream_info, media_fragment: fragment_identifier, master_file: master_file)
]
)
end
Expand Down
3 changes: 2 additions & 1 deletion app/models/stream_token.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ def self.get_session_tokens_for(session: {}, targets: [])
# all attributes must be present for all items in upsert array
attrs[:id] = existing_token_hash[attrs[:token]] if existing_token_hash[attrs[:token]].present?
end
result = StreamToken.upsert_all(token_attributes)

result = token_attributes.present? ? StreamToken.upsert_all(token_attributes) : []

# Fetch StreamToken fresh so they can be put into session and returned
tokens = StreamToken.where(id: result.to_a.pluck("id"))
Expand Down
6 changes: 3 additions & 3 deletions spec/helpers/security_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@
before { allow(Settings.controlled_digital_lending).to receive(:enable).and_return(false) }

it 'rewrites urls in the stream_infos' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info_hash = helper.secure_stream_infos([master_file], [media_object])
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
Expand All @@ -190,7 +190,7 @@
before { FactoryBot.create(:checkout, media_object_id: media_object.id, user_id: user.id)}

it 'rewrites urls in the stream_infos' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info_hash = helper.secure_stream_infos([master_file], [media_object])
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
Expand All @@ -202,7 +202,7 @@

context 'the user does not have the item checked out' do
it 'does not rewrite urls in the stream_info' do
stream_info_hash = helper.secure_stream_infos([master_file], media_object.id)
stream_info_hash = helper.secure_stream_infos([master_file], [media_object])
stream_info = stream_info_hash[master_file.id]
[:stream_hls].each do |protocol|
stream_info[protocol].each do |quality|
Expand Down

0 comments on commit c0d8cba

Please sign in to comment.