[tests] Added tests

Author: pandafy

openwisp_controller/config/apps.py

@@ -264,7 +264,7 @@ def enable_cache_invalidation(self):
         Triggers the cache invalidation for the
         device config checksum (view and model method)
         """
-        from .controller.views import DeviceChecksumView
+        from .controller.views import DeviceChecksumView, VpnChecksumView
         from .handlers import (
             device_cache_invalidation_handler,
             devicegroup_change_handler,
@@ -293,6 +293,21 @@ def enable_cache_invalidation(self):
             DeviceChecksumView.invalidate_checksum_cache,
             dispatch_uid="invalidate_checksum_cache",
         )
+        # VPN cache invalidation
+        post_save.connect(
+            VpnChecksumView.invalidate_get_vpn_cache,
+            sender=self.vpn_model,
+            dispatch_uid="invalidate_get_vpn_cache",
+        )
+        pre_delete.connect(
+            VpnChecksumView.invalidate_get_vpn_cache,
+            sender=self.vpn_model,
+            dispatch_uid="vpn_server_pre_delete_invalidate_get_vpn_cache",
+        )
+        vpn_server_modified.connect(
+            VpnChecksumView.invalidate_get_vpn_cache,
+            dispatch_uid="vpn_server_modified_invalidate_get_vpn_cache",
+        )
         device_group_changed.connect(
             devicegroup_change_handler,
             sender=self.device_model,

openwisp_controller/config/base/vpn.py

@@ -148,7 +148,7 @@ class Meta:
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         # for internal usage
-        self._send_vpn_modified_after_save = False
+        self._should_send_vpn_modified_after_save = False
 
     def clean(self, *args, **kwargs):
         super().clean(*args, **kwargs)
@@ -280,10 +280,10 @@ def save(self, *args, **kwargs):
             raise e
         if create_dh:
             transaction.on_commit(lambda: create_vpn_dh.delay(self.id))
-        if not created and self._send_vpn_modified_after_save:
+        if not created and self._should_send_vpn_modified_after_save:
             self.invalidate_checksum_cache()
             self._send_vpn_modified_signal()
-            self._send_vpn_modified_after_save = False
+            self._should_send_vpn_modified_after_save = False
         # For ZeroTier VPN server, if the
         # ZeroTier network is created successfully,
         # this method triggers a background task to
@@ -310,7 +310,7 @@ def _check_changes(self):
         current = self._meta.model.objects.only(*attrs).get(pk=self.pk)
         for attr in attrs:
             if getattr(self, attr) != getattr(current, attr):
-                self._send_vpn_modified_after_save = True
+                self._should_send_vpn_modified_after_save = True
                 break
 
     def _send_vpn_modified_signal(self):

openwisp_controller/config/controller/views.py

@@ -127,6 +127,18 @@ def get_device_args_rewrite(view):
     return pk.hex
 
 
+def get_vpn_args_rewrite(view):
+    """
+    Use only the PK parameter for calculating the cache key for VPN
+    """
+    pk = view.kwargs["pk"]
+    try:
+        pk = uuid.UUID(pk)
+    except ValueError:
+        return pk
+    return pk.hex
+
+
 class DeviceChecksumView(UpdateLastIpMixin, GetDeviceView):
     """
     returns device's configuration checksum
@@ -463,8 +475,10 @@ class GetVpnView(SingleObjectMixin, View):
     model = Vpn
 
     def get_object(self, *args, **kwargs):
-        queryset = self.model.objects.select_related("organization").filter(
-            Q(organization__is_active=True) | Q(organization__isnull=True)
+        queryset = (
+            self.model.objects.select_related("organization")
+            .filter(Q(organization__is_active=True) | Q(organization__isnull=True))
+            .select_related("ca", "cert", "subnet", "ip")
         )
         return get_object_or_404(queryset, *args, **kwargs)
 
@@ -475,21 +489,40 @@ class VpnChecksumView(GetVpnView):
     """
 
     def get(self, request, *args, **kwargs):
-        vpn = self.get_object(*args, **kwargs)
+        vpn = self.get_vpn()
         bad_request = forbid_unallowed(request, "GET", "key", vpn.key)
         if bad_request:
             return bad_request
         checksum_requested.send(sender=vpn.__class__, instance=vpn, request=request)
         return ControllerResponse(vpn.get_cached_checksum(), content_type="text/plain")
 
+    @cache_memoize(
+        timeout=Vpn._CHECKSUM_CACHE_TIMEOUT, args_rewrite=get_vpn_args_rewrite
+    )
+    def get_vpn(self):
+        pk = self.kwargs["pk"]
+        logger.debug(f"retrieving VPN ID {pk} from DB")
+        return self.get_object(pk=pk)
+
+    @classmethod
+    def invalidate_get_vpn_cache(cls, instance, **kwargs):
+        """
+        Called from signal receiver which performs cache invalidation
+        """
+        view = cls()
+        pk = str(instance.pk.hex)
+        view.kwargs = {"pk": pk}
+        view.get_vpn.invalidate(view)
+        logger.debug(f"invalidated view cache for VPN ID {pk}")
+
 
 class VpnDownloadConfigView(GetVpnView):
     """
     returns configuration archive as attachment
     """
 
     def get(self, request, *args, **kwargs):
-        vpn = self.get_object(*args, **kwargs)
+        vpn = self.get_vpn()
         bad_request = forbid_unallowed(request, "GET", "key", vpn.key)
         if bad_request:
             return bad_request

openwisp_controller/config/handlers.py

@@ -140,7 +140,7 @@ def devicegroup_templates_change_handler(instance, **kwargs):
 
 def organization_disabled_handler(instance, **kwargs):
     """
-    Asynchronously invalidates DeviceCheckView.get_device cache
+    Asynchronously invalidates device and VPN controller views cache
     """
     if instance.is_active:
         return
@@ -151,4 +151,4 @@ def organization_disabled_handler(instance, **kwargs):
     if instance.is_active == db_instance.is_active:
         # No change in is_active
         return
-    tasks.invalidate_device_checksum_view_cache.delay(str(instance.id))
+    tasks.invalidate_controller_views_cache.delay(str(instance.id))

openwisp_controller/config/tasks.py

@@ -149,11 +149,31 @@ def bulk_invalidate_config_get_cached_checksum(query_params):
 
 
 @shared_task(base=OpenwispCeleryTask)
-def invalidate_device_checksum_view_cache(organization_id):
-    from .controller.views import DeviceChecksumView
+def invalidate_controller_views_cache(organization_id):
+    """
+    Asynchronously invalidates device and VPN controller views cache
+    """
+    from .controller.views import DeviceChecksumView, VpnChecksumView
 
     Device = load_model("config", "Device")
+    Vpn = load_model("config", "Vpn")
+
+    # Invalidate device views cache
     for device in (
         Device.objects.filter(organization_id=organization_id).only("id").iterator()
     ):
         DeviceChecksumView.invalidate_get_device_cache(device)
+
+    # Invalidate VPN views cache
+    for vpn in (
+        Vpn.objects.filter(organization_id=organization_id).only("id").iterator()
+    ):
+        VpnChecksumView.invalidate_get_vpn_cache(vpn)
+
+
+@shared_task(base=OpenwispCeleryTask)
+def invalidate_device_checksum_view_cache(organization_id):
+    """
+    Deprecated: Use invalidate_controller_views_cache instead.
+    """
+    return invalidate_controller_views_cache(organization_id)

openwisp_controller/config/tests/test_controller.py

@@ -12,14 +12,15 @@
 
 from .. import settings as app_settings
 from ..base.base import logger as base_config_logger
-from ..controller.views import DeviceChecksumView
+from ..controller.views import DeviceChecksumView, VpnChecksumView
 from ..controller.views import logger as controller_views_logger
 from ..signals import (
     checksum_requested,
     config_download_requested,
     config_modified,
     config_status_changed,
     device_registered,
+    vpn_server_modified,
 )
 from .utils import CreateConfigTemplateMixin, TestVpnX509Mixin
 
@@ -365,11 +366,20 @@ def test_device_download_config_405(self):
         self.assertEqual(response.status_code, 405)
 
     def test_vpn_checksum(self):
-        v = self._create_vpn()
-        url = reverse("controller:vpn_checksum", args=[v.pk])
-        response = self.client.get(url, {"key": v.key})
-        self.assertEqual(response.content.decode(), v.checksum)
-        self._check_header(response)
+        vpn = self._create_vpn()
+        url = reverse("controller:vpn_checksum", args=[vpn.pk])
+
+        with self.subTest("First request will calculate the checksum"):
+            with self.assertNumQueries(3):
+                response = self.client.get(url, {"key": vpn.key})
+            self.assertEqual(response.content.decode(), vpn.checksum)
+            self._check_header(response)
+
+        with self.subTest("Second request will return cached checksum"):
+            with self.assertNumQueries(0):
+                response = self.client.get(url, {"key": vpn.key})
+            self.assertEqual(response.content.decode(), vpn.checksum)
+            self._check_header(response)
 
     def test_vpn_checksum_bad_uuid(self):
         v = self._create_vpn()
@@ -408,6 +418,49 @@ def test_vpn_checksum_org_disabled(self):
             vpn, reverse("controller:vpn_checksum", args=[vpn.pk])
         )
 
+    def test_vpn_get_object_cached(self):
+        vpn = self._create_vpn()
+        view = VpnChecksumView()
+        view.kwargs = {"pk": str(vpn.pk)}
+
+        with self.subTest("check cache set"):
+            with patch("django.core.cache.cache.set") as mock:
+                self.assertEqual(view.get_vpn(), vpn)
+                mock.assert_called_once()
+
+        with self.subTest("check cache get"):
+            with patch("django.core.cache.cache.get", return_value=vpn) as mock:
+                view.get_vpn()
+                mock.assert_called_once()
+
+        with self.subTest("check cache invalidation"):
+            with patch("django.core.cache.cache.delete") as mock:
+                view.get_vpn.invalidate(view)
+                mock.assert_called_once()
+
+    def test_vpn_checksum_cache_invalidation_handler(self):
+        vpn = self._create_vpn()
+        url = reverse("controller:vpn_checksum", args=[vpn.pk])
+        # Warm up the cache
+        with self.assertNumQueries(1):
+            response = self.client.get(url, {"key": vpn.key})
+        self.assertEqual(response.content.decode(), vpn.checksum)
+
+        # Cache works are expected
+        with self.assertNumQueries(0):
+            response = self.client.get(url, {"key": vpn.key})
+        self.assertEqual(response.content.decode(), vpn.checksum)
+
+        # Change VPN config to trigger invalidation
+        vpn.config["openvpn"][0]["proto"] = "tcp-server"
+        vpn.full_clean()
+        vpn.save()
+
+        del vpn.backend_instance
+        with self.assertNumQueries(1):
+            response = self.client.get(url, {"key": vpn.key})
+        self.assertEqual(response.content.decode(), vpn.checksum)
+
     def test_vpn_download_config(self):
         v = self._create_vpn()
         url = reverse("controller:vpn_download_config", args=[v.pk])

runtests.py

@@ -53,7 +53,7 @@ def run_tests(extra_args, settings_module, test_app):
         "--tag",
         "selenium_tests",
     ] + base_args
-    run_tests(psql_args, "openwisp2.postgresql_settings", test_app)
+    # run_tests(psql_args, "openwisp2.postgresql_settings", test_app)
 
     # Run pytest tests
-    sys.exit(pytest.main([app_dir]))
+    # sys.exit(pytest.main([app_dir]))