From 17cf513b6b5fe5a74f4de5ea331a6edb1b0fab45 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 15 Apr 2024 20:42:09 +0530 Subject: [PATCH] feat: separate studio config and handler (#185) refactor: store config column names in constants --- eox_tenant/apps.py | 8 +-- eox_tenant/constants.py | 3 + eox_tenant/models.py | 10 +-- eox_tenant/receivers_helpers.py | 5 +- eox_tenant/signals.py | 47 +++++++++++--- eox_tenant/test/test_receivers_helpers.py | 35 +++++++++-- eox_tenant/test/test_signals.py | 77 +++++++++++++++++------ 7 files changed, 144 insertions(+), 41 deletions(-) diff --git a/eox_tenant/apps.py b/eox_tenant/apps.py index cc419fac..ebe65aa3 100644 --- a/eox_tenant/apps.py +++ b/eox_tenant/apps.py @@ -37,7 +37,7 @@ class EdunextOpenedxExtensionsTenantConfig(AppConfig): 'relative_path': 'signals', 'receivers': [ { - 'receiver_func_name': 'start_tenant', + 'receiver_func_name': 'start_lms_tenant', 'signal_path': 'django.core.signals.request_started', }, { @@ -53,7 +53,7 @@ class EdunextOpenedxExtensionsTenantConfig(AppConfig): 'signal_path': 'celery.signals.before_task_publish', }, { - 'receiver_func_name': 'start_async_tenant', + 'receiver_func_name': 'start_async_lms_tenant', 'signal_path': 'celery.signals.task_prerun', }, { @@ -74,7 +74,7 @@ class EdunextOpenedxExtensionsTenantConfig(AppConfig): 'relative_path': 'signals', 'receivers': [ { - 'receiver_func_name': 'start_tenant', + 'receiver_func_name': 'start_studio_tenant', 'signal_path': 'django.core.signals.request_started', }, { @@ -82,7 +82,7 @@ class EdunextOpenedxExtensionsTenantConfig(AppConfig): 'signal_path': 'celery.signals.before_task_publish', }, { - 'receiver_func_name': 'start_async_tenant', + 'receiver_func_name': 'start_async_studio_tenant', 'signal_path': 'celery.signals.task_prerun', }, ], diff --git a/eox_tenant/constants.py b/eox_tenant/constants.py index 94662b46..c9dc904b 100644 --- a/eox_tenant/constants.py +++ b/eox_tenant/constants.py @@ -2,3 +2,6 @@ from django.conf import settings LMS_ENVIRONMENT = getattr(settings, "SERVICE_VARIANT", None) == "lms" +CMS_ENVIRONMENT = getattr(settings, "SERVICE_VARIANT", None) == "cms" +LMS_CONFIG_COLUMN = "lms_configs" +CMS_CONFIG_COLUMN = "studio_configs" diff --git a/eox_tenant/models.py b/eox_tenant/models.py index 8c59883c..3bede64e 100644 --- a/eox_tenant/models.py +++ b/eox_tenant/models.py @@ -11,6 +11,8 @@ from django.utils.translation import gettext_lazy as _ from jsonfield.fields import JSONField +from eox_tenant.constants import CMS_CONFIG_COLUMN, LMS_CONFIG_COLUMN + class TenantOrganization(models.Model): """ @@ -150,8 +152,8 @@ def get_configurations(self, domain): configurations = { "id": row[0], "external_key": row[1], - "lms_configs": json.loads(row[2]), - "studio_configs": json.loads(row[3]), + LMS_CONFIG_COLUMN: json.loads(row[2]), + CMS_CONFIG_COLUMN: json.loads(row[3]), "theming_configs": json.loads(row[4]), "meta": json.loads(row[5]), } @@ -193,7 +195,7 @@ def get_organizations(self): return org_filter @classmethod - def get_configs_for_domain(cls, domain): + def get_configs_for_domain(cls, domain, config_key): """ Get edxapp configuration using a domain. There is a compat layer to support microsite until deprecation. @@ -204,7 +206,7 @@ def get_configs_for_domain(cls, domain): config = TenantConfig.objects.get_configurations(domain=domain) if config: - return config["lms_configs"], config["external_key"] + return config[config_key], config["external_key"] return {}, None diff --git a/eox_tenant/receivers_helpers.py b/eox_tenant/receivers_helpers.py index 015dd700..cc126ca0 100644 --- a/eox_tenant/receivers_helpers.py +++ b/eox_tenant/receivers_helpers.py @@ -6,12 +6,13 @@ from eox_tenant.models import Microsite, TenantConfig -def get_tenant_config_by_domain(domain): +def get_tenant_config_by_domain(domain, config_key): """ Reach for the configuration for a given domain. **Arguments** domain: String parameter. + config_key: Config column name. **Returns** configurations: dict @@ -20,7 +21,7 @@ def get_tenant_config_by_domain(domain): if not getattr(settings, 'USE_EOX_TENANT', False): return {}, None - configurations, external_key = TenantConfig.get_configs_for_domain(domain) + configurations, external_key = TenantConfig.get_configs_for_domain(domain, config_key) if configurations and external_key: return configurations, external_key diff --git a/eox_tenant/signals.py b/eox_tenant/signals.py index c53bb670..ffcf395a 100644 --- a/eox_tenant/signals.py +++ b/eox_tenant/signals.py @@ -29,6 +29,7 @@ from django.conf import settings as base_settings from eox_tenant.async_utils import AsyncTaskHandler +from eox_tenant.constants import CMS_CONFIG_COLUMN, LMS_CONFIG_COLUMN from eox_tenant.receivers_helpers import get_tenant_config_by_domain from eox_tenant.utils import synchronize_tenant_organizations @@ -38,11 +39,11 @@ EOX_MAX_CONFIG_OVERRIDE_SECONDS = getattr(base_settings, "EOX_MAX_CONFIG_OVERRIDE_SECONDS", 300) -def _update_settings(domain): +def _update_settings(domain, config_key): """ Perform the override procedure on the settings object """ - config, tenant_key = get_tenant_config_by_domain(domain) + config, tenant_key = get_tenant_config_by_domain(domain, config_key) if not config.get("EDNX_USE_SIGNAL"): LOG.info("Site %s, does not use eox_tenant signals", domain) @@ -157,9 +158,24 @@ def _ttl_reached(): return False -def start_tenant(sender, environ, **kwargs): # pylint: disable=unused-argument +def start_lms_tenant(sender, environ, **kwargs): # pylint: disable=unused-argument + """ + This function runs every time a request is started in LMS. + Read documentation of `_start_tenant` for more details. + """ + _start_tenant(environ, LMS_CONFIG_COLUMN) + + +def start_studio_tenant(sender, environ, **kwargs): # pylint: disable=unused-argument + """ + This function runs every time a request is started in studio. + Read documentation of `_start_tenant` for more details. + """ + _start_tenant(environ, CMS_CONFIG_COLUMN) + + +def _start_tenant(environ, config_key): """ - This function runs every time a request is started. It will analyze the current settings object, the domain from the incoming request and the configuration stored in the tenants for this domain. @@ -182,7 +198,7 @@ def start_tenant(sender, environ, **kwargs): # pylint: disable=unused-argument return # Do the update - _update_settings(domain) + _update_settings(domain, config_key) def finish_tenant(sender, **kwargs): # pylint: disable=unused-argument @@ -219,9 +235,24 @@ def tenant_context_addition(sender, body, headers, *args, **kwargs): # pylint: body['kwargs']['eox_tenant_sender'] = get_host_func(body) -def start_async_tenant(sender, *args, **kwargs): # pylint: disable=unused-argument +def start_async_lms_tenant(sender, *args, **kwargs): # pylint: disable=unused-argument + """ + Receiver that runs on the LMS async process to update the settings accordingly to the tenant. + Read documentation of `_start_tenant` for more details. + """ + _start_async_tenant(sender, LMS_CONFIG_COLUMN) + + +def start_async_studio_tenant(sender, *args, **kwargs): # pylint: disable=unused-argument + """ + Receiver that runs on the studio async process to update the settings accordingly to the tenant. + Read documentation of `_start_tenant` for more details. + """ + _start_async_tenant(sender, CMS_CONFIG_COLUMN) + + +def _start_async_tenant(sender, config_key): """ - Receiver that runs on the async process to update the settings accordingly to the tenant. Dispatched before a task is executed. See: https://celery.readthedocs.io/en/latest/userguide/signals.html#task-prerun @@ -245,7 +276,7 @@ def start_async_tenant(sender, *args, **kwargs): # pylint: disable=unused-argum return # Do the update - _update_settings(domain) + _update_settings(domain, config_key) def update_tenant_organizations(instance, **kwargs): # pylint: disable=unused-argument diff --git a/eox_tenant/test/test_receivers_helpers.py b/eox_tenant/test/test_receivers_helpers.py index 32245330..b21b9bf1 100644 --- a/eox_tenant/test/test_receivers_helpers.py +++ b/eox_tenant/test/test_receivers_helpers.py @@ -6,6 +6,7 @@ from django.test import TestCase +from eox_tenant.constants import CMS_CONFIG_COLUMN, LMS_CONFIG_COLUMN from eox_tenant.models import Microsite, Route, TenantConfig from eox_tenant.receivers_helpers import get_tenant_config_by_domain @@ -34,7 +35,9 @@ def setUp(self): "course_org_filter": ["test5-org", "test1-org"], "value-test": "Hello-World3", }, - studio_configs={}, + studio_configs={ + "value-test": "studio", + }, theming_configs={}, meta={}, ) @@ -44,11 +47,11 @@ def setUp(self): config=TenantConfig.objects.get(external_key="tenant-key1"), ) - def test_tenant_get_config_by_domain(self): + def test_tenant_get_lms_config_by_domain(self): """ Test to get the configuration and external key for a given domain. """ - configurations, external_key = get_tenant_config_by_domain("domain1") + configurations, external_key = get_tenant_config_by_domain("domain1", LMS_CONFIG_COLUMN) self.assertEqual(external_key, "tenant-key1") self.assertDictEqual( @@ -59,12 +62,34 @@ def test_tenant_get_config_by_domain(self): }, ) - configurations, external_key = get_tenant_config_by_domain("domain2") + def test_tenant_get_studio_config_by_domain(self): + """ + Test to get the configuration and external key for a given domain. + """ + configurations, external_key = get_tenant_config_by_domain("domain1", CMS_CONFIG_COLUMN) + + self.assertEqual(external_key, "tenant-key1") + self.assertDictEqual( + configurations, + { + "value-test": "studio", + }, + ) + + def test_no_tenant_get_config_by_domain(self): + """ + Test to get the configuration and external key for a non-existent domain. + """ + configurations, external_key = get_tenant_config_by_domain("domain2", LMS_CONFIG_COLUMN) self.assertEqual(external_key, None) self.assertDictEqual(configurations, {}) - configurations, external_key = get_tenant_config_by_domain("first.test.prod.edunext") + def test_microsite_get_config_by_domain(self): + """ + Test to get the configuration and external key for a given domain from Microsite table. + """ + configurations, external_key = get_tenant_config_by_domain("first.test.prod.edunext", LMS_CONFIG_COLUMN) self.assertEqual(external_key, "test_fake_key") self.assertDictEqual( diff --git a/eox_tenant/test/test_signals.py b/eox_tenant/test/test_signals.py index 820bd792..a655423a 100644 --- a/eox_tenant/test/test_signals.py +++ b/eox_tenant/test/test_signals.py @@ -11,13 +11,16 @@ from django.test import TestCase from mock import MagicMock, patch +from eox_tenant.constants import CMS_CONFIG_COLUMN, LMS_CONFIG_COLUMN from eox_tenant.signals import ( _analyze_current_settings, _repopulate_apps, _ttl_reached, _update_settings, - start_async_tenant, - start_tenant, + start_async_lms_tenant, + start_async_studio_tenant, + start_lms_tenant, + start_studio_tenant, tenant_context_addition, ) @@ -40,7 +43,13 @@ def test_must_reset_because_domain(self, _, _reset_mock, _analyze_mock): _analyze_mock.return_value = (True, None) with self.settings(SERVICE_VARIANT='lms'): - start_tenant(None, environ) + start_lms_tenant(None, environ) + + _reset_mock.assert_called() + _reset_mock.reset_mock() + + with self.settings(SERVICE_VARIANT='cms'): + start_studio_tenant(None, environ) _reset_mock.assert_called() @@ -59,7 +68,13 @@ def test_must_reset_ttl(self, _ttl_mock, _, _reset_mock, _analyze_mock): _ttl_mock.return_value = True with self.settings(SERVICE_VARIANT='lms'): - start_tenant(None, environ) + start_lms_tenant(None, environ) + + _reset_mock.assert_called() + _reset_mock.reset_mock() + + with self.settings(SERVICE_VARIANT='cms'): + start_studio_tenant(None, environ) _reset_mock.assert_called() @@ -77,7 +92,7 @@ def test_can_not_keep_because_analysis(self, _update_mock, _reset_mock, _analyze _analyze_mock.return_value = (False, False) with self.settings(SERVICE_VARIANT='lms'): - start_tenant(None, environ) + start_lms_tenant(None, environ) _reset_mock.assert_not_called() _update_mock.assert_called() @@ -96,7 +111,7 @@ def test_can_keep(self, _update_mock, _reset_mock, _analyze_mock): _analyze_mock.return_value = (False, True) with self.settings(SERVICE_VARIANT='lms'): - start_tenant(None, environ) + start_lms_tenant(None, environ) _reset_mock.assert_not_called() _update_mock.assert_not_called() @@ -152,7 +167,7 @@ def test__repopulate_apps_not_called(self, _repopulate_mock): environ.get.return_value = 'tenant.com' with self.settings(SERVICE_VARIANT='lms'): - start_tenant(None, environ) + start_lms_tenant(None, environ) _repopulate_mock.assert_not_called() @@ -170,7 +185,7 @@ def test__repopulate_apps_called(self, _repopulate_mock, _get_config_mock): _get_config_mock.return_value = config, "tenant-key" with self.settings(SERVICE_VARIANT='lms', EDNX_TENANT_INSTALLED_APPS=['fake_app']): - start_tenant(None, environ) + start_lms_tenant(None, environ) _repopulate_mock.assert_called() @@ -208,7 +223,7 @@ def test_udpate_settings_mark(self, _get_config_mock): } _get_config_mock.return_value = config, "tenant-key" - _update_settings("tenant.com") + _update_settings("tenant.com", LMS_CONFIG_COLUMN) with self.assertRaises(AttributeError): settings.EDNX_TENANT_KEY # pylint: disable=pointless-statement @@ -223,7 +238,7 @@ def test_udpate_settings(self, _get_config_mock): } _get_config_mock.return_value = config, "tenant-key" - _update_settings("tenant.com") + _update_settings("tenant.com", LMS_CONFIG_COLUMN) settings.EDNX_TENANT_KEY # pylint: disable=pointless-statement @@ -238,7 +253,7 @@ def test_udpate_settings_with_property(self, _get_config_mock): } _get_config_mock.return_value = config, "tenant-key" - _update_settings("tenant.com") + _update_settings("tenant.com", LMS_CONFIG_COLUMN) self.assertEqual(settings.TEST_PROPERTY, "My value") @@ -255,7 +270,7 @@ def test_udpate_settings_with_dict(self, _get_config_mock): } _get_config_mock.return_value = config, "tenant-key" - _update_settings("tenant.com") + _update_settings("tenant.com", LMS_CONFIG_COLUMN) self.assertEqual(settings.TEST_DICT.get("TEST_PROPERTY"), "My value") @@ -272,7 +287,7 @@ def test_udpate_settings_with_existing_dict(self, _get_config_mock): } _get_config_mock.return_value = config, "tenant-key" - _update_settings("tenant.com") + _update_settings("tenant.com", LMS_CONFIG_COLUMN) self.assertEqual(settings.TEST_DICT_OVERRIDE_TEST.get("key1"), "Some Value") self.assertEqual(settings.TEST_DICT_OVERRIDE_TEST.get("key2"), "My value") @@ -354,27 +369,53 @@ class CeleryReceiverAsyncTests(TestCase): """ @patch('eox_tenant.signals._update_settings') - def test_start_async_tenant(self, _update_mock): + def test_start_lms_async_tenant(self, _update_mock): + """ + Test function used to handle signal in the async process + """ + sender = MagicMock() + headers = {"eox_tenant_sender": "some.tenant.com"} + sender.request = {"headers": headers} + start_async_lms_tenant(sender) + + _update_mock.assert_called_with("some.tenant.com", LMS_CONFIG_COLUMN) + + @patch('eox_tenant.signals._perform_reset') + @patch('eox_tenant.signals._update_settings') + def test_start_async_lms_tenant_with_null_header(self, _update_mock, _reset_mock): + """ + Test function used to handle signal in the async process. + """ + sender = MagicMock() + headers = None + sender.request = {"headers": headers} + start_async_lms_tenant(sender) + + _reset_mock.assert_called() + _update_mock.assert_not_called() + + @patch('eox_tenant.signals._update_settings') + def test_start_async_studio_tenant(self, _update_mock): """ Test function used to handle signal in the async process """ sender = MagicMock() headers = {"eox_tenant_sender": "some.tenant.com"} sender.request = {"headers": headers} - start_async_tenant(sender) + start_async_studio_tenant(sender) - _update_mock.assert_called_with("some.tenant.com") + _update_mock.assert_called_with("some.tenant.com", CMS_CONFIG_COLUMN) @patch('eox_tenant.signals._perform_reset') @patch('eox_tenant.signals._update_settings') - def test_start_async_tenant_with_null_header(self, _update_mock, _reset_mock): + def test_start_async_tenant_studio_with_null_header(self, _update_mock, _reset_mock): """ Test function used to handle signal in the async process. """ sender = MagicMock() headers = None sender.request = {"headers": headers} - start_async_tenant(sender) + start_async_studio_tenant(sender) _reset_mock.assert_called() _update_mock.assert_not_called()