diff --git a/apiary.apib b/apiary.apib index 211e7359..d0bd3e7c 100644 --- a/apiary.apib +++ b/apiary.apib @@ -89,6 +89,86 @@ Page numbering is 1-based. Omitting the `?page` parameter will return the first ] } +## Group Tasks + +## Tasks Collection [/tasks/] + +### List All Tasks [GET] + +List all recent tasks for a user. + ++ Response 200 (application/json) + + { + "count": 1, + "next": null, + "previous": null, + "results": [ + { + "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3", + "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 + ] + } + } + ] + } + +### Create a New Task [POST] + +Queue a new task. + ++ Request (application/json) + + { + "task_info": { + "repo_slug": "repo", + "ids": [1] + }, + "task_type": "resource_export" + } + ++ Response 200 (application/json) + + { + "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3" + } + +## Task [/tasks/{task_id}/] + ++ Parameters + + task_id: `45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3` (string, required) - task identifier + +### Retrieve a Task [GET] + ++ Response 200 (application/json) + + { + "id": "45e3c830-0ff8-4d84-85b4-c0a6e3ce81b3", + "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 Repositories ## Repository Collection [/repositories/] @@ -477,6 +557,8 @@ Clears the export list for this repository. ## LearningResourceExportTasks Collection [/repositories/{repo_slug}/learning_resource_export_tasks/] +Deprecated. Use /api/v1/tasks/ instead. + + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository @@ -515,7 +597,9 @@ 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}/ +## LearningResourceExportTask [/repositories/{repo_slug}/learning_resource_export_tasks/{task_id}/] + +Deprecated. Use /api/v1/tasks/ instead. + Parameters + repo_slug: `physics-1` (string, required) - slug for the repository 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..fadfb03a 100644 --- a/rest/serializers.py +++ b/rest/serializers.py @@ -21,6 +21,8 @@ SerializerMethodField, IntegerField, FloatField, + DictField, + BooleanField, ) from rest.util import LambdaDefault, RequiredBooleanField @@ -308,6 +310,16 @@ class LearningResourceExportTaskSerializer(Serializer): id = CharField() status = CharField() url = CharField() + collision = BooleanField() + + +class TaskSerializer(Serializer): + """Serializer for tasks.""" + id = CharField() + status = 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..88bccdad --- /dev/null +++ b/rest/tasks.py @@ -0,0 +1,211 @@ +""" +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 django.http.response import Http404 +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() + elif task.failed(): + result = {'error': str(task.result)} + + 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. + async_result = AsyncResult(task_id) + + if initial_state == SUCCESS: + state = "success" + result = initial_data['result'] + elif initial_state in (FAILURE, REVOKED): + state = "failure" + result = initial_data['result'] + elif async_result.successful(): + state = "success" + result = async_result.get() + elif async_result.failed(): + state = "failure" + result = {'error': str(async_result.result)} + + 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 track_task(session, task, task_type, task_info): + """ + Add a Celery task to the session. + + Args: + session (SessionStore): The request session. + 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). + """ + initial_data = create_initial_task_dict(task, task_type, task_info) + if TASK_KEY not in session: + session[TASK_KEY] = {} + session[TASK_KEY][task.id] = initial_data + session.modified = True + return initial_data + + +def create_task(session, user_id, task_type, task_info): + """ + Start a new Celery task from REST API. + + 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 + )) + + 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 = track_task(session, result, task_type, task_info) + + return initial_data + else: + raise ValidationError("Unknown task_type {task_type}".format( + task_type=task_type + )) + + +def remove_task(session, task_id): + """ + Cancel task and remove task from task list. + + Args: + session (SessionStore): The request session. + task_id (int): The task id. + """ + tasks = session.get(TASK_KEY, {}) + if task_id not in tasks: + raise Http404 + + AsyncResult(task_id).revoke() + del tasks[task_id] + session[TASK_KEY] = tasks diff --git a/rest/tests/base.py b/rest/tests/base.py index be925c6e..c0e04a86 100644 --- a/rest/tests/base.py +++ b/rest/tests/base.py @@ -772,11 +772,58 @@ def create_learning_resource_export(self, repo_slug, input_dict, ), resp['Location']) return result_dict + def get_tasks(self, expected_status=HTTP_200_OK): + """ + Helper method to retrieve tasks for the user. + """ + 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: + return as_json(resp) + + def get_task(self, task_id, expected_status=HTTP_200_OK): + """ + Helper method to retrieve task for the user. + """ + resp = self.client.get( + '/api/v1/tasks/{task_id}/'.format( + task_id=task_id, + ) + ) + self.assertEqual(expected_status, resp.status_code) + if expected_status == HTTP_200_OK: + return as_json(resp) + + 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/tasks/', + json.dumps(input_dict), + content_type='application/json', + ) + self.assertEqual(expected_status, resp.status_code) + if expected_status == HTTP_201_CREATED: + return as_json(resp) + + def delete_task(self, task_id, expected_status=HTTP_204_NO_CONTENT): + """ + Delete a task and remove it from task list. + """ + resp = self.client.delete('/api/v1/tasks/{task_id}/'.format( + task_id=task_id + )) + self.assertEqual(expected_status, resp.status_code) + def get_learning_resource_export_tasks(self, repo_slug, expected_status=HTTP_200_OK): """ - Helper method to retrieve export tasks for the user. Should be only - one. + Helper method to retrieve export tasks for the user using deprecated + API. Should be only one. """ url = ( '/api/v1/repositories/{repo_slug}/' @@ -796,7 +843,8 @@ def get_learning_resource_export_tasks(self, repo_slug, def get_learning_resource_export_task(self, repo_slug, task_id, expected_status=HTTP_200_OK): """ - Helper method to retrieve export task for the user. + Helper method to retrieve export task for the user using deprecated + API. """ resp = self.client.get( '/api/v1/repositories/{repo_slug}/' 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..4d048667 --- /dev/null +++ b/rest/tests/test_tasks.py @@ -0,0 +1,349 @@ +""" +Tests for tasks API. +""" + +from __future__ import unicode_literals +from tempfile import mkdtemp +from shutil import rmtree +import tarfile +import os +import json +import mock + +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) + + def test_remove_task(self): + """ + Test that the task is removed from list properly. + """ + task_id = 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 + } + )['id'] + self.assertEqual(self.get_task(task_id)['status'], 'success') + + # Other users can't stop tasks they don't own. + client2 = Client() + client2.login( + username=self.user_norepo.username, + password=self.PASSWORD + ) + resp = client2.delete("{base}tasks/{task_id}/".format( + base=API_BASE, + task_id=task_id + )) + self.assertEqual(resp.status_code, HTTP_404_NOT_FOUND) + + self.delete_task(task_id) + self.get_task(task_id, expected_status=HTTP_404_NOT_FOUND) + self.assertEqual(self.get_tasks()['count'], 0) + + @override_settings( + CELERY_EAGER_PROPAGATES_EXCEPTIONS=False + ) + def test_failure(self): + """ + Test that the task exception text is returned in the result. + """ + + with mock.patch('exporter.tasks.export_resources_to_tarball') as imp: + imp.side_effect = Exception("Failure") + task_id = 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 + } + )['id'] + result = self.get_task(task_id) + self.assertEqual(result['status'], 'failure') + self.assertEqual(result['result'], {'error': 'Failure'}) + + @override_settings( + DEFAULT_FILE_STORAGE='storages.backends.overwrite.OverwriteStorage' + ) + def test_old_export_tasks_api(self): + """ + Test deprecated export tasks API. + """ + 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")) + + # Test single task API. + self.assertEqual( + self.get_learning_resource_export_task(self.repo.slug, task_id), + result + ) + + 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/urls.py b/rest/urls.py index 311155f4..9367e739 100644 --- a/rest/urls.py +++ b/rest/urls.py @@ -14,6 +14,8 @@ LearningResourceExportList, LearningResourceExportTaskDetail, LearningResourceExportTaskList, + TaskDetail, + TaskList, LearningResourceList, LearningResourceTypeList, RepoMemberGroupList, @@ -43,6 +45,7 @@ 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 = [ @@ -127,7 +130,13 @@ name='learning-resource-export-task-list'), url(REPOSITORY_EXPORT_TASK_URL + r'(?P[-\w]+)/$', LearningResourceExportTaskDetail.as_view(), - name='learning-resource-export-task-list'), + name='learning-resource-export-task-detail'), + url(TASK_URL + r'$', + TaskList.as_view(), + name='task-list'), + url(TASK_URL + r'(?P[-\w]+)/$', + TaskDetail.as_view(), + name='task-detail'), url("^learning_resource_types/$", LearningResourceTypeList.as_view(), name='learning-resource-type-list'), url(REPOSITORY_SEARCH_URL, RepositorySearchList.as_view({'get': 'list'}), diff --git a/rest/views.py b/rest/views.py index 454dc3ce..346e6ceb 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, @@ -40,6 +40,7 @@ GroupSerializer, LearningResourceExportSerializer, LearningResourceExportTaskSerializer, + TaskSerializer, LearningResourceSerializer, LearningResourceTypeSerializer, RepositorySearchSerializer, @@ -63,6 +64,15 @@ ViewTermPermission, ViewVocabularyPermission, ) +from rest.tasks import ( + create_task, + create_task_result_dict, + get_task, + get_tasks, + remove_task, + EXPORTS_KEY, + EXPORT_TASK_TYPE, +) from rest.util import CheckValidMemberParamMixin from search.api import construct_queryset from search.tasks import index_resources @@ -79,10 +89,8 @@ get_resource, ) -EXPORTS_KEY = 'learning_resource_exports' -EXPORT_TASK_KEY = 'learning_resource_export_tasks' - +# pylint: disable=too-many-lines class RepositoryList(ListCreateAPIView): """REST list view for Repository.""" serializer_class = RepositorySerializer @@ -655,7 +663,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,53 +749,9 @@ 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): """ - View for export tasks for a user. + View for export tasks for a user. Deprecated! Use /api/v1/tasks/ instead. """ serializer_class = LearningResourceExportTaskSerializer permission_classes = ( @@ -796,14 +760,24 @@ class LearningResourceExportTaskList(ListCreateAPIView): ) def get_queryset(self): - """Get tasks for this user.""" + """Get export tasks for this user.""" repo_slug = self.kwargs['repo_slug'] - try: - tasks = self.request.session[EXPORT_TASK_KEY][repo_slug] - except KeyError: - tasks = {} - - return [create_task_result_dict(task) for task in tasks.values()] + export_tasks = [ + task for task in get_tasks(self.request.session).values() + if task['task_type'] == EXPORT_TASK_TYPE and + task['task_info']['repo_slug'] == repo_slug + ] + + task_results = [create_task_result_dict(task) for task in export_tasks] + + return [ + { + "id": result['id'], + "status": result['status'], + "url": result['result']['url'], + "collision": result['result']['collision'] + } for result in task_results + ] def post(self, request, *args, **kwargs): """ @@ -815,54 +789,34 @@ def post(self, request, *args, **kwargs): 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'] - 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 - )) + export_tasks = [ + task for task in get_tasks(self.request.session).values() + if task['task_type'] == EXPORT_TASK_TYPE and + task['task_info']['repo_slug'] == repo_slug + ] # 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 + for task in export_tasks: + remove_task(self.request.session, task['id']) + + try: + ids = self.request.data['ids'] + except KeyError: + raise ValidationError("Missing ids") + + task = create_task( + self.request.session, + self.request.user.id, + EXPORT_TASK_TYPE, + { + 'repo_slug': repo_slug, + 'ids': ids + } + ) return Response( - {"id": result.id}, + {"id": task['id']}, status=status.HTTP_201_CREATED ) @@ -881,19 +835,94 @@ def get_object(self): """ Retrieve current information about an export task. """ - try: - task_id = self.kwargs['task_id'] - except ValueError: + task_id = self.kwargs['task_id'] + initial_dict = get_task(self.request.session, task_id) + if initial_dict is None: raise Http404 + result = create_task_result_dict(initial_dict) + return { + "id": result['id'], + "status": result['status'], + "url": result['result']['url'], + "collision": result['result']['collision'] + } + + +class TaskList(ListCreateAPIView): + """ + View for tasks for a user. + """ + serializer_class = TaskSerializer + permission_classes = ( + IsAuthenticated, + ) + + def get_queryset(self): + """Get tasks for this user.""" + 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 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. + """ + + 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: - initial_dict = self.request.session[ - EXPORT_TASK_KEY][self.kwargs['repo_slug']][task_id] - except KeyError: + 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']}, + status=status.HTTP_201_CREATED + ) + + +class TaskDetail(RetrieveDestroyAPIView): + """ + Detail view for a user's tasks. + """ + serializer_class = TaskSerializer + permission_classes = ( + IsAuthenticated, + ) + + def get_object(self): + """ + Retrieve current information about a task. + """ + 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) + def delete(self, request, *args, **kwargs): + """ + Remove task from list, revoke if necessary. + """ + task_id = self.kwargs['task_id'] + remove_task(self.request.session, task_id) + return Response(status=status.HTTP_204_NO_CONTENT) + def calculate_selected_facets(selected_facet_params, facet_counts): """ 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() {