From 0ddf2a67ab5c698da6e6702b3ca7c1c6987d8191 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Sat, 15 Feb 2025 00:48:42 +0530 Subject: [PATCH] [fix] Improved query for RADIUS monitoring dashboard charts Previously, the queries performed datetime interpolation at the database level, which prevented the database from efficiently using indexes. This change ensures that filters use UTC datetime objects directly, allowing the database to leverage indexes for improved query performance. Fixed also the target_url for "Today's RADIUS Session" chart. --- .../integrations/monitoring/apps.py | 27 ++++++++---- .../monitoring/tests/test_admin.py | 42 ++++++++++++++++--- .../integrations/monitoring/utils.py | 39 ++++++++++++----- 3 files changed, 83 insertions(+), 25 deletions(-) diff --git a/openwisp_radius/integrations/monitoring/apps.py b/openwisp_radius/integrations/monitoring/apps.py index c1ad0be1..a8a1eb95 100644 --- a/openwisp_radius/integrations/monitoring/apps.py +++ b/openwisp_radius/integrations/monitoring/apps.py @@ -1,9 +1,10 @@ +from datetime import timedelta + from django.apps import AppConfig from django.db import models from django.db.models import Count, Sum from django.db.models.functions import Cast, Round from django.db.models.signals import post_save -from django.utils.timezone import localdate from django.utils.translation import gettext_lazy as _ from openwisp_monitoring.monitoring.configuration import ( _register_chart_configuration_choice, @@ -13,7 +14,11 @@ from openwisp_utils.admin_theme import register_dashboard_chart -from .utils import get_datetime_filter_start_date, get_datetime_filter_stop_date +from .utils import ( + get_datetime_filter_start_datetime, + get_datetime_filter_stop_datetime, + get_utc_datetime_from_local_date, +) class OpenwispRadiusMonitoringConfig(AppConfig): @@ -36,7 +41,10 @@ def register_dashboard_charts(self): 'app_label': 'openwisp_radius', 'model': 'radiusaccounting', 'filter': { - 'start_time__date': localdate, + # This will filter the sessions for today + 'start_time__gte': get_utc_datetime_from_local_date, + 'start_time__lt': lambda: get_utc_datetime_from_local_date() + + timedelta(days=1), }, 'aggregate': { 'open': Count( @@ -57,8 +65,8 @@ def register_dashboard_charts(self): 'closed': 'False', }, 'main_filters': { - 'start_time__gte': get_datetime_filter_start_date, - 'start_time__lt': get_datetime_filter_stop_date, + 'start_time__gte': get_datetime_filter_start_datetime, + 'start_time__lt': get_datetime_filter_stop_datetime, }, 'labels': { 'open': 'Open', @@ -74,7 +82,10 @@ def register_dashboard_charts(self): 'app_label': 'openwisp_radius', 'model': 'radiusaccounting', 'filter': { - 'start_time__date': localdate, + # This will filter the sessions for today + 'start_time__gte': get_utc_datetime_from_local_date, + 'start_time__lt': lambda: get_utc_datetime_from_local_date() + + timedelta(days=1), }, 'aggregate': { 'download_traffic': Round( @@ -94,8 +105,8 @@ def register_dashboard_charts(self): 'upload_traffic': 'Upload traffic (GB)', }, 'main_filters': { - 'start_time__gte': get_datetime_filter_start_date, - 'start_time__lt': get_datetime_filter_stop_date, + 'start_time__gte': get_datetime_filter_start_datetime, + 'start_time__lt': get_datetime_filter_stop_datetime, }, 'filtering': 'False', }, diff --git a/openwisp_radius/integrations/monitoring/tests/test_admin.py b/openwisp_radius/integrations/monitoring/tests/test_admin.py index 1e41dc6a..e85efbee 100644 --- a/openwisp_radius/integrations/monitoring/tests/test_admin.py +++ b/openwisp_radius/integrations/monitoring/tests/test_admin.py @@ -1,16 +1,20 @@ -from django.test import TestCase, tag +from django.test import TestCase, override_settings, tag from django.urls import reverse -from django.utils import timezone +from freezegun import freeze_time from openwisp_radius.tests import _RADACCT, CreateRadiusObjectsMixin -from ..utils import get_datetime_filter_start_date, get_datetime_filter_stop_date +from ..utils import ( + get_datetime_filter_start_datetime, + get_datetime_filter_stop_datetime, +) from .mixins import CreateDeviceMonitoringMixin @tag('radius_monitoring') class TestDeviceAdmin(CreateRadiusObjectsMixin, CreateDeviceMonitoringMixin, TestCase): app_label = 'config' + radius_label = 'openwisp_radius' def setUp(self): admin = self._create_admin() @@ -31,16 +35,26 @@ def test_radius_session_tab(self): '', ) + @freeze_time('2025-02-13 00:00:00+05:30') + @override_settings(TIME_ZONE='Asia/Kolkata') def test_radius_dashboard_chart_data(self): options = _RADACCT.copy() + # Create RADIUS Accounting with UTC time options['unique_id'] = '117' - options['start_time'] = timezone.now().strftime('%Y-%m-%d 00:00:00') + options['start_time'] = '2025-02-12T18:29:00+00:00' options['input_octets'] = '1234567890' + # Create RADIUS Accounting with IST time + self._create_radius_accounting(**options) + options['unique_id'] = '118' + options['start_time'] = '2025-02-13 00:00:00+05:30' + # The dashboard queries RadiusAccounting with localdate, + # therefore, it should not return the RADIUS Accounting created + # with UTC time as it will be on the previous day. self._create_radius_accounting(**options) response = self.client.get(reverse('admin:index')) self.assertEqual(response.status_code, 200) - start_time = get_datetime_filter_start_date() - end_time = get_datetime_filter_stop_date() + start_time = get_datetime_filter_start_datetime() + end_time = get_datetime_filter_stop_datetime() self.assertContains( response, ( @@ -60,3 +74,19 @@ def test_radius_dashboard_chart_data(self): '}' ), ) + + def test_radius_dashboard_chart_filter_url(self): + path = reverse(f'admin:{self.radius_label}_radiusaccounting_changelist') + response = self.client.get( + '{path}?start_time__gte={start_time}&start_time__lt={stop_time}'.format( + path=path, + start_time=get_datetime_filter_start_datetime(), + stop_time=get_datetime_filter_stop_datetime(), + ) + ) + self.assertEqual(response.status_code, 200) + self.assertNotEqual(response.request['QUERY_STRING'], 'e=1') + self.assertContains( + response, + '
', + ) diff --git a/openwisp_radius/integrations/monitoring/utils.py b/openwisp_radius/integrations/monitoring/utils.py index dbbb3375..26a599f6 100644 --- a/openwisp_radius/integrations/monitoring/utils.py +++ b/openwisp_radius/integrations/monitoring/utils.py @@ -1,27 +1,44 @@ import hashlib from datetime import datetime, timedelta +from urllib.parse import urlencode from django.utils import timezone local_timezone = timezone.get_current_timezone() -def _get_formatted_datetime_string(date_time): - return ( - str(datetime.combine(date_time, datetime.min.time()).astimezone(local_timezone)) - .replace(' ', '+') - .replace(':', '%3A') +def _get_urlencoded_datetime(date_time): + """ + URL encodes the date_time string and returns the value. + """ + return urlencode({'': date_time}).split('=')[1] + + +def get_today_start_datetime(): + """ + Returns the beginning of the current day in local timezone. + """ + + return timezone.make_aware( + datetime.combine(timezone.localdate(), datetime.min.time()) ) -def get_datetime_filter_start_date(): - start_date = timezone.localdate() - return _get_formatted_datetime_string(start_date) +def get_utc_datetime_from_local_date(): + """ + Returns UTC time for the beginning of the current day in local timezone. + """ + return get_today_start_datetime().astimezone(timezone.utc) + + +def get_datetime_filter_start_datetime(): + today = get_today_start_datetime() + return _get_urlencoded_datetime(today) -def get_datetime_filter_stop_date(): - stop_date = timezone.localdate() + timedelta(days=1) - return _get_formatted_datetime_string(stop_date) +def get_datetime_filter_stop_datetime(): + tomorrow = get_today_start_datetime() + timedelta(days=1) + return _get_urlencoded_datetime(tomorrow) def sha1_hash(input_string):