diff --git a/isic/core/health.py b/isic/core/health.py index eb1f6efa..afc901a7 100644 --- a/isic/core/health.py +++ b/isic/core/health.py @@ -2,7 +2,7 @@ import logging from django.contrib.auth.models import User -from django.db.models import Count, Q +from django.db.models import Q import pyexiv2 from resonant_utils.files import field_file_to_local_path @@ -10,7 +10,6 @@ from isic.core.models.image import Image from isic.core.models.image_embedding import ImageEmbedding from isic.core.models.isic_id import IsicId -from isic.ingest.models.accession import Accession from isic.login.models import Profile logger = logging.getLogger(__name__) @@ -217,56 +216,6 @@ def check_published_images_have_attribution() -> HealthCheckResult: ) -def check_lesions_have_single_patient() -> HealthCheckResult: - # This invariant was previously enforced with an ExclusionConstraint, but those are - # unsupported by greenmask (https://github.com/GreenmaskIO/greenmask/issues/451). - lesions_with_multiple_patients = ( - Accession.objects.filter(lesion_id__isnull=False, patient_id__isnull=False) - .values("lesion_id") - .annotate(num_patients=Count("patient_id", distinct=True)) - .filter(num_patients__gt=1) - .count() - ) - - passed = lesions_with_multiple_patients == 0 - message = ( - "All lesions belong to a single patient" - if passed - else f"{lesions_with_multiple_patients} lesions belong to multiple patients" - ) - - return HealthCheckResult( - name="lesions_have_single_patient", - passed=passed, - message=message, - ) - - -def check_rcm_cases_have_single_lesion() -> HealthCheckResult: - # This invariant was previously enforced with an ExclusionConstraint, but those are - # unsupported by greenmask (https://github.com/GreenmaskIO/greenmask/issues/451). - rcm_cases_with_multiple_lesions = ( - Accession.objects.filter(rcm_case_id__isnull=False, lesion_id__isnull=False) - .values("rcm_case_id") - .annotate(num_lesions=Count("lesion_id", distinct=True)) - .filter(num_lesions__gt=1) - .count() - ) - - passed = rcm_cases_with_multiple_lesions == 0 - message = ( - "All RCM cases belong to a single lesion" - if passed - else f"{rcm_cases_with_multiple_lesions} RCM cases belong to multiple lesions" - ) - - return HealthCheckResult( - name="rcm_cases_have_single_lesion", - passed=passed, - message=message, - ) - - def check_embeddings_only_for_public_images() -> HealthCheckResult: embeddings_for_private_images = ImageEmbedding.objects.filter(image__public=False).count() @@ -297,8 +246,6 @@ def check_embeddings_only_for_public_images() -> HealthCheckResult: ("iptc_metadata_consistency", check_iptc_metadata_consistency), ("published_images_have_attribution", check_published_images_have_attribution), ("embeddings_only_for_public_images", check_embeddings_only_for_public_images), - ("lesions_have_single_patient", check_lesions_have_single_patient), - ("rcm_cases_have_single_lesion", check_rcm_cases_have_single_lesion), ] diff --git a/isic/ingest/migrations/0041_accession_accession_lesion_id_patient_id_exclusion_and_more.py b/isic/ingest/migrations/0041_accession_accession_lesion_id_patient_id_exclusion_and_more.py new file mode 100644 index 00000000..1be34609 --- /dev/null +++ b/isic/ingest/migrations/0041_accession_accession_lesion_id_patient_id_exclusion_and_more.py @@ -0,0 +1,34 @@ +# Generated by Django 5.2.3 on 2026-06-01 04:58 + +from django.conf import settings +import django.contrib.postgres.constraints +from django.db import migrations, models +import django.db.models.constraints + + +class Migration(migrations.Migration): + dependencies = [ + ("ingest", "0040_remove_accession_accession_lesion_id_patient_id_exclusion_and_more"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddConstraint( + model_name="accession", + constraint=django.contrib.postgres.constraints.ExclusionConstraint( + condition=models.Q(("lesion_id__isnull", False), ("patient_id__isnull", False)), + deferrable=django.db.models.constraints.Deferrable["IMMEDIATE"], + expressions=[("lesion_id", "="), ("patient_id", "<>")], + name="accession_lesion_id_patient_id_exclusion", + ), + ), + migrations.AddConstraint( + model_name="accession", + constraint=django.contrib.postgres.constraints.ExclusionConstraint( + condition=models.Q(("rcm_case_id__isnull", False), ("lesion_id__isnull", False)), + deferrable=django.db.models.constraints.Deferrable["IMMEDIATE"], + expressions=[("rcm_case_id", "="), ("lesion_id", "<>")], + name="accession_rcm_case_id_lesion_id_exclusion", + ), + ), + ] diff --git a/isic/ingest/models/accession.py b/isic/ingest/models/accession.py index 3987b439..e3a3a3f0 100644 --- a/isic/ingest/models/accession.py +++ b/isic/ingest/models/accession.py @@ -10,12 +10,13 @@ from uuid import uuid4 from django.contrib.auth.models import User +from django.contrib.postgres.constraints import ExclusionConstraint from django.core.exceptions import ValidationError from django.core.files import File from django.core.files.storage import storages from django.core.files.uploadedfile import InMemoryUploadedFile from django.db import models, transaction -from django.db.models import FileField, FloatField, IntegerField, Transform +from django.db.models import Deferrable, FileField, FloatField, IntegerField, Transform from django.db.models.constraints import CheckConstraint, UniqueConstraint from django.db.models.fields import Field from django.db.models.functions import Cast, Round @@ -358,6 +359,26 @@ class Meta(CreationSortedTimeStampedModel.Meta): ) | ~Q(concomitant_biopsy=True), ), + # identical lesion_id implies identical patient_id + ExclusionConstraint( + name="accession_lesion_id_patient_id_exclusion", + expressions=[ + ("lesion_id", "="), + ("patient_id", "<>"), + ], + condition=Q(lesion_id__isnull=False) & Q(patient_id__isnull=False), + deferrable=Deferrable.IMMEDIATE, + ), + # identical rcm_case_id implies identical lesion_id + ExclusionConstraint( + name="accession_rcm_case_id_lesion_id_exclusion", + expressions=[ + ("rcm_case_id", "="), + ("lesion_id", "<>"), + ], + condition=Q(rcm_case_id__isnull=False) & Q(lesion_id__isnull=False), + deferrable=Deferrable.IMMEDIATE, + ), # each RCM case can only have at most one macroscopic image UniqueConstraint( name="accession_unique_rcm_case_id_macroscopic_image",