[tests] Improved tests and rebase

- Completed rebase.
- Improved test_snmp to make sure it works on current version of openwisp_monitoring.
- Updated settings of check acc to new openwisp_controller snmp credentials.
- Formatted code and fixed all QA warnings.

openwisp_monitoring/check/classes/ping.py

@@ -8,6 +8,7 @@
 from openwisp_utils.utils import deep_merge_dicts
 
 from ... import settings as monitoring_settings
+from .. import settings as app_settings
 from ..exceptions import OperationalError
 from .base import BaseCheck
 

openwisp_monitoring/check/classes/snmp_devicemonitoring.py

@@ -18,7 +18,7 @@
 class Snmp(BaseCheck, MetricChartsMixin):
     def check(self, store=True):
         result = self.netengine_instance.to_dict()
-        self._init_previous_data(data=getattr(self.related_object, 'data', {}))
+        self._init_previous_data()
         self.related_object.data = result
         if store:
             self.store_result(result)
@@ -32,7 +32,7 @@ def store_result(self, data):
         device_data = DeviceData.objects.get(pk=pk)
         device_data.data = data
         device_data.save_data()
-        self._write(pk, data)
+        self._write(pk)
 
     @cached_property
     def netengine_instance(self):
@@ -43,13 +43,14 @@ def netengine_instance(self):
     @cached_property
     def credential_instance(self):
         return Credentials.objects.filter(
-            deviceconnection__device_id=self.related_object, connector__endswith='Snmp',
+            deviceconnection__device_id=self.related_object,
+            connector__endswith='OpenWRTSnmp',
         ).last()
 
     def _get_connnector(self):
         connectors = {
-            'openwisp_controller.connection.connectors.snmp.Snmp': OpenWRT,
-            'openwisp_controller.connection.connectors.airos.snmp.Snmp': AirOS,
+            'openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp': OpenWRT,
+            'openwisp_controller.connection.connectors.airos.snmp.AirOsSnmp': AirOS,
         }
         try:
             return connectors.get(self.credential_instance.connector, OpenWRT)

openwisp_monitoring/check/settings.py

@@ -5,10 +5,14 @@
     (
         ('openwisp_monitoring.check.classes.Ping', 'Ping'),
         ('openwisp_monitoring.check.classes.ConfigApplied', 'Configuration Applied'),
-        ('openwisp_monitoring.check.classes.Snmp', 'SNMP Device Monitoring',),
+        (
+            'openwisp_monitoring.check.classes.Snmp',
+            'SNMP Device Monitoring',
+        ),
     ),
 )
 AUTO_PING = get_settings_value('AUTO_PING', True)
 AUTO_CONFIG_CHECK = get_settings_value('AUTO_DEVICE_CONFIG_CHECK', True)
 AUTO_SNMP = get_settings_value('AUTO_SNMP', False)
 MANAGEMENT_IP_ONLY = get_settings_value('MANAGEMENT_IP_ONLY', True)
+PING_CHECK_CONFIG = get_settings_value('PING_CHECK_CONFIG', {})

openwisp_monitoring/check/tasks.py

@@ -112,7 +112,9 @@ def auto_create_snmp_devicemonitoring(
     Check = check_model or get_check_model()
     devicemonitoring_path = 'openwisp_monitoring.check.classes.Snmp'
     has_check = Check.objects.filter(
-        object_id=object_id, content_type__model='device', check=devicemonitoring_path
+        object_id=object_id,
+        content_type__model='device',
+        check_type=devicemonitoring_path,
     ).exists()
     # create new check only if necessary
     if has_check:
@@ -121,7 +123,7 @@ def auto_create_snmp_devicemonitoring(
     ct = content_type_model.objects.get(app_label=app_label, model=model)
     check = Check(
         name='SNMP Device Monitoring',
-        check=devicemonitoring_path,
+        check_type=devicemonitoring_path,
         content_type=ct,
         object_id=object_id,
     )

openwisp_monitoring/check/tests/test_models.py

@@ -54,7 +54,10 @@ def test_check_class(self):
             )
             self.assertEqual(c.check_class, ConfigApplied)
         with self.subTest('Test DeviceMonitoring check Class'):
-            c = Check(name='Snmp class check', check=self._SNMP_DEVICEMONITORING,)
+            c = Check(
+                name='Snmp class check',
+                check_type=self._SNMP_DEVICEMONITORING,
+            )
             self.assertEqual(c.check_class, Snmp)
 
     def test_base_check_class(self):
@@ -92,7 +95,7 @@ def test_check_instance(self):
         with self.subTest('Test DeviceMonitoring check instance'):
             c = Check(
                 name='DeviceMonitoring class check',
-                check=self._SNMP_DEVICEMONITORING,
+                check_type=self._SNMP_DEVICEMONITORING,
                 content_object=obj,
                 params={},
             )
@@ -254,7 +257,7 @@ def test_device_unreachable_no_config_check(self):
         d = self.device_model.objects.first()
         d.monitoring.update_status('critical')
         self.assertEqual(Check.objects.count(), 2)
-        c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first()
+        c2 = Check.objects.filter(check_type=self._CONFIG_APPLIED).first()
         c2.perform_check()
         self.assertEqual(Metric.objects.count(), 0)
         self.assertIsNone(c2.perform_check())
@@ -265,7 +268,7 @@ def test_device_unknown_no_config_check(self):
         d = self.device_model.objects.first()
         d.monitoring.update_status('unknown')
         self.assertEqual(Check.objects.count(), 2)
-        c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first()
+        c2 = Check.objects.filter(check_type=self._CONFIG_APPLIED).first()
         c2.perform_check()
         self.assertEqual(Metric.objects.count(), 0)
         self.assertEqual(Notification.objects.count(), 0)

openwisp_monitoring/check/tests/test_ping.py

@@ -265,7 +265,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.filter(check=self._PING).first()
+        check = Check.objects.filter(check_type=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

@@ -20,7 +20,11 @@ class TestSnmp(CreateConnectionsMixin, TestDeviceMonitoringMixin, TransactionTes
     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)
+        check = Check(
+            name='SNMP check',
+            check_type=self._SNMPDEVICEMONITORING,
+            content_object=device,
+        )
         with patch(
             'openwisp_monitoring.check.classes.snmp_devicemonitoring.OpenWRT'
         ) as p:
@@ -31,11 +35,15 @@ def test_snmp_perform_check(self):
     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)
+        check = Check(
+            name='SNMP check',
+            check_type=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',
+            connector='openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp',
         )
         self._create_device_connection(
             credentials=cred, device=device, update_strategy=UPDATE_STRATEGIES[1][0]

openwisp_monitoring/device/api/views.py

@@ -18,6 +18,7 @@
 from pytz.exceptions import UnknownTimeZoneError
 from rest_framework import serializers, status
 from rest_framework.generics import GenericAPIView
+from rest_framework.permissions import BasePermission
 from rest_framework.response import Response
 from swapper import load_model
 
@@ -36,6 +37,7 @@
 
 logger = logging.getLogger(__name__)
 Chart = load_model('monitoring', 'Chart')
+Check = load_model('check', 'Check')
 Metric = load_model('monitoring', 'Metric')
 AlertSettings = load_model('monitoring', 'AlertSettings')
 Device = load_model('config', 'Device')
@@ -44,22 +46,28 @@
 Location = load_model('geo', 'Location')
 
 
-class DevicePermission(BasePermission):
+class DevicePermission(BasePermission):  # noqa
     def has_object_permission(self, request, view, obj):
         return request.query_params.get('key') == obj.key
 
 
 class MetricChartsMixin:
-    def _write(self, pk, data=None):
+    def _write(self, pk, time=None, current=False):
         """
         write metrics to database
         """
-        if data is None:
-            # saves raw device data
+        # If object not of type check ie. It is DeviceData
+        if not hasattr(self, 'check_instance'):
             self.instance.save_data()
             data = self.instance.data
+            device_extra_tags = self._get_extra_tags(self.instance)
+        # Get data attribute from DeviceData object
+        else:
+            devicedata_instance = DeviceData.objects.get(pk=pk)
+            data = getattr(devicedata_instance, 'data', {})
+            device_extra_tags = self._get_extra_tags(devicedata_instance)
+
         ct = ContentType.objects.get_for_model(Device)
-        device_extra_tags = self._get_extra_tags(self.instance)
         for interface in data.get('interfaces', []):
             ifname = interface['name']
             extra_tags = Metric._sort_dict(device_extra_tags)
@@ -358,13 +366,19 @@ def _create_access_tech_chart(self, metric):
         chart.full_clean()
         chart.save()
 
-    def _init_previous_data(self, data=None):
+    def _init_previous_data(self):
         """
         makes NetJSON interfaces of previous
         snapshots more easy to access
         """
-        if data is None:
+
+        # If object not of type check
+        if not hasattr(self, 'check_instance'):
             data = self.instance.data or {}
+        # Get data attribute from Device object
+        else:
+            data = getattr(self.related_object, 'data', {})
+
         if data:
             data = deepcopy(data)
             data['interfaces_dict'] = {}
@@ -375,7 +389,7 @@ def _init_previous_data(self, data=None):
 
 class DeviceMetricView(GenericAPIView, MetricChartsMixin):
     model = DeviceData
-    queryset = DeviceData.objects.all()
+    queryset = DeviceData.objects.select_related('devicelocation').all()
     serializer_class = serializers.Serializer
     permission_classes = [DevicePermission]
     schema = schema
@@ -421,13 +435,21 @@ def _get_charts_data(self, charts, time, timezone):
             # prepare chart dict
             try:
                 chart_dict = chart.read(time=time, x_axys=x_axys, timezone=timezone)
+                if not chart_dict['traces']:
+                    continue
                 chart_dict['description'] = chart.description
-                chart_dict['title'] = chart.title.format(metric=chart.metric)
+                chart_dict['title'] = chart.title.format(
+                    metric=chart.metric, **chart.metric.tags
+                )
                 chart_dict['type'] = chart.type
                 chart_dict['unit'] = chart.unit
                 chart_dict['summary_labels'] = chart.summary_labels
                 chart_dict['colors'] = chart.colors
                 chart_dict['colorscale'] = chart.colorscale
+                for attr in ['fill', 'xaxis', 'yaxis']:
+                    value = getattr(chart, attr)
+                    if value:
+                        chart_dict[attr] = value
             except InvalidChartConfigException:
                 logger.exception(f'Skipped chart for metric {chart.metric}')
                 continue
@@ -492,14 +514,28 @@ def post(self, request, pk):
         except ValidationError as e:
             logger.info(e.message)
             return Response(e.message, status=status.HTTP_400_BAD_REQUEST)
+        time_obj = request.query_params.get(
+            'time', now().utcnow().strftime('%d-%m-%Y_%H:%M:%S.%f')
+        )
+        current = request.query_params.get('current', False)
+        try:
+            time = datetime.strptime(time_obj, '%d-%m-%Y_%H:%M:%S.%f').replace(
+                tzinfo=UTC
+            )
+        except ValueError:
+            return Response({'detail': _('Incorrect time format')}, status=400)
         try:
             # write data
-            self._write(self.instance.pk)
+            self._write(self.instance.pk, time=time)
         except ValidationError as e:
             logger.info(e.message_dict)
             return Response(e.message_dict, status=status.HTTP_400_BAD_REQUEST)
         device_metrics_received.send(
-            sender=self.model, instance=self.instance, request=request
+            sender=self.model,
+            instance=self.instance,
+            request=request,
+            time=time,
+            current=current,
         )
         return Response(None)