From a55b406c53389669e6749f60892f3900fe4550ba Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Mon, 4 Aug 2014 14:51:29 -0400 Subject: [PATCH] LMS-11177 Add Split to configuration settings. --- .../lib/xmodule/xmodule/modulestore/mixed.py | 3 +- .../modulestore/modulestore_settings.py | 22 ++++++ .../tests/test_modulestore_settings.py | 79 +++++++++++++++++-- lms/envs/common.py | 10 +++ 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index eca62de852..23ec15276f 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -508,9 +508,10 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): if store.get_modulestore_type() == store_type: self.modulestores.insert(0, self.modulestores.pop(i)) found = True - yield + break if not found: raise Exception(u"Cannot find store of type {}".format(store_type)) + yield finally: self.modulestores = previous_store_list diff --git a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py index fc7c0b33e9..9ea62aa2c4 100644 --- a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py @@ -3,6 +3,7 @@ This file contains helper functions for configuring module_store_setting setting """ import warnings +import copy def convert_module_store_setting_if_needed(module_store_setting): @@ -66,6 +67,27 @@ def convert_module_store_setting_if_needed(module_store_setting): ) assert isinstance(module_store_setting['default']['OPTIONS']['stores'], list) + + # If Split is not defined but the DraftMongoModuleStore is configured, add Split as a copy of Draft + mixed_stores = module_store_setting['default']['OPTIONS']['stores'] + is_split_defined = any(('DraftVersioningModuleStore' in store['ENGINE']) for store in mixed_stores) + if not is_split_defined: + # find first setting of mongo store + mongo_store = next( + (store for store in mixed_stores if ( + 'DraftMongoModuleStore' in store['ENGINE'] or 'DraftModuleStore' in store['ENGINE'] + )), + None + ) + if mongo_store: + # deepcopy mongo -> split + split_store = copy.deepcopy(mongo_store) + # update the ENGINE and NAME fields + split_store['ENGINE'] = 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore' + split_store['NAME'] = 'split' + # add split to the end of the list + mixed_stores.append(split_store) + return module_store_setting diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py index a0b63d4691..70dcf2af4e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -77,15 +77,48 @@ class ModuleStoreSettingsMigration(TestCase): } } + ALREADY_UPDATED_MIXED_CONFIG = { + 'default': { + 'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore', + 'OPTIONS': { + 'mappings': {}, + 'reference_type': 'Location', + 'stores': [ + { + 'NAME': 'split', + 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', + 'DOC_STORE_CONFIG': {}, + 'OPTIONS': { + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + 'fs_root': "fs_root", + 'render_template': 'edxmako.shortcuts.render_to_string', + } + }, + { + 'NAME': 'draft', + 'ENGINE': 'xmodule.modulestore.mongo.draft.DraftModuleStore', + 'DOC_STORE_CONFIG': {}, + 'OPTIONS': { + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + 'fs_root': "fs_root", + 'render_template': 'edxmako.shortcuts.render_to_string', + } + }, + ] + } + } + } + + def _get_mixed_stores(self, mixed_setting): """ - Helper for accessing stores in a configuration setting for the Mixed modulestore + Helper for accessing stores in a configuration setting for the Mixed modulestore. """ return mixed_setting["default"]["OPTIONS"]["stores"] def assertStoreValuesEqual(self, store_setting1, store_setting2): """ - Tests whether the fields in the given store_settings are equal + Tests whether the fields in the given store_settings are equal. """ store_fields = ["OPTIONS", "DOC_STORE_CONFIG"] for field in store_fields: @@ -108,26 +141,56 @@ class ModuleStoreSettingsMigration(TestCase): return new_mixed_setting, new_stores[0] + def is_split_configured(self, mixed_setting): + """ + Tests whether the split module store is configured in the given setting. + """ + stores = self._get_mixed_stores(mixed_setting) + split_settings = [store for store in stores if 'DraftVersioningModuleStore' in store['ENGINE']] + if len(split_settings): + # there should only be one setting for split + self.assertEquals(len(split_settings), 1) + # verify name + self.assertEquals(split_settings[0]['NAME'], 'split') + # verify split config settings equal those of mongo + self.assertStoreValuesEqual( + split_settings[0], + next((store for store in stores if 'DraftModuleStore' in store['ENGINE']), None) + ) + return len(split_settings) > 0 + def test_convert_into_mixed(self): old_setting = self.OLD_CONFIG - _, new_default_store_setting = self.assertMigrated(old_setting) + new_mixed_setting, new_default_store_setting = self.assertMigrated(old_setting) self.assertStoreValuesEqual(new_default_store_setting, old_setting["default"]) self.assertEqual(new_default_store_setting["ENGINE"], old_setting["default"]["ENGINE"]) + self.assertFalse(self.is_split_configured(new_mixed_setting)) def test_convert_from_old_mongo_to_draft_store(self): old_setting = self.OLD_CONFIG_WITH_DIRECT_MONGO - _, new_default_store_setting = self.assertMigrated(old_setting) + new_mixed_setting, new_default_store_setting = self.assertMigrated(old_setting) self.assertStoreValuesEqual(new_default_store_setting, old_setting["default"]) self.assertEqual(new_default_store_setting["ENGINE"], "xmodule.modulestore.mongo.draft.DraftModuleStore") + self.assertTrue(self.is_split_configured(new_mixed_setting)) def test_convert_from_dict_to_list(self): old_mixed_setting = self.OLD_MIXED_CONFIG_WITH_DICT new_mixed_setting, new_default_store_setting = self.assertMigrated(old_mixed_setting) self.assertEqual(new_default_store_setting["ENGINE"], "the_default_store") + self.assertTrue(self.is_split_configured(new_mixed_setting)) + + # exclude split when comparing old and new, since split was added as part of the migration + new_stores = [store for store in self._get_mixed_stores(new_mixed_setting) if store['NAME'] != 'split'] + old_stores = self._get_mixed_stores(self.OLD_MIXED_CONFIG_WITH_DICT) # compare each store configured in mixed - old_stores = self._get_mixed_stores(self.OLD_MIXED_CONFIG_WITH_DICT) - new_stores = self._get_mixed_stores(new_mixed_setting) self.assertEqual(len(new_stores), len(old_stores)) - for new_store_setting in self._get_mixed_stores(new_mixed_setting): - self.assertStoreValuesEqual(new_store_setting, old_stores[new_store_setting['NAME']]) + for new_store in new_stores: + self.assertStoreValuesEqual(new_store, old_stores[new_store['NAME']]) + + def test_no_conversion(self): + # make sure there is no migration done on an already updated config + old_mixed_setting = self.ALREADY_UPDATED_MIXED_CONFIG + new_mixed_setting, new_default_store_setting = self.assertMigrated(old_mixed_setting) + self.assertTrue(self.is_split_configured(new_mixed_setting)) + self.assertEquals(old_mixed_setting, new_mixed_setting) diff --git a/lms/envs/common.py b/lms/envs/common.py index 7ae3f8346b..b2b9632968 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -524,6 +524,16 @@ MODULESTORE = { 'default_class': 'xmodule.hidden_module.HiddenDescriptor', } }, + { + 'NAME': 'split', + 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', + 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, + 'OPTIONS': { + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + 'fs_root': DATA_DIR, + 'render_template': 'edxmako.shortcuts.render_to_string', + } + }, ] } }