Skip to content
Open
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
4 changes: 4 additions & 0 deletions openwisp_controller/config/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
ObjectDoesNotExist,
ValidationError,
)
from django.db import models
from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonResponse
from django.http.response import HttpResponseForbidden
from django.shortcuts import get_object_or_404
Expand Down Expand Up @@ -217,6 +218,9 @@ def _get_preview_instance(self, request):
key = "{relation}_id".format(relation=key)
# pass non-empty string or None
kwargs[key] = value or None
# parse JSON strings for JSONField fields
elif isinstance(field, models.JSONField) and isinstance(value, str):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

try removing this after adding encoder=DjangoJSONEncoder

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@nemesifier I tested removing the JSON parsing code but it breaks the preview functionality. The DjangoJSONEncoder solves lazy translation serialization but doesn't handle form data parsing. When the admin preview receives form data, JSONField values come as JSON strings that need to be parsed to Python objects before model validation.
Without the parsing code, tests fail with "Unexpected configuration format" because the model expects a dict but receives a string. The encoder and parsing code solve different problems:

  • DjangoJSONEncoder: Handles lazy translations during model save
  • Admin parsing: Converts form strings to Python objects for preview

Both are needed for full functionality. Should I keep the current implementation?

Screenshot from 2026-02-04 23-13-36

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.

Makes sense, the explanation is clear. One small thing on line 224 though — if json.loads fails, the raw invalid string gets passed through silently which causes a confusing error downstream. Since Django will handle it cleanly as a ValidationError anyway, you can just drop the try/except entirely:

elif isinstance(field, models.JSONField) and isinstance(value, str):
    kwargs[key] = json.loads(value) if value else None

kwargs[key] = json.loads(value) if value else None
# put regular field values in kwargs dict
else:
kwargs[key] = value
Expand Down
7 changes: 3 additions & 4 deletions openwisp_controller/config/base/base.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import collections
import hashlib
import json
import logging
from copy import deepcopy

from cache_memoize import cache_memoize
from django.core.exceptions import ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models
from django.db.models import JSONField
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 netjsonconfig.exceptions import ValidationError as SchemaError

from openwisp_utils.base import TimeStampedEditableModel
Expand Down Expand Up @@ -126,8 +126,7 @@ class BaseConfig(BaseModel):
_("configuration"),
default=dict,
help_text=_("configuration in NetJSON DeviceConfiguration format"),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's add: encoder=DjangoJSONEncoder, see openwisp/openwisp-notifications#438, applies to all JSONField instances

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

updated that

dump_kwargs={"indent": 4},
encoder=DjangoJSONEncoder,
)

__template__ = False
Expand Down
6 changes: 3 additions & 3 deletions openwisp_controller/config/base/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
from cache_memoize import cache_memoize
from django.core.cache import cache
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models, transaction
from django.db.models import JSONField
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from model_utils import Choices
from model_utils.fields import StatusField
from netjsonconfig import OpenWrt
Expand Down Expand Up @@ -90,8 +91,7 @@ class AbstractConfig(ChecksumCacheMixin, BaseConfig):
'" target="_blank">'
"configuration variables</a>"
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
dump_kwargs={"indent": 4},
encoder=DjangoJSONEncoder,
)
checksum_db = models.CharField(
_("configuration checksum"),
Expand Down
10 changes: 4 additions & 6 deletions openwisp_controller/config/base/device_group.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import collections
from copy import deepcopy

import jsonschema
from django.core.exceptions import ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models
from django.db.models import JSONField
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from jsonschema.exceptions import ValidationError as SchemaError
from swapper import get_model_name, load_model

Expand Down Expand Up @@ -39,24 +39,22 @@ class AbstractDeviceGroup(OrgMixin, TimeStampedEditableModel):
meta_data = JSONField(
blank=True,
default=dict,
load_kwargs={"object_pairs_hook": collections.OrderedDict},
dump_kwargs={"indent": 4},
help_text=_(
"Store custom metadata related to this group. This field is intended "
"for arbitrary data that does not affect device configuration and can "
"be retrieved via the REST API for integrations or external tools."
),
verbose_name=_("Metadata"),
encoder=DjangoJSONEncoder,
)
context = JSONField(
blank=True,
default=dict,
load_kwargs={"object_pairs_hook": collections.OrderedDict},
dump_kwargs={"indent": 4},
help_text=_(
"Define configuration variables available to all devices in this group"
),
verbose_name=_("Configuration Variables"),
encoder=DjangoJSONEncoder,
)

def __str__(self):
Expand Down
7 changes: 3 additions & 4 deletions openwisp_controller/config/base/multitenancy.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import collections
from copy import deepcopy

import swapper
from django.core.exceptions import ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models
from django.db.models import JSONField
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField

from openwisp_utils.base import KeyField, UUIDModel
from openwisp_utils.fields import FallbackBooleanChoiceField
Expand Down Expand Up @@ -42,13 +42,12 @@ class AbstractOrganizationConfigSettings(UUIDModel):
context = JSONField(
blank=True,
default=dict,
load_kwargs={"object_pairs_hook": collections.OrderedDict},
dump_kwargs={"indent": 4},
help_text=_(
"Define reusable configuration variables available "
"to all devices in this organization"
),
verbose_name=_("Configuration Variables"),
encoder=DjangoJSONEncoder,
)

class Meta:
Expand Down
6 changes: 3 additions & 3 deletions openwisp_controller/config/base/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
from copy import copy

from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.db import models, transaction
from django.db.models import JSONField
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from netjsonconfig.exceptions import ValidationError as NetjsonconfigValidationError
from swapper import get_model_name, load_model
from taggit.managers import TaggableManager
Expand Down Expand Up @@ -101,8 +102,7 @@ class AbstractTemplate(ShareableOrgMixinUniqueName, BaseConfig):
"These values are used during validation and when a variable is "
"not provided by the device, group, or organization."
),
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
encoder=DjangoJSONEncoder,
)
__template__ = True

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.10.5 on 2017-05-03 17:54

import collections
import re
import uuid

import django.core.validators
import django.utils.timezone
import jsonfield.fields
import model_utils.fields
from django.conf import settings
from django.db import migrations, models
Expand Down Expand Up @@ -58,12 +56,10 @@ class Migration(migrations.Migration):
),
(
"config",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text="configuration in NetJSON DeviceConfiguration format",
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="configuration",
),
),
Expand Down Expand Up @@ -250,12 +246,10 @@ class Migration(migrations.Migration):
),
(
"config",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text="configuration in NetJSON DeviceConfiguration format",
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="configuration",
),
),
Expand Down Expand Up @@ -347,11 +341,9 @@ class Migration(migrations.Migration):
("name", models.CharField(max_length=64, unique=True)),
(
"config",
jsonfield.fields.JSONField(
models.JSONField(
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text="configuration in NetJSON DeviceConfiguration format",
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="configuration",
),
),
Expand Down
8 changes: 2 additions & 6 deletions openwisp_controller/config/migrations/0018_config_context.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Generated by Django 2.1.4 on 2019-01-09 01:52

import collections

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


class Migration(migrations.Migration):
Expand All @@ -13,15 +11,13 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name="config",
name="context",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
dump_kwargs={"indent": 4},
help_text=(
'Additional <a href="http://netjsonconfig.openwisp.org'
'/en/stable/general/basics.html#context" target="_blank">context '
"(configuration variables)</a> in JSON format"
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
null=True,
),
)
Expand Down
8 changes: 2 additions & 6 deletions openwisp_controller/config/migrations/0023_update_context.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Generated by Django 3.0.3 on 2020-02-26 19:58

import collections

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


class Migration(migrations.Migration):
Expand All @@ -13,17 +11,15 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name="config",
name="context",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text=(
"allows overriding "
'<a href="https://openwisp.io/docs/stable/controller/user/variables.html' # noqa: E501
'" target="_blank">'
"configuration variables</a>"
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
),
)
]
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Generated by Django 3.0.3 on 2020-06-29 23:54

import collections

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


class Migration(migrations.Migration):
Expand All @@ -13,16 +11,14 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name="template",
name="default_values",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text=(
"Define default values for the variables used in this template. "
"These values are used during validation and when a variable is "
"not provided by the device, group, or organization."
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="Default Values",
),
)
Expand Down
6 changes: 1 addition & 5 deletions openwisp_controller/config/migrations/0036_device_group.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
# Generated by Django 3.1.12 on 2021-06-14 17:51

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 @@ -56,11 +54,9 @@ class Migration(migrations.Migration):
),
(
"meta_data",
jsonfield.fields.JSONField(
models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
load_kwargs={"object_pairs_hook": collections.OrderedDict},
help_text=(
"Store custom metadata related to this group. "
"This field is intended for arbitrary data that "
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Generated by Django 3.2.19 on 2023-06-27 14:45

import collections

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


class Migration(migrations.Migration):
Expand All @@ -15,15 +13,13 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name="devicegroup",
name="context",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
default=dict,
dump_kwargs={"ensure_ascii": False, "indent": 4},
help_text=(
"Define configuration variables available "
"to all devices in this group"
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="Configuration Variables",
),
),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Generated by Django 3.2.20 on 2023-07-22 10:49

import collections

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


class Migration(migrations.Migration):
Expand All @@ -15,15 +13,13 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name="organizationconfigsettings",
name="context",
field=jsonfield.fields.JSONField(
field=models.JSONField(
blank=True,
default=dict,
dump_kwargs={"indent": 4},
help_text=(
"Define reusable configuration variables available "
"to all devices in this organization"
),
load_kwargs={"object_pairs_hook": collections.OrderedDict},
verbose_name="Configuration Variables",
),
),
Expand Down
Loading
Loading