Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions openwisp_monitoring/check/base/models.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
from collections import OrderedDict

from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models
from django.utils.functional import cached_property
from django.utils.module_loading import import_string
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField

from openwisp_utils.base import TimeStampedEditableModel

Expand Down Expand Up @@ -37,13 +35,12 @@ class AbstractCheck(TimeStampedEditableModel):
db_index=True,
max_length=128,
)
params = JSONField(
params = models.JSONField(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

_("parameters"),
default=dict,
blank=True,
help_text=_("parameters needed to perform the check"),
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
encoder=DjangoJSONEncoder,
)

class Meta:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
# Generated by Django 3.1.2 on 2020-11-18 12:55

import collections
import uuid

import django.db.models.deletion
import django.utils.timezone
import jsonfield.fields
import model_utils.fields
import swapper
from django.db import migrations, models
Expand Down Expand Up @@ -67,12 +65,10 @@ class Migration(migrations.Migration):
),
(
"params",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"indent": 4},
help_text="parameters needed to perform the check",
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="parameters",
),
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

import django.core.serializers.json
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("check", "0011_check_active_object_checks_idx"),
]

operations = [
migrations.AlterField(
model_name="check",
name="params",
field=models.JSONField(
blank=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
help_text="parameters needed to perform the check",
verbose_name="parameters",
),
),
]
12 changes: 11 additions & 1 deletion openwisp_monitoring/check/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import timedelta
from datetime import date, timedelta
from unittest.mock import patch

from django.core.exceptions import ValidationError
Expand Down Expand Up @@ -31,6 +31,16 @@ def test_check_str(self):
c = Check(name="Test check")
self.assertEqual(str(c), c.name)

def test_params_support_django_json_encoder_values(self):
# date requires DjangoJSONEncoder, default JSONEncoder cannot serialize it.
check = Check.objects.create(
name="Test check",
check_type=self._PING,
params={"date": date(2026, 5, 14)},
)
check.refresh_from_db()
self.assertEqual(check.params["date"], "2026-05-14")

def test_check_no_content_type(self):
c = Check(name="Ping class check", check_type=self._PING)
m = c.check_instance._get_metric()
Expand Down
12 changes: 5 additions & 7 deletions openwisp_monitoring/monitoring/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.core.validators import MaxValueValidator
from django.db import IntegrityError, models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from openwisp_notifications.signals import notify
from pytz import timezone as tz
from pytz import utc
Expand Down Expand Up @@ -91,20 +91,18 @@ class AbstractMetric(TimeStampedEditableModel):
)
object_id = models.CharField(max_length=36, db_index=True, blank=True, null=True)
content_object = GenericForeignKey("content_type", "object_id")
main_tags = JSONField(
main_tags = models.JSONField(

@Eeshu-Yadav Eeshu-Yadav Apr 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add encoder=DjangoJSONEncoder to all JSONField instances (this one, extra_tags, and params in check/base/models.py).
This is needed to prevent serialization errors with lazy translation objects. See openwisp/openwisp-notifications#438 and the reference commit: openwisp/openwisp-notifications@bad5232

@nemesifier requested this same change on both the controller PR (openwisp/openwisp-controller#1214) and the radius PR (openwisp/openwisp-radius#619). It should be applied consistently across all openwisp repos.

from django.core.serializers.json import DjangoJSONEncoder

main_tags = models.JSONField(
    _("main tags"),
    default=dict,
    blank=True,
    db_index=True,
    encoder=DjangoJSONEncoder,
)

The AlterField migrations (0012, 0013, and the test migrations) will also need encoder=django.core.serializers.json.DjangoJSONEncoder in their field definitions.

_("main tags"),
default=dict,
blank=True,
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
db_index=True,
encoder=DjangoJSONEncoder,
)
extra_tags = JSONField(
extra_tags = models.JSONField(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

_("extra tags"),
default=dict,
blank=True,
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
encoder=DjangoJSONEncoder,
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# NULL means the health has yet to be assessed
is_healthy = models.BooleanField(default=None, null=True, blank=True, db_index=True)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
# Generated by Django 3.2.12 on 2022-04-23 17:50

import collections

import jsonfield.fields
from django.db import migrations
from django.db import migrations, models


class Migration(migrations.Migration):
Expand All @@ -16,23 +13,19 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name="metric",
name="main_tags",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
db_index=True,
default=dict,
dump_kwargs={"indent": 4},
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="main tags",
),
),
migrations.AddField(
model_name="metric",
name="extra_tags",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
default=dict,
dump_kwargs={"indent": 4},
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="extra tags",
),
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

import django.core.serializers.json
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("monitoring", "0012_migrate_signal_metrics"),
]

operations = [
migrations.AlterField(
model_name="metric",
name="main_tags",
field=models.JSONField(
blank=True,
db_index=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
verbose_name="main tags",
),
),
migrations.AlterField(
model_name="metric",
name="extra_tags",
field=models.JSONField(
blank=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
verbose_name="extra tags",
),
),
]
13 changes: 12 additions & 1 deletion openwisp_monitoring/monitoring/tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import timedelta
from datetime import date, timedelta
from unittest.mock import patch

from django.contrib.auth import get_user_model
Expand Down Expand Up @@ -34,6 +34,17 @@ def test_general_metric_str(self):
m = Metric(name="Test metric")
self.assertEqual(str(m), m.name)

def test_tags_support_django_json_encoder_values(self):
# date requires DjangoJSONEncoder, default JSONEncoder cannot serialize it.
metric = Metric.objects.create(
name="Test metric",
main_tags={"date": date(2026, 5, 14)},
extra_tags={"date": date(2026, 5, 14)},
)
metric.refresh_from_db()
self.assertEqual(metric.main_tags["date"], "2026-05-14")
self.assertEqual(metric.extra_tags["date"], "2026-05-14")

def test_chart_str(self):
c = self._create_chart()
self.assertEqual(str(c), c.label)
Expand Down
6 changes: 1 addition & 5 deletions tests/openwisp2/sample_check/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
# Generated by Django 3.0.5 on 2020-05-27 03:04

import collections
import uuid

import django.db.models.deletion
import django.utils.timezone
import jsonfield.fields
import model_utils.fields
from django.db import migrations, models

Expand Down Expand Up @@ -74,12 +72,10 @@ class Migration(migrations.Migration):
),
(
"params",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"indent": 4},
help_text="parameters needed to perform the check",
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="parameters",
),
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

import django.core.serializers.json
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("sample_check", "0003_add_check_inline_permissions"),
]

operations = [
migrations.AlterField(
model_name="check",
name="params",
field=models.JSONField(
blank=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
help_text="parameters needed to perform the check",
verbose_name="parameters",
),
),
]
10 changes: 2 additions & 8 deletions tests/openwisp2/sample_monitoring/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# Generated by Django 3.0.5 on 2020-05-27 03:04

import collections
import uuid

import django
import django.core.validators
import django.db.models.deletion
import django.utils.timezone
import jsonfield
import model_utils.fields
import swapper
from django.db import migrations, models
Expand Down Expand Up @@ -104,22 +102,18 @@ class Migration(migrations.Migration):
),
(
"main_tags",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
db_index=True,
default=dict,
dump_kwargs={"indent": 4},
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="main tags",
),
),
(
"extra_tags",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"indent": 4},
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="extra tags",
),
),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

import django.core.serializers.json
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("sample_monitoring", "0004_alter_metric_field_name"),
]

operations = [
migrations.AlterField(
model_name="metric",
name="main_tags",
field=models.JSONField(
blank=True,
db_index=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
verbose_name="main tags",
),
Comment on lines +17 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

db_index=True on JSONField creates a B-tree index, not GIN

On PostgreSQL, db_index=True on a jsonb column produces a B-tree index on the binary JSON representation. If main_tags is queried using key containment or key existence operators (@>, ?), a GIN index would be significantly more effective. This mirrors the original jsonfield behavior and is not a regression, but worth revisiting if monitoring queries filter by individual tag keys.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tests/openwisp2/sample_monitoring/migrations/0005_replace_jsonfield_with_django_builtin.py`
around lines 16 - 21, The migration sets db_index=True on the JSONField
`main_tags` (models.JSONField), which creates a B-tree index; change this to use
a GIN index instead by removing db_index=True from the JSONField and adding an
Index entry using django.contrib.postgres.indexes.GinIndex for the `main_tags`
column (e.g. add a migrations.AddIndex that references
GinIndex(fields=['main_tags']) in the same migration so queries using
containment/existence operators use a proper GIN index).

),
migrations.AlterField(
model_name="metric",
name="extra_tags",
field=models.JSONField(
blank=True,
default=dict,
encoder=django.core.serializers.json.DjangoJSONEncoder,
verbose_name="extra tags",
),
),
]
Loading