diff --git a/lms/djangoapps/courseware/tests/test_block_render.py b/lms/djangoapps/courseware/tests/test_block_render.py index 55896d4fa9..0504772aa5 100644 --- a/lms/djangoapps/courseware/tests/test_block_render.py +++ b/lms/djangoapps/courseware/tests/test_block_render.py @@ -487,11 +487,10 @@ class BlockRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.mock_user, request, block, field_data_cache, course.id, course=course ) - # check that _unwrapped_field_data is the same as the original + # check that block.runtime.service(block, 'field-data-unbound') is the same as the original # _field_data, but now _field_data as been reset. - # pylint: disable=protected-access - assert block._unwrapped_field_data is original_field_data # lint-amnesty, pylint: disable=no-member - assert block._unwrapped_field_data is not block._field_data # lint-amnesty, pylint: disable=no-member + assert block.runtime.service(block, 'field-data-unbound') is original_field_data + assert block.runtime.service(block, 'field-data-unbound') is not block._field_data # pylint: disable=protected-access, line-too-long # now bind this block to a few other students for user in [UserFactory(), UserFactory(), self.mock_user]: @@ -513,7 +512,8 @@ class BlockRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): # the OverrideFieldData should point to the date FieldData assert isinstance(block._field_data._authored_data._source.fallback, DateLookupFieldData) # lint-amnesty, pylint: disable=no-member, line-too-long - assert block._field_data._authored_data._source.fallback._defaults is block._unwrapped_field_data # lint-amnesty, pylint: disable=no-member, line-too-long + assert block._field_data._authored_data._source.fallback._defaults \ + is block.runtime.service(block, 'field-data-unbound') def test_hash_resource(self): """ diff --git a/lms/djangoapps/gating/tests/test_integration.py b/lms/djangoapps/gating/tests/test_integration.py index ffa9ef243f..3b399e1b89 100644 --- a/lms/djangoapps/gating/tests/test_integration.py +++ b/lms/djangoapps/gating/tests/test_integration.py @@ -52,11 +52,9 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): org='edX', number='EDX101', run='EDX101_RUN1', - display_name='edX 101' - ) - with modulestore().bulk_operations(course.id): - course.enable_subsection_gating = True - grading_policy = { + display_name='edX 101', + enable_subsection_gating=True, + grading_policy={ "GRADER": [{ "type": "Homework", "min_count": 3, @@ -64,10 +62,9 @@ class TestGatedContent(MilestonesTestCaseMixin, SharedModuleStoreTestCase): "short_label": "HW", "weight": 1.0 }] - } - course.grading_policy = grading_policy - course.save() - + }, + ) + with modulestore().bulk_operations(course.id): # create chapter cls.chapter1 = BlockFactory.create( parent=course, diff --git a/xmodule/modulestore/inheritance.py b/xmodule/modulestore/inheritance.py index 5e24c39031..489365e80e 100644 --- a/xmodule/modulestore/inheritance.py +++ b/xmodule/modulestore/inheritance.py @@ -3,11 +3,13 @@ Support for inheritance of fields down an XBlock hierarchy. """ +import warnings from django.utils import timezone from xblock.core import XBlockMixin from xblock.fields import Boolean, Dict, Float, Integer, List, Scope, String from xblock.runtime import KeyValueStore, KvsFieldData +from xmodule.error_block import ErrorBlock from xmodule.fields import Date, Timedelta from xmodule.partitions.partitions import UserPartition @@ -280,7 +282,10 @@ def compute_inherited_metadata(block): NOTE: This means that there is no such thing as lazy loading at the moment--this accesses all the children.""" if block.has_children: - parent_metadata = block.xblock_kvs.inherited_settings.copy() + if isinstance(block.xblock_kvs, InheritanceKeyValueStore): + parent_metadata = block.xblock_kvs.inherited_settings.copy() + else: + parent_metadata = {} # add any of block's explicitly set fields to the inheriting list for field in InheritanceMixin.fields.values(): # lint-amnesty, pylint: disable=no-member if field.is_set_on(block): @@ -301,10 +306,23 @@ def inherit_metadata(block, inherited_data): `inherited_data`: A dictionary mapping field names to the values that they should inherit """ - try: + if isinstance(block, ErrorBlock): + return + + block_type = block.scope_ids.block_type + if isinstance(block.xblock_kvs, InheritanceKeyValueStore): + # This XBlock's field_data is backed by InheritanceKeyValueStore, which supports pre-computed inherited fields block.xblock_kvs.inherited_settings = inherited_data - except AttributeError: # the kvs doesn't have inherited_settings probably b/c it's an error block - pass + else: + # We cannot apply pre-computed field data to this XBlock during import, but inheritance should still work + # normally when it's used in Studio/LMS, which use a different runtime. + # Though if someone ever needs a hacky temporary fix here, it's possible here to force it with: + # init_dict = {key: getattr(block, key) for key in block.fields.keys()} + # block._field_data = InheritanceKeyValueStore(init_dict) + warnings.warn( + f'Cannot inherit metadata to {block_type} block with KVS {block.xblock_kvs}', + stacklevel=2, + ) def own_metadata(block): @@ -316,7 +334,17 @@ def own_metadata(block): class InheritingFieldData(KvsFieldData): - """A `FieldData` implementation that can inherit value from parents to children.""" + """ + A `FieldData` implementation that can inherit value from parents to children. + + This wraps a KeyValueStore, and will work fine with any KVS implementation. + Sometimes this wraps a subclass of InheritanceKeyValueStore, but that's not + a requirement. + + This class is the way that inheritance "normally" works in modulestore. + During XML import/export, however, a different mechanism is used: + InheritanceKeyValueStore. + """ def __init__(self, inheritable_names, **kwargs): """ @@ -379,6 +407,15 @@ class InheritanceKeyValueStore(KeyValueStore): dict-based storage of fields and lookup of inherited values. Note: inherited_settings is a dict of key to json values (internal xblock field repr) + + Using this KVS is an alternative to using InheritingFieldData(). That one works with any KVS, like + DictKeyValueStore, and doesn't require any special behavior. On the other hand, this InheritanceKeyValueStore only + does inheritance properly if you first use compute_inherited_metadata() to walk the tree of XBlocks and pre-compute + the inherited metadata for the whole tree, storing it in the inherited_settings field of each instance of this KVS. + + 🟥 Warning: Unlike the base class, this KVS makes the assumption that you're using a completely separate KVS + instance for every XBlock, so that we only have to look at the "field_name" part of the key. You cannot use this + as a drop-in replacement for DictKeyValueStore for this reason. """ def __init__(self, initial_values=None, inherited_settings=None): super().__init__() diff --git a/xmodule/modulestore/mongo/base.py b/xmodule/modulestore/mongo/base.py index fd434adc0e..469ef608a5 100644 --- a/xmodule/modulestore/mongo/base.py +++ b/xmodule/modulestore/mongo/base.py @@ -170,6 +170,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li A system that has a cache of block json that it will use to load blocks from, with a backup of calling to the underlying modulestore for more data """ + + # This CachingDescriptorSystem runtime sets block._field_data on each block via construct_xblock_from_class(), + # rather than the newer approach of providing a "field-data" service via runtime.service(). As a result, during + # bind_for_student() we can't just set ._bound_field_data; we must overwrite block._field_data. + uses_deprecated_field_data = True + def __repr__(self): return "CachingDescriptorSystem{!r}".format(( self.modulestore, @@ -303,6 +309,18 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li error_msg=exc_info_to_str(sys.exc_info()) ) + def service(self, block, service_name): + """ + Return a service, or None. + Services are objects implementing arbitrary other interfaces. + """ + # A very minimal shim for compatibility with the new API for how we access field data in split mongo: + if service_name == 'field-data-unbound': + return block._field_data # pylint: disable=protected-access + elif service_name == 'field-data': + return block._bound_field_data if hasattr(block, "_bound_field_data") else block._field_data + return super().service(block, service_name) + def _convert_reference_to_key(self, ref_string): """ Convert a single serialized UsageKey string in a ReferenceField into a UsageKey. diff --git a/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 5cb0bb3571..b3234df5a2 100644 --- a/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -2,6 +2,7 @@ import logging import sys +import weakref from fs.osfs import OSFS from lazy import lazy @@ -13,6 +14,7 @@ from xmodule.error_block import ErrorBlock from xmodule.errortracker import exc_info_to_str from xmodule.library_tools import LibraryToolsService from xmodule.mako_block import MakoDescriptorSystem +from xmodule.modulestore import BlockData from xmodule.modulestore.edit_info import EditInfoRuntimeMixin from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import InheritanceMixin, inheriting_field_data @@ -73,6 +75,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li self.default_class = default_class self.local_modules = {} self._services['library_tools'] = LibraryToolsService(modulestore, user_id=None) + # Cache of block field datas, keyed by the XBlock instance (since the ScopeId changes!) + self.block_field_datas = weakref.WeakKeyDictionary() @lazy def _parent_map(self): # lint-amnesty, pylint: disable=missing-function-docstring @@ -159,7 +163,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li # is the intended one when not given a course_entry_override; thus, the caching of the last branch/course id. def xblock_from_json(self, class_, course_key, block_key, block_data, course_entry_override=None, **kwargs): """ - Load and return block info. + Instantiate an XBlock of the specified class, using the provided data. """ if course_entry_override is None: course_entry_override = self.course_entry @@ -167,37 +171,93 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li # most recent retrieval is most likely the right one for next caller (see comment above fn) self.course_entry = CourseEnvelope(course_entry_override.course_key, self.course_entry.structure) - definition_id = block_data.definition - # If no usage id is provided, generate an in-memory id if block_key is None: block_key = BlockKey(block_data.block_type, LocalId()) + # Construct the Block Usage Locator: + block_locator = course_key.make_usage_key(block_type=block_key.type, block_id=block_key.id) + + # If no definition id is provided, generate an in-memory id + definition_id = block_data.definition or LocalId() + + try: + block = self.construct_xblock_from_class( + class_, + ScopeIds(None, block_key.type, definition_id, block_locator), + for_parent=kwargs.get('for_parent'), + # Pass this tuple on for use by _init_field_data_for_block() when field data is initialized. + cds_init_args=(block_data, definition_id, kwargs.get("field_decorator")), + # Passing field_data here is deprecated, so we don't. Get it via block.service(block, "field-data"). + # https://github.com/openedx/XBlock/blob/e89cbc5/xblock/mixins.py#L200-L207 + ) + except Exception: # pylint: disable=broad-except + log.warning("Failed to load descriptor", exc_info=True) + return ErrorBlock.from_json( + block_data, + self, + course_entry_override.course_key.make_usage_key( + block_type='error', + block_id=block_key.id + ), + error_msg=exc_info_to_str(sys.exc_info()) + ) + + edit_info = block_data.edit_info + block._edited_by = edit_info.edited_by # pylint: disable=protected-access + block._edited_on = edit_info.edited_on # pylint: disable=protected-access + block.previous_version = edit_info.previous_version + block.update_version = edit_info.update_version + block.source_version = edit_info.source_version + block.definition_locator = DefinitionLocator(block_key.type, definition_id) + + # If this is an in-memory block, store it in this system + if isinstance(block_locator.block_id, LocalId): + self.local_modules[block_locator] = block + + return block + + def service(self, block, service_name): + """ + Return a service, or None. + Services are objects implementing arbitrary other interfaces. + """ + # Implement field data service: + if service_name in ('field-data', 'field-data-unbound'): + if hasattr(block, "_bound_field_data") and service_name != "field-data-unbound": + # Return the user-specific wrapped field data that gets set onto the block during XModule.bind_for_user + # This complexity is due to XModule heritage. If we didn't load the block as one step, then sometimes + # "rebind" it to be user-specific as a later step, we could load the field data and overrides at once. + return block._bound_field_data # pylint: disable=protected-access + if block not in self.block_field_datas: + try: + self._init_field_data_for_block(block) + except: + # Don't try again pointlessly every time another field is accessed + self.block_field_datas[block] = None + raise + return self.block_field_datas[block] + return super().service(block, service_name) + + def _init_field_data_for_block(self, block): + """ + Initialize the field-data service for the given block. + """ + block_key = BlockKey.from_usage_key(block.scope_ids.usage_id) + course_key = block.scope_ids.usage_id.context_key + try: + (block_data, definition_id, field_decorator) = block.get_cds_init_args() + except KeyError: + # This block was not instantiate via xblock_from_json()/modulestore but rather via the "direct" XBlock API + # such as using construct_xblock_from_class(). It probably doesn't yet exist in modulestore. + block_data = BlockData() + definition_id = LocalId() + field_decorator = None + class_ = self.load_block_type(block.scope_ids.block_type) convert_fields = lambda field: self.modulestore.convert_references_to_keys( course_key, class_, field, self.course_entry.structure['blocks'], ) - if definition_id is not None and not block_data.definition_loaded: - definition_loader = DefinitionLazyLoader( - self.modulestore, - course_key, - block_key.type, - definition_id, - convert_fields, - ) - else: - definition_loader = None - - # If no definition id is provide, generate an in-memory id - if definition_id is None: - definition_id = LocalId() - - # Construct the Block Usage Locator: - block_locator = course_key.make_usage_key( - block_type=block_key.type, - block_id=block_key.id, - ) - converted_fields = convert_fields(block_data.fields) converted_defaults = convert_fields(block_data.defaults) if block_key in self._parent_map: @@ -218,58 +278,42 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li except AttributeError: pass - try: - kvs = SplitMongoKVS( - definition_loader, - converted_fields, - converted_defaults, - parent=parent, - aside_fields=aside_fields, - field_decorator=kwargs.get('field_decorator') + if not isinstance(definition_id, LocalId) and not block_data.definition_loaded: + definition_loader = DefinitionLazyLoader( + self.modulestore, + course_key, + block_key.type, + definition_id, + convert_fields, ) + else: + definition_loader = None - if InheritanceMixin in self.modulestore.xblock_mixins: - field_data = inheriting_field_data(kvs) - else: - field_data = KvsFieldData(kvs) + kvs = SplitMongoKVS( + definition_loader, + converted_fields, + converted_defaults, + parent=parent, + aside_fields=aside_fields, + field_decorator=field_decorator, + ) - block = self.construct_xblock_from_class( - class_, - ScopeIds(None, block_key.type, definition_id, block_locator), - field_data, - for_parent=kwargs.get('for_parent') - ) - except Exception: # pylint: disable=broad-except - log.warning("Failed to load descriptor", exc_info=True) - return ErrorBlock.from_json( - block_data, - self, - course_entry_override.course_key.make_usage_key( - block_type='error', - block_id=block_key.id - ), - error_msg=exc_info_to_str(sys.exc_info()) - ) - - edit_info = block_data.edit_info - block._edited_by = edit_info.edited_by # pylint: disable=protected-access - block._edited_on = edit_info.edited_on # pylint: disable=protected-access - block.previous_version = edit_info.previous_version - block.update_version = edit_info.update_version - block.source_version = edit_info.source_version - block.definition_locator = DefinitionLocator(block_key.type, definition_id) + if InheritanceMixin in self.modulestore.xblock_mixins: + field_data = inheriting_field_data(kvs) + else: + field_data = KvsFieldData(kvs) + # Save the field_data now, because some of the wrappers will try to read fields from the block while they + # determine if they want to wrap the field_data or not. + self.block_field_datas[block] = field_data + # Now add in any wrappers that want to be added: for wrapper in self.modulestore.xblock_field_data_wrappers: - block._field_data = wrapper(block, block._field_data) # pylint: disable=protected-access + self.block_field_datas[block] = wrapper(block, self.block_field_datas[block]) - # decache any pending field settings - block.save() - - # If this is an in-memory block, store it in this system - if isinstance(block_locator.block_id, LocalId): - self.local_modules[block_locator] = block - - return block + # Note: user-specific wrappers like LmsFieldData or OverrideFieldData do not get set here. They get set later + # during bind_for_student(), which wraps the field-data and sets block._bound_field_data. Calling + # runtime.service(block, "field-data") or block._field_data (deprecated) will load block._bound_field_data if + # the block has been bound to a user, otherwise it returns the wrapped field data we just created above. def get_edited_by(self, xblock): """ diff --git a/xmodule/modulestore/xml.py b/xmodule/modulestore/xml.py index 38e3c22189..c4a4d66cbf 100644 --- a/xmodule/modulestore/xml.py +++ b/xmodule/modulestore/xml.py @@ -7,13 +7,13 @@ import json import logging import os import re +import warnings import sys from collections import defaultdict from contextlib import contextmanager from importlib import import_module from fs.osfs import OSFS -from lazy import lazy from lxml import etree from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator, LibraryLocator @@ -37,7 +37,7 @@ from xmodule.x_module import ( # lint-amnesty, pylint: disable=unused-import ) from .exceptions import ItemNotFoundError -from .inheritance import InheritanceKeyValueStore, compute_inherited_metadata, inheriting_field_data +from .inheritance import compute_inherited_metadata, inheriting_field_data edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, remove_blank_text=True) @@ -239,6 +239,24 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # lint-amnesty, pyl **kwargs ) + # pylint: disable=keyword-arg-before-vararg + def construct_xblock_from_class(self, cls, scope_ids, field_data=None, *args, **kwargs): + """ + Construct a new xblock of type cls, mixing in the mixins + defined for this application. + """ + if field_data: + # Currently, *some* XBlocks (those with XmlMixin) use XmlMixin.parse_xml() which instantiates + # its own key value store for field data. That is something which should be left to the runtime, for + # consistent behavior across all XBlocks, not controlled by individual XBlock implementations. + # We cannot just ignore field_data here though, because parse_xml may have pre-loaded data into it that we + # would otherwise lose. + warnings.warn( + 'XBlocks should not instantiate their own field_data store during parse_xml()', + DeprecationWarning, stacklevel=2, + ) + return super().construct_xblock_from_class(cls, scope_ids, field_data, *args, **kwargs) + # id_generator is ignored, because each ImportSystem is already local to # a course, and has it's own id_generator already in place def add_node_as_child(self, block, node, id_generator): # lint-amnesty, pylint: disable=signature-differs @@ -899,26 +917,10 @@ class LibraryXMLModuleStore(XMLModuleStore): """ return LibraryLocator(org=org, library=library) - @staticmethod - def patch_block_kvs(library_block): - """ - Metadata inheritance can be done purely through XBlocks, but in the import phase - a root block with an InheritanceKeyValueStore is assumed to be at the top of the hierarchy. - This should change in the future, but as XBlocks don't have this KVS, we have to patch it - here manually. - """ - init_dict = {key: getattr(library_block, key) for key in library_block.fields.keys()} - # if set, invalidate '_unwrapped_field_data' so it will be reset - # the next time it will be called - lazy.invalidate(library_block, '_unwrapped_field_data') - # pylint: disable=protected-access - library_block._field_data = inheriting_field_data(InheritanceKeyValueStore(init_dict)) - def content_importers(self, system, course_block, course_dir, url_name): """ Handle Metadata inheritance for Libraries. """ - self.patch_block_kvs(course_block) compute_inherited_metadata(course_block) def get_library(self, library_id, depth=0, **kwargs): # pylint: disable=unused-argument diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index 3fb444a1c1..95feedeb9c 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -276,12 +276,6 @@ class ImportTestCase(BaseCourseTestCase): # lint-amnesty, pylint: disable=missi ) block = system.process_xml(start_xml) - # pylint: disable=protected-access - original_unwrapped = block._unwrapped_field_data - LibraryXMLModuleStore.patch_block_kvs(block) - # '_unwrapped_field_data' is reset in `patch_block_kvs` - # pylint: disable=protected-access - assert original_unwrapped is not block._unwrapped_field_data compute_inherited_metadata(block) # Check the course block, since it has inheritance block = block.get_children()[0] @@ -322,7 +316,6 @@ class ImportTestCase(BaseCourseTestCase): # lint-amnesty, pylint: disable=missi '''.format(org=ORG, course=COURSE, url_name=url_name) block = system.process_xml(start_xml) - LibraryXMLModuleStore.patch_block_kvs(block) compute_inherited_metadata(block) # Run the checks on the course node instead. block = block.get_children()[0] @@ -390,7 +383,6 @@ class ImportTestCase(BaseCourseTestCase): # lint-amnesty, pylint: disable=missi '''.format(due=course_due, org=ORG, course=COURSE, url_name=url_name) block = system.process_xml(start_xml) - LibraryXMLModuleStore.patch_block_kvs(block) # Chapter is two levels down here. child = block.get_children()[0].get_children()[0] # pylint: disable=protected-access diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 5478ccc45d..f3e7395fa3 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -10,7 +10,6 @@ from functools import partial import yaml from django.conf import settings -from lazy import lazy from lxml import etree from opaque_keys.edx.asides import AsideDefinitionKeyV2, AsideUsageKeyV2 from opaque_keys.edx.keys import UsageKey @@ -314,9 +313,22 @@ class XModuleMixin(XModuleFields, XBlock): def __init__(self, *args, **kwargs): self._asides = [] + # Initialization data used by CachingDescriptorSystem to defer FieldData initialization + self._cds_init_args = kwargs.pop("cds_init_args", None) super().__init__(*args, **kwargs) + def get_cds_init_args(self): + """ Get initialization data used by CachingDescriptorSystem to defer FieldData initialization """ + if self._cds_init_args is None: + raise KeyError("cds_init_args was not provided for this XBlock") + if self._cds_init_args is False: + raise RuntimeError("Tried to get CachingDescriptorSystem cds_init_args twice for the same XBlock.") + args = self._cds_init_args + # Free the memory and set this False to flag any double-access bugs. This only needs to be read once. + self._cds_init_args = False + return args + @property def runtime(self): return self._runtime @@ -367,7 +379,7 @@ class XModuleMixin(XModuleFields, XBlock): def location(self, value): assert isinstance(value, UsageKey) self.scope_ids = self.scope_ids._replace( - def_id=value, + def_id=value, # Note: assigning a UsageKey as def_id is OK in old mongo / import system but wrong in split usage_id=value, ) @@ -417,14 +429,6 @@ class XModuleMixin(XModuleFields, XBlock): self.save() return self._field_data._kvs # pylint: disable=protected-access - @lazy - def _unwrapped_field_data(self): - """ - This property hold the value _field_data here before we wrap it in - the LmsFieldData or OverrideFieldData classes. - """ - return self._field_data - def add_aside(self, aside): """ save connected asides @@ -651,14 +655,17 @@ class XModuleMixin(XModuleFields, XBlock): if field in self._dirty_fields: del self._dirty_fields[field] - if wrappers is None: - wrappers = [] - - wrapped_field_data = self._unwrapped_field_data - for wrapper in wrappers: - wrapped_field_data = wrapper(wrapped_field_data) - - self._field_data = wrapped_field_data + if wrappers: + # Put user-specific wrappers around the field-data service for this block. + # Note that these are different from modulestore.xblock_field_data_wrappers, which are not user-specific. + wrapped_field_data = self.runtime.service(self, 'field-data-unbound') + for wrapper in wrappers: + wrapped_field_data = wrapper(wrapped_field_data) + self._bound_field_data = wrapped_field_data + if getattr(self.runtime, "uses_deprecated_field_data", False): + # This approach is deprecated but old mongo's CachingDescriptorSystem still requires it. + # For Split mongo's CachingDescriptor system, don't set ._field_data this way. + self._field_data = wrapped_field_data @property def non_editable_metadata_fields(self): diff --git a/xmodule/xml_block.py b/xmodule/xml_block.py index cb7c096ce9..45b7589357 100644 --- a/xmodule/xml_block.py +++ b/xmodule/xml_block.py @@ -360,6 +360,9 @@ class XmlMixin: field_data['children'] = children field_data['xml_attributes']['filename'] = definition.get('filename', ['', None]) # for git link + # TODO: we shouldn't be instantiating our own field data instance here, but rather just call to + # runtime.construct_xblock_from_class() and then set fields on the returned block. + # See the base XBlock class (XmlSerializationMixin.parse_xml) for how it should be done. kvs = InheritanceKeyValueStore(initial_values=field_data) field_data = KvsFieldData(kvs)