From 6b339b5852be1b90fe032b9a679db233f53a0ab2 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Mon, 3 Jul 2023 11:45:17 +0530 Subject: [PATCH] feat: separate studio config and handler --- eox_tenant/apps.py | 8 +-- eox_tenant/constants.py | 1 + eox_tenant/models.py | 4 +- eox_tenant/receivers_helpers.py | 4 +- eox_tenant/signals.py | 46 +++++++++++--- eox_tenant/test/test_receivers_helpers.py | 34 ++++++++-- eox_tenant/test/test_signals.py | 76 +++++++++++++++++------ 7 files changed, 134 insertions(+), 39 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..7feb4e12 100644 --- a/eox_tenant/constants.py +++ b/eox_tenant/constants.py @@ -2,3 +2,4 @@ from django.conf import settings LMS_ENVIRONMENT = getattr(settings, "SERVICE_VARIANT", None) == "lms" +CMS_ENVIRONMENT = getattr(settings, "SERVICE_VARIANT", None) == "cms" diff --git a/eox_tenant/models.py b/eox_tenant/models.py index 551baeb6..0c970e01 100644 --- a/eox_tenant/models.py +++ b/eox_tenant/models.py @@ -183,7 +183,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. @@ -194,7 +194,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..58f923de 100644 --- a/eox_tenant/receivers_helpers.py +++ b/eox_tenant/receivers_helpers.py @@ -6,7 +6,7 @@ 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. @@ -20,7 +20,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 40db51fd..b37144cf 100644 --- a/eox_tenant/signals.py +++ b/eox_tenant/signals.py @@ -38,11 +38,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 +157,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_configs") + + +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, "studio_configs") + + +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 +197,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 +234,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_configs") + + +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, "studio_configs") + + +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 +275,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..ae250cb3 100644 --- a/eox_tenant/test/test_receivers_helpers.py +++ b/eox_tenant/test/test_receivers_helpers.py @@ -34,7 +34,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 +46,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_configs") self.assertEqual(external_key, "tenant-key1") self.assertDictEqual( @@ -59,12 +61,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", "studio_configs") + + 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_configs") 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_configs") 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 8c4dd3ff..ba69b7ff 100644 --- a/eox_tenant/test/test_signals.py +++ b/eox_tenant/test/test_signals.py @@ -16,8 +16,10 @@ _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 +42,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 +67,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 +91,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 +110,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 +166,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 +184,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 +222,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_configs") with self.assertRaises(AttributeError): settings.EDNX_TENANT_KEY # pylint: disable=pointless-statement @@ -223,7 +237,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_configs") settings.EDNX_TENANT_KEY # pylint: disable=pointless-statement @@ -238,7 +252,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_configs") self.assertEqual(settings.TEST_PROPERTY, "My value") @@ -255,7 +269,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_configs") self.assertEqual(settings.TEST_DICT.get("TEST_PROPERTY"), "My value") @@ -272,7 +286,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_configs") self.assertEqual(settings.TEST_DICT_OVERRIDE_TEST.get("key1"), "Some Value") self.assertEqual(settings.TEST_DICT_OVERRIDE_TEST.get("key2"), "My value") @@ -354,27 +368,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_configs") + + @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", "studio_configs") @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()