From fa856d333882bc3af953aecf9271f8517535b58c Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Mon, 6 Jan 2014 13:32:32 -0500 Subject: [PATCH 1/2] Logging to catch duplicate instructor tasks --- lms/djangoapps/instructor_task/api_helper.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 37a9852caa..606907cdae 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -52,8 +52,21 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester): """ if _task_is_running(course_id, task_type, task_key): + log.warning("Duplicate task found for task_type %s and task_key %s", task_type, task_key) raise AlreadyRunningError("requested task is already running") + try: + most_recent_id = InstructorTask.objects.latest('id').id + except InstructorTask.DoesNotExist: + most_recent_id = "None found" + finally: + log.warning( + "No duplicate tasks found: task_type %s, task_key %s, and most recent task_id = %s", + task_type, + task_key, + most_recent_id + ) + # Create log entry now, so that future requests will know it's running. return InstructorTask.create(course_id, task_type, task_key, task_input, requester) From 8a0f785cbc90e257cbf2380d8e13a0f292f772fb Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Mon, 6 Jan 2014 18:00:32 -0500 Subject: [PATCH 2/2] Fix STUD-1129: handle out-of-range page numbers --- .../contentstore/tests/test_assets.py | 5 +++ cms/djangoapps/contentstore/views/assets.py | 33 ++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_assets.py b/cms/djangoapps/contentstore/tests/test_assets.py index 59c1cb068b..4bc14ad4a3 100644 --- a/cms/djangoapps/contentstore/tests/test_assets.py +++ b/cms/djangoapps/contentstore/tests/test_assets.py @@ -60,6 +60,11 @@ class AssetsToyCourseTestCase(CourseTestCase): self.assert_correct_asset_response(url + "?page_size=2", 0, 2, 3) self.assert_correct_asset_response(url + "?page_size=2&page=1", 2, 1, 3) + # Verify querying outside the range of valid pages + self.assert_correct_asset_response(url + "?page_size=2&page=-1", 0, 2, 3) + self.assert_correct_asset_response(url + "?page_size=2&page=2", 2, 1, 3) + self.assert_correct_asset_response(url + "?page_size=3&page=1", 0, 3, 3) + def assert_correct_asset_response(self, url, expected_start, expected_length, expected_total): resp = self.client.get(url, HTTP_ACCEPT='application/json') json_response = json.loads(resp.content) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 556465c9d2..663cf22f64 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -27,7 +27,7 @@ from django.http import HttpResponseNotFound import json from django.utils.translation import ugettext as _ from pymongo import DESCENDING - +import math __all__ = ['assets_handler'] @@ -91,17 +91,20 @@ def _assets_json(request, location): """ requested_page = int(request.REQUEST.get('page', 0)) requested_page_size = int(request.REQUEST.get('page_size', 50)) + sort = [('uploadDate', DESCENDING)] + current_page = max(requested_page, 0) start = current_page * requested_page_size - - old_location = loc_mapper().translate_locator_to_location(location) - - course_reference = StaticContent.compute_location(old_location.org, old_location.course, old_location.name) - assets, total_count = contentstore().get_all_content_for_course( - course_reference, start=start, maxresults=requested_page_size, sort=[('uploadDate', DESCENDING)] - ) + assets, total_count = _get_assets_for_page(request, location, current_page, requested_page_size, sort) end = start + len(assets) + # If the query is beyond the final page, then re-query the final page so that at least one asset is returned + if requested_page > 0 and start >= total_count: + current_page = int(math.floor((total_count - 1) / requested_page_size)) + start = current_page * requested_page_size + assets, total_count = _get_assets_for_page(request, location, current_page, requested_page_size, sort) + end = start + len(assets) + asset_json = [] for asset in assets: asset_id = asset['_id'] @@ -123,6 +126,20 @@ def _assets_json(request, location): }) +def _get_assets_for_page(request, location, current_page, page_size, sort): + """ + Returns the list of assets for the specified page and page size. + """ + start = current_page * page_size + + old_location = loc_mapper().translate_locator_to_location(location) + + course_reference = StaticContent.compute_location(old_location.org, old_location.course, old_location.name) + return contentstore().get_all_content_for_course( + course_reference, start=start, maxresults=page_size, sort=sort + ) + + @require_POST @ensure_csrf_cookie @login_required