From 0f70b0aa8ac0914d5b0cb7fed1ebe2510e3931f1 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Wed, 8 May 2013 14:09:29 -0400 Subject: [PATCH] refactored api resource map and added total to search query --- lms/djangoapps/notes/api.py | 129 +++++++++++++++++++++------------- lms/djangoapps/notes/utils.py | 6 +- 2 files changed, 83 insertions(+), 52 deletions(-) diff --git a/lms/djangoapps/notes/api.py b/lms/djangoapps/notes/api.py index a5f39e3cea..7252fdd232 100644 --- a/lms/djangoapps/notes/api.py +++ b/lms/djangoapps/notes/api.py @@ -9,55 +9,66 @@ import logging log = logging.getLogger(__name__) -API_SETTINGS = { - 'MAX_NOTE_LIMIT': 100 # Max number of annotations to retrieve at one time +API_SETTINGS = { + # Version + 'META': {'name': 'Notes API', 'version': '1.0'}, + + # Maps resources to HTTP methods + 'RESOURCE_MAP': { + 'root': {'GET': 'root'}, + 'notes': {'GET': 'index', 'POST': 'create'}, + 'note': {'GET': 'read', 'PUT': 'update', 'DELETE': 'delete'}, + 'search': {'GET': 'search'}, + }, + + # Cap the number of notes that can be returned in one request + 'MAX_NOTE_LIMIT': 1000, } #----------------------------------------------------------------------# # API requests are routed through api_request() using the resource map. -def api_resource_map(): - ''' Maps API resources to (method, action) pairs. ''' - (GET, PUT, POST, DELETE) = ('GET', 'PUT', 'POST', 'DELETE') - return { - 'root': {GET: root}, - 'notes': {GET: index, POST: create}, - 'note': {GET: read, PUT: update, DELETE: delete}, - 'search': {GET: search} - } - @login_required def api_request(request, course_id, **kwargs): - ''' Routes API requests to the appropriate action method and formats the results - (defaults to JSON). - - Raises a 404 if the resource type doesn't exist, or if there is no action - method associated with the HTTP method. + ''' + Routes API requests to the appropriate action method and returns JSON. + Raises a 404 if the requested resource does not exist or notes are + disabled for the course. ''' + + # Verify that notes are enabled for the course course = get_course_with_access(request.user, course_id, 'load') if not notes_enabled_for_course(course): log.debug('Notes not enabled for course') raise Http404 - resource_map = api_resource_map() + # Locate and validate the requested resource + resource_map = API_SETTINGS.get('RESOURCE_MAP', {}) resource_name = kwargs.pop('resource') + resource_method = request.method resource = resource_map.get(resource_name) if resource is None: log.debug('Resource "{0}" does not exist'.format(resource_name)) raise Http404 - if request.method not in resource.keys(): - log.debug('Resource "{0}" does not support method "{1}"'.format(resource_name, request.method)) + if resource_method not in resource.keys(): + log.debug('Resource "{0}" does not support method "{1}"'.format(resource_name, resource_method)) raise Http404 - log.debug("API request: {0} {1}".format(request.method, resource_name)) + # Find the associated function definition and execute the request + func = resource.get(resource_method) + module = globals() + if func not in module: + log.debug('Function "{0}" does not exist for request {1} {2}'.format(action, resource_method, resource_name)) + raise Http404 - action = resource.get(request.method) - result = action(request, course_id, **kwargs) + log.debug('API request: {0} {1}'.format(resource_method, resource_name)) + result = module[func](request, course_id, **kwargs) - response = result[0] + # Format and output the results data = None + response = result[0] if len(result) == 2: data = result[1] @@ -65,13 +76,13 @@ def api_request(request, course_id, **kwargs): response['Content-type'] = formatted[0] response.content = formatted[1] - log.debug("API response: {0}".format(formatted)) + log.debug('API response: {0}'.format(formatted)) return response def api_format(request, response, data): - ''' Returns a two-element list containing the content type and content. - This method does not modify the request or response. + ''' + Returns a two-element list containing the content type and content. ''' content_type = 'application/json' if data is None: @@ -84,7 +95,9 @@ def api_format(request, response, data): # API actions exposed via the resource map. def index(request, course_id): - ''' Returns a list of annotation objects. ''' + ''' + Returns a list of annotation objects. + ''' MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT') notes = Note.objects.order_by('id').filter(course_id=course_id, @@ -93,7 +106,9 @@ def index(request, course_id): return [HttpResponse(), [note.as_dict() for note in notes]] def create(request, course_id): - ''' Receives an annotation object to create and returns a 303 with the read location. ''' + ''' + Receives an annotation object to create and returns a 303 with the read location. + ''' note = Note(course_id=course_id, user=request.user) try: @@ -109,7 +124,9 @@ def create(request, course_id): return [response, None] def read(request, course_id, note_id): - ''' Returns a single annotation object. ''' + ''' + Returns a single annotation object. + ''' try: note = Note.objects.get(id=note_id) except: @@ -121,7 +138,9 @@ def read(request, course_id, note_id): return [HttpResponse(), note.as_dict()] def update(request, course_id, note_id): - ''' Updates an annotation object and returns a 303 with the read location. ''' + ''' + Updates an annotation object and returns a 303 with the read location. + ''' try: note = Note.objects.get(id=note_id) except: @@ -144,7 +163,9 @@ def update(request, course_id, note_id): return [response, None] def delete(request, course_id, note_id): - ''' Deletes the annotation object and returns a 204 with no content. ''' + ''' + Deletes the annotation object and returns a 204 with no content. + ''' try: note = Note.objects.get(id=note_id) except: @@ -158,39 +179,47 @@ def delete(request, course_id, note_id): return [HttpResponse('', status=204), None] def search(request, course_id): - ''' Returns a subset of annotation objects based on a search query.. ''' + ''' + Returns a subset of annotation objects based on a search query. + ''' MAX_LIMIT = API_SETTINGS.get('MAX_NOTE_LIMIT') # search parameters - limit = request.GET.get('limit') - offset = request.GET.get('offset') - uri = request.GET.get('uri') + offset = request.GET.get('offset', '') + limit = request.GET.get('limit', '') + uri = request.GET.get('uri', '') # validate search parameters - if limit is not None and limit.isdigit(): + if offset.isdigit(): + offset = int(offset) + else: + offset = 0 + + if limit.isdigit(): limit = int(limit) if limit == 0 or limit > MAX_LIMIT: limit = MAX_LIMIT else: limit = MAX_LIMIT - if offset is not None and offset.isdigit(): - offset = int(offset) - else: - offset = 0 - - # search filters + # set filters filters = {'course_id': course_id, 'user': request.user} - if uri is not None: + if uri != '': filters['uri'] = uri - start = offset - end = offset + limit - notes = Note.objects.order_by('id').filter(**filters)[start:end] - result = {'rows': [note.as_dict() for note in notes]} + # retrieve notes + notes = Note.objects.order_by('id').filter(**filters) + total = notes.count() + rows = notes[offset:offset+limit] + result = { + 'total': notes.count(), + 'rows': [note.as_dict() for note in rows] + } return [HttpResponse(), result] def root(request, course_id): - ''' Returns version information about the API. ''' - return [HttpResponse(), {'name': 'Notes API', 'version': '1.0'}] + ''' + Returns version information about the API. + ''' + return [HttpResponse(), API_SETTINGS.get('META')] diff --git a/lms/djangoapps/notes/utils.py b/lms/djangoapps/notes/utils.py index e06df3e42a..5e3c0182fa 100644 --- a/lms/djangoapps/notes/utils.py +++ b/lms/djangoapps/notes/utils.py @@ -1,5 +1,7 @@ -# TODO: make a separate policy setting to enable/disable notes. def notes_enabled_for_course(course): - ''' Returns True if notes are enabled for the course, False otherwise. ''' + ''' + Returns True if the notes app is enabled for the course, False otherwise. + ''' + # TODO: create a separate policy setting to enable/disable notes notes_tab_type = 'notes' return next((True for tab in course.tabs if tab['type'] == notes_tab_type), False)