diff --git a/apiary.apib b/apiary.apib index 211e7359..dee7d629 100644 --- a/apiary.apib +++ b/apiary.apib @@ -473,16 +473,13 @@ Clears the export list for this repository. + Response 204 (application/json) -## Group LearningResourceExportTasks +## Group Tasks -## LearningResourceExportTasks Collection [/repositories/{repo_slug}/learning_resource_export_tasks/] +## Tasks Collection [/tasks/] -+ Parameters - + repo_slug: `physics-1` (string, required) - slug for the repository +### List All Tasks [GET] -### List All LearningResourceExportTasks [GET] - -List all recent LearningResourceExportTasks for a user. +List all recent tasks for a user. + Response 200 (application/json) @@ -494,19 +491,34 @@ List all recent LearningResourceExportTasks for a user. { "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3", "status": "processing", - "url": "/media/resource_exports/sarah_exports.tar.gz" + "result": { + "url": "/media/resource_exports/sarah_exports.tar.gz", + "collision": false + }, + "task_type": "resource_export", + "task_info": { + "repo_slug": "repo", + "ids": [ + 23517, + 23518 + ] + } } ] } -### Create a New LearningResourceExportTask [POST] +### Create a New Task [POST] -Queue a new LearningResourceExportTask task for the given LearningResource ids. +Queue a new task. + Request (application/json) { - "ids": [1] + "task_info": { + "repo_slug": "repo", + "ids": [1] + }, + "task_type": "resource_export" } + Response 200 (application/json) @@ -515,20 +527,30 @@ Queue a new LearningResourceExportTask task for the given LearningResource ids. "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3" } -## LearningResourceExportTask [/repositories/{repo_slug}/learning_resource_export_tasks/{task_id}/ +## Task [/tasks/{task_id}/ + Parameters - + repo_slug: `physics-1` (string, required) - slug for the repository + task_id: `45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3` (string, required) - task identifier -### Retrieve a LearningResourceExportTask [GET] +### Retrieve a Task [GET] + Response 200 (application/json) { "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3", - "status": "success", - "url": "/media/resource_exports/sarah_exports.tar.gz" + "status": "processing", + "result": { + "url": "/media/resource_exports/sarah_exports.tar.gz", + "collision": false + }, + "task_type": "resource_export", + "task_info": { + "repo_slug": "repo", + "ids": [ + 23517, + 23518 + ] + } } ## Group StaticAssets diff --git a/exporter/tasks.py b/exporter/tasks.py index 0386ce75..d315be8e 100644 --- a/exporter/tasks.py +++ b/exporter/tasks.py @@ -4,6 +4,8 @@ from __future__ import unicode_literals +from django.core.files.storage import default_storage + from lore.celery import async from exporter.api import export_resources_to_tarball @@ -18,8 +20,14 @@ def export_resources(learning_resources, username): LearningResources to export in tarball username (unicode): Name of user Returns: - (unicode, bool): - First item is newly created temp directory with files inside of it. - Second item is True if a static asset collision was detected. + dict: + name is path of tarball. + url is URL of tarball using django-storage. + collision is True if a static asset collision was detected. """ - return export_resources_to_tarball(learning_resources, username) + name, collision = export_resources_to_tarball(learning_resources, username) + return { + "name": name, + "url": default_storage.url(name), + "collision": collision + } diff --git a/exporter/tests/test_export.py b/exporter/tests/test_export.py index 7fd23259..13b4e85d 100644 --- a/exporter/tests/test_export.py +++ b/exporter/tests/test_export.py @@ -175,8 +175,10 @@ def test_export_task(self): """Test exporting resources task.""" resources = LearningResource.objects.all() - path, collision = export_resources.delay( + result = export_resources.delay( resources, self.user.username).get() + path = result['name'] + collision = result['collision'] tempdir = mkdtemp() self.assertTrue(collision) diff --git a/rest/serializers.py b/rest/serializers.py index fc47a7af..1f6a4166 100644 --- a/rest/serializers.py +++ b/rest/serializers.py @@ -21,6 +21,7 @@ SerializerMethodField, IntegerField, FloatField, + DictField, ) from rest.util import LambdaDefault, RequiredBooleanField @@ -303,11 +304,13 @@ class LearningResourceExportSerializer(Serializer): id = IntegerField() -class LearningResourceExportTaskSerializer(Serializer): +class TaskSerializer(Serializer): """Serializer for export tasks for LearningResource.""" id = CharField() status = CharField() - url = CharField() + result = DictField() + task_type = CharField() + task_info = DictField() class RepositorySearchSerializer(Serializer): diff --git a/rest/tasks.py b/rest/tasks.py new file mode 100644 index 00000000..a6bbb818 --- /dev/null +++ b/rest/tasks.py @@ -0,0 +1,186 @@ +""" +Functions to manipulate tasks via REST API. +""" + +from __future__ import unicode_literals + +from celery.result import AsyncResult +from celery.states import FAILURE, SUCCESS, REVOKED +from django.contrib.auth.models import User +from rest_framework.exceptions import ValidationError + +from exporter.tasks import export_resources +from learningresources.api import get_repo +from learningresources.models import LearningResource + +TASK_KEY = 'tasks' +EXPORT_TASK_TYPE = 'resource_export' +EXPORTS_KEY = 'learning_resource_exports' + + +def create_initial_task_dict(task, task_type, task_info): + """ + Create initial task data about a newly created Celery task. + + Args: + task (Task): A Celery task. + task_type (unicode): Type of task. + task_info (dict): Extra information about a task. + Returns: + dict: Initial data about task. + """ + + result = None + if task.successful(): + result = task.get() + + return { + "id": task.id, + "initial_state": task.state, + "task_type": task_type, + "task_info": task_info, + "result": result + } + + +def create_task_result_dict(initial_data): + """ + Convert initial data we put in session to dict for REST API. + This will use the id to look up current data about task to return + to user. + + Args: + task (dict): Initial data about task stored in session. + Returns: + dict: Updated data about task. + """ + initial_state = initial_data['initial_state'] + task_id = initial_data['id'] + task_type = initial_data['task_type'] + task_info = initial_data['task_info'] + + state = "processing" + result = None + # initial_state is a workaround for EagerResult used in testing. + # In production initial_state should usually be pending. + if initial_state == SUCCESS: + state = "success" + result = initial_data['result'] + elif initial_state in (FAILURE, REVOKED): + state = "failure" + else: + async_result = AsyncResult(task_id) + + if async_result.successful(): + state = "success" + elif async_result.failed(): + state = "failure" + + if async_result.successful(): + result = async_result.get() + + return { + "id": task_id, + "status": state, + "result": result, + "task_type": task_type, + "task_info": task_info + } + + +def get_tasks(session): + """ + Get initial task data for session. + + Args: + session (SessionStore): The request session. + Returns: + dict: + The initial task data stored in session for all user's tasks. The + keys are task ids and the values are initial task data. + """ + try: + return session[TASK_KEY] + except KeyError: + return {} + + +def get_task(session, task_id): + """ + Get initial task data for a single task. + + Args: + session (SessionStore): The request session. + task_id (unicode): The task id. + Returns: + dict: The initial task data stored in session. + """ + try: + return session[TASK_KEY][task_id] + except KeyError: + return None + + +def create_task(session, user_id, task_type, task_info): + """ + Start a new Celery task. This will stop other tasks of the same type. + + Args: + session (SessionStore): The request session. + user_id (int): The id for user creating task. + task_type (unicode): The type of task being started. + task_info (dict): Extra information about the task. + Returns: + dict: The initial task data (will also be stored in session). + """ + + if task_type == EXPORT_TASK_TYPE: + try: + repo_slug = task_info['repo_slug'] + except KeyError: + raise ValidationError("Missing repo_slug") + + # Verify repository ownership. + get_repo(repo_slug, user_id) + + try: + exports = set(session[EXPORTS_KEY][repo_slug]) + except KeyError: + exports = set() + + try: + ids = task_info['ids'] + except KeyError: + raise ValidationError("Missing ids") + + for resource_id in ids: + if resource_id not in exports: + raise ValidationError("id {id} is not in export list".format( + id=resource_id + )) + + # Cancel any old tasks of same type and repo, remove from list. + other_tasks = {} + for task_id, data in session.get(TASK_KEY, {}).items(): + if data['task_type'] == task_type and \ + data['task_info']['repo_slug'] == repo_slug: + AsyncResult(task_id).revoke() + else: + other_tasks[task_id] = data + session[TASK_KEY] = other_tasks + + learning_resources = LearningResource.objects.filter(id__in=ids).all() + user = User.objects.get(id=user_id) + result = export_resources.delay(learning_resources, user.username) + + # Put new task in session. + initial_data = create_initial_task_dict( + result, task_type, task_info) + session[TASK_KEY][result.id] = initial_data + session.modified = True + + return initial_data + else: + raise ValidationError("Unknown task_type {task_type}".format( + task_type=task_type + )) diff --git a/rest/tests/base.py b/rest/tests/base.py index be925c6e..517e7538 100644 --- a/rest/tests/base.py +++ b/rest/tests/base.py @@ -772,36 +772,23 @@ def create_learning_resource_export(self, repo_slug, input_dict, ), resp['Location']) return result_dict - def get_learning_resource_export_tasks(self, repo_slug, - expected_status=HTTP_200_OK): + def get_tasks(self, expected_status=HTTP_200_OK): """ - Helper method to retrieve export tasks for the user. Should be only - one. + Helper method to retrieve tasks for the user. """ - url = ( - '/api/v1/repositories/{repo_slug}/' - 'learning_resource_export_tasks/'.format( - repo_slug=repo_slug, - ) - ) + url = '/api/v1/tasks/' self.assert_options_head(url, expected_status) resp = self.client.get(url) self.assertEqual(expected_status, resp.status_code) if expected_status == HTTP_200_OK: - result = as_json(resp) - # Currently we only allow one task at a time. - self.assertLessEqual(result['count'], 1) - return result + return as_json(resp) - def get_learning_resource_export_task(self, repo_slug, task_id, - expected_status=HTTP_200_OK): + def get_task(self, task_id, expected_status=HTTP_200_OK): """ - Helper method to retrieve export task for the user. + Helper method to retrieve task for the user. """ resp = self.client.get( - '/api/v1/repositories/{repo_slug}/' - 'learning_resource_export_tasks/{task_id}/'.format( - repo_slug=repo_slug, + '/api/v1/tasks/{task_id}/'.format( task_id=task_id, ) ) @@ -809,17 +796,13 @@ def get_learning_resource_export_task(self, repo_slug, task_id, if expected_status == HTTP_200_OK: return as_json(resp) - def create_learning_resource_export_task(self, repo_slug, input_dict, - expected_status=HTTP_201_CREATED): + def create_task(self, input_dict, expected_status=HTTP_201_CREATED): """ Helper method to create a task for the user to export a tarball of LearningResources. """ resp = self.client.post( - '/api/v1/repositories/{repo_slug}/' - 'learning_resource_export_tasks/'.format( - repo_slug=repo_slug, - ), + '/api/v1/tasks/', json.dumps(input_dict), content_type='application/json', ) diff --git a/rest/tests/test_export.py b/rest/tests/test_export.py deleted file mode 100644 index f763817a..00000000 --- a/rest/tests/test_export.py +++ /dev/null @@ -1,89 +0,0 @@ -""" -Tests for export -""" - -from __future__ import unicode_literals -from tempfile import mkdtemp -from shutil import rmtree -import tarfile -import os - -from django.utils.text import slugify -from django.test import override_settings -from rest_framework.status import HTTP_200_OK, HTTP_404_NOT_FOUND -from six import BytesIO -from six.moves import reload_module # pylint: disable=import-error - -import ui.urls -from rest.tests.base import ( - RESTTestCase -) -from learningresources.models import LearningResource -from exporter.tests.test_export import assert_resource_directory - - -class TestExport(RESTTestCase): - """Test for export.""" - - @override_settings( - DEFAULT_FILE_STORAGE='storages.backends.overwrite.OverwriteStorage' - ) - def test_create_new_task(self): - """Test a basic export.""" - self.import_course_tarball(self.repo) - resources = LearningResource.objects.filter( - course__repository__id=self.repo.id).all() - for resource in resources: - self.create_learning_resource_export(self.repo.slug, { - "id": resource.id - }) - - # Skip first one to test that it's excluded from export. - task_id = self.create_learning_resource_export_task( - self.repo.slug, - {"ids": [r.id for r in resources[1:]]} - )['id'] - - result = self.get_learning_resource_export_tasks( - self.repo.slug)['results'][0] - self.assertEqual(task_id, result['id']) - self.assertEqual("success", result['status']) - self.assertTrue(result['url'].startswith( - "/media/resource_exports/test_exports.tar")) - - with self.settings( - DEFAULT_FILE_STORAGE='storages.backends.s3boto.S3BotoStorage' - ): - # change the default file storage to S3 - reload_module(ui.urls) - # the view is not available any more - resp = self.client.get(result['url']) - self.assertEqual(resp.status_code, HTTP_404_NOT_FOUND) - - # Update for change in file storage. - reload_module(ui.urls) - - resp = self.client.get(result['url']) - self.assertEqual(HTTP_200_OK, resp.status_code) - - tempdir = mkdtemp() - - def make_path(resource): - """Create a path that should exist for a resource.""" - type_name = resource.learning_resource_type.name - return os.path.join( - tempdir, type_name, "{id}_{url_name}.xml".format( - id=resource.id, - url_name=slugify(resource.url_name)[:200], - ) - ) - try: - fakefile = BytesIO(b"".join(resp.streaming_content)) - with tarfile.open(fileobj=fakefile, mode="r:gz") as tar: - tar.extractall(path=tempdir) - - self.assertFalse(os.path.isfile(make_path(resources[0]))) - assert_resource_directory(self, resources[1:], tempdir) - - finally: - rmtree(tempdir) diff --git a/rest/tests/test_tasks.py b/rest/tests/test_tasks.py new file mode 100644 index 00000000..86cad8a6 --- /dev/null +++ b/rest/tests/test_tasks.py @@ -0,0 +1,221 @@ +""" +Tests for export +""" + +from __future__ import unicode_literals +from tempfile import mkdtemp +from shutil import rmtree +import tarfile +import os +import json + +from django.utils.text import slugify +from django.test import override_settings, Client +from rest_framework.status import ( + HTTP_200_OK, + HTTP_400_BAD_REQUEST, + HTTP_403_FORBIDDEN, + HTTP_404_NOT_FOUND, +) +from six import BytesIO +from six.moves import reload_module # pylint: disable=import-error + +import ui.urls +from rest.tests.base import ( + RESTTestCase, + API_BASE, + as_json +) +from rest.tasks import EXPORT_TASK_TYPE +from learningresources.models import LearningResource +from exporter.tests.test_export import assert_resource_directory + + +class TestTasks(RESTTestCase): + """Tests for tasks.""" + + @override_settings( + DEFAULT_FILE_STORAGE='storages.backends.overwrite.OverwriteStorage' + ) + def test_create_export_task(self): + """Test a basic export.""" + self.import_course_tarball(self.repo) + resources = LearningResource.objects.filter( + course__repository__id=self.repo.id).all() + for resource in resources: + self.create_learning_resource_export(self.repo.slug, { + "id": resource.id + }) + + # Skip first one to test that it's excluded from export. + resource_ids = [r.id for r in resources[1:]] + + # Missing task_info. + self.create_task( + { + "ids": resource_ids, + "task_type": EXPORT_TASK_TYPE + }, + expected_status=HTTP_400_BAD_REQUEST + ) + + # Missing repo slug. + self.create_task( + { + "ids": resource_ids, + "task_type": EXPORT_TASK_TYPE, + "task_info": {} + }, + expected_status=HTTP_400_BAD_REQUEST + ) + + # Missing ids. + self.create_task( + { + "task_type": EXPORT_TASK_TYPE, + "task_info": { + "repo_slug": self.repo.slug, + }, + }, + expected_status=HTTP_400_BAD_REQUEST + ) + + # Missing task_type. + self.create_task( + { + "ids": resource_ids, + "task_info": { + "repo_slug": self.repo.slug, + }, + }, + expected_status=HTTP_400_BAD_REQUEST + ) + + # Invalid task type. + self.create_task( + { + "ids": resource_ids, + "task_type": "missing", + "task_info": { + "repo_slug": self.repo.slug, + }, + }, + expected_status=HTTP_400_BAD_REQUEST + ) + + # Invalid repo. + self.create_task( + { + "ids": resource_ids, + "task_type": EXPORT_TASK_TYPE, + "task_info": { + "repo_slug": "missing", + }, + }, + expected_status=HTTP_404_NOT_FOUND + ) + + # User doesn't own repo. + client2 = Client() + client2.login( + username=self.user_norepo.username, + password=self.PASSWORD + ) + resp = client2.post( + "{base}tasks/".format(base=API_BASE), + json.dumps({ + "ids": resource_ids, + "task_type": EXPORT_TASK_TYPE, + "task_info": { + "repo_slug": self.repo.slug + } + }), + content_type='application/json' + ) + self.assertEqual(resp.status_code, HTTP_403_FORBIDDEN) + + # Start export task. Due to CELERY_ALWAYS_EAGER setting this will + # block until it completes in testing. + task_id = self.create_task( + { + "task_info": { + "repo_slug": self.repo.slug, + # Skip first one to test that it's excluded from export. + "ids": [r.id for r in resources[1:]], + }, + "task_type": EXPORT_TASK_TYPE + } + )['id'] + + # Before we move on, confirm that other user can't see these tasks. + resp = client2.get("{base}tasks/".format(base=API_BASE)) + self.assertEqual(resp.status_code, HTTP_200_OK) + self.assertEqual(as_json(resp)['count'], 0) + + result = self.get_tasks()['results'][0] + self.assertEqual(task_id, result['id']) + self.assertEqual("success", result['status']) + self.assertEqual(result['task_type'], EXPORT_TASK_TYPE) + self.assertTrue(result['result']['url'].startswith( + "/media/resource_exports/test_exports.tar")) + # webGLDemo.css shows up twice + self.assertTrue(result['result']['collision']) + + with self.settings( + DEFAULT_FILE_STORAGE='storages.backends.s3boto.S3BotoStorage' + ): + # change the default file storage to S3 + reload_module(ui.urls) + # the view is not available any more + resp = self.client.get(result['result']['url']) + self.assertEqual(resp.status_code, HTTP_404_NOT_FOUND) + + # Update for change in file storage. + reload_module(ui.urls) + + resp = self.client.get(result['result']['url']) + self.assertEqual(HTTP_200_OK, resp.status_code) + + tempdir = mkdtemp() + + def make_path(resource): + """Create a path that should exist for a resource.""" + type_name = resource.learning_resource_type.name + return os.path.join( + tempdir, type_name, "{id}_{url_name}.xml".format( + id=resource.id, + url_name=slugify(resource.url_name)[:200], + ) + ) + try: + fakefile = BytesIO(b"".join(resp.streaming_content)) + with tarfile.open(fileobj=fakefile, mode="r:gz") as tar: + tar.extractall(path=tempdir) + + self.assertFalse(os.path.isfile(make_path(resources[0]))) + assert_resource_directory(self, resources[1:], tempdir) + + finally: + rmtree(tempdir) + + def test_tasks_in_session(self): + """ + Test that the task list clears after logout. + """ + self.assertEqual(self.get_tasks()['count'], 0) + self.create_task( + { + "task_info": { + "repo_slug": self.repo.slug, + # No ids to export, but task should still run. + "ids": [], + }, + "task_type": EXPORT_TASK_TYPE + } + ) + self.assertEqual(self.get_tasks()['count'], 1) + + # Since tasks are stored in session currently they are wiped out. + self.logout() + self.login(self.user.username) + self.assertEqual(self.get_tasks()['count'], 0) diff --git a/rest/urls.py b/rest/urls.py index 311155f4..8b609e67 100644 --- a/rest/urls.py +++ b/rest/urls.py @@ -12,8 +12,8 @@ LearningResourceDetail, LearningResourceExportDetail, LearningResourceExportList, - LearningResourceExportTaskDetail, - LearningResourceExportTaskList, + TaskDetail, + TaskList, LearningResourceList, LearningResourceTypeList, RepoMemberGroupList, @@ -40,9 +40,7 @@ REPOSITORY_EXPORTS_URL = ( r'^repositories/(?P[-\w]+)/learning_resource_exports/' ) -REPOSITORY_EXPORT_TASK_URL = ( - r'^repositories/(?P[-\w]+)/learning_resource_export_tasks/' -) +TASK_URL = r'^tasks/' REPOSITORY_SEARCH_URL = r'^repositories/(?P[-\w]+)/search/' urlpatterns = [ @@ -122,11 +120,11 @@ url(REPOSITORY_EXPORTS_URL + r'(?P[-\w]+)/(?P\d+)/$', LearningResourceExportDetail.as_view(), name='learning-resource-export-detail'), - url(REPOSITORY_EXPORT_TASK_URL + r'$', - LearningResourceExportTaskList.as_view(), + url(TASK_URL + r'$', + TaskList.as_view(), name='learning-resource-export-task-list'), - url(REPOSITORY_EXPORT_TASK_URL + r'(?P[-\w]+)/$', - LearningResourceExportTaskDetail.as_view(), + url(TASK_URL + r'(?P[-\w]+)/$', + TaskDetail.as_view(), name='learning-resource-export-task-list'), url("^learning_resource_types/$", LearningResourceTypeList.as_view(), name='learning-resource-type-list'), diff --git a/rest/views.py b/rest/views.py index 454dc3ce..b0832da3 100644 --- a/rest/views.py +++ b/rest/views.py @@ -6,8 +6,7 @@ from django.http.response import Http404 from django.contrib.auth.models import User -from django.core.exceptions import PermissionDenied -from django.core.files.storage import default_storage +from django.core.exceptions import PermissionDenied as DjangoPermissionDenied from django.db import transaction from rest_framework import status from rest_framework.exceptions import ValidationError @@ -23,11 +22,12 @@ from rest_framework.reverse import reverse from rest_framework.permissions import IsAuthenticated from rest_framework.viewsets import GenericViewSet -from celery.states import FAILURE, SUCCESS, REVOKED -from celery.result import AsyncResult from statsd.defaults.django import statsd -from exporter.tasks import export_resources +from learningresources.api import ( + PermissionDenied, + NotFound, +) from roles.permissions import GroupTypes, BaseGroupTypes from roles.api import ( assign_user_to_repo_group, @@ -39,7 +39,7 @@ CourseSerializer, GroupSerializer, LearningResourceExportSerializer, - LearningResourceExportTaskSerializer, + TaskSerializer, LearningResourceSerializer, LearningResourceTypeSerializer, RepositorySearchSerializer, @@ -63,6 +63,13 @@ ViewTermPermission, ViewVocabularyPermission, ) +from rest.tasks import ( + create_task, + create_task_result_dict, + get_task, + get_tasks, + EXPORTS_KEY, +) from rest.util import CheckValidMemberParamMixin from search.api import construct_queryset from search.tasks import index_resources @@ -79,9 +86,6 @@ get_resource, ) -EXPORTS_KEY = 'learning_resource_exports' -EXPORT_TASK_KEY = 'learning_resource_export_tasks' - class RepositoryList(ListCreateAPIView): """REST list view for Repository.""" @@ -655,7 +659,7 @@ def create(self, request, *args, **kwargs): learning_resource = get_resource(lr_id, self.request.user.id) if learning_resource.course.repository.slug != repo_slug: - raise PermissionDenied() + raise DjangoPermissionDenied() if EXPORTS_KEY not in self.request.session: self.request.session[EXPORTS_KEY] = {} @@ -741,139 +745,58 @@ def perform_destroy(self, instance): raise Http404 -def create_task_result_dict(task): - """ - Convert initial data we put in session to dict for REST API. - This will use the id to look up current data about task to return - to user. - - Args: - task (dict): Initial data about task stored in session. - Returns: - dict: Current data about task. - """ - initial_state = task['initial_state'] - task_id = task['id'] - - state = "processing" - url = "" - collision = False - # initial_state is a workaround for EagerResult used in testing. - # In production initial_state should usually be pending. - if initial_state == SUCCESS: - state = "success" - url = task['url'] - elif initial_state in (FAILURE, REVOKED): - state = "failure" - else: - result = AsyncResult(task_id) - - if result.successful(): - state = "success" - elif result.failed(): - state = "failure" - - if result.successful(): - name, collision = result.get() - url = default_storage.url(name) - - return { - "id": task_id, - "status": state, - "url": url, - "collision": collision - } - - -class LearningResourceExportTaskList(ListCreateAPIView): +class TaskList(ListCreateAPIView): """ View for export tasks for a user. """ - serializer_class = LearningResourceExportTaskSerializer + serializer_class = TaskSerializer permission_classes = ( - ViewRepoPermission, IsAuthenticated, ) def get_queryset(self): """Get tasks for this user.""" - repo_slug = self.kwargs['repo_slug'] - try: - tasks = self.request.session[EXPORT_TASK_KEY][repo_slug] - except KeyError: - tasks = {} + tasks = get_tasks(self.request.session) return [create_task_result_dict(task) for task in tasks.values()] def post(self, request, *args, **kwargs): """ - Create a new export task for this user. + Create a new task for this user. Note that this also cancels and clears any old tasks for the user, so there should be only one task in the list at any time. - - The export list will be left alone. Once the list is exported - the client may DELETE the export list as a separate REST call. """ - repo_slug = self.kwargs['repo_slug'] + if 'task_type' not in self.request.data: + raise ValidationError("Missing task_type.") + if 'task_info' not in self.request.data: + raise ValidationError("Missing task_info.") + try: - exports = set(self.request.session[EXPORTS_KEY][repo_slug]) - except KeyError: - exports = set() - - ids = self.request.data['ids'] - for resource_id in ids: - if resource_id not in exports: - raise ValidationError("id {id} is not in export list".format( - id=resource_id - )) - - # Cancel any old tasks. - old_tasks = self.request.session.get( - EXPORT_TASK_KEY, {}).get(repo_slug, {}) - for task_id in old_tasks.keys(): - AsyncResult(task_id).revoke() - - # Clear task list. - if EXPORT_TASK_KEY not in self.request.session: - self.request.session[EXPORT_TASK_KEY] = {} - self.request.session[EXPORT_TASK_KEY][repo_slug] = {} - - learning_resources = LearningResource.objects.filter( - id__in=ids).all() - result = export_resources.delay( - learning_resources, self.request.user.username) - - if result.successful(): - name, collision = result.get() - url = default_storage.url(name) - else: - collision = False - url = "" - - # Put new task in session. - self.request.session[EXPORT_TASK_KEY][repo_slug][result.id] = { - "id": result.id, - "initial_state": result.state, - "url": url, - "collision": collision - } - self.request.session.modified = True + result = create_task( + self.request.session, + self.request.user.id, + self.request.data['task_type'], + self.request.data['task_info'] + ) + except PermissionDenied: + raise DjangoPermissionDenied + except NotFound: + raise Http404 return Response( - {"id": result.id}, + {"id": result['id']}, status=status.HTTP_201_CREATED ) -class LearningResourceExportTaskDetail(RetrieveAPIView): +class TaskDetail(RetrieveAPIView): """ Detail view for a user's export tasks. """ - serializer_class = LearningResourceExportTaskSerializer + serializer_class = TaskSerializer permission_classes = ( - ViewRepoPermission, IsAuthenticated, ) @@ -881,15 +804,9 @@ def get_object(self): """ Retrieve current information about an export task. """ - try: - task_id = self.kwargs['task_id'] - except ValueError: - raise Http404 - - try: - initial_dict = self.request.session[ - EXPORT_TASK_KEY][self.kwargs['repo_slug']][task_id] - except KeyError: + task_id = self.kwargs['task_id'] + initial_dict = get_task(self.request.session, task_id) + if initial_dict is None: raise Http404 return create_task_result_dict(initial_dict) diff --git a/ui/jstests/test_lr_exports.jsx b/ui/jstests/test_lr_exports.jsx index 61ebbe1a..224e5ff7 100644 --- a/ui/jstests/test_lr_exports.jsx +++ b/ui/jstests/test_lr_exports.jsx @@ -25,6 +25,10 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', QUnit.module('Test exports panel', { beforeEach: function () { TestUtils.setup(); + + // The mocked AJAX omits task_info and task_type but currently they aren't + // used in the javascript. + TestUtils.initMockjax({ url: '/api/v1/repositories/repo/learning_resource_exports/user/', type: 'GET', @@ -48,7 +52,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', } }); TestUtils.initMockjax({ - url: '/api/v1/repositories/repo/learning_resource_export_tasks/', + url: '/api/v1/tasks/', type: 'GET', responseText: { "count": 1, @@ -58,27 +62,28 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', { "id": "task1", "status": "successful", - "url": "/media/resource_exports/export_task1.tar.gz", - "collision": false + "result": { + "url": "/media/resource_exports/export_task1.tar.gz", + "collision": false + } } ] } }); TestUtils.initMockjax({ - url: '/api/v1/repositories/repo/learning_resource_export_tasks/', + url: '/api/v1/tasks/', type: 'POST', responseText: { "id": "task2" } }); TestUtils.initMockjax({ - url: '/api/v1/repositories/repo/learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "processing", - "url": "", - "collision": false + "result": null } }); TestUtils.initMockjax({ @@ -147,14 +152,12 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "processing", - "url": "", - "collision": false + "result": null } }); @@ -171,14 +174,15 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "success", - "url": "/media/resource_exports/export_task2.tar.gz", - "collision": false + "result": { + "url": "/media/resource_exports/export_task2.tar.gz", + "collision": false + } } }); @@ -300,8 +304,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', // The task fails! TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/', + url: '/api/v1/tasks/', type: 'POST', status: 400 }); @@ -369,8 +372,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', var button = $node.find("button")[0]; TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', status: 400 }); @@ -455,8 +457,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', status: 400 }); @@ -541,14 +542,12 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "processing", - "url": "", - "collision": false + "result": null } }); @@ -566,14 +565,12 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "failure", - "url": "", - "collision": false + "result": null } }); @@ -655,14 +652,12 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "processing", - "url": "", - "collision": false + "result": null } }); @@ -680,8 +675,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', status: 500 }); @@ -768,14 +762,15 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', ); TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/task2/', + url: '/api/v1/tasks/task2/', type: 'GET', responseText: { "id": "task2", "status": "success", - "url": "/media/resource_exports/export_task2.tar.gz", - "collision": true + "result": { + "url": "/media/resource_exports/export_task2.tar.gz", + "collision": true + } } }); TestUtils.replaceMockjax({ @@ -867,14 +862,17 @@ define(['QUnit', 'jquery', 'lr_exports', 'react', // This matches against an empty set of ids which verifies that // exportsSelected affects what's sent to the server. TestUtils.replaceMockjax({ - url: '/api/v1/repositories/repo/' + - 'learning_resource_export_tasks/', + url: '/api/v1/tasks/', type: 'POST', responseText: { id: "task2" }, data: JSON.stringify({ - ids: [] + task_type: "resource_export", + task_info: { + ids: [], + repo_slug: "repo" + } }) }); diff --git a/ui/static/ui/js/lr_exports.jsx b/ui/static/ui/js/lr_exports.jsx index fefbf0f8..9e316227 100644 --- a/ui/static/ui/js/lr_exports.jsx +++ b/ui/static/ui/js/lr_exports.jsx @@ -145,9 +145,14 @@ define("lr_exports", $.ajax({ type: "POST", - url: "/api/v1/repositories/" + this.props.repoSlug + - "/learning_resource_export_tasks/", - data: JSON.stringify({ids: this.state.exportsSelected}), + url: "/api/v1/tasks/", + data: JSON.stringify({ + task_type: "resource_export", + task_info: { + ids: this.state.exportsSelected, + repo_slug: this.props.repoSlug + } + }), contentType: 'application/json' } ).then(function(result) { @@ -175,8 +180,7 @@ define("lr_exports", updateStatus: function(taskId) { var thiz = this; - $.get("/api/v1/repositories/" + this.props.repoSlug + - "/learning_resource_export_tasks/" + taskId + "/") + $.get("/api/v1/tasks/" + taskId + "/") .done(function (result) { if (!thiz.isMounted()) { return; @@ -206,8 +210,8 @@ define("lr_exports", }); thiz.setState({ - url: result.url, - collision: result.collision + url: result.result.url, + collision: result.result.collision }); } else if (result.status === "processing") { setTimeout(function() {