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  <jamesw@instructure.com>
QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2017-11-09 14:33:32 -07:00
parent 579ea9305f
commit ad18235015
27 changed files with 161 additions and 47 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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)

View File

@ -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")

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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')

View File

@ -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)

View File

@ -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 = {}

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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']

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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,

View File

@ -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])

View File

@ -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

52
lib/package_root.rb Normal file
View File

@ -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 <http://www.gnu.org/licenses/>.
#
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

View File

@ -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

View File

@ -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 <http://www.gnu.org/licenses/>.
#
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