From c172123506b218c4386ae940d475699fed478eba Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 13 Jul 2023 23:36:23 +0530 Subject: [PATCH] [req-changes] Updated README and fixed precedence of variables --- README.rst | 46 +++++++++---------- openwisp_controller/config/base/config.py | 31 ++++++++----- ...context.py => 0049_devicegroup_context.py} | 2 +- .../config/tests/test_admin.py | 6 +-- .../config/tests/test_device.py | 34 ++++++++++++++ 5 files changed, 79 insertions(+), 40 deletions(-) rename openwisp_controller/config/migrations/{0048_devicegroup_context.py => 0049_devicegroup_context.py} (93%) diff --git a/README.rst b/README.rst index 569a2c89b..0be7fe2ff 100644 --- a/README.rst +++ b/README.rst @@ -536,10 +536,20 @@ the order (high to low) of their precedence: 1. `User defined device variables <#user-defined-device-variables>`_ 2. `Predefined device variables <#predefined-device-variables>`_ -3. `Global variables <#global-variables>`_ -4. `Group variables <#group-variables>`_ +3. `Group variables <#group-variables>`_ +4. `Global variables <#global-variables>`_ 5. `Template default values <#template-default-values>`_ +User defined device variables +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In the device configuration section you can find a section named +"Configuration variables" where it is possible to define the configuration +variables and their values, as shown in the example below: + +.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png + :alt: context + Predefined device variables ~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -550,15 +560,19 @@ Each device gets the following attributes passed as configuration variables: * ``name`` * ``mac_address`` -User defined device variables -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Group variables +~~~~~~~~~~~~~~~ -In the device configuration section you can find a section named -"Configuration variables" where it is possible to define the configuration -variables and their values, as shown in the example below: +Variables can also be defined in `Device groups <#device-groups>`__. -.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png - :alt: context +Refer the `Group configuration variables `_ +section for detailed information. + +Global variables +~~~~~~~~~~~~~~~~ + +Variables can also be defined globally using the +`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting. Template default values ~~~~~~~~~~~~~~~~~~~~~~~ @@ -581,20 +595,6 @@ The default values of variables can be manipulated from the section .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/template-default-values.png :alt: default values -Group variables -~~~~~~~~~~~~~~~ - -Variables can also be defined in `Device groups <#device-groups>`__. - -Refer the `Group configuration variables `_ -section for detailed information. - -Global variables -~~~~~~~~~~~~~~~~ - -Variables can also be defined globally using the -`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting. - System defined variables ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index a535ad13e..d3f149915 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -558,7 +558,7 @@ def _has_device(self): return hasattr(self, 'device') def get_vpn_context(self): - context = super().get_context() + context = {} for vpnclient in self.vpnclient_set.all().select_related('vpn', 'cert'): vpn = vpnclient.vpn vpn_id = vpn.pk.hex @@ -599,15 +599,11 @@ def get_context(self, system=False): additional context passed to netjsonconfig """ c = collections.OrderedDict() - extra = {} - if hasattr(self, 'device') and self.device._get_group(): - extra.update(self.device._get_group().get_context()) - extra.update(self.get_vpn_context()) - for func in self._config_context_functions: - extra.update(func(config=self)) - if app_settings.HARDWARE_ID_ENABLED and self._has_device(): - extra.update({'hardware_id': str(self.device.hardware_id)}) + # Add global variables + context = super().get_context() if self._has_device(): + # These pre-defined variables are needed at the start of OrderedDict. + # Hence, they are added separately. c.update( [ ('name', self.name), @@ -616,9 +612,20 @@ def get_context(self, system=False): ('key', self.key), ] ) - if self.context and not system: - extra.update(self.context) - c.update(sorted(extra.items())) + if self.device._get_group(): + # Add device group variables + context.update(self.device._get_group().get_context()) + # Add predefined variables + context.update(self.get_vpn_context()) + for func in self._config_context_functions: + context.update(func(config=self)) + if app_settings.HARDWARE_ID_ENABLED: + context.update({'hardware_id': str(self.device.hardware_id)}) + + if self.context and not system: + context.update(self.context) + + c.update(sorted(context.items())) return c def get_system_context(self): diff --git a/openwisp_controller/config/migrations/0048_devicegroup_context.py b/openwisp_controller/config/migrations/0049_devicegroup_context.py similarity index 93% rename from openwisp_controller/config/migrations/0048_devicegroup_context.py rename to openwisp_controller/config/migrations/0049_devicegroup_context.py index 6dcb6af03..c54c41024 100644 --- a/openwisp_controller/config/migrations/0048_devicegroup_context.py +++ b/openwisp_controller/config/migrations/0049_devicegroup_context.py @@ -9,7 +9,7 @@ class Migration(migrations.Migration): dependencies = [ - ('config', '0047_add_organizationlimits'), + ('config', '0048_wifi_radio_band_migration'), ] operations = [ diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index cd2552154..c54012ed6 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -1519,16 +1519,14 @@ def test_system_context(self): path = reverse(f'admin:{self.app_label}_device_change', args=[d.pk]) self._test_system_context_field_helper(path) - def test_no_system_context(self): + @patch.dict(app_settings.CONTEXT, {}) + def test_no_system_context(self, *args): self._create_template() - old_context = app_settings.CONTEXT.copy() - app_settings.CONTEXT = {} path = reverse(f'admin:{self.app_label}_template_add') r = self.client.get(path) self.assertContains( r, 'There are no system defined variables available right now' ) - app_settings.CONTEXT = old_context def test_config_form_old_templates(self): config = self._create_config(organization=self._get_org()) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index d7d5498d3..5b5222c5b 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -321,6 +321,40 @@ def test_device_get_system_context(self): self.assertEqual(system_context['SSID'], 'OpenWISP') self.assertNotIn('test', system_context.keys()) + @mock.patch.dict(app_settings.CONTEXT, {'variable_type': 'global-context-variable'}) + def test_configuration_variable_priority(self, *args): + org = self._get_org() + device_group = self._create_device_group( + organization=org, context={'variable_type': 'device-group-variable'} + ) + device = self._create_device(organization=org, group=device_group) + config = self._create_config( + device=device, + context={ + 'id': 'user-defined-variable', + 'variable_type': 'user-defined-variable', + }, + ) + + # User defined variable has highest precedence + self.assertEqual(config.get_context()['id'], 'user-defined-variable') + self.assertEqual(config.get_context()['variable_type'], 'user-defined-variable') + # Second precedence is given to predefined device variables + config.context = {} + self.assertEqual( + config.get_context()['id'], + str(device.pk), + ) + # It is not possible to overwrite the pre-defined device variables. + # Therefore, for further tests, "tesT" variable is used. + # Third precedence is given to the device group variable' + self.assertEqual(config.get_context()['variable_type'], 'device-group-variable') + # Fourth precedence is given to global variables + device.group = None + self.assertEqual( + config.get_context()['variable_type'], 'global-context-variable' + ) + def test_management_ip_changed_not_emitted_on_creation(self): with catch_signal(management_ip_changed) as handler: self._create_device(organization=self._get_org())