From 84bc81f7eb0ce0692c357c2f2e41fcf4e6aa5b5d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 3 Oct 2012 09:07:09 -0400 Subject: [PATCH] refactor StaticContent id scheme to be the same as moduledata. Also expose a new 'get all content for course' which will be needed to support a asset management page. --- cms/djangoapps/contentstore/views.py | 17 +++--- common/djangoapps/cache_toolbox/core.py | 15 +++--- common/djangoapps/contentserver/middleware.py | 8 +-- common/lib/xmodule/xmodule/capa_module.py | 5 +- .../xmodule/xmodule/contentstore/content.py | 53 +++++++++++++++---- .../lib/xmodule/xmodule/contentstore/mongo.py | 27 ++++++++-- common/lib/xmodule/xmodule/html_module.py | 8 ++- common/lib/xmodule/xmodule/x_module.py | 3 +- 8 files changed, 97 insertions(+), 39 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 1443f37c2d..3fe7cd5def 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -429,14 +429,13 @@ def clone_item(request): return HttpResponse(json.dumps({'id': dest_location.url()})) -''' -cdodge: this method allows for POST uploading of files into the course asset library, which will -be supported by GridFS in MongoDB. -''' #@login_required #@ensure_csrf_cookie def upload_asset(request, org, course, coursename): - + ''' + cdodge: this method allows for POST uploading of files into the course asset library, which will + be supported by GridFS in MongoDB. + ''' if request.method != 'POST': # (cdodge) @todo: Is there a way to do a - say - 'raise Http400'? return HttpResponseBadRequest() @@ -463,7 +462,7 @@ def upload_asset(request, org, course, coursename): mime_type = request.FILES['file'].content_type filedata = request.FILES['file'].read() - file_location = StaticContent.compute_location_filename(org, course, name) + file_location = StaticContent.compute_location(org, course, name) content = StaticContent(file_location, name, mime_type, filedata) @@ -476,7 +475,7 @@ def upload_asset(request, org, course, coursename): # browser-side caching support. We *could* re-fetch the saved content so that we have the # timestamp populated, but we might as well wait for the first real request to come in # to re-populate the cache. - del_cached_content(file_location) + del_cached_content(content.location) # if we're uploading an image, then let's generate a thumbnail so that we can # serve it up when needed without having to rescale on the fly @@ -504,7 +503,7 @@ def upload_asset(request, org, course, coursename): # .thumbnail.jpg thumbnail_name = os.path.splitext(name)[0] + '.thumbnail.jpg' # then just store this thumbnail as any other piece of content - thumbnail_file_location = StaticContent.compute_location_filename(org, course, + thumbnail_file_location = StaticContent.compute_location(org, course, thumbnail_name) thumbnail_content = StaticContent(thumbnail_file_location, thumbnail_name, 'image/jpeg', thumbnail_file) @@ -512,7 +511,7 @@ def upload_asset(request, org, course, coursename): # remove any cached content at this location, as thumbnails are treated just like any # other bit of static content - del_cached_content(thumbnail_file_location) + del_cached_content(thumbnail_content.location) except: # catch, log, and continue as thumbnails are not a hard requirement logging.error('Failed to generate thumbnail for {0}. Continuing...'.format(name)) diff --git a/common/djangoapps/cache_toolbox/core.py b/common/djangoapps/cache_toolbox/core.py index a7f0c0819f..caba804750 100644 --- a/common/djangoapps/cache_toolbox/core.py +++ b/common/djangoapps/cache_toolbox/core.py @@ -12,7 +12,7 @@ from django.core.cache import cache from django.db import DEFAULT_DB_ALIAS from . import app_settings - +from xmodule.contentstore.content import StaticContent def get_instance(model, instance_or_pk, timeout=None, using=None): """ @@ -108,14 +108,11 @@ def instance_key(model, instance_or_pk): getattr(instance_or_pk, 'pk', instance_or_pk), ) -def content_key(filename): - return 'content:%s' % (filename) - def set_cached_content(content): - cache.set(content_key(content.filename), content) + cache.set(content.get_id(), content) -def get_cached_content(filename): - return cache.get(content_key(filename)) +def get_cached_content(location): + return cache.get(StaticContent.get_id_from_location(location)) -def del_cached_content(filename): - cache.delete(content_key(filename)) +def del_cached_content(location): + cache.delete(StaticContent.get_id_from_location(location)) diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index 56d4ed8d1c..bc5d80842e 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -12,14 +12,14 @@ from xmodule.exceptions import NotFoundError class StaticContentServer(object): def process_request(self, request): # look to see if the request is prefixed with 'c4x' tag - if request.path.startswith('/' + XASSET_LOCATION_TAG): - + if request.path.startswith('/' + XASSET_LOCATION_TAG +'/'): + loc = StaticContent.get_location_from_path(request.path) # first look in our cache so we don't have to round-trip to the DB - content = get_cached_content(request.path) + content = get_cached_content(loc) if content is None: # nope, not in cache, let's fetch from DB try: - content = contentstore().find(request.path) + content = contentstore().find(loc) except NotFoundError: raise Http404 diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index a821ec9c12..f6054c494d 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -339,7 +339,10 @@ class CapaModule(XModule): # NOTE: rewrite_content_links is defined in XModule # This is a bit unfortunate and I'm sure we'll try to considate this into # a one step process. - html = rewrite_links(html, self.rewrite_content_links) + try: + html = rewrite_links(html, self.rewrite_content_links) + except: + logging.error('error rewriting links in {0}'.format(html)) # now do the substitutions which are filesystem based, e.g. '/static/' prefixes return self.system.replace_urls(html, self.metadata['data_dir']) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 712c5e7851..d406e4b85c 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -1,26 +1,59 @@ XASSET_LOCATION_TAG = 'c4x' XASSET_SRCREF_PREFIX = 'xasset:' +import logging +from xmodule.modulestore import Location + class StaticContent(object): - def __init__(self, filename, name, content_type, data, last_modified_at=None): - self.filename = filename - self.name = name + def __init__(self, loc, name, content_type, data, last_modified_at=None): + self.location = loc + self.name = name #a display string which can be edited, and thus not part of the location which needs to be fixed self.content_type = content_type self.data = data self.last_modified_at = last_modified_at - @staticmethod - def compute_location_filename(org, course, name): - return '/{0}/{1}/{2}/asset/{3}'.format(XASSET_LOCATION_TAG, org, course, name) -''' -Abstraction for all ContentStore providers (e.g. MongoDB) -''' + @staticmethod + def compute_location(org, course, name, revision=None): + return Location([XASSET_LOCATION_TAG, org, course, 'asset', name, revision]) + + def get_id(self): + return StaticContent.get_id_from_location(self.location) + + def get_url_path(self): + return StaticContent.get_url_path_from_location(self.location) + + @staticmethod + def get_url_path_from_location(location): + return "/{tag}/{org}/{course}/{category}/{name}".format(**location.dict()) + + @staticmethod + def get_id_from_location(location): + return { 'tag':location.tag, 'org' : location.org, 'course' : location.course, + 'category' : location.category, 'name' : location.name, + 'revision' : location.revision} + @staticmethod + def get_location_from_path(path): + # remove leading / character if it is there one + if path.startswith('/'): + path = path[1:] + + return Location(path.split('/')) + + @staticmethod + def get_id_from_path(path): + return get_id_from_location(get_location_from_path(path)) + + class ContentStore(object): + ''' + Abstraction for all ContentStore providers (e.g. MongoDB) + ''' def save(self, content): raise NotImplementedError def find(self, filename): raise NotImplementedError - + def get_all_content_for_course(self, location): + raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 7903a77cb6..56079aa964 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -1,7 +1,11 @@ +from bson.son import SON from pymongo import Connection import gridfs from gridfs.errors import NoFile +from xmodule.modulestore.mongo import location_to_query, Location +from xmodule.contentstore.content import XASSET_LOCATION_TAG + import sys import logging @@ -14,19 +18,34 @@ class MongoContentStore(ContentStore): logging.debug( 'Using MongoDB for static content serving at host={0} db={1}'.format(host,db)) _db = Connection(host=host, port=port)[db] self.fs = gridfs.GridFS(_db) + self.fs_files = _db["fs.files"] # the underlying collection GridFS uses + def save(self, content): - with self.fs.new_file(filename=content.filename, content_type=content.content_type, displayname=content.name) as fp: + id = content.get_id() + + # Seems like with the GridFS we can't update existing ID's we have to do a delete/add pair + if self.fs.exists({"_id" : id}): + self.fs.delete(id) + + with self.fs.new_file(_id = id, content_type=content.content_type, displayname=content.name) as fp: fp.write(content.data) return content - def find(self, filename): + def find(self, location): + id = StaticContent.get_id_from_location(location) try: - with self.fs.get_last_version(filename) as fp: - return StaticContent(fp.filename, fp.displayname, fp.content_type, fp.read(), fp.uploadDate) + with self.fs.get(id) as fp: + return StaticContent(location, fp.displayname, fp.content_type, fp.read(), fp.uploadDate) except NoFile: raise NotFoundError() + def get_all_content_info_for_course(self, location): + course_filter = Location(XASSET_LOCATION_TAG, category="asset",course=location.course,org=location.org) + # 'borrow' the function 'location_to_query' from the Mongo modulestore implementation + items = self.fs_files.find(location_to_query(course_filter)) + return items + diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 0ed64897e2..632a3a64ce 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -26,7 +26,13 @@ class HtmlModule(XModule): def get_html(self): # cdodge: perform link substitutions for any references to course static content (e.g. images) - return rewrite_links(self.html, self.rewrite_content_links) + _html = self.html + try: + _html = rewrite_links(_html, self.rewrite_content_links) + except: + logging.error('error rewriting links on the following HTML content: {0}'.format(_html)) + + return _html def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index dd0df2125a..43b7cec3af 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -328,7 +328,8 @@ class XModule(HTMLSnippet): name = link[len(XASSET_SRCREF_PREFIX):] loc = Location(self.location) # resolve the reference to our internal 'filepath' which - link = StaticContent.compute_location_filename(loc.org, loc.course, name) + content_loc = StaticContent.compute_location(loc.org, loc.course, name) + link = StaticContent.get_url_path_from_location(content_loc) return link