[change] Replace third-party JSONField with Django built-in JSONField #253

jsonfield is kept in requirements.txt because historical migration
files still import it; it can be dropped once those migrations are
squashed.

JSON columns in admin search_fields are cast to text in
get_search_results to avoid the unsupported ILIKE operator on
PostgreSQL jsonb. The same cast is applied to the __contains lookups
in Node.get_from_address, Node.count_address and Link.get_from_nodes
so they keep working on SQLite/SpatiaLite. The
upgrade_from_django_netjsongraph command JSON-decodes the legacy
addresses/properties string fields before loading them, so they are
stored as proper JSON values by Django's JSONField.

Fixes #253

Author: Eeshu-Yadav

openwisp_network_topology/admin.py

@@ -2,7 +2,8 @@
 from django import forms
 from django.contrib import admin, messages
 from django.contrib.admin import ModelAdmin
-from django.db.models import Q
+from django.db.models import Q, TextField
+from django.db.models.functions import Cast
 from django.template.response import TemplateResponse
 from django.urls import re_path, reverse
 from django.utils.safestring import mark_safe
@@ -274,7 +275,7 @@ class NodeAdmin(NodeLinkMixin, BaseAdmin):
     form = UserPropertiesForm
     change_form_template = "admin/topology/node/change_form.html"
     list_display = ["get_name", "organization", "topology", "addresses"]
-    search_fields = ["addresses", "label", "properties"]
+    search_fields = ["label", "_addresses_text", "_properties_text"]
     list_filter = [
         (MultitenantOrgFilter),
         (TopologyFilter),
@@ -292,6 +293,14 @@ class NodeAdmin(NodeLinkMixin, BaseAdmin):
         "modified",
     ]
 
+    def get_search_results(self, request, queryset, search_term):
+        if search_term:
+            queryset = queryset.annotate(
+                _addresses_text=Cast("addresses", output_field=TextField()),
+                _properties_text=Cast("properties", output_field=TextField()),
+            )
+        return super().get_search_results(request, queryset, search_term)
+
     def change_view(self, request, object_id, form_url="", extra_context=None):
         extra_context = extra_context or {}
         link_model = self.model.source_link_set.field.model
@@ -315,10 +324,24 @@ class LinkAdmin(NodeLinkMixin, BaseAdmin):
     search_fields = [
         "source__label",
         "target__label",
-        "source__addresses",
-        "target__addresses",
-        "properties",
+        "_source_addresses_text",
+        "_target_addresses_text",
+        "_properties_text",
     ]
+
+    def get_search_results(self, request, queryset, search_term):
+        if search_term:
+            queryset = queryset.annotate(
+                _source_addresses_text=Cast(
+                    "source__addresses", output_field=TextField()
+                ),
+                _target_addresses_text=Cast(
+                    "target__addresses", output_field=TextField()
+                ),
+                _properties_text=Cast("properties", output_field=TextField()),
+            )
+        return super().get_search_results(request, queryset, search_term)
+
     list_display = [
         "__str__",
         "organization",

openwisp_network_topology/base/link.py

@@ -4,13 +4,14 @@
 
 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 Q
+from django.db.models import JSONField, Q, TextField
+from django.db.models.functions import Cast
 from django.db.models.signals import post_delete, post_save
 from django.dispatch import receiver
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
-from jsonfield import JSONField
 from model_utils import Choices
 from model_utils.fields import StatusField
 from rest_framework.utils.encoders import JSONEncoder
@@ -48,16 +49,14 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
     properties = JSONField(
         default=dict,
         blank=True,
-        load_kwargs={"object_pairs_hook": OrderedDict},
-        dump_kwargs={"indent": 4, "cls": JSONEncoder},
+        encoder=DjangoJSONEncoder,
     )
     user_properties = JSONField(
         verbose_name=_("user defined properties"),
         help_text=_("If you need to add additional data to this link use this field"),
         default=dict,
         blank=True,
-        load_kwargs={"object_pairs_hook": OrderedDict},
-        dump_kwargs={"indent": 4, "cls": JSONEncoder},
+        encoder=DjangoJSONEncoder,
     )
     status_changed = models.DateTimeField(auto_now=True)
 
@@ -177,12 +176,20 @@ def get_from_nodes(cls, source, target, topology):
         :param topology: Topology instance
         :returns: Link object or None
         """
-        source = '"{}"'.format(source)
-        target = '"{}"'.format(target)
+        source_needle = '"{}"'.format(source)
+        target_needle = '"{}"'.format(target)
+        qs = cls.objects.annotate(
+            _source_addresses_text=Cast("source__addresses", output_field=TextField()),
+            _target_addresses_text=Cast("target__addresses", output_field=TextField()),
+        )
         q = Q(
-            source__addresses__contains=source, target__addresses__contains=target
-        ) | Q(source__addresses__contains=target, target__addresses__contains=source)
-        return cls.objects.filter(q).filter(topology=topology).first()
+            _source_addresses_text__contains=source_needle,
+            _target_addresses_text__contains=target_needle,
+        ) | Q(
+            _source_addresses_text__contains=target_needle,
+            _target_addresses_text__contains=source_needle,
+        )
+        return qs.filter(q).filter(topology=topology).first()
 
     @classmethod
     def delete_expired_links(cls):

openwisp_network_topology/base/node.py

@@ -5,13 +5,15 @@
 
 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, TextField
+from django.db.models.functions import Cast
 from django.db.models.signals import post_delete, post_save
 from django.dispatch import receiver
 from django.utils.functional import cached_property
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
-from jsonfield import JSONField
 from rest_framework.utils.encoders import JSONEncoder
 
 from openwisp_users.mixins import ShareableOrgMixin
@@ -32,20 +34,18 @@ class AbstractNode(ShareableOrgMixin, TimeStampedEditableModel):
     )
     label = models.CharField(max_length=64, blank=True)
     # netjson ID and local_addresses
-    addresses = JSONField(default=[], blank=True)
+    addresses = JSONField(default=list, blank=True, encoder=DjangoJSONEncoder)
     properties = JSONField(
         default=dict,
         blank=True,
-        load_kwargs={"object_pairs_hook": OrderedDict},
-        dump_kwargs={"indent": 4, "cls": JSONEncoder},
+        encoder=DjangoJSONEncoder,
     )
     user_properties = JSONField(
         verbose_name=_("user defined properties"),
         help_text=_("If you need to add additional data to this node use this field"),
         default=dict,
         blank=True,
-        load_kwargs={"object_pairs_hook": OrderedDict},
-        dump_kwargs={"indent": 4, "cls": JSONEncoder},
+        encoder=DjangoJSONEncoder,
     )
 
     class Meta:
@@ -138,10 +138,13 @@ def get_from_address(cls, address, topology):
         :param topology: Topology instance
         :returns: Node object or None
         """
-        address = '"{}"'.format(address)
-        return cls.objects.filter(
-            topology=topology, addresses__contains=address
-        ).first()
+        needle = '"{}"'.format(address)
+        return (
+            cls.objects.filter(topology=topology)
+            .annotate(_addresses_text=Cast("addresses", output_field=TextField()))
+            .filter(_addresses_text__contains=needle)
+            .first()
+        )
 
     @classmethod
     def count_address(cls, address, topology):
@@ -151,10 +154,13 @@ def count_address(cls, address, topology):
         :param topology: Topology instance
         :returns: int
         """
-        address = '"{}"'.format(address)
-        return cls.objects.filter(
-            topology=topology, addresses__contains=address
-        ).count()
+        needle = '"{}"'.format(address)
+        return (
+            cls.objects.filter(topology=topology)
+            .annotate(_addresses_text=Cast("addresses", output_field=TextField()))
+            .filter(_addresses_text__contains=needle)
+            .count()
+        )
 
     @classmethod
     def delete_expired_nodes(cls):

openwisp_network_topology/management/commands/upgrade_from_django_netjsongraph.py

@@ -79,6 +79,10 @@ def handle(self, *args, **options):
         for data in netjsongraph_data:
             data["fields"]["organization"] = str(org.pk)
             data["model"] = f'{self.app_label}.{data["model"].split(".")[1]}'
+            for json_field in ("addresses", "properties"):
+                value = data["fields"].get(json_field)
+                if isinstance(value, str):
+                    data["fields"][json_field] = json.loads(value)
         # Save in anotherfile
         with open(f'{options["backup"]}/netjsongraph_loaded.json', "w") as outfile:
             json.dump(netjsongraph_data, outfile)

openwisp_network_topology/migrations/0017_migrate_to_django_jsonfield.py

@@ -0,0 +1,61 @@
+import django.core.serializers.json
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("topology", "0016_alter_topology_parser"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="link",
+            name="properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="link",
+            name="user_properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+                help_text="If you need to add additional data to this link use this field",
+                verbose_name="user defined properties",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="addresses",
+            field=models.JSONField(
+                blank=True,
+                default=list,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="user_properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+                help_text="If you need to add additional data to this node use this field",
+                verbose_name="user defined properties",
+            ),
+        ),
+    ]

openwisp_network_topology/tests/test_link.py

@@ -118,7 +118,7 @@ def test_get_from_nodes(self):
             target=node2,
             cost=1.0,
             cost_text="100mbit/s",
-            properties='{"pretty": true}',
+            properties={"pretty": True},
         )
         link.full_clean()
         link.save()

openwisp_network_topology/tests/test_topology.py

@@ -155,8 +155,8 @@ def test_update_added(self):
         t.update()
         self.assertEqual(self.node_model.objects.count(), 2)
         self.assertEqual(self.link_model.objects.count(), 1)
-        node1 = self.node_model.objects.get(addresses__contains='"192.168.0.1"')
-        node2 = self.node_model.objects.get(addresses__contains='"192.168.0.2"')
+        node1 = self.node_model.get_from_address("192.168.0.1", t)
+        node2 = self.node_model.get_from_address("192.168.0.2", t)
         self.assertEqual(node1.local_addresses, ["10.0.0.1"])
         self.assertEqual(node1.properties, {"gateway": True})
         link = self.link_model.objects.first()
@@ -297,8 +297,8 @@ def _test_receive_added(self, expiration_time=0):
         t.receive(data)
         self.assertEqual(self.node_model.objects.count(), 2)
         self.assertEqual(self.link_model.objects.count(), 1)
-        node1 = self.node_model.objects.get(addresses__contains='"192.168.0.1"')
-        node2 = self.node_model.objects.get(addresses__contains='"192.168.0.2"')
+        node1 = self.node_model.get_from_address("192.168.0.1", t)
+        node2 = self.node_model.get_from_address("192.168.0.2", t)
         self.assertEqual(node1.local_addresses, ["10.0.0.1"])
         self.assertEqual(node1.properties, {"gateway": True})
         link = self.link_model.objects.first()

tests/openwisp2/sample_network_topology/migrations/0005_migrate_to_django_jsonfield.py

@@ -0,0 +1,61 @@
+import django.core.serializers.json
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("sample_network_topology", "0004_default_groups"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="link",
+            name="properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="link",
+            name="user_properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+                help_text="If you need to add additional data to this link use this field",
+                verbose_name="user defined properties",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="addresses",
+            field=models.JSONField(
+                blank=True,
+                default=list,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="node",
+            name="user_properties",
+            field=models.JSONField(
+                blank=True,
+                default=dict,
+                encoder=django.core.serializers.json.DjangoJSONEncoder,
+                help_text="If you need to add additional data to this node use this field",
+                verbose_name="user defined properties",
+            ),
+        ),
+    ]