From ad18235015a2ba469e6c133de4351c8c3a5760c7 Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Thu, 9 Nov 2017 14:33:32 -0700 Subject: [PATCH] refactor package content management extract PackageRoot to handle common tasks on extracted packages, such as translating between absolute and package-relative paths, and enumerating contents test plan: - ensure the package attached to the ticket is handled correctly - regression test course copies and Canvas cartridge imports refs ADMIN-528 Change-Id: I76b67076a48c8acedcde523d46aac8e2ccef76d0 Reviewed-on: https://gerrit.instructure.com/132389 Tested-by: Jenkins Reviewed-by: James Williams QA-Review: Deepeeca Soundarrajan Product-Review: Jeremy Stanley --- .../lib/moodle_importer/converter.rb | 2 +- .../lib/qti/assessment_item_converter.rb | 6 +-- .../lib/qti/assessment_test_converter.rb | 6 +-- .../plugins/qti_exporter/lib/qti/converter.rb | 10 ++-- lib/canvas/migration/archive.rb | 10 ++-- lib/canvas/migration/migrator.rb | 13 +++-- .../converters/assignment_epub_converter.rb | 4 +- .../epub/converters/cartridge_converter.rb | 2 +- .../epub/converters/module_epub_converter.rb | 2 +- .../epub/converters/quiz_epub_converter.rb | 4 +- .../epub/converters/topic_epub_converter.rb | 4 +- .../epub/converters/wiki_epub_converter.rb | 2 +- .../importer/canvas/assignment_converter.rb | 4 +- lib/cc/importer/canvas/converter.rb | 4 +- lib/cc/importer/canvas/course_settings.rb | 2 +- lib/cc/importer/canvas/quiz_converter.rb | 2 +- .../canvas/quiz_metadata_converter.rb | 2 +- .../importer/canvas/tool_profile_converter.rb | 2 +- lib/cc/importer/canvas/topic_converter.rb | 4 +- .../importer/canvas/webcontent_converter.rb | 2 +- lib/cc/importer/canvas/wiki_converter.rb | 2 +- lib/cc/importer/standard/converter.rb | 6 +-- lib/cc/importer/standard/quiz_converter.rb | 2 +- .../importer/standard/webcontent_converter.rb | 4 +- lib/package_root.rb | 52 ++++++++++++++++++ .../canvas/tool_profile_converter_spec.rb | 2 +- spec/lib/package_root_spec.rb | 53 +++++++++++++++++++ 27 files changed, 161 insertions(+), 47 deletions(-) create mode 100644 lib/package_root.rb create mode 100644 spec/lib/package_root_spec.rb diff --git a/gems/plugins/moodle_importer/lib/moodle_importer/converter.rb b/gems/plugins/moodle_importer/lib/moodle_importer/converter.rb index 9062c7844d1..3fc0a78d148 100644 --- a/gems/plugins/moodle_importer/lib/moodle_importer/converter.rb +++ b/gems/plugins/moodle_importer/lib/moodle_importer/converter.rb @@ -23,7 +23,7 @@ module Moodle def export(to_export = Canvas::Migration::Migrator::SCRAPE_ALL_HASH) unzip_archive - migrator = Moodle2CC::Migrator.new @unzipped_file_path, Dir.mktmpdir, 'format' => 'canvas', 'logger' => self + migrator = Moodle2CC::Migrator.new @package_root.root_path, Dir.mktmpdir, 'format' => 'canvas', 'logger' => self migrator.migrate if migrator.last_error diff --git a/gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb b/gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb index 8a5bb7c21fd..b9a18377a37 100644 --- a/gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb +++ b/gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb @@ -28,7 +28,7 @@ class AssessmentItemConverter DEFAULT_POINTS_POSSIBLE = 1 UNSUPPORTED_TYPES = ['File Upload', 'Hot Spot', 'Quiz Bowl', 'WCT_JumbledSentence'] - attr_reader :base_dir, :identifier, :href, :interaction_type, :title, :question + attr_reader :package_root, :identifier, :href, :interaction_type, :title, :question def initialize(opts) @log = Canvas::Migration::logger @@ -44,9 +44,9 @@ class AssessmentItemConverter end if @manifest_node - @base_dir = opts[:base_dir] + @package_root = PackageRoot.new(opts[:base_dir]) @identifier = @manifest_node['identifier'] - @href = File.join(@base_dir, @manifest_node['href']) + @href = @package_root.item_path(@manifest_node['href']) if title = @manifest_node.at_css('title langstring') || title = @manifest_node.at_css('xmlns|title xmlns|langstring', 'xmlns' => Qti::Converter::IMS_MD) @title = title.text end diff --git a/gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb b/gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb index d64149eaad7..133a6ce783b 100644 --- a/gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb +++ b/gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb @@ -23,13 +23,13 @@ class AssessmentTestConverter include HtmlHelper DEFAULT_POINTS_POSSIBLE = 1 - attr_reader :base_dir, :identifier, :href, :interaction_type, :title, :quiz + attr_reader :package_root, :identifier, :href, :interaction_type, :title, :quiz def initialize(manifest_node, base_dir, opts={}) @log = Canvas::Migration::logger @manifest_node = manifest_node - @base_dir = base_dir - @href = File.join(@base_dir, @manifest_node['href']) + @package_root = PackageRoot.new(base_dir) + @href = @package_root.item_path(@manifest_node['href']) @converted_questions = opts[:converted_questions] @opts = opts diff --git a/gems/plugins/qti_exporter/lib/qti/converter.rb b/gems/plugins/qti_exporter/lib/qti/converter.rb index debd2ae1fc7..04b3f4b1c11 100644 --- a/gems/plugins/qti_exporter/lib/qti/converter.rb +++ b/gems/plugins/qti_exporter/lib/qti/converter.rb @@ -49,8 +49,8 @@ class Converter < Canvas::Migration::Migrator def export unzip_archive - if Converter.is_qti_2(File.join(@unzipped_file_path, MANIFEST_FILE)) - @dest_dir_2_1 = @unzipped_file_path + if Converter.is_qti_2(@package_root.item_path(MANIFEST_FILE)) + @dest_dir_2_1 = @package_root.root_path @converted = true else run_qti_converter @@ -61,7 +61,7 @@ class Converter < Canvas::Migration::Migrator @course[:assessment_questions] = convert_questions(:file_path_map => path_map, :flavor => @flavor) @course[:assessments] = convert_assessments(@course[:assessment_questions][:assessment_questions]) - original_manifest_path = File.join(@unzipped_file_path, MANIFEST_FILE) + original_manifest_path = @package_root.item_path(MANIFEST_FILE) if File.exist?(original_manifest_path) @manifest = Nokogiri::XML(File.open(original_manifest_path)) post_process_assessments # bring in canvas metadata if available @@ -94,7 +94,7 @@ class Converter < Canvas::Migration::Migrator def run_qti_converter # convert to 2.1 @dest_dir_2_1 = Dir.mktmpdir(QTI_2_OUTPUT_PATH) - command = Qti.get_conversion_command(@dest_dir_2_1, @unzipped_file_path) + command = Qti.get_conversion_command(@dest_dir_2_1, @package_root.root_path) logger.debug "Running migration command: #{command}" python_std_out = `#{command}` @@ -156,7 +156,7 @@ class Converter < Canvas::Migration::Migrator end def apply_respondus_settings - settings_path = File.join(@unzipped_file_path, 'settings.xml') + settings_path = @package_root.item_path('settings.xml') if File.file?(settings_path) doc = Nokogiri::XML(File.open(settings_path)) end diff --git a/lib/canvas/migration/archive.rb b/lib/canvas/migration/archive.rb index 46014f8e45a..0b704d8a6b4 100644 --- a/lib/canvas/migration/archive.rb +++ b/lib/canvas/migration/archive.rb @@ -60,7 +60,7 @@ module Canvas::Migration zip_file.read(nest_entry_if_needed(entry)) else unzip_archive - path = File.join(self.unzipped_file_path, entry) + path = package_root.item_path(entry) File.exist?(path) && File.read(path) end end @@ -72,7 +72,7 @@ module Canvas::Migration # if it's not an actual zip file # just extract the package (or try to) and look for the file unzip_archive - File.exist?(File.join(self.unzipped_file_path, entry)) + File.exist?(package_root.item_path(entry)) end end @@ -109,6 +109,10 @@ module Canvas::Migration @unzipped_file_path end + def package_root + @package_root ||= PackageRoot.new(self.unzipped_file_path) + end + def get_converter Canvas::Migration::PackageIdentifier.new(self).get_converter end @@ -140,7 +144,7 @@ module Canvas::Migration # it into the directory with the given file name def prepare_cartridge_file(file_name='imsmanifest.xml') if self.path.ends_with?('xml') - FileUtils::cp(self.path, File.join(self.unzipped_file_path, file_name)) + FileUtils::cp(self.path, package_root.item_path(file_name)) else unzip_archive end diff --git a/lib/canvas/migration/migrator.rb b/lib/canvas/migration/migrator.rb index d2a88fe9cbe..b4a8396a34e 100644 --- a/lib/canvas/migration/migrator.rb +++ b/lib/canvas/migration/migrator.rb @@ -21,7 +21,8 @@ class Migrator include MigratorHelper SCRAPE_ALL_HASH = { 'course_outline' => true, 'announcements' => true, 'assignments' => true, 'goals' => true, 'rubrics' => true, 'web_links' => true, 'learning_modules' => true, 'calendar_events' => true, 'calendar_start' => nil, 'calendar_end' => nil, 'discussions' => true, 'assessments' => true, 'question_bank' => true, 'all_files' => true, 'groups' => true, 'assignment_groups' => true, 'tasks' => true, 'wikis' => true } - attr_accessor :course, :unzipped_file_path, :extra_settings, :total_error_count + # TODO remove unzipped_file_path once plugins stop using it + attr_accessor :course, :unzipped_file_path, :extra_settings, :total_error_count, :package_root attr_reader :base_export_dir, :manifest, :import_objects, :settings def initialize(settings, migration_type) @@ -35,12 +36,16 @@ class Migrator if @settings[:unzipped_file_path] @unzipped = true + # TODO remove unzipped_file_path once plugins stop using it @unzipped_file_path = @settings[:unzipped_file_path] + @package_root = PackageRoot.new(@unzipped_file_path) elsif !@settings[:no_archive_file] @archive = @settings[:archive] || Canvas::Migration::Archive.new(@settings) @archive_file = @archive.file + # TODO remove unzipped_file_path once plugins stop using it @unzipped_file_path = @archive.unzipped_file_path @archive_file_path = @archive.path + @package_root = PackageRoot.new(@unzipped_file_path) end @base_export_dir = @settings[:base_download_dir] || find_export_dir @@ -69,7 +74,7 @@ class Migrator end def get_full_path(file_name) - File.join(@unzipped_file_path, file_name) if file_name + @package_root.item_path(file_name) if file_name.present? end def move_archive_to(full_path) @@ -81,13 +86,13 @@ class Migrator end def package_course_files(base_dir=nil) - base_dir ||= @unzipped_file_path + package_root = base_dir ? PackageRoot.new(base_dir) : @package_root zip_file = File.join(@base_export_dir, MigratorHelper::ALL_FILES_ZIP) make_export_dir Zip::File.open(zip_file, 'w') do |zipfile| @course[:file_map].each_value do |val| - file_path = File.join(base_dir, val[:real_path] || val[:path_name]) + file_path = package_root.item_path(val[:real_path] || val[:path_name]) val.delete :real_path if File.exist?(file_path) zipfile.add(val[:path_name], file_path) diff --git a/lib/cc/exporter/epub/converters/assignment_epub_converter.rb b/lib/cc/exporter/epub/converters/assignment_epub_converter.rb index 698e0ad4da5..6aea1de54d9 100644 --- a/lib/cc/exporter/epub/converters/assignment_epub_converter.rb +++ b/lib/cc/exporter/epub/converters/assignment_epub_converter.rb @@ -25,8 +25,8 @@ module CC::Exporter::Epub::Converters meta_path = res.at_css('file[href$="assignment_settings.xml"]') next unless meta_path - meta_path = File.join @unzipped_file_path, meta_path['href'] - html_path = File.join @unzipped_file_path, res.at_css('file[href$="html"]')['href'] + meta_path = @package_root.item_path meta_path['href'] + html_path = @package_root.item_path res.at_css('file[href$="html"]')['href'] meta_node = open_file_xml(meta_path) html_node = open_file(html_path) diff --git a/lib/cc/exporter/epub/converters/cartridge_converter.rb b/lib/cc/exporter/epub/converters/cartridge_converter.rb index 18c1db4ef38..7bb84ad62d0 100644 --- a/lib/cc/exporter/epub/converters/cartridge_converter.rb +++ b/lib/cc/exporter/epub/converters/cartridge_converter.rb @@ -84,7 +84,7 @@ module CC::Exporter::Epub::Converters def export(export_type) unzip_archive - @manifest = open_file(File.join(@unzipped_file_path, MANIFEST_FILE)) + @manifest = open_file(@package_root.item_path(MANIFEST_FILE)) get_all_resources(@manifest) @course[:title] = get_node_val(@manifest, "string") diff --git a/lib/cc/exporter/epub/converters/module_epub_converter.rb b/lib/cc/exporter/epub/converters/module_epub_converter.rb index 041cf9b51fd..285bd3bbb43 100644 --- a/lib/cc/exporter/epub/converters/module_epub_converter.rb +++ b/lib/cc/exporter/epub/converters/module_epub_converter.rb @@ -20,7 +20,7 @@ module CC::Exporter::Epub::Converters include CC::Exporter def settings_doc(html = false) - path = File.join(@unzipped_file_path, "course_settings", "module_meta.xml") + path = @package_root.item_path("course_settings", "module_meta.xml") return nil unless File.exist? path if html open_file path diff --git a/lib/cc/exporter/epub/converters/quiz_epub_converter.rb b/lib/cc/exporter/epub/converters/quiz_epub_converter.rb index 08c6e45ae38..3c264fc9ede 100644 --- a/lib/cc/exporter/epub/converters/quiz_epub_converter.rb +++ b/lib/cc/exporter/epub/converters/quiz_epub_converter.rb @@ -22,13 +22,13 @@ module CC::Exporter::Epub::Converters def convert_quizzes quizzes = [] @manifest.css('resource[type$=assessment]').each do |quiz| - xml_path = File.join @unzipped_file_path, quiz.at_css('file[href$="xml"]')['href'] + xml_path = @package_root.item_path quiz.at_css('file[href$="xml"]')['href'] meta_node = open_file_xml(xml_path) ident = get_node_att(meta_node, "assessment", "ident") quiz_meta_path = "#{ident}/assessment_meta.xml" - quiz_meta_link = File.join @unzipped_file_path, quiz_meta_path + quiz_meta_link = @package_root.item_path quiz_meta_path quiz_meta_data = open_file_xml(quiz_meta_link) quiz = convert_quiz(quiz_meta_data) diff --git a/lib/cc/exporter/epub/converters/topic_epub_converter.rb b/lib/cc/exporter/epub/converters/topic_epub_converter.rb index e8f5d6f9635..ccb31a1d315 100644 --- a/lib/cc/exporter/epub/converters/topic_epub_converter.rb +++ b/lib/cc/exporter/epub/converters/topic_epub_converter.rb @@ -24,11 +24,11 @@ module CC::Exporter::Epub::Converters announcements = [] @manifest.css('resource[type=imsdt_xmlv1p1]').each do |res| - cc_path = File.join @unzipped_file_path, res.at_css('file')['href'] + cc_path = @package_root.item_path res.at_css('file')['href'] canvas_id = get_node_att(res, 'dependency', 'identifierref') if canvas_id && (meta_res = @manifest.at_css(%{resource[identifier="#{canvas_id}"]})) - canvas_path = File.join @unzipped_file_path, meta_res.at_css('file')['href'] + canvas_path = @package_root.item_path meta_res.at_css('file')['href'] meta_node = open_file_xml(canvas_path) else meta_node = nil diff --git a/lib/cc/exporter/epub/converters/wiki_epub_converter.rb b/lib/cc/exporter/epub/converters/wiki_epub_converter.rb index 9b0cb28e0f1..1a37f049ef7 100644 --- a/lib/cc/exporter/epub/converters/wiki_epub_converter.rb +++ b/lib/cc/exporter/epub/converters/wiki_epub_converter.rb @@ -23,7 +23,7 @@ module CC::Exporter::Epub::Converters def convert_wikis wikis = [] - wiki_dir = File.join(@unzipped_file_path, 'wiki_content') + wiki_dir = @package_root.item_path('wiki_content') Dir["#{wiki_dir}/**/**"].each do |path| doc = open_file_xml(path) workflow_state = get_node_val(doc, 'meta[name=workflow_state] @content') diff --git a/lib/cc/importer/canvas/assignment_converter.rb b/lib/cc/importer/canvas/assignment_converter.rb index 98aa96860e3..359ad6ffac4 100644 --- a/lib/cc/importer/canvas/assignment_converter.rb +++ b/lib/cc/importer/canvas/assignment_converter.rb @@ -25,8 +25,8 @@ module CC::Importer::Canvas @manifest.css('resource[type$=learning-application-resource]').each do |res| if meta_path = res.at_css('file[href$="assignment_settings.xml"]') - meta_path = File.join @unzipped_file_path, meta_path['href'] - html_path = File.join @unzipped_file_path, res.at_css('file[href$="html"]')['href'] + meta_path = @package_root.item_path meta_path['href'] + html_path = @package_root.item_path res.at_css('file[href$="html"]')['href'] meta_node = open_file_xml(meta_path) html_node = open_file(html_path) diff --git a/lib/cc/importer/canvas/converter.rb b/lib/cc/importer/canvas/converter.rb index 34f6e12e6d1..e8d767020ca 100644 --- a/lib/cc/importer/canvas/converter.rb +++ b/lib/cc/importer/canvas/converter.rb @@ -44,7 +44,7 @@ module CC::Importer::Canvas unzip_archive set_progress(5) - @manifest = open_file(File.join(@unzipped_file_path, MANIFEST_FILE)) + @manifest = open_file(@package_root.item_path(MANIFEST_FILE)) get_all_resources(@manifest) convert_all_course_settings @@ -83,7 +83,7 @@ module CC::Importer::Canvas end def read_external_content - folder = File.join(@unzipped_file_path, EXTERNAL_CONTENT_FOLDER) + folder = @package_root.item_path(EXTERNAL_CONTENT_FOLDER) return unless File.directory?(folder) external_content = {} diff --git a/lib/cc/importer/canvas/course_settings.rb b/lib/cc/importer/canvas/course_settings.rb index ed32fbb31f7..f8505ecd6bb 100644 --- a/lib/cc/importer/canvas/course_settings.rb +++ b/lib/cc/importer/canvas/course_settings.rb @@ -23,7 +23,7 @@ module CC::Importer::Canvas include ModuleConverter def settings_doc(file, html = false) - path = File.join(@unzipped_file_path, COURSE_SETTINGS_DIR, file) + path = @package_root.item_path(COURSE_SETTINGS_DIR, file) return nil unless File.exist? path if html open_file path diff --git a/lib/cc/importer/canvas/quiz_converter.rb b/lib/cc/importer/canvas/quiz_converter.rb index 10949aa86f2..4c9fd6ab46c 100644 --- a/lib/cc/importer/canvas/quiz_converter.rb +++ b/lib/cc/importer/canvas/quiz_converter.rb @@ -22,7 +22,7 @@ module CC::Importer::Canvas def convert_quizzes assessments = [] - qti_folder = File.join(@unzipped_file_path, ASSESSMENT_NON_CC_FOLDER) + qti_folder = @package_root.item_path(ASSESSMENT_NON_CC_FOLDER) return unless File.exist?(qti_folder) && File.directory?(qti_folder) diff --git a/lib/cc/importer/canvas/quiz_metadata_converter.rb b/lib/cc/importer/canvas/quiz_metadata_converter.rb index 91bd149c68c..ec36d99bd67 100644 --- a/lib/cc/importer/canvas/quiz_metadata_converter.rb +++ b/lib/cc/importer/canvas/quiz_metadata_converter.rb @@ -29,7 +29,7 @@ module CC::Importer::Canvas res.css('file').select{|f| f['href'].to_s.end_with?(ASSESSMENT_META)}.each do |file| meta_path = file['href'] if quiz = quiz_map[meta_path] - doc = open_file_xml(File.join(@unzipped_file_path, meta_path)) + doc = open_file_xml(@package_root.item_path(meta_path)) get_quiz_meta(doc, quiz) end end diff --git a/lib/cc/importer/canvas/tool_profile_converter.rb b/lib/cc/importer/canvas/tool_profile_converter.rb index 33a98a678a8..a0187b5b1c1 100644 --- a/lib/cc/importer/canvas/tool_profile_converter.rb +++ b/lib/cc/importer/canvas/tool_profile_converter.rb @@ -25,7 +25,7 @@ module CC::Importer::Canvas @manifest.css('resource[type=tool_profile]').each do |res| file = res.at_css('file') next unless file - file_path = File.join @unzipped_file_path, file['href'] + file_path = @package_root.item_path file['href'] json = JSON.parse(File.read(file_path)) json['resource_href'] = file['href'] json['migration_id'] = res['identifier'] diff --git a/lib/cc/importer/canvas/topic_converter.rb b/lib/cc/importer/canvas/topic_converter.rb index 40ca686f1f8..8273d8d1515 100644 --- a/lib/cc/importer/canvas/topic_converter.rb +++ b/lib/cc/importer/canvas/topic_converter.rb @@ -24,11 +24,11 @@ module CC::Importer::Canvas announcements = [] @manifest.css('resource[type=imsdt_xmlv1p1]').each do |res| - cc_path = File.join @unzipped_file_path, res.at_css('file')['href'] + cc_path = @package_root.item_path res.at_css('file')['href'] cc_id = res['identifier'] canvas_id = get_node_att(res, 'dependency', 'identifierref') if canvas_id && meta_res = @manifest.at_css(%{resource[identifier="#{canvas_id}"]}) - canvas_path = File.join @unzipped_file_path, meta_res.at_css('file')['href'] + canvas_path = @package_root.item_path meta_res.at_css('file')['href'] meta_node = open_file_xml(canvas_path) else meta_node = nil diff --git a/lib/cc/importer/canvas/webcontent_converter.rb b/lib/cc/importer/canvas/webcontent_converter.rb index e090ce0e328..312e89d5a12 100644 --- a/lib/cc/importer/canvas/webcontent_converter.rb +++ b/lib/cc/importer/canvas/webcontent_converter.rb @@ -38,7 +38,7 @@ module CC::Importer::Canvas end def convert_file_metadata(file_map) - path = File.join(@unzipped_file_path, COURSE_SETTINGS_DIR, FILES_META) + path = @package_root.item_path(COURSE_SETTINGS_DIR, FILES_META) return unless File.exist? path doc = open_file_xml path diff --git a/lib/cc/importer/canvas/wiki_converter.rb b/lib/cc/importer/canvas/wiki_converter.rb index 749a1d59836..d5049910077 100644 --- a/lib/cc/importer/canvas/wiki_converter.rb +++ b/lib/cc/importer/canvas/wiki_converter.rb @@ -22,7 +22,7 @@ module CC::Importer::Canvas def convert_wikis wikis = [] - wiki_dir = File.join(@unzipped_file_path, WIKI_FOLDER) + wiki_dir = @package_root.item_path(WIKI_FOLDER) Dir["#{wiki_dir}/**/**"].each do |path| next if File.directory?(path) doc = open_file(path) diff --git a/lib/cc/importer/standard/converter.rb b/lib/cc/importer/standard/converter.rb index d6ea657e1f5..0b763d90d2a 100644 --- a/lib/cc/importer/standard/converter.rb +++ b/lib/cc/importer/standard/converter.rb @@ -47,7 +47,7 @@ module CC::Importer::Standard @course[:assignments] ||= [] @archive.prepare_cartridge_file(MANIFEST_FILE) - @manifest = open_file_xml(File.join(@unzipped_file_path, MANIFEST_FILE)) + @manifest = open_file_xml(@package_root.item_path(MANIFEST_FILE)) @manifest.remove_namespaces! get_all_resources(@manifest) @@ -96,9 +96,9 @@ module CC::Importer::Standard @file_path_migration_id[path.gsub(%r{\$[^$]*\$|\.\./}, '').sub(WEB_RESOURCES_FOLDER + '/', '')] unless mig_id - full_path = File.expand_path(File.join(@unzipped_file_path, path)) + full_path = @package_root.item_path(path) - if full_path.start_with?(File.expand_path(@unzipped_file_path)) && File.exists?(full_path) + if File.exists?(full_path) # try to make it work even if the file wasn't technically included in the manifest :/ mig_id = Digest::MD5.hexdigest(path) file = {:path_name => path, :migration_id => mig_id, diff --git a/lib/cc/importer/standard/quiz_converter.rb b/lib/cc/importer/standard/quiz_converter.rb index e0c2229736f..2342d58e5ec 100644 --- a/lib/cc/importer/standard/quiz_converter.rb +++ b/lib/cc/importer/standard/quiz_converter.rb @@ -23,7 +23,7 @@ module CC::Importer::Standard quizzes = [] questions = [] - conversion_dir = File.join(@unzipped_file_path, "temp_qti_conversions") + conversion_dir = @package_root.item_path("temp_qti_conversions") resources_by_type("imsqti").each do |res| path = res[:href] || (res[:files] && res[:files].first && res[:files].first[:href]) diff --git a/lib/cc/importer/standard/webcontent_converter.rb b/lib/cc/importer/standard/webcontent_converter.rb index f76f4452b9f..06dbfebaf00 100644 --- a/lib/cc/importer/standard/webcontent_converter.rb +++ b/lib/cc/importer/standard/webcontent_converter.rb @@ -81,11 +81,11 @@ module CC::Importer::Standard file_map.each_value do |val| next if zipfile.entries.include?(val[:path_name]) - file_path = File.join(@unzipped_file_path, val[:path_name]) + file_path = @package_root.item_path(val[:path_name]) if File.exist?(file_path) zipfile.add(val[:path_name], file_path) if !File.directory?(file_path) else - web_file_path = File.join(@unzipped_file_path, WEB_RESOURCES_FOLDER, val[:path_name]) + web_file_path = @package_root.item_path(WEB_RESOURCES_FOLDER, val[:path_name]) if File.exist?(web_file_path) zipfile.add(val[:path_name], web_file_path) if !File.directory?(web_file_path) else diff --git a/lib/package_root.rb b/lib/package_root.rb new file mode 100644 index 00000000000..5f9e3f7ea30 --- /dev/null +++ b/lib/package_root.rb @@ -0,0 +1,52 @@ +# +# Copyright (C) 2017 - present 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 . +# + +require 'pathname' + +class PackageRoot + # initialize with the root directory of an extracted package + def initialize(root_path) + @root_path = Pathname(root_path).realpath + @prefix = @root_path.to_s + '/' + end + + # return the root path. NOTE: don't manually File.join this; use item_path instead + def root_path + @root_path.to_s + end + + # return the absolute path of an item in the package, given one or more relative path entries + # e.g., if the root_path is /tmp/blah and args are ['foo', 'bar'], returns "/tmp/blah/foo/bar" + # raises an error if ".." path entries would traverse above the root_path in the file system. + def item_path(*relative_path_entries) + path = Pathname(File.join(@prefix, *relative_path_entries)).cleanpath.to_s + raise ArgumentError, "invalid relative_path_entries" unless path.start_with?(@prefix) + path + end + + # given a full path to an item in the package, return its path relative to the package root + def relative_path(item_path) + Pathname(item_path).realpath.relative_path_from(@root_path).to_s + end + + # enumerate files matching the given pattern + def contents(pattern = '**/*') + Dir[@root_path.join(pattern).to_s] + end + +end \ No newline at end of file diff --git a/spec/lib/cc/importer/canvas/tool_profile_converter_spec.rb b/spec/lib/cc/importer/canvas/tool_profile_converter_spec.rb index 54de90ec252..6b1f2ab2fff 100644 --- a/spec/lib/cc/importer/canvas/tool_profile_converter_spec.rb +++ b/spec/lib/cc/importer/canvas/tool_profile_converter_spec.rb @@ -26,7 +26,7 @@ describe CC::Importer::Canvas::ToolProfileConverter do def initialize(manifest, path) @manifest = manifest - @unzipped_file_path = path + @package_root = PackageRoot.new(path) end end.new(manifest, path) end diff --git a/spec/lib/package_root_spec.rb b/spec/lib/package_root_spec.rb new file mode 100644 index 00000000000..00bf63acc47 --- /dev/null +++ b/spec/lib/package_root_spec.rb @@ -0,0 +1,53 @@ +# +# Copyright (C) 2017 - present 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 . +# + +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb') +require 'lib/package_root' + +describe PackageRoot do + let(:root_path) { File.expand_path(File.join(File.dirname(__FILE__), '../fixtures/importer/unzipped')) } + let(:subject) { PackageRoot.new(root_path) } + + it "returns the root_path" do + expect(subject.root_path).to eq(root_path) + end + + it "returns the name of an item inside" do + expect(subject.item_path('imsmanifest.xml')).to eq(File.join(root_path, 'imsmanifest.xml')) + end + + it "follows .. paths" do + expect(subject.item_path('course_settings', '..', 'imsmanifest.xml')).to eq(File.join(root_path, 'imsmanifest.xml')) + end + + it "refuses to follow .. paths above the package root" do + expect { + subject.item_path('course_settings', '..', '..', 'assessments.json') + }.to raise_error(ArgumentError) + end + + it "makes relative paths" do + expect(subject.relative_path(File.join(root_path, 'course_settings', 'course_settings.xml'))).to eq 'course_settings/course_settings.xml' + end + + it "enumerates contents" do + expect(subject.contents).to include File.join(root_path, 'imsmanifest.xml') + expect(subject.contents).to include File.join(root_path, 'course_settings', 'course_settings.xml') + expect(subject.contents('**/files_meta.xml').to_a).to eq([File.join(root_path, 'course_settings/files_meta.xml')]) + end +end