diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index e0203e3b00..74df1cc5ad 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -3,6 +3,7 @@ Script for importing courseware from XML format """ from django.core.management.base import BaseCommand from django_comment_common.utils import are_permissions_roles_seeded, seed_permissions_roles +from lms.djangoapps.dashboard.git_import import DEFAULT_COURSE_CODE_LIB_FILENAME from xmodule.contentstore.django import contentstore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -22,14 +23,26 @@ class Command(BaseCommand): metavar='course_dir') parser.add_argument('--nostatic', action='store_true', - help='skip import of static content') + help='Skip import of static content') + parser.add_argument('--nocodelib', + action='store_true', + help=( + 'Skip import of custom code library if it exists' + '(NOTE: If static content is imported, the code library will also ' + 'be imported and this flag will be ignored)' + )), + parser.add_argument('--code-lib-filename', + default=DEFAULT_COURSE_CODE_LIB_FILENAME, + help='Filename of the course code library (if it exists)') def handle(self, *args, **options): data_dir = options['data_directory'] - do_import_static = not options['nostatic'] source_dirs = options['course_dirs'] if len(source_dirs) == 0: source_dirs = None + do_import_static = not options.get('nostatic', False) + do_import_code_lib = not options.get('nocodelib', False) + course_code_lib_filename = options.get('code_lib_filename') self.stdout.write("Importing. Data_dir={data}, source_dirs={courses}\n".format( data=data_dir, @@ -40,8 +53,9 @@ class Command(BaseCommand): course_items = import_course_from_xml( mstore, ModuleStoreEnum.UserID.mgmt_command, data_dir, source_dirs, load_error_modules=False, static_content_store=contentstore(), verbose=True, - do_import_static=do_import_static, + do_import_static=do_import_static, do_import_code_lib=do_import_code_lib, create_if_not_present=True, + python_lib_filename=course_code_lib_filename, ) for course in course_items: diff --git a/common/djangoapps/util/sandboxing.py b/common/djangoapps/util/sandboxing.py index 0c47cf2471..fa90ec8f4f 100644 --- a/common/djangoapps/util/sandboxing.py +++ b/common/djangoapps/util/sandboxing.py @@ -1,9 +1,7 @@ import re from django.conf import settings - -# We'll make assets named this be importable by Python code in the sandbox. -PYTHON_LIB_ZIP = "python_lib.zip" +from lms.djangoapps.dashboard.git_import import DEFAULT_COURSE_CODE_LIB_FILENAME def can_execute_unsafe_code(course_id): @@ -32,8 +30,9 @@ def can_execute_unsafe_code(course_id): def get_python_lib_zip(contentstore, course_id): - """Return the bytes of the python_lib.zip file, if any.""" - asset_key = course_id.make_asset_key("asset", PYTHON_LIB_ZIP) + """Return the bytes of the course code library file, if it exists.""" + python_lib_filename = getattr(settings, 'COURSE_CODE_LIB_FILENAME', DEFAULT_COURSE_CODE_LIB_FILENAME) + asset_key = course_id.make_asset_key("asset", python_lib_filename) zip_lib = contentstore().find(asset_key, throw_on_not_found=False) if zip_lib is not None: return zip_lib.data diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index bb9e5f3e65..bdb6db4ebd 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -9,11 +9,17 @@ from xmodule.x_module import XModuleMixin from opaque_keys.edx.locations import Location from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import InheritanceMixin -from xmodule.modulestore.xml_importer import _update_and_import_module, _update_module_location +from xmodule.modulestore.xml_importer import ( + StaticContentImporter, + _update_and_import_module, + _update_module_location +) from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from opaque_keys.edx.keys import CourseKey from xmodule.tests import DATA_DIR +import os from uuid import uuid4 +from path import Path as path import unittest import importlib @@ -333,3 +339,50 @@ class UpdateLocationTest(ModuleStoreNoSettings): # Expect these fields pass "is_set_on" test for field in self.CONTENT_FIELDS + self.SETTINGS_FIELDS + self.CHILDREN_FIELDS: self.assertTrue(new_version.fields[field].is_set_on(new_version)) + + +class StaticContentImporterTest(unittest.TestCase): + def setUp(self): + self.course_data_path = path('/path') + self.mocked_content_store = mock.Mock() + self.static_content_importer = StaticContentImporter( + static_content_store=self.mocked_content_store, + course_data_path=self.course_data_path, + target_id=CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') + ) + + def test_import_static_content_directory(self): + static_content_dir = 'static' + expected_base_dir = path(self.course_data_path / static_content_dir) + mocked_os_walk_yield = [ + ('static', None, ['file1.txt', 'file2.txt']), + ('static/inner', None, ['file1.txt']), + ] + with mock.patch( + 'xmodule.modulestore.xml_importer.os.walk', + return_value=mocked_os_walk_yield + ), mock.patch.object( + self.static_content_importer, 'import_static_file' + ) as patched_import_static_file: + self.static_content_importer.import_static_content_directory('static') + patched_import_static_file.assert_any_call( + 'static/file1.txt', base_dir=expected_base_dir + ) + patched_import_static_file.assert_any_call( + 'static/file2.txt', base_dir=expected_base_dir + ) + patched_import_static_file.assert_any_call( + 'static/inner/file1.txt', base_dir=expected_base_dir + ) + + def test_import_static_file(self): + base_dir = path('/path/to/dir') + full_file_path = os.path.join(base_dir, 'static/some_file.txt') + self.mocked_content_store.generate_thumbnail.return_value = (None, None) + with mock.patch("__builtin__.open", mock.mock_open(read_data="data")) as mock_file: + self.static_content_importer.import_static_file( + full_file_path=full_file_path, + base_dir=base_dir + ) + mock_file.assert_called_with(full_file_path, 'rb') + self.mocked_content_store.assert_called_once() diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 78ddf52984..5e0bf94ce4 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -54,96 +54,104 @@ from xmodule.util.misc import escape_invalid_characters log = logging.getLogger(__name__) +DEFAULT_STATIC_CONTENT_SUBDIR = 'static' -def import_static_content( - course_data_path, static_content_store, - target_id, subpath='static', verbose=False): - remap_dict = {} +class StaticContentImporter: + def __init__(self, static_content_store, course_data_path, target_id): + self.static_content_store = static_content_store + self.target_id = target_id + self.course_data_path = course_data_path + try: + with open(course_data_path / 'policies/assets.json') as f: + self.policy = json.load(f) + except (IOError, ValueError) as err: + # xml backed courses won't have this file, only exported courses; + # so, its absence is not really an exception. + self.policy = {} - # now import all static assets - static_dir = course_data_path / subpath - try: - with open(course_data_path / 'policies/assets.json') as f: - policy = json.load(f) - except (IOError, ValueError) as err: - # xml backed courses won't have this file, only exported courses; - # so, its absence is not really an exception. - policy = {} + mimetypes.add_type('application/octet-stream', '.sjson') + mimetypes.add_type('application/octet-stream', '.srt') + self.mimetypes_list = mimetypes.types_map.values() - verbose = True + def import_static_content_directory(self, content_subdir=DEFAULT_STATIC_CONTENT_SUBDIR, verbose=False): + remap_dict = {} - mimetypes.add_type('application/octet-stream', '.sjson') - mimetypes.add_type('application/octet-stream', '.srt') - mimetypes_list = mimetypes.types_map.values() + static_dir = self.course_data_path / content_subdir + for dirname, _, filenames in os.walk(static_dir): + for filename in filenames: - for dirname, _, filenames in os.walk(static_dir): - for filename in filenames: + file_path = os.path.join(dirname, filename) - content_path = os.path.join(dirname, filename) - - if re.match(ASSET_IGNORE_REGEX, filename): - if verbose: - log.debug('skipping static content %s...', content_path) - continue - - if verbose: - log.debug('importing static content %s...', content_path) - - try: - with open(content_path, 'rb') as f: - data = f.read() - except IOError: - if filename.startswith('._'): - # OS X "companion files". See - # http://www.diigo.com/annotated/0c936fda5da4aa1159c189cea227e174 + if re.match(ASSET_IGNORE_REGEX, filename): + if verbose: + log.debug('skipping static content %s...', file_path) continue - # Not a 'hidden file', then re-raise exception - raise - # strip away leading path from the name - fullname_with_subpath = content_path.replace(static_dir, '') - if fullname_with_subpath.startswith('/'): - fullname_with_subpath = fullname_with_subpath[1:] - asset_key = StaticContent.compute_location(target_id, fullname_with_subpath) + if verbose: + log.debug('importing static content %s...', file_path) - policy_ele = policy.get(asset_key.path, {}) + imported_file_attrs = self.import_static_file(file_path, base_dir=static_dir) - # During export display name is used to create files, strip away slashes from name - displayname = escape_invalid_characters( - name=policy_ele.get('displayname', filename), - invalid_char_list=['/', '\\'] - ) - locked = policy_ele.get('locked', False) - mime_type = policy_ele.get('contentType') + if imported_file_attrs: + # store the remapping information which will be needed + # to subsitute in the module data + remap_dict[imported_file_attrs[0]] = imported_file_attrs[1] - # Check extracted contentType in list of all valid mimetypes - if not mime_type or mime_type not in mimetypes_list: - mime_type = mimetypes.guess_type(filename)[0] # Assign guessed mimetype - content = StaticContent( - asset_key, displayname, mime_type, data, - import_path=fullname_with_subpath, locked=locked - ) + return remap_dict - # first let's save a thumbnail so we can get back a thumbnail location - thumbnail_content, thumbnail_location = static_content_store.generate_thumbnail(content) + def import_static_file(self, full_file_path, base_dir): + filename = os.path.basename(full_file_path) + try: + with open(full_file_path, 'rb') as f: + data = f.read() + except IOError: + # OS X "companion files". See + # http://www.diigo.com/annotated/0c936fda5da4aa1159c189cea227e174 + if filename.startswith('._'): + return None + # Not a 'hidden file', then re-raise exception + raise - if thumbnail_content is not None: - content.thumbnail_location = thumbnail_location + # strip away leading path from the name + file_subpath = full_file_path.replace(base_dir, '') + if file_subpath.startswith('/'): + file_subpath = file_subpath[1:] + asset_key = StaticContent.compute_location(self.target_id, file_subpath) - # then commit the content - try: - static_content_store.save(content) - except Exception as err: - log.exception(u'Error importing {0}, error={1}'.format( - fullname_with_subpath, err - )) + policy_ele = self.policy.get(asset_key.path, {}) - # store the remapping information which will be needed - # to subsitute in the module data - remap_dict[fullname_with_subpath] = asset_key + # During export display name is used to create files, strip away slashes from name + displayname = escape_invalid_characters( + name=policy_ele.get('displayname', filename), + invalid_char_list=['/', '\\'] + ) + locked = policy_ele.get('locked', False) + mime_type = policy_ele.get('contentType') - return remap_dict + # Check extracted contentType in list of all valid mimetypes + if not mime_type or mime_type not in self.mimetypes_list: + mime_type = mimetypes.guess_type(filename)[0] # Assign guessed mimetype + content = StaticContent( + asset_key, displayname, mime_type, data, + import_path=file_subpath, locked=locked + ) + + # first let's save a thumbnail so we can get back a thumbnail location + thumbnail_content, thumbnail_location = self.static_content_store.generate_thumbnail(content) + + if thumbnail_content is not None: + content.thumbnail_location = thumbnail_location + + # then commit the content + try: + self.static_content_store.save(content) + except Exception as err: + log.exception(u'Error importing {0}, error={1}'.format( + file_subpath, err + )) + + return file_subpath, asset_key class ImportManager(object): @@ -186,8 +194,10 @@ class ImportManager(object): default_class='xmodule.raw_module.RawDescriptor', load_error_modules=True, static_content_store=None, target_id=None, verbose=False, - do_import_static=True, create_if_not_present=False, - raise_on_failure=False + do_import_static=True, do_import_code_lib=True, + create_if_not_present=False, raise_on_failure=False, + static_content_subdir=DEFAULT_STATIC_CONTENT_SUBDIR, + python_lib_filename='python_lib.zip', ): self.store = store self.user_id = user_id @@ -197,7 +207,10 @@ class ImportManager(object): self.static_content_store = static_content_store self.target_id = target_id self.verbose = verbose + self.static_content_subdir = static_content_subdir + self.python_lib_filename = python_lib_filename self.do_import_static = do_import_static + self.do_import_code_lib = do_import_code_lib self.create_if_not_present = create_if_not_present self.raise_on_failure = raise_on_failure self.xml_module_store = self.store_class( @@ -224,18 +237,35 @@ class ImportManager(object): """ Import all static items into the content store. """ - if self.static_content_store is not None and self.do_import_static: - # first pass to find everything in /static/ - import_static_content( - data_path, self.static_content_store, - dest_id, subpath='static', verbose=self.verbose - ) - - elif self.verbose and not self.do_import_static: + static_content_importer = StaticContentImporter( + self.static_content_store, + course_data_path=data_path, + target_id=dest_id + ) + if self.verbose and not self.do_import_static: log.debug( "Skipping import of static content, " "since do_import_static=%s", self.do_import_static ) + if self.do_import_code_lib: + log.debug( + "Importing code library anyway " + "since do_import_code_lib=%s", self.do_import_code_lib + ) + + if self.static_content_store is not None: + if self.do_import_static: + # first pass to find everything in the static content directory + static_content_importer.import_static_content_directory( + content_subdir=self.static_content_subdir, verbose=self.verbose + ) + elif self.do_import_code_lib and self.python_lib_filename: + python_lib_dir_path = data_path / self.static_content_subdir + python_lib_full_path = python_lib_dir_path / self.python_lib_filename + if os.path.isfile(python_lib_full_path): + static_content_importer.import_static_file( + python_lib_full_path, base_dir=python_lib_dir_path + ) # no matter what do_import_static is, import "static_import" directory @@ -249,9 +279,8 @@ class ImportManager(object): simport = 'static_import' if os.path.exists(data_path / simport): - import_static_content( - data_path, self.static_content_store, - dest_id, subpath=simport, verbose=self.verbose + static_content_importer.import_static_content_directory( + content_subdir=simport, verbose=self.verbose ) def import_asset_metadata(self, data_dir, course_id): diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py index 68b7d9d073..aae17d462a 100644 --- a/common/lib/xmodule/xmodule/tests/test_import_static.py +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -3,7 +3,7 @@ Tests that check that we ignore the appropriate files when importing courses. """ import unittest from mock import Mock -from xmodule.modulestore.xml_importer import import_static_content +from xmodule.modulestore.xml_importer import StaticContentImporter from opaque_keys.edx.locator import CourseLocator from xmodule.tests import DATA_DIR @@ -15,7 +15,12 @@ class IgnoredFilesTestCase(unittest.TestCase): course_id = CourseLocator("edX", "tilde", "Fall_2012") content_store = Mock() content_store.generate_thumbnail.return_value = ("content", "location") - import_static_content(course_dir, content_store, course_id) + static_content_importer = StaticContentImporter( + static_content_store=content_store, + course_data_path=course_dir, + target_id=course_id + ) + static_content_importer.import_static_content_directory() saved_static_content = [call[0][0] for call in content_store.save.call_args_list] name_val = {sc.name: sc.data for sc in saved_static_content} self.assertIn("example.txt", name_val) @@ -30,7 +35,12 @@ class IgnoredFilesTestCase(unittest.TestCase): course_id = CourseLocator("edX", "dot-underscore", "2014_Fall") content_store = Mock() content_store.generate_thumbnail.return_value = ("content", "location") - import_static_content(course_dir, content_store, course_id) + static_content_importer = StaticContentImporter( + static_content_store=content_store, + course_data_path=course_dir, + target_id=course_id + ) + static_content_importer.import_static_content_directory() saved_static_content = [call[0][0] for call in content_store.save.call_args_list] name_val = {sc.name: sc.data for sc in saved_static_content} self.assertIn("example.txt", name_val) diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index eb0dd1ee6c..4a187ea9a7 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -22,6 +22,7 @@ from dashboard.models import CourseImportLog log = logging.getLogger(__name__) DEFAULT_GIT_REPO_DIR = '/edx/var/app/edxapp/course_repos' +DEFAULT_COURSE_CODE_LIB_FILENAME = 'python_lib.zip' class GitImportError(Exception): @@ -184,6 +185,8 @@ def add_repo(repo, rdir_in, branch=None): git_repo_dir = getattr(settings, 'GIT_REPO_DIR', DEFAULT_GIT_REPO_DIR) git_import_static = getattr(settings, 'GIT_IMPORT_STATIC', True) + git_import_code_lib = getattr(settings, 'GIT_IMPORT_CODE_LIB', True) + course_code_lib_filename = getattr(settings, 'COURSE_CODE_LIB_FILENAME', DEFAULT_COURSE_CODE_LIB_FILENAME) # Set defaults even if it isn't defined in settings mongo_db = { @@ -271,8 +274,11 @@ def add_repo(repo, rdir_in, branch=None): loggers.append(logger) try: - management.call_command('import', git_repo_dir, rdir, - nostatic=not git_import_static) + management.call_command( + 'import', git_repo_dir, rdir, + nostatic=not git_import_static, nocodelib=not git_import_code_lib, + code_lib_filename=course_code_lib_filename + ) except CommandError: raise GitImportErrorXmlImportFailed() except NotImplementedError: diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 73274e6c91..55981d8c8e 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -371,6 +371,8 @@ BADGR_TIMEOUT = ENV_TOKENS.get('BADGR_TIMEOUT', BADGR_TIMEOUT) # git repo loading environment GIT_REPO_DIR = ENV_TOKENS.get('GIT_REPO_DIR', '/edx/var/edxapp/course_repos') GIT_IMPORT_STATIC = ENV_TOKENS.get('GIT_IMPORT_STATIC', True) +GIT_IMPORT_CODE_LIB = ENV_TOKENS.get('GIT_IMPORT_CODE_LIB', True) +COURSE_CODE_LIB_FILENAME = ENV_TOKENS.get('COURSE_CODE_LIB_FILENAME', 'python_lib.zip') for name, value in ENV_TOKENS.get("CODE_JAIL", {}).items(): oldvalue = CODE_JAIL.get(name)