Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Zip files for Course Import #33561

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions cms/djangoapps/contentstore/api/views/course_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from cms.djangoapps.contentstore.storage import course_import_export_storage
from cms.djangoapps.contentstore.tasks import CourseImportTask, import_olx
from cms.djangoapps.contentstore.utils import IMPORTABLE_FILE_TYPES
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes

from .utils import course_author_access_required
Expand All @@ -44,8 +45,8 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):
"""
**Use Case**

* Start an asynchronous task to import a course from a .tar.gz file into
the specified course ID, overwriting the existing course
* Start an asynchronous task to import a course from a .tar.gz or .zip
file into the specified course ID, overwriting the existing course
* Get a status on an asynchronous task import

**Example Requests**
Expand All @@ -59,7 +60,7 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):

* course_id: (required) A string representation of a Course ID,
e.g., course-v1:edX+DemoX+Demo_Course
* course_data: (required) The course .tar.gz file to import
* course_data: (required) The course .tar.gz or .zip file to import

**POST Response Values**

Expand All @@ -83,7 +84,7 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):
A GET request must include the following parameters.

* task_id: (required) The UUID of the task to check, e.g. "4b357bb3-2a1e-441d-9f6c-2210cf76606f"
* filename: (required) The filename of the uploaded course .tar.gz
* filename: (required) The filename of the uploaded course .tar.gz or .zip

**GET Response Values**

Expand Down Expand Up @@ -124,7 +125,7 @@ def post(self, request, course_key):
)

filename = request.FILES['course_data'].name
if not filename.endswith('.tar.gz'):
if not filename.endswith(IMPORTABLE_FILE_TYPES):
raise self.api_error(
status_code=status.HTTP_400_BAD_REQUEST,
developer_message='Parameter in the wrong format',
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
UNKNOWN_ERROR_IN_IMPORT = _('Unknown error while importing course.')
UNKNOWN_ERROR_IN_UNPACKING = _('An Unknown error occurred during the unpacking step.')
UNKNOWN_USER_ID = _('Unknown User ID: {0}')
UNSAFE_TAR_FILE = _('Unsafe tar file. Aborting import.')
UNSAFE_ARCHIVE_FILE = _('Unsafe archive file. Aborting import.')
USER_PERMISSION_DENIED = _('User permission denied.')
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import base64
import os
import tarfile

from django.conf import settings
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
Expand All @@ -16,7 +15,7 @@
from path import Path

from cms.djangoapps.contentstore.utils import add_instructor
from openedx.core.lib.extract_tar import safetar_extractall
from openedx.core.lib.extract_archives import safe_extractall
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -47,13 +46,10 @@ def handle(self, *args, **options):
course_dir = data_root / subdir

# Extract library archive
tar_file = tarfile.open(archive_path) # lint-amnesty, pylint: disable=consider-using-with
try:
safetar_extractall(tar_file, course_dir)
safe_extractall(archive_path, course_dir)
except SuspiciousOperation as exc:
raise CommandError(f'\n=== Course import {archive_path}: Unsafe tar file - {exc.args[0]}\n') # lint-amnesty, pylint: disable=raise-missing-from
finally:
tar_file.close()

# Paths to the library.xml file
abs_xml_path = os.path.join(course_dir, 'library')
Expand Down
22 changes: 12 additions & 10 deletions cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@
SearchIndexingError
)
from cms.djangoapps.contentstore.storage import course_import_export_storage
from cms.djangoapps.contentstore.utils import initialize_permissions, reverse_usage_url, translation_language
from cms.djangoapps.contentstore.utils import (
initialize_permissions,
reverse_usage_url,
translation_language,
IMPORTABLE_FILE_TYPES,
)
from cms.djangoapps.models.settings.course_metadata import CourseMetadata

from common.djangoapps.course_action_state.models import CourseRerunState
Expand All @@ -60,7 +65,7 @@
from openedx.core.djangoapps.discussions.tasks import update_unit_discussion_state_from_discussion_blocks
from openedx.core.djangoapps.embargo.models import CountryAccessRule, RestrictedCourse
from openedx.core.lib.blockstore_api import get_collection
from openedx.core.lib.extract_tar import safetar_extractall
from openedx.core.lib.extract_archives import safe_extractall
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.course_block import CourseFields # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.exceptions import SerializationError # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -448,7 +453,7 @@ def generate_name(cls, arguments_dict):
# lint-amnesty, pylint: disable=too-many-statements
def import_olx(self, user_id, course_key_string, archive_path, archive_name, language):
"""
Import a course or library from a provided OLX .tar.gz archive.
Import a course or library from a provided OLX .tar.gz or .zip archive.
"""
set_code_owner_attribute_from_module(__name__)
current_step = 'Unpacking'
Expand Down Expand Up @@ -485,7 +490,7 @@ def user_has_access(user):

def file_is_supported():
"""Check if it is a supported file."""
file_is_valid = archive_name.endswith('.tar.gz')
file_is_valid = archive_name.endswith(IMPORTABLE_FILE_TYPES)

if not file_is_valid:
message = f'Unsupported file {archive_name}'
Expand Down Expand Up @@ -614,17 +619,14 @@ def read_chunk():

# try-finally block for proper clean up after receiving file.
try:
tar_file = tarfile.open(temp_filepath) # lint-amnesty, pylint: disable=consider-using-with
try:
safetar_extractall(tar_file, (course_dir + '/'))
safe_extractall(temp_filepath, course_dir)
except SuspiciousOperation as exc:
with translation_language(language):
self.status.fail(UserErrors.UNSAFE_TAR_FILE)
LOGGER.error(f'{log_prefix}: Unsafe tar file')
self.status.fail(UserErrors.UNSAFE_ARCHIVE_FILE)
LOGGER.error(f'{log_prefix}: Unsafe archive file')
monitor_import_failure(courselike_key, current_step, exception=exc)
return
finally:
tar_file.close()

current_step = 'Verifying'
self.status.set_state(current_step)
Expand Down
1 change: 1 addition & 0 deletions cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService


IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip')
log = logging.getLogger(__name__)


Expand Down
17 changes: 11 additions & 6 deletions cms/djangoapps/contentstore/views/import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,13 @@
from ..storage import course_import_export_storage
from ..tasks import CourseExportTask, CourseImportTask, export_olx, import_olx
from ..toggles import use_new_export_page, use_new_import_page
from ..utils import reverse_course_url, reverse_library_url, get_export_url, get_import_url

from ..utils import (
reverse_course_url,
reverse_library_url,
get_export_url,
get_import_url,
IMPORTABLE_FILE_TYPES,
)
__all__ = [
'import_handler', 'import_status_handler',
'export_handler', 'export_output_handler', 'export_status_handler',
Expand All @@ -70,7 +75,7 @@ def import_handler(request, course_key_string):
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
json: import a course via the .tar.gz or .zip file specified in request.FILES
"""
courselike_key = CourseKey.from_string(course_key_string)
library = isinstance(courselike_key, LibraryLocator)
Expand Down Expand Up @@ -122,7 +127,7 @@ def _write_chunk(request, courselike_key): # lint-amnesty, pylint: disable=too-
"""
Write the OLX file data chunk from the given request to the local filesystem.
"""
# Upload .tar.gz to local filesystem for one-server installations not using S3 or Swift
# Upload .tar.gz or .zip to local filesystem for one-server installations not using S3 or Swift
data_root = path(settings.GITHUB_REPO_ROOT)
subdir = base64.urlsafe_b64encode(repr(courselike_key).encode('utf-8')).decode('utf-8')
course_dir = data_root / subdir
Expand All @@ -140,8 +145,8 @@ def error_response(message, status, stage):
# Use sessions to keep info about import progress
_save_request_status(request, courselike_string, 0)

if not filename.endswith('.tar.gz'):
error_message = _('We only support uploading a .tar.gz file.')
if not filename.endswith(IMPORTABLE_FILE_TYPES):
error_message = _('We support uploading files in one of the following formats: {IMPORTABLE_FILE_TYPES}')
_save_request_status(request, courselike_string, -1)
monitor_import_failure(courselike_key, current_step, message=error_message)
return error_response(error_message, 415, 0)
Expand Down
Loading
Loading