Skip to content

Commit 6bdb628

Browse files
authored
[change] Replaced third-party JSONField with Django built-in JSONField
- Replaced the third party ``jsonfield.JSONField`` with Django's built in ``models.JSONField`` on the ``Node`` and ``Link`` models, which on PostgreSQL also changes the underlying column type from ``text`` to ``jsonb`` (`#253 <https://github.com/openwisp/openwisp-network-topology/issues/253>`_). - Admin search and internal ``__contains`` lookups on the JSON columns now cast the value to text so they keep working on every supported database backend
1 parent b7842a3 commit 6bdb628

9 files changed

Lines changed: 208 additions & 46 deletions

File tree

openwisp_network_topology/admin.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
from django import forms
33
from django.contrib import admin, messages
44
from django.contrib.admin import ModelAdmin
5-
from django.db.models import Q
5+
from django.db.models import Q, TextField
6+
from django.db.models.functions import Cast
67
from django.template.response import TemplateResponse
78
from django.urls import re_path, reverse
89
from django.utils.safestring import mark_safe
@@ -274,7 +275,7 @@ class NodeAdmin(NodeLinkMixin, BaseAdmin):
274275
form = UserPropertiesForm
275276
change_form_template = "admin/topology/node/change_form.html"
276277
list_display = ["get_name", "organization", "topology", "addresses"]
277-
search_fields = ["addresses", "label", "properties"]
278+
search_fields = ["label", "_addresses_text", "_properties_text"]
278279
list_filter = [
279280
(MultitenantOrgFilter),
280281
(TopologyFilter),
@@ -292,6 +293,14 @@ class NodeAdmin(NodeLinkMixin, BaseAdmin):
292293
"modified",
293294
]
294295

296+
def get_search_results(self, request, queryset, search_term):
297+
if search_term:
298+
queryset = queryset.annotate(
299+
_addresses_text=Cast("addresses", output_field=TextField()),
300+
_properties_text=Cast("properties", output_field=TextField()),
301+
)
302+
return super().get_search_results(request, queryset, search_term)
303+
295304
def change_view(self, request, object_id, form_url="", extra_context=None):
296305
extra_context = extra_context or {}
297306
link_model = self.model.source_link_set.field.model
@@ -315,10 +324,24 @@ class LinkAdmin(NodeLinkMixin, BaseAdmin):
315324
search_fields = [
316325
"source__label",
317326
"target__label",
318-
"source__addresses",
319-
"target__addresses",
320-
"properties",
327+
"_source_addresses_text",
328+
"_target_addresses_text",
329+
"_properties_text",
321330
]
331+
332+
def get_search_results(self, request, queryset, search_term):
333+
if search_term:
334+
queryset = queryset.annotate(
335+
_source_addresses_text=Cast(
336+
"source__addresses", output_field=TextField()
337+
),
338+
_target_addresses_text=Cast(
339+
"target__addresses", output_field=TextField()
340+
),
341+
_properties_text=Cast("properties", output_field=TextField()),
342+
)
343+
return super().get_search_results(request, queryset, search_term)
344+
322345
list_display = [
323346
"__str__",
324347
"organization",

openwisp_network_topology/base/link.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44

55
import swapper
66
from django.core.exceptions import ValidationError
7+
from django.core.serializers.json import DjangoJSONEncoder
78
from django.db import models
8-
from django.db.models import Q
9+
from django.db.models import JSONField, Q, TextField
10+
from django.db.models.functions import Cast
911
from django.db.models.signals import post_delete, post_save
1012
from django.dispatch import receiver
1113
from django.utils.timezone import now
1214
from django.utils.translation import gettext_lazy as _
13-
from jsonfield import JSONField
1415
from model_utils import Choices
1516
from model_utils.fields import StatusField
1617
from rest_framework.utils.encoders import JSONEncoder
@@ -48,16 +49,14 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
4849
properties = JSONField(
4950
default=dict,
5051
blank=True,
51-
load_kwargs={"object_pairs_hook": OrderedDict},
52-
dump_kwargs={"indent": 4, "cls": JSONEncoder},
52+
encoder=DjangoJSONEncoder,
5353
)
5454
user_properties = JSONField(
5555
verbose_name=_("user defined properties"),
5656
help_text=_("If you need to add additional data to this link use this field"),
5757
default=dict,
5858
blank=True,
59-
load_kwargs={"object_pairs_hook": OrderedDict},
60-
dump_kwargs={"indent": 4, "cls": JSONEncoder},
59+
encoder=DjangoJSONEncoder,
6160
)
6261
status_changed = models.DateTimeField(auto_now=True)
6362

@@ -177,12 +176,20 @@ def get_from_nodes(cls, source, target, topology):
177176
:param topology: Topology instance
178177
:returns: Link object or None
179178
"""
180-
source = '"{}"'.format(source)
181-
target = '"{}"'.format(target)
179+
source_needle = '"{}"'.format(source)
180+
target_needle = '"{}"'.format(target)
181+
qs = cls.objects.annotate(
182+
_source_addresses_text=Cast("source__addresses", output_field=TextField()),
183+
_target_addresses_text=Cast("target__addresses", output_field=TextField()),
184+
)
182185
q = Q(
183-
source__addresses__contains=source, target__addresses__contains=target
184-
) | Q(source__addresses__contains=target, target__addresses__contains=source)
185-
return cls.objects.filter(q).filter(topology=topology).first()
186+
_source_addresses_text__contains=source_needle,
187+
_target_addresses_text__contains=target_needle,
188+
) | Q(
189+
_source_addresses_text__contains=target_needle,
190+
_target_addresses_text__contains=source_needle,
191+
)
192+
return qs.filter(q).filter(topology=topology).first()
186193

187194
@classmethod
188195
def delete_expired_links(cls):

openwisp_network_topology/base/node.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55

66
import swapper
77
from django.core.exceptions import ValidationError
8+
from django.core.serializers.json import DjangoJSONEncoder
89
from django.db import models
10+
from django.db.models import JSONField, TextField
11+
from django.db.models.functions import Cast
912
from django.db.models.signals import post_delete, post_save
1013
from django.dispatch import receiver
1114
from django.utils.functional import cached_property
1215
from django.utils.timezone import now
1316
from django.utils.translation import gettext_lazy as _
14-
from jsonfield import JSONField
1517
from rest_framework.utils.encoders import JSONEncoder
1618

1719
from openwisp_users.mixins import ShareableOrgMixin
@@ -32,20 +34,18 @@ class AbstractNode(ShareableOrgMixin, TimeStampedEditableModel):
3234
)
3335
label = models.CharField(max_length=64, blank=True)
3436
# netjson ID and local_addresses
35-
addresses = JSONField(default=[], blank=True)
37+
addresses = JSONField(default=list, blank=True, encoder=DjangoJSONEncoder)
3638
properties = JSONField(
3739
default=dict,
3840
blank=True,
39-
load_kwargs={"object_pairs_hook": OrderedDict},
40-
dump_kwargs={"indent": 4, "cls": JSONEncoder},
41+
encoder=DjangoJSONEncoder,
4142
)
4243
user_properties = JSONField(
4344
verbose_name=_("user defined properties"),
4445
help_text=_("If you need to add additional data to this node use this field"),
4546
default=dict,
4647
blank=True,
47-
load_kwargs={"object_pairs_hook": OrderedDict},
48-
dump_kwargs={"indent": 4, "cls": JSONEncoder},
48+
encoder=DjangoJSONEncoder,
4949
)
5050

5151
class Meta:
@@ -138,10 +138,13 @@ def get_from_address(cls, address, topology):
138138
:param topology: Topology instance
139139
:returns: Node object or None
140140
"""
141-
address = '"{}"'.format(address)
142-
return cls.objects.filter(
143-
topology=topology, addresses__contains=address
144-
).first()
141+
needle = '"{}"'.format(address)
142+
return (
143+
cls.objects.filter(topology=topology)
144+
.annotate(_addresses_text=Cast("addresses", output_field=TextField()))
145+
.filter(_addresses_text__contains=needle)
146+
.first()
147+
)
145148

146149
@classmethod
147150
def count_address(cls, address, topology):
@@ -151,10 +154,13 @@ def count_address(cls, address, topology):
151154
:param topology: Topology instance
152155
:returns: int
153156
"""
154-
address = '"{}"'.format(address)
155-
return cls.objects.filter(
156-
topology=topology, addresses__contains=address
157-
).count()
157+
needle = '"{}"'.format(address)
158+
return (
159+
cls.objects.filter(topology=topology)
160+
.annotate(_addresses_text=Cast("addresses", output_field=TextField()))
161+
.filter(_addresses_text__contains=needle)
162+
.count()
163+
)
158164

159165
@classmethod
160166
def delete_expired_nodes(cls):

openwisp_network_topology/integrations/device/tests/test_wifi_mesh.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,30 +93,30 @@ def test_simple_mesh(self):
9393
Node.objects.filter(
9494
topology=topology,
9595
organization=org,
96-
properties__contains=(
97-
'{\n "ht": true,\n "vht": null,\n "mfp": false,\n'
98-
' "wmm": true,\n "vendor": "TP-LINK TECHNOLOGIES CO.,LTD."\n}'
99-
),
96+
properties__ht=True,
97+
properties__vht=None,
98+
properties__mfp=False,
99+
properties__wmm=True,
100+
properties__vendor="TP-LINK TECHNOLOGIES CO.,LTD.",
100101
).count(),
101102
3,
102103
)
103104
self.assertEqual(
104105
Link.objects.filter(
105106
topology=topology,
106107
organization=org,
107-
properties__contains='"noise": -94',
108-
)
109-
.filter(properties__contains='"signal": -58')
110-
.filter(properties__contains='"mesh_llid": 19500')
111-
.filter(properties__contains='"mesh_plid": 24500')
112-
.count(),
108+
properties__noise=-94,
109+
properties__signal=-58,
110+
properties__mesh_llid=19500,
111+
properties__mesh_plid=24500,
112+
).count(),
113113
3,
114114
)
115115
self.assertEqual(
116116
Link.objects.filter(
117117
topology=topology,
118118
organization=org,
119-
properties__contains='"mesh_non_peer_ps": "INCONSISTENT: (LISTEN / ACTIVE)"',
119+
properties__mesh_non_peer_ps="INCONSISTENT: (LISTEN / ACTIVE)",
120120
).count(),
121121
1,
122122
)

openwisp_network_topology/management/commands/upgrade_from_django_netjsongraph.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ def handle(self, *args, **options):
7979
for data in netjsongraph_data:
8080
data["fields"]["organization"] = str(org.pk)
8181
data["model"] = f'{self.app_label}.{data["model"].split(".")[1]}'
82+
for json_field in ("addresses", "properties"):
83+
value = data["fields"].get(json_field)
84+
if isinstance(value, str):
85+
data["fields"][json_field] = json.loads(value)
8286
# Save in anotherfile
8387
with open(f'{options["backup"]}/netjsongraph_loaded.json', "w") as outfile:
8488
json.dump(netjsongraph_data, outfile)
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import django.core.serializers.json
2+
from django.db import migrations, models
3+
4+
5+
class Migration(migrations.Migration):
6+
7+
dependencies = [
8+
("topology", "0016_alter_topology_parser"),
9+
]
10+
11+
operations = [
12+
migrations.AlterField(
13+
model_name="link",
14+
name="properties",
15+
field=models.JSONField(
16+
blank=True,
17+
default=dict,
18+
encoder=django.core.serializers.json.DjangoJSONEncoder,
19+
),
20+
),
21+
migrations.AlterField(
22+
model_name="link",
23+
name="user_properties",
24+
field=models.JSONField(
25+
blank=True,
26+
default=dict,
27+
encoder=django.core.serializers.json.DjangoJSONEncoder,
28+
help_text="If you need to add additional data to this link use this field",
29+
verbose_name="user defined properties",
30+
),
31+
),
32+
migrations.AlterField(
33+
model_name="node",
34+
name="addresses",
35+
field=models.JSONField(
36+
blank=True,
37+
default=list,
38+
encoder=django.core.serializers.json.DjangoJSONEncoder,
39+
),
40+
),
41+
migrations.AlterField(
42+
model_name="node",
43+
name="properties",
44+
field=models.JSONField(
45+
blank=True,
46+
default=dict,
47+
encoder=django.core.serializers.json.DjangoJSONEncoder,
48+
),
49+
),
50+
migrations.AlterField(
51+
model_name="node",
52+
name="user_properties",
53+
field=models.JSONField(
54+
blank=True,
55+
default=dict,
56+
encoder=django.core.serializers.json.DjangoJSONEncoder,
57+
help_text="If you need to add additional data to this node use this field",
58+
verbose_name="user defined properties",
59+
),
60+
),
61+
]

openwisp_network_topology/tests/test_link.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ def test_get_from_nodes(self):
118118
target=node2,
119119
cost=1.0,
120120
cost_text="100mbit/s",
121-
properties='{"pretty": true}',
121+
properties={"pretty": True},
122122
)
123123
link.full_clean()
124124
link.save()

openwisp_network_topology/tests/test_topology.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ def test_update_added(self):
155155
t.update()
156156
self.assertEqual(self.node_model.objects.count(), 2)
157157
self.assertEqual(self.link_model.objects.count(), 1)
158-
node1 = self.node_model.objects.get(addresses__contains='"192.168.0.1"')
159-
node2 = self.node_model.objects.get(addresses__contains='"192.168.0.2"')
158+
node1 = self.node_model.get_from_address("192.168.0.1", t)
159+
node2 = self.node_model.get_from_address("192.168.0.2", t)
160160
self.assertEqual(node1.local_addresses, ["10.0.0.1"])
161161
self.assertEqual(node1.properties, {"gateway": True})
162162
link = self.link_model.objects.first()
@@ -297,8 +297,8 @@ def _test_receive_added(self, expiration_time=0):
297297
t.receive(data)
298298
self.assertEqual(self.node_model.objects.count(), 2)
299299
self.assertEqual(self.link_model.objects.count(), 1)
300-
node1 = self.node_model.objects.get(addresses__contains='"192.168.0.1"')
301-
node2 = self.node_model.objects.get(addresses__contains='"192.168.0.2"')
300+
node1 = self.node_model.get_from_address("192.168.0.1", t)
301+
node2 = self.node_model.get_from_address("192.168.0.2", t)
302302
self.assertEqual(node1.local_addresses, ["10.0.0.1"])
303303
self.assertEqual(node1.properties, {"gateway": True})
304304
link = self.link_model.objects.first()

0 commit comments

Comments
 (0)