From a9d1d4eefc37e260a95fe3b4e5a116a6cc9797b9 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 28 Oct 2013 11:46:29 -0400 Subject: [PATCH] Convert import to new URL pattern. --- .../contentstore/tests/test_contentstore.py | 7 +- .../contentstore/tests/test_import_export.py | 37 +- .../contentstore/views/import_export.py | 367 +++++++++--------- cms/templates/import.html | 6 +- cms/templates/widgets/header.html | 3 +- cms/urls.py | 10 +- 6 files changed, 217 insertions(+), 213 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 3ba3e30493..b40a821ca9 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1592,10 +1592,7 @@ class ContentStoreTest(ModuleStoreTestCase): # go to various pages # import page - resp = self.client.get(reverse('import_course', - kwargs={'org': loc.org, - 'course': loc.course, - 'name': loc.name})) + resp = self.client.get(new_location.url_reverse('import/', ''), HTTP_ACCEPT='text/html') self.assertEqual(resp.status_code, 200) # export page @@ -1632,9 +1629,7 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertEqual(resp.status_code, 200) # assets_handler (HTML for full page content) - new_location = loc_mapper().translate_location(loc.course_id, loc, False, True) url = new_location.url_reverse('assets/', '') - resp = self.client.get(url, HTTP_ACCEPT='text/html') self.assertEqual(resp.status_code, 200) diff --git a/cms/djangoapps/contentstore/tests/test_import_export.py b/cms/djangoapps/contentstore/tests/test_import_export.py index 9f60680d17..9f6b44ff1d 100644 --- a/cms/djangoapps/contentstore/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/tests/test_import_export.py @@ -13,9 +13,9 @@ from uuid import uuid4 from pymongo import MongoClient from .utils import CourseTestCase -from django.core.urlresolvers import reverse from django.test.utils import override_settings from django.conf import settings +from xmodule.modulestore.django import loc_mapper from xmodule.contentstore.django import _CONTENTSTORE @@ -32,11 +32,10 @@ class ImportTestCase(CourseTestCase): def setUp(self): super(ImportTestCase, self).setUp() - self.url = reverse("import_course", kwargs={ - 'org': self.course.location.org, - 'course': self.course.location.course, - 'name': self.course.location.name, - }) + self.new_location = loc_mapper().translate_location( + self.course.location.course_id, self.course.location, False, True + ) + self.url = self.new_location.url_reverse('import/', '') self.content_dir = path(tempfile.mkdtemp()) def touch(name): @@ -89,13 +88,13 @@ class ImportTestCase(CourseTestCase): self.assertEquals(resp.status_code, 415) # Check that `import_status` returns the appropriate stage (i.e., the # stage at which import failed). - status_url = reverse("import_status", kwargs={ - 'org': self.course.location.org, - 'course': self.course.location.course, - 'name': os.path.split(self.bad_tar)[1], - }) - resp_status = self.client.get(status_url) - log.debug(str(self.client.session["import_status"])) + resp_status = self.client.get( + self.new_location.url_reverse( + 'import_status', + os.path.split(self.bad_tar)[1] + ) + ) + self.assertEquals(json.loads(resp_status.content)["ImportStatus"], 2) @@ -200,11 +199,11 @@ class ImportTestCase(CourseTestCase): # Check that `import_status` returns the appropriate stage (i.e., # either 3, indicating all previous steps are completed, or 0, # indicating no upload in progress) - status_url = reverse("import_status", kwargs={ - 'org': self.course.location.org, - 'course': self.course.location.course, - 'name': os.path.split(self.good_tar)[1], - }) - resp_status = self.client.get(status_url) + resp_status = self.client.get( + self.new_location.url_reverse( + 'import_status', + os.path.split(self.good_tar)[1] + ) + ) import_status = json.loads(resp_status.content)["ImportStatus"] self.assertIn(import_status, (0, 3)) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index f16bc374dc..d8da8d0186 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -17,7 +17,8 @@ from django_future.csrf import ensure_csrf_cookie from django.core.urlresolvers import reverse from django.core.servers.basehttp import FileWrapper from django.core.files.temp import NamedTemporaryFile -from django.core.exceptions import SuspiciousOperation +from django.core.exceptions import SuspiciousOperation, PermissionDenied +from django.http import HttpResponseNotFound from django.views.decorators.http import require_http_methods, require_GET from django.utils.translation import ugettext as _ @@ -30,13 +31,15 @@ from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore import Location from xmodule.exceptions import SerializationError +from xmodule.modulestore.locator import BlockUsageLocator +from .access import has_access from .access import get_location_and_verify_access from util.json_request import JsonResponse from extract_tar import safetar_extractall -__all__ = ['import_course', 'import_status', 'generate_export_course', 'export_course'] +__all__ = ['import_handler', 'import_status_handler', 'generate_export_course', 'export_course'] log = logging.getLogger(__name__) @@ -45,212 +48,221 @@ log = logging.getLogger(__name__) CONTENT_RE = re.compile(r"(?P\d{1,11})-(?P\d{1,11})/(?P\d{1,11})") +@login_required @ensure_csrf_cookie @require_http_methods(("GET", "POST", "PUT")) -@login_required -def import_course(request, org, course, name): +def import_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): """ - This method will handle a POST request to upload and import a .tar.gz file - into a specified course + The restful handler for importing a course. + + GET + html: return html page for import page + json: not supported + POST or PUT + json: import a course via the .tar.gz file specified in request.FILES """ - location = get_location_and_verify_access(request, org, course, name) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, location): + raise PermissionDenied() - if request.method == 'POST': + old_location = loc_mapper().translate_locator_to_location(location) - data_root = path(settings.GITHUB_REPO_ROOT) - course_subdir = "{0}-{1}-{2}".format(org, course, name) - course_dir = data_root / course_subdir - - filename = request.FILES['course-data'].name - if not filename.endswith('.tar.gz'): - return JsonResponse( - { - 'ErrMsg': _('We only support uploading a .tar.gz file.'), - 'Stage': 1 - }, - status=415 - ) - temp_filepath = course_dir / filename - - if not course_dir.isdir(): - os.mkdir(course_dir) - - logging.debug('importing course to {0}'.format(temp_filepath)) - - # Get upload chunks byte ranges - try: - matches = CONTENT_RE.search(request.META["HTTP_CONTENT_RANGE"]) - content_range = matches.groupdict() - except KeyError: # Single chunk - # no Content-Range header, so make one that will work - content_range = {'start': 0, 'stop': 1, 'end': 2} - - # stream out the uploaded files in chunks to disk - if int(content_range['start']) == 0: - mode = "wb+" + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + if request.method == 'GET': + raise NotImplementedError('coming soon') else: - mode = "ab+" - size = os.path.getsize(temp_filepath) - # Check to make sure we haven't missed a chunk - # This shouldn't happen, even if different instances are handling - # the same session, but it's always better to catch errors earlier. - if size < int(content_range['start']): - log.warning( - "Reported range %s does not match size downloaded so far %s", - content_range['start'], - size - ) + data_root = path(settings.GITHUB_REPO_ROOT) + course_subdir = "{0}-{1}-{2}".format(old_location.org, old_location.course, old_location.name) + course_dir = data_root / course_subdir + + filename = request.FILES['course-data'].name + if not filename.endswith('.tar.gz'): return JsonResponse( { - 'ErrMsg': _('File upload corrupted. Please try again'), + 'ErrMsg': _('We only support uploading a .tar.gz file.'), 'Stage': 1 }, - status=409 + status=415 ) - # The last request sometimes comes twice. This happens because - # nginx sends a 499 error code when the response takes too long. - elif size > int(content_range['stop']) and size == int(content_range['end']): - return JsonResponse({'ImportStatus': 1}) + temp_filepath = course_dir / filename - with open(temp_filepath, mode) as temp_file: - for chunk in request.FILES['course-data'].chunks(): - temp_file.write(chunk) + if not course_dir.isdir(): + os.mkdir(course_dir) - size = os.path.getsize(temp_filepath) + logging.debug('importing course to {0}'.format(temp_filepath)) - if int(content_range['stop']) != int(content_range['end']) - 1: - # More chunks coming - return JsonResponse({ - "files": [{ - "name": filename, - "size": size, - "deleteUrl": "", - "deleteType": "", - "url": reverse('import_course', kwargs={ - 'org': location.org, - 'course': location.course, - 'name': location.name - }), - "thumbnailUrl": "" - }] - }) - - else: # This was the last chunk. - - # Use sessions to keep info about import progress - session_status = request.session.setdefault("import_status", {}) - key = org + course + filename - session_status[key] = 1 - request.session.modified = True - - # Do everything from now on in a try-finally block to make sure - # everything is properly cleaned up. + # Get upload chunks byte ranges try: + matches = CONTENT_RE.search(request.META["HTTP_CONTENT_RANGE"]) + content_range = matches.groupdict() + except KeyError: # Single chunk + # no Content-Range header, so make one that will work + content_range = {'start': 0, 'stop': 1, 'end': 2} - tar_file = tarfile.open(temp_filepath) - try: - safetar_extractall(tar_file, (course_dir + '/').encode('utf-8')) - except SuspiciousOperation as exc: + # stream out the uploaded files in chunks to disk + if int(content_range['start']) == 0: + mode = "wb+" + else: + mode = "ab+" + size = os.path.getsize(temp_filepath) + # Check to make sure we haven't missed a chunk + # This shouldn't happen, even if different instances are handling + # the same session, but it's always better to catch errors earlier. + if size < int(content_range['start']): + log.warning( + "Reported range %s does not match size downloaded so far %s", + content_range['start'], + size + ) return JsonResponse( { - 'ErrMsg': 'Unsafe tar file. Aborting import.', - 'SuspiciousFileOperationMsg': exc.args[0], + 'ErrMsg': _('File upload corrupted. Please try again'), 'Stage': 1 }, + status=409 + ) + # The last request sometimes comes twice. This happens because + # nginx sends a 499 error code when the response takes too long. + elif size > int(content_range['stop']) and size == int(content_range['end']): + return JsonResponse({'ImportStatus': 1}) + + with open(temp_filepath, mode) as temp_file: + for chunk in request.FILES['course-data'].chunks(): + temp_file.write(chunk) + + size = os.path.getsize(temp_filepath) + + if int(content_range['stop']) != int(content_range['end']) - 1: + # More chunks coming + return JsonResponse({ + "files": [{ + "name": filename, + "size": size, + "deleteUrl": "", + "deleteType": "", + "url": location.url_reverse('import/', ''), + "thumbnailUrl": "" + }] + }) + + else: # This was the last chunk. + + # Use sessions to keep info about import progress + session_status = request.session.setdefault("import_status", {}) + key = location.course_id + filename + session_status[key] = 1 + request.session.modified = True + + # Do everything from now on in a try-finally block to make sure + # everything is properly cleaned up. + try: + + tar_file = tarfile.open(temp_filepath) + try: + safetar_extractall(tar_file, (course_dir + '/').encode('utf-8')) + except SuspiciousOperation as exc: + return JsonResponse( + { + 'ErrMsg': 'Unsafe tar file. Aborting import.', + 'SuspiciousFileOperationMsg': exc.args[0], + 'Stage': 1 + }, + status=400 + ) + finally: + tar_file.close() + + session_status[key] = 2 + request.session.modified = True + + # find the 'course.xml' file + def get_all_files(directory): + """ + For each file in the directory, yield a 2-tuple of (file-name, + directory-path) + """ + for dirpath, _dirnames, filenames in os.walk(directory): + for filename in filenames: + yield (filename, dirpath) + + def get_dir_for_fname(directory, filename): + """ + Returns the dirpath for the first file found in the directory + with the given name. If there is no file in the directory with + the specified name, return None. + """ + for fname, dirpath in get_all_files(directory): + if fname == filename: + return dirpath + return None + + fname = "course.xml" + + dirpath = get_dir_for_fname(course_dir, fname) + + if not dirpath: + return JsonResponse( + { + + 'ErrMsg': _('Could not find the course.xml file in the package.'), + 'Stage': 2 + }, + status=415 + ) + + logging.debug('found course.xml at {0}'.format(dirpath)) + + if dirpath != course_dir: + for fname in os.listdir(dirpath): + shutil.move(dirpath / fname, course_dir) + + _module_store, course_items = import_from_xml( + modulestore('direct'), + settings.GITHUB_REPO_ROOT, + [course_subdir], + load_error_modules=False, + static_content_store=contentstore(), + target_location_namespace=old_location, + draft_store=modulestore() + ) + + logging.debug('new course at {0}'.format(course_items[0].location)) + + session_status[key] = 3 + request.session.modified = True + + create_all_course_groups(request.user, course_items[0].location) + logging.debug('created all course groups at {0}'.format(course_items[0].location)) + + # Send errors to client with stage at which error occured. + except Exception as exception: # pylint: disable=W0703 + return JsonResponse( + { + 'ErrMsg': str(exception), + 'Stage': session_status[key] + }, status=400 ) + finally: - tar_file.close() + shutil.rmtree(course_dir) - session_status[key] = 2 - request.session.modified = True - - # find the 'course.xml' file - def get_all_files(directory): - """ - For each file in the directory, yield a 2-tuple of (file-name, - directory-path) - """ - for dirpath, _dirnames, filenames in os.walk(directory): - for filename in filenames: - yield (filename, dirpath) - - def get_dir_for_fname(directory, filename): - """ - Returns the dirpath for the first file found in the directory - with the given name. If there is no file in the directory with - the specified name, return None. - """ - for fname, dirpath in get_all_files(directory): - if fname == filename: - return dirpath - return None - - fname = "course.xml" - - dirpath = get_dir_for_fname(course_dir, fname) - - if not dirpath: - return JsonResponse( - { - - 'ErrMsg': _('Could not find the course.xml file in the package.'), - 'Stage': 2 - }, - status=415 - ) - - logging.debug('found course.xml at {0}'.format(dirpath)) - - if dirpath != course_dir: - for fname in os.listdir(dirpath): - shutil.move(dirpath / fname, course_dir) - - _module_store, course_items = import_from_xml( - modulestore('direct'), - settings.GITHUB_REPO_ROOT, - [course_subdir], - load_error_modules=False, - static_content_store=contentstore(), - target_location_namespace=location, - draft_store=modulestore() - ) - - logging.debug('new course at {0}'.format(course_items[0].location)) - - session_status[key] = 3 - request.session.modified = True - - create_all_course_groups(request.user, course_items[0].location) - logging.debug('created all course groups at {0}'.format(course_items[0].location)) - - # Send errors to client with stage at which error occured. - except Exception as exception: # pylint: disable=W0703 - return JsonResponse( - { - 'ErrMsg': str(exception), - 'Stage': session_status[key] - }, - status=400 - ) - - finally: - shutil.rmtree(course_dir) - - return JsonResponse({'Status': 'OK'}) - else: - course_module = modulestore().get_item(location) - new_location = loc_mapper().translate_location(course_module.location.course_id, course_module.location, False, True) + return JsonResponse({'Status': 'OK'}) + elif request.method == 'GET': # assume html + course_module = modulestore().get_item(old_location) return render_to_response('import.html', { 'context_course': course_module, - 'successful_import_redirect_url': new_location.url_reverse("course/", "") + 'successful_import_redirect_url': location.url_reverse("course/", ""), + 'import_status_url': location.url_reverse("import_status/", "fillerName"), }) + else: + return HttpResponseNotFound() @require_GET @ensure_csrf_cookie @login_required -def import_status(request, org, course, name): +def import_status_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None, filename=None): """ Returns an integer corresponding to the status of a file import. These are: @@ -260,10 +272,13 @@ def import_status(request, org, course, name): 3 : Importing to mongo """ + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, location): + raise PermissionDenied() try: session_status = request.session["import_status"] - status = session_status[org + course + name] + status = session_status[location.course_id + filename] except KeyError: status = 0 diff --git a/cms/templates/import.html b/cms/templates/import.html index 9b1eca234e..5485af08aa 100644 --- a/cms/templates/import.html +++ b/cms/templates/import.html @@ -1,7 +1,6 @@ <%inherit file="base.html" /> <%namespace name='static' file='static_content.html'/> <%! - from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ %> <%block name="title">${_("Course Import")} @@ -28,8 +27,7 @@

${_("During the initial stages of the import process, please do not navigate away from this page.")}

-
+ @@ -158,7 +156,7 @@ var chooseBtn = $('.choose-file-button'); var allStats = $('#status-infos'); -var feedbackUrl = "${reverse('import_status', kwargs=dict(org=context_course.location.org, course=context_course.location.course, name='fillerName'))}" +var feedbackUrl = "${import_status_url}"; var defaults = [ '${_("There was an error during the upload process.")}\n', diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 764b1429dc..d1eb2a3ac6 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -20,6 +20,7 @@ checklists_url = location.url_reverse('checklists/', '') course_team_url = location.url_reverse('course_team/', '') assets_url = location.url_reverse('assets/', '') + import_url = location.url_reverse('import/', '') %>

${_("Current Course:")} @@ -91,7 +92,7 @@ ${_("Checklists")}
  • - ${_("Import")} + ${_("Import")}
  • ${_("Export")} diff --git a/cms/urls.py b/cms/urls.py index 74a65df011..98a9e60d49 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -1,4 +1,3 @@ -import re from django.conf import settings from django.conf.urls import patterns, include, url @@ -34,11 +33,6 @@ urlpatterns = patterns('', # nopep8 url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'), url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'), - url(r'^(?P[^/]+)/(?P[^/]+)/import/(?P[^/]+)$', - 'contentstore.views.import_course', name='import_course'), - url(r'^(?P[^/]+)/(?P[^/]+)/import_status/(?P[^/]+)$', - 'contentstore.views.import_status', name='import_status'), - url(r'^(?P[^/]+)/(?P[^/]+)/export/(?P[^/]+)$', 'contentstore.views.export_course', name='export_course'), url(r'^(?P[^/]+)/(?P[^/]+)/generate_export/(?P[^/]+)$', @@ -129,7 +123,9 @@ urlpatterns += patterns( url(r'(?ix)^checklists/{}(/)?(?P\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'), url(r'(?ix)^course_team/{}(/)?(?P.+)?$'.format(parsers.URL_RE_SOURCE), 'course_team_handler'), url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan'), - url(r'(?ix)^assets/{}(/)?(?P.+)?$'.format(parsers.URL_RE_SOURCE), 'assets_handler') + url(r'(?ix)^assets/{}(/)?(?P.+)?$'.format(parsers.URL_RE_SOURCE), 'assets_handler'), + url(r'(?ix)^import/{}$'.format(parsers.URL_RE_SOURCE), 'import_handler'), + url(r'(?ix)^import_status/{}/(?P.+)$'.format(parsers.URL_RE_SOURCE), 'import_status_handler'), ) js_info_dict = {