ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix exports when media object is also in files 

when exporting rich text containing media objects, these
get written to media_objects/<media_id> in the export
package--unless the media object is linked to a file,
in which case we declined to write a second copy

the problem is, media links in the export were rewritten
to use the media_objects/<media_id> path anyway, and
these links were broken if the file was exported.

to fix this, we will always use the file's path in the
export if the media object is linked to a file

test plan:
 - have canvas talking to a working notorious instance
 - upload/record a video in a rich text (i.e., in a page)
 - upload a video to files, and embed the video in a page
 - ensure the videos play
 - ensure the course can be copied and the videos play
   (they will keep the same media id)
 - ensure the course can be exported to a common cartridge
   and re-imported to a new course, and the videos play
   (they will have new media ids)

also:
 - enable offline web exports in the account
 - add the two pages to a module
 - perform an offline web export from the modules page
 - extract the package and open index.html in a browser
 - the videos should play
   (note that they look weird in safari; this is something
   we should address separately)

fixes LS-1729

Change-Id: Ib913ee990886ea734f3a29893705a9d382e46e0c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256655
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Nate Armstrong <narmstrong@instructure.com>
Reviewed-by: Nate Armstrong <narmstrong@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2021-01-13 10:32:04 -07:00
parent 8b497c373c
commit 812eae65b6
4 changed files with 68 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
lib/cc/cc_helper.rb
View File

@ -317,9 +317,9 @@ module CCHelper
obj = MediaObject.by_media_id(media_id).first
if obj && migration_id = @key_generator.create_key(obj)
@used_media_objects << obj
info = CCHelper.media_object_info(obj, nil, media_object_flavor)
info = CCHelper.media_object_info(obj, course: @course, flavor: media_object_flavor)
@media_object_infos[obj.id] = info
anchor['href'] = File.join(WEB_CONTENT_TOKEN, MEDIA_OBJECTS_FOLDER, info[:filename])
anchor['href'] = File.join(WEB_CONTENT_TOKEN, info[:path])
end
end
@ -329,9 +329,9 @@ module CCHelper
obj = MediaObject.by_media_id(media_id).take
if obj && migration_id = @key_generator.create_key(obj)
@used_media_objects << obj
info = CCHelper.media_object_info(obj, nil, media_object_flavor)
info = CCHelper.media_object_info(obj, course: @course, flavor: media_object_flavor)
@media_object_infos[obj.id] = info
iframe['src'] = File.join(WEB_CONTENT_TOKEN, MEDIA_OBJECTS_FOLDER, info[:filename])
iframe['src'] = File.join(WEB_CONTENT_TOKEN, info[:path])
end
end
@ -359,7 +359,7 @@ module CCHelper
end
end
def self.media_object_info(obj, client = nil, flavor = nil)
def self.media_object_info(obj, course: nil, client: nil, flavor: nil)
unless client
client = CanvasKaltura::ClientV3.new
client.startSession(CanvasKaltura::SessionType::ADMIN)
@ -371,10 +371,17 @@ module CCHelper
else
asset = client.flavorAssetGetOriginalAsset(obj.media_id)
end
# we use the media_id as the export filename, since it is guaranteed to
# be unique
filename = "#{obj.media_id}.#{asset[:fileExt]}" if asset
{ :asset => asset, :filename => filename }
attachment = course && obj.attachment_id && course.attachments.not_deleted.find_by_id(obj.attachment_id)
path = if attachment
# if the media object is associated with a file in the course, use the file's path in the export, to avoid exporting it twice
attachment.full_display_path.sub(/^#{Regexp.quote(Folder::ROOT_FOLDER_NAME)}/, '')
else
# otherwise export to a file named after the media id
filename = obj.media_id
filename += ".#{asset[:fileExt]}" if asset
File.join(MEDIA_OBJECTS_FOLDER, filename)
end
{ :asset => asset, :path => path }
end
# sub_path is the last part of a file url: /courses/1/files/1(/download)

46
lib/cc/web_resources.rb
View File

@ -34,7 +34,7 @@ module CC
course_folder = Folder.root_folders(@course).first
files_with_metadata = { :folders => [], :files => [] }
@added_attachment_ids = Set.new
@added_attachments = {}
zipper = ContentZipper.new
zipper.user = @user
@ -47,8 +47,8 @@ module CC
next
end
@added_attachment_ids << file.id
path = File.join(folder_names, file.display_name)
@added_attachments[file.id] = path
migration_id = create_key(file)
if file_or_folder_restricted?(file) || file.usage_rights || file.display_name != file.unencoded_filename
files_with_metadata[:files] << [file, migration_id]
@ -205,7 +205,7 @@ module CC
# check to make sure we don't export more than 4 gigabytes of media objects
total_size = 0
html_content_exporter.used_media_objects.each do |obj|
next if @added_attachment_ids&.include?(obj.attachment_id)
next if @added_attachments&.key?(obj.attachment_id)
info = html_content_exporter.media_object_infos[obj.id]
next unless info && info[:asset] && info[:asset][:size]
@ -223,33 +223,35 @@ module CC
tracks = {}
html_content_exporter.used_media_objects.each do |obj|
next if @added_attachment_ids&.include?(obj.attachment_id)
begin
migration_id = create_key(obj)
info = html_content_exporter.media_object_infos[obj.id]
next unless info && info[:asset]
unless CanvasKaltura::ClientV3::ASSET_STATUSES[info[:asset][:status]] == :READY &&
url = (client.flavorAssetGetPlaylistUrl(obj.media_id, info[:asset][:id]) || client.flavorAssetGetDownloadUrl(info[:asset][:id]))
add_error(I18n.t('course_exports.errors.media_file', "A media file failed to export"))
next
end
path = File.join(CCHelper::WEB_RESOURCES_FOLDER, info[:path])
path = File.join(CCHelper::WEB_RESOURCES_FOLDER, CCHelper::MEDIA_OBJECTS_FOLDER, info[:filename])
CanvasHttp.get(url) do |http_response|
raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i) unless http_response.code.to_i == 200
@zip_file.get_output_stream(path) do |stream|
http_response.read_body(stream)
# download from kaltura if the file wasn't already exported here in add_course_files
if !@added_attachments || @added_attachments[obj.attachment_id] != path
unless CanvasKaltura::ClientV3::ASSET_STATUSES[info[:asset][:status]] == :READY &&
url = (client.flavorAssetGetPlaylistUrl(obj.media_id, info[:asset][:id]) || client.flavorAssetGetDownloadUrl(info[:asset][:id]))
add_error(I18n.t('course_exports.errors.media_file', "A media file failed to export"))
next
end
end
@resources.resource(
"type" => CCHelper::WEBCONTENT,
:identifier => migration_id,
:href => path
) do |res|
res.file(:href => path)
CanvasHttp.get(url) do |http_response|
raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i) unless http_response.code.to_i == 200
@zip_file.get_output_stream(path) do |stream|
http_response.read_body(stream)
end
end
@resources.resource(
"type" => CCHelper::WEBCONTENT,
:identifier => migration_id,
:href => path
) do |res|
res.file(:href => path)
end
end
process_media_tracks(tracks, migration_id, obj, path)

4
spec/lib/cc/cc_exporter_spec.rb
View File

@ -363,7 +363,7 @@ describe "Common Cartridge exporting" do
allow(CC::CCHelper).to receive(:media_object_info).and_return({
:asset => { :id => "some-kaltura-id", :size => 1234, :status => '2' },
:filename => "some-kaltura-id"
:path => "media_objects/some-kaltura-id"
})
allow(CanvasKaltura::ClientV3).to receive(:config).and_return({})
allow(CanvasKaltura::ClientV3).to receive(:startSession)
@ -566,7 +566,7 @@ describe "Common Cartridge exporting" do
allow_any_instance_of(CanvasKaltura::ClientV3).to receive(:startSession)
allow_any_instance_of(CanvasKaltura::ClientV3).to receive(:flavorAssetGetPlaylistUrl).and_return('http://www.example.com/blah.flv')
stub_request(:get, 'http://www.example.com/blah.flv').to_return(body: "", status: 200)
allow(CC::CCHelper).to receive(:media_object_info).and_return({asset: {id: 1, status: '2'}, filename: 'blah.flv'})
allow(CC::CCHelper).to receive(:media_object_info).and_return({asset: {id: 1, status: '2'}, path: 'blah.flv'})
obj = @course.media_objects.create! media_id: '0_deadbeef'
track = obj.media_tracks.create! kind: 'subtitles', locale: 'tlh', content: "Hab SoSlI' Quch!"
page = @course.wiki_pages.create!(:title => "wiki", :body => "ohai")

26
spec/lib/cc/cc_helper_spec.rb
View File

@ -105,6 +105,32 @@ describe CC::CCHelper do
expect(@exporter.media_object_infos[@obj.id][:asset][:id]).to eq 'one'
end
it "links media to exported file if it exists" do
folder = folder_model(name: 'something', context: @course)
att = attachment_model(display_name: 'lolcats.mp4', context: @course, folder: folder, uploaded_data: stub_file_data('lolcats_.mp4', '...', 'video/mp4'))
@obj.attachment = att
@obj.save!
@exporter = CC::CCHelper::HtmlContentExporter.new(@course, @user)
html = %{<iframe style="width: 400px; height: 225px; display: inline-block;" title="this is a media comment" data-media-type="video" src="http://example.com/media_objects_iframe/abcde?type=video" allowfullscreen="allowfullscreen" allow="fullscreen" data-media-id="abcde"></iframe>}
translated = @exporter.html_content(html)
expect(translated).to include %{src="%24IMS-CC-FILEBASE%24/something/lolcats.mp4"}
end
it "does not link media to file in another course" do
temp = @course
other_course = course_factory
folder = folder_model(name: 'something', context: other_course)
att = attachment_model(display_name: 'lolcats.mp4', context: other_course, folder: folder, uploaded_data: stub_file_data('lolcats_.mp4', '...', 'video/mp4'))
@obj.attachment = att
@obj.save!
@course = temp
@exporter = CC::CCHelper::HtmlContentExporter.new(@course, @user)
html = %{<iframe style="width: 400px; height: 225px; display: inline-block;" title="this is a media comment" data-media-type="video" src="http://example.com/media_objects_iframe/abcde?type=video" allowfullscreen="allowfullscreen" allow="fullscreen" data-media-id="abcde"></iframe>}
translated = @exporter.html_content(html)
expect(translated).to include %{src="%24IMS-CC-FILEBASE%24/media_objects/abcde.mp4"}
end
it "ignores new RCE media iframes with an unknown media id" do
@exporter = CC::CCHelper::HtmlContentExporter.new(@course, @user)
html = %{<iframe style="width: 400px; height: 225px; display: inline-block;" title="this is a media comment" data-media-type="video" src="http://example.com/media_objects_iframe/deadbeef?type=video" allowfullscreen="allowfullscreen" allow="fullscreen" data-media-id="deadbeef"></iframe>}