[feature] Cache VPN server checksum #1064

Closes #1064

Author: pandafy

openwisp_controller/config/base/base.py

@@ -1,8 +1,10 @@
 import collections
 import hashlib
 import json
+import logging
 from copy import deepcopy
 
+from cache_memoize import cache_memoize
 from django.core.exceptions import ValidationError
 from django.db import models
 from django.utils.functional import cached_property
@@ -15,6 +17,53 @@
 
 from .. import settings as app_settings
 
+logger = logging.getLogger(__name__)
+
+
+def get_cached_checksum_args_rewrite(instance):
+    """
+    Use only the PK parameter for calculating the cache key
+    """
+    return instance.pk.hex
+
+
+class ChecksumCacheMixin:
+    """
+    Mixin that provides checksum caching functionality
+    """
+
+    _CHECKSUM_CACHE_TIMEOUT = 60 * 60 * 24 * 30  # 30 days
+
+    @cache_memoize(
+        timeout=_CHECKSUM_CACHE_TIMEOUT,
+        args_rewrite=get_cached_checksum_args_rewrite
+    )
+    def get_cached_checksum(self):
+        """
+        Handles caching,
+        timeout=None means value is cached indefinitely
+        (invalidation handled on post_save/post_delete signal)
+        """
+        logger.debug(f"calculating checksum for {self.__class__.__name__} ID {self.pk}")
+        return self.checksum
+
+    @classmethod
+    def bulk_invalidate_get_cached_checksum(cls, query_params):
+        """
+        Bulk invalidate checksum cache for multiple instances
+        """
+        for instance in cls.objects.only("id").filter(**query_params).iterator():
+            instance.get_cached_checksum.invalidate(instance)
+
+    def invalidate_checksum_cache(self):
+        """
+        Invalidate the checksum cache for this instance
+        """
+        self.get_cached_checksum.invalidate(self)
+        logger.debug(
+            f"invalidated checksum cache for {self.__class__.__name__} ID {self.pk}"
+        )
+
 
 class BaseModel(TimeStampedEditableModel):
     """

openwisp_controller/config/base/config.py

@@ -3,7 +3,6 @@
 import re
 from collections import defaultdict
 
-from cache_memoize import cache_memoize
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
 from django.db import models, transaction
 from django.utils.translation import gettext_lazy as _
@@ -24,7 +23,7 @@
 )
 from ..sortedm2m.fields import SortedManyToManyField
 from ..utils import get_default_templates_queryset
-from .base import BaseConfig
+from .base import BaseConfig, ChecksumCacheMixin
 
 logger = logging.getLogger(__name__)
 
@@ -38,14 +37,7 @@ def __str__(self):
         return _("Relationship with {0}").format(self.template.name)
 
 
-def get_cached_checksum_args_rewrite(config):
-    """
-    Use only the PK parameter for calculating the cache key
-    """
-    return config.pk.hex
-
-
-class AbstractConfig(BaseConfig):
+class AbstractConfig(ChecksumCacheMixin, BaseConfig):
     """
     Abstract model implementing the
     NetJSON DeviceConfiguration object
@@ -150,23 +142,6 @@ def key(self):
         """
         return self.device.key
 
-    @cache_memoize(
-        timeout=_CHECKSUM_CACHE_TIMEOUT, args_rewrite=get_cached_checksum_args_rewrite
-    )
-    def get_cached_checksum(self):
-        """
-        Handles caching,
-        timeout=None means value is cached indefinitely
-        (invalidation handled on post_save/post_delete signal)
-        """
-        logger.debug(f"calculating checksum for config ID {self.pk}")
-        return self.checksum
-
-    @classmethod
-    def bulk_invalidate_get_cached_checksum(cls, query_params):
-        for config in cls.objects.only("id").filter(**query_params).iterator():
-            config.get_cached_checksum.invalidate(config)
-
     @classmethod
     def get_template_model(cls):
         return cls.templates.rel.model

openwisp_controller/config/base/vpn.py

@@ -33,7 +33,7 @@
     trigger_zerotier_server_update,
     trigger_zerotier_server_update_member,
 )
-from .base import BaseConfig
+from .base import BaseConfig, ChecksumCacheMixin
 
 logger = logging.getLogger(__name__)
 
@@ -43,7 +43,7 @@ def _peer_cache_key(vpn):
     return str(vpn.pk)
 
 
-class AbstractVpn(ShareableOrgMixinUniqueName, BaseConfig):
+class AbstractVpn(ChecksumCacheMixin, ShareableOrgMixinUniqueName, BaseConfig):
     """
     Abstract VPN model
     """
@@ -281,6 +281,7 @@ def save(self, *args, **kwargs):
         if create_dh:
             transaction.on_commit(lambda: create_vpn_dh.delay(self.id))
         if not created and self._send_vpn_modified_after_save:
+            self.invalidate_checksum_cache()
             self._send_vpn_modified_signal()
             self._send_vpn_modified_after_save = False
         # For ZeroTier VPN server, if the
@@ -308,10 +309,9 @@ 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):
-                continue
-            self._send_vpn_modified_after_save = True
-            break
+            if getattr(self, attr) != getattr(current, attr):
+                self._send_vpn_modified_after_save = True
+                break
 
     def _send_vpn_modified_signal(self):
         vpn_server_modified.send(sender=self.__class__, instance=self)

openwisp_controller/config/controller/views.py

@@ -480,7 +480,7 @@ def get(self, request, *args, **kwargs):
         if bad_request:
             return bad_request
         checksum_requested.send(sender=vpn.__class__, instance=vpn, request=request)
-        return ControllerResponse(vpn.checksum, content_type="text/plain")
+        return ControllerResponse(vpn.get_cached_checksum(), content_type="text/plain")
 
 
 class VpnDownloadConfigView(GetVpnView):

openwisp_controller/config/tests/test_config.py

@@ -11,7 +11,7 @@
 from openwisp_utils.tests import catch_signal
 
 from .. import settings as app_settings
-from ..base.config import logger as config_model_logger
+from ..base.base import logger as base_config_logger
 from ..signals import config_backend_changed, config_modified, config_status_changed
 from .utils import CreateConfigTemplateMixin, CreateDeviceGroupMixin, TestVpnX509Mixin
 
@@ -229,20 +229,20 @@ def test_get_cached_checksum(self):
                 mocked_get.assert_called_once()
 
         with self.subTest("ensure fresh checksum is calculated when cache is clear"):
-            with patch.object(config_model_logger, "debug") as mocked_debug:
+            with patch.object(base_config_logger, "debug") as mocked_debug:
                 c.get_cached_checksum.invalidate(c)
                 self.assertEqual(len(c.get_cached_checksum()), 32)
                 mocked_debug.assert_called_once()
 
         with self.subTest(
             "ensure fresh checksum is NOT calculated when cache is present"
         ):
-            with patch.object(config_model_logger, "debug") as mocked_debug:
+            with patch.object(base_config_logger, "debug") as mocked_debug:
                 self.assertEqual(len(c.get_cached_checksum()), 32)
                 mocked_debug.assert_not_called()
 
         with self.subTest("ensure cache invalidation works"):
-            with patch.object(config_model_logger, "debug") as mocked_debug:
+            with patch.object(base_config_logger, "debug") as mocked_debug:
                 old_checksum = c.checksum
                 c.config["general"]["timezone"] = "Europe/Rome"
                 c.full_clean()
@@ -253,7 +253,7 @@ def test_get_cached_checksum(self):
                 mocked_debug.assert_called_once()
 
         with self.subTest("test cache invalidation when config templates are changed"):
-            with patch.object(config_model_logger, "debug") as mocked_debug:
+            with patch.object(base_config_logger, "debug") as mocked_debug:
                 old_checksum = c.checksum
                 template = self._create_template()
                 c.templates.add(template)

openwisp_controller/config/tests/test_controller.py

@@ -11,7 +11,7 @@
 from openwisp_utils.tests import capture_any_output, catch_signal
 
 from .. import settings as app_settings
-from ..base.config import logger as config_model_logger
+from ..base.base import logger as base_config_logger
 from ..controller.views import DeviceChecksumView
 from ..controller.views import logger as controller_views_logger
 from ..signals import (
@@ -186,7 +186,7 @@ def test_get_cached_checksum(self):
                     controller_views_logger, "debug"
                 ) as mocked_view_debug:
                     with patch.object(
-                        config_model_logger, "debug"
+                        base_config_logger, "debug"
                     ) as mocked_config_debug:
                         self.client.get(
                             url, {"key": d.key, "management_ip": "10.0.0.2"}
@@ -200,7 +200,7 @@ def test_get_cached_checksum(self):
                     controller_views_logger, "debug"
                 ) as mocked_view_debug:
                     with patch.object(
-                        config_model_logger, "debug"
+                        base_config_logger, "debug"
                     ) as mocked_config_debug:
                         self.client.get(
                             url, {"key": d.key, "management_ip": "10.0.0.3"}
@@ -223,7 +223,7 @@ def test_get_cached_checksum(self):
                     controller_views_logger, "debug"
                 ) as mocked_view_debug:
                     with patch.object(
-                        config_model_logger, "debug"
+                        base_config_logger, "debug"
                     ) as mocked_config_debug:
                         self.client.get(
                             url, {"key": d.key, "management_ip": "10.0.0.3"}
@@ -233,7 +233,7 @@ def test_get_cached_checksum(self):
 
         with self.subTest("ensure cache invalidation works"):
             old_checksum = d.config.checksum
-            with patch.object(config_model_logger, "debug") as mocked_debug:
+            with patch.object(base_config_logger, "debug") as mocked_debug:
                 d.config.config["general"]["timezone"] = "Europe/Rome"
                 d.config.full_clean()
                 d.config.save()