Json field to text field (#40)

* Use TextField instead of JsonField for comma separated lists

* pylance vs pyright

* Test migrations

* Rename validator

* Improve lines validator

* Adjust lines separator

.devcontainer/devcontainer.json

@@ -29,7 +29,6 @@
         "mikestead.dotenv",
         "monosans.djlint",
         "ms-azuretools.vscode-docker",
-        "ms-pyright.pyright",
         "ms-python.black-formatter",
         "ms-python.python",
         "ms-python.vscode-pylance",

.gitpod.yml

@@ -35,10 +35,9 @@ vscode:
     - "mikestead.dotenv"
     - "monosans.djlint"
     - "ms-azuretools.vscode-docker"
-    - "ms-pyright.pyright"
+    - "ms-pyright.pyright" # Replaces pylance as it is not available in Open VSX
     - "ms-python.black-formatter"
     - "ms-python.python"
-    # - "ms-python.vscode-pylance" # Pylance not available in Open VSX
     - "streetsidesoftware.code-spell-checker"
     - "tamasfe.even-better-toml"
     - "wayou.vscode-todo-highlight"

.vscode/extensions.json

@@ -12,7 +12,6 @@
     "mikestead.dotenv",
     "monosans.djlint",
     "ms-azuretools.vscode-docker",
-    "ms-pyright.pyright",
     "ms-python.black-formatter",
     "ms-python.python",
     "ms-python.vscode-pylance",

adit/api/serializers.py

@@ -1,7 +1,9 @@
+from django.core import exceptions
 from rest_framework import serializers
 from rest_framework.fields import empty
 
 from adit.core.models import DicomNode, TransferJob, TransferTask
+from adit.core.validators import validate_uids
 
 
 class DicomNodeSerializer(serializers.ModelSerializer):
@@ -20,11 +22,17 @@ def __init__(self, instance=None, data=empty, max_series_count=100, **kwargs):
         super().__init__(instance=instance, data=data, **kwargs)
 
     def validate_series_uids(self, series_uids):
-        # TODO validate the series UID itself
-        if len(series_uids) > self.max_series_count:
+        try:
+            validate_uids(series_uids)
+        except exceptions.ValidationError as err:
+            raise serializers.ValidationError(err.message)
+
+        series_uid_list = list(filter(len, map(str.strip, series_uids.split(","))))
+        if len(series_uid_list) > self.max_series_count:
             raise serializers.ValidationError(
                 f"Maximum {self.max_series_count} series per task allowed."
             )
+
         return series_uids
 
 

adit/batch_query/factories.py

@@ -29,10 +29,14 @@ class Meta:
     accession_number = factory.Faker("numerify", text="############")
     study_date_start = factory.Faker("date_between", start_date="-2y", end_date="-1y")
     study_date_end = factory.Faker("date_between", start_date="-1y", end_date="today")
-    modalities = factory.Faker("random_elements", elements=("CT", "MR", "DX"), unique=True)
+    modalities = factory.LazyFunction(
+        lambda: ", ".join(fake.random_elements(elements=("CT", "MR", "DX"), unique=True))
+    )
     study_description = factory.Faker("street_name")
     series_description = factory.Faker("street_name")
-    series_numbers = factory.Faker("random_elements", elements=SERIES_NUMBER, unique=True)
+    series_numbers = factory.LazyFunction(
+        lambda: ", ".join(fake.random_elements(elements=SERIES_NUMBER, unique=True))
+    )
     pseudonym = factory.Faker("pystr", min_chars=10, max_chars=10)
 
 
@@ -48,7 +52,9 @@ class Meta:
     accession_number = factory.Faker("ean")
     study_date = factory.Faker("date_between", start_date="-2y", end_date="today")
     study_time = factory.Faker("time_object")
-    modalities = factory.Faker("random_elements", elements=("CT", "MR", "DX"), unique=True)
+    modalities = factory.LazyFunction(
+        lambda: ", ".join(fake.random_elements(elements=("CT", "MR", "DX"), unique=True))
+    )
     image_count = factory.Faker("random_int", min=3, max=1500)
     study_description = factory.Faker("street_name")
     series_description = factory.Faker("street_name")

adit/batch_query/migrations/0015_alter_batchqueryresult_modalities_and_more.py

@@ -0,0 +1,52 @@
+# Generated by Django 4.2.2 on 2023-06-23 08:25
+
+import adit.core.validators
+import django.core.validators
+from django.db import migrations, models
+import re
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("batch_query", "0014_alter_batchqueryresult_series_number"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="batchqueryresult",
+            name="modalities",
+            field=models.TextField(
+                blank=True, default="", validators=[adit.core.validators.validate_modalities]
+            ),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name="batchquerytask",
+            name="lines",
+            field=models.TextField(
+                validators=[
+                    django.core.validators.RegexValidator(
+                        re.compile("^\\d+(?:,\\d+)*\\Z"),
+                        code="invalid",
+                        message="Enter only digits separated by commas.",
+                    )
+                ]
+            ),
+        ),
+        migrations.AlterField(
+            model_name="batchquerytask",
+            name="modalities",
+            field=models.TextField(
+                blank=True, default="", validators=[adit.core.validators.validate_modalities]
+            ),
+            preserve_default=False,
+        ),
+        migrations.AlterField(
+            model_name="batchquerytask",
+            name="series_numbers",
+            field=models.TextField(
+                blank=True, default="", validators=[adit.core.validators.validate_series_numbers]
+            ),
+            preserve_default=False,
+        ),
+    ]

adit/batch_query/migrations/0016_convert_json_to_text.py

@@ -0,0 +1,73 @@
+# Generated by Django 4.2.2 on 2023-06-23 09:46
+
+import json
+from django.apps import AppConfig
+from django.db import migrations
+
+
+def convert_json_to_text(apps: AppConfig, schema_editor):
+    BatchQueryTask = apps.get_model("batch_query.BatchQueryTask")
+    for query in BatchQueryTask.objects.all():
+        if not query.lines:
+            query.lines = ""
+        else:
+            query.lines = ", ".join(json.loads(query.lines))
+
+        if not query.modalities:
+            query.modalities = ""
+        else:
+            query.modalities = ", ".join(json.loads(query.modalities))
+
+        if not query.series_numbers:
+            query.series_number = ""
+        else:
+            query.series_numbers = ", ".join(json.loads(query.series_numbers))
+
+        query.save()
+
+    BatchQueryResult = apps.get_model("batch_query.BatchQueryResult")
+    for result in BatchQueryResult.objects.all():
+        if not result.modalities:
+            result.modalities = ""
+        else:
+            result.modalities = ", ".join(json.loads(result.modalities))
+
+        result.save()
+
+
+def convert_text_to_json(apps: AppConfig, schema_editor):
+    BatchQueryTask = apps.get_model("batch_query.BatchQueryTask")
+    for query in BatchQueryTask.objects.all():
+        if not query.lines:
+            query.lines = "[]"
+        else:
+            query.lines = json.dumps(list(map(str.strip, query.lines.split(","))))
+
+        if not query.modalities:
+            query.modalities = "null"
+        else:
+            query.modalities = json.dumps(list(map(str.strip, query.modalities.split(","))))
+
+        if not query.series_numbers:
+            query.series_number = "null"
+        else:
+            query.series_numbers = json.dumps(list(map(str.strip, query.series_numbers.split(","))))
+
+        query.save()
+
+    BatchQueryResult = apps.get_model("batch_query.BatchQueryResult")
+    for result in BatchQueryResult.objects.all():
+        if not result.modalities:
+            result.modalities = None
+        else:
+            result.modalities = json.dumps(list(map(str.strip, result.modalities.split(","))))
+
+        result.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("batch_query", "0015_alter_batchqueryresult_modalities_and_more"),
+    ]
+
+    operations = [migrations.RunPython(convert_json_to_text, convert_text_to_json)]

adit/batch_query/migrations/0017_alter_batchquerytask_lines.py

@@ -0,0 +1,25 @@
+# Generated by Django 4.2.2 on 2023-06-25 10:57
+
+import django.core.validators
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("batch_query", "0016_convert_json_to_text"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="batchquerytask",
+            name="lines",
+            field=models.TextField(
+                validators=[
+                    django.core.validators.RegexValidator(
+                        message="Enter only digits separated by commas.",
+                        regex="^\\s*\\d+(?:\\s*,\\s*\\d+)*\\s*\\Z",
+                    )
+                ]
+            ),
+        ),
+    ]

adit/batch_query/models.py

@@ -11,6 +11,7 @@
     no_backslash_char_validator,
     no_control_chars_validator,
     no_wildcard_chars_validator,
+    pos_int_list_validator,
     validate_modalities,
     validate_series_number,
     validate_series_numbers,
@@ -42,7 +43,7 @@ def get_absolute_url(self):
 
 class BatchQueryTask(DicomTask):
     job = models.ForeignKey(BatchQueryJob, on_delete=models.CASCADE, related_name="tasks")
-    lines = models.JSONField(default=list)
+    lines = models.TextField(validators=[pos_int_list_validator])
     patient_id = models.CharField(
         blank=True,
         max_length=64,
@@ -88,8 +89,7 @@ class BatchQueryTask(DicomTask):
         blank=True,
         error_messages={"invalid": "Invalid date format."},
     )
-    modalities = models.JSONField(
-        null=True,
+    modalities = models.TextField(
         blank=True,
         validators=[validate_modalities],
     )
@@ -101,8 +101,7 @@ class BatchQueryTask(DicomTask):
         blank=True,
         max_length=64,
     )
-    series_numbers = models.JSONField(
-        null=True,
+    series_numbers = models.TextField(
         blank=True,
         validators=[validate_series_numbers],
     )
@@ -115,6 +114,30 @@ class BatchQueryTask(DicomTask):
     if TYPE_CHECKING:
         results = RelatedManager["BatchQueryResult"]()
 
+    @property
+    def lines_list(self) -> list[str]:
+        return list(filter(len, map(str.strip, self.lines.split(","))))
+
+    @lines_list.setter
+    def lines_list(self, value: list[str]) -> None:
+        self.lines = ", ".join(value)
+
+    @property
+    def modalities_list(self) -> list[str]:
+        return list(filter(len, map(str.strip, self.modalities.split(","))))
+
+    @modalities_list.setter
+    def modalities_list(self, value: list[str]) -> None:
+        self.modalities = ", ".join(value)
+
+    @property
+    def series_numbers_list(self) -> list[str]:
+        return list(filter(len, map(str.strip, self.series_numbers.split(","))))
+
+    @series_numbers_list.setter
+    def series_numbers_list(self, value: list[str]) -> None:
+        self.series_numbers = ", ".join(value)
+
     def clean(self) -> None:
         if not self.accession_number and not self.modalities:
             raise ValidationError("Missing Modality.")
@@ -163,8 +186,7 @@ class BatchQueryResult(models.Model):
     )
     study_date = models.DateField()
     study_time = models.TimeField()
-    modalities = models.JSONField(
-        null=True,
+    modalities = models.TextField(
         blank=True,
         validators=[validate_modalities],
     )
@@ -226,6 +248,14 @@ class BatchQueryResult(models.Model):
     def __str__(self):
         return f"{self.__class__.__name__} [ID {self.id}]"
 
+    @property
+    def modalities_list(self) -> list[str]:
+        return list(filter(len, map(str.strip, self.modalities.split(","))))
+
+    @modalities_list.setter
+    def modalities_list(self, value: list[str]) -> None:
+        self.modalities = ", ".join(value)
+
     @property
     def study_date_time(self):
         return datetime.combine(self.study_date, self.study_time)

adit/batch_query/parsers.py

@@ -41,10 +41,7 @@ def transform_value(self, field: str, value: str) -> str | list[str] | None:
                 return m.group(1)
 
         if field in ["modalities", "series_numbers"]:
-            values = value.split(",")
-            values = map(str.strip, values)
-            values = filter(len, values)
-            return list(values)
+            return ", ".join(filter(len, map(str.strip, value.split(","))))
 
         if field == "patient_name":
             return person_name_to_dicom(value)

adit/batch_query/tables.py

@@ -39,6 +39,3 @@ class Meta:
             "id": "batch_query_result_table",
             "class": "table table-bordered table-hover",
         }
-
-    def render_modalities(self, value):
-        return ", ".join(value)

adit/batch_query/tests/test_migrations.py

@@ -0,0 +1,48 @@
+# type: ignore
+
+import pytest
+from django_test_migrations.migrator import Migrator
+
+from ..factories import BatchQueryJobFactory
+
+
+@pytest.mark.django_db
+def test_0016_convert_json_to_text(migrator: Migrator):
+    old_state = migrator.apply_initial_migration(
+        ("batch_query", "0015_alter_batchqueryresult_modalities_and_more")
+    )
+    BatchQueryTask = old_state.apps.get_model("batch_query", "BatchQueryTask")
+    BatchQueryResult = old_state.apps.get_model("batch_query", "BatchQueryResult")
+
+    job = BatchQueryJobFactory.create()
+    query = BatchQueryTask.objects.create(
+        job_id=job.id,
+        task_id="123",
+        lines='["1", "2", "3"]',
+        modalities='["CT", "MR"]',
+        series_numbers='["4", "5", "6"]',
+    )
+    result = BatchQueryResult.objects.create(
+        job_id=job.id,
+        query_id=query.id,
+        patient_id="12345",
+        patient_name="Doe^John",
+        patient_birth_date="1980-01-01",
+        study_date="2010-01-01",
+        study_time="12:00",
+        modalities='["CT", "MR"]',
+        study_uid="1.2.3",
+    )
+
+    new_state = migrator.apply_tested_migration(("batch_query", "0016_convert_json_to_text"))
+    BatchQueryTask = new_state.apps.get_model("batch_query", "BatchQueryTask")
+    BatchQueryResult = new_state.apps.get_model("batch_query", "BatchQueryResult")
+
+    query = BatchQueryTask.objects.get(id=query.id)
+    result = BatchQueryResult.objects.get(id=result.id)
+
+    assert query.lines == "1, 2, 3"
+    assert query.modalities == "CT, MR"
+    assert query.series_numbers == "4, 5, 6"
+
+    assert result.modalities == "CT, MR"