From e69955b437de54a456b7509129ae79a72c4c736f Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 18 Sep 2013 11:39:12 -0400 Subject: [PATCH 1/2] Pylint/pep8 fixes --- cms/djangoapps/contentstore/views/assets.py | 1 - .../xmodule/xmodule/contentstore/content.py | 4 ++-- .../lib/xmodule/xmodule/contentstore/mongo.py | 24 +++++++++---------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index b1fb148300..3d5704b869 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -10,7 +10,6 @@ from django.views.decorators.http import require_POST from mitxmako.shortcuts import render_to_response from cache_toolbox.core import del_cached_content -from auth.authz import create_all_course_groups from xmodule.contentstore.django import contentstore from xmodule.modulestore.django import modulestore diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 69a2d6a5ef..bd3a403bb1 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -15,9 +15,9 @@ from PIL import Image class StaticContent(object): def __init__(self, loc, name, content_type, data, last_modified_at=None, thumbnail_location=None, import_path=None, - length=None): + length=None, locked=False): 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.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.length = length diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 482ce585e6..78e8b65e1b 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -24,15 +24,15 @@ class MongoContentStore(ContentStore): self.fs = gridfs.GridFS(_db, bucket) - self.fs_files = _db[bucket + ".files"] # the underlying collection GridFS uses + self.fs_files = _db[bucket + ".files"] # the underlying collection GridFS uses def save(self, content): - id = content.get_id() + content_id = content.get_id() # Seems like with the GridFS we can't update existing ID's we have to do a delete/add pair - self.delete(id) + self.delete(content_id) - with self.fs.new_file(_id=id, filename=content.get_url_path(), content_type=content.content_type, + with self.fs.new_file(_id=content_id, filename=content.get_url_path(), content_type=content.content_type, displayname=content.name, thumbnail_location=content.thumbnail_location, import_path=content.import_path) as fp: if hasattr(content.data, '__iter__'): @@ -43,25 +43,25 @@ class MongoContentStore(ContentStore): return content - def delete(self, id): - if self.fs.exists({"_id": id}): - self.fs.delete(id) + def delete(self, content_id): + if self.fs.exists({"_id": content_id}): + self.fs.delete(content_id) def find(self, location, throw_on_not_found=True, as_stream=False): - id = StaticContent.get_id_from_location(location) + content_id = StaticContent.get_id_from_location(location) try: if as_stream: - fp = self.fs.get(id) return StaticContentStream(location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate, thumbnail_location=fp.thumbnail_location if hasattr(fp, 'thumbnail_location') else None, import_path=fp.import_path if hasattr(fp, 'import_path') else None, length=fp.length) + fp = self.fs.get(content_id) else: - with self.fs.get(id) as fp: return StaticContent(location, fp.displayname, fp.content_type, fp.read(), last_modified_at=fp.uploadDate, thumbnail_location=fp.thumbnail_location if hasattr(fp, 'thumbnail_location') else None, import_path=fp.import_path if hasattr(fp, 'import_path') else None, length=fp.length) + with self.fs.get(content_id) as fp: except NoFile: if throw_on_not_found: raise NotFoundError() @@ -69,9 +69,9 @@ class MongoContentStore(ContentStore): return None def get_stream(self, location): - id = StaticContent.get_id_from_location(location) + content_id = StaticContent.get_id_from_location(location) try: - handle = self.fs.get(id) + handle = self.fs.get(content_id) except NoFile: raise NotFoundError() From 7d13eb8f7d38546c88c0e5dbf2fa99ad2eafe709 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 19 Sep 2013 16:44:54 -0400 Subject: [PATCH 2/2] Add locked attribute to assets. --- .../xmodule/xmodule/contentstore/content.py | 7 +- .../lib/xmodule/xmodule/contentstore/mongo.py | 82 +++++++++++++++++-- .../xmodule/modulestore/tests/test_mongo.py | 51 ++++++++++++ 3 files changed, 128 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index bd3a403bb1..495b24426f 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -26,6 +26,7 @@ class StaticContent(object): # optional information about where this file was imported from. This is needed to support import/export # cycles self.import_path = import_path + self.locked = locked @property def is_thumbnail(self): @@ -133,10 +134,10 @@ class StaticContent(object): class StaticContentStream(StaticContent): def __init__(self, loc, name, content_type, stream, last_modified_at=None, thumbnail_location=None, import_path=None, - length=None): + length=None, locked=False): super(StaticContentStream, self).__init__(loc, name, content_type, None, last_modified_at=last_modified_at, thumbnail_location=thumbnail_location, import_path=import_path, - length=length) + length=length, locked=locked) self._stream = stream def stream_data(self): @@ -153,7 +154,7 @@ class StaticContentStream(StaticContent): self._stream.seek(0) content = StaticContent(self.location, self.name, self.content_type, self._stream.read(), last_modified_at=self.last_modified_at, thumbnail_location=self.thumbnail_location, - import_path=self.import_path, length=self.length) + import_path=self.import_path, length=self.length, locked=self.locked) return content diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 78e8b65e1b..2043cad4d6 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -34,7 +34,9 @@ class MongoContentStore(ContentStore): with self.fs.new_file(_id=content_id, filename=content.get_url_path(), content_type=content.content_type, displayname=content.name, thumbnail_location=content.thumbnail_location, - import_path=content.import_path) as fp: + import_path=content.import_path, + # getattr b/c caching may mean some pickled instances don't have attr + locked=getattr(content, 'locked', False)) as fp: if hasattr(content.data, '__iter__'): for chunk in content.data: fp.write(chunk) @@ -51,17 +53,21 @@ class MongoContentStore(ContentStore): content_id = StaticContent.get_id_from_location(location) try: if as_stream: - return StaticContentStream(location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate, - thumbnail_location=fp.thumbnail_location if hasattr(fp, 'thumbnail_location') else None, - import_path=fp.import_path if hasattr(fp, 'import_path') else None, - length=fp.length) fp = self.fs.get(content_id) + return StaticContentStream( + location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate, + thumbnail_location=getattr(fp, 'thumbnail_location', None), + import_path=getattr(fp, 'import_path', None), + length=fp.length, locked=getattr(fp, 'locked', False) + ) else: - return StaticContent(location, fp.displayname, fp.content_type, fp.read(), last_modified_at=fp.uploadDate, - thumbnail_location=fp.thumbnail_location if hasattr(fp, 'thumbnail_location') else None, - import_path=fp.import_path if hasattr(fp, 'import_path') else None, - length=fp.length) with self.fs.get(content_id) as fp: + return StaticContent( + location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate, + thumbnail_location=getattr(fp, 'thumbnail_location', None), + import_path=getattr(fp, 'import_path', None), + length=fp.length, locked=getattr(fp, 'locked', False) + ) except NoFile: if throw_on_not_found: raise NotFoundError() @@ -135,3 +141,61 @@ class MongoContentStore(ContentStore): # 'borrow' the function 'location_to_query' from the Mongo modulestore implementation items = self.fs_files.find(location_to_query(course_filter)) return list(items) + + def set_attr(self, location, attr, value=True): + """ + Add/set the given attr on the asset at the given location. Does not allow overwriting gridFS built in + attrs such as _id, md5, uploadDate, length. Value can be any type which pymongo accepts. + + Returns nothing + + Raises NotFoundError if no such item exists + Raises AttributeError is attr is one of the build in attrs. + + :param location: a c4x asset location + :param attr: which attribute to set + :param value: the value to set it to (any type pymongo accepts such as datetime, number, string) + """ + self.set_attrs(location, {attr: value}) + + def get_attr(self, location, attr, default=None): + """ + Get the value of attr set on location. If attr is unset, it returns default. Unlike set, this accessor + does allow getting the value of reserved keywords. + :param location: a c4x asset location + """ + return self.get_attrs(location).get(attr, default) + + def set_attrs(self, location, attr_dict): + """ + Like set_attr but sets multiple key value pairs. + + Returns nothing. + + Raises NotFoundError if no such item exists + Raises AttributeError is attr_dict has any attrs which are one of the build in attrs. + + :param location: a c4x asset location + """ + for attr in attr_dict.iterkeys(): + if attr in ['_id', 'md5', 'uploadDate', 'length']: + raise AttributeError("{} is a protected attribute.".format(attr)) + item = self.fs_files.find_one(location_to_query(location)) + if item is None: + raise NotFoundError() + self.fs_files.update({"_id": item["_id"]}, {"$set": attr_dict}) + + def get_attrs(self, location): + """ + Gets all of the attributes associated with the given asset. Note, returns even built in attrs + such as md5 which you cannot resubmit in an update; so, don't call set_attrs with the result of this + but only with the set of attrs you want to explicitly update. + + The attrs will be a superset of _id, contentType, chunkSize, filename, uploadDate, & md5 + + :param location: a c4x asset location + """ + item = self.fs_files.find_one(location_to_query(location)) + if item is None: + raise NotFoundError() + return item diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index ba1266cb14..e89aa21a16 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -20,6 +20,8 @@ from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint from xmodule.contentstore.mongo import MongoContentStore from xmodule.modulestore.tests.test_modulestore import check_path_to_location +from IPython.testing.nose_assert_methods import assert_in, assert_not_in +from xmodule.exceptions import NotFoundError log = logging.getLogger(__name__) @@ -203,6 +205,55 @@ class TestMongoModuleStore(object): assert_equals('Resources', get_tab_name(3)) assert_equals('Discussion', get_tab_name(4)) + def test_contentstore_attrs(self): + """ + Test getting, setting, and defaulting the locked attr and arbitrary attrs. + """ + location = Location('i4x', 'edX', 'toy', 'course', '2012_Fall') + course_content = TestMongoModuleStore.content_store.get_all_content_for_course(location) + assert len(course_content) > 0 + # a bit overkill, could just do for content[0] + for content in course_content: + assert not content.get('locked', False) + assert not TestMongoModuleStore.content_store.get_attr(content['_id'], 'locked', False) + attrs = TestMongoModuleStore.content_store.get_attrs(content['_id']) + assert_in('uploadDate', attrs) + assert not attrs.get('locked', False) + TestMongoModuleStore.content_store.set_attr(content['_id'], 'locked', True) + assert TestMongoModuleStore.content_store.get_attr(content['_id'], 'locked', False) + attrs = TestMongoModuleStore.content_store.get_attrs(content['_id']) + assert_in('locked', attrs) + assert attrs['locked'] is True + TestMongoModuleStore.content_store.set_attrs(content['_id'], {'miscel': 99}) + assert_equals(TestMongoModuleStore.content_store.get_attr(content['_id'], 'miscel'), 99) + assert_raises( + AttributeError, TestMongoModuleStore.content_store.set_attr, course_content[0], + 'md5', 'ff1532598830e3feac91c2449eaa60d6' + ) + assert_raises( + AttributeError, TestMongoModuleStore.content_store.set_attrs, course_content[0], + {'foo': 9, 'md5': 'ff1532598830e3feac91c2449eaa60d6'} + ) + assert_raises( + NotFoundError, TestMongoModuleStore.content_store.get_attr, + Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'), + 'displayname' + ) + assert_raises( + NotFoundError, TestMongoModuleStore.content_store.set_attr, + Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'), + 'displayname', 'hello' + ) + assert_raises( + NotFoundError, TestMongoModuleStore.content_store.get_attrs, + Location('bogus', 'bogus', 'bogus', 'asset', 'bogus') + ) + assert_raises( + NotFoundError, TestMongoModuleStore.content_store.set_attrs, + Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'), + {'displayname': 'hello'} + ) + class TestMongoKeyValueStore(object): """