Merge pull request #17900 from open-craft/kshitij/swift_export_issue

Fix course export issues

cms/djangoapps/contentstore/views/import_export.py

@@ -13,8 +13,9 @@ from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django.core.files import File
+from django.core.files.storage import FileSystemStorage
 from django.db import transaction
-from django.http import Http404, HttpResponse, HttpResponseNotFound
+from django.http import Http404, HttpResponse, HttpResponseNotFound, StreamingHttpResponse
 from django.utils.translation import ugettext as _
 from django.views.decorators.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_GET, require_http_methods
@@ -22,6 +23,7 @@ from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import LibraryLocator
 from path import Path as path
 from six import text_type
+from storages.backends.s3boto import S3BotoStorage
 from user_tasks.conf import settings as user_tasks_settings
 from user_tasks.models import UserTaskArtifact, UserTaskStatus
 from wsgiref.util import FileWrapper
@@ -273,8 +275,8 @@ def send_tarball(tarball, size):
     """
     Renders a tarball to response, for use when sending a tar.gz file to the user.
     """
-    wrapper = FileWrapper(tarball)
-    response = HttpResponse(wrapper, content_type='application/x-tgz')
+    wrapper = FileWrapper(tarball, settings.COURSE_EXPORT_DOWNLOAD_CHUNK_SIZE)
+    response = StreamingHttpResponse(wrapper, content_type='application/x-tgz')
     response['Content-Disposition'] = 'attachment; filename=%s' % os.path.basename(tarball.name.encode('utf-8'))
     response['Content-Length'] = size
     return response
@@ -369,7 +371,9 @@ def export_status_handler(request, course_key_string):
     elif task_status.state == UserTaskStatus.SUCCEEDED:
         status = 3
         artifact = UserTaskArtifact.objects.get(status=task_status, name='Output')
-        if hasattr(artifact.file.storage, 'bucket'):
+        if isinstance(artifact.file.storage, FileSystemStorage):
+            output_url = reverse_course_url('export_output_handler', course_key)
+        elif isinstance(artifact.file.storage, S3BotoStorage):
             filename = os.path.basename(artifact.file.name).encode('utf-8')
             disposition = 'attachment; filename="{}"'.format(filename)
             output_url = artifact.file.storage.url(artifact.file.name, response_headers={
@@ -378,9 +382,7 @@ def export_status_handler(request, course_key_string):
                 'response-content-type': 'application/x-tgz'
             })
         else:
-            # local file, serve from the authorization wrapper view
-            output_url = reverse_course_url('export_output_handler', course_key)
-
+            output_url = artifact.file.storage.url(artifact.file.name)
     elif task_status.state in (UserTaskStatus.FAILED, UserTaskStatus.CANCELED):
         status = max(-(task_status.completed_steps + 1), -2)
         errors = UserTaskArtifact.objects.filter(status=task_status, name='Error')

cms/djangoapps/contentstore/views/tests/test_import_export.py

@@ -13,10 +13,14 @@ from uuid import uuid4
 import ddt
 import lxml
 from django.conf import settings
+from django.core.files.storage import FileSystemStorage
 from django.test.utils import override_settings
 from milestones.tests.utils import MilestonesTestCaseMixin
+from mock import Mock, patch
 from opaque_keys.edx.locator import LibraryLocator
 from path import Path as path
+from storages.backends.s3boto import S3BotoStorage
+from user_tasks.models import UserTaskStatus
 
 from contentstore.tests.test_libraries import LibraryTestCase
 from contentstore.tests.utils import CourseTestCase
@@ -132,6 +136,9 @@ class ImportTestCase(CourseTestCase):
     """
     Unit tests for importing a course or Library
     """
+
+    CREATE_USER = True
+
     def setUp(self):
         super(ImportTestCase, self).setUp()
         self.url = reverse_course_url('import_handler', self.course.id)
@@ -689,6 +696,75 @@ class ExportTestCase(CourseTestCase):
         resp = client.get(reverse_course_url('export_output_handler', self.course.id))
         self.assertEqual(resp.status_code, 403)
 
+    def _mock_artifact(self, spec=None, file_url=None):
+        """
+        Creates a Mock of the UserTaskArtifact model for testing exports handler
+        code without touching the database.
+        """
+        mock_artifact = Mock()
+        mock_artifact.file.name = 'testfile.tar.gz'
+        mock_artifact.file.storage = Mock(spec=spec)
+        mock_artifact.file.storage.url.return_value = file_url
+        return mock_artifact
+
+    @patch('contentstore.views.import_export._latest_task_status')
+    @patch('user_tasks.models.UserTaskArtifact.objects.get')
+    def test_export_status_handler_other(
+            self,
+            mock_get_user_task_artifact,
+            mock_latest_task_status,
+    ):
+        """
+        Verify that the export status handler generates the correct export path
+        for storage providers other than ``FileSystemStorage`` and
+        ``S3BotoStorage``
+        """
+        mock_latest_task_status.return_value = Mock(state=UserTaskStatus.SUCCEEDED)
+        mock_get_user_task_artifact.return_value = self._mock_artifact(
+            file_url='/path/to/testfile.tar.gz',
+        )
+        resp = self.client.get(self.status_url)
+        result = json.loads(resp.content)
+        self.assertEqual(result['ExportOutput'], '/path/to/testfile.tar.gz')
+
+    @patch('contentstore.views.import_export._latest_task_status')
+    @patch('user_tasks.models.UserTaskArtifact.objects.get')
+    def test_export_status_handler_s3(
+            self,
+            mock_get_user_task_artifact,
+            mock_latest_task_status,
+    ):
+        """
+        Verify that the export status handler generates the correct export path
+        for the ``S3BotoStorage`` storage provider
+        """
+        mock_latest_task_status.return_value = Mock(state=UserTaskStatus.SUCCEEDED)
+        mock_get_user_task_artifact.return_value = self._mock_artifact(
+            spec=S3BotoStorage,
+            file_url='/s3/file/path/testfile.tar.gz',
+        )
+        resp = self.client.get(self.status_url)
+        result = json.loads(resp.content)
+        self.assertEqual(result['ExportOutput'], '/s3/file/path/testfile.tar.gz')
+
+    @patch('contentstore.views.import_export._latest_task_status')
+    @patch('user_tasks.models.UserTaskArtifact.objects.get')
+    def test_export_status_handler_filesystem(
+            self,
+            mock_get_user_task_artifact,
+            mock_latest_task_status,
+    ):
+        """
+        Verify that the export status handler generates the correct export path
+        for the ``FileSystemStorage`` storage provider
+        """
+        mock_latest_task_status.return_value = Mock(state=UserTaskStatus.SUCCEEDED)
+        mock_get_user_task_artifact.return_value = self._mock_artifact(spec=FileSystemStorage)
+        resp = self.client.get(self.status_url)
+        result = json.loads(resp.content)
+        file_export_output_url = reverse_course_url('export_output_handler', self.course.id)
+        self.assertEqual(result['ExportOutput'], file_export_output_url)
+
 
 @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
 class TestLibraryImportExport(CourseTestCase):

cms/envs/common.py

@@ -1536,3 +1536,8 @@ COMPLETION_VIDEO_COMPLETE_PERCENTAGE = 0.95
 from openedx.core.djangoapps.plugins import plugin_apps, plugin_settings, constants as plugin_constants
 INSTALLED_APPS.extend(plugin_apps.get_apps(plugin_constants.ProjectType.CMS))
 plugin_settings.add_plugins(__name__, plugin_constants.ProjectType.CMS, plugin_constants.SettingsType.COMMON)
+
+# Course exports streamed in blocks of this size. 8192 or 8kb is the default
+# setting for the FileWrapper class used to iterate over the export file data.
+# See: https://docs.python.org/2/library/wsgiref.html#wsgiref.util.FileWrapper
+COURSE_EXPORT_DOWNLOAD_CHUNK_SIZE = 8192