From d77491e46a11b913a31616b4f9adc035c53cc3c6 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Fri, 6 Sep 2013 14:21:30 -0400
Subject: [PATCH] Pull XModule attributes out into a mixin that can be applied to xblocks
---
cms/envs/common.py | 3 ++-
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py | 2 --
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py | 3 ++-
common/lib/xmodule/xmodule/tests/__init__.py | 8 ++------
common/lib/xmodule/xmodule/tests/test_import.py | 3 ++-
common/lib/xmodule/xmodule/tests/test_xml_module.py | 6 +++---
common/lib/xmodule/xmodule/x_module.py | 325 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
lms/envs/common.py | 3 ++-
8 files changed, 148 insertions(+), 205 deletions(-)
diff --git a/cms/envs/common.py b/cms/envs/common.py
index 7f2559e..13faf55 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -31,6 +31,7 @@ from path import path
from lms.xblock.mixin import LmsBlockMixin
from cms.xmodule_namespace import CmsBlockMixin
from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
############################ FEATURE CONFIGURATION #############################
@@ -168,7 +169,7 @@ MIDDLEWARE_CLASSES = (
# This should be moved into an XBlock Runtime/Application object
# once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, CmsBlockMixin, InheritanceMixin)
+XBLOCK_MIXINS = (LmsBlockMixin, CmsBlockMixin, InheritanceMixin, XModuleMixin)
############################ SIGNAL HANDLERS ################################
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index d85092b..60bfa66 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -43,8 +43,6 @@ log = logging.getLogger(__name__)
#==============================================================================
-
-
class SplitMongoModuleStore(ModuleStoreBase):
"""
A Mongodb backed ModuleStore supporting versions, inheritance,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
index 0f1b65d..be5c61a 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
@@ -15,6 +15,7 @@ from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemN
DuplicateItemError
from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, VersionTree, DescriptionLocator
from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
from pytz import UTC
from path import path
import re
@@ -34,7 +35,7 @@ class SplitModuleTest(unittest.TestCase):
'db': 'test_xmodule',
'collection': 'modulestore{0}'.format(uuid.uuid4().hex),
'fs_root': '',
- 'xblock_mixins': (InheritanceMixin,)
+ 'xblock_mixins': (InheritanceMixin, XModuleMixin)
}
MODULESTORE = {
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index 088b2c3..99caec0 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -11,15 +11,11 @@ import json
import os
import unittest
-import fs
-import fs.osfs
-import numpy
from mock import Mock
from path import path
-import calc
from xblock.field_data import DictFieldData
-from xmodule.x_module import ModuleSystem, XModuleDescriptor, DescriptorSystem
+from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin
from xmodule.modulestore.inheritance import InheritanceMixin
from xmodule.mako_module import MakoDescriptorSystem
@@ -81,7 +77,7 @@ def get_test_descriptor_system():
resources_fs=Mock(),
error_tracker=Mock(),
render_template=lambda template, context: repr(context),
- mixins=(InheritanceMixin,),
+ mixins=(InheritanceMixin, XModuleMixin),
)
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 19232e3..16c8275 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -13,6 +13,7 @@ from xmodule.xml_module import is_pointer_tag
from xmodule.modulestore import Location
from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
from xmodule.modulestore.inheritance import compute_inherited_metadata
+from xmodule.x_module import XModuleMixin
from xmodule.fields import Date
from xmodule.tests import DATA_DIR
from xmodule.modulestore.inheritance import InheritanceMixin
@@ -42,7 +43,7 @@ class DummySystem(ImportSystem):
error_tracker=error_tracker,
parent_tracker=parent_tracker,
load_error_modules=load_error_modules,
- mixins=(InheritanceMixin,)
+ mixins=(InheritanceMixin, XModuleMixin)
)
def render_template(self, _template, _context):
diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py
index 9d11b2b..da6062f 100644
--- a/common/lib/xmodule/xmodule/tests/test_xml_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py
@@ -12,10 +12,10 @@ from xblock.runtime import DbModel
from xmodule.fields import Date, Timedelta
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
-from xmodule.x_module import XModuleFields
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
from xmodule.course_module import CourseDescriptor
from xmodule.seq_module import SequenceDescriptor
+from xmodule.x_module import XModuleMixin
from xmodule.tests import get_test_descriptor_system
from xmodule.tests.xml import XModuleXmlImportTest
@@ -65,7 +65,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
# Also tests that xml_attributes is filtered out of XmlDescriptor.
self.assertEqual(1, len(editable_fields), editable_fields)
self.assert_field_values(
- editable_fields, 'display_name', XModuleFields.display_name,
+ editable_fields, 'display_name', XModuleMixin.display_name,
explicitly_set=False, value=None, default_value=None
)
@@ -73,7 +73,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
# Tests that explicitly_set is correct when a value overrides the default (not inheritable).
editable_fields = self.get_xml_editable_fields(DictFieldData({'display_name': 'foo'}))
self.assert_field_values(
- editable_fields, 'display_name', XModuleFields.display_name,
+ editable_fields, 'display_name', XModuleMixin.display_name,
explicitly_set=True, value='foo', default_value=None
)
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 63a73c4..63ff80b 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -1,5 +1,4 @@
import logging
-import copy
import yaml
import os
@@ -11,7 +10,7 @@ from xmodule.modulestore import Location
from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError
from xblock.core import XBlock
-from xblock.fields import Scope, String, Integer, Float, List
+from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String
from xblock.fragment import Fragment
from xblock.runtime import Runtime
from xmodule.modulestore.locator import BlockUsageLocator
@@ -83,7 +82,13 @@ class HTMLSnippet(object):
.format(self.__class__))
-class XModuleFields(object):
+class XModuleMixin(XBlockMixin):
+ """
+ Fields and methods used by XModules internally.
+
+ Adding this Mixin to an :class:`XBlock` allows it to cooperate with old-style :class:`XModules`
+ """
+
display_name = String(
display_name="Display Name",
help="This name appears in the horizontal navigation at the top of the page.",
@@ -93,41 +98,6 @@ class XModuleFields(object):
default=None
)
-
-class XModule(XModuleFields, HTMLSnippet, XBlock):
- ''' Implements a generic learning module.
-
- Subclasses must at a minimum provide a definition for get_html in order
- to be displayed to users.
-
- See the HTML module for a simple example.
- '''
-
- # The default implementation of get_icon_class returns the icon_class
- # attribute of the class
- #
- # This attribute can be overridden by subclasses, and
- # the function can also be overridden if the icon class depends on the data
- # in the module
- icon_class = 'other'
-
-
- def __init__(self, descriptor, *args, **kwargs):
- '''
- Construct a new xmodule
-
- runtime: An XBlock runtime allowing access to external resources
-
- descriptor: the XModuleDescriptor that this module is an instance of.
-
- field_data: A dictionary-like object that maps field names to values
- for those fields.
- '''
- super(XModule, self).__init__(*args, **kwargs)
- self.system = self.runtime
- self.descriptor = descriptor
- self._loaded_children = None
-
@property
def id(self):
return self.location.url()
@@ -155,31 +125,133 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
@property
def url_name(self):
- if self.descriptor:
- return self.descriptor.url_name
- elif isinstance(self.location, Location):
+ if isinstance(self.location, Location):
return self.location.name
elif isinstance(self.location, BlockUsageLocator):
return self.location.usage_id
else:
raise InsufficientSpecificationError()
-
@property
def display_name_with_default(self):
- '''
+ """
Return a display name for the module: use display_name if defined in
metadata, otherwise convert the url name.
- '''
+ """
name = self.display_name
if name is None:
name = self.url_name.replace('_', ' ')
return name
+ def get_explicitly_set_fields_by_scope(self, scope=Scope.content):
+ """
+ Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including
+ any set to None.)
+ """
+ result = {}
+ for field in self.fields.values():
+ if (field.scope == scope and field.is_set_on(self)):
+ result[field.name] = field.read_json(self)
+ return result
+
+ @property
+ def xblock_kvs(self):
+ """
+ Use w/ caution. Really intended for use by the persistence layer.
+ """
+ # if caller wants kvs, caller's assuming it's up to date; so, decache it
+ self.save()
+ return self._field_data._kvs # pylint: disable=protected-access
+
def get_children(self):
- '''
+ """Returns a list of XBlock instances for the children of
+ this module"""
+
+ if not self.has_children:
+ return []
+
+ if getattr(self, '_child_instances', None) is None:
+ self._child_instances = [] # pylint: disable=attribute-defined-outside-init
+ for child_loc in self.children:
+ try:
+ child = self.runtime.get_block(child_loc)
+ except ItemNotFoundError:
+ log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc))
+ continue
+ self._child_instances.append(child)
+
+ return self._child_instances
+
+ def get_required_module_descriptors(self):
+ """Returns a list of XModuleDescriptor instances upon which this module depends, but are
+ not children of this module"""
+ return []
+
+ def get_display_items(self):
+ """
+ Returns a list of descendent module instances that will display
+ immediately inside this module.
+ """
+ items = []
+ for child in self.get_children():
+ items.extend(child.displayable_items())
+
+ return items
+
+ def displayable_items(self):
+ """
+ Returns list of displayable modules contained by this module. If this
+ module is visible, should return [self].
+ """
+ return [self]
+
+ def get_child_by(self, selector):
+ """
+ Return a child XBlock that matches the specified selector
+ """
+ for child in self.get_children():
+ if selector(child):
+ return child
+ return None
+
+
+class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-method
+ """ Implements a generic learning module.
+
+ Subclasses must at a minimum provide a definition for get_html in order
+ to be displayed to users.
+
+ See the HTML module for a simple example.
+ """
+
+ # The default implementation of get_icon_class returns the icon_class
+ # attribute of the class
+ #
+ # This attribute can be overridden by subclasses, and
+ # the function can also be overridden if the icon class depends on the data
+ # in the module
+ icon_class = 'other'
+
+ def __init__(self, descriptor, *args, **kwargs):
+ """
+ Construct a new xmodule
+
+ runtime: An XBlock runtime allowing access to external resources
+
+ descriptor: the XModuleDescriptor that this module is an instance of.
+
+ field_data: A dictionary-like object that maps field names to values
+ for those fields.
+ """
+ super(XModule, self).__init__(*args, **kwargs)
+ self.system = self.runtime
+ self.descriptor = descriptor
+ self._loaded_children = None
+
+ def get_children(self):
+ """
Return module instances for all the children of this module.
- '''
+ """
if self._loaded_children is None:
child_descriptors = self.get_child_descriptors()
@@ -200,7 +272,7 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
return '<x_module(id={0})>'.format(self.id)
def get_child_descriptors(self):
- '''
+ """
Returns the descriptors of the child modules
Overriding this changes the behavior of get_children and
@@ -211,40 +283,13 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
These children will be the same children returned by the
descriptor unless descriptor.has_dynamic_children() is true.
- '''
- return self.descriptor.get_children()
-
- def get_child_by(self, selector):
"""
- Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise.
- """
- for child in self.get_children():
- if selector(child):
- return child
- return None
-
- def get_display_items(self):
- '''
- Returns a list of descendent module instances that will display
- immediately inside this module.
- '''
- items = []
- for child in self.get_children():
- items.extend(child.displayable_items())
-
- return items
-
- def displayable_items(self):
- '''
- Returns list of displayable modules contained by this module. If this
- module is visible, should return [self].
- '''
- return [self]
+ return self.descriptor.get_children()
def get_icon_class(self):
- '''
+ """
Return a css class identifying this module in the context of an icon
- '''
+ """
return self.icon_class
# Functions used in the LMS
@@ -267,7 +312,7 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
return None
def max_score(self):
- ''' Maximum score. Two notes:
+ """ Maximum score. Two notes:
* This is generic; in abstract, a problem could be 3/5 points on one
randomization, and 5/7 on another
@@ -275,22 +320,22 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
* In practice, this is a Very Bad Idea, and (a) will break some code
in place (although that code should get fixed), and (b) break some
analytics we plan to put in place.
- '''
+ """
return None
def get_progress(self):
- ''' Return a progress.Progress object that represents how far the
+ """ Return a progress.Progress object that represents how far the
student has gone in this module. Must be implemented to get correct
progress tracking behavior in nesting modules like sequence and
vertical.
If this module has no notion of progress, return None.
- '''
+ """
return None
def handle_ajax(self, _dispatch, _data):
- ''' dispatch is last part of the URL.
- data is a dictionary-like object with the content of the request'''
+ """ dispatch is last part of the URL.
+ data is a dictionary-like object with the content of the request"""
return ""
@@ -381,7 +426,7 @@ class ResourceTemplates(object):
return None
-class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
+class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
"""
An XModuleDescriptor is a specification for an element of a course. This
could be a problem, an organizational element (a group of content), or a
@@ -446,86 +491,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
self.edited_by = self.edited_on = self.previous_version = self.update_version = self.definition_locator = None
self._child_instances = None
- @property
- def id(self):
- return self.location.url()
-
- @property
- def category(self):
- return self.scope_ids.block_type
-
- @property
- def location(self):
- try:
- return Location(self.scope_ids.usage_id)
- except InvalidLocationError:
- if isinstance(self.scope_ids.usage_id, BlockUsageLocator):
- return self.scope_ids.usage_id
- else:
- return BlockUsageLocator(self.scope_ids.usage_id)
-
- @location.setter
- def location(self, value):
- self.scope_ids = self.scope_ids._replace(
- def_id=value,
- usage_id=value,
- )
-
- @property
- def url_name(self):
- if isinstance(self.location, Location):
- return self.location.name
- elif isinstance(self.location, BlockUsageLocator):
- return self.location.usage_id
- else:
- raise InsufficientSpecificationError()
-
-
- @property
- def display_name_with_default(self):
- '''
- Return a display name for the module: use display_name if defined in
- metadata, otherwise convert the url name.
- '''
- name = self.display_name
- if name is None:
- name = self.url_name.replace('_', ' ')
- return name
-
- def get_required_module_descriptors(self):
- """Returns a list of XModuleDescritpor instances upon which this module depends, but are
- not children of this module"""
- return []
-
- def get_children(self):
- """Returns a list of XModuleDescriptor instances for the children of
- this module"""
- if not self.has_children:
- return []
-
- if self._child_instances is None:
- self._child_instances = []
- for child_loc in self.children:
- if isinstance(child_loc, XModuleDescriptor):
- child = child_loc
- else:
- try:
- child = self.runtime.get_block(child_loc)
- except ItemNotFoundError:
- log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc))
- continue
- self._child_instances.append(child)
-
- return self._child_instances
-
- def get_child_by(self, selector):
- """
- Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise.
- """
- for child in self.get_children():
- if selector(child):
- return child
- return None
def xmodule(self, system):
"""
@@ -619,15 +584,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
raise NotImplementedError(
'Modules must implement export_to_xml to enable xml export')
- @property
- def xblock_kvs(self):
- """
- Use w/ caution. Really intended for use by the persistence layer.
- """
- # if caller wants kvs, caller's assuming it's up to date; so, decache it
- self.save()
- return self._field_data._kvs
-
# =============================== BUILTIN METHODS ==========================
def __eq__(self, other):
return (self.scope_ids == other.scope_ids and
@@ -655,17 +611,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
return [XBlock.tags, XBlock.name]
- def get_explicitly_set_fields_by_scope(self, scope=Scope.content):
- """
- Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including
- any set to None.)
- """
- result = {}
- for field in self.fields.values():
- if (field.scope == scope and field.is_set_on(self)):
- result[field.name] = field.read_json(self)
- return result
-
@property
def editable_metadata_fields(self):
"""
@@ -825,7 +770,7 @@ class XMLParsingSystem(DescriptorSystem):
class ModuleSystem(Runtime):
- '''
+ """
This is an abstraction such that x_modules can function independent
of the courseware (e.g. import into other types of courseware, LMS,
or if we want to have a sandbox server for user-contributed content)
@@ -835,7 +780,7 @@ class ModuleSystem(Runtime):
Note that these functions can be closures over e.g. a django request
and user, or other environment-specific info.
- '''
+ """
def __init__(
self, ajax_url, track_function, get_module, render_template,
replace_urls, xmodule_field_data, user=None, filestore=None,
@@ -844,7 +789,7 @@ class ModuleSystem(Runtime):
open_ended_grading_interface=None, s3_interface=None,
cache=None, can_execute_unsafe_code=None, replace_course_urls=None,
replace_jump_to_id_urls=None, **kwargs):
- '''
+ """
Create a closure around the system environment.
ajax_url - the url where ajax calls to the encapsulating module go.
@@ -893,7 +838,7 @@ class ModuleSystem(Runtime):
can_execute_unsafe_code - A function returning a boolean, whether or
not to allow the execution of unsafe, unsandboxed code.
- '''
+ """
super(ModuleSystem, self).__init__(**kwargs)
self.ajax_url = ajax_url
@@ -926,11 +871,11 @@ class ModuleSystem(Runtime):
self.replace_jump_to_id_urls = replace_jump_to_id_urls
def get(self, attr):
- ''' provide uniform access to attributes (like etree).'''
+ """ provide uniform access to attributes (like etree)."""
return self.__dict__.get(attr)
def set(self, attr, val):
- '''provide uniform access to attributes (like etree)'''
+ """provide uniform access to attributes (like etree)"""
self.__dict__[attr] = val
def __repr__(self):
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 941809b..3858b1c 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -32,6 +32,7 @@ from .discussionsettings import *
from lms.xblock.mixin import LmsBlockMixin
from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
################################### FEATURES ###################################
# The display name of the platform to be used in templates/emails/etc.
@@ -363,7 +364,7 @@ CONTENTSTORE = None
# This should be moved into an XBlock Runtime/Application object
# once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin)
+XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin)
#################### Python sandbox ############################################
--
libgit2 0.26.0