[change] Refactored WhoIsService, tasks, test cases

Moved all tasks to service file to keep one source.
Refactored the way whois_info is fetcehd for a device.
Modified the order of whois checks to keep
expensive checks at the bottom.
As in views like `DeviceChecksumView` whole device
instance is being cached, now org settings will be
fetched from db rather than through device to
maintain consistency. This has led to increase in
queries in some test cases.

Signed-off-by: DragnEmperor <[email protected]>

Author: DragnEmperor

openwisp_controller/config/base/device.py

@@ -1,3 +1,4 @@
+from functools import cached_property
 from hashlib import md5
 
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
@@ -18,7 +19,6 @@
     management_ip_changed,
 )
 from ..validators import device_name_validator, mac_address_validator
-from ..whois.services import WhoIsService
 from .base import BaseModel
 
 
@@ -381,8 +381,7 @@ def _check_last_ip(self):
         if self._initial_last_ip == models.DEFERRED:
             return
 
-        service = WhoIsService(self)
-        service.trigger_whois_lookup()
+        self.whois_service.trigger_whois_lookup()
 
         self._initial_last_ip = self.last_ip
 
@@ -424,13 +423,15 @@ def status(self):
         """
         return self._get_config_attr("get_status_display")
 
-    @property
-    def whois_info(self):
+    @cached_property
+    def whois_service(self):
         """
-        Used as a shortcut to get WhoIsInfo
+        Used as a shortcut to get WhoIsService instance
+        for the device.
         """
-        service = WhoIsService(self)
-        return service.get_whois_info()
+        from ..whois.service import WhoIsService
+
+        return WhoIsService(self)
 
     def get_default_templates(self):
         """

openwisp_controller/config/controller/views.py

@@ -55,9 +55,7 @@ def get_object(self, *args, **kwargs):
             "organization__modified",
         )
         queryset = (
-            self.model.objects.select_related(
-                "organization", "config", "organization__config_settings"
-            )
+            self.model.objects.select_related("organization", "config")
             .defer(*defer)
             .exclude(config__status="deactivated")
         )

openwisp_controller/config/tests/test_controller.py

@@ -181,7 +181,7 @@ def test_get_cached_checksum(self):
         url = reverse("controller:device_checksum", args=[d.pk])
 
         with self.subTest("first request does not return value from cache"):
-            with self.assertNumQueries(3):
+            with self.assertNumQueries(4):
                 with patch.object(
                     controller_views_logger, "debug"
                 ) as mocked_view_debug:
@@ -1314,12 +1314,12 @@ def test_ip_fields_not_duplicated(self):
         c2 = self._create_config(device=d2)
         org2 = self._create_org(name="org2", shared_secret="123456")
         c3 = self._create_config(organization=org2)
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(7):
             self.client.get(
                 reverse("controller:device_checksum", args=[c3.device.pk]),
                 {"key": c3.device.key, "management_ip": "192.168.1.99"},
             )
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(7):
             self.client.get(
                 reverse("controller:device_checksum", args=[c1.device.pk]),
                 {"key": c1.device.key, "management_ip": "192.168.1.99"},
@@ -1332,7 +1332,7 @@ def test_ip_fields_not_duplicated(self):
             )
         # triggers more queries because devices with conflicting addresses
         # need to be updated, luckily it does not happen often
-        with self.assertNumQueries(12):
+        with self.assertNumQueries(9):
             self.client.get(
                 reverse("controller:device_checksum", args=[c2.device.pk]),
                 {"key": c2.device.key, "management_ip": "192.168.1.99"},
@@ -1361,14 +1361,14 @@ def test_organization_shares_management_ip_address_space(self):
         org1_config = self._create_config(organization=org1)
         org2 = self._create_org(name="org2", shared_secret="org2")
         org2_config = self._create_config(organization=org2)
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(7):
             self.client.get(
                 reverse("controller:device_checksum", args=[org1_config.device_id]),
                 {"key": org1_config.device.key, "management_ip": "192.168.1.99"},
             )
         # Device from another organization sends conflicting management IP
         # Extra queries due to conflict resolution
-        with self.assertNumQueries(12):
+        with self.assertNumQueries(9):
             self.client.get(
                 reverse("controller:device_checksum", args=[org2_config.device_id]),
                 {"key": org2_config.device.key, "management_ip": "192.168.1.99"},

openwisp_controller/config/utils.py

@@ -7,8 +7,6 @@
 from django.urls import path, re_path
 from openwisp_notifications.utils import _get_object_link
 
-from .whois.services import WhoIsService
-
 logger = logging.getLogger(__name__)
 
 
@@ -76,9 +74,8 @@ def update_last_ip(device, request):
         device.last_ip = ip
         update_fields.append("last_ip")
     # for cases of devices who do not have whois record
-    elif not getattr(device, "whois_info", None):
-        service = WhoIsService(device)
-        service.trigger_whois_lookup()
+    elif not device.whois_service.get_whois_info():
+        device.whois_service.trigger_whois_lookup()
     if device.management_ip != management_ip:
         device.management_ip = management_ip
         update_fields.append("management_ip")

openwisp_controller/config/whois/__init__.py

@@ -1,2 +0,0 @@
-# This file is created to allow test runners to discover the tests
-# defined in this directory.

openwisp_controller/config/whois/handlers.py

@@ -1,10 +1,12 @@
 from django.db import transaction
 
-from .services import WhoIsService
+from .service import WhoIsService
 
 
 # Remove the related WhoIs record for that ip from db and cache
 # If other active devices are linked to it, then new lookup will
 # be triggered for them.
 def device_whois_info_delete_handler(instance, **kwargs):
-    transaction.on_commit(lambda: WhoIsService.delete_whois_record(instance.last_ip))
+    transaction.on_commit(
+        lambda: WhoIsService.delete_whois_record.delay(instance.last_ip)
+    )