diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 46f439b055b0..e0d58b32f0dd 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -2,7 +2,7 @@ Script for importing courseware from XML format """ -from django.core.management.base import BaseCommand, CommandError +from django.core.management.base import BaseCommand, CommandError, make_option from xmodule.modulestore.xml_importer import import_from_xml from xmodule.modulestore.django import modulestore from xmodule.contentstore.django import contentstore @@ -14,18 +14,26 @@ class Command(BaseCommand): """ help = 'Import the specified data directory into the default ModuleStore' + option_list = BaseCommand.option_list + ( + make_option('--nostatic', + action='store_true', + help='Skip import of static content'), + ) + def handle(self, *args, **options): "Execute the command" if len(args) == 0: - raise CommandError("import requires at least one argument: [...]") + raise CommandError("import requires at least one argument: [--nostatic] [...]") data_dir = args[0] + do_import_static = not (options.get('nostatic', False)) if len(args) > 1: course_dirs = args[1:] else: course_dirs = None print("Importing. Data_dir={data}, course_dirs={courses}".format( data=data_dir, - courses=course_dirs)) + courses=course_dirs, + dis=do_import_static)) import_from_xml(modulestore('direct'), data_dir, course_dirs, load_error_modules=False, - static_content_store=contentstore(), verbose=True) + static_content_store=contentstore(), verbose=True, do_import_static=do_import_static) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 96b0b84e3661..2a69fc451cf5 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -476,7 +476,7 @@ def test_asset_import(self): content_store = contentstore() module_store = modulestore('direct') - import_from_xml(module_store, 'common/test/data/', ['toy'], static_content_store=content_store) + import_from_xml(module_store, 'common/test/data/', ['toy'], static_content_store=content_store, verbose=True) course_location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') course = module_store.get_item(course_location) diff --git a/cms/djangoapps/contentstore/tests/test_import_nostatic.py b/cms/djangoapps/contentstore/tests/test_import_nostatic.py new file mode 100644 index 000000000000..aad6ffbfe464 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_import_nostatic.py @@ -0,0 +1,123 @@ +#pylint: disable=E1101 +''' +Tests for importing with no static +''' + +from django.test.client import Client +from django.test.utils import override_settings +from django.conf import settings +from path import path +import copy + +from django.contrib.auth.models import User + +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + +from xmodule.modulestore import Location +from xmodule.modulestore.django import modulestore +from xmodule.contentstore.django import contentstore +from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.contentstore.content import StaticContent + +from xmodule.course_module import CourseDescriptor + +from xmodule.exceptions import NotFoundError +from uuid import uuid4 + + +TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) +TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex + + +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) +class ContentStoreImportNoStaticTest(ModuleStoreTestCase): + """ + Tests that rely on the toy and test_import_course courses. + NOTE: refactor using CourseFactory so they do not. + """ + def setUp(self): + + settings.MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data') + settings.MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data') + uname = 'testuser' + email = 'test+courses@edx.org' + password = 'foo' + + # Create the use so we can log them in. + self.user = User.objects.create_user(uname, email, password) + + # Note that we do not actually need to do anything + # for registration if we directly mark them active. + self.user.is_active = True + # Staff has access to view all courses + self.user.is_staff = True + + # Save the data that we've just changed to the db. + self.user.save() + + self.client = Client() + self.client.login(username=uname, password=password) + + def load_test_import_course(self): + ''' + Load the standard course used to test imports (for do_import_static=False behavior). + ''' + content_store = contentstore() + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['test_import_course'], static_content_store=content_store, do_import_static=False, verbose=True) + course_location = CourseDescriptor.id_to_location('edX/test_import_course/2012_Fall') + course = module_store.get_item(course_location) + self.assertIsNotNone(course) + + return module_store, content_store, course, course_location + + def test_static_import(self): + ''' + Stuff in static_import should always be imported into contentstore + ''' + _, content_store, course, course_location = self.load_test_import_course() + + # make sure we have ONE asset in our contentstore ("should_be_imported.html") + all_assets = content_store.get_all_content_for_course(course_location) + print "len(all_assets)=%d" % len(all_assets) + self.assertEqual(len(all_assets), 1) + + content = None + try: + location = StaticContent.get_location_from_path('/c4x/edX/test_import_course/asset/should_be_imported.html') + content = content_store.find(location) + except NotFoundError: + pass + + self.assertIsNotNone(content) + + # make sure course.lms.static_asset_path is correct + print "static_asset_path = {0}".format(course.lms.static_asset_path) + self.assertEqual(course.lms.static_asset_path, 'test_import_course') + + def test_asset_import_nostatic(self): + ''' + This test validates that an image asset is NOT imported when do_import_static=False + ''' + content_store = contentstore() + + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['toy'], static_content_store=content_store, do_import_static=False, verbose=True) + + course_location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') + module_store.get_item(course_location) + + # make sure we have NO assets in our contentstore + all_assets = content_store.get_all_content_for_course(course_location) + print "len(all_assets)=%d" % len(all_assets) + self.assertEqual(len(all_assets), 0) + + def test_no_static_link_rewrites_on_import(self): + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['toy'], do_import_static=False, verbose=True) + + handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'course_info', 'handouts', None])) + self.assertIn('/static/', handouts.data) + + handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'html', 'toyhtml', None])) + self.assertIn('/static/', handouts.data) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index d7f2df832290..712664bf39ad 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -90,7 +90,7 @@ def replace_course_url(match): return re.sub(_url_replace_regex('/course/'), replace_course_url, text) -def replace_static_urls(text, data_directory, course_id=None): +def replace_static_urls(text, data_directory, course_id=None, static_asset_path=''): """ Replace /static/$stuff urls either with their correct url as generated by collectstatic, (/static/$md5_hashed_stuff) or by the course-specific content static url @@ -100,6 +100,7 @@ def replace_static_urls(text, data_directory, course_id=None): text: The source text to do the substitution in data_directory: The directory in which course data is stored course_id: The course identifier used to distinguish static content for this course in studio + static_asset_path: Path for static assets, which overrides data_directory and course_namespace, if nonempty """ def replace_static_url(match): @@ -116,7 +117,7 @@ def replace_static_url(match): if settings.DEBUG and finders.find(rest, True): return original # if we're running with a MongoBacked store course_namespace is not None, then use studio style urls - elif course_id and modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE: + elif (not static_asset_path) and course_id and modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE: # first look in the static file pipeline and see if we are trying to reference # a piece of static content which is in the mitx repo (e.g. JS associated with an xmodule) @@ -135,7 +136,7 @@ def replace_static_url(match): url = StaticContent.convert_legacy_static_url_with_course_id(rest, course_id) # Otherwise, look the file up in staticfiles_storage, and append the data directory if needed else: - course_path = "/".join((data_directory, rest)) + course_path = "/".join((static_asset_path or data_directory, rest)) try: if staticfiles_storage.exists(rest): @@ -152,7 +153,7 @@ def replace_static_url(match): return re.sub( - _url_replace_regex('/static/(?!{data_dir})'.format(data_dir=data_directory)), + _url_replace_regex('/static/(?!{data_dir})'.format(data_dir=static_asset_path or data_directory)), replace_static_url, text ) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 6b7339559917..16fe12371bb7 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -76,7 +76,7 @@ def _get_html(): return _get_html -def replace_static_urls(get_html, data_dir, course_id=None): +def replace_static_urls(get_html, data_dir, course_id=None, static_asset_path=''): """ Updates the supplied module with a new get_html function that wraps the old get_html function and substitutes urls of the form /static/... @@ -85,7 +85,7 @@ def replace_static_urls(get_html, data_dir, course_id=None): @wraps(get_html) def _get_html(): - return static_replace.replace_static_urls(get_html(), data_dir, course_id) + return static_replace.replace_static_urls(get_html(), data_dir, course_id, static_asset_path=static_asset_path) return _get_html diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py index 1314c720948b..aeec53cc294d 100644 --- a/common/lib/xmodule/xmodule/modulestore/inheritance.py +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -9,7 +9,8 @@ # intended to be set per-course, but can be overridden in for specific # elements. Can be a float. 'days_early_for_beta', - 'giturl' # for git edit link + 'giturl', # for git edit link + 'static_asset_path', # for static assets placed outside xcontent contentstore ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 17036a16bf76..40b3c6fd8308 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -10,7 +10,9 @@ from xmodule.tests import DATA_DIR from xmodule.modulestore import Location from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore -from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.draft import DraftModuleStore +from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint +from xmodule.contentstore.mongo import MongoContentStore from xmodule.modulestore.tests.test_modulestore import check_path_to_location @@ -35,7 +37,7 @@ def setupClass(cls): # is ok only as long as none of the tests modify the db. # If (when!) that changes, need to either reload the db, or load # once and copy over to a tmp db for each test. - cls.store = cls.initdb() + cls.store, cls.content_store, cls.draft_store = cls.initdb() @classmethod def teardownClass(cls): @@ -46,10 +48,28 @@ def teardownClass(cls): def initdb(): # connect to the db store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS) + # since MongoModuleStore and MongoContentStore are basically assumed to be together, create this class + # as well + content_store = MongoContentStore(HOST, DB) + # + # Also test draft store imports + # + draft_store = DraftModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS) # Explicitly list the courses to load (don't want the big one) - courses = ['toy', 'simple'] - import_from_xml(store, DATA_DIR, courses) - return store + courses = ['toy', 'simple', 'simple_with_draft'] + import_from_xml(store, DATA_DIR, courses, draft_store=draft_store, static_content_store=content_store) + + # also test a course with no importing of static content + import_from_xml( + store, + DATA_DIR, + ['test_import_course'], + static_content_store=content_store, + do_import_static=False, + verbose=True + ) + + return store, content_store, draft_store @staticmethod def destroy_db(connection): @@ -77,10 +97,12 @@ def test_mongo_modulestore_type(self): def test_get_courses(self): '''Make sure the course objects loaded properly''' courses = self.store.get_courses() - assert_equals(len(courses), 2) + assert_equals(len(courses), 4) courses.sort(key=lambda c: c.id) assert_equals(courses[0].id, 'edX/simple/2012_Fall') - assert_equals(courses[1].id, 'edX/toy/2012_Fall') + assert_equals(courses[1].id, 'edX/simple_with_draft/2012_Fall') + assert_equals(courses[2].id, 'edX/test_import_course/2012_Fall') + assert_equals(courses[3].id, 'edX/toy/2012_Fall') def test_loads(self): assert_not_equals( @@ -112,6 +134,13 @@ def test_path_to_location(self): '''Make sure that path_to_location works''' check_path_to_location(self.store) + def test_xlinter(self): + ''' + Run through the xlinter, we know the 'toy' course has violations, but the + number will continue to grow over time, so just check > 0 + ''' + assert_not_equals(perform_xlint(DATA_DIR, ['toy']), 0) + def test_get_courses_has_no_templates(self): courses = self.store.get_courses() for course in courses: @@ -129,7 +158,7 @@ def get_tab_name(index): Assumes the information is desired for courses[1] ('toy' course). """ - return courses[1].tabs[index]['name'] + return courses[2].tabs[index]['name'] # There was a bug where model.save was not getting called after the static tab name # was set set for tabs that have a URL slug. 'Syllabus' and 'Resources' fall into that diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 7bea0fdcacb6..d20bf264aaf3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -51,7 +51,10 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ content.thumbnail_location = thumbnail_location #then commit the content - static_content_store.save(content) + try: + static_content_store.save(content) + except Exception as err: + log.exception('Error importing {0}, error={1}'.format(fullname_with_subpath, err)) #store the remapping information which will be needed to subsitute in the module data remap_dict[fullname_with_subpath] = content_loc.name @@ -64,7 +67,8 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ def import_from_xml(store, data_dir, course_dirs=None, default_class='xmodule.raw_module.RawDescriptor', load_error_modules=True, static_content_store=None, target_location_namespace=None, - verbose=False, draft_store=None): + verbose=False, draft_store=None, + do_import_static=True): """ Import the specified xml data_dir into the "store" modulestore, using org and course as the location org and course. @@ -77,6 +81,10 @@ def import_from_xml(store, data_dir, course_dirs=None, expects a 'url_name' as an identifier to where things are on disk e.g. ../policies//policy.json as well as metadata keys in the policy.json. so we need to keep the original url_name during import + do_import_static: if False, then static files are not imported into the static content store. This can be employed for courses which + have substantial unchanging static content, which is to inefficient to import every time the course is loaded. + Static content for some courses may also be served directly by nginx, instead of going through django. + """ xml_module_store = XMLModuleStore( @@ -116,8 +124,17 @@ def import_from_xml(store, data_dir, course_dirs=None, course_data_path = path(data_dir) / module.data_dir course_location = module.location + log.debug('======> IMPORTING course to location {0}'.format(course_location)) + module = remap_namespace(module, target_location_namespace) + if not do_import_static: + module.lms.static_asset_path = module.data_dir # for old-style xblock where this was actually linked to kvs + module._model_data['static_asset_path'] = module.data_dir + log.debug('course static_asset_path={0}'.format(module.lms.static_asset_path)) + + log.debug('course data_dir={0}'.format(module.data_dir)) + # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS, # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that - @@ -129,18 +146,35 @@ def import_from_xml(store, data_dir, course_dirs=None, {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge import_module(module, store, course_data_path, static_content_store, course_location, - target_location_namespace or course_location) + target_location_namespace or course_location, do_import_static=do_import_static) course_items.append(module) # then import all the static content - if static_content_store is not None: + if static_content_store is not None and do_import_static: _namespace_rename = target_location_namespace if target_location_namespace is not None else course_location # first pass to find everything in /static/ import_static_content(xml_module_store.modules[course_id], course_location, course_data_path, static_content_store, _namespace_rename, subpath='static', verbose=verbose) + elif verbose and not do_import_static: + log.debug('Skipping import of static content, since do_import_static={0}'.format(do_import_static)) + + # no matter what do_import_static is, import "static_import" directory + + # This is needed because the "about" pages (eg "overview") are loaded via load_extra_content, and + # do not inherit the lms metadata from the course module, and thus do not get "static_content_store" + # properly defined. Static content referenced in those extra pages thus need to come through the + # c4x:// contentstore, unfortunately. Tell users to copy that content into the "static_import" subdir. + + simport = 'static_import' + if os.path.exists(course_data_path / simport): + _namespace_rename = target_location_namespace if target_location_namespace is not None else course_location + + import_static_content(xml_module_store.modules[course_id], course_location, course_data_path, static_content_store, + _namespace_rename, subpath=simport, verbose=verbose) + # finally loop through all the modules for module in xml_module_store.modules[course_id].itervalues(): if module.category == 'course': @@ -156,7 +190,8 @@ def import_from_xml(store, data_dir, course_dirs=None, log.debug('importing module location {0}'.format(module.location)) import_module(module, store, course_data_path, static_content_store, course_location, - target_location_namespace if target_location_namespace else course_location) + target_location_namespace if target_location_namespace else course_location, + do_import_static=do_import_static) # now import any 'draft' items if draft_store is not None: @@ -176,7 +211,8 @@ def import_from_xml(store, data_dir, course_dirs=None, def import_module(module, store, course_data_path, static_content_store, - source_course_location, dest_course_location, allow_not_found=False): + source_course_location, dest_course_location, allow_not_found=False, + do_import_static=True): logging.debug('processing import of module {0}...'.format(module.location.url())) @@ -196,7 +232,7 @@ def import_module(module, store, course_data_path, static_content_store, else: module_data = content - if isinstance(module_data, basestring): + if isinstance(module_data, basestring) and do_import_static: # we want to convert all 'non-portable' links in the module_data (if it is a string) to # portable strings (e.g. /static/) module_data = rewrite_nonportable_content_links( diff --git a/common/test/data/simple_with_draft/README.md b/common/test/data/simple_with_draft/README.md new file mode 100644 index 000000000000..69ff6b4ed099 --- /dev/null +++ b/common/test/data/simple_with_draft/README.md @@ -0,0 +1,2 @@ +This is a simple, but non-trivial, course using multiple module types and some nested structure. + diff --git a/common/test/data/simple_with_draft/course.xml b/common/test/data/simple_with_draft/course.xml new file mode 100644 index 000000000000..c13068601200 --- /dev/null +++ b/common/test/data/simple_with_draft/course.xml @@ -0,0 +1,31 @@ + + + + +
+ + + +
+
+ +
diff --git a/common/test/data/simple_with_draft/drafts/vertical/test_vertical.xml b/common/test/data/simple_with_draft/drafts/vertical/test_vertical.xml new file mode 100644 index 000000000000..4433d282a431 --- /dev/null +++ b/common/test/data/simple_with_draft/drafts/vertical/test_vertical.xml @@ -0,0 +1,5 @@ + + + Foobar - edit in draft + + \ No newline at end of file diff --git a/common/test/data/simple_with_draft/html/toylab.html b/common/test/data/simple_with_draft/html/toylab.html new file mode 100644 index 000000000000..81df84bd6345 --- /dev/null +++ b/common/test/data/simple_with_draft/html/toylab.html @@ -0,0 +1,3 @@ +Lab 2A: Superposition Experiment + +

Isn't the toy course great?

diff --git a/common/test/data/simple_with_draft/problem/L1_Problem_1.xml b/common/test/data/simple_with_draft/problem/L1_Problem_1.xml new file mode 100644 index 000000000000..2ba061790426 --- /dev/null +++ b/common/test/data/simple_with_draft/problem/L1_Problem_1.xml @@ -0,0 +1,43 @@ + + +

+

Finger Exercise 1

+

+

+Here are two definitions:

+
    +
  1. +

    +Declarative knowledge refers to statements of fact.

    +
  2. +
  3. +

    +Imperative knowledge refers to 'how to' methods.

    +
  4. +
+

+Which of the following choices is correct?

+
    +
  1. +

    +Statement 1 is true, Statement 2 is false

    +
  2. +
  3. +

    +Statement 1 is false, Statement 2 is true

    +
  4. +
  5. +

    +Statement 1 and Statement 2 are both false

    +
  6. +
  7. +

    +Statement 1 and Statement 2 are both true

    +
  8. +
+

+ + + +

+
diff --git a/common/test/data/simple_with_draft/problem/ps01-simple.xml b/common/test/data/simple_with_draft/problem/ps01-simple.xml new file mode 100644 index 000000000000..e70d8f2c8d99 --- /dev/null +++ b/common/test/data/simple_with_draft/problem/ps01-simple.xml @@ -0,0 +1,62 @@ +