From 4e65a436c7c3659189b5e062f49afb09b148f38e Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Mon, 9 Dec 2019 00:21:15 -0400 Subject: [PATCH] Fix evaluation priority of the bootstrap settings Closes GitLab issue #702. Thanks to Kevin Pawsey (@kevinpawsey) for the report and the help debuging the issue. Signed-off-by: Roberto Rosario --- HISTORY.rst | 3 + mayan/apps/smart_settings/tests/literals.py | 3 + mayan/apps/smart_settings/tests/mixins.py | 43 ++++++++++++- mayan/apps/smart_settings/tests/test_utils.py | 61 +++++++++++++++++++ mayan/apps/smart_settings/utils.py | 33 +++++++--- 5 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 mayan/apps/smart_settings/tests/test_utils.py diff --git a/HISTORY.rst b/HISTORY.rst index 28be891016..ae0515b35f 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -15,6 +15,9 @@ mark widget. - Add icons to the smart settings links. - Fix docker-runtest-all target. +- Fix the evaluation priority of the bootstrap settings. Closes GitLab issue + #702. Thanks to Kevin Pawsey (@kevinpawsey) for the report and the help + debuging the issue. 3.3.3 (2019-12-05) ================== diff --git a/mayan/apps/smart_settings/tests/literals.py b/mayan/apps/smart_settings/tests/literals.py index 8f946eaa46..bdcc4711fc 100644 --- a/mayan/apps/smart_settings/tests/literals.py +++ b/mayan/apps/smart_settings/tests/literals.py @@ -3,6 +3,8 @@ from __future__ import unicode_literals ENVIRONMENT_TEST_NAME = 'PAGINATE_BY' ENVIRONMENT_TEST_VALUE = '999999' +TEST_BOOTSTAP_SETTING_NAME = 'TEST_BOOTSTRAP_SETTING' + TEST_NAMESPACE_LABEL = 'test namespace label' TEST_NAMESPACE_NAME = 'test_namespace_name' @@ -10,3 +12,4 @@ TEST_SETTING_GLOBAL_NAME = 'SMART_SETTINGS_TEST_SETTING' TEST_SETTING_INITIAL_VALUE = 'test_setting_initial_value' TEST_SETTING_DEFAULT_VALUE = 'test_setting_default_value' TEST_SETTING_VALUE = 'test_setting_value' +TEST_SETTING_VALUE_OVERRIDE = 'test value override' diff --git a/mayan/apps/smart_settings/tests/mixins.py b/mayan/apps/smart_settings/tests/mixins.py index e926b1b4dc..93d77987cd 100644 --- a/mayan/apps/smart_settings/tests/mixins.py +++ b/mayan/apps/smart_settings/tests/mixins.py @@ -1,13 +1,52 @@ from __future__ import absolute_import, unicode_literals +from django.utils.encoding import force_bytes + +from mayan.apps.storage.utils import NamedTemporaryFile + from ..classes import Namespace +from ..utils import BaseSetting, SettingNamespaceSingleton from .literals import ( - TEST_NAMESPACE_LABEL, TEST_NAMESPACE_NAME, TEST_SETTING_DEFAULT_VALUE, - TEST_SETTING_GLOBAL_NAME + TEST_BOOTSTAP_SETTING_NAME, TEST_NAMESPACE_LABEL, TEST_NAMESPACE_NAME, + TEST_SETTING_DEFAULT_VALUE, TEST_SETTING_GLOBAL_NAME ) +class BoostrapSettingTestMixin(object): + def _create_test_bootstrap_singleton(self): + self.test_globals = {} + self.test_globals['BASE_DIR'] = '' + self.setting_namespace = SettingNamespaceSingleton( + global_symbol_table=self.test_globals + ) + + def _create_test_config_file(self, value): + with NamedTemporaryFile() as file_object: + self._set_environment_variable( + name='MAYAN_CONFIGURATION_FILEPATH', + value=file_object.name + ) + + file_object.write( + force_bytes( + '{}: {}'.format( + TEST_BOOTSTAP_SETTING_NAME, value + ) + ) + ) + file_object.seek(0) + + self.setting_namespace.update_globals() + + def _register_test_boostrap_setting(self): + SettingNamespaceSingleton.register_setting( + name=TEST_BOOTSTAP_SETTING_NAME, klass=BaseSetting, kwargs={ + 'has_default': True, 'default_value': 'value default' + } + ) + + class SmartSettingsTestCaseMixin(object): def setUp(self): super(SmartSettingsTestCaseMixin, self).setUp() diff --git a/mayan/apps/smart_settings/tests/test_utils.py b/mayan/apps/smart_settings/tests/test_utils.py new file mode 100644 index 0000000000..5f1c025f9d --- /dev/null +++ b/mayan/apps/smart_settings/tests/test_utils.py @@ -0,0 +1,61 @@ +from __future__ import absolute_import, unicode_literals + +from mayan.apps.common.tests.base import BaseTestCase +from mayan.apps.common.tests.mixins import EnvironmentTestCaseMixin + +from .literals import ( + TEST_BOOTSTAP_SETTING_NAME, TEST_SETTING_VALUE, + TEST_SETTING_VALUE_OVERRIDE +) +from .mixins import BoostrapSettingTestMixin + + +class BoostrapSettingTestCase( + BoostrapSettingTestMixin, EnvironmentTestCaseMixin, BaseTestCase +): + def setUp(self): + super(BoostrapSettingTestCase, self).setUp() + self._register_test_boostrap_setting() + self._create_test_bootstrap_singleton() + + def test_bootstrap_environment_overrides_config(self): + self._set_environment_variable( + name='MAYAN_{}'.format(TEST_BOOTSTAP_SETTING_NAME), + value=TEST_SETTING_VALUE_OVERRIDE + ) + + self._create_test_config_file(value=TEST_SETTING_VALUE) + + self.assertEqual( + self.test_globals[TEST_BOOTSTAP_SETTING_NAME], + TEST_SETTING_VALUE_OVERRIDE + ) + + def test_bootstrap_config_overrides_settings(self): + self.test_globals[TEST_BOOTSTAP_SETTING_NAME] = TEST_SETTING_VALUE + + self._create_test_config_file(value=TEST_SETTING_VALUE_OVERRIDE) + + self.assertEqual( + self.test_globals[TEST_BOOTSTAP_SETTING_NAME], + TEST_SETTING_VALUE_OVERRIDE + ) + + def test_bootstrap_settings_overrides_default(self): + self.test_globals[ + TEST_BOOTSTAP_SETTING_NAME + ] = TEST_SETTING_VALUE_OVERRIDE + + self.setting_namespace.update_globals() + + self.assertEqual( + self.test_globals[TEST_BOOTSTAP_SETTING_NAME], + TEST_SETTING_VALUE_OVERRIDE + ) + + def test_bootstrap_default(self): + self.setting_namespace.update_globals() + + self.assertEqual( + self.test_globals[TEST_BOOTSTAP_SETTING_NAME], 'value default' + ) diff --git a/mayan/apps/smart_settings/utils.py b/mayan/apps/smart_settings/utils.py index f82cf49124..b120ca5567 100644 --- a/mayan/apps/smart_settings/utils.py +++ b/mayan/apps/smart_settings/utils.py @@ -141,12 +141,24 @@ class BaseSetting(object): By default will try to get the value from the namespace symbol table, then the configuration file, and finally from the environment. """ + # Resolution order + # 1 - Environment + # 2 - Config + # 3 - Global + # 4 - Default try: - return self.namespace.global_symbol_table.get( - self.name, self.namespace.get_config_file_setting(name=self.name) - ) - except SettingNamespaceSingleton.SettingNotFound: return self.load_environment_value() + except SettingNamespaceSingleton.SettingNotFound: + try: + return self.namespace.get_config_file_setting(name=self.name) + except SettingNamespaceSingleton.SettingNotFound: + try: + return self.namespace.global_symbol_table[self.name] + except KeyError: + if self.has_default: + return self.get_default_value() + else: + raise SettingNamespaceSingleton.SettingNotFound def load_environment_value(self): value = self._get_environment_value() @@ -161,10 +173,7 @@ class BaseSetting(object): ) ) else: - if self.has_default: - return self.get_default_value() - else: - raise SettingNamespaceSingleton.SettingNotFound + raise SettingNamespaceSingleton.SettingNotFound class FilesystemBootstrapSetting(BaseSetting): @@ -190,7 +199,13 @@ class FilesystemBootstrapSetting(BaseSetting): because not even the config file setup has completed. This setting only supports being set from the environment. """ - return self.load_environment_value() + try: + return self.load_environment_value() + except SettingNamespaceSingleton.SettingNotFound: + if self.has_default: + return self.get_default_value() + else: + raise SettingNamespaceSingleton.SettingNotFound class MediaBootstrapSetting(FilesystemBootstrapSetting):