[change] Invalidate checksum cache on save operation

Author: pandafy

openwisp_controller/config/apps.py

@@ -285,14 +285,6 @@ def enable_cache_invalidation(self):
             DeviceChecksumView.invalidate_get_device_cache_on_config_deactivated,
             dispatch_uid="config_deactivated_invalidate_get_device_cache",
         )
-        config_deactivating.connect(
-            DeviceChecksumView.invalidate_checksum_cache,
-            dispatch_uid="config_deactivated_invalidate_get_device_cache",
-        )
-        config_modified.connect(
-            DeviceChecksumView.invalidate_checksum_cache,
-            dispatch_uid="invalidate_checksum_cache",
-        )
         # VPN cache invalidation
         post_save.connect(
             GetVpnView.invalidate_get_vpn_cache,

openwisp_controller/config/base/config.py

@@ -183,7 +183,7 @@ def bulk_invalidate_get_cached_checksum(cls, query_params):
         for instance in cls.objects.only("id").filter(**query_params).iterator():
             has_changed = instance.update_status_if_checksum_changed()
             if has_changed:
-                instance.get_cached_checksum.invalidate(instance)
+                instance.invalidate_checksum_cache()
 
     @classmethod
     def get_template_model(cls):
@@ -573,10 +573,10 @@ def clean(self):
     def save(self, *args, **kwargs):
         created = self._state.adding
         # check if config has been modified (so we can emit signals)
-        if not created:
+        if created:
+            self.checksum_db = self.checksum
+        else:
             self._check_changes()
-        self._invalidate_backend_instance_cache()
-        self.checksum_db = self.checksum
         self._just_created = created
         result = super().save(*args, **kwargs)
         # add default templates if config has just been created
@@ -620,17 +620,12 @@ def _check_changes(self):
         if self.backend != current.backend:
             # storing old backend to send backend change signal after save
             self._old_backend = current.backend
-        self._invalidate_backend_instance_cache()
-        if self.checksum != current.checksum:
-            if self.status != "modified":
-                self.set_status_modified(save=False)
-            else:
-                # config modified signal is always sent
-                # regardless of the current status
-                self._send_config_modified_after_save = True
+        self.update_status_if_checksum_changed(
+            save=False,
+        )
 
     def update_status_if_checksum_changed(
-        self, save=True, send_config_modified_signal=True
+        self, save=True, update_checksum_db=True, send_config_modified_signal=True
     ):
         """
         Updates the instance status if its checksum has changed.
@@ -649,7 +644,11 @@ def update_status_if_checksum_changed(
                     extra_update_fields=["checksum_db"],
                 )
             else:
-                self._update_checksum_db(new_checksum=self.checksum_db)
+                if update_checksum_db:
+                    self._update_checksum_db(new_checksum=self.checksum_db)
+                if send_config_modified_signal:
+                    self._send_config_modified_after_save = True
+            self.invalidate_checksum_cache()
         return checksum_changed
 
     def _has_configuration_checksum_changed(self):
@@ -758,7 +757,7 @@ def set_status_modified(
     ):
         if send_config_modified_signal:
             self._send_config_modified_after_save = True
-        self._set_status("modified", save, extra_update_fields)
+        self._set_status("modified", save, extra_update_fields=extra_update_fields)
 
     def set_status_applied(self, save=True):
         self._set_status("applied", save)
@@ -790,12 +789,14 @@ def deactivate(self):
         # otherwise, the "enforce_required_templates" and "add_default_templates"
         # methods would re-add required/default templates.
         # The "templates_changed" receiver skips post_clear action. Thus,
-        # we need to update the checksum_db field manually.
+        # we need to update the checksum_db field manually and invalidate
+        # the cache.
         self.config = {}
         self.set_status_deactivating(save=False)
         self.templates.clear()
         self._invalidate_backend_instance_cache()
         self.checksum_db = self.checksum
+        self.invalidate_checksum_cache()
         self.save()
         if old_checksum == self.checksum_db:
             # Accelerate deactivation if the configuration remains

openwisp_controller/config/tests/test_config.py

@@ -250,7 +250,7 @@ def test_get_cached_checksum(self):
                 del c.backend_instance
                 self.assertNotEqual(c.checksum, old_checksum)
                 self.assertEqual(c.get_cached_checksum(), c.checksum)
-                mocked_debug.assert_not_called()
+                mocked_debug.assert_called_once()
 
         with self.subTest("test cache invalidation when config templates are changed"):
             with patch.object(base_config_logger, "debug") as mocked_debug:
@@ -260,7 +260,16 @@ def test_get_cached_checksum(self):
                 del c.backend_instance
                 self.assertNotEqual(c.checksum, old_checksum)
                 self.assertEqual(c.get_cached_checksum(), c.checksum)
-                mocked_debug.assert_not_called()
+                mocked_debug.assert_called_once()
+
+        with self.subTest('cache invalidation works when config is deactivated'):
+            with patch.object(base_config_logger, "debug") as mocked_debug:
+                old_checksum = c.checksum
+                c.deactivate()
+                del c.backend_instance
+                self.assertNotEqual(c.checksum, old_checksum)
+                self.assertEqual(c.get_cached_checksum(), c.checksum)
+                mocked_debug.assert_called_once()
 
     def test_backend_import_error(self):
         """
@@ -828,8 +837,15 @@ def test_config_modified_sent(self):
 
     def test_check_changes_query(self):
         config = self._create_config(organization=self._get_org())
-        with self.assertNumQueries(10):
-            config._check_changes()
+        with self.subTest('No changes made to the config object'):
+            with self.assertNumQueries(3):
+                config._check_changes()
+
+        with self.subTest('Changes made to the config object'):
+            config.templates.add(self._create_template())
+            config.config = {"general": {"description": "test"}}
+            with self.assertNumQueries(4):
+                config._check_changes()
 
     def test_config_get_system_context(self):
         config = self._create_config(
@@ -962,7 +978,7 @@ def test_certificate_renew_invalidates_checksum_cache(self):
             vpnclient_cert.renew()
             # An additional call from cache invalidation of
             # DeviceGroupCommonName View
-            self.assertEqual(mocked_delete.call_count, 2)
+            self.assertEqual(mocked_delete.call_count, 3)
             del config.backend_instance
             self.assertNotEqual(config.get_cached_checksum(), old_checksum)
             config.refresh_from_db()

openwisp_controller/config/tests/test_controller.py

@@ -240,7 +240,7 @@ def test_get_cached_checksum(self):
                 del d.config.backend_instance
                 self.assertNotEqual(d.config.checksum, old_checksum)
                 self.assertEqual(d.config.get_cached_checksum(), d.config.checksum)
-                mocked_debug.assert_not_called()
+                mocked_debug.assert_called_once()
 
     def test_device_checksum_requested_signal_is_emitted(self):
         d = self._create_device_config()