[feature] Implemented caching of device checksum view openwisp#396

- implemented caching of device checksum view
  - cache the query that retrieves the device
  - cache the generated checksum
- implemented cache invalidation and update

Closes openwisp#396

openwisp_controller/config/apps.py

@@ -1,7 +1,7 @@
 from django.apps import AppConfig
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
-from django.db.models.signals import m2m_changed, post_delete
+from django.db.models.signals import m2m_changed, post_delete, post_save
 from django.utils.translation import ugettext_lazy as _
 from openwisp_notifications.types import (
     register_notification_type,
@@ -10,6 +10,7 @@
 from swapper import get_model_name, load_model
 
 from . import settings as app_settings
+from .signals import config_modified
 
 # ensure Device.hardware_id field is not flagged as unique
 # (because it's flagged as unique_together with organization)
@@ -27,17 +28,21 @@ def ready(self, *args, **kwargs):
         self.add_default_menu_items()
         self.register_notification_types()
         self.add_ignore_notification_widget()
+        self.enable_cache_invalidation()
 
     def __setmodels__(self):
+        self.device_model = load_model('config', 'Device')
         self.config_model = load_model('config', 'Config')
         self.vpnclient_model = load_model('config', 'VpnClient')
 
     def connect_signals(self):
         """
         * handlers for creating notifications
         * m2m validation before templates are added/removed to a config
+        * enforcement of required templates
         * automatic vpn client management on m2m_changed
         * automatic vpn client removal
+        * cache invalidation
         """
         from . import handlers  # noqa
 
@@ -56,6 +61,7 @@ def connect_signals(self):
             sender=self.config_model.templates.through,
             dispatch_uid='config.manage_vpn_clients',
         )
+        # the order of the following connect() call must be maintained
         m2m_changed.connect(
             self.config_model.enforce_required_templates,
             sender=self.config_model.templates.through,
@@ -138,3 +144,20 @@ def add_ignore_notification_widget(self):
                 'OPENWISP_NOTIFICATIONS_IGNORE_ENABLED_ADMIN',
                 obj_notification_widget,
             )
+
+    def enable_cache_invalidation(self):
+        """
+        Triggers the cache invalidation for the
+        device config checksum (view and model method)
+        """
+        from .controller.views import DeviceChecksumView
+
+        post_save.connect(
+            DeviceChecksumView.invalidate_get_device_cache,
+            sender=self.device_model,
+            dispatch_uid='invalidate_get_device_cache',
+        )
+        config_modified.connect(
+            DeviceChecksumView.invalidate_checksum_cache,
+            dispatch_uid='invalidate_checksum_cache',
+        )

openwisp_controller/config/base/config.py

@@ -1,5 +1,7 @@
 import collections
+import logging
 
+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 ugettext_lazy as _
@@ -14,6 +16,8 @@
 from ..utils import get_default_templates_queryset
 from .base import BaseConfig
 
+logger = logging.getLogger(__name__)
+
 
 class TemplatesThrough(object):
     """
@@ -24,6 +28,13 @@ 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):
     """
     Abstract model implementing the
@@ -70,6 +81,8 @@ class AbstractConfig(BaseConfig):
         dump_kwargs={'indent': 4},
     )
 
+    _CHECKSUM_CACHE_TIMEOUT = 60 * 60 * 24 * 30  # 10 days
+
     class Meta:
         abstract = True
         verbose_name = _('configuration')
@@ -114,6 +127,18 @@ 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 get_template_model(cls):
         return cls.templates.rel.model

openwisp_controller/config/controller/views.py

@@ -1,6 +1,10 @@
 import json
+import logging
+import uuid
 from ipaddress import ip_address
 
+from cache_memoize import cache_memoize
+from django.core.cache import cache
 from django.core.exceptions import FieldDoesNotExist, ValidationError
 from django.db import transaction
 from django.db.models import Q
@@ -23,19 +27,36 @@
 )
 
 Device = load_model('config', 'Device')
+Config = load_model('config', 'Config')
 OrganizationConfigSettings = load_model('config', 'OrganizationConfigSettings')
 Vpn = load_model('config', 'Vpn')
 
+logger = logging.getLogger(__name__)
 
-class BaseConfigView(SingleObjectMixin, View):
+
+class GetDeviceView(SingleObjectMixin, View):
     """
     Base view that implements a ``get_object`` method
-    Subclassed by all views dealing with existing objects
+    Subclassed by all device views which deal with existing objects
     """
 
+    model = Device
+
     def get_object(self, *args, **kwargs):
-        kwargs['config__isnull'] = False
-        return get_object_or_404(self.model, *args, **kwargs)
+        kwargs.update({'organization__is_active': True, 'config__isnull': False})
+        defer = (
+            'notes',
+            'organization__name',
+            'organization__description',
+            'organization__email',
+            'organization__url',
+            'organization__created',
+            'organization__modified',
+        )
+        queryset = self.model.objects.select_related('organization', 'config').defer(
+            *defer
+        )
+        return get_object_or_404(queryset, *args, **kwargs)
 
 
 class CsrfExtemptMixin(object):
@@ -52,62 +73,111 @@ class UpdateLastIpMixin(object):
     def update_last_ip(self, device, request):
         result = update_last_ip(device, request)
         if result:
-            # avoid that any other device in the
-            # same org stays with the same management_ip
-            # This can happen when management interfaces are using DHCP
-            # and they get a new address which was previously used by another
-            # device that may now be offline, without this fix, we will end up
-            # with two devices having the same management_ip, which will
-            # cause OpenWISP to be confused
-            self.model.objects.filter(
-                organization=device.organization, management_ip=device.management_ip
-            ).exclude(pk=device.pk).update(management_ip='')
-            # in the case of last_ip, we take a different approach,
-            # because it may be a public IP. If it's a public IP we will
-            # allow it to be duplicated
-            if ip_address(device.last_ip).is_private:
-                Device.objects.filter(
-                    organization=device.organization, last_ip=device.last_ip
-                ).exclude(pk=device.pk).update(last_ip='')
+            self._remove_duplicated_management_ip(device)
+            self._remove_duplicated_last_ip(device)
         return result
 
+    def _remove_duplicated_management_ip(self, device):
+        # avoid that any other device in the
+        # same org stays with the same management_ip
+        # This can happen when management interfaces are using DHCP
+        # and they get a new address which was previously used by another
+        # device that may now be offline, without this fix, we will end up
+        # with two devices having the same management_ip, which will
+        # cause OpenWISP to be confused
+        if not device.management_ip:
+            return
+        queryset = self.model.objects.filter(
+            organization_id=device.organization_id, management_ip=device.management_ip
+        ).exclude(pk=device.pk)
+        for dupe in queryset.only('pk', 'key', 'management_ip'):
+            dupe.management_ip = ''
+            dupe.save(update_fields=['management_ip'])
+
+    def _remove_duplicated_last_ip(self, device):
+        # in the case of last_ip, we take a different approach,
+        # because it may be a public IP. If it's a public IP we will
+        # allow it to be duplicated
+        if not device.last_ip or not ip_address(device.last_ip).is_private:
+            return
+        queryset = Device.objects.filter(
+            organization_id=device.organization_id, last_ip=device.last_ip
+        ).exclude(pk=device.pk)
+        for dupe in queryset.only('pk', 'key', 'last_ip', 'management_ip'):
+            dupe.last_ip = ''
+            dupe.save(update_fields=['last_ip'])
+
 
-class ActiveOrgMixin(object):
+def get_device_args_rewrite(view):
     """
-    adds check to organization.is_active to ``get_object`` method
+    Use only the PK parameter for calculating the cache key
     """
+    # avoid ambiguity between hex format and dashed string
+    # return view.kwargs['pk']
+    pk = view.kwargs['pk']
+    try:
+        pk = uuid.UUID(pk)
+    except ValueError:
+        return pk
+    return pk.hex
 
-    def get_object(self, *args, **kwargs):
-        kwargs['organization__is_active'] = True
-        return super().get_object(*args, **kwargs)
 
-
-class DeviceChecksumView(ActiveOrgMixin, UpdateLastIpMixin, BaseConfigView):
+class DeviceChecksumView(UpdateLastIpMixin, GetDeviceView):
     """
     returns device's configuration checksum
     """
 
-    model = Device
-
-    def get(self, request, *args, **kwargs):
-        device = self.get_object(*args, **kwargs)
+    def get(self, request, pk):
+        device = self.get_device()
         bad_request = forbid_unallowed(request, 'GET', 'key', device.key)
         if bad_request:
             return bad_request
-        self.update_last_ip(device, request)
+        updated = self.update_last_ip(device, request)
+        # updates cache if ip addresses changed
+        if updated:
+            self.update_device_cache(device)
         checksum_requested.send(
             sender=device.__class__, instance=device, request=request
         )
-        return ControllerResponse(device.config.checksum, content_type='text/plain')
+        return ControllerResponse(
+            device.config.get_cached_checksum(), content_type='text/plain'
+        )
 
+    @cache_memoize(
+        timeout=Config._CHECKSUM_CACHE_TIMEOUT, args_rewrite=get_device_args_rewrite
+    )
+    def get_device(self):
+        pk = self.kwargs['pk']
+        logger.debug(f'retrieving device ID {pk} from DB')
+        return self.get_object(pk=pk)
 
-class DeviceDownloadConfigView(ActiveOrgMixin, BaseConfigView):
+    def update_device_cache(self, device):
+        cache.set(self.get_device.get_cache_key(self), device)
+
+    @classmethod
+    def invalidate_get_device_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_device.invalidate(view)
+        logger.debug(f'invalidated view cache for device ID {pk}')
+
+    @classmethod
+    def invalidate_checksum_cache(cls, instance, device, **kwargs):
+        """
+        Called from signal receiver which performs cache invalidation
+        """
+        instance.get_cached_checksum.invalidate(instance)
+
+
+class DeviceDownloadConfigView(GetDeviceView):
     """
     returns configuration archive as attachment
     """
 
-    model = Device
-
     def get(self, request, *args, **kwargs):
         device = self.get_object(*args, **kwargs)
         bad_request = forbid_unallowed(request, 'GET', 'key', device.key)
@@ -119,13 +189,11 @@ def get(self, request, *args, **kwargs):
         return send_device_config(device.config, request)
 
 
-class DeviceUpdateInfoView(ActiveOrgMixin, CsrfExtemptMixin, BaseConfigView):
+class DeviceUpdateInfoView(CsrfExtemptMixin, GetDeviceView):
     """
     updates general information about the device
     """
 
-    model = Device
-
     UPDATABLE_FIELDS = ['os', 'model', 'system']
 
     def post(self, request, *args, **kwargs):
@@ -153,13 +221,11 @@ def post(self, request, *args, **kwargs):
         return ControllerResponse('update-info: success', content_type='text/plain')
 
 
-class DeviceReportStatusView(ActiveOrgMixin, CsrfExtemptMixin, BaseConfigView):
+class DeviceReportStatusView(CsrfExtemptMixin, GetDeviceView):
     """
     updates status of config objects
     """
 
-    model = Device
-
     def post(self, request, *args, **kwargs):
         device = self.get_object(*args, **kwargs)
         config = device.config
@@ -359,13 +425,26 @@ def post(self, request, *args, **kwargs):
         )
 
 
-class VpnChecksumView(BaseConfigView):
+class GetVpnView(SingleObjectMixin, View):
     """
-    returns vpn's configuration checksum
+    Base view that implements a ``get_object`` method
+    Subclassed by all vpn views which deal with existing objects
     """
 
     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)
+        )
+        return get_object_or_404(queryset, *args, **kwargs)
+
+
+class VpnChecksumView(GetVpnView):
+    """
+    returns vpn's configuration checksum
+    """
+
     def get(self, request, *args, **kwargs):
         vpn = self.get_object(*args, **kwargs)
         bad_request = forbid_unallowed(request, 'GET', 'key', vpn.key)
@@ -375,13 +454,11 @@ def get(self, request, *args, **kwargs):
         return ControllerResponse(vpn.checksum, content_type='text/plain')
 
 
-class VpnDownloadConfigView(BaseConfigView):
+class VpnDownloadConfigView(GetVpnView):
     """
     returns configuration archive as attachment
     """
 
-    model = Vpn
-
     def get(self, request, *args, **kwargs):
         vpn = self.get_object(*args, **kwargs)
         bad_request = forbid_unallowed(request, 'GET', 'key', vpn.key)