diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py
index 3a8e0b0222..a16251fa0f 100644
--- a/cms/djangoapps/contentstore/tests/test_crud.py
+++ b/cms/djangoapps/contentstore/tests/test_crud.py
@@ -218,7 +218,7 @@ class TemplateTests(unittest.TestCase):
)
usage_id = json_data.get('_id', None)
if not '_inherited_settings' in json_data and parent_xblock is not None:
- json_data['_inherited_settings'] = parent_xblock.xblock_kvs.get_inherited_settings().copy()
+ json_data['_inherited_settings'] = parent_xblock.xblock_kvs.inherited_settings.copy()
json_fields = json_data.get('fields', {})
for field_name in inheritance.InheritanceMixin.fields:
if field_name in json_fields:
diff --git a/cms/static/coffee/spec/views/metadata_edit_spec.coffee b/cms/static/coffee/spec/views/metadata_edit_spec.coffee
index b3e4567d82..3d2e8c244b 100644
--- a/cms/static/coffee/spec/views/metadata_edit_spec.coffee
+++ b/cms/static/coffee/spec/views/metadata_edit_spec.coffee
@@ -26,7 +26,6 @@ describe "Test Metadata Editor", ->
explicitly_set: true,
field_name: "display_name",
help: "Specifies the name for this component.",
- inheritable: false,
options: [],
type: CMS.Models.Metadata.GENERIC_TYPE,
value: "Word cloud"
@@ -38,7 +37,6 @@ describe "Test Metadata Editor", ->
explicitly_set: false,
field_name: "show_answer",
help: "When should you show the answer",
- inheritable: true,
options: [
{"display_name": "Always", "value": "always"},
{"display_name": "Answered", "value": "answered"},
@@ -54,7 +52,6 @@ describe "Test Metadata Editor", ->
explicitly_set: false,
field_name: "num_inputs",
help: "Number of text boxes for student to input words/sentences.",
- inheritable: false,
options: {min: 1},
type: CMS.Models.Metadata.INTEGER_TYPE,
value: 5
@@ -66,7 +63,6 @@ describe "Test Metadata Editor", ->
explicitly_set: true,
field_name: "weight",
help: "Weight for this problem",
- inheritable: true,
options: {min: 1.3, max:100.2, step:0.1},
type: CMS.Models.Metadata.FLOAT_TYPE,
value: 10.2
@@ -78,7 +74,6 @@ describe "Test Metadata Editor", ->
explicitly_set: false,
field_name: "list",
help: "A list of things.",
- inheritable: false,
options: [],
type: CMS.Models.Metadata.LIST_TYPE,
value: ["the first display value", "the second"]
@@ -99,7 +94,6 @@ describe "Test Metadata Editor", ->
explicitly_set: true,
field_name: "unknown_type",
help: "Mystery property.",
- inheritable: false,
options: [
{"display_name": "Always", "value": "always"},
{"display_name": "Answered", "value": "answered"},
@@ -145,7 +139,6 @@ describe "Test Metadata Editor", ->
explicitly_set: false,
field_name: "display_name",
help: "",
- inheritable: false,
options: [],
type: CMS.Models.Metadata.GENERIC_TYPE,
value: null
diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py
index a9de875128..78d584580f 100644
--- a/common/lib/xmodule/xmodule/modulestore/inheritance.py
+++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py
@@ -3,6 +3,7 @@ from pytz import UTC
from xblock.fields import Scope, Boolean, String, Float, XBlockMixin
from xmodule.fields import Date, Timedelta
+from xblock.runtime import KeyValueStore
class InheritanceMixin(XBlockMixin):
@@ -51,16 +52,18 @@ def compute_inherited_metadata(descriptor):
NOTE: This means that there is no such thing as lazy loading at the
moment--this accesses all the children."""
- for child in descriptor.get_children():
- inherit_metadata(
- child,
- {
- name: field.read_from(descriptor)
- for name, field in InheritanceMixin.fields.items()
- if field.is_set_on(descriptor)
- }
- )
- compute_inherited_metadata(child)
+ if descriptor.has_children:
+ parent_metadata = descriptor.xblock_kvs.inherited_settings.copy()
+ # add any of descriptor's explicitly set fields to the inheriting list
+ for field in InheritanceMixin.fields.values():
+ # pylint: disable = W0212
+ if descriptor._field_data.has(descriptor, field.name):
+ # inherited_settings values are json repr
+ parent_metadata[field.name] = field.read_json(descriptor)
+
+ for child in descriptor.get_children():
+ inherit_metadata(child, parent_metadata)
+ compute_inherited_metadata(child)
def inherit_metadata(descriptor, inherited_data):
@@ -72,53 +75,46 @@ def inherit_metadata(descriptor, inherited_data):
`inherited_data`: A dictionary mapping field names to the values that
they should inherit
"""
- # The inherited values that are actually being used.
- if not hasattr(descriptor, '_inherited_metadata'):
- setattr(descriptor, '_inherited_metadata', {})
-
- # All inheritable metadata values (for which a value exists in field_data).
- if not hasattr(descriptor, '_inheritable_metadata'):
- setattr(descriptor, '_inheritable_metadata', {})
-
- # Set all inheritable metadata from kwargs that are
- # in self.inheritable_metadata and aren't already set in metadata
- for name, field in InheritanceMixin.fields.items():
- if name not in inherited_data:
- continue
- inherited_value = inherited_data[name]
-
- descriptor._inheritable_metadata[name] = inherited_value
- if not field.is_set_on(descriptor):
- descriptor._inherited_metadata[name] = inherited_value
- field.write_to(descriptor, inherited_value)
- # We've updated the fields on the descriptor, so we need to save it
- descriptor.save()
+ try:
+ descriptor.xblock_kvs.inherited_settings = inherited_data
+ except AttributeError: # the kvs doesn't have inherited_settings probably b/c it's an error module
+ pass
def own_metadata(module):
- # IN SPLIT MONGO this is just ['metadata'] as it keeps ['_inherited_metadata'] separate!
- # FIXME move into kvs? will that work for xml mongo?
"""
Return a dictionary that contains only non-inherited field keys,
mapped to their serialized values
"""
- inherited_metadata = getattr(module, '_inherited_metadata', {})
- metadata = {}
- for name, field in module.fields.items():
- # Only save metadata that wasn't inherited
- if field.scope != Scope.settings:
- continue
+ return module.get_explicitly_set_fields_by_scope(Scope.settings)
- if not field.is_set_on(module):
- continue
+class InheritanceKeyValueStore(KeyValueStore):
+ """
+ Common superclass for kvs's which know about inheritance of settings. Offers simple
+ dict-based storage of fields and lookup of inherited values.
- if name in inherited_metadata and field.read_from(module) == inherited_metadata.get(name):
- continue
+ Note: inherited_settings is a dict of key to json values (internal xblock field repr)
+ """
+ def __init__(self, initial_values=None, inherited_settings=None):
+ super(InheritanceKeyValueStore, self).__init__()
+ self.inherited_settings = inherited_settings or {}
+ self._fields = initial_values or {}
- try:
- metadata[name] = field.read_json(module)
- except KeyError:
- # Ignore any missing keys in _field_data
- pass
+ def get(self, key):
+ return self._fields[key.field_name]
- return metadata
+ def set(self, key, value):
+ # xml backed courses are read-only, but they do have some computed fields
+ self._fields[key.field_name] = value
+
+ def delete(self, key):
+ del self._fields[key.field_name]
+
+ def has(self, key):
+ return key.field_name in self._fields
+
+ def default(self, key):
+ """
+ Check to see if the default should be from inheritance rather than from the field's global default
+ """
+ return self.inherited_settings[key.field_name]
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 1051676484..11fd392134 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -17,25 +17,23 @@ import sys
import logging
import copy
-from collections import namedtuple
from fs.osfs import OSFS
from itertools import repeat
from path import path
from operator import attrgetter
-from uuid import uuid4
from importlib import import_module
from xmodule.errortracker import null_error_tracker, exc_info_to_str
from xmodule.mako_module import MakoDescriptorSystem
from xmodule.x_module import XModuleDescriptor
from xmodule.error_module import ErrorDescriptor
-from xblock.runtime import DbModel, KeyValueStore
+from xblock.runtime import DbModel
from xblock.exceptions import InvalidScopeError
from xblock.fields import Scope, ScopeIds
from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son, MONGO_MODULESTORE_TYPE
from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata
+from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
log = logging.getLogger(__name__)
@@ -58,12 +56,13 @@ class InvalidWriteError(Exception):
"""
-class MongoKeyValueStore(KeyValueStore):
+class MongoKeyValueStore(InheritanceKeyValueStore):
"""
A KeyValueStore that maps keyed data access to one of the 3 data areas
known to the MongoModuleStore (data, children, and metadata)
"""
def __init__(self, data, children, metadata):
+ super(MongoKeyValueStore, self).__init__()
self._data = data
self._children = children
self._metadata = metadata
@@ -201,10 +200,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
# Convert the serialized fields values in self.cached_metadata
# to python values
- metadata_to_inherit = {
- key: module.fields[key].from_json(value)
- for key, value in self.cached_metadata.get(non_draft_loc.url(), {}).items()
- }
+ metadata_to_inherit = self.cached_metadata.get(non_draft_loc.url(), {})
inherit_metadata(module, metadata_to_inherit)
# decache any computed pending field settings
module.save()
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py
index 361ffff85d..e2d8fd30a9 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py
@@ -4,37 +4,31 @@ from collections import namedtuple
from xblock.runtime import KeyValueStore
from xblock.exceptions import InvalidScopeError
from .definition_lazy_loader import DefinitionLazyLoader
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore
# id is a BlockUsageLocator, def_id is the definition's guid
SplitMongoKVSid = namedtuple('SplitMongoKVSid', 'id, def_id')
-PROVENANCE_LOCAL = 'local'
-PROVENANCE_DEFAULT = 'default'
-PROVENANCE_INHERITED = 'inherited'
-
-class SplitMongoKVS(KeyValueStore):
+class SplitMongoKVS(InheritanceKeyValueStore):
"""
A KeyValueStore that maps keyed data access to one of the 3 data areas
known to the MongoModuleStore (data, children, and metadata)
"""
- def __init__(self, definition, fields, _inherited_settings):
+ def __init__(self, definition, fields, inherited_settings):
"""
:param definition: either a lazyloader or definition id for the definition
:param fields: a dictionary of the locally set fields
- :param _inherited_settings: the value of each inheritable field from above this.
+ :param inherited_settings: the json value of each inheritable field from above this.
Note, local fields may override and disagree w/ this b/c this says what the value
should be if the field is undefined.
"""
- # ensure kvs's don't share objects w/ others so that changes can't appear in separate ones
- # the particular use case was that changes to kvs's were polluting caches. My thinking was
- # that kvs's should be independent thus responsible for the isolation.
+ super(SplitMongoKVS, self).__init__(copy.copy(fields), inherited_settings)
self._definition = definition # either a DefinitionLazyLoader or the db id of the definition.
# if the db id, then the definition is presumed to be loaded into _fields
- self._fields = copy.copy(fields)
- self._inherited_settings = _inherited_settings
+
def get(self, key):
# simplest case, field is directly set
@@ -49,12 +43,8 @@ class SplitMongoKVS(KeyValueStore):
# didn't find children in _fields; so, see if there's a default
raise KeyError()
elif key.scope == Scope.settings:
- # didn't find in _fields; so, get from inheritance since not locally set
- if key.field_name in self._inherited_settings:
- return self._inherited_settings[key.field_name]
- else:
- # or get default
- raise KeyError()
+ # get default which may be the inherited value
+ raise KeyError()
elif key.scope == Scope.content:
if isinstance(self._definition, DefinitionLazyLoader):
self._load_definition()
@@ -113,40 +103,6 @@ class SplitMongoKVS(KeyValueStore):
# if someone changes it so that they do, then change any tests of field.name in xx._field_data
return key.field_name in self._fields
- # would like to just take a key, but there's a bunch of magic in DbModel for constructing the key via
- # a private method
- def field_value_provenance(self, key_scope, key_name):
- """
- Where the field value comes from: one of [PROVENANCE_LOCAL, PROVENANCE_DEFAULT, PROVENANCE_INHERITED].
- """
- # handle any special cases
- if key_scope == Scope.content:
- if key_name == 'location':
- return PROVENANCE_LOCAL
- elif key_name == 'category':
- return PROVENANCE_LOCAL
- else:
- self._load_definition()
- if key_name in self._fields:
- return PROVENANCE_LOCAL
- else:
- return PROVENANCE_DEFAULT
- elif key_scope == Scope.parent:
- return PROVENANCE_DEFAULT
- # catch the locally set state
- elif key_name in self._fields:
- return PROVENANCE_LOCAL
- elif key_scope == Scope.settings and key_name in self._inherited_settings:
- return PROVENANCE_INHERITED
- else:
- return PROVENANCE_DEFAULT
-
- def get_inherited_settings(self):
- """
- Get the settings set by the ancestors (which locally set fields may override or not)
- """
- return self._inherited_settings
-
def _load_definition(self):
"""
Update fields w/ the lazily loaded definitions
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index ad5fa9c3cb..fefa668a56 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -19,7 +19,9 @@ from path import path
import calc
from xblock.field_data import DictFieldData
-from xmodule.x_module import ModuleSystem, XModuleDescriptor
+from xmodule.x_module import ModuleSystem, XModuleDescriptor, DescriptorSystem
+from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.mako_module import MakoDescriptorSystem
# Location of common test DATA directory
@@ -64,7 +66,20 @@ def get_test_system():
node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
xblock_field_data=lambda descriptor: descriptor._field_data,
anonymous_student_id='student',
- open_ended_grading_interface= open_ended_grading_interface
+ open_ended_grading_interface=open_ended_grading_interface
+ )
+
+
+def get_test_descriptor_system():
+ """
+ Construct a test DescriptorSystem instance.
+ """
+ return MakoDescriptorSystem(
+ load_item=Mock(),
+ resources_fs=Mock(),
+ error_tracker=Mock(),
+ render_template=lambda template, context: repr(context),
+ mixins=(InheritanceMixin,),
)
diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py
index 515e367ee0..2e59c545a4 100644
--- a/common/lib/xmodule/xmodule/tests/test_editing_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py
@@ -9,7 +9,7 @@ from xmodule.editing_module import TabsEditingDescriptor
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
-from .import get_test_system
+from xmodule.tests import get_test_descriptor_system
log = logging.getLogger(__name__)
@@ -19,7 +19,7 @@ class TabsEditingDescriptorTestCase(unittest.TestCase):
def setUp(self):
super(TabsEditingDescriptorTestCase, self).setUp()
- system = get_test_system()
+ system = get_test_descriptor_system()
system.render_template = Mock(return_value="
Test Template HTML
")
self.tabs = [
{
@@ -44,8 +44,8 @@ class TabsEditingDescriptorTestCase(unittest.TestCase):
]
TabsEditingDescriptor.tabs = self.tabs
- self.descriptor = TabsEditingDescriptor(
- runtime=system,
+ self.descriptor = system.construct_xblock_from_class(
+ TabsEditingDescriptor,
field_data=DictFieldData({}),
scope_ids=ScopeIds(None, None, None, None),
)
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 6ab5e29ad1..8f24112bc6 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -158,11 +158,10 @@ class ImportTestCase(BaseCourseTestCase):
# Check that the child inherits due correctly
child = descriptor.get_children()[0]
self.assertEqual(child.due, ImportTestCase.date.from_json(v))
- self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
- self.assertEqual(1, len(child._inherited_metadata))
+ # need to convert v to canonical json b4 comparing
self.assertEqual(
- datetime.datetime(2013, 3, 20, 17, 0, tzinfo=UTC()),
- child._inherited_metadata['due']
+ ImportTestCase.date.to_json(ImportTestCase.date.from_json(v)),
+ child.xblock_kvs.inherited_settings['due']
)
# Now export and check things
@@ -218,8 +217,6 @@ class ImportTestCase(BaseCourseTestCase):
# Check that the child does not inherit a value for due
child = descriptor.get_children()[0]
self.assertEqual(child.due, None)
- # pylint: disable=W0212
- self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
self.assertLessEqual(
child.start,
datetime.datetime.now(UTC())
@@ -249,10 +246,9 @@ class ImportTestCase(BaseCourseTestCase):
self.assertEqual(descriptor.due, ImportTestCase.date.from_json(course_due))
self.assertEqual(child.due, ImportTestCase.date.from_json(child_due))
# Test inherited metadata. Due does not appear here (because explicitly set on child).
- self.assertEqual(1, len(child._inheritable_metadata))
self.assertEqual(
- datetime.datetime(2013, 3, 20, 17, 0, tzinfo=UTC()),
- child._inheritable_metadata['due']
+ ImportTestCase.date.to_json(ImportTestCase.date.from_json(course_due)),
+ child.xblock_kvs.inherited_settings['due']
)
def test_is_pointer_tag(self):
@@ -288,14 +284,14 @@ class ImportTestCase(BaseCourseTestCase):
print("Starting import")
course = self.get_course('toy')
- def check_for_key(key, node):
+ def check_for_key(key, node, value):
"recursive check for presence of key"
print("Checking {0}".format(node.location.url()))
- self.assertTrue(node._field_data.has(node, key))
+ self.assertEqual(getattr(node, key), value)
for c in node.get_children():
- check_for_key(key, c)
+ check_for_key(key, c, value)
- check_for_key('graceperiod', course)
+ check_for_key('graceperiod', course, course.graceperiod)
def test_policy_loading(self):
"""Make sure that when two courses share content with the same
diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 1001ae8180..708c874fc3 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -18,7 +18,6 @@ from mock import Mock
from . import LogicTest
from lxml import etree
-from .import get_test_system
from xmodule.modulestore import Location
from xmodule.video_module import VideoDescriptor, _create_youtube_string
from .test_import import DummySystem
@@ -26,6 +25,7 @@ from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
from textwrap import dedent
+from xmodule.tests import get_test_descriptor_system
class VideoModuleTest(LogicTest):
@@ -124,9 +124,9 @@ class VideoDescriptorTest(unittest.TestCase):
"""Test for VideoDescriptor"""
def setUp(self):
- system = get_test_system()
- self.descriptor = VideoDescriptor(
- runtime=system,
+ system = get_test_descriptor_system()
+ self.descriptor = system.construct_xblock_from_class(
+ VideoDescriptor,
field_data=DictFieldData({}),
scope_ids=ScopeIds(None, None, None, None),
)
@@ -304,7 +304,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
a few weeks).
"""
module_system = DummySystem(load_error_modules=True)
- xml_data ='''
+ xml_data = '''
''')
- self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
+ self.assertXmlEqual(expected, xml)
def test_export_to_xml_empty_parameters(self):
"""Test XML export with defaults."""
diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
index 1333cf8cb4..666922a270 100644
--- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
+++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
@@ -29,6 +29,7 @@ from xmodule.conditional_module import ConditionalDescriptor
from xmodule.randomize_module import RandomizeDescriptor
from xmodule.vertical_module import VerticalDescriptor
from xmodule.wrapper_module import WrapperDescriptor
+from xmodule.tests import get_test_descriptor_system
LEAF_XMODULES = (
AnnotatableDescriptor,
@@ -80,20 +81,12 @@ class TestXBlockWrapper(object):
)
return runtime
- @property
- def leaf_descriptor_runtime(self):
- runtime = MakoDescriptorSystem(
- load_item=Mock(),
- resources_fs=Mock(),
- error_tracker=Mock(),
- render_template=(lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)),
- )
- return runtime
-
def leaf_descriptor(self, descriptor_cls):
location = 'i4x://org/course/category/name'
- return descriptor_cls(
- self.leaf_descriptor_runtime,
+ runtime = get_test_descriptor_system()
+ runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
+ return runtime.construct_xblock_from_class(
+ descriptor_cls,
DictFieldData({}),
ScopeIds(None, descriptor_cls.__name__, location, location)
)
@@ -110,16 +103,12 @@ class TestXBlockWrapper(object):
runtime.position = 2
return runtime
- @property
- def container_descriptor_runtime(self):
- runtime = Mock()
- runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
- return runtime
-
def container_descriptor(self, descriptor_cls):
location = 'i4x://org/course/category/name'
- return descriptor_cls(
- self.container_descriptor_runtime,
+ runtime = get_test_descriptor_system()
+ runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
+ return runtime.construct_xblock_from_class(
+ descriptor_cls,
DictFieldData({
'children': range(3)
}),
diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py
index a7bce08fd8..1515c76237 100644
--- a/common/lib/xmodule/xmodule/tests/test_xml_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py
@@ -7,9 +7,11 @@ from xblock.field_data import DictFieldData
from xmodule.fields import Date, Timedelta
from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
import unittest
-from .import get_test_system
from nose.tools import assert_equals # pylint: disable=E0611
from mock import Mock
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from xblock.runtime import DbModel
+from xmodule.tests import get_test_descriptor_system
class CrazyJsonString(String):
@@ -34,6 +36,11 @@ class TestFields(object):
values=[{'display_name': 'first', 'value': 'value a'},
{'display_name': 'second', 'value': 'value b'}]
)
+ showanswer = String(
+ help="When to show the problem answer to the student",
+ scope=Scope.settings,
+ default="finished"
+ )
# Used for testing select type
float_select = Float(scope=Scope.settings, default=.999, values=[1.23, 0.98])
# Used for testing float type
@@ -48,10 +55,10 @@ class EditableMetadataFieldsTest(unittest.TestCase):
editable_fields = self.get_xml_editable_fields(DictFieldData({}))
# Tests that the xblock fields (currently tags and name) get filtered out.
# Also tests that xml_attributes is filtered out of XmlDescriptor.
- self.assertEqual(1, len(editable_fields), "Expected only 1 editable field for xml descriptor.")
+ self.assertEqual(1, len(editable_fields), editable_fields)
self.assert_field_values(
editable_fields, 'display_name', XModuleFields.display_name,
- explicitly_set=False, inheritable=False, value=None, default_value=None
+ explicitly_set=False, value=None, default_value=None
)
def test_override_default(self):
@@ -59,50 +66,51 @@ class EditableMetadataFieldsTest(unittest.TestCase):
editable_fields = self.get_xml_editable_fields(DictFieldData({'display_name': 'foo'}))
self.assert_field_values(
editable_fields, 'display_name', XModuleFields.display_name,
- explicitly_set=True, inheritable=False, value='foo', default_value=None
+ explicitly_set=True, value='foo', default_value=None
)
def test_integer_field(self):
descriptor = self.get_descriptor(DictFieldData({'max_attempts': '7'}))
editable_fields = descriptor.editable_metadata_fields
- self.assertEqual(7, len(editable_fields))
+ self.assertEqual(8, len(editable_fields))
self.assert_field_values(
editable_fields, 'max_attempts', TestFields.max_attempts,
- explicitly_set=True, inheritable=False, value=7, default_value=1000, type='Integer',
+ explicitly_set=True, value=7, default_value=1000, type='Integer',
options=TestFields.max_attempts.values
)
self.assert_field_values(
editable_fields, 'display_name', TestFields.display_name,
- explicitly_set=False, inheritable=False, value='local default', default_value='local default'
+ explicitly_set=False, value='local default', default_value='local default'
)
editable_fields = self.get_descriptor(DictFieldData({})).editable_metadata_fields
self.assert_field_values(
editable_fields, 'max_attempts', TestFields.max_attempts,
- explicitly_set=False, inheritable=False, value=1000, default_value=1000, type='Integer',
+ explicitly_set=False, value=1000, default_value=1000, type='Integer',
options=TestFields.max_attempts.values
)
def test_inherited_field(self):
- model_val = {'display_name': 'inherited'}
- descriptor = self.get_descriptor(DictFieldData(model_val))
- # Mimic an inherited value for display_name (inherited and inheritable are the same in this case).
- descriptor._inherited_metadata = model_val
- descriptor._inheritable_metadata = model_val
+ kvs = InheritanceKeyValueStore(initial_values={}, inherited_settings={'showanswer': 'inherited'})
+ model_data = DbModel(kvs)
+ descriptor = self.get_descriptor(model_data)
editable_fields = descriptor.editable_metadata_fields
self.assert_field_values(
- editable_fields, 'display_name', TestFields.display_name,
- explicitly_set=False, inheritable=True, value='inherited', default_value='inherited'
+ editable_fields, 'showanswer', InheritanceMixin.showanswer,
+ explicitly_set=False, value='inherited', default_value='inherited'
)
- descriptor = self.get_descriptor(DictFieldData({'display_name': 'explicit'}))
# Mimic the case where display_name WOULD have been inherited, except we explicitly set it.
- descriptor._inheritable_metadata = {'display_name': 'inheritable value'}
- descriptor._inherited_metadata = {}
+ kvs = InheritanceKeyValueStore(
+ initial_values={'showanswer': 'explicit'},
+ inherited_settings={'showanswer': 'inheritable value'}
+ )
+ model_data = DbModel(kvs)
+ descriptor = self.get_descriptor(model_data)
editable_fields = descriptor.editable_metadata_fields
self.assert_field_values(
- editable_fields, 'display_name', TestFields.display_name,
- explicitly_set=True, inheritable=True, value='explicit', default_value='inheritable value'
+ editable_fields, 'showanswer', InheritanceMixin.showanswer,
+ explicitly_set=True, value='explicit', default_value='inheritable value'
)
def test_type_and_options(self):
@@ -115,41 +123,44 @@ class EditableMetadataFieldsTest(unittest.TestCase):
# Tests for select
self.assert_field_values(
editable_fields, 'string_select', TestFields.string_select,
- explicitly_set=False, inheritable=False, value='default value', default_value='default value',
+ explicitly_set=False, value='default value', default_value='default value',
type='Select', options=[{'display_name': 'first', 'value': 'value a JSON'},
{'display_name': 'second', 'value': 'value b JSON'}]
)
self.assert_field_values(
editable_fields, 'float_select', TestFields.float_select,
- explicitly_set=False, inheritable=False, value=.999, default_value=.999,
+ explicitly_set=False, value=.999, default_value=.999,
type='Select', options=[1.23, 0.98]
)
self.assert_field_values(
editable_fields, 'boolean_select', TestFields.boolean_select,
- explicitly_set=False, inheritable=False, value=None, default_value=None,
+ explicitly_set=False, value=None, default_value=None,
type='Select', options=[{'display_name': "True", "value": True}, {'display_name': "False", "value": False}]
)
# Test for float
self.assert_field_values(
editable_fields, 'float_non_select', TestFields.float_non_select,
- explicitly_set=False, inheritable=False, value=.999, default_value=.999,
+ explicitly_set=False, value=.999, default_value=.999,
type='Float', options={'min': 0, 'step': .3}
)
self.assert_field_values(
editable_fields, 'list_field', TestFields.list_field,
- explicitly_set=False, inheritable=False, value=[], default_value=[],
+ explicitly_set=False, value=[], default_value=[],
type='List'
)
# Start of helper methods
def get_xml_editable_fields(self, field_data):
- system = get_test_system()
- system.render_template = Mock(return_value="
")
- return TestModuleDescriptor(runtime=system, field_data=field_data, scope_ids=Mock())
+ return system.construct_xblock_from_class(TestModuleDescriptor, field_data=field_data, scope_ids=Mock())
- def assert_field_values(self, editable_fields, name, field, explicitly_set, inheritable, value, default_value,
+ def assert_field_values(self, editable_fields, name, field, explicitly_set, value, default_value,
type='Generic', options=[]):
test_field = editable_fields[name]
@@ -178,7 +189,6 @@ class EditableMetadataFieldsTest(unittest.TestCase):
self.assertEqual(type, test_field['type'])
self.assertEqual(explicitly_set, test_field['explicitly_set'])
- self.assertEqual(inheritable, test_field['inheritable'])
class TestSerialize(unittest.TestCase):
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index f76a8278c7..10ab6e816c 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -15,6 +15,8 @@ import logging
from lxml import etree
from pkg_resources import resource_string
+import datetime
+import time
from django.http import Http404
from django.conf import settings
@@ -28,8 +30,8 @@ from xblock.fields import Scope, String, Boolean, Float, List, Integer, ScopeIds
from xblock.field_data import DictFieldData
-import datetime
-import time
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore
+from xblock.runtime import DbModel
log = logging.getLogger(__name__)
@@ -240,9 +242,11 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location))
field_data = VideoDescriptor._parse_video_xml(xml_data)
field_data['location'] = location
+ kvs = InheritanceKeyValueStore(initial_values=field_data)
+ field_data = DbModel(kvs)
video = system.construct_xblock_from_class(
cls,
- DictFieldData(field_data),
+ field_data,
# We're loading a descriptor, so student_id is meaningless
# We also don't have separate notions of definition and usage ids yet,
@@ -259,25 +263,22 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
youtube_string = _create_youtube_string(self)
# Mild workaround to ensure that tests pass -- if a field
# is set to its default value, we don't need to write it out.
- if youtube_string == '1.00:OEoXaMPEzfM':
- youtube_string = ''
+ if youtube_string and youtube_string != '1.00:OEoXaMPEzfM':
+ xml.set('youtube', unicode(youtube_string))
+ xml.set('url_name', self.url_name)
attrs = {
'display_name': self.display_name,
'show_captions': json.dumps(self.show_captions),
- 'youtube': youtube_string,
'start_time': datetime.timedelta(seconds=self.start_time),
'end_time': datetime.timedelta(seconds=self.end_time),
'sub': self.sub,
- 'url_name': self.url_name
}
- fields = {field.name: field for field in self.fields.values()}
for key, value in attrs.items():
# Mild workaround to ensure that tests pass -- if a field
- # is set to its default value, we don't need to write it out.
- if key in fields and fields[key].default == getattr(self, key):
- continue
+ # is set to its default value, we don't write it out.
if value:
- xml.set(key, unicode(value))
+ if key in self.fields and self.fields[key].is_set_on(self):
+ xml.set(key, unicode(value))
for source in self.html5_sources:
ele = etree.Element('source')
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 2ae3a5ae4f..02feebea1b 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -648,26 +648,17 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
# We are not allowing editing of xblock tag and name fields at this time (for any component).
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.)
"""
- if scope == Scope.settings and hasattr(self, '_inherited_metadata'):
- inherited_metadata = getattr(self, '_inherited_metadata')
- result = {}
- for field in self.fields.values():
- if (field.scope == scope and
- self._field_data.has(self, field.name) and
- field.name not in inherited_metadata):
- result[field.name] = self._field_data.get(self, field.name)
- return result
- else:
- result = {}
- for field in self.fields.values():
- if (field.scope == scope and self._field_data.has(self, field.name)):
- result[field.name] = self._field_data.get(self, field.name)
- return result
+ result = {}
+ for field in self.fields.values():
+ if (field.scope == scope and self._field_data.has(self, field.name)):
+ result[field.name] = self._field_data.get(self, field.name)
+ return result
@property
def editable_metadata_fields(self):
@@ -676,8 +667,14 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
Can be limited by extending `non_editable_metadata_fields`.
"""
- inherited_metadata = getattr(self, '_inherited_metadata', {})
- inheritable_metadata = getattr(self, '_inheritable_metadata', {})
+ def jsonify_value(field, json_choice):
+ if isinstance(json_choice, dict) and 'value' in json_choice:
+ json_choice = dict(json_choice) # make a copy so below doesn't change the original
+ json_choice['value'] = field.to_json(json_choice['value'])
+ else:
+ json_choice = field.to_json(json_choice)
+ return json_choice
+
metadata_fields = {}
# Only use the fields from this class, not mixins
@@ -688,56 +685,35 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
if field.scope != Scope.settings or field in self.non_editable_metadata_fields:
continue
- inheritable = False
- value = getattr(self, field.name)
- default_value = field.default
- explicitly_set = self._field_data.has(self, field.name)
- if field.name in inheritable_metadata:
- inheritable = True
- default_value = field.from_json(inheritable_metadata.get(field.name))
- if field.name in inherited_metadata:
- explicitly_set = False
+ # gets the 'default_value' and 'explicitly_set' attrs
+ metadata_fields[field.name] = self.runtime.get_field_provenance(self, field)
+ metadata_fields[field.name]['field_name'] = field.name
+ metadata_fields[field.name]['display_name'] = field.display_name
+ metadata_fields[field.name]['help'] = field.help
+ metadata_fields[field.name]['value'] = field.read_json(self)
# We support the following editors:
# 1. A select editor for fields with a list of possible values (includes Booleans).
# 2. Number editors for integers and floats.
# 3. A generic string editor for anything else (editing JSON representation of the value).
editor_type = "Generic"
- values = copy.deepcopy(field.values)
- if isinstance(values, tuple):
- values = list(values)
- if isinstance(values, list):
- if len(values) > 0:
- editor_type = "Select"
- for index, choice in enumerate(values):
- json_choice = copy.deepcopy(choice)
- if isinstance(json_choice, dict) and 'value' in json_choice:
- json_choice['value'] = field.to_json(json_choice['value'])
- else:
- json_choice = field.to_json(json_choice)
- values[index] = json_choice
+ values = field.values
+ if isinstance(values, (tuple, list)) and len(values) > 0:
+ editor_type = "Select"
+ values = [jsonify_value(field, json_choice) for json_choice in values]
elif isinstance(field, Integer):
editor_type = "Integer"
elif isinstance(field, Float):
editor_type = "Float"
elif isinstance(field, List):
editor_type = "List"
- metadata_fields[field.name] = {
- 'field_name': field.name,
- 'type': editor_type,
- 'display_name': field.display_name,
- 'value': field.to_json(value),
- 'options': [] if values is None else values,
- 'default_value': field.to_json(default_value),
- 'inheritable': inheritable,
- 'explicitly_set': explicitly_set,
- 'help': field.help,
- }
+ metadata_fields[field.name]['type'] = editor_type
+ metadata_fields[field.name]['options'] = [] if values is None else values
return metadata_fields
# ~~~~~~~~~~~~~~~ XBlock API Wrappers ~~~~~~~~~~~~~~~~
- def studio_view(self, context):
+ def studio_view(self, _context):
"""
Return a fragment with the html from this XModuleDescriptor's editing view
@@ -750,6 +726,7 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
class DescriptorSystem(Runtime):
+
def __init__(self, load_item, resources_fs, error_tracker, **kwargs):
"""
load_item: Takes a Location and returns an XModuleDescriptor
@@ -797,6 +774,33 @@ class DescriptorSystem(Runtime):
"""See documentation for `xblock.runtime:Runtime.get_block`"""
return self.load_item(block_id)
+ def get_field_provenance(self, xblock, field):
+ """
+ For the given xblock, return a dict for the field's current state:
+ {
+ 'default_value': what json'd value will take effect if field is unset: either the field default or
+ inherited value,
+ 'explicitly_set': boolean for whether the current value is set v default/inherited,
+ }
+ :param xblock:
+ :param field:
+ """
+ # in runtime b/c runtime contains app-specific xblock behavior. Studio's the only app
+ # which needs this level of introspection right now. runtime also is 'allowed' to know
+ # about the kvs, dbmodel, etc.
+
+ result = {}
+ result['explicitly_set'] = xblock._field_data.has(xblock, field.name)
+ try:
+ block_inherited = xblock.xblock_kvs.inherited_settings
+ except AttributeError: # if inherited_settings doesn't exist on kvs
+ block_inherited = {}
+ if field.name in block_inherited:
+ result['default_value'] = block_inherited[field.name]
+ else:
+ result['default_value'] = field.to_json(field.default)
+ return result
+
class XMLParsingSystem(DescriptorSystem):
def __init__(self, process_xml, policy, **kwargs):
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 174dce03e6..13f0fddd48 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -9,10 +9,9 @@ from lxml import etree
from xblock.fields import Dict, Scope, ScopeIds
from xmodule.x_module import (XModuleDescriptor, policy_key)
from xmodule.modulestore import Location
-from xmodule.modulestore.inheritance import own_metadata
+from xmodule.modulestore.inheritance import own_metadata, InheritanceKeyValueStore
from xmodule.modulestore.xml_exporter import EdxJSONEncoder
-
-from xblock.field_data import DictFieldData
+from xblock.runtime import DbModel
log = logging.getLogger(__name__)
@@ -365,10 +364,12 @@ class XmlDescriptor(XModuleDescriptor):
field_data['xml_attributes'][key] = value
field_data['location'] = location
field_data['category'] = xml_object.tag
+ kvs = InheritanceKeyValueStore(initial_values=field_data)
+ field_data = DbModel(kvs)
return system.construct_xblock_from_class(
cls,
- DictFieldData(field_data),
+ field_data,
# We're loading a descriptor, so student_id is meaningless
# We also don't have separate notions of definition and usage ids yet,