[feature] Invalidate VPN cache when organization config variables change

When organization configuration variables are updated, VPN cache wasn't
being invalidated automatically. This caused VPNs to use stale config
data leading to connectivity issues.

Added signal handler to detect context changes in OrganizationConfigSettings
and trigger cache invalidation for all VPNs in that organization. Also
added comprehensive tests to ensure the functionality works correctly.

Fixes #1098

Author: Srinath0916

openwisp_controller/config/apps.py

@@ -270,6 +270,7 @@ def enable_cache_invalidation(self):
             device_cache_invalidation_handler,
             devicegroup_change_handler,
             devicegroup_delete_handler,
+            organization_config_settings_change_handler,
             vpn_server_change_handler,
         )
 
@@ -336,6 +337,11 @@ def enable_cache_invalidation(self):
             sender=self.vpn_model,
             dispatch_uid="vpn.invalidate_checksum_cache",
         )
+        post_save.connect(
+            organization_config_settings_change_handler,
+            sender=self.organization_config_settings_model,
+            dispatch_uid="organization_config_settings.invalidate_vpn_cache",
+        )
 
     def register_dashboard_charts(self):
         register_dashboard_chart(

openwisp_controller/config/handlers.py

@@ -152,3 +152,22 @@ def organization_disabled_handler(instance, **kwargs):
         # No change in is_active
         return
     tasks.invalidate_controller_views_cache.delay(str(instance.id))
+
+
+def organization_config_settings_change_handler(instance, **kwargs):
+    """
+    Invalidates VPN cache when OrganizationConfigSettings context changes.
+    """
+    if instance._state.adding:
+        return
+
+    try:
+        db_instance = instance.__class__.objects.only("context").get(id=instance.id)
+        if db_instance.context != instance.context:
+            transaction.on_commit(
+                lambda: tasks.invalidate_organization_vpn_cache.delay(
+                    str(instance.organization_id)
+                )
+            )
+    except instance.__class__.DoesNotExist:
+        pass

openwisp_controller/config/tasks.py

@@ -100,6 +100,27 @@ def invalidate_vpn_server_devices_cache_change(vpn_pk):
     VpnClient.invalidate_clients_cache(vpn)
 
 
+@shared_task(soft_time_limit=7200)
+def invalidate_organization_vpn_cache(organization_id):
+    """
+    Invalidates VPN cache for all VPNs in an organization when
+    organization configuration variables change.
+    """
+    from .controller.views import GetVpnView
+
+    Vpn = load_model("config", "Vpn")
+
+    try:
+        vpn_queryset = Vpn.objects.filter(organization_id=organization_id).only("id")
+        for vpn in vpn_queryset.iterator():
+            GetVpnView.invalidate_get_vpn_cache(vpn)
+            vpn.invalidate_checksum_cache()
+    except Exception as e:
+        logger.error(
+            f"Error invalidating VPN cache for organization {organization_id}: {e}"
+        )
+
+
 @shared_task(soft_time_limit=7200)
 def invalidate_devicegroup_cache_delete(instance_id, model_name, **kwargs):
     from .api.views import DeviceGroupCommonName

openwisp_controller/config/tests/test_organization_vpn_cache.py

@@ -0,0 +1,180 @@
+"""
+Tests for VPN cache invalidation when Organization Configuration variables change
+Addresses GitHub issue #1098
+"""
+
+from unittest import mock
+
+from django.test import TestCase, TransactionTestCase
+from django.test.utils import override_settings
+from swapper import load_model
+
+from openwisp_controller.config.tests.utils import CreateConfigTemplateMixin
+
+Device = load_model('config', 'Device')
+Vpn = load_model('config', 'Vpn')
+OrganizationConfigSettings = load_model('config', 'OrganizationConfigSettings')
+
+
+class TestOrganizationVpnCacheInvalidation(CreateConfigTemplateMixin, TransactionTestCase):
+    """
+    Test VPN cache invalidation when Organization Configuration variables change
+    """
+
+    def setUp(self):
+        super().setUp()
+
+    def _create_org_config_settings(self, organization=None, context=None):
+        """Helper method to create organization config settings"""
+        if organization is None:
+            organization = self._get_org()
+        if context is None:
+            context = {'test_var': 'initial_value'}
+
+        return OrganizationConfigSettings.objects.create(
+            organization=organization,
+            context=context
+        )
+
+    def test_vpn_cache_invalidated_on_context_change(self):
+        """
+        Test that VPN cache is invalidated when organization context changes
+        """
+        org = self._get_org()
+        vpn = self._create_vpn(organization=org)
+
+        org_config = self._create_org_config_settings(
+            organization=org,
+            context={'api_key': 'test123'}
+        )
+
+        with mock.patch(
+            'openwisp_controller.config.tasks.invalidate_organization_vpn_cache.delay'
+        ) as mock_task:
+            org_config.context = {'api_key': 'test456'}
+            org_config.save()
+
+            mock_task.assert_called_once_with(str(org.id))
+
+    def test_vpn_cache_not_invalidated_on_non_context_change(self):
+        """
+        Test that VPN cache is NOT invalidated when non-context fields change
+        """
+        org = self._get_org()
+        vpn = self._create_vpn(organization=org)
+
+        org_config = self._create_org_config_settings(organization=org)
+
+        with mock.patch(
+            'openwisp_controller.config.tasks.invalidate_organization_vpn_cache.delay'
+        ) as mock_task:
+            org_config.registration_enabled = False
+            org_config.save()
+
+            mock_task.assert_not_called()
+
+    def test_vpn_cache_not_invalidated_on_create(self):
+        """
+        Test that VPN cache is NOT invalidated when creating new org config settings
+        """
+        org = self._get_org()
+        vpn = self._create_vpn(organization=org)
+
+        with mock.patch(
+            'openwisp_controller.config.tasks.invalidate_organization_vpn_cache.delay'
+        ) as mock_task:
+            self._create_org_config_settings(organization=org)
+
+            mock_task.assert_not_called()
+
+    @mock.patch('openwisp_controller.config.controller.views.GetVpnView.invalidate_get_vpn_cache')
+    def test_invalidate_organization_vpn_cache_task(self, mock_invalidate_view_cache):
+        """
+        Test the invalidate_organization_vpn_cache task directly
+        """
+        from openwisp_controller.config.tasks import invalidate_organization_vpn_cache
+
+        org = self._get_org()
+        vpn1 = self._create_vpn(organization=org, name='vpn1')
+        vpn2 = self._create_vpn(organization=org, name='vpn2')
+
+        other_org = self._create_org(name='other-org', slug='other-org')
+        vpn3 = self._create_vpn(organization=other_org, name='vpn3')
+
+        with mock.patch.object(vpn1, 'invalidate_checksum_cache') as mock_vpn1_cache, \
+             mock.patch.object(vpn2, 'invalidate_checksum_cache') as mock_vpn2_cache, \
+             mock.patch.object(vpn3, 'invalidate_checksum_cache') as mock_vpn3_cache:
+
+            invalidate_organization_vpn_cache(str(org.id))
+
+            self.assertEqual(mock_invalidate_view_cache.call_count, 2)
+            mock_vpn1_cache.assert_called_once()
+            mock_vpn2_cache.assert_called_once()
+            mock_vpn3_cache.assert_not_called()
+
+    @override_settings(CELERY_TASK_ALWAYS_EAGER=True)
+    def test_end_to_end_cache_invalidation(self):
+        """
+        Test end-to-end cache invalidation with actual task execution
+        """
+        org = self._get_org()
+        vpn = self._create_vpn(organization=org)
+
+        org_config = self._create_org_config_settings(organization=org)
+
+        with mock.patch.object(vpn, 'invalidate_checksum_cache') as mock_vpn_cache, \
+             mock.patch(
+                 'openwisp_controller.config.controller.views.GetVpnView.invalidate_get_vpn_cache'
+             ) as mock_view_cache:
+
+            org_config.context = {'changed': 'value'}
+            org_config.save()
+
+            mock_vpn_cache.assert_called_once()
+            mock_view_cache.assert_called_once_with(vpn)
+
+    def test_task_error_handling(self):
+        """
+        Test that the task handles errors gracefully
+        """
+        from openwisp_controller.config.tasks import invalidate_organization_vpn_cache
+
+        try:
+            invalidate_organization_vpn_cache('non-existent-id')
+        except Exception as e:
+            self.fail(f"Task should handle errors gracefully, but raised: {e}")
+
+
+class TestOrganizationVpnCacheInvalidationSimple(CreateConfigTemplateMixin, TestCase):
+    """
+    Simple test cases for basic functionality verification
+    """
+
+    def test_handler_function_exists(self):
+        """Test that the handler function is properly defined"""
+        from openwisp_controller.config.handlers import organization_config_settings_change_handler
+
+        self.assertTrue(callable(organization_config_settings_change_handler))
+
+    def test_task_function_exists(self):
+        """Test that the task function is properly defined"""
+        from openwisp_controller.config.tasks import invalidate_organization_vpn_cache
+
+        self.assertTrue(callable(invalidate_organization_vpn_cache))
+
+    def test_signal_connection(self):
+        """Test that the signal is properly connected"""
+        from django.db.models.signals import post_save
+        from openwisp_controller.config.handlers import organization_config_settings_change_handler
+
+        receivers = post_save._live_receivers(sender=OrganizationConfigSettings)
+
+        handler_connected = any(
+            receiver.__name__ == 'organization_config_settings_change_handler'
+            for receiver in receivers
+        )
+
+        self.assertTrue(
+            handler_connected,
+            "organization_config_settings_change_handler should be connected to post_save signal"
+        )