[chores] Requested changes

openwisp_monitoring/check/classes/base.py

@@ -2,6 +2,8 @@
 from django.core.exceptions import ValidationError
 from swapper import load_model
 
+from .. import settings as app_settings
+
 Check = load_model('check', 'Check')
 Metric = load_model('monitoring', 'Metric')
 Device = load_model('config', 'Device')
@@ -49,3 +51,13 @@ def _get_or_create_metric(self, configuration=None):
         )
         metric, created = Metric._get_or_create(**options)
         return metric, created
+
+    def _get_ip(self):
+        """
+        Figures out ip to use or fails raising OperationalError
+        """
+        device = self.related_object
+        ip = device.management_ip
+        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
+            ip = device.last_ip
+        return ip

openwisp_monitoring/check/classes/ping.py

@@ -6,7 +6,6 @@
 from swapper import load_model
 
 from ... import settings as monitoring_settings
-from .. import settings as app_settings
 from ..exceptions import OperationalError
 from .base import BaseCheck
 
@@ -123,16 +122,6 @@ def _get_param(self, param):
         """
         return self.params.get(param, self.schema['properties'][param]['default'])
 
-    def _get_ip(self):
-        """
-        Figures out ip to use or fails raising OperationalError
-        """
-        device = self.related_object
-        ip = device.management_ip
-        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
-            ip = device.last_ip
-        return ip
-
     def _command(self, command):
         """
         Executes command (easier to mock)

openwisp_monitoring/check/classes/snmp_devicemonitoring.py

@@ -1,12 +1,9 @@
-from copy import deepcopy
-
 from django.utils.functional import cached_property
 from netengine.backends.snmp.openwrt import OpenWRT
 from swapper import load_model
 
 from openwisp_monitoring.device.api.views import MetricChartsMixin
 
-from .. import settings as app_settings
 from .base import BaseCheck
 
 Chart = load_model('monitoring', 'Chart')
@@ -19,7 +16,7 @@
 class SnmpDeviceMonitoring(BaseCheck, MetricChartsMixin):
     def check(self, store=True):
         result = self.netengine_instance.to_dict()
-        self._init_previous_data()
+        self._init_previous_data(data=getattr(self.related_object, 'data', {}))
         self.related_object.data = result
         if store:
             self.store_result(result)
@@ -35,38 +32,11 @@ def store_result(self, data):
     @cached_property
     def netengine_instance(self):
         ip = self._get_ip()
-        return OpenWRT(host=ip, **self.credential_params)
+        return OpenWRT(host=ip, **self._get_credential_params())
 
-    @cached_property
-    def credential_params(self):
-        params = {}
+    def _get_credential_params(self):
         cred = Credentials.objects.filter(
             deviceconnection__device_id=self.related_object,
             connector='openwisp_controller.connection.connectors.snmp.Snmp',
         ).last()
-        if cred is not None:
-            params.update(cred.params)
-        return params
-
-    def _get_ip(self):
-        """
-        Figures out ip to use or fails raising OperationalError
-        """
-        device = self.related_object
-        ip = device.management_ip
-        if not ip and not app_settings.MANAGEMENT_IP_ONLY:
-            ip = device.last_ip
-        return ip
-
-    def _init_previous_data(self):
-        """
-        makes NetJSON interfaces of previous
-        snapshots more easy to access
-        """
-        data = getattr(self.related_object, 'data', {})
-        if data:
-            data = deepcopy(data)
-            data['interfaces_dict'] = {}
-        for interface in data.get('interfaces', []):
-            data['interfaces_dict'][interface['name']] = interface
-        self._previous_data = data
+        return getattr(cred, 'params', {})

openwisp_monitoring/check/tests/test_ping.py

@@ -227,7 +227,7 @@ def test_auto_chart_disabled(self, *args):
         device = self._create_device(organization=self._create_org())
         device.last_ip = '127.0.0.1'
         device.save()
-        check = Check.objects.first()
+        check = Check.objects.filter(check=self._PING).first()
         self.assertEqual(Chart.objects.count(), 0)
         check.perform_check()
         self.assertEqual(Chart.objects.count(), 0)

openwisp_monitoring/check/tests/test_snmp.py

@@ -0,0 +1,48 @@
+from unittest.mock import patch
+
+from django.test import TransactionTestCase
+from swapper import load_model
+
+from openwisp_controller.connection.settings import UPDATE_STRATEGIES
+from openwisp_controller.connection.tests.base import CreateConnectionsMixin
+
+from ...device.tests import TestDeviceMonitoringMixin
+from .. import settings
+from .utils import MockOpenWRT
+
+Check = load_model('check', 'Check')
+Credentials = load_model('connection', 'Credentials')
+
+
+class TestSnmp(CreateConnectionsMixin, TestDeviceMonitoringMixin, TransactionTestCase):
+    _SNMPDEVICEMONITORING = settings.CHECK_CLASSES[2][0]
+
+    def test_snmp_perform_check(self):
+        device = self._create_device()
+        device.management_ip = '192.168.1.1'
+        check = Check(check=self._SNMPDEVICEMONITORING, content_object=device)
+        with patch(
+            'openwisp_monitoring.check.classes.snmp_devicemonitoring.OpenWRT'
+        ) as p:
+            p.side_effect = MockOpenWRT
+            check.perform_check(store=False)
+            p.assert_called_once_with(host='192.168.1.1')
+
+    def test_snmp_perform_check_with_credentials(self):
+        device = self._create_device()
+        device.management_ip = '192.168.1.1'
+        check = Check(check=self._SNMPDEVICEMONITORING, content_object=device)
+        params = {'community': 'public', 'agent': 'my-agent', 'port': 161}
+        cred = self._create_credentials(
+            params=params,
+            connector='openwisp_controller.connection.connectors.snmp.Snmp',
+        )
+        self._create_device_connection(
+            credentials=cred, device=device, update_strategy=UPDATE_STRATEGIES[1][0]
+        )
+        with patch(
+            'openwisp_monitoring.check.classes.snmp_devicemonitoring.OpenWRT'
+        ) as p:
+            p.side_effect = MockOpenWRT
+            check.perform_check(store=False)
+            p.assert_called_once_with(host='192.168.1.1', **params)