From 8053b7d90c6128f90032e28ca4b6872e3d70927f Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Fri, 27 May 2022 15:55:12 +0500 Subject: [PATCH] refactor: replace coursekey.course with coursekey.library (#30398) --- .../djangoapps/split_modulestore_django/models.py | 3 ++- common/lib/xmodule/xmodule/contentstore/mongo.py | 4 ++-- common/lib/xmodule/xmodule/modulestore/__init__.py | 5 ++++- .../split_mongo/caching_descriptor_system.py | 6 ++++-- .../xmodule/modulestore/split_mongo/split.py | 11 ++++++----- common/lib/xmodule/xmodule/util/misc.py | 14 ++++++++++++++ 6 files changed, 32 insertions(+), 11 deletions(-) diff --git a/common/djangoapps/split_modulestore_django/models.py b/common/djangoapps/split_modulestore_django/models.py index b3f8b6d145..ddcdaf5308 100644 --- a/common/djangoapps/split_modulestore_django/models.py +++ b/common/djangoapps/split_modulestore_django/models.py @@ -9,6 +9,7 @@ from opaque_keys.edx.django.models import LearningContextKeyField from simple_history.models import HistoricalRecords from xmodule.modulestore import ModuleStoreEnum +from xmodule.util.misc import get_library_or_course_attribute User = get_user_model() @@ -92,7 +93,7 @@ class SplitModulestoreCourseIndex(models.Model): return { "_id": ObjectId(self.objectid), "org": self.course_id.org, - "course": self.course_id.course, + "course": get_library_or_course_attribute(self.course_id), "run": self.course_id.run, # pylint: disable=no-member "edited_by": self.edited_by_id, "edited_on": self.edited_on, diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index 2a9848bc2a..66d9474cde 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -18,7 +18,7 @@ from xmodule.contentstore.content import XASSET_LOCATION_TAG from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import ASSET_IGNORE_REGEX from xmodule.mongo_utils import connect_to_mongodb, create_collection_index -from xmodule.util.misc import escape_invalid_characters +from xmodule.util.misc import escape_invalid_characters, get_library_or_course_attribute from .content import ContentStore, StaticContent, StaticContentStream @@ -614,7 +614,7 @@ def query_for_course(course_key, category=None): dbkey = SON([ (f'{prefix}.tag', XASSET_LOCATION_TAG), (f'{prefix}.org', course_key.org), - (f'{prefix}.course', course_key.course), + (f'{prefix}.course', get_library_or_course_attribute(course_key)), ]) if category: dbkey[f'{prefix}.category'] = category diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index ff6a04bd67..c7224147f0 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -26,6 +26,7 @@ from xblock.runtime import Mixologist from openedx.core.lib.json_utils import EdxJSONEncoder from xmodule.assetstore import AssetMetadata from xmodule.errortracker import make_error_tracker +from xmodule.util.misc import get_library_or_course_attribute from .exceptions import InsufficientSpecificationError, InvalidLocationError @@ -203,9 +204,11 @@ class BulkOperationsMixin: if ignore_case: for key, record in self._active_bulk_ops.records.items(): # Shortcut: check basic equivalence for cases where org/course/run might be None. + key_library = get_library_or_course_attribute(key) + course_library = get_library_or_course_attribute(course_key) if (key == course_key) or ( # lint-amnesty, pylint: disable=too-many-boolean-expressions (key.org and key.org.lower() == course_key.org.lower()) and - (key.course and key.course.lower() == course_key.course.lower()) and + (key_library and key_library.lower() == course_library.lower()) and (key.run and key.run.lower() == course_key.run.lower()) ): return record diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index a2df7f1a3d..6849cc2c68 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -20,6 +20,7 @@ from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.split_mongo.definition_lazy_loader import DefinitionLazyLoader from xmodule.modulestore.split_mongo.id_manager import SplitMongoIdManager from xmodule.modulestore.split_mongo.split_mongo_kvs import SplitMongoKVS +from xmodule.util.misc import get_library_or_course_attribute from xmodule.x_module import XModuleMixin log = logging.getLogger(__name__) @@ -47,8 +48,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): # li underlying modulestore """ # needed by capa_problem (as runtime.resources_fs via this.resources_fs) - if course_entry.course_key.course: - root = modulestore.fs_root / course_entry.course_key.org / course_entry.course_key.course / course_entry.course_key.run # lint-amnesty, pylint: disable=line-too-long + course_library = get_library_or_course_attribute(course_entry.course_key) + if course_library: + root = modulestore.fs_root / course_entry.course_key.org / course_library / course_entry.course_key.run # lint-amnesty, pylint: disable=line-too-long else: root = modulestore.fs_root / str(course_entry.structure['_id']) root.makedirs_p() # create directory if it doesn't exist diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 7878b85d07..3ff9ebc38f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -104,6 +104,7 @@ from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope from xmodule.modulestore.split_mongo.mongo_connection import DuplicateKeyError, DjangoFlexPersistenceBackend from xmodule.modulestore.store_utilities import DETACHED_XBLOCK_TYPES from xmodule.partitions.partitions_service import PartitionService +from xmodule.util.misc import get_library_or_course_attribute from ..exceptions import ItemNotFoundError from .caching_descriptor_system import CachingDescriptorSystem @@ -214,7 +215,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin): if not isinstance(course_key, (CourseLocator, LibraryLocator)): raise TypeError(f'{course_key!r} is not a CourseLocator or LibraryLocator') # handle version_guid based retrieval locally - if course_key.org is None or course_key.course is None or course_key.run is None: + if course_key.org is None or get_library_or_course_attribute(course_key) is None or course_key.run is None: return self._active_bulk_ops.records[ course_key.replace(org=None, course=None, run=None, branch=None) ] @@ -231,7 +232,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin): if not isinstance(course_key, (CourseLocator, LibraryLocator)): raise TypeError(f'{course_key!r} is not a CourseLocator or LibraryLocator') - if course_key.org and course_key.course and course_key.run: + if course_key.org and get_library_or_course_attribute(course_key) and course_key.run: del self._active_bulk_ops.records[course_key.replace(branch=None, version_guid=None)] else: del self._active_bulk_ops.records[ @@ -821,7 +822,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ if not course_key.version_guid: head_validation = True - if head_validation and course_key.org and course_key.course and course_key.run: + if head_validation and course_key.org and get_library_or_course_attribute(course_key) and course_key.run: if course_key.branch is None: raise InsufficientSpecificationError(course_key) @@ -1897,7 +1898,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): index_entry = { '_id': ObjectId(), 'org': locator.org, - 'course': locator.course, + 'course': get_library_or_course_attribute(locator), 'run': locator.run, 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), @@ -2923,7 +2924,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): :param force: if false, raises VersionConflictError if the current head of the course != the one identified by course_key """ - if course_key.org is None or course_key.course is None or course_key.run is None or course_key.branch is None: + if course_key.org is None or get_library_or_course_attribute(course_key) is None or course_key.run is None or course_key.branch is None: return None else: index_entry = self.get_course_index(course_key) diff --git a/common/lib/xmodule/xmodule/util/misc.py b/common/lib/xmodule/xmodule/util/misc.py index 29c2893469..d6dda5aa51 100644 --- a/common/lib/xmodule/xmodule/util/misc.py +++ b/common/lib/xmodule/xmodule/util/misc.py @@ -6,6 +6,10 @@ Miscellaneous utility functions. import re from xmodule.annotator_mixin import html_to_text +from opaque_keys.edx.locator import ( + CourseLocator, + LibraryLocator, +) def escape_invalid_characters(name, invalid_char_list, replace_with='_'): @@ -110,3 +114,13 @@ def is_xblock_an_assignment(xblock): weight = getattr(xblock, 'weight', 1) scored = has_score and (weight is None or weight > 0) return graded and scored + + +def get_library_or_course_attribute(locator): + """ + Returns respective attribute to distinguish betweeen LibraryLocator and CourseLocator objects. + """ + if isinstance(locator, CourseLocator): + return locator.course + if isinstance(locator, LibraryLocator): + return locator.library