[charts] Add way to register and unregister new chart configurations #86

nepython · nepython · commit ebc41fe131be · 2020-07-16T00:58:29.000+05:30
Closes #86
diff --git a/README.rst b/README.rst
@@ -476,6 +476,77 @@ information by performing lookups on the OUI
 
 This feature is enabled by default.
 
+Registering / Unregistering Chart Configuration
+-----------------------------------------------
+
+**OpenWISP Monitoring** provides registering and unregistering chart configuration through utility functions
+``openwisp_monitoring.monitoring.charts.register_chart`` and ``openwisp_monitoring.monitoring.charts.unregister_chart``.
+Using these functions you can register or unregister chart configurations from anywhere in your code.
+
+register_chart
+~~~~~~~~~~~~~~
+
+This function is used to register a new chart configuration from anywhere in your code.
+
++--------------------------+-----------------------------------------------------+
+|      **Parameter**       |                   **Description**                   |
++--------------------------+-----------------------------------------------------+
+|      **chart_name**:     | A ``str`` defining name of the chart configuration. |
++--------------------------+-----------------------------------------------------+
+| **chart_configuration**: | A ``dict`` defining configuration of the chart.     |
++--------------------------+-----------------------------------------------------+
+
+An example usage has been shown below.
+
+.. code-block:: python
+
+    from openwisp_monitoring.monitoring import register_chart
+
+    # Define configuration of your chart
+    chart_config = {
+        'type': 'histogram',
+        'title': 'Histogram',
+        'description': 'Histogram',
+        'top_fields': 2,
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT {fields|SUM|/ 1} FROM {key} "
+                "WHERE time >= '{time}' AND content_type = "
+                "'{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    }
+
+    # Register your custom chart configuration
+    register_chart('chart_name', chart_config)
+
+**Note**: It will raise ``ImproperlyConfigured`` exception if a chart configuration
+is already registered with same name (not to be confused with verbose_name).
+
+unregister_chart
+~~~~~~~~~~~~~~~~
+
+This function is used to unregister a chart configuration from anywhere in your code.
+
++------------------+-----------------------------------------------------+
+|  **Parameter**   |                   **Description**                   |
++------------------+-----------------------------------------------------+
+|  **chart_name**: | A ``str`` defining name of the chart configuration. |
++------------------+-----------------------------------------------------+
+
+An example usage is shown below.
+
+.. code-block:: python
+
+    from openwisp_monitoring.monitoring import unregister_chart
+
+    # Unregister previously registered chart configuration
+    unregister_chart('chart_name')
+
+**Note**: It will raise ``ImproperlyConfigured`` exception if the concerned chart
+configuration is not registered.
+
 Signals
 -------
 
diff --git a/openwisp_monitoring/monitoring/__init__.py b/openwisp_monitoring/monitoring/__init__.py
@@ -1 +1,3 @@
+from .charts import register_chart, unregister_chart  # noqa
+
 default_app_config = 'openwisp_monitoring.monitoring.apps.MonitoringConfig'
diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py
@@ -21,9 +21,9 @@
 
 from ...db import default_chart_query, timeseries_db
 from ..charts import (
+    CHART_CONFIGURATION_CHOICES,
     DEFAULT_COLORS,
     get_chart_configuration,
-    get_chart_configuration_choices,
 )
 from ..exceptions import InvalidChartConfigException, InvalidMetricConfigException
 from ..metrics import get_metric_configuration, get_metric_configuration_choices
@@ -224,12 +224,11 @@ def _notify_users(self, notification_type, alert_settings):
 
 
 class AbstractChart(TimeStampedEditableModel):
-    CHARTS = get_chart_configuration()
     metric = models.ForeignKey(
         get_model_name('monitoring', 'Metric'), on_delete=models.CASCADE
     )
     configuration = models.CharField(
-        max_length=16, null=True, choices=get_chart_configuration_choices()
+        max_length=16, null=True, choices=CHART_CONFIGURATION_CHOICES
     )
     GROUP_MAP = {
         '1d': '10m',
@@ -261,7 +260,7 @@ def _clean_query(self):
     @property
     def config_dict(self):
         try:
-            return self.CHARTS[self.configuration]
+            return get_chart_configuration()[self.configuration]
         except KeyError as e:
             raise InvalidChartConfigException(
                 f'Invalid chart configuration: "{self.configuration}"'
diff --git a/openwisp_monitoring/monitoring/charts.py b/openwisp_monitoring/monitoring/charts.py
@@ -1,3 +1,4 @@
+from django.core.exceptions import ImproperlyConfigured
 from django.utils.translation import gettext_lazy as _
 from openwisp_monitoring.db import chart_query
 
@@ -140,29 +141,75 @@
 }
 
 
+def _validate_chart_configuration(chart_config):
+    assert 'type' in chart_config
+    assert 'title' in chart_config
+    assert 'description' in chart_config
+    assert 'order' in chart_config
+    assert 'query' in chart_config
+    if chart_config['query'] is None:
+        assert 'unit' in chart_config
+    if 'colorscale' in chart_config:
+        assert 'max' in chart_config['colorscale']
+        assert 'min' in chart_config['colorscale']
+        assert 'label' in chart_config['colorscale']
+        assert 'scale' in chart_config['colorscale']
+
+
 def get_chart_configuration():
     charts = deep_merge_dicts(DEFAULT_CHARTS, app_settings.ADDITIONAL_CHARTS)
     # ensure configuration is not broken
     for key, options in charts.items():
-        assert 'type' in options
-        assert 'title' in options
-        assert 'description' in options
-        assert 'order' in options
-        assert 'query' in options
-        if options['query'] is None:
-            assert 'unit' in options
-        if 'colorscale' in options:
-            assert 'max' in options['colorscale']
-            assert 'min' in options['colorscale']
-            assert 'label' in options['colorscale']
-            assert 'scale' in options['colorscale']
+        _validate_chart_configuration(options)
     return charts
 
 
+def register_chart(chart_name, chart_config):
+    """
+    Registers a new chart configuration.
+    """
+    if not isinstance(chart_name, str):
+        raise ImproperlyConfigured('Chart name should be type "str".')
+    if not isinstance(chart_config, dict):
+        raise ImproperlyConfigured('Chart configuration should be type "dict".')
+    if chart_name in DEFAULT_CHARTS:
+        raise ImproperlyConfigured(
+            f'{chart_name} is an already registered Chart Configuration.'
+        )
+
+    _validate_chart_configuration(chart_config)
+    DEFAULT_CHARTS.update({chart_name: chart_config})
+    _register_chart_configuration_choice(chart_name, chart_config)
+
+
+def unregister_chart(chart_name):
+    if not isinstance(chart_name, str):
+        raise ImproperlyConfigured('Chart configuration name should be type "str"')
+    if chart_name not in DEFAULT_CHARTS:
+        raise ImproperlyConfigured(f'No such Chart configuation "{chart_name}"')
+    DEFAULT_CHARTS.pop(chart_name)
+    _unregister_chart_configuration_choice(chart_name)
+
+
+def _register_chart_configuration_choice(chart_name, chart_config):
+    name = chart_config.get('verbose_name', chart_name)
+    CHART_CONFIGURATION_CHOICES.append((chart_name, name))
+
+
+def _unregister_chart_configuration_choice(chart_name):
+    for index, (key, name) in enumerate(CHART_CONFIGURATION_CHOICES):
+        if key == chart_name:
+            CHART_CONFIGURATION_CHOICES.pop(index)
+            return
+
+
 def get_chart_configuration_choices():
     charts = get_chart_configuration()
     choices = []
     for key in sorted(charts.keys()):
         label = charts[key].get('label', charts[key]['title'])
         choices.append((key, label))
     return choices
+
+
+CHART_CONFIGURATION_CHOICES = get_chart_configuration_choices()
diff --git a/openwisp_monitoring/monitoring/migrations/0006_add_configuration.py b/openwisp_monitoring/monitoring/migrations/0006_add_configuration.py
@@ -1,7 +1,7 @@
 # Generated by Django 3.0.4 on 2020-04-28 17:22
 
 from django.db import migrations, models
-from ..charts import get_chart_configuration_choices
+from ..charts import CHART_CONFIGURATION_CHOICES
 
 
 class Migration(migrations.Migration):
@@ -15,7 +15,7 @@ class Migration(migrations.Migration):
             model_name='graph',
             name='configuration',
             field=models.CharField(
-                choices=get_chart_configuration_choices(), max_length=16, null=True
+                choices=CHART_CONFIGURATION_CHOICES, max_length=16, null=True
             ),
         ),
     ]
diff --git a/openwisp_monitoring/monitoring/tests/__init__.py b/openwisp_monitoring/monitoring/tests/__init__.py
@@ -7,13 +7,114 @@
 
 from ...db import timeseries_db
 from ...db.backends import TIMESERIES_DB
+from .. import register_chart, unregister_chart
 
 start_time = now()
 ten_minutes_ago = start_time - timedelta(minutes=10)
 Chart = load_model('monitoring', 'Chart')
 Metric = load_model('monitoring', 'Metric')
 AlertSettings = load_model('monitoring', 'AlertSettings')
 
+# this custom chart configuration is used for automated testing purposes
+charts = {
+    'histogram': {
+        'type': 'histogram',
+        'title': 'Histogram',
+        'description': 'Histogram',
+        'top_fields': 2,
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT {fields|SUM|/ 1} FROM {key} "
+                "WHERE time >= '{time}' AND content_type = "
+                "'{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+    'dummy': {
+        'type': 'line',
+        'title': 'Dummy chart',
+        'description': 'Dummy chart for testing purposes.',
+        'unit': 'candies',
+        'order': 999,
+        'query': None,
+    },
+    'bad_test': {
+        'type': 'line',
+        'title': 'Bugged chart for testing purposes',
+        'description': 'Bugged chart for testing purposes.',
+        'unit': 'bugs',
+        'order': 999,
+        'query': {'influxdb': "BAD"},
+    },
+    'default': {
+        'type': 'line',
+        'title': 'Default query for testing purposes',
+        'description': 'Default query for testing purposes',
+        'unit': 'n.',
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT {field_name} FROM {key} WHERE time >= '{time}' AND "
+                "content_type = '{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+    'multiple_test': {
+        'type': 'line',
+        'title': 'Multiple test',
+        'description': 'For testing purposes',
+        'unit': 'n.',
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT {field_name}, value2 FROM {key} WHERE time >= '{time}' AND "
+                "content_type = '{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+    'mean_test': {
+        'type': 'line',
+        'title': 'Mean test',
+        'description': 'For testing purposes',
+        'unit': 'n.',
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT MEAN({field_name}) AS {field_name} FROM {key} WHERE time >= '{time}' AND "
+                "content_type = '{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+    'sum_test': {
+        'type': 'line',
+        'title': 'Sum test',
+        'description': 'For testing purposes',
+        'unit': 'n.',
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT SUM({field_name}) AS {field_name} FROM {key} WHERE time >= '{time}' AND "
+                "content_type = '{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+    'top_fields_mean': {
+        'type': 'histogram',
+        'title': 'Top fields mean test',
+        'description': 'For testing purposes',
+        'top_fields': 2,
+        'order': 999,
+        'query': {
+            'influxdb': (
+                "SELECT {fields|MEAN} FROM {key} "
+                "WHERE time >= '{time}' AND content_type = "
+                "'{content_type}' AND object_id = '{object_id}'"
+            )
+        },
+    },
+}
+
 
 class TestMonitoringMixin(TestOrganizationMixin):
     ORIGINAL_DB = TIMESERIES_DB['NAME']
@@ -26,10 +127,14 @@ def setUpClass(cls):
         timeseries_db.db_name = cls.TEST_DB
         del timeseries_db.get_db
         timeseries_db.create_database()
+        for key, value in charts.items():
+            register_chart(key, value)
 
     @classmethod
     def tearDownClass(cls):
         timeseries_db.drop_database()
+        for key in charts.keys():
+            unregister_chart(key)
 
     def tearDown(self):
         timeseries_db.delete_metric_data()
diff --git a/openwisp_monitoring/monitoring/tests/test_charts.py b/openwisp_monitoring/monitoring/tests/test_charts.py
diff --git a/tests/openwisp2/sample_monitoring/migrations/0001_initial.py b/tests/openwisp2/sample_monitoring/migrations/0001_initial.py
diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py

Original file line number	Diff line number	Diff line change
`@@ -1 +1,3 @@`
	`1`	`+from .charts import register_chart, unregister_chart # noqa`
	`2`	`+`
`1`	`3`	`default_app_config = 'openwisp_monitoring.monitoring.apps.MonitoringConfig'`