From 03caf94c9842ddc89224cedd917d72cc83f76fbd Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 15 Mar 2013 16:26:39 -0400 Subject: [PATCH 01/14] remove redundent XML attribtues which are in metadata --- common/lib/xmodule/xmodule/templates/discussion/default.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/templates/discussion/default.yaml b/common/lib/xmodule/xmodule/templates/discussion/default.yaml index d34e6378e6..49c0ce9c48 100644 --- a/common/lib/xmodule/xmodule/templates/discussion/default.yaml +++ b/common/lib/xmodule/xmodule/templates/discussion/default.yaml @@ -5,5 +5,5 @@ metadata: id: 6002x_group_discussion_by_this discussion_category: Week 1 data: | - + children: [] From b54ebb346027dbe46e17d606a9865f95cbf062a5 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 15 Mar 2013 22:43:20 -0400 Subject: [PATCH 02/14] make discussion module use MetadataOnlyEditingDescriptor which will not present a code edit region and only display the metadata editor --- cms/templates/widgets/metadata-only-edit.html | 1 + common/lib/xmodule/xmodule/discussion_module.py | 3 ++- common/lib/xmodule/xmodule/editing_module.py | 10 ++++++++++ .../xmodule/js/src/raw/edit/metadata-only.coffee | 5 +++++ common/lib/xmodule/xmodule/raw_module.py | 2 +- 5 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 cms/templates/widgets/metadata-only-edit.html create mode 100644 common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.coffee diff --git a/cms/templates/widgets/metadata-only-edit.html b/cms/templates/widgets/metadata-only-edit.html new file mode 100644 index 0000000000..a784f3798c --- /dev/null +++ b/cms/templates/widgets/metadata-only-edit.html @@ -0,0 +1 @@ +<%include file="metadata-edit.html" /> diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index 7725a88e77..a0a5207e16 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -3,6 +3,7 @@ from pkg_resources import resource_string, resource_listdir from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor +from xmodule.editing_module import MetadataOnlyEditingDescriptor from xblock.core import String, Scope @@ -28,7 +29,7 @@ class DiscussionModule(DiscussionFields, XModule): return self.system.render_template('discussion/_discussion_module.html', context) -class DiscussionDescriptor(DiscussionFields, RawDescriptor): +class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor): module_class = DiscussionModule template_dir_name = "discussion" diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index b93727a96b..1f07861ae8 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -40,6 +40,16 @@ class XMLEditingDescriptor(EditingDescriptor): js = {'coffee': [resource_string(__name__, 'js/src/raw/edit/xml.coffee')]} js_module_name = "XMLEditingDescriptor" +class MetadataOnlyEditingDescriptor(EditingDescriptor): + """ + Module that provides a raw editing view of its data as XML. It does not perform + any validation of its definition + """ + + js = {'coffee': [resource_string(__name__, 'js/src/raw/edit/metadata-only.coffee')]} + js_module_name = "MetadataOnlyEditingDescriptor" + + mako_template = "widgets/metadata-only-edit.html" class JSONEditingDescriptor(EditingDescriptor): """ diff --git a/common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.coffee b/common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.coffee new file mode 100644 index 0000000000..8c9afe86aa --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/raw/edit/metadata-only.coffee @@ -0,0 +1,5 @@ +class @MetadataOnlyEditingDescriptor extends XModule.Descriptor + constructor: (@element) -> + + save: -> + data: null diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 2c6e157018..6b5c2441be 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -1,5 +1,5 @@ from lxml import etree -from xmodule.editing_module import XMLEditingDescriptor +from xmodule.editing_module import XMLEditingDescriptor, MetadataOnlyEditingDescriptor from xmodule.xml_module import XmlDescriptor import logging import sys From 3e65a688288695c8b1042de9ce56f3cc175d47c4 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 15 Mar 2013 23:01:03 -0400 Subject: [PATCH 03/14] auto generate the discussion id on create. Also add 'discussion_id' to the list of 'system-metadata' so that end users cannot edit it --- common/lib/xmodule/xmodule/modulestore/mongo.py | 7 +++++++ .../lib/xmodule/xmodule/templates/discussion/default.yaml | 2 +- common/lib/xmodule/xmodule/x_module.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index c5e5bbfdf8..d115a92852 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -9,6 +9,7 @@ from fs.osfs import OSFS from itertools import repeat from path import path from datetime import datetime, timedelta +from uuid import uuid4 from importlib import import_module from xmodule.errortracker import null_error_tracker, exc_info_to_str @@ -496,6 +497,12 @@ class MongoModuleStore(ModuleStoreBase): """ try: source_item = self.collection.find_one(location_to_query(source)) + + # allow for some programmatically generated substitutions in metadata, e.g. Discussion_id's should be auto-generated + for key in source_item['metadata'].keys(): + if source_item['metadata'][key] == '$$GUID$$': + source_item['metadata'][key] = uuid4().hex + source_item['_id'] = Location(location).dict() self.collection.insert(source_item) item = self._load_items([source_item])[0] diff --git a/common/lib/xmodule/xmodule/templates/discussion/default.yaml b/common/lib/xmodule/xmodule/templates/discussion/default.yaml index 49c0ce9c48..049e34b3e7 100644 --- a/common/lib/xmodule/xmodule/templates/discussion/default.yaml +++ b/common/lib/xmodule/xmodule/templates/discussion/default.yaml @@ -2,7 +2,7 @@ metadata: display_name: Discussion Tag for: Topic-Level Student-Visible Label - id: 6002x_group_discussion_by_this + id: $$GUID$$ discussion_category: Week 1 data: | diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 02835e0d5d..5e1ba826b8 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -340,7 +340,7 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): # cdodge: this is a list of metadata names which are 'system' metadata # and should not be edited by an end-user - system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft'] + system_metadata_fields = ['data_dir', 'published_date', 'published_by', 'is_draft', 'discussion_id'] # A list of descriptor attributes that must be equal for the descriptors to # be equal From 9a18685c1c674668500d2382e223d932f6c5177d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 18 Mar 2013 09:01:54 -0400 Subject: [PATCH 04/14] add a discussion module write through cache to speed up Forum rendering --- cms/djangoapps/contentstore/utils.py | 1 + cms/one_time_startup.py | 8 +++ .../cache_toolbox/discussion_cache.py | 52 +++++++++++++++++++ .../xmodule/xmodule/modulestore/__init__.py | 3 +- .../lib/xmodule/xmodule/modulestore/mongo.py | 20 ++++++- lms/djangoapps/django_comment_client/utils.py | 16 +----- 6 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 common/djangoapps/cache_toolbox/discussion_cache.py diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 0a99441fe9..7d9d539604 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -6,6 +6,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info'] + def get_modulestore(location): """ Returns the correct modulestore to use for modifying the specified location diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py index 38a2fef847..961ffd15fb 100644 --- a/cms/one_time_startup.py +++ b/cms/one_time_startup.py @@ -1,6 +1,8 @@ from dogapi import dog_http_api, dog_stats_api from django.conf import settings from xmodule.modulestore.django import modulestore +from cache_toolbox.discussion_cache import discussion_cache_register_for_updates +from django.dispatch import Signal from django.core.cache import get_cache, InvalidCacheBackendError @@ -9,6 +11,12 @@ for store_name in settings.MODULESTORE: store = modulestore(store_name) store.metadata_inheritance_cache = cache + modulestore_update_signal = Signal( + providing_args=['modulestore', 'course_id', 'location'] + ) + store.modulestore_update_signal = modulestore_update_signal + discussion_cache_register_for_updates(store) + if hasattr(settings, 'DATADOG_API'): dog_http_api.api_key = settings.DATADOG_API dog_stats_api.start(api_key=settings.DATADOG_API, statsd=True) diff --git a/common/djangoapps/cache_toolbox/discussion_cache.py b/common/djangoapps/cache_toolbox/discussion_cache.py new file mode 100644 index 0000000000..57390528bf --- /dev/null +++ b/common/djangoapps/cache_toolbox/discussion_cache.py @@ -0,0 +1,52 @@ +import logging +from django.core.cache import cache, get_cache +from datetime import datetime, timedelta + +def _get_discussion_cache(): + return cache + + +def get_discussion_cache_key(course_id): + return 'discussion_{0}'.format(course_id) + + +def get_discussion_cache_entry(modulestore, course_id): + cache_entry = None + cache = _get_discussion_cache() + + if cache is not None: + cache_entry = cache.get(get_discussion_cache_key(course_id), None) + if cache_entry is not None: + delta = datetime.now() - cache_entry.get('timestamp', datetime.min) + if delta > Timedelta(0,300): + cache_entry = None + + if cache_entry is None: + cache_entry = generate_discussion_cache_entry(modulestore, course_id) + + return cache_entry.get('modules',[]) + + +def generate_discussion_cache_entry(modulestore, course_id): + components = course_id.split('/') + all_discussion_modules = modulestore.get_items(['i4x', components[0], components[1], 'discussion', None], + course_id=course_id) + + cache = _get_discussion_cache() + if cache is not None: + cache.set(get_discussion_cache_key(course_id), {'modules': all_discussion_modules, 'timestamp': datetime.now()}) + return all_discussion_modules + + +def modulestore_update_signal_handler(modulestore = None, course_id = None, location = None, **kwargs): + """called when there is an write event in our modulestore + """ + if location.category == 'discussion': + logging.debug('******* got modulestore update signal. Regenerating discussion cache for {0}'.format(course_id)) + # refresh the cache entry if we've changed a discussion module + generate_discussion_cache_entry(modulestore, course_id) + + +def discussion_cache_register_for_updates(modulestore): + if modulestore.modulestore_update_signal is not None: + modulestore.modulestore_update_signal.connect(modulestore_update_signal_handler) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 022e016a58..a1a159b700 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -251,7 +251,6 @@ class Location(_LocationBase): def __repr__(self): return "Location%s" % repr(tuple(self)) - @property def course_id(self): """Return the ID of the Course that this item belongs to by looking @@ -413,7 +412,6 @@ class ModuleStore(object): return courses - class ModuleStoreBase(ModuleStore): ''' Implement interface functionality that can be shared. @@ -424,6 +422,7 @@ class ModuleStoreBase(ModuleStore): ''' self._location_errors = {} # location -> ErrorLog self.metadata_inheritance_cache = None + self.modulestore_update_signal = None # can be set by runtime to route notifications of datastore changes def _get_errorlog(self, location): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index d115a92852..b0b8b11aef 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -31,6 +31,10 @@ log = logging.getLogger(__name__) # there is only one revision for each item. Once we start versioning inside the CMS, # that assumption will have to change +def get_course_id_no_run(location): + ''' + ''' + return "/".join([location.org, location.course]) class MongoKeyValueStore(KeyValueStore): """ @@ -527,6 +531,15 @@ class MongoModuleStore(ModuleStoreBase): # recompute (and update) the metadata inheritance tree which is cached self.get_cached_metadata_inheritance_tree(Location(location), force_refresh = True) + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) + + def fire_updated_modulestore_signal(self, course_id, location): + if self.modulestore_update_signal is not None: + self.modulestore_update_signal.send(None, + modulestore = self, + course_id = course_id, + location = location + ) def get_course_for_item(self, location, depth=0): ''' @@ -594,6 +607,8 @@ class MongoModuleStore(ModuleStoreBase): self._update_single_item(location, {'definition.children': children}) # recompute (and update) the metadata inheritance tree which is cached self.get_cached_metadata_inheritance_tree(Location(location), force_refresh = True) + # fire signal that we've written to DB + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) def update_metadata(self, location, metadata): """ @@ -619,7 +634,8 @@ class MongoModuleStore(ModuleStoreBase): self._update_single_item(location, {'metadata': metadata}) # recompute (and update) the metadata inheritance tree which is cached - self.get_cached_metadata_inheritance_tree(loc, force_refresh = True) + self.get_cached_metadata_inheritance_tree(loc, force_refresh = True) + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) def delete_item(self, location): """ @@ -640,7 +656,7 @@ class MongoModuleStore(ModuleStoreBase): self.collection.remove({'_id': Location(location).dict()}) # recompute (and update) the metadata inheritance tree which is cached self.get_cached_metadata_inheritance_tree(Location(location), force_refresh = True) - + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) def get_parent_locations(self, location, course_id): '''Find all locations that are the parents of this location in this diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 42233b84da..fbd0b29eca 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -16,6 +16,7 @@ from django.utils import simplejson from django_comment_client.models import Role from django_comment_client.permissions import check_permissions_by_view from xmodule.modulestore.exceptions import NoPathToItem +from cache_toolbox.discussion_cache import get_discussion_cache_entry from mitxmako import middleware import pystache_custom as pystache @@ -146,28 +147,15 @@ def sort_map_entries(category_map): def initialize_discussion_info(course): - global _DISCUSSIONINFO - # only cache in-memory discussion information for 10 minutes - # this is because we need a short-term hack fix for - # mongo-backed courseware whereby new discussion modules can be added - # without LMS service restart - - if _DISCUSSIONINFO[course.id]: - timestamp = _DISCUSSIONINFO[course.id].get('timestamp', datetime.now()) - age = datetime.now() - timestamp - # expire every 5 minutes - if age.seconds < 300: - return - course_id = course.id discussion_id_map = {} unexpanded_category_map = defaultdict(list) # get all discussion models within this course_id - all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course, 'discussion', None], course_id=course_id) + all_modules = get_discussion_cache_entry(modulestore(), course_id) for module in all_modules: skip_module = False From a5845a2087b6bca5e79b0a3ec3e266f28da20734 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 21 Mar 2013 15:46:57 -0400 Subject: [PATCH 05/14] remove discussion caching for now. It might be overkill and I need to figure out how to serialize out the resultset into the cache as writing modules into the cache causes a pickle error --- .../cache_toolbox/discussion_cache.py | 17 ++++++++--------- lms/djangoapps/django_comment_client/utils.py | 3 ++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/cache_toolbox/discussion_cache.py b/common/djangoapps/cache_toolbox/discussion_cache.py index 57390528bf..bac41d6c16 100644 --- a/common/djangoapps/cache_toolbox/discussion_cache.py +++ b/common/djangoapps/cache_toolbox/discussion_cache.py @@ -1,13 +1,13 @@ import logging from django.core.cache import cache, get_cache -from datetime import datetime, timedelta +from datetime import datetime def _get_discussion_cache(): - return cache + return get_cache('mongo_metadata_inheritance') def get_discussion_cache_key(course_id): - return 'discussion_{0}'.format(course_id) + return 'discussion_items_{0}'.format(course_id) def get_discussion_cache_entry(modulestore, course_id): @@ -16,10 +16,7 @@ def get_discussion_cache_entry(modulestore, course_id): if cache is not None: cache_entry = cache.get(get_discussion_cache_key(course_id), None) - if cache_entry is not None: - delta = datetime.now() - cache_entry.get('timestamp', datetime.min) - if delta > Timedelta(0,300): - cache_entry = None + # @todo: add expiry here if cache_entry is None: cache_entry = generate_discussion_cache_entry(modulestore, course_id) @@ -33,9 +30,11 @@ def generate_discussion_cache_entry(modulestore, course_id): course_id=course_id) cache = _get_discussion_cache() + entry = {'modules': all_discussion_modules, 'timestamp': datetime.now()} + logging.debug('**** entry = {0}'.format(entry)) if cache is not None: - cache.set(get_discussion_cache_key(course_id), {'modules': all_discussion_modules, 'timestamp': datetime.now()}) - return all_discussion_modules + cache.set(get_discussion_cache_key(course_id), entry) + return entry def modulestore_update_signal_handler(modulestore = None, course_id = None, location = None, **kwargs): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index fbd0b29eca..3515760b98 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -155,7 +155,8 @@ def initialize_discussion_info(course): unexpanded_category_map = defaultdict(list) # get all discussion models within this course_id - all_modules = get_discussion_cache_entry(modulestore(), course_id) + all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course, + 'discussion', None], course_id=course_id) for module in all_modules: skip_module = False From 7d22d48e14cdcc24e9f4d1308be4a9bfb443f5df Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 21 Mar 2013 15:52:48 -0400 Subject: [PATCH 06/14] disable the discussion cache update signal until we get solution for pickling --- common/djangoapps/cache_toolbox/discussion_cache.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/cache_toolbox/discussion_cache.py b/common/djangoapps/cache_toolbox/discussion_cache.py index bac41d6c16..cf5495e6bf 100644 --- a/common/djangoapps/cache_toolbox/discussion_cache.py +++ b/common/djangoapps/cache_toolbox/discussion_cache.py @@ -25,13 +25,15 @@ def get_discussion_cache_entry(modulestore, course_id): def generate_discussion_cache_entry(modulestore, course_id): + # make this a NOP for now. We have to figure out how to pickle the result set + return + components = course_id.split('/') all_discussion_modules = modulestore.get_items(['i4x', components[0], components[1], 'discussion', None], course_id=course_id) cache = _get_discussion_cache() entry = {'modules': all_discussion_modules, 'timestamp': datetime.now()} - logging.debug('**** entry = {0}'.format(entry)) if cache is not None: cache.set(get_discussion_cache_key(course_id), entry) return entry @@ -40,6 +42,7 @@ def generate_discussion_cache_entry(modulestore, course_id): def modulestore_update_signal_handler(modulestore = None, course_id = None, location = None, **kwargs): """called when there is an write event in our modulestore """ + return if location.category == 'discussion': logging.debug('******* got modulestore update signal. Regenerating discussion cache for {0}'.format(course_id)) # refresh the cache entry if we've changed a discussion module From cdc353b61c6eccad59784010e3e548f1a69a7537 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 28 Mar 2013 14:07:13 -0400 Subject: [PATCH 07/14] add back dropped import --- cms/one_time_startup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py index bc8f9c91dd..c2cf90dfac 100644 --- a/cms/one_time_startup.py +++ b/cms/one_time_startup.py @@ -3,6 +3,7 @@ from django.conf import settings from xmodule.modulestore.django import modulestore from cache_toolbox.discussion_cache import discussion_cache_register_for_updates from django.dispatch import Signal +from request_cache.middleware import RequestCache from django.core.cache import get_cache, InvalidCacheBackendError From 02f97858f63ac99f8a7a6dedf6b1f2033000c202 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 29 Mar 2013 16:32:53 -0400 Subject: [PATCH 08/14] move the provisioning seeding from LMS into Common. Add provisioning seeding calling from create new course view. --- cms/djangoapps/contentstore/views.py | 4 +++ .../commands/seed_permissions_roles.py | 25 ++----------------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 561708c833..1a1016cea9 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -14,6 +14,7 @@ from xmodule.modulestore.xml_exporter import export_to_xml from tempfile import mkdtemp from django.core.servers.basehttp import FileWrapper from django.core.files.temp import NamedTemporaryFile +from util.forum_util import seed_permissions_roles # to install PIL on MacOSX: 'easy_install http://dist.repoze.org/PIL-1.1.6.tar.gz' from PIL import Image @@ -1477,6 +1478,9 @@ def create_new_course(request): create_all_course_groups(request.user, new_course.location) + # provision the forum + seed_permissions_roles(new_course.location.course_id) + return HttpResponse(json.dumps({'id': new_course.location.url()})) diff --git a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py index 6a31e73af3..cb14bb7bb1 100644 --- a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py +++ b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py @@ -1,5 +1,5 @@ from django.core.management.base import BaseCommand, CommandError -from django_comment_client.models import Permission, Role +from util.forum_util import seed_permissions_roles class Command(BaseCommand): @@ -12,26 +12,5 @@ class Command(BaseCommand): if len(args) > 1: raise CommandError("Too many arguments") course_id = args[0] - administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] - moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] - community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] - student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] - for per in ["vote", "update_thread", "follow_thread", "unfollow_thread", - "update_comment", "create_sub_comment", "unvote", "create_thread", - "follow_commentable", "unfollow_commentable", "create_comment", ]: - student_role.add_permission(per) - - for per in ["edit_content", "delete_thread", "openclose_thread", - "endorse_comment", "delete_comment", "see_all_cohorts"]: - moderator_role.add_permission(per) - - for per in ["manage_moderator"]: - administrator_role.add_permission(per) - - moderator_role.inherit_permissions(student_role) - - # For now, Community TA == Moderator, except for the styling. - community_ta_role.inherit_permissions(moderator_role) - - administrator_role.inherit_permissions(moderator_role) + seed_permissions_roles(course_id) From e717244611933a03de95f6e3137995ca9ac73bdc Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 5 Apr 2013 15:27:50 -0400 Subject: [PATCH 09/14] remove discussion cache, which isn't used --- cms/one_time_startup.py | 9 +--- .../cache_toolbox/discussion_cache.py | 54 ------------------- common/lib/xmodule/xmodule/editing_module.py | 6 ++- .../lib/xmodule/xmodule/modulestore/mongo.py | 16 +++--- lms/djangoapps/django_comment_client/utils.py | 1 - 5 files changed, 12 insertions(+), 74 deletions(-) delete mode 100644 common/djangoapps/cache_toolbox/discussion_cache.py diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py index c2cf90dfac..5cc74a8fd7 100644 --- a/cms/one_time_startup.py +++ b/cms/one_time_startup.py @@ -1,11 +1,10 @@ from dogapi import dog_http_api, dog_stats_api from django.conf import settings from xmodule.modulestore.django import modulestore -from cache_toolbox.discussion_cache import discussion_cache_register_for_updates from django.dispatch import Signal from request_cache.middleware import RequestCache -from django.core.cache import get_cache, InvalidCacheBackendError +from django.core.cache import get_cache cache = get_cache('mongo_metadata_inheritance') for store_name in settings.MODULESTORE: @@ -13,12 +12,8 @@ for store_name in settings.MODULESTORE: store.metadata_inheritance_cache_subsystem = cache store.request_cache = RequestCache.get_request_cache() - modulestore_update_signal = Signal( - providing_args=['modulestore', 'course_id', 'location'] - ) + modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location']) store.modulestore_update_signal = modulestore_update_signal - discussion_cache_register_for_updates(store) - if hasattr(settings, 'DATADOG_API'): dog_http_api.api_key = settings.DATADOG_API dog_stats_api.start(api_key=settings.DATADOG_API, statsd=True) diff --git a/common/djangoapps/cache_toolbox/discussion_cache.py b/common/djangoapps/cache_toolbox/discussion_cache.py deleted file mode 100644 index cf5495e6bf..0000000000 --- a/common/djangoapps/cache_toolbox/discussion_cache.py +++ /dev/null @@ -1,54 +0,0 @@ -import logging -from django.core.cache import cache, get_cache -from datetime import datetime - -def _get_discussion_cache(): - return get_cache('mongo_metadata_inheritance') - - -def get_discussion_cache_key(course_id): - return 'discussion_items_{0}'.format(course_id) - - -def get_discussion_cache_entry(modulestore, course_id): - cache_entry = None - cache = _get_discussion_cache() - - if cache is not None: - cache_entry = cache.get(get_discussion_cache_key(course_id), None) - # @todo: add expiry here - - if cache_entry is None: - cache_entry = generate_discussion_cache_entry(modulestore, course_id) - - return cache_entry.get('modules',[]) - - -def generate_discussion_cache_entry(modulestore, course_id): - # make this a NOP for now. We have to figure out how to pickle the result set - return - - components = course_id.split('/') - all_discussion_modules = modulestore.get_items(['i4x', components[0], components[1], 'discussion', None], - course_id=course_id) - - cache = _get_discussion_cache() - entry = {'modules': all_discussion_modules, 'timestamp': datetime.now()} - if cache is not None: - cache.set(get_discussion_cache_key(course_id), entry) - return entry - - -def modulestore_update_signal_handler(modulestore = None, course_id = None, location = None, **kwargs): - """called when there is an write event in our modulestore - """ - return - if location.category == 'discussion': - logging.debug('******* got modulestore update signal. Regenerating discussion cache for {0}'.format(course_id)) - # refresh the cache entry if we've changed a discussion module - generate_discussion_cache_entry(modulestore, course_id) - - -def discussion_cache_register_for_updates(modulestore): - if modulestore.modulestore_update_signal is not None: - modulestore.modulestore_update_signal.connect(modulestore_update_signal_handler) \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index 1f07861ae8..df4ebc5646 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -40,10 +40,11 @@ class XMLEditingDescriptor(EditingDescriptor): js = {'coffee': [resource_string(__name__, 'js/src/raw/edit/xml.coffee')]} js_module_name = "XMLEditingDescriptor" + class MetadataOnlyEditingDescriptor(EditingDescriptor): """ - Module that provides a raw editing view of its data as XML. It does not perform - any validation of its definition + Module which only provides an editing interface for the metadata, it does + not expose a UI for editing the module data """ js = {'coffee': [resource_string(__name__, 'js/src/raw/edit/metadata-only.coffee')]} @@ -51,6 +52,7 @@ class MetadataOnlyEditingDescriptor(EditingDescriptor): mako_template = "widgets/metadata-only-edit.html" + class JSONEditingDescriptor(EditingDescriptor): """ Module that provides a raw editing view of its data as XML. It does not perform diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index ae03775963..1bc5209736 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -329,7 +329,7 @@ class MongoModuleStore(ModuleStoreBase): ''' key = metadata_cache_key(location) tree = {} - + if not force_refresh: # see if we are first in the request cache (if present) if self.request_cache is not None and key in self.request_cache.data.get('metadata_inheritance', {}): @@ -344,7 +344,7 @@ class MongoModuleStore(ModuleStoreBase): if not tree: # if not in subsystem, or we are on force refresh, then we have to compute tree = self.compute_metadata_inheritance_tree(location) - + # now write out computed tree to caching subsystem (e.g. memcached), if available if self.metadata_inheritance_cache_subsystem is not None: self.metadata_inheritance_cache_subsystem.set(key, tree) @@ -578,11 +578,8 @@ class MongoModuleStore(ModuleStoreBase): def fire_updated_modulestore_signal(self, course_id, location): if self.modulestore_update_signal is not None: - self.modulestore_update_signal.send(None, - modulestore = self, - course_id = course_id, - location = location - ) + self.modulestore_update_signal.send(self, modulestore=self, course_id=course_id, + location=location) def get_course_for_item(self, location, depth=0): ''' @@ -682,8 +679,7 @@ class MongoModuleStore(ModuleStoreBase): self._update_single_item(location, {'metadata': metadata}) # recompute (and update) the metadata inheritance tree which is cached self.refresh_cached_metadata_inheritance_tree(loc) - self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) - + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) def delete_item(self, location): """ @@ -707,7 +703,7 @@ class MongoModuleStore(ModuleStoreBase): safe=self.collection.safe) # recompute (and update) the metadata inheritance tree which is cached self.refresh_cached_metadata_inheritance_tree(Location(location)) - self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) + self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) def get_parent_locations(self, location, course_id): '''Find all locations that are the parents of this location in this diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 3515760b98..9bfb9a9d0d 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -16,7 +16,6 @@ from django.utils import simplejson from django_comment_client.models import Role from django_comment_client.permissions import check_permissions_by_view from xmodule.modulestore.exceptions import NoPathToItem -from cache_toolbox.discussion_cache import get_discussion_cache_entry from mitxmako import middleware import pystache_custom as pystache From 0781f3e166ce8c5a813332463a9bf30b8542a1fc Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 5 Apr 2013 15:28:19 -0400 Subject: [PATCH 10/14] add new file --- common/djangoapps/util/forum_util.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 common/djangoapps/util/forum_util.py diff --git a/common/djangoapps/util/forum_util.py b/common/djangoapps/util/forum_util.py new file mode 100644 index 0000000000..5871580849 --- /dev/null +++ b/common/djangoapps/util/forum_util.py @@ -0,0 +1,26 @@ +from django_comment_client.models import Permission, Role + +def seed_permissions_roles(course_id): + administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] + moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] + community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] + student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] + + for per in ["vote", "update_thread", "follow_thread", "unfollow_thread", + "update_comment", "create_sub_comment", "unvote", "create_thread", + "follow_commentable", "unfollow_commentable", "create_comment", ]: + student_role.add_permission(per) + + for per in ["edit_content", "delete_thread", "openclose_thread", + "endorse_comment", "delete_comment", "see_all_cohorts"]: + moderator_role.add_permission(per) + + for per in ["manage_moderator"]: + administrator_role.add_permission(per) + + moderator_role.inherit_permissions(student_role) + + # For now, Community TA == Moderator, except for the styling. + community_ta_role.inherit_permissions(moderator_role) + + administrator_role.inherit_permissions(moderator_role) \ No newline at end of file From a3de4ff625f0045dbfab1eee11a65ba06a214a6a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 5 Apr 2013 15:59:01 -0400 Subject: [PATCH 11/14] add unit test to confirm 'id' generation --- .../contentstore/tests/test_contentstore.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 49a609a879..fd3165f4a6 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -578,6 +578,19 @@ class ContentStoreTest(ModuleStoreTestCase): # make sure we found the item (e.g. it didn't error while loading) self.assertTrue(did_load_item) + def test_forum_id_generation(self): + import_from_xml(modulestore(), 'common/test/data/', ['full']) + module_store = modulestore('direct') + new_component_location = Location('i4x', 'edX', 'full', 'discussion', 'new_component') + source_template_location = Location('i4x', 'edx', 'templates', 'discussion', 'Discussion_Tag') + + # crate a new module and add it as a child to a vertical + module_store.clone_item(source_template_location, new_component_location) + + new_discussion_item = module_store.get_item(new_component_location) + + self.assertNotEquals(new_discussion_item.discussion_id, '$$GUID$$') + def test_metadata_inheritance(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) From 7c48f9a5ed262dcba2dd777b115444b3a9782971 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sat, 6 Apr 2013 10:43:40 -0400 Subject: [PATCH 12/14] add unit test to verify that the store updated signal actually fired. Also fix bug where cloning item doesn't update the inherited metadata caches --- .../contentstore/tests/test_contentstore.py | 27 +++++++++++++++++++ .../lib/xmodule/xmodule/modulestore/mongo.py | 4 ++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index fd3165f4a6..b9406c288c 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -13,6 +13,7 @@ import copy from json import loads from django.contrib.auth.models import User +from django.dispatch import Signal from contentstore.utils import get_modulestore from .utils import ModuleStoreTestCase, parse_json @@ -591,6 +592,32 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertNotEquals(new_discussion_item.discussion_id, '$$GUID$$') + def test_update_modulestore_signal_did_fire(self): + + import_from_xml(modulestore(), 'common/test/data/', ['full']) + module_store = modulestore('direct') + + try: + module_store.modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location']) + + self.got_signal = False + + def _signal_hander(modulestore=None, course_id=None, location=None, **kwargs): + self.got_signal = True + + module_store.modulestore_update_signal.connect(_signal_hander) + + new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component') + source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page') + + # crate a new module + module_store.clone_item(source_template_location, new_component_location) + + finally: + module_store.modulestore_update_signal = None + + self.assertTrue(self.got_signal) + def test_metadata_inheritance(self): import_from_xml(modulestore(), 'common/test/data/', ['full']) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 1bc5209736..d584438efe 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -537,6 +537,7 @@ class MongoModuleStore(ModuleStoreBase): Clone a new item that is a copy of the item at the location `source` and writes it to `location` """ + item = None try: source_item = self.collection.find_one(location_to_query(source)) @@ -568,7 +569,6 @@ class MongoModuleStore(ModuleStoreBase): course.tabs = existing_tabs self.update_metadata(course.location, course._model_data._kvs._metadata) - return item except pymongo.errors.DuplicateKeyError: raise DuplicateItemError(location) @@ -576,6 +576,8 @@ class MongoModuleStore(ModuleStoreBase): self.refresh_cached_metadata_inheritance_tree(Location(location)) self.fire_updated_modulestore_signal(get_course_id_no_run(Location(location)), Location(location)) + return item + def fire_updated_modulestore_signal(self, course_id, location): if self.modulestore_update_signal is not None: self.modulestore_update_signal.send(self, modulestore=self, course_id=course_id, From 5e3ca48c8dc6d392d10bb94cfbe8d6200e43ff0a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 10 Apr 2013 09:40:23 -0400 Subject: [PATCH 13/14] address some violations --- common/djangoapps/util/forum_util.py | 11 ++++++----- common/lib/xmodule/xmodule/raw_module.py | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/common/djangoapps/util/forum_util.py b/common/djangoapps/util/forum_util.py index 5871580849..c926a9fe3d 100644 --- a/common/djangoapps/util/forum_util.py +++ b/common/djangoapps/util/forum_util.py @@ -1,4 +1,5 @@ -from django_comment_client.models import Permission, Role +from django_comment_client.models import Role + def seed_permissions_roles(course_id): administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] @@ -7,12 +8,12 @@ def seed_permissions_roles(course_id): student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] for per in ["vote", "update_thread", "follow_thread", "unfollow_thread", - "update_comment", "create_sub_comment", "unvote", "create_thread", - "follow_commentable", "unfollow_commentable", "create_comment", ]: + "update_comment", "create_sub_comment", "unvote", "create_thread", + "follow_commentable", "unfollow_commentable", "create_comment", ]: student_role.add_permission(per) for per in ["edit_content", "delete_thread", "openclose_thread", - "endorse_comment", "delete_comment", "see_all_cohorts"]: + "endorse_comment", "delete_comment", "see_all_cohorts"]: moderator_role.add_permission(per) for per in ["manage_moderator"]: @@ -23,4 +24,4 @@ def seed_permissions_roles(course_id): # For now, Community TA == Moderator, except for the styling. community_ta_role.inherit_permissions(moderator_role) - administrator_role.inherit_permissions(moderator_role) \ No newline at end of file + administrator_role.inherit_permissions(moderator_role) diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index 6b5c2441be..554be73926 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -1,5 +1,5 @@ from lxml import etree -from xmodule.editing_module import XMLEditingDescriptor, MetadataOnlyEditingDescriptor +from xmodule.editing_module import XMLEditingDescriptor from xmodule.xml_module import XmlDescriptor import logging import sys @@ -29,6 +29,6 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor): line, offset = err.position msg = ("Unable to create xml for problem {loc}. " "Context: '{context}'".format( - context=lines[line - 1][offset - 40:offset + 40], - loc=self.location)) + context=lines[line - 1][offset - 40:offset + 40], + loc=self.location)) raise Exception, msg, sys.exc_info()[2] From e0bc823365da26ee87163af2fc1f9817f7b28cf0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 11 Apr 2013 10:42:03 -0400 Subject: [PATCH 14/14] remove auto-provisioning because of a dependency problem between cms and lms --- cms/djangoapps/contentstore/views.py | 4 --- common/djangoapps/util/forum_util.py | 27 ------------------- .../commands/seed_permissions_roles.py | 26 ++++++++++++++++-- 3 files changed, 24 insertions(+), 33 deletions(-) delete mode 100644 common/djangoapps/util/forum_util.py diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 8e1d2e5bfa..227379979e 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -13,7 +13,6 @@ from xmodule.modulestore.xml_exporter import export_to_xml from tempfile import mkdtemp from django.core.servers.basehttp import FileWrapper from django.core.files.temp import NamedTemporaryFile -from util.forum_util import seed_permissions_roles # to install PIL on MacOSX: 'easy_install http://dist.repoze.org/PIL-1.1.6.tar.gz' from PIL import Image @@ -1515,9 +1514,6 @@ def create_new_course(request): create_all_course_groups(request.user, new_course.location) - # provision the forum - seed_permissions_roles(new_course.location.course_id) - return HttpResponse(json.dumps({'id': new_course.location.url()})) diff --git a/common/djangoapps/util/forum_util.py b/common/djangoapps/util/forum_util.py deleted file mode 100644 index c926a9fe3d..0000000000 --- a/common/djangoapps/util/forum_util.py +++ /dev/null @@ -1,27 +0,0 @@ -from django_comment_client.models import Role - - -def seed_permissions_roles(course_id): - administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] - moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] - community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] - student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] - - for per in ["vote", "update_thread", "follow_thread", "unfollow_thread", - "update_comment", "create_sub_comment", "unvote", "create_thread", - "follow_commentable", "unfollow_commentable", "create_comment", ]: - student_role.add_permission(per) - - for per in ["edit_content", "delete_thread", "openclose_thread", - "endorse_comment", "delete_comment", "see_all_cohorts"]: - moderator_role.add_permission(per) - - for per in ["manage_moderator"]: - administrator_role.add_permission(per) - - moderator_role.inherit_permissions(student_role) - - # For now, Community TA == Moderator, except for the styling. - community_ta_role.inherit_permissions(moderator_role) - - administrator_role.inherit_permissions(moderator_role) diff --git a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py index cb14bb7bb1..9d6eefd11d 100644 --- a/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py +++ b/lms/djangoapps/django_comment_client/management/commands/seed_permissions_roles.py @@ -1,5 +1,5 @@ from django.core.management.base import BaseCommand, CommandError -from util.forum_util import seed_permissions_roles +from django_comment_client.models import Role class Command(BaseCommand): @@ -13,4 +13,26 @@ class Command(BaseCommand): raise CommandError("Too many arguments") course_id = args[0] - seed_permissions_roles(course_id) + administrator_role = Role.objects.get_or_create(name="Administrator", course_id=course_id)[0] + moderator_role = Role.objects.get_or_create(name="Moderator", course_id=course_id)[0] + community_ta_role = Role.objects.get_or_create(name="Community TA", course_id=course_id)[0] + student_role = Role.objects.get_or_create(name="Student", course_id=course_id)[0] + + for per in ["vote", "update_thread", "follow_thread", "unfollow_thread", + "update_comment", "create_sub_comment", "unvote", "create_thread", + "follow_commentable", "unfollow_commentable", "create_comment", ]: + student_role.add_permission(per) + + for per in ["edit_content", "delete_thread", "openclose_thread", + "endorse_comment", "delete_comment", "see_all_cohorts"]: + moderator_role.add_permission(per) + + for per in ["manage_moderator"]: + administrator_role.add_permission(per) + + moderator_role.inherit_permissions(student_role) + + # For now, Community TA == Moderator, except for the styling. + community_ta_role.inherit_permissions(moderator_role) + + administrator_role.inherit_permissions(moderator_role)