diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index 5001712f15..e146cb914c 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -29,8 +29,10 @@ from xmodule.modulestore.tests.django_utils import ( ) from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST -from xmodule.modulestore.tests.test_cross_modulestore_import_export import MongoContentstoreBuilder -from xmodule.modulestore.tests.utils import create_modulestore_instance, LocationMixin, MixedSplitTestCase +from xmodule.modulestore.tests.utils import ( + create_modulestore_instance, LocationMixin, + MixedSplitTestCase, MongoContentstoreBuilder +) from xmodule.tests import DATA_DIR from xmodule.x_module import XModuleMixin from xmodule.partitions.partitions import UserPartition diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index cb64b0ba6f..32a04f1083 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -315,7 +315,8 @@ class BulkOperationsMixin(object): Sends out the signal that items have been published from within this course. """ if self.signal_handler and bulk_ops_record.has_publish_item: - self.signal_handler.send("course_published", course_key=course_id) + # We remove the branch, because publishing always means copying from draft to published + self.signal_handler.send("course_published", course_key=course_id.for_branch(None)) bulk_ops_record.has_publish_item = False def send_bulk_library_updated_signal(self, bulk_ops_record, library_id): @@ -1345,22 +1346,6 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): parent.children.append(item.location) self.update_item(parent, user_id) - def _flag_publish_event(self, course_key): - """ - Wrapper around calls to fire the course_published signal - Unless we're nested in an active bulk operation, this simply fires the signal - otherwise a publish will be signalled at the end of the bulk operation - - Arguments: - course_key - course_key to which the signal applies - """ - if self.signal_handler: - bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None - if bulk_record and bulk_record.active: - bulk_record.has_publish_item = True - else: - self.signal_handler.send("course_published", course_key=course_key) - def _flag_library_updated_event(self, library_key): """ Wrapper around calls to fire the library_updated signal diff --git a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py index 27895ce06d..79f567e66b 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py +++ b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py @@ -5,7 +5,7 @@ This module provides an abstraction for Module Stores that support Draft and Pub import threading from abc import ABCMeta, abstractmethod from contextlib import contextmanager -from . import ModuleStoreEnum +from . import ModuleStoreEnum, BulkOperationsMixin # Things w/ these categories should never be marked as version=DRAFT DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info'] @@ -62,7 +62,7 @@ class BranchSettingMixin(object): return self.default_branch_setting_func() -class ModuleStoreDraftAndPublished(BranchSettingMixin): +class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin): """ A mixin for a read-write database backend that supports two branches, Draft and Published, with options to prefer Draft and fallback to Published. @@ -87,6 +87,11 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin): @abstractmethod def unpublish(self, location, user_id): + """ + Turn the published version into a draft, removing the published version. + + Raises: InvalidVersionError if called on a DIRECT_ONLY_CATEGORY + """ raise NotImplementedError @abstractmethod @@ -112,6 +117,23 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin): """ raise NotImplementedError + def _flag_publish_event(self, course_key): + """ + Wrapper around calls to fire the course_published signal + Unless we're nested in an active bulk operation, this simply fires the signal + otherwise a publish will be signalled at the end of the bulk operation + + Arguments: + course_key - course_key to which the signal applies + """ + if self.signal_handler: + bulk_record = self._get_bulk_ops_record(course_key) if isinstance(self, BulkOperationsMixin) else None + if bulk_record and bulk_record.active: + bulk_record.has_publish_item = True + else: + # We remove the branch, because publishing always means copying from draft to published + self.signal_handler.send("course_published", course_key=course_key.for_branch(None)) + class UnsupportedRevisionError(ValueError): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 6e6335d3bd..b75a337e63 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -754,6 +754,10 @@ class DraftModuleStore(MongoModuleStore): NOTE: unlike publish, this gives an error if called above the draftable level as it's intended to remove things from the published version """ + # ensure we are not creating a DRAFT of an item that is direct-only + if location.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + self._verify_branch_setting(ModuleStoreEnum.Branch.draft_preferred) self._convert_to_draft(location, user_id, delete_published=True) diff --git a/common/lib/xmodule/xmodule/modulestore/perf_tests/test_asset_import_export.py b/common/lib/xmodule/xmodule/modulestore/perf_tests/test_asset_import_export.py index b14b391129..ae29396c46 100644 --- a/common/lib/xmodule/xmodule/modulestore/perf_tests/test_asset_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/perf_tests/test_asset_import_export.py @@ -16,7 +16,7 @@ from xmodule.assetstore import AssetMetadata from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml -from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( +from xmodule.modulestore.tests.utils import ( MODULESTORE_SETUPS, SHORT_NAME_MAP, TEST_DATA_DIR, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index 9995f0e6be..739fcf32ea 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -13,7 +13,13 @@ from time import time # Import this just to export it from pymongo.errors import DuplicateKeyError # pylint: disable=unused-import -from django.core.cache import get_cache, InvalidCacheBackendError + +try: + from django.core.cache import get_cache, InvalidCacheBackendError + DJANGO_AVAILABLE = True +except ImportError: + DJANGO_AVAILABLE = False + import dogstats_wrapper as dog_stats_api from contracts import check, new_contract @@ -216,15 +222,16 @@ class CourseStructureCache(object): for set and get. """ def __init__(self): - self.no_cache_found = False - try: - self.cache = get_cache('course_structure_cache') - except InvalidCacheBackendError: - self.no_cache_found = True + self.cache = None + if DJANGO_AVAILABLE: + try: + self.cache = get_cache('course_structure_cache') + except InvalidCacheBackendError: + pass def get(self, key, course_context=None): """Pull the compressed, pickled struct data from cache and deserialize.""" - if self.no_cache_found: + if self.cache is None: return None with TIMER.timer("CourseStructureCache.get", course_context) as tagger: @@ -245,7 +252,7 @@ class CourseStructureCache(object): def set(self, key, structure, course_context=None): """Given a structure, will pickle, compress, and write to cache.""" - if self.no_cache_found: + if self.cache is None: return None with TIMER.timer("CourseStructureCache.set", course_context) as tagger: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f7239b13e2..9027f7290c 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -2383,7 +2383,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if original_structure['root'] == block_key: raise ValueError("Cannot delete the root of a course") if block_key not in original_structure['blocks']: - raise ValueError("Cannot delete a block that does not exist") + raise ValueError("Cannot delete block_key {} from course {}, because that block does not exist.".format( + block_key, + usage_locator, + )) index_entry = self._get_index_if_valid(usage_locator.course_key, force) new_structure = self.version_structure(usage_locator.course_key, original_structure, user_id) new_blocks = new_structure['blocks'] @@ -3034,9 +3037,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): Delete the orphan and any of its descendants which no longer have parents. """ if len(self._get_parents_from_structure(orphan, structure)) == 0: - for child in structure['blocks'][orphan].fields.get('children', []): + orphan_data = structure['blocks'].pop(orphan) + for child in orphan_data.fields.get('children', []): self._delete_if_true_orphan(BlockKey(*child), structure) - del structure['blocks'][orphan] @contract(returns=BlockData) def _new_block(self, user_id, category, block_fields, definition_id, new_id, raw=False, block_defaults=None): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 55d7b6872d..bdd4696106 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -189,39 +189,41 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli currently only provided by contentstore.views.item.orphan_handler Otherwise, raises a ValueError. """ + allowed_revisions = [ + None, + ModuleStoreEnum.RevisionOption.published_only, + ModuleStoreEnum.RevisionOption.all + ] + if revision not in allowed_revisions: + raise UnsupportedRevisionError(allowed_revisions) + + autopublish_parent = False with self.bulk_operations(location.course_key): if isinstance(location, LibraryUsageLocator): branches_to_delete = [ModuleStoreEnum.BranchName.library] # Libraries don't yet have draft/publish support - elif revision == ModuleStoreEnum.RevisionOption.published_only: - branches_to_delete = [ModuleStoreEnum.BranchName.published] + elif location.category in DIRECT_ONLY_CATEGORIES: + branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft] elif revision == ModuleStoreEnum.RevisionOption.all: branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft] - elif revision is None: - branches_to_delete = [ModuleStoreEnum.BranchName.draft] else: - raise UnsupportedRevisionError( - [ - None, - ModuleStoreEnum.RevisionOption.published_only, - ModuleStoreEnum.RevisionOption.all - ] - ) + if revision == ModuleStoreEnum.RevisionOption.published_only: + branches_to_delete = [ModuleStoreEnum.BranchName.published] + elif revision is None: + branches_to_delete = [ModuleStoreEnum.BranchName.draft] + parent_loc = self.get_parent_location(location.for_branch(ModuleStoreEnum.BranchName.draft)) + autopublish_parent = ( + not skip_auto_publish and + parent_loc is not None and + parent_loc.block_type in DIRECT_ONLY_CATEGORIES + ) self._flag_publish_event(location.course_key) for branch in branches_to_delete: branched_location = location.for_branch(branch) - parent_loc = self.get_parent_location(branched_location) - SplitMongoModuleStore.delete_item(self, branched_location, user_id) - # publish parent w/o child if deleted element is direct only (not based on type of parent) - # publish vertical to behave more like the old mongo/draft modulestore - TNL-2593 - if ( - branch == ModuleStoreEnum.BranchName.draft and - branched_location.block_type in (DIRECT_ONLY_CATEGORIES + ['vertical']) and - parent_loc and - not skip_auto_publish - ): - # will publish if its not an orphan - self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) + super(DraftVersioningModuleStore, self).delete_item(branched_location, user_id) + + if autopublish_parent: + self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) def _map_revision_to_branch(self, key, revision=None): """ @@ -375,6 +377,9 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli Deletes the published version of the item. Returns the newly unpublished item. """ + if location.block_type in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + with self.bulk_operations(location.course_key): self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft), **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py b/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py index 80ca48cf6d..2a16ace037 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_asides.py @@ -5,7 +5,7 @@ from xblock.core import XBlockAside from xblock.fields import Scope, String from xblock.fragment import Fragment from unittest import TestCase -from xmodule.modulestore.tests.test_cross_modulestore_import_export import XmlModulestoreBuilder +from xmodule.modulestore.tests.utils import XmlModulestoreBuilder from mock import patch diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index b859d502a9..b37da813c5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -14,7 +14,7 @@ from xmodule.assetstore import AssetMetadata from xmodule.modulestore import ModuleStoreEnum, SortedAssetList, IncorrectlySortedList from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( +from xmodule.modulestore.tests.utils import ( MIXED_MODULESTORE_BOTH_SETUP, MODULESTORE_SETUPS, XmlModulestoreBuilder, MixedModulestoreBuilder ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index a5e7f94b50..c4c4974b84 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -15,7 +15,6 @@ from contextlib import contextmanager, nested import itertools import os from path import Path as path -import random from shutil import rmtree from tempfile import mkdtemp @@ -24,326 +23,18 @@ from nose.plugins.attrib import attr from mock import patch from xmodule.tests import CourseComparisonTest -from xmodule.modulestore.mongo.base import ModuleStoreEnum -from xmodule.modulestore.mongo.draft import DraftModuleStore -from xmodule.modulestore.mixed import MixedModuleStore -from xmodule.contentstore.mongo import MongoContentStore from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml -from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.utils import mock_tab_from_json from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.partitions.tests.test_partitions import PartitionTestCase from xmodule.x_module import XModuleMixin -from xmodule.modulestore.xml import XMLModuleStore - -TEST_DATA_DIR = 'common/test/data/' - - -COMMON_DOCSTORE_CONFIG = { - 'host': MONGO_HOST, - 'port': MONGO_PORT_NUM, -} -DATA_DIR = path(__file__).dirname().parent.parent / "tests" / "data" / "xml-course-root" - -XBLOCK_MIXINS = (InheritanceMixin, XModuleMixin) - - -class MemoryCache(object): - """ - This fits the metadata_inheritance_cache_subsystem interface used by - the modulestore, and stores the data in a dictionary in memory. - """ - def __init__(self): - self._data = {} - - def get(self, key, default=None): - """ - Get a key from the cache. - - Args: - key: The key to update. - default: The value to return if the key hasn't been set previously. - """ - return self._data.get(key, default) - - def set(self, key, value): - """ - Set a key in the cache. - - Args: - key: The key to update. - value: The value change the key to. - """ - self._data[key] = value - - -class MongoContentstoreBuilder(object): - """ - A builder class for a MongoContentStore. - """ - @contextmanager - def build(self): - """ - A contextmanager that returns a MongoContentStore, and deletes its contents - when the context closes. - """ - contentstore = MongoContentStore( - db='contentstore{}'.format(random.randint(0, 10000)), - collection='content', - **COMMON_DOCSTORE_CONFIG - ) - contentstore.ensure_indexes() - - try: - yield contentstore - finally: - # Delete the created database - contentstore._drop_database() # pylint: disable=protected-access - - def __repr__(self): - return 'MongoContentstoreBuilder()' - - -class StoreBuilderBase(object): - """ - Base class for all modulestore builders. - """ - @contextmanager - def build(self, **kwargs): - """ - Build the modulstore, optionally building the contentstore as well. - """ - contentstore = kwargs.pop('contentstore', None) - if not contentstore: - with self.build_without_contentstore() as (contentstore, modulestore): - yield contentstore, modulestore - else: - with self.build_with_contentstore(contentstore) as modulestore: - yield modulestore - - @contextmanager - def build_without_contentstore(self): - """ - Build both the contentstore and the modulestore. - """ - with MongoContentstoreBuilder().build() as contentstore: - with self.build_with_contentstore(contentstore) as modulestore: - yield contentstore, modulestore - - -class MongoModulestoreBuilder(StoreBuilderBase): - """ - A builder class for a DraftModuleStore. - """ - @contextmanager - def build_with_contentstore(self, contentstore): - """ - A contextmanager that returns an isolated mongo modulestore, and then deletes - all of its data at the end of the context. - - Args: - contentstore: The contentstore that this modulestore should use to store - all of its assets. - """ - doc_store_config = dict( - db='modulestore{}'.format(random.randint(0, 10000)), - collection='xmodule', - asset_collection='asset_metadata', - **COMMON_DOCSTORE_CONFIG - ) - - # Set up a temp directory for storing filesystem content created during import - fs_root = mkdtemp() - - # pylint: disable=attribute-defined-outside-init - modulestore = DraftModuleStore( - contentstore, - doc_store_config, - fs_root, - render_template=repr, - branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, - metadata_inheritance_cache_subsystem=MemoryCache(), - xblock_mixins=XBLOCK_MIXINS, - ) - modulestore.ensure_indexes() - - try: - yield modulestore - finally: - # Delete the created database - modulestore._drop_database() # pylint: disable=protected-access - - # Delete the created directory on the filesystem - rmtree(fs_root, ignore_errors=True) - - def __repr__(self): - return 'MongoModulestoreBuilder()' - - -class VersioningModulestoreBuilder(StoreBuilderBase): - """ - A builder class for a VersioningModuleStore. - """ - @contextmanager - def build_with_contentstore(self, contentstore): - """ - A contextmanager that returns an isolated versioning modulestore, and then deletes - all of its data at the end of the context. - - Args: - contentstore: The contentstore that this modulestore should use to store - all of its assets. - """ - # pylint: disable=unreachable - doc_store_config = dict( - db='modulestore{}'.format(random.randint(0, 10000)), - collection='split_module', - **COMMON_DOCSTORE_CONFIG - ) - # Set up a temp directory for storing filesystem content created during import - fs_root = mkdtemp() - - modulestore = DraftVersioningModuleStore( - contentstore, - doc_store_config, - fs_root, - render_template=repr, - xblock_mixins=XBLOCK_MIXINS, - ) - modulestore.ensure_indexes() - - try: - yield modulestore - finally: - # Delete the created database - modulestore._drop_database() # pylint: disable=protected-access - - # Delete the created directory on the filesystem - rmtree(fs_root, ignore_errors=True) - - def __repr__(self): - return 'SplitModulestoreBuilder()' - - -class XmlModulestoreBuilder(StoreBuilderBase): - """ - A builder class for a XMLModuleStore. - """ - # pylint: disable=unused-argument - @contextmanager - def build_with_contentstore(self, contentstore=None, course_ids=None): - """ - A contextmanager that returns an isolated xml modulestore - - Args: - contentstore: The contentstore that this modulestore should use to store - all of its assets. - """ - modulestore = XMLModuleStore( - DATA_DIR, - course_ids=course_ids, - default_class='xmodule.hidden_module.HiddenDescriptor', - xblock_mixins=XBLOCK_MIXINS, - ) - - yield modulestore - - -class MixedModulestoreBuilder(StoreBuilderBase): - """ - A builder class for a MixedModuleStore. - """ - def __init__(self, store_builders, mappings=None): - """ - Args: - store_builders: A list of modulestore builder objects. These will be instantiated, in order, - as the backing stores for the MixedModuleStore. - mappings: Any course mappings to pass to the MixedModuleStore on instantiation. - """ - self.store_builders = store_builders - self.mappings = mappings or {} - self.mixed_modulestore = None - - @contextmanager - def build_with_contentstore(self, contentstore): - """ - A contextmanager that returns a mixed modulestore built on top of modulestores - generated by other builder classes. - - Args: - contentstore: The contentstore that this modulestore should use to store - all of its assets. - """ - names, generators = zip(*self.store_builders) - - with nested(*(gen.build_with_contentstore(contentstore) for gen in generators)) as modulestores: - # Make the modulestore creation function just return the already-created modulestores - store_iterator = iter(modulestores) - create_modulestore_instance = lambda *args, **kwargs: store_iterator.next() - - # Generate a fake list of stores to give the already generated stores appropriate names - stores = [{'NAME': name, 'ENGINE': 'This space deliberately left blank'} for name in names] - - self.mixed_modulestore = MixedModuleStore( - contentstore, - self.mappings, - stores, - create_modulestore_instance=create_modulestore_instance, - xblock_mixins=XBLOCK_MIXINS, - ) - - yield self.mixed_modulestore - - def __repr__(self): - return 'MixedModulestoreBuilder({!r}, {!r})'.format(self.store_builders, self.mappings) - - def asset_collection(self): - """ - Returns the collection storing the asset metadata. - """ - all_stores = self.mixed_modulestore.modulestores - if len(all_stores) > 1: - return None - - store = all_stores[0] - if hasattr(store, 'asset_collection'): - # Mongo modulestore beneath mixed. - # Returns the entire collection with *all* courses' asset metadata. - return store.asset_collection - else: - # Split modulestore beneath mixed. - # Split stores all asset metadata in the structure collection. - return store.db_connection.structures - -MIXED_MODULESTORE_BOTH_SETUP = MixedModulestoreBuilder([ - ('draft', MongoModulestoreBuilder()), - ('split', VersioningModulestoreBuilder()) -]) -DRAFT_MODULESTORE_SETUP = MixedModulestoreBuilder([('draft', MongoModulestoreBuilder())]) -SPLIT_MODULESTORE_SETUP = MixedModulestoreBuilder([('split', VersioningModulestoreBuilder())]) -MIXED_MODULESTORE_SETUPS = ( - DRAFT_MODULESTORE_SETUP, - SPLIT_MODULESTORE_SETUP, +from xmodule.modulestore.tests.utils import ( + MongoContentstoreBuilder, MODULESTORE_SETUPS, SPLIT_MODULESTORE_SETUP, + CONTENTSTORE_SETUPS, TEST_DATA_DIR ) -MIXED_MS_SETUPS_SHORT = ( - 'mixed_mongo', - 'mixed_split', -) -DIRECT_MODULESTORE_SETUPS = ( - MongoModulestoreBuilder(), - # VersioningModulestoreBuilder(), # FUTUREDO: LMS-11227 -) -DIRECT_MS_SETUPS_SHORT = ( - 'mongo', - #'split', -) -MODULESTORE_SETUPS = DIRECT_MODULESTORE_SETUPS + MIXED_MODULESTORE_SETUPS -MODULESTORE_SHORTNAMES = DIRECT_MS_SETUPS_SHORT + MIXED_MS_SETUPS_SHORT -SHORT_NAME_MAP = dict(zip(MODULESTORE_SETUPS, MODULESTORE_SHORTNAMES)) -CONTENTSTORE_SETUPS = (MongoContentstoreBuilder(),) COURSE_DATA_NAMES = ( 'toy', 'manual-testing-complete', diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index ac5895cb1b..9685a5eb3e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -9,14 +9,13 @@ import itertools import mimetypes from uuid import uuid4 from contextlib import contextmanager -from mock import patch +from mock import patch, Mock, call # Mixed modulestore depends on django, so we'll manually configure some django settings # before importing the module # TODO remove this import and the configuration -- xmodule should not depend on django! from django.conf import settings # This import breaks this test file when run separately. Needs to be fixed! (PLAT-449) -from mock_django import mock_signal_receiver from nose.plugins.attrib import attr import pymongo from pytz import UTC @@ -26,12 +25,11 @@ from tempfile import mkdtemp from xmodule.x_module import XModuleMixin from xmodule.modulestore.edit_info import EditInfoMixin from xmodule.modulestore.inheritance import InheritanceMixin -from xmodule.modulestore.tests.test_cross_modulestore_import_export import MongoContentstoreBuilder +from xmodule.modulestore.tests.utils import MongoContentstoreBuilder from xmodule.contentstore.content import StaticContent from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml -from xmodule.modulestore.django import SignalHandler if not settings.configured: settings.configure() @@ -93,19 +91,19 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): OPTIONS = { 'stores': [ { - 'NAME': 'draft', + 'NAME': ModuleStoreEnum.Type.mongo, 'ENGINE': 'xmodule.modulestore.mongo.draft.DraftModuleStore', 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options }, { - 'NAME': 'split', + 'NAME': ModuleStoreEnum.Type.split, 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options }, { - 'NAME': 'xml', + 'NAME': ModuleStoreEnum.Type.xml, 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', 'OPTIONS': { 'data_dir': DATA_DIR, @@ -291,8 +289,11 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( category='chapter', name='Overview' ) + self._create_course(self.course_locations[self.MONGO_COURSEID].course_key) + self.assertEquals(default, self.store.get_modulestore_type(self.course.id)) + @ddt.ddt @attr('mongo') @@ -300,7 +301,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): """ Tests of the MixedModulestore interface methods. """ - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_modulestore_type(self, default_ms): """ Make sure we get back the store type we expect for given mappings @@ -312,16 +313,15 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertEqual(self.store.get_modulestore_type( self._course_key_from_string(self.XML_COURSEID2)), ModuleStoreEnum.Type.xml ) - mongo_ms_type = ModuleStoreEnum.Type.mongo if default_ms == 'draft' else ModuleStoreEnum.Type.split self.assertEqual(self.store.get_modulestore_type( - self._course_key_from_string(self.MONGO_COURSEID)), mongo_ms_type + self._course_key_from_string(self.MONGO_COURSEID)), default_ms ) # try an unknown mapping, it should be the 'default' store self.assertEqual(self.store.get_modulestore_type( - SlashSeparatedCourseKey('foo', 'bar', '2012_Fall')), mongo_ms_type + SlashSeparatedCourseKey('foo', 'bar', '2012_Fall')), default_ms ) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_modulestore_cache(self, default_ms): """ Make sure we cache discovered course mappings @@ -356,7 +356,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # problem: One lookup to locate an item that exists # fake: one w/ wildcard version # split has one lookup for the course and then one for the course items - @ddt.data(('draft', [1, 1], 0), ('split', [2, 2], 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, [1, 1], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) @ddt.unpack def test_has_item(self, default_ms, max_find, max_send): self.initdb(default_ms) @@ -384,7 +384,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # split: # problem: active_versions, structure # non-existent problem: ditto - @ddt.data(('draft', [3, 2], 0), ('split', [2, 2], 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, [3, 2], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) @ddt.unpack def test_get_item(self, default_ms, max_find, max_send): self.initdb(default_ms) @@ -412,7 +412,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # wildcard query, 6! load pertinent items for inheritance calls, load parents, course root fetch (why) # Split: # active_versions (with regex), structure, and spurious active_versions refetch - @ddt.data(('draft', 14, 0), ('split', 3, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 14, 0), (ModuleStoreEnum.Type.split, 3, 0)) @ddt.unpack def test_get_items(self, default_ms, max_find, max_send): self.initdb(default_ms) @@ -440,7 +440,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # sends: update problem and then each ancestor up to course (edit info) # split: active_versions, definitions (calculator field), structures # 2 sends to update index & structure (note, it would also be definition if a content field changed) - @ddt.data(('draft', 7, 5), ('split', 3, 2)) + @ddt.data((ModuleStoreEnum.Type.mongo, 7, 5), (ModuleStoreEnum.Type.split, 3, 2)) @ddt.unpack def test_update_item(self, default_ms, max_find, max_send): """ @@ -465,7 +465,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertEqual(problem.max_attempts, 2, "Update didn't persist") - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes_direct_only(self, default_ms): """ Tests that has_changes() returns false when a new xblock in a direct only category is checked @@ -486,7 +486,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertFalse(self.store.has_changes(test_course)) self.assertFalse(self.store.has_changes(chapter)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes(self, default_ms): """ Tests that has_changes() only returns true when changes are present @@ -521,7 +521,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): component = self.store.publish(component.location, self.user_id) self.assertFalse(self.store.has_changes(component)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_unit_stuck_in_draft_mode(self, default_ms): """ After revert_to_published() the has_changes() should return false if draft has no changes @@ -553,7 +553,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): component = self.store.publish(component.location, self.user_id) self.assertFalse(self.store.has_changes(component)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_unit_stuck_in_published_mode(self, default_ms): """ After revert_to_published() the has_changes() should return true if draft has changes @@ -590,7 +590,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Verify that changes are present self.assertTrue(self.store.has_changes(component)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_unit_stuck_in_published_mode_after_delete(self, default_ms): """ Test that a unit does not get stuck in published mode @@ -633,7 +633,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): vertical = self.store.get_item(vertical.location) self.assertTrue(self._has_changes(vertical.location)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_publish_automatically_after_delete_unit(self, default_ms): """ Check that sequential publishes automatically after deleting a unit @@ -676,7 +676,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): return locations - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes_ancestors(self, default_ms): """ Tests that has_changes() returns true on ancestors when a child is changed @@ -706,7 +706,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): for key in locations: self.assertFalse(self._has_changes(locations[key])) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes_publish_ancestors(self, default_ms): """ Tests that has_changes() returns false after a child is published only if all children are unchanged @@ -743,7 +743,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertFalse(self._has_changes(locations['grandparent'])) self.assertFalse(self._has_changes(locations['parent'])) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes_add_remove_child(self, default_ms): """ Tests that has_changes() returns true for the parent when a child with changes is added @@ -776,7 +776,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertFalse(self._has_changes(locations['grandparent'])) self.assertFalse(self._has_changes(locations['parent'])) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_has_changes_non_direct_only_children(self, default_ms): """ Tests that has_changes() returns true after editing the child of a vertical (both not direct only categories). @@ -810,7 +810,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertTrue(self._has_changes(child.location)) @ddt.data(*itertools.product( - ('draft', 'split'), + (ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split), (ModuleStoreEnum.Branch.draft_preferred, ModuleStoreEnum.Branch.published_only) )) @ddt.unpack @@ -842,14 +842,14 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Split # Find: active_versions, 2 structures (published & draft), definition (unnecessary) # Sends: updated draft and published structures and active_versions - @ddt.data(('draft', 7, 2), ('split', 4, 3)) + @ddt.data((ModuleStoreEnum.Type.mongo, 7, 2), (ModuleStoreEnum.Type.split, 3, 3)) @ddt.unpack def test_delete_item(self, default_ms, max_find, max_send): """ Delete should reject on r/o db and work on r/w one """ self.initdb(default_ms) - if default_ms == 'draft' and mongo_uses_error_check(self.store): + if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store): max_find += 1 # r/o try deleting the chapter (is here to ensure it can't be deleted) @@ -874,7 +874,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Split: # queries: active_versions, draft and published structures, definition (unnecessary) # sends: update published (why?), draft, and active_versions - @ddt.data(('draft', 9, 2), ('split', 4, 3)) + @ddt.data((ModuleStoreEnum.Type.mongo, 9, 2), (ModuleStoreEnum.Type.split, 4, 3)) @ddt.unpack def test_delete_private_vertical(self, default_ms, max_find, max_send): """ @@ -882,7 +882,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): behavioral properties which this deletion test gets at. """ self.initdb(default_ms) - if default_ms == 'draft' and mongo_uses_error_check(self.store): + if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store): max_find += 1 # create and delete a private vertical with private children private_vert = self.store.create_child( @@ -927,7 +927,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Split: # find: active_version & structure (cached) # send: update structure and active_versions - @ddt.data(('draft', 4, 1), ('split', 2, 2)) + @ddt.data((ModuleStoreEnum.Type.mongo, 4, 1), (ModuleStoreEnum.Type.split, 2, 2)) @ddt.unpack def test_delete_draft_vertical(self, default_ms, max_find, max_send): """ @@ -957,7 +957,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): private_leaf.display_name = 'change me' private_leaf = self.store.update_item(private_leaf, self.user_id) # test succeeds if delete succeeds w/o error - if default_ms == 'draft' and mongo_uses_error_check(self.store): + if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store): max_find += 1 with check_mongo_calls(max_find, max_send): self.store.delete_item(private_leaf.location, self.user_id) @@ -970,7 +970,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # 1) wildcard split search, # 2-4) active_versions, structure, definition (s/b lazy; so, unnecessary) # 5) wildcard draft mongo which has none - @ddt.data(('draft', 3, 0), ('split', 5, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 3, 0), (ModuleStoreEnum.Type.split, 5, 0)) @ddt.unpack def test_get_courses(self, default_ms, max_find, max_send): self.initdb(default_ms) @@ -989,7 +989,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): published_courses = self.store.get_courses(remove_branch=True) self.assertEquals([c.id for c in draft_courses], [c.id for c in published_courses]) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_create_child_detached_tabs(self, default_ms): """ test 'create_child' method with a detached category ('static_tab') @@ -1014,7 +1014,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): """ Test that the xml modulestore only loaded the courses from the maps. """ - self.initdb('draft') + self.initdb(ModuleStoreEnum.Type.mongo) xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) # pylint: disable=protected-access courses = xml_store.get_courses() self.assertEqual(len(courses), 2) @@ -1028,7 +1028,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): """ Test that the xml modulestore doesn't allow write ops. """ - self.initdb('draft') + self.initdb(ModuleStoreEnum.Type.mongo) xml_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.xml) # pylint: disable=protected-access # the important thing is not which exception it raises but that it raises an exception with self.assertRaises(AttributeError): @@ -1036,7 +1036,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # draft is 2: find out which ms owns course, get item # split: active_versions, structure, definition (to load course wiki string) - @ddt.data(('draft', 2, 0), ('split', 3, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 3, 0)) @ddt.unpack def test_get_course(self, default_ms, max_find, max_send): """ @@ -1051,7 +1051,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): course = self.store.get_item(self.course_locations[self.XML_COURSEID1]) self.assertEqual(course.id, self.course_locations[self.XML_COURSEID1].course_key) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_library(self, default_ms): """ Test that create_library and get_library work regardless of the default modulestore. @@ -1076,7 +1076,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # still only 2) # Draft: get_parent # Split: active_versions, structure - @ddt.data(('draft', 1, 0), ('split', 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) @ddt.unpack def test_get_parent_locations(self, default_ms, max_find, max_send): """ @@ -1102,7 +1102,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.store.get_parent_location(child_location, revision=revision) ) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_parent_locations_moved_child(self, default_ms): self.initdb(default_ms) self._create_block_hierarchy() @@ -1153,7 +1153,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): (child_to_move_location, new_parent_published_location, ModuleStoreEnum.RevisionOption.published_only), ]) - @ddt.data('draft') + @ddt.data(ModuleStoreEnum.Type.mongo) def test_get_parent_locations_deleted_child(self, default_ms): self.initdb(default_ms) self._create_block_hierarchy() @@ -1184,7 +1184,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): (child_to_delete_location, None, ModuleStoreEnum.RevisionOption.published_only), ]) - @ddt.data('draft') + @ddt.data(ModuleStoreEnum.Type.mongo) def test_get_parent_location_draft(self, default_ms): """ Test that "get_parent_location" method returns first published parent @@ -1227,7 +1227,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # 8-9. get vertical, compute inheritance # 10-11. get other vertical_x1b (why?) and compute inheritance # Split: active_versions & structure - @ddt.data(('draft', [12, 3], 0), ('split', [2, 2], 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [2, 2], 0)) @ddt.unpack def test_path_to_location(self, default_ms, num_finds, num_sends): """ @@ -1278,7 +1278,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with the toy and simple courses loaded. """ # only needs course_locations set - self.initdb('draft') + self.initdb(ModuleStoreEnum.Type.mongo) course_key = self.course_locations[self.XML_COURSEID1].course_key video_key = course_key.make_usage_key('video', 'Welcome') chapter_key = course_key.make_usage_key('chapter', 'Overview') @@ -1312,7 +1312,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertEqual(5, navigation_index("5_2")) self.assertEqual(7, navigation_index("7_3_5_6_")) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_revert_to_published_root_draft(self, default_ms): """ Test calling revert_to_published on draft vertical. @@ -1344,7 +1344,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertBlocksEqualByFields(reverted_parent, published_parent) self.assertFalse(self._has_changes(self.vertical_x1a)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_revert_to_published_root_published(self, default_ms): """ Test calling revert_to_published on a published vertical with a draft child. @@ -1364,7 +1364,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): reverted_problem = self.store.get_item(self.problem_x1a_1) self.assertEqual(orig_display_name, reverted_problem.display_name) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_revert_to_published_no_draft(self, default_ms): """ Test calling revert_to_published on vertical with no draft content does nothing. @@ -1379,7 +1379,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertBlocksEqualByFields(orig_vertical, reverted_vertical) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_revert_to_published_no_published(self, default_ms): """ Test calling revert_to_published on vertical with no published version errors. @@ -1389,7 +1389,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.assertRaises(InvalidVersionError): self.store.revert_to_published(self.vertical_x1a, self.user_id) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_revert_to_published_direct_only(self, default_ms): """ Test calling revert_to_published on a direct-only item is a no-op. @@ -1404,7 +1404,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Draft: get all items which can be or should have parents # Split: active_versions, structure - @ddt.data(('draft', 1, 0), ('split', 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) @ddt.unpack def test_get_orphans(self, default_ms, max_find, max_send): """ @@ -1442,7 +1442,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) self.assertItemsEqual(found_orphans, orphan_locations) - @ddt.data('draft') + @ddt.data(ModuleStoreEnum.Type.mongo) def test_get_non_orphan_parents(self, default_ms): """ Test finding non orphan parents from many possible parents. @@ -1504,7 +1504,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.assertRaises(ReferentialIntegrityError): self.store.get_parent_location(self.problem_x1a_1) - @ddt.data('draft') + @ddt.data(ModuleStoreEnum.Type.mongo) def test_create_item_from_parent_location(self, default_ms): """ Test a code path missed by the above: passing an old-style location as parent but no @@ -1520,7 +1520,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) self.assertEqual(len(orphans), 0, "unexpected orphans: {}".format(orphans)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_create_item_populates_edited_info(self, default_ms): self.initdb(default_ms) block = self.store.create_item( @@ -1531,7 +1531,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertEqual(self.user_id, block.edited_by) self.assertGreater(datetime.datetime.now(UTC), block.edited_on) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_create_item_populates_subtree_edited_info(self, default_ms): self.initdb(default_ms) block = self.store.create_item( @@ -1544,7 +1544,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Draft: wildcard search of draft and split # Split: wildcard search of draft and split - @ddt.data(('draft', 2, 0), ('split', 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 2, 0), (ModuleStoreEnum.Type.split, 2, 0)) @ddt.unpack def test_get_courses_for_wiki(self, default_ms, max_find, max_send): """ @@ -1582,14 +1582,14 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Sends: # - insert structure # - write index entry - @ddt.data(('draft', 2, 6), ('split', 3, 2)) + @ddt.data((ModuleStoreEnum.Type.mongo, 2, 6), (ModuleStoreEnum.Type.split, 3, 2)) @ddt.unpack def test_unpublish(self, default_ms, max_find, max_send): """ Test calling unpublish """ self.initdb(default_ms) - if default_ms == 'draft' and mongo_uses_error_check(self.store): + if default_ms == ModuleStoreEnum.Type.mongo and mongo_uses_error_check(self.store): max_find += 1 self._create_block_hierarchy() @@ -1620,7 +1620,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Draft: specific query for revision None # Split: active_versions, structure - @ddt.data(('draft', 1, 0), ('split', 2, 0)) + @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 2, 0)) @ddt.unpack def test_has_published_version(self, default_ms, max_find, max_send): """ @@ -1661,7 +1661,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertTrue(self.store.has_changes(item)) self.assertTrue(self.store.has_published_version(item)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_update_edit_info_ancestors(self, default_ms): """ Tests that edited_on, edited_by, subtree_edited_on, and subtree_edited_by are set correctly during update @@ -1737,7 +1737,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # Verify that others have unchanged edit info check_node(sibling.location, None, after_create, self.user_id, None, after_create, self.user_id) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_update_edit_info(self, default_ms): """ Tests that edited_on and edited_by are set correctly during an update @@ -1767,7 +1767,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertLess(old_edited_on, updated_component.edited_on) self.assertEqual(updated_component.edited_by, edit_user) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_update_published_info(self, default_ms): """ Tests that published_on and published_by are set correctly @@ -1801,7 +1801,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.assertLessEqual(old_time, updated_component.published_on) self.assertEqual(updated_component.published_by, publish_user) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_auto_publish(self, default_ms): """ Test that the correct things have been published automatically @@ -1871,7 +1871,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): chapter = self.store.get_item(chapter.location.for_branch(None)) self.assertTrue(self.store.has_published_version(chapter)) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_get_courses_for_wiki_shared(self, default_ms): """ Test two courses sharing the same wiki @@ -1926,7 +1926,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): wiki_courses ) - @ddt.data('draft', 'split') + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_branch_setting(self, default_ms): """ Test the branch_setting context manager @@ -2127,56 +2127,57 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): def test_bulk_operations_signal_firing(self, default): """ Signals should be fired right before bulk_operations() exits. """ with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) with self.store.default_store(default): - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) - receiver.reset_mock() + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) + signal_handler.reset_mock() - course_key = course.id + course_key = course.id - def _clear_bulk_ops_record(course_key): # pylint: disable=unused-argument - """ - Check if the signal has been fired. - The course_published signal fires before the _clear_bulk_ops_record. - """ - self.assertEqual(receiver.call_count, 1) + def _clear_bulk_ops_record(course_key): # pylint: disable=unused-argument + """ + Check if the signal has been fired. + The course_published signal fires before the _clear_bulk_ops_record. + """ + signal_handler.send.assert_called_with('course_published', course_key=course.id) - with patch.object( - self.store.thread_cache.default_store, '_clear_bulk_ops_record', wraps=_clear_bulk_ops_record - ) as mock_clear_bulk_ops_record: + with patch.object( + self.store.thread_cache.default_store, '_clear_bulk_ops_record', wraps=_clear_bulk_ops_record + ) as mock_clear_bulk_ops_record: - with self.store.bulk_operations(course_key): - categories = DIRECT_ONLY_CATEGORIES - for block_type in categories: - self.store.create_item(self.user_id, course_key, block_type) - self.assertEqual(receiver.call_count, 0) + with self.store.bulk_operations(course_key): + categories = DIRECT_ONLY_CATEGORIES + for block_type in categories: + self.store.create_item(self.user_id, course_key, block_type) + signal_handler.send.assert_not_called() - self.assertEqual(mock_clear_bulk_ops_record.call_count, 1) + self.assertEqual(mock_clear_bulk_ops_record.call_count, 1) - self.assertEqual(receiver.call_count, 1) + signal_handler.send.assert_called_with('course_published', course_key=course.id) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_publish_signal_direct_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2184,38 +2185,40 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - course_key = course.id + course_key = course.id - # Test non-draftable block types. The block should be published with every change. - categories = DIRECT_ONLY_CATEGORIES - for block_type in categories: - log.debug('Testing with block type %s', block_type) - receiver.reset_mock() - block = self.store.create_item(self.user_id, course_key, block_type) - self.assertEqual(receiver.call_count, 1) + # Test non-draftable block types. The block should be published with every change. + categories = DIRECT_ONLY_CATEGORIES + for block_type in categories: + log.debug('Testing with block type %s', block_type) + signal_handler.reset_mock() + block = self.store.create_item(self.user_id, course_key, block_type) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - block.display_name = block_type - self.store.update_item(block, self.user_id) - self.assertEqual(receiver.call_count, 2) + signal_handler.reset_mock() + block.display_name = block_type + self.store.update_item(block, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - self.store.publish(block.location, self.user_id) - self.assertEqual(receiver.call_count, 3) + signal_handler.reset_mock() + self.store.publish(block.location, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_publish_signal_rerun_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2223,30 +2226,30 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - course_key = course.id + course_key = course.id - # Test course re-runs - receiver.reset_mock() - dest_course_id = self.store.make_course_key("org.other", "course.other", "run.other") - self.store.clone_course(course_key, dest_course_id, self.user_id) - self.assertEqual(receiver.call_count, 1) + # Test course re-runs + signal_handler.reset_mock() + dest_course_id = self.store.make_course_key("org.other", "course.other", "run.other") + self.store.clone_course(course_key, dest_course_id, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=dest_course_id) @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_publish_signal_import_firing(self, default, _from_json): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2254,28 +2257,32 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Test course imports - # Note: The signal is fired once when the course is created and - # a second time after the actual data import. - receiver.reset_mock() - import_course_from_xml( - self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, - static_content_store=contentstore, - create_if_not_present=True, - ) - self.assertEqual(receiver.call_count, 2) + # Test course imports + # Note: The signal is fired once when the course is created and + # a second time after the actual data import. + import_course_from_xml( + self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, + static_content_store=contentstore, + create_if_not_present=True, + ) + signal_handler.send.assert_has_calls([ + call('pre_publish', course_key=self.store.make_course_key('edX', 'toy', '2012_Fall')), + call('course_published', course_key=self.store.make_course_key('edX', 'toy', '2012_Fall')), + call('pre_publish', course_key=self.store.make_course_key('edX', 'toy', '2012_Fall')), + call('course_published', course_key=self.store.make_course_key('edX', 'toy', '2012_Fall')), + ]) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_publish_signal_publish_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2283,51 +2290,55 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - # Test a draftable block type, which needs to be explicitly published, and nest it within the - # normal structure - this is important because some implementors change the parent when adding a - # non-published child; if parent is in DIRECT_ONLY_CATEGORIES then this should not fire the event - receiver.reset_mock() - section = self.store.create_item(self.user_id, course.id, 'chapter') - self.assertEqual(receiver.call_count, 1) + # Test a draftable block type, which needs to be explicitly published, and nest it within the + # normal structure - this is important because some implementors change the parent when adding a + # non-published child; if parent is in DIRECT_ONLY_CATEGORIES then this should not fire the event + signal_handler.reset_mock() + section = self.store.create_item(self.user_id, course.id, 'chapter') + signal_handler.send.assert_called_with('course_published', course_key=course.id) - subsection = self.store.create_child(self.user_id, section.location, 'sequential') - self.assertEqual(receiver.call_count, 2) + signal_handler.reset_mock() + subsection = self.store.create_child(self.user_id, section.location, 'sequential') + signal_handler.send.assert_called_with('course_published', course_key=course.id) - # 'units' and 'blocks' are draftable types - receiver.reset_mock() - unit = self.store.create_child(self.user_id, subsection.location, 'vertical') - self.assertEqual(receiver.call_count, 0) + # 'units' and 'blocks' are draftable types + signal_handler.reset_mock() + unit = self.store.create_child(self.user_id, subsection.location, 'vertical') + signal_handler.send.assert_not_called() - block = self.store.create_child(self.user_id, unit.location, 'problem') - self.assertEqual(receiver.call_count, 0) + block = self.store.create_child(self.user_id, unit.location, 'problem') + signal_handler.send.assert_not_called() - self.store.update_item(block, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.update_item(block, self.user_id) + signal_handler.send.assert_not_called() - self.store.publish(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 1) + signal_handler.reset_mock() + self.store.publish(unit.location, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - self.store.unpublish(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 2) + signal_handler.reset_mock() + self.store.unpublish(unit.location, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - self.store.delete_item(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 3) + signal_handler.reset_mock() + self.store.delete_item(unit.location, self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_bulk_course_publish_signal_direct_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2335,41 +2346,41 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - course_key = course.id + course_key = course.id - # Test non-draftable block types. No signals should be received until - receiver.reset_mock() - with self.store.bulk_operations(course_key): - categories = DIRECT_ONLY_CATEGORIES - for block_type in categories: - log.debug('Testing with block type %s', block_type) - block = self.store.create_item(self.user_id, course_key, block_type) - self.assertEqual(receiver.call_count, 0) + # Test non-draftable block types. No signals should be received until + signal_handler.reset_mock() + with self.store.bulk_operations(course_key): + categories = DIRECT_ONLY_CATEGORIES + for block_type in categories: + log.debug('Testing with block type %s', block_type) + block = self.store.create_item(self.user_id, course_key, block_type) + signal_handler.send.assert_not_called() - block.display_name = block_type - self.store.update_item(block, self.user_id) - self.assertEqual(receiver.call_count, 0) + block.display_name = block_type + self.store.update_item(block, self.user_id) + signal_handler.send.assert_not_called() - self.store.publish(block.location, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.publish(block.location, self.user_id) + signal_handler.send.assert_not_called() - self.assertEqual(receiver.call_count, 1) + signal_handler.send.assert_called_with('course_published', course_key=course.id) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_bulk_course_publish_signal_publish_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2377,74 +2388,74 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_published) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Course creation and publication should fire the signal - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - self.assertEqual(receiver.call_count, 1) + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - course_key = course.id + course_key = course.id - # Test a draftable block type, which needs to be explicitly published, and nest it within the - # normal structure - this is important because some implementors change the parent when adding a - # non-published child; if parent is in DIRECT_ONLY_CATEGORIES then this should not fire the event - receiver.reset_mock() - with self.store.bulk_operations(course_key): - section = self.store.create_item(self.user_id, course_key, 'chapter') - self.assertEqual(receiver.call_count, 0) + # Test a draftable block type, which needs to be explicitly published, and nest it within the + # normal structure - this is important because some implementors change the parent when adding a + # non-published child; if parent is in DIRECT_ONLY_CATEGORIES then this should not fire the event + signal_handler.reset_mock() + with self.store.bulk_operations(course_key): + section = self.store.create_item(self.user_id, course_key, 'chapter') + signal_handler.send.assert_not_called() - subsection = self.store.create_child(self.user_id, section.location, 'sequential') - self.assertEqual(receiver.call_count, 0) + subsection = self.store.create_child(self.user_id, section.location, 'sequential') + signal_handler.send.assert_not_called() - # 'units' and 'blocks' are draftable types - unit = self.store.create_child(self.user_id, subsection.location, 'vertical') - self.assertEqual(receiver.call_count, 0) + # 'units' and 'blocks' are draftable types + unit = self.store.create_child(self.user_id, subsection.location, 'vertical') + signal_handler.send.assert_not_called() - block = self.store.create_child(self.user_id, unit.location, 'problem') - self.assertEqual(receiver.call_count, 0) + block = self.store.create_child(self.user_id, unit.location, 'problem') + signal_handler.send.assert_not_called() - self.store.update_item(block, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.update_item(block, self.user_id) + signal_handler.send.assert_not_called() - self.store.publish(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.publish(unit.location, self.user_id) + signal_handler.send.assert_not_called() - self.store.unpublish(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.unpublish(unit.location, self.user_id) + signal_handler.send.assert_not_called() - self.store.delete_item(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 0) + self.store.delete_item(unit.location, self.user_id) + signal_handler.send.assert_not_called() - self.assertEqual(receiver.call_count, 1) + signal_handler.send.assert_called_with('course_published', course_key=course.id) - # Test editing draftable block type without publish - receiver.reset_mock() - with self.store.bulk_operations(course_key): - unit = self.store.create_child(self.user_id, subsection.location, 'vertical') - self.assertEqual(receiver.call_count, 0) - block = self.store.create_child(self.user_id, unit.location, 'problem') - self.assertEqual(receiver.call_count, 0) - self.store.publish(unit.location, self.user_id) - self.assertEqual(receiver.call_count, 0) - self.assertEqual(receiver.call_count, 1) + # Test editing draftable block type without publish + signal_handler.reset_mock() + with self.store.bulk_operations(course_key): + unit = self.store.create_child(self.user_id, subsection.location, 'vertical') + signal_handler.send.assert_not_called() + block = self.store.create_child(self.user_id, unit.location, 'problem') + signal_handler.send.assert_not_called() + self.store.publish(unit.location, self.user_id) + signal_handler.send.assert_not_called() + signal_handler.send.assert_called_with('course_published', course_key=course.id) - receiver.reset_mock() - with self.store.bulk_operations(course_key): - self.assertEqual(receiver.call_count, 0) - unit.display_name = "Change this unit" - self.store.update_item(unit, self.user_id) - self.assertEqual(receiver.call_count, 0) - self.assertEqual(receiver.call_count, 0) + signal_handler.reset_mock() + with self.store.bulk_operations(course_key): + signal_handler.send.assert_not_called() + unit.display_name = "Change this unit" + self.store.update_item(unit, self.user_id) + signal_handler.send.assert_not_called() + signal_handler.send.assert_not_called() @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_deleted_signal(self, default): with MongoContentstoreBuilder().build() as contentstore: + signal_handler = Mock(name='signal_handler') self.store = MixedModuleStore( contentstore=contentstore, create_modulestore_instance=create_modulestore_instance, mappings={}, - signal_handler=SignalHandler(MixedModuleStore), + signal_handler=signal_handler, **self.OPTIONS ) self.addCleanup(self.store.close_all_connections) @@ -2452,18 +2463,130 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): with self.store.default_store(default): self.assertIsNotNone(self.store.thread_cache.default_store.signal_handler) - with mock_signal_receiver(SignalHandler.course_deleted) as receiver: - self.assertEqual(receiver.call_count, 0) + signal_handler.send.assert_not_called() - # Create a course - course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) - course_key = course.id + # Create a course + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + course_key = course.id - # Delete the course - course = self.store.delete_course(course_key, self.user_id) + # Delete the course + course = self.store.delete_course(course_key, self.user_id) - # Verify that the signal was emitted - self.assertEqual(receiver.call_count, 1) + # Verify that the signal was emitted + signal_handler.send.assert_called_with('course_deleted', course_key=course_key) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_delete_published_item_orphans(self, default_store): + """ + Tests delete published item dont create any oprhans in course + """ + self.initdb(default_store) + course_locator = self.course.id + + chapter = self.store.create_child( + self.user_id, self.course.location, 'chapter', block_id='section_one' + ) + + sequential = self.store.create_child( + self.user_id, chapter.location, 'sequential', block_id='subsection_one' + ) + + vertical = self.store.create_child( + self.user_id, sequential.location, 'vertical', block_id='moon_unit' + ) + + problem = self.store.create_child( + self.user_id, vertical.location, 'problem', block_id='problem' + ) + + self.store.publish(chapter.location, self.user_id) + # Verify that there are no changes + self.assertFalse(self._has_changes(chapter.location)) + self.assertFalse(self._has_changes(sequential.location)) + self.assertFalse(self._has_changes(vertical.location)) + self.assertFalse(self._has_changes(problem.location)) + + # No orphans in course + course_orphans = self.store.get_orphans(course_locator) + self.assertEqual(len(course_orphans), 0) + self.store.delete_item(vertical.location, self.user_id) + + # No orphans in course after delete, except + # in old mongo, which still creates orphans + course_orphans = self.store.get_orphans(course_locator) + if default_store == ModuleStoreEnum.Type.mongo: + self.assertEqual(len(course_orphans), 1) + else: + self.assertEqual(len(course_orphans), 0) + + course_locator_publish = course_locator.for_branch(ModuleStoreEnum.BranchName.published) + # No published oprhans after delete, except + # in old mongo, which still creates orphans + course_publish_orphans = self.store.get_orphans(course_locator_publish) + + if default_store == ModuleStoreEnum.Type.mongo: + self.assertEqual(len(course_publish_orphans), 1) + else: + self.assertEqual(len(course_publish_orphans), 0) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_delete_draft_item_orphans(self, default_store): + """ + Tests delete draft item create no orphans in course + """ + self.initdb(default_store) + course_locator = self.course.id + + chapter = self.store.create_child( + self.user_id, self.course.location, 'chapter', block_id='section_one' + ) + + sequential = self.store.create_child( + self.user_id, chapter.location, 'sequential', block_id='subsection_one' + ) + + vertical = self.store.create_child( + self.user_id, sequential.location, 'vertical', block_id='moon_unit' + ) + + problem = self.store.create_child( + self.user_id, vertical.location, 'problem', block_id='problem' + ) + + self.store.publish(chapter.location, self.user_id) + # Verify that there are no changes + self.assertFalse(self._has_changes(chapter.location)) + self.assertFalse(self._has_changes(sequential.location)) + self.assertFalse(self._has_changes(vertical.location)) + self.assertFalse(self._has_changes(problem.location)) + + # No orphans in course + course_orphans = self.store.get_orphans(course_locator) + self.assertEqual(len(course_orphans), 0) + + problem.display_name = 'changed' + problem = self.store.update_item(problem, self.user_id) + self.assertTrue(self._has_changes(vertical.location)) + self.assertTrue(self._has_changes(problem.location)) + + self.store.delete_item(vertical.location, self.user_id) + # No orphans in course after delete, except + # in old mongo, which still creates them + course_orphans = self.store.get_orphans(course_locator) + if default_store == ModuleStoreEnum.Type.mongo: + self.assertEqual(len(course_orphans), 1) + else: + self.assertEqual(len(course_orphans), 0) + + course_locator_publish = course_locator.for_branch(ModuleStoreEnum.BranchName.published) + # No published orphans after delete, except + # in old mongo, which still creates them + course_publish_orphans = self.store.get_orphans(course_locator_publish) + + if default_store == ModuleStoreEnum.Type.mongo: + self.assertEqual(len(course_publish_orphans), 1) + else: + self.assertEqual(len(course_publish_orphans), 0) @ddt.ddt diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py index 4a36d879e6..3cbdd2c39f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py @@ -11,7 +11,7 @@ import ddt from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml from xmodule.modulestore.tests.factories import check_mongo_calls -from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( +from xmodule.modulestore.tests.utils import ( MixedModulestoreBuilder, VersioningModulestoreBuilder, MongoModulestoreBuilder, TEST_DATA_DIR ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index efd2cba3c3..7b824e2c75 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -19,7 +19,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.xml_exporter import export_course_to_xml from xmodule.modulestore.tests.test_split_w_old_mongo import SplitWMongoCourseBootstrapper from xmodule.modulestore.tests.factories import check_mongo_calls, mongo_uses_error_check, CourseFactory, ItemFactory -from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( +from xmodule.modulestore.tests.utils import ( MongoContentstoreBuilder, MODULESTORE_SETUPS, DRAFT_MODULESTORE_SETUP, SPLIT_MODULESTORE_SETUP, MongoModulestoreBuilder, ) @@ -925,49 +925,16 @@ class ElementalUnpublishingTests(DraftPublishedOpBaseTestSetup): self.assertOLXIsDraftOnly(block_list_unpublished_children) self.assertOLXIsDraftOnly(block_list_untouched) - @ddt.data(DRAFT_MODULESTORE_SETUP, MongoModulestoreBuilder()) - def test_unpublish_old_mongo_draft_sequential(self, modulestore_builder): + @ddt.data(SPLIT_MODULESTORE_SETUP, DRAFT_MODULESTORE_SETUP, MongoModulestoreBuilder()) + def test_unpublish_draft_sequential(self, modulestore_builder): with self._setup_test(modulestore_builder): - # MODULESTORE_DIFFERENCE: - # In old Mongo, you cannot successfully unpublish an autopublished sequential. - # An exception is thrown. block_list_to_unpublish = ( ('sequential', 'sequential03'), ) with self.assertRaises(InvalidVersionError): self.unpublish(block_list_to_unpublish) - @ddt.data(SPLIT_MODULESTORE_SETUP) - def test_unpublish_split_draft_sequential(self, modulestore_builder): - with self._setup_test(modulestore_builder): - - # MODULESTORE_DIFFERENCE: - # In Split, the sequential is deleted. - # The sequential's children are orphaned - but they stay in - # the same draft state they were before. - block_list_to_unpublish = ( - ('sequential', 'sequential03'), - ) - block_list_unpublished_children = ( - ('vertical', 'vertical06'), - ('vertical', 'vertical07'), - ('html', 'html12'), - ('html', 'html13'), - ('html', 'html14'), - ('html', 'html15'), - ) - # The autopublished sequential is published - its children are draft. - self.assertOLXIsPublishedOnly(block_list_to_unpublish) - self.assertOLXIsDraftOnly(block_list_unpublished_children) - # Unpublish the sequential. - self.unpublish(block_list_to_unpublish) - # Since the sequential was autopublished, a draft version of the sequential never existed. - # So unpublishing the sequential doesn't make it a draft - it deletes it! - self.assertOLXIsDeleted(block_list_to_unpublish) - # Its children are orphaned and remain as drafts. - self.assertOLXIsDraftOnly(block_list_unpublished_children) - @ddt.ddt class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup): @@ -1122,20 +1089,7 @@ class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup): self.assertOLXIsPublishedOnly(block_list_to_delete) self.delete_item(block_list_to_delete, revision=revision) self._check_for_item_deletion(block_list_to_delete, result) - # MODULESTORE_DIFFERENCE - if self.is_split_modulestore: - # Split: - if revision == ModuleStoreEnum.RevisionOption.published_only: - # If deleting published_only items, the children that are drafts remain. - self.assertOLXIsDraftOnly(block_list_children) - else: - self.assertOLXIsDeleted(block_list_children) - elif self.is_old_mongo_modulestore: - # Old Mongo: - # If deleting draft_only or both items, the drafts will be deleted. - self.assertOLXIsDeleted(block_list_children) - else: - raise Exception("Must test either Old Mongo or Split modulestore!") + self.assertOLXIsDeleted(block_list_children) @ddt.data(*itertools.product( MODULESTORE_SETUPS, @@ -1176,20 +1130,7 @@ class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup): self.delete_item(block_list_to_delete, revision=revision) self._check_for_item_deletion(block_list_to_delete, result) self.assertOLXIsDeleted(autopublished_children) - # MODULESTORE_DIFFERENCE - if self.is_split_modulestore: - # Split: - if revision == ModuleStoreEnum.RevisionOption.published_only: - # If deleting published_only items, the children that are drafts remain. - self.assertOLXIsDraftOnly(block_list_draft_children) - else: - self.assertOLXIsDeleted(block_list_draft_children) - elif self.is_old_mongo_modulestore: - # Old Mongo: - # If deleting draft_only or both items, the drafts will be deleted. - self.assertOLXIsDeleted(block_list_draft_children) - else: - raise Exception("Must test either Old Mongo or Split modulestore!") + self.assertOLXIsDeleted(block_list_draft_children) @ddt.ddt diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py new file mode 100644 index 0000000000..5a3fd41689 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py @@ -0,0 +1,340 @@ +""" +Tests of modulestore semantics: How do the interfaces methods of ModuleStore relate to each other? +""" + +import ddt +import itertools +from collections import namedtuple + +from xmodule.modulestore.tests.utils import ( + PureModulestoreTestCase, MongoModulestoreBuilder, + SPLIT_MODULESTORE_SETUP +) +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.draft_and_published import DIRECT_ONLY_CATEGORIES +from xblock.core import XBlock + +DETACHED_BLOCK_TYPES = dict(XBlock.load_tagged_classes('detached')) + +# These tests won't work with courses, since they're creating blocks inside courses +TESTABLE_BLOCK_TYPES = set(DIRECT_ONLY_CATEGORIES) +TESTABLE_BLOCK_TYPES.discard('course') + +TestField = namedtuple('TestField', ['field_name', 'initial', 'updated']) + + +@ddt.ddt +class DirectOnlyCategorySemantics(PureModulestoreTestCase): + """ + Verify the behavior of Direct Only items + blocks intended to store snippets of course content. + """ + + __test__ = False + + DATA_FIELDS = { + 'about': TestField('data', '
test data
', '
different test data
'), + 'chapter': TestField('is_entrance_exam', True, False), + 'sequential': TestField('is_entrance_exam', True, False), + 'static_tab': TestField('data', '
test data
', '
different test data
'), + 'course_info': TestField('data', '
test data
', '
different test data
'), + } + + def setUp(self): + super(DirectOnlyCategorySemantics, self).setUp() + self.course = CourseFactory.create( + org='test_org', + number='999', + run='test_run', + display_name='My Test Course', + modulestore=self.store + ) + + def assertBlockDoesntExist(self, block_usage_key, draft=None): + """ + Verify that loading ``block_usage_key`` raises an ItemNotFoundError. + + Arguments: + block_usage_key: The xblock to check. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + if draft is None or draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + with self.assertRaises(ItemNotFoundError): + self.store.get_item(block_usage_key) + + if draft is None or not draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + with self.assertRaises(ItemNotFoundError): + self.store.get_item(block_usage_key) + + def assertBlockHasContent(self, block_usage_key, field_name, content, draft=None): + """ + Assert that the block ``block_usage_key`` has the value ``content`` for ``field_name`` + when it is loaded. + + Arguments: + block_usage_key: The xblock to check. + field_name (string): The name of the field to check. + content: The value to assert is in the field. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + if draft is None or not draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + target_block = self.store.get_item( + block_usage_key, + ) + self.assertEquals(content, target_block.fields[field_name].read_from(target_block)) + + if draft is None or draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + target_block = self.store.get_item( + block_usage_key, + ) + self.assertEquals(content, target_block.fields[field_name].read_from(target_block)) + + def assertParentOf(self, parent_usage_key, child_usage_key, draft=None): + """ + Assert that the block ``parent_usage_key`` has ``child_usage_key`` listed + as one of its ``.children``. + + Arguments: + parent_usage_key: The xblock to check as a parent. + child_usage_key: The xblock to check as a child. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + if draft is None or not draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + parent_block = self.store.get_item( + parent_usage_key, + ) + self.assertIn(child_usage_key, parent_block.children) + + if draft is None or draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + parent_block = self.store.get_item( + parent_usage_key, + ) + self.assertIn(child_usage_key, parent_block.children) + + def assertNotParentOf(self, parent_usage_key, child_usage_key, draft=None): + """ + Assert that the block ``parent_usage_key`` does not have ``child_usage_key`` listed + as one of its ``.children``. + + Arguments: + parent_usage_key: The xblock to check as a parent. + child_usage_key: The xblock to check as a child. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + if draft is None or not draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + parent_block = self.store.get_item( + parent_usage_key, + ) + self.assertNotIn(child_usage_key, parent_block.children) + + if draft is None or draft: + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + parent_block = self.store.get_item( + parent_usage_key, + ) + self.assertNotIn(child_usage_key, parent_block.children) + + def assertCoursePointsToBlock(self, block_usage_key, draft=None): + """ + Assert that the context course for the test has ``block_usage_key`` listed + as one of its ``.children``. + + Arguments: + block_usage_key: The xblock to check. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + self.assertParentOf(self.course.scope_ids.usage_id, block_usage_key, draft=draft) + + def assertCourseDoesntPointToBlock(self, block_usage_key, draft=None): + """ + Assert that the context course for the test does not have ``block_usage_key`` listed + as one of its ``.children``. + + Arguments: + block_usage_key: The xblock to check. + draft (optional): If omitted, verify both published and draft branches. + If True, verify only the draft branch. If False, verify only the + published branch. + """ + self.assertNotParentOf(self.course.scope_ids.usage_id, block_usage_key, draft=draft) + + def is_detached(self, block_type): + """ + Return True if ``block_type`` is a detached block. + """ + return block_type in DETACHED_BLOCK_TYPES + + @ddt.data(*TESTABLE_BLOCK_TYPES) + def test_create(self, block_type): + self._do_create(block_type) + + # This function is split out from the test_create method so that it can be called + # by other tests + def _do_create(self, block_type): + """ + Create a block of block_type (which should be a DIRECT_ONLY_CATEGORY), + and then verify that it was created successfully, and is visible in + both published and draft branches. + """ + block_usage_key = self.course.id.make_usage_key(block_type, 'test_block') + + self.assertBlockDoesntExist(block_usage_key) + self.assertCourseDoesntPointToBlock(block_usage_key) + + test_data = self.DATA_FIELDS[block_type] + + initial_field_value = test_data.initial + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + if self.is_detached(block_type): + block = self.store.create_xblock( + self.course.runtime, + self.course.id, + block_usage_key.block_type, + block_id=block_usage_key.block_id + ) + block.fields[test_data.field_name].write_to(block, initial_field_value) + self.store.update_item(block, ModuleStoreEnum.UserID.test, allow_not_found=True) + else: + block = self.store.create_child( + user_id=ModuleStoreEnum.UserID.test, + parent_usage_key=self.course.scope_ids.usage_id, + block_type=block_type, + block_id=block_usage_key.block_id, + fields={test_data.field_name: initial_field_value}, + ) + + if self.is_detached(block_type): + self.assertCourseDoesntPointToBlock(block_usage_key) + else: + self.assertCoursePointsToBlock(block_usage_key) + self.assertBlockHasContent(block_usage_key, test_data.field_name, initial_field_value) + + return block_usage_key + + @ddt.data(*TESTABLE_BLOCK_TYPES) + def test_update(self, block_type): + block_usage_key = self._do_create(block_type) + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + block = self.store.get_item(block_usage_key) + + test_data = self.DATA_FIELDS[block_type] + + updated_field_value = test_data.updated + self.assertNotEquals(updated_field_value, block.fields[test_data.field_name].read_from(block)) + + block.fields[test_data.field_name].write_to(block, updated_field_value) + + self.store.update_item(block, ModuleStoreEnum.UserID.test, allow_not_found=True) + + self.assertBlockHasContent(block_usage_key, test_data.field_name, updated_field_value) + + @ddt.data(*TESTABLE_BLOCK_TYPES) + def test_delete(self, block_type): + block_usage_key = self._do_create(block_type) + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.store.delete_item(block_usage_key, ModuleStoreEnum.UserID.test) + + self.assertCourseDoesntPointToBlock(block_usage_key) + self.assertBlockDoesntExist(block_usage_key) + + @ddt.data(*itertools.product(['chapter', 'sequential'], [True, False])) + @ddt.unpack + def test_delete_child(self, block_type, child_published): + block_usage_key = self.course.id.make_usage_key(block_type, 'test_block') + child_usage_key = self.course.id.make_usage_key('html', 'test_child') + + self.assertCourseDoesntPointToBlock(block_usage_key) + self.assertBlockDoesntExist(block_usage_key) + self.assertBlockDoesntExist(child_usage_key) + + test_data = self.DATA_FIELDS[block_type] + child_data = '
child value
' + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.store.create_child( + user_id=ModuleStoreEnum.UserID.test, + parent_usage_key=self.course.scope_ids.usage_id, + block_type=block_type, + block_id=block_usage_key.block_id, + fields={test_data.field_name: test_data.initial}, + ) + + self.store.create_child( + user_id=ModuleStoreEnum.UserID.test, + parent_usage_key=block_usage_key, + block_type=child_usage_key.block_type, + block_id=child_usage_key.block_id, + fields={'data': child_data}, + ) + + if child_published: + self.store.publish(child_usage_key, ModuleStoreEnum.UserID.test) + + self.assertCoursePointsToBlock(block_usage_key) + + if child_published: + self.assertParentOf(block_usage_key, child_usage_key) + else: + self.assertParentOf(block_usage_key, child_usage_key, draft=True) + # N.B. whether the direct-only parent block points to the child in the publish branch + # is left as undefined behavior + + self.assertBlockHasContent(block_usage_key, test_data.field_name, test_data.initial) + + if child_published: + self.assertBlockHasContent(child_usage_key, 'data', child_data) + else: + self.assertBlockHasContent(child_usage_key, 'data', child_data, draft=True) + self.assertBlockDoesntExist(child_usage_key, draft=False) + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.store.delete_item(child_usage_key, ModuleStoreEnum.UserID.test) + + self.assertCoursePointsToBlock(block_usage_key) + self.assertNotParentOf(block_usage_key, child_usage_key) + + if child_published and self.store.get_modulestore_type(self.course.id) == ModuleStoreEnum.Type.mongo: + # N.B. This block is being left as an orphan in old-mongo. This test will + # fail when that is fixed. At that time, this condition should just be removed, + # as SplitMongo and OldMongo will have the same semantics. + self.assertBlockHasContent(child_usage_key, 'data', child_data) + else: + self.assertBlockDoesntExist(child_usage_key) + + +class TestSplitDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): + """ + Verify DIRECT_ONLY_CATEGORY semantics against the SplitMongoModulestore. + """ + MODULESTORE = SPLIT_MODULESTORE_SETUP + __test__ = True + + +class TestMongoDirectOnlyCategorySemantics(DirectOnlyCategorySemantics): + """ + Verify DIRECT_ONLY_CATEGORY semantics against the MongoModulestore + """ + MODULESTORE = MongoModulestoreBuilder() + __test__ = True diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index 1ae6109dfe..5ef3ddbcd2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -13,7 +13,7 @@ from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.mongo import DraftMongoModuleStore from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST -from xmodule.modulestore.tests.test_cross_modulestore_import_export import MemoryCache +from xmodule.modulestore.tests.utils import MemoryCache @attr('mongo') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index aa8832ba10..3cdd0c36e9 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -1,18 +1,30 @@ """ Helper classes and methods for running modulestore tests without Django. """ +import random + +from contextlib import contextmanager, nested from importlib import import_module from opaque_keys.edx.keys import UsageKey +from path import Path as path +from shutil import rmtree +from tempfile import mkdtemp from unittest import TestCase + from xblock.fields import XBlockMixin from xmodule.x_module import XModuleMixin +from xmodule.contentstore.mongo import MongoContentStore from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from xmodule.modulestore.edit_info import EditInfoMixin from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.mongo.base import ModuleStoreEnum +from xmodule.modulestore.mongo.draft import DraftModuleStore +from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore from xmodule.modulestore.tests.factories import ItemFactory from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST +from xmodule.modulestore.xml import XMLModuleStore from xmodule.tests import DATA_DIR @@ -174,3 +186,326 @@ class ProceduralCourseTestMixin(object): with self.store.bulk_operations(self.course.id, emit_signals=emit_signals): descend(self.course, ['chapter', 'sequential', 'vertical', 'problem']) + + +class MemoryCache(object): + """ + This fits the metadata_inheritance_cache_subsystem interface used by + the modulestore, and stores the data in a dictionary in memory. + """ + def __init__(self): + self._data = {} + + def get(self, key, default=None): + """ + Get a key from the cache. + + Args: + key: The key to update. + default: The value to return if the key hasn't been set previously. + """ + return self._data.get(key, default) + + def set(self, key, value): + """ + Set a key in the cache. + + Args: + key: The key to update. + value: The value change the key to. + """ + self._data[key] = value + + +class MongoContentstoreBuilder(object): + """ + A builder class for a MongoContentStore. + """ + @contextmanager + def build(self): + """ + A contextmanager that returns a MongoContentStore, and deletes its contents + when the context closes. + """ + contentstore = MongoContentStore( + db='contentstore{}'.format(random.randint(0, 10000)), + collection='content', + **COMMON_DOCSTORE_CONFIG + ) + contentstore.ensure_indexes() + + try: + yield contentstore + finally: + # Delete the created database + contentstore._drop_database() # pylint: disable=protected-access + + def __repr__(self): + return 'MongoContentstoreBuilder()' + + +class StoreBuilderBase(object): + """ + Base class for all modulestore builders. + """ + @contextmanager + def build(self, **kwargs): + """ + Build the modulstore, optionally building the contentstore as well. + """ + contentstore = kwargs.pop('contentstore', None) + if not contentstore: + with self.build_without_contentstore() as (contentstore, modulestore): + yield contentstore, modulestore + else: + with self.build_with_contentstore(contentstore) as modulestore: + yield modulestore + + @contextmanager + def build_without_contentstore(self): + """ + Build both the contentstore and the modulestore. + """ + with MongoContentstoreBuilder().build() as contentstore: + with self.build_with_contentstore(contentstore) as modulestore: + yield contentstore, modulestore + + +class MongoModulestoreBuilder(StoreBuilderBase): + """ + A builder class for a DraftModuleStore. + """ + @contextmanager + def build_with_contentstore(self, contentstore): + """ + A contextmanager that returns an isolated mongo modulestore, and then deletes + all of its data at the end of the context. + + Args: + contentstore: The contentstore that this modulestore should use to store + all of its assets. + """ + doc_store_config = dict( + db='modulestore{}'.format(random.randint(0, 10000)), + collection='xmodule', + asset_collection='asset_metadata', + **COMMON_DOCSTORE_CONFIG + ) + + # Set up a temp directory for storing filesystem content created during import + fs_root = mkdtemp() + + # pylint: disable=attribute-defined-outside-init + modulestore = DraftModuleStore( + contentstore, + doc_store_config, + fs_root, + render_template=repr, + branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, + metadata_inheritance_cache_subsystem=MemoryCache(), + xblock_mixins=XBLOCK_MIXINS, + ) + modulestore.ensure_indexes() + + try: + yield modulestore + finally: + # Delete the created database + modulestore._drop_database() # pylint: disable=protected-access + + # Delete the created directory on the filesystem + rmtree(fs_root, ignore_errors=True) + + def __repr__(self): + return 'MongoModulestoreBuilder()' + + +class VersioningModulestoreBuilder(StoreBuilderBase): + """ + A builder class for a VersioningModuleStore. + """ + @contextmanager + def build_with_contentstore(self, contentstore): + """ + A contextmanager that returns an isolated versioning modulestore, and then deletes + all of its data at the end of the context. + + Args: + contentstore: The contentstore that this modulestore should use to store + all of its assets. + """ + # pylint: disable=unreachable + doc_store_config = dict( + db='modulestore{}'.format(random.randint(0, 10000)), + collection='split_module', + **COMMON_DOCSTORE_CONFIG + ) + # Set up a temp directory for storing filesystem content created during import + fs_root = mkdtemp() + + modulestore = DraftVersioningModuleStore( + contentstore, + doc_store_config, + fs_root, + render_template=repr, + xblock_mixins=XBLOCK_MIXINS, + ) + modulestore.ensure_indexes() + + try: + yield modulestore + finally: + # Delete the created database + modulestore._drop_database() # pylint: disable=protected-access + + # Delete the created directory on the filesystem + rmtree(fs_root, ignore_errors=True) + + def __repr__(self): + return 'SplitModulestoreBuilder()' + + +class XmlModulestoreBuilder(StoreBuilderBase): + """ + A builder class for a XMLModuleStore. + """ + # pylint: disable=unused-argument + @contextmanager + def build_with_contentstore(self, contentstore=None, course_ids=None): + """ + A contextmanager that returns an isolated xml modulestore + + Args: + contentstore: The contentstore that this modulestore should use to store + all of its assets. + """ + modulestore = XMLModuleStore( + DATA_DIR, + course_ids=course_ids, + default_class='xmodule.hidden_module.HiddenDescriptor', + xblock_mixins=XBLOCK_MIXINS, + ) + + yield modulestore + + +class MixedModulestoreBuilder(StoreBuilderBase): + """ + A builder class for a MixedModuleStore. + """ + def __init__(self, store_builders, mappings=None): + """ + Args: + store_builders: A list of modulestore builder objects. These will be instantiated, in order, + as the backing stores for the MixedModuleStore. + mappings: Any course mappings to pass to the MixedModuleStore on instantiation. + """ + self.store_builders = store_builders + self.mappings = mappings or {} + self.mixed_modulestore = None + + @contextmanager + def build_with_contentstore(self, contentstore): + """ + A contextmanager that returns a mixed modulestore built on top of modulestores + generated by other builder classes. + + Args: + contentstore: The contentstore that this modulestore should use to store + all of its assets. + """ + names, generators = zip(*self.store_builders) + + with nested(*(gen.build_with_contentstore(contentstore) for gen in generators)) as modulestores: + # Make the modulestore creation function just return the already-created modulestores + store_iterator = iter(modulestores) + next_modulestore = lambda *args, **kwargs: store_iterator.next() + + # Generate a fake list of stores to give the already generated stores appropriate names + stores = [{'NAME': name, 'ENGINE': 'This space deliberately left blank'} for name in names] + + self.mixed_modulestore = MixedModuleStore( + contentstore, + self.mappings, + stores, + create_modulestore_instance=next_modulestore, + xblock_mixins=XBLOCK_MIXINS, + ) + + yield self.mixed_modulestore + + def __repr__(self): + return 'MixedModulestoreBuilder({!r}, {!r})'.format(self.store_builders, self.mappings) + + def asset_collection(self): + """ + Returns the collection storing the asset metadata. + """ + all_stores = self.mixed_modulestore.modulestores + if len(all_stores) > 1: + return None + + store = all_stores[0] + if hasattr(store, 'asset_collection'): + # Mongo modulestore beneath mixed. + # Returns the entire collection with *all* courses' asset metadata. + return store.asset_collection + else: + # Split modulestore beneath mixed. + # Split stores all asset metadata in the structure collection. + return store.db_connection.structures + + +COMMON_DOCSTORE_CONFIG = { + 'host': MONGO_HOST, + 'port': MONGO_PORT_NUM, +} +DATA_DIR = path(__file__).dirname().parent.parent / "tests" / "data" / "xml-course-root" +TEST_DATA_DIR = 'common/test/data/' + +XBLOCK_MIXINS = (InheritanceMixin, XModuleMixin) + + +MIXED_MODULESTORE_BOTH_SETUP = MixedModulestoreBuilder([ + ('draft', MongoModulestoreBuilder()), + ('split', VersioningModulestoreBuilder()) +]) +DRAFT_MODULESTORE_SETUP = MixedModulestoreBuilder([('draft', MongoModulestoreBuilder())]) +SPLIT_MODULESTORE_SETUP = MixedModulestoreBuilder([('split', VersioningModulestoreBuilder())]) +MIXED_MODULESTORE_SETUPS = ( + DRAFT_MODULESTORE_SETUP, + SPLIT_MODULESTORE_SETUP, +) +MIXED_MS_SETUPS_SHORT = ( + 'mixed_mongo', + 'mixed_split', +) +DIRECT_MODULESTORE_SETUPS = ( + MongoModulestoreBuilder(), + # VersioningModulestoreBuilder(), # FUTUREDO: LMS-11227 +) +DIRECT_MS_SETUPS_SHORT = ( + 'mongo', + #'split', +) +MODULESTORE_SETUPS = DIRECT_MODULESTORE_SETUPS + MIXED_MODULESTORE_SETUPS +MODULESTORE_SHORTNAMES = DIRECT_MS_SETUPS_SHORT + MIXED_MS_SETUPS_SHORT +SHORT_NAME_MAP = dict(zip(MODULESTORE_SETUPS, MODULESTORE_SHORTNAMES)) + +CONTENTSTORE_SETUPS = (MongoContentstoreBuilder(),) + + +class PureModulestoreTestCase(TestCase): + """ + A TestCase designed to make testing Modulestore implementations without using Django + easier. + """ + + MODULESTORE = None + + def setUp(self): + super(PureModulestoreTestCase, self).setUp() + + builder = self.MODULESTORE.build() + self.assets, self.store = builder.__enter__() + self.addCleanup(builder.__exit__, None, None, None) diff --git a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py index aaafca9454..6743dcbf4e 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py @@ -22,7 +22,7 @@ from xmodule.course_metadata_utils import ( may_certify_for_course, ) from xmodule.fields import Date -from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( +from xmodule.modulestore.tests.utils import ( MongoModulestoreBuilder, VersioningModulestoreBuilder, MixedModulestoreBuilder diff --git a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py index b46e0fabbc..161037a50f 100644 --- a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py +++ b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py @@ -45,7 +45,7 @@ class TestCCXModulestoreWrapper(SharedModuleStoreTestCase): ) for _ in xrange(2) for s in sequentials ] cls.blocks = [ - ItemFactory.create(parent=v) for _ in xrange(2) for v in verticals + ItemFactory.create(parent=v, category='html') for _ in xrange(2) for v in verticals ] def setUp(self):