improve get_items and has_path_to_root with temporary caches.
This commit is contained in:
@@ -51,6 +51,7 @@ from xmodule.modulestore.edit_info import EditInfoRuntimeMixin
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError
|
||||
from xmodule.modulestore.inheritance import InheritanceMixin, inherit_metadata, InheritanceKeyValueStore
|
||||
from xmodule.modulestore.xml import CourseLocationManager
|
||||
from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
|
||||
from xmodule.services import SettingsService
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -77,8 +78,6 @@ BLOCK_TYPES_WITH_CHILDREN = list(set(
|
||||
# at module level, cache one instance of OSFS per filesystem root.
|
||||
_OSFS_INSTANCE = {}
|
||||
|
||||
_DETACHED_CATEGORIES = [name for name, __ in XBlock.load_tagged_classes("detached")]
|
||||
|
||||
|
||||
class MongoRevisionKey(object):
|
||||
"""
|
||||
@@ -269,7 +268,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
|
||||
)
|
||||
if parent_url:
|
||||
parent = self._convert_reference_to_key(parent_url)
|
||||
if not parent and category not in _DETACHED_CATEGORIES + ['course']:
|
||||
|
||||
if not parent and category not in DETACHED_XBLOCK_TYPES.union(['course']):
|
||||
# try looking it up just-in-time (but not if we're working with a detached block).
|
||||
parent = self.modulestore.get_parent_location(
|
||||
as_published(location),
|
||||
@@ -965,7 +965,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
|
||||
of inherited metadata onto the item
|
||||
"""
|
||||
category = item['location']['category']
|
||||
apply_cached_metadata = category not in _DETACHED_CATEGORIES and \
|
||||
apply_cached_metadata = category not in DETACHED_XBLOCK_TYPES and \
|
||||
not (category == 'course' and depth == 0)
|
||||
return apply_cached_metadata
|
||||
|
||||
|
||||
@@ -83,6 +83,7 @@ from ..exceptions import ItemNotFoundError
|
||||
from .caching_descriptor_system import CachingDescriptorSystem
|
||||
from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection, DuplicateKeyError
|
||||
from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope
|
||||
from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
|
||||
from xmodule.error_module import ErrorDescriptor
|
||||
from collections import defaultdict
|
||||
from types import NoneType
|
||||
@@ -1197,15 +1198,25 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
if 'category' in qualifiers:
|
||||
qualifiers['block_type'] = qualifiers.pop('category')
|
||||
|
||||
detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")]
|
||||
|
||||
# don't expect caller to know that children are in fields
|
||||
if 'children' in qualifiers:
|
||||
settings['children'] = qualifiers.pop('children')
|
||||
|
||||
# No need of these caches unless include_orphans is set to False
|
||||
path_cache = None
|
||||
parents_cache = None
|
||||
|
||||
if not include_orphans:
|
||||
path_cache = {}
|
||||
parents_cache = self.build_block_key_to_parents_mapping(course.structure)
|
||||
|
||||
for block_id, value in course.structure['blocks'].iteritems():
|
||||
if _block_matches_all(value):
|
||||
if not include_orphans:
|
||||
if self.has_path_to_root(block_id, course) or block_id.type in detached_categories:
|
||||
if ( # pylint: disable=bad-continuation
|
||||
block_id.type in DETACHED_XBLOCK_TYPES or
|
||||
self.has_path_to_root(block_id, course, path_cache, parents_cache)
|
||||
):
|
||||
items.append(block_id)
|
||||
else:
|
||||
items.append(block_id)
|
||||
@@ -1215,22 +1226,60 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
|
||||
else:
|
||||
return []
|
||||
|
||||
def has_path_to_root(self, block_key, course):
|
||||
def build_block_key_to_parents_mapping(self, structure):
|
||||
"""
|
||||
Given a structure, builds block_key to parents mapping for all block keys in structure
|
||||
and returns it
|
||||
|
||||
:param structure: db json of course structure
|
||||
|
||||
:return dict: a dictionary containing mapping of block_keys against their parents.
|
||||
"""
|
||||
children_to_parents = defaultdict(list)
|
||||
for parent_key, value in structure['blocks'].iteritems():
|
||||
for child_key in value.fields.get('children', []):
|
||||
children_to_parents[child_key].append(parent_key)
|
||||
|
||||
return children_to_parents
|
||||
|
||||
def has_path_to_root(self, block_key, course, path_cache=None, parents_cache=None):
|
||||
"""
|
||||
Check recursively if an xblock has a path to the course root
|
||||
|
||||
:param block_key: BlockKey of the component whose path is to be checked
|
||||
:param course: actual db json of course from structures
|
||||
:param path_cache: a dictionary that records which modules have a path to the root so that we don't have to
|
||||
double count modules if we're computing this for a list of modules in a course.
|
||||
:param parents_cache: a dictionary containing mapping of block_key to list of its parents. Optionally, this
|
||||
should be built for course structure to make this method faster.
|
||||
|
||||
:return Bool: whether or not component has path to the root
|
||||
"""
|
||||
|
||||
xblock_parents = self._get_parents_from_structure(block_key, course.structure)
|
||||
if path_cache and block_key in path_cache:
|
||||
return path_cache[block_key]
|
||||
|
||||
if parents_cache is None:
|
||||
xblock_parents = self._get_parents_from_structure(block_key, course.structure)
|
||||
else:
|
||||
xblock_parents = parents_cache[block_key]
|
||||
|
||||
if len(xblock_parents) == 0 and block_key.type in ["course", "library"]:
|
||||
# Found, xblock has the path to the root
|
||||
if path_cache is not None:
|
||||
path_cache[block_key] = True
|
||||
|
||||
return True
|
||||
|
||||
return any(self.has_path_to_root(xblock_parent, course) for xblock_parent in xblock_parents)
|
||||
has_path = any(
|
||||
self.has_path_to_root(xblock_parent, course, path_cache, parents_cache)
|
||||
for xblock_parent in xblock_parents
|
||||
)
|
||||
|
||||
if path_cache is not None:
|
||||
path_cache[block_key] = has_path
|
||||
|
||||
return has_path
|
||||
|
||||
def get_parent_location(self, locator, **kwargs):
|
||||
"""
|
||||
|
||||
@@ -3,6 +3,9 @@ import logging
|
||||
from collections import namedtuple
|
||||
|
||||
import uuid
|
||||
from xblock.core import XBlock
|
||||
|
||||
DETACHED_XBLOCK_TYPES = set(name for name, __ in XBlock.load_tagged_classes("detached"))
|
||||
|
||||
|
||||
def _prefix_only_url_replace_regex(pattern):
|
||||
|
||||
@@ -22,8 +22,6 @@ from pytz import UTC
|
||||
from shutil import rmtree
|
||||
from tempfile import mkdtemp
|
||||
|
||||
from xblock.core import XBlock
|
||||
|
||||
from xmodule.x_module import XModuleMixin
|
||||
from xmodule.modulestore.edit_info import EditInfoMixin
|
||||
from xmodule.modulestore.inheritance import InheritanceMixin
|
||||
@@ -44,6 +42,7 @@ from xmodule.modulestore.draft_and_published import UnsupportedRevisionError, DI
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError, NoPathToItem
|
||||
from xmodule.modulestore.mixed import MixedModuleStore
|
||||
from xmodule.modulestore.search import path_to_location, navigation_index
|
||||
from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES
|
||||
from xmodule.modulestore.tests.factories import check_mongo_calls, check_exact_number_of_calls, \
|
||||
mongo_uses_error_check
|
||||
from xmodule.modulestore.tests.utils import create_modulestore_instance, LocationMixin, mock_tab_from_json
|
||||
@@ -463,16 +462,13 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
|
||||
test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
|
||||
course_key = test_course.id
|
||||
|
||||
# get detached category list
|
||||
detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")]
|
||||
|
||||
items = self.store.get_items(course_key)
|
||||
# Check items found are either course or about type
|
||||
self.assertTrue(set(['course', 'about']).issubset(set([item.location.block_type for item in items])))
|
||||
# Assert that about is a detached category found in get_items
|
||||
self.assertIn(
|
||||
[item.location.block_type for item in items if item.location.block_type == 'about'][0],
|
||||
detached_categories
|
||||
DETACHED_XBLOCK_TYPES
|
||||
)
|
||||
self.assertEqual(len(items), 2)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user