don't wait on media object processing when importing a course

Just fire off the kaltura upload jobs in the background, so that the
import can continue. Attachments in the special media_objects export
folder will still be deleted when those uploads complete.

Note we still wait for canvas common cartidge imports, since we need the
completed media object imports in order to translate the rich text
content. We plan to address this limitation later.

test plan:

 * do a course import from a source other than canvas CC, verify that
   the import finishes without waiting on Kaltura.
 * do a course import from a canvas CC package with
   some audio/video. Verify that the import finishes after waiting on
   kaltura, but the attachments in /media_objects are removed as the
   kaltura jobs complete.

fixes #5383

Change-Id: I7dbf12c211fc332fe9d4f16bc4d31fd0656c1941
Reviewed-on: https://gerrit.instructure.com/8031
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Bracken Mosbacker <bracken@instructure.com>

app/models/course.rb

@@ -1603,6 +1603,13 @@ class Course < ActiveRecord::Base
   end
   private :process_migration_files
 
+  def import_media_objects(mo_attachments, migration)
+    wait_for_completion = (migration && migration.migration_settings[:worker_class] == CC::Importer::Canvas::Converter.name)
+    unless mo_attachments.blank?
+      MediaObject.add_media_files(mo_attachments, wait_for_completion)
+    end
+  end
+
   def import_from_migration(data, params, migration)
     params ||= {:copy=>{}}
     logger.debug "starting import"
@@ -1621,11 +1628,7 @@ class Course < ActiveRecord::Base
       process_migration_files(data, migration); migration.fast_update_progress(18)
       Attachment.process_migration(data, migration); migration.fast_update_progress(20)
       mo_attachments = self.imported_migration_items.find_all { |i| i.is_a?(Attachment) && i.media_entry_id.present? }
-      # we'll wait synchronously for the media objects to be uploaded, so that
-      # we have the media_ids that we need later.
-      unless mo_attachments.blank?
-        import_media_objects_and_attachments(mo_attachments)
-      end
+      import_media_objects(mo_attachments, migration)
     end
 
     # needs to happen after the files are processed, so that they are available in the syllabus
@@ -1712,29 +1715,6 @@ class Course < ActiveRecord::Base
     end
   end
 
-  def import_media_objects_and_attachments(mo_attachments)
-    MediaObject.add_media_files(mo_attachments, true)
-    # attachments in /media_objects were created on export, soley to
-    # download and include a media object in the export. now that they've
-    # been sent to kaltura, we can remove them.
-    failed_uploads, mo_attachments = mo_attachments.partition { |a| a.media_object.nil? }
-
-    unless failed_uploads.empty?
-      add_migration_warning(t('errors.import.kaltura', "There was an error importing Kaltura media objects. Some or all of your media was not imported."), failed_uploads.map(&:id).join(','))
-    end
-
-    to_remove = mo_attachments.find_all { |a| a.full_path.starts_with?(File.join(Folder::ROOT_FOLDER_NAME, CC::CCHelper::MEDIA_OBJECTS_FOLDER) + '/') }
-    to_remove.each do |a|
-      a.destroy(false)
-    end
-    folder = to_remove.last.folder if to_remove.last
-    if folder && folder.file_attachments.active.count == 0 && folder.active_sub_folders.count == 0
-      folder.destroy
-    end
-  rescue Exception => e
-    add_migration_warning(t('errors.import.kaltura', "There was an error importing Kaltura media objects. Some or all of your media was not imported."), e)
-  end
-
   def add_migration_warning(message, exception='')
     return unless @content_migration
     @content_migration.add_warning(message, exception)

app/models/media_object.rb

@@ -47,7 +47,7 @@ class MediaObject < ActiveRecord::Base
     end
     super
   end
-  
+
   # if wait_for_completion is true, this will wait SYNCHRONOUSLY for the bulk
   # upload to complete. Wrap it in a timeout if you ever want it to give up
   # waiting.
@@ -115,7 +115,7 @@ class MediaObject < ActiveRecord::Base
       end
     end
   end
-  
+
   def self.build_media_objects(data, root_account_id)
     root_account = Account.find_by_id(root_account_id)
     data[:entries].each do |entry|
@@ -128,19 +128,24 @@ class MediaObject < ActiveRecord::Base
         mo.context = attachment.context
         mo.attachment_id = attachment.id
         attachment.update_attribute(:media_entry_id, entry[:entryId])
+        # check for attachments that were created temporarily, just to import a media object
+        if attachment.full_path.starts_with?(File.join(Folder::ROOT_FOLDER_NAME, CC::CCHelper::MEDIA_OBJECTS_FOLDER) + '/')
+          attachment.destroy(false)
+        end
       end
       mo.context ||= mo.root_account
       mo.save
     end
   end
-  
+
   def self.refresh_media_files(bulk_upload_id, attachment_ids, root_account_id, attempt=0)
     client = Kaltura::ClientV3.new
     client.startSession(Kaltura::SessionType::ADMIN)
     res = client.bulkUploadGet(bulk_upload_id)
     if !res[:ready]
-      if attempt < 5
-        MediaObject.send_at(10.minute.from_now, :refresh_media_files, bulk_upload_id, attachment_ids, root_account_id, attempt + 1) 
+      if attempt < Setting.get('media_object_bulk_refresh_max_attempts', '5').to_i
+        wait_period = Setting.get('media_object_bulk_refresh_wait_period', '30').to_i
+        MediaObject.send_at(wait_period.minutes.from_now, :refresh_media_files, bulk_upload_id, attachment_ids, root_account_id, attempt + 1)
       else
         # if it fails, then the attachment should no longer consider itself kalturable
         Attachment.update_all({:media_entry_id => nil}, "id IN (#{attachment_ids.join(",")}) OR root_attachment_id IN (#{attachment_ids.join(",")})") unless attachment_ids.empty?

lib/cc/importer/cc_worker.rb

@@ -30,6 +30,7 @@ module Canvas::Migration
           
           converter_class = Worker::get_converter(settings)
           converter = converter_class.new(settings)
+
           course = converter.export
           export_folder_path = course[:export_folder_path]
           overview_file_path = course[:overview_file_path]
@@ -43,6 +44,7 @@ module Canvas::Migration
             Canvas::Migration::Worker::clear_exported_data(export_folder_path)
           end
 
+          cm.migration_settings[:worker_class] = converter_class.name
           cm.migration_settings[:migration_ids_to_import] = {:copy=>{:assessment_questions=>true}}
           cm.workflow_state = :exported
           cm.progress = 0

spec/lib/cc/importer/cc_worker_spec.rb

@@ -0,0 +1,30 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb')
+
+describe Canvas::Migration::Worker::CCWorker do
+  it "should set the worker_class on the migration" do
+    cm = ContentMigration.create!(:migration_settings => { :no_archive_file => true })
+    Canvas::Migration::Worker.expects(:get_converter).with(anything).returns(CC::Importer::Canvas::Converter)
+    CC::Importer::Canvas::Converter.any_instance.expects(:export).returns({})
+    worker = Canvas::Migration::Worker::CCWorker.new(cm.id)
+    worker.perform().should == true
+    cm.reload.migration_settings[:worker_class].should == 'CC::Importer::Canvas::Converter'
+  end
+end

spec/models/course_spec.rb

@@ -2724,6 +2724,25 @@ describe Course, "section_visibility" do
   end
 end
 
+describe Course, ".import_from_migration" do
+  before do
+    attachment_model(:uploaded_data => stub_file_data('test.m4v', 'asdf', 'video/mp4'))
+    course_with_teacher
+  end
+
+  it "should wait for media objects on canvas cartridge import" do
+    migration = mock(:migration_settings => { 'worker_class' => 'CC::Importer::Canvas::Converter' }.with_indifferent_access)
+    MediaObject.expects(:add_media_files).with([@attachment], true)
+    @course.import_media_objects([@attachment], migration)
+  end
+
+  it "should not wait for media objects on other import" do
+    migration = mock(:migration_settings => { 'worker_class' => 'CC::Importer::Standard::Converter' }.with_indifferent_access)
+    MediaObject.expects(:add_media_files).with([@attachment], false)
+    @course.import_media_objects([@attachment], migration)
+  end
+end
+
 describe Course, "enrollments" do
   it "should update enrollments' root_account_id when necessary" do
     a1 = Account.create!

spec/models/media_object_spec.rb

@@ -32,4 +32,22 @@ describe MediaObject do
       lambda { MediaObject.find_by_media_id('fjdksl') }.should raise_error
     end
   end
+
+  describe ".build_media_objects" do
+    it "should delete attachments created temporarily for import" do
+      course
+      folder = Folder.assert_path(CC::CCHelper::MEDIA_OBJECTS_FOLDER, @course)
+      @a1 = attachment_model(:folder => folder, :uploaded_data => stub_file_data('video1.mp4', nil, 'video/mp4'))
+      @a2 = attachment_model(:context => @course, :uploaded_data => stub_file_data('video1.mp4', nil, 'video/mp4'))
+      data = {
+        :entries => [
+          { :originalId => @a1.id, },
+          { :originalId => @a2.id, },
+        ],
+      }
+      MediaObject.build_media_objects(data, Account.default.id)
+      @a1.reload.file_state.should == 'deleted'
+      @a2.reload.file_state.should == 'available'
+    end
+  end
 end