[fix] Add regression tests and formatting fixes for VpnClient post_delete openwisp#1221

Added regression tests to verify that VpnClient.post_delete signal fires
correctly during VPN template removal and device deactivation. Tests
assert that the VPN peer cache is invalidated via _invalidate_peer_cache
and that certificates are properly revoked for auto_cert VpnClient
instances.

Applied Black and isort formatting fixes to config.py and serializers.py
where per-instance deletion with select_related('vpn', 'cert', 'ip')
replaced bulk QuerySet.delete() within transaction.atomic() blocks to
avoid N+1 queries.

Closes openwisp#1221

Author: mn-ram

openwisp_controller/config/api/serializers.py

@@ -202,11 +202,11 @@ def _update_config(self, device, config_data):
                 with transaction.atomic():
                     vpn_list = config.templates.filter(type="vpn").values_list("vpn")
                     if vpn_list:
-                        for vpnclient in config.vpnclient_set.select_related(
-                            'vpn', 'cert', 'ip'
-                        ).exclude(
-                            vpn__in=vpn_list
-                        ).iterator():
+                        for vpnclient in (
+                            config.vpnclient_set.select_related("vpn", "cert", "ip")
+                            .exclude(vpn__in=vpn_list)
+                            .iterator()
+                        ):
                             vpnclient.delete()
                     config.templates.set(config_templates, clear=True)
             config.save()

openwisp_controller/config/base/config.py

@@ -340,7 +340,7 @@ def manage_vpn_clients(cls, action, instance, pk_set, **kwargs):
                 # delete all vpn clients and return.
                 with transaction.atomic():
                     for vpnclient in instance.vpnclient_set.select_related(
-                        'vpn', 'cert', 'ip'
+                        "vpn", "cert", "ip"
                     ).iterator():
                         vpnclient.delete()
             return
@@ -375,11 +375,13 @@ def manage_vpn_clients(cls, action, instance, pk_set, **kwargs):
             # ones, have been fully added. At that point, we can identify and
             # delete VpnClient objects not linked to the final template set.
             with transaction.atomic():
-                for vpnclient in instance.vpnclient_set.select_related(
-                    'vpn', 'cert', 'ip'
-                ).exclude(
-                    template_id__in=instance.templates.values_list("id", flat=True)
-                ).iterator():
+                for vpnclient in (
+                    instance.vpnclient_set.select_related("vpn", "cert", "ip")
+                    .exclude(
+                        template_id__in=instance.templates.values_list("id", flat=True)
+                    )
+                    .iterator()
+                ):
                     vpnclient.delete()
 
         if action == "post_add":

openwisp_controller/config/tests/test_vpn.py

@@ -286,6 +286,45 @@ def _assert_vpn_client_cert(cert, vpn_client, cert_ct, vpn_client_ct):
             vpnclient.save()
             _assert_vpn_client_cert(cert, vpnclient, 1, 0)
 
+    def test_vpn_client_post_delete_on_template_removal(self):
+        """Regression test for #1221: VpnClient.post_delete must fire
+        when a VPN template is removed so that peer cache is invalidated
+        and certificates are properly revoked."""
+        org = self._get_org()
+        vpn = self._create_vpn()
+        t = self._create_template(name="vpn-test", type="vpn", vpn=vpn, auto_cert=True)
+        c = self._create_config(organization=org)
+        c.templates.add(t)
+        vpnclient = c.vpnclient_set.first()
+        self.assertIsNotNone(vpnclient)
+        cert_pk = vpnclient.cert.pk
+        with mock.patch.object(Vpn, "_invalidate_peer_cache") as mock_invalidate:
+            c.templates.remove(t)
+            mock_invalidate.assert_called()
+        self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
+        # Certificate should be revoked (auto_cert=True)
+        self.assertTrue(Cert.objects.get(pk=cert_pk).revoked)
+
+    def test_vpn_client_post_delete_on_device_deactivation(self):
+        """Regression test for #1221: VpnClient.post_delete must fire
+        when a device is deactivated so that peer cache is invalidated
+        and certificates are properly revoked."""
+        org = self._get_org()
+        vpn = self._create_vpn()
+        t = self._create_template(name="vpn-test", type="vpn", vpn=vpn, auto_cert=True)
+        d = self._create_device(organization=org)
+        c = self._create_config(device=d)
+        c.templates.add(t)
+        vpnclient = c.vpnclient_set.first()
+        self.assertIsNotNone(vpnclient)
+        cert_pk = vpnclient.cert.pk
+        with mock.patch.object(Vpn, "_invalidate_peer_cache") as mock_invalidate:
+            d.deactivate()
+            mock_invalidate.assert_called()
+        self.assertFalse(VpnClient.objects.filter(pk=vpnclient.pk).exists())
+        # Certificate should be revoked (auto_cert=True)
+        self.assertTrue(Cert.objects.get(pk=cert_pk).revoked)
+
     def test_vpn_client_get_common_name(self):
         vpn = self._create_vpn()
         d = self._create_device()