From 4d3627657360dc7bdfa703718de545c187e226f7 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Tue, 26 Aug 2014 10:52:54 -0400 Subject: [PATCH] LMS-11297 BokChoy and Acceptance tests configure default_store with update_module_store_settings. --- cms/envs/acceptance.py | 3 +- cms/envs/bok_choy.py | 1 + .../modulestore/modulestore_settings.py | 33 ++++++++++++++++--- .../tests/test_modulestore_settings.py | 33 ++++++++++++------- lms/envs/acceptance.py | 3 +- lms/envs/bok_choy.py | 1 + 6 files changed, 56 insertions(+), 18 deletions(-) diff --git a/cms/envs/acceptance.py b/cms/envs/acceptance.py index d2593d16a3..5e596783cd 100644 --- a/cms/envs/acceptance.py +++ b/cms/envs/acceptance.py @@ -50,7 +50,8 @@ update_module_store_settings( module_store_options={ 'default_class': 'xmodule.raw_module.RawDescriptor', 'fs_root': TEST_ROOT / "data", - } + }, + default_store=os.environ.get('DEFAULT_STORE', 'draft'), ) CONTENTSTORE = { diff --git a/cms/envs/bok_choy.py b/cms/envs/bok_choy.py index ff58e1d511..a7c54076e9 100644 --- a/cms/envs/bok_choy.py +++ b/cms/envs/bok_choy.py @@ -36,6 +36,7 @@ update_module_store_settings( xml_store_options={ 'data_dir': (TEST_ROOT / "data").abspath(), }, + default_store=os.environ.get('DEFAULT_STORE', 'draft'), ) # Enable django-pipeline and staticfiles diff --git a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py index 1726c78437..ee1a7fc106 100644 --- a/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/modulestore_settings.py @@ -34,6 +34,7 @@ def convert_module_store_setting_if_needed(module_store_setting): if module_store_setting is None: return None + # Convert to Mixed, if needed if module_store_setting['default']['ENGINE'] != 'xmodule.modulestore.mixed.MixedModuleStore': warnings.warn("Direct access to a modulestore is deprecated. Please use MixedModuleStore.", DeprecationWarning) @@ -54,7 +55,8 @@ def convert_module_store_setting_if_needed(module_store_setting): ) module_store_setting = new_module_store_setting - elif isinstance(module_store_setting['default']['OPTIONS']['stores'], dict): + # Convert from dict, if needed + elif isinstance(get_mixed_stores(module_store_setting), dict): warnings.warn( "Using a dict for the Stores option in the MixedModuleStore is deprecated. Please use a list instead.", DeprecationWarning @@ -62,13 +64,13 @@ def convert_module_store_setting_if_needed(module_store_setting): # convert old-style (unordered) dict to (an ordered) list module_store_setting['default']['OPTIONS']['stores'] = convert_old_stores_into_list( - module_store_setting['default']['OPTIONS']['stores'] + get_mixed_stores(module_store_setting) ) + assert isinstance(get_mixed_stores(module_store_setting), list) - assert isinstance(module_store_setting['default']['OPTIONS']['stores'], list) - + # Add Split, if needed # 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'] + mixed_stores = get_mixed_stores(module_store_setting) is_split_defined = any((store['ENGINE'].endswith('.DraftVersioningModuleStore')) for store in mixed_stores) if not is_split_defined: # find first setting of mongo store @@ -95,10 +97,14 @@ def update_module_store_settings( doc_store_settings=None, module_store_options=None, xml_store_options=None, + default_store=None, ): """ Updates the settings for each store defined in the given module_store_setting settings with the given doc store configuration and options, overwriting existing keys. + + If default_store is specified, the given default store is moved to the top of the + list of stores. """ for store in module_store_setting['default']['OPTIONS']['stores']: if store['NAME'] == 'xml': @@ -106,3 +112,20 @@ def update_module_store_settings( else: module_store_options and store['OPTIONS'].update(module_store_options) doc_store_settings and store['DOC_STORE_CONFIG'].update(doc_store_settings) + + if default_store: + mixed_stores = get_mixed_stores(module_store_setting) + for store in mixed_stores: + if store['NAME'] == default_store: + # move the found store to the top of the list + mixed_stores.remove(store) + mixed_stores.insert(0, store) + return + raise Exception("Could not find setting for requested default store: {}".format(default_store)) + + +def get_mixed_stores(mixed_setting): + """ + Helper for accessing stores in a configuration setting for the Mixed modulestore. + """ + return mixed_setting["default"]["OPTIONS"]["stores"] 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 166bf22327..f7a3335fc2 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore_settings.py @@ -2,10 +2,16 @@ Tests for testing the modulestore settings migration code. """ import copy +import ddt from unittest import TestCase -from xmodule.modulestore.modulestore_settings import convert_module_store_setting_if_needed +from xmodule.modulestore.modulestore_settings import ( + convert_module_store_setting_if_needed, + update_module_store_settings, + get_mixed_stores, +) +@ddt.ddt class ModuleStoreSettingsMigration(TestCase): """ Tests for the migration code for the module store settings @@ -108,12 +114,6 @@ class ModuleStoreSettingsMigration(TestCase): } - def _get_mixed_stores(self, mixed_setting): - """ - 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. @@ -134,7 +134,7 @@ class ModuleStoreSettingsMigration(TestCase): self.assertEqual(new_mixed_setting["default"]["ENGINE"], "xmodule.modulestore.mixed.MixedModuleStore") # check whether the stores are in an ordered list - new_stores = self._get_mixed_stores(new_mixed_setting) + new_stores = get_mixed_stores(new_mixed_setting) self.assertIsInstance(new_stores, list) return new_mixed_setting, new_stores[0] @@ -143,7 +143,7 @@ class ModuleStoreSettingsMigration(TestCase): """ Tests whether the split module store is configured in the given setting. """ - stores = self._get_mixed_stores(mixed_setting) + stores = get_mixed_stores(mixed_setting) split_settings = [store for store in stores if store['ENGINE'].endswith('.DraftVersioningModuleStore')] if len(split_settings): # there should only be one setting for split @@ -178,8 +178,8 @@ class ModuleStoreSettingsMigration(TestCase): 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) + new_stores = [store for store in get_mixed_stores(new_mixed_setting) if store['NAME'] != 'split'] + old_stores = get_mixed_stores(self.OLD_MIXED_CONFIG_WITH_DICT) # compare each store configured in mixed self.assertEqual(len(new_stores), len(old_stores)) @@ -192,3 +192,14 @@ class ModuleStoreSettingsMigration(TestCase): 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) + + @ddt.data('draft', 'split') + def test_update_settings(self, default_store): + mixed_setting = self.ALREADY_UPDATED_MIXED_CONFIG + update_module_store_settings(mixed_setting, default_store=default_store) + self.assertTrue(get_mixed_stores(mixed_setting)[0]['NAME'] == default_store) + + def test_update_settings_error(self): + mixed_setting = self.ALREADY_UPDATED_MIXED_CONFIG + with self.assertRaises(Exception): + update_module_store_settings(mixed_setting, default_store='non-existent store') diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index 8b8959c9a6..e4361ebacc 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -50,7 +50,8 @@ update_module_store_settings( }, module_store_options={ 'fs_root': TEST_ROOT / "data", - } + }, + default_store=os.environ.get('DEFAULT_STORE', 'draft'), ) CONTENTSTORE = { 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index 7c47b9fbea..fd797b9bc5 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -39,6 +39,7 @@ update_module_store_settings( xml_store_options={ 'data_dir': (TEST_ROOT / "data").abspath(), }, + default_store=os.environ.get('DEFAULT_STORE', 'draft'), ) # Configure the LMS to use our stub XQueue implementation