Merge pull request #847 from ImageMarkup/isic-161-add-lesion-retrieval

Add lesion listing endpoint

isic/conftest.py

@@ -9,6 +9,7 @@
     AccessionReviewFactory,
     CohortFactory,
     ContributorFactory,
+    LesionFactory,
     MetadataFileFactory,
     ZipUploadFactory,
 )
@@ -81,6 +82,7 @@ def eager_celery(settings):
 register(AccessionReviewFactory)
 register(CohortFactory)
 register(ContributorFactory)
+register(LesionFactory)
 register(MetadataFileFactory)
 register(ZipUploadFactory)
 

isic/core/models/image.py

@@ -198,7 +198,8 @@ def view_image_list(user_obj: User, qs: QuerySet[Image] | None = None) -> QueryS
         if user_obj.is_staff:
             return qs
         elif user_obj.is_authenticated:
-            # Note: permissions here must be also modified in build_elasticsearch_query
+            # Note: permissions here must be also modified in build_elasticsearch_query and
+            # LesionPermissions.view_lesion_list.
             return qs.filter(
                 Q(public=True)
                 | Q(accession__cohort__contributor__owners=user_obj)

isic/ingest/api.py

@@ -10,13 +10,46 @@
 from s3_file_field.widgets import S3PlaceholderFile
 
 from isic.auth import is_authenticated, is_staff
+from isic.core.api.image import ImageOut
 from isic.core.pagination import CursorPagination
 from isic.core.permissions import get_visible_objects
-from isic.ingest.models import Accession, Cohort, Contributor, MetadataFile
+from isic.ingest.models import Accession, Cohort, Contributor, Lesion, MetadataFile
 from isic.ingest.services.accession import accession_create
 from isic.ingest.services.accession.review import accession_review_bulk_create
 from isic.ingest.tasks import update_metadata_task
 
+lesion_router = Router()
+
+
+class LesionOut(ModelSchema):
+    class Meta:
+        model = Lesion
+        fields = ["id"]
+
+    images: list[ImageOut]
+
+    @staticmethod
+    def resolve_images(obj: Lesion) -> list[ImageOut]:
+        return [accession.image for accession in obj.accessions.all() if accession.published]
+
+
+@lesion_router.get(
+    "/", response=list[LesionOut], summary="Return a list of lesions.", include_in_schema=False
+)
+@paginate(CursorPagination)
+def lesion_list(request: HttpRequest):
+    # ordering is necessary for the paginator
+    return get_visible_objects(
+        request.user,
+        "ingest.view_lesion",
+        Lesion.objects.alias(c=Count("accessions__image"))
+        .prefetch_related("accessions__image")
+        .prefetch_related("accessions__cohort")
+        .filter(c__gt=0)
+        .order_by("id"),
+    )
+
+
 accession_router = Router()
 
 

isic/ingest/models/lesion.py

@@ -1,8 +1,13 @@
 import random
 
+from django.contrib.auth.models import User
+from django.contrib.postgres.aggregates import BoolAnd
 from django.core.validators import RegexValidator
 from django.db import models
 from django.db.models.constraints import CheckConstraint, UniqueConstraint
+from django.db.models.expressions import F
+from django.db.models.functions.comparison import Coalesce
+from django.db.models.query import QuerySet
 from django.db.models.query_utils import Q
 
 from isic.core.constants import LESION_ID_REGEX
@@ -38,3 +43,49 @@ class Meta:
 
     def __str__(self):
         return f"{self.private_lesion_id}->{self.id}"
+
+
+class LesionPermissions:
+    model = Lesion
+    perms = ["view_lesion"]
+    filters = {"view_lesion": "view_lesion_list"}
+
+    @staticmethod
+    def view_lesion_list(user_obj: User, qs: QuerySet[Lesion] | None = None) -> QuerySet[Lesion]:
+        qs = qs if qs is not None else Lesion.objects.all()
+
+        if user_obj.is_staff:
+            return qs
+        elif user_obj.is_authenticated:
+            return qs.filter(
+                id__in=Lesion.objects.values("id")
+                # if an image doesn't have shares it will return null which is skipped by BoolAnd.
+                # this can lead to a scenario where a lesion has a private image without shares but
+                # is still visible because bool_and(null, true) = true.
+                # coalesce it so that bool_and always has non-null values to work with.
+                .alias(user_share_id=Coalesce(F("accessions__image__shares"), -1))
+                .annotate(
+                    # note that these requirements are copied from ImagePermissions.view_image_list
+                    visible=BoolAnd(
+                        Q(accessions__image__public=True)
+                        | Q(cohort__contributor__owners=user_obj)
+                        | Q(user_share_id=user_obj.id)
+                    )
+                )
+                .filter(visible=True)
+                .values("id")
+            )
+        else:
+            return qs.filter(
+                id__in=Lesion.objects.values("id")
+                .annotate(visible=BoolAnd(Q(accessions__image__public=True)))
+                .filter(visible=True)
+                .values("id")
+            )
+
+    @staticmethod
+    def view_lesion(user_obj: User, obj: Lesion) -> bool:
+        return LesionPermissions.view_lesion_list(user_obj).contains(obj)
+
+
+Lesion.perms_class = LesionPermissions

isic/ingest/tests/factories.py

@@ -5,7 +5,7 @@
 
 from isic.core.models import CopyrightLicense
 from isic.factories import UserFactory
-from isic.ingest.models import Accession, Cohort, Contributor, MetadataFile, ZipUpload
+from isic.ingest.models import Accession, Cohort, Contributor, Lesion, MetadataFile, ZipUpload
 from isic.ingest.models.accession_review import AccessionReview
 
 from .csv_streams import csv_stream_without_filename_column
@@ -109,3 +109,12 @@ class Meta:
     accession = factory.SubFactory(AccessionFactory)
     reviewed_at = factory.Faker("date_time", tzinfo=factory.Faker("pytimezone"))
     value = factory.Faker("boolean")
+
+
+class LesionFactory(factory.django.DjangoModelFactory):
+    class Meta:
+        model = Lesion
+
+    id = factory.Sequence(lambda n: f"IL_{n:07}")
+    cohort = factory.SubFactory(CohortFactory)
+    private_lesion_id = factory.Faker("uuid4")

isic/ingest/tests/test_api_lesion.py

@@ -0,0 +1,73 @@
+import pytest
+from pytest_lazyfixture import lazy_fixture
+
+
+@pytest.mark.django_db
+def test_api_lesion(authenticated_client, lesion_factory, image_factory):
+    lesion = lesion_factory()
+    image_factory(accession__lesion=lesion)
+
+    resp = authenticated_client.get("/api/v2/lesions/")
+    assert resp.status_code == 200, resp.json()
+
+
+@pytest.mark.django_db
+def test_api_lesion_ignores_imageless_lesions(authenticated_client, lesion_factory, user):
+    # give access to the lesion to ensure this isn't passing due to lack of permissions
+    lesion_factory(cohort__contributor__owners=[user])
+
+    resp = authenticated_client.get("/api/v2/lesions/")
+    assert resp.status_code == 200, resp.json()
+    assert len(resp.json()["results"]) == 0
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    ["client_", "image_public_states", "expected_lesion_count"],
+    [
+        [lazy_fixture("client"), [True, True], 1],
+        [lazy_fixture("client"), [True, False], 0],
+        [lazy_fixture("client"), [False, False], 0],
+        [lazy_fixture("authenticated_client"), [True, True], 1],
+        [lazy_fixture("authenticated_client"), [True, False], 0],
+        [lazy_fixture("authenticated_client"), [False, False], 0],
+        [lazy_fixture("staff_client"), [True, True], 1],
+        [lazy_fixture("staff_client"), [True, False], 1],
+        [lazy_fixture("staff_client"), [False, False], 1],
+    ],
+)
+def test_api_lesion_permissions_public(
+    client_,
+    image_public_states,
+    expected_lesion_count,
+    lesion_factory,
+    image_factory,
+    user_factory,
+    user,
+):
+    other_user = user_factory()
+    lesion = lesion_factory(cohort__contributor__owners=[other_user])
+
+    for public in image_public_states:
+        image_factory(
+            accession__lesion=lesion,
+            public=public,
+            accession__cohort__contributor__owners=[other_user],
+        )
+
+    resp = client_.get("/api/v2/lesions/")
+    assert resp.status_code == 200, resp.json()
+    assert len(resp.json()["results"]) == expected_lesion_count
+
+
+@pytest.mark.django_db
+def test_api_lesion_permissions_contributor(
+    authenticated_client, lesion_factory, image_factory, contributor
+):
+    lesion = lesion_factory()
+    image_factory(
+        accession__lesion=lesion, accession__cohort__contributor=contributor, public=False
+    )
+
+    resp = authenticated_client.get("/api/v2/lesions/")
+    assert resp.status_code == 200, resp.json()

isic/urls.py

@@ -19,6 +19,7 @@
     autocomplete_router,
     cohort_router,
     contributor_router,
+    lesion_router,
     metadata_file_router,
 )
 from isic.studies.api import annotation_router, study_router, study_task_router
@@ -42,6 +43,7 @@
 api.add_router("/collections/", collection_router, tags=["collections"])
 api.add_router("/contributors/", contributor_router, tags=["contributors"])
 api.add_router("/images/", image_router, tags=["images"])
+api.add_router("/lesions/", lesion_router, tags=["lesions"])
 api.add_router("/metadata-files/", metadata_file_router, tags=["metadata-files"])
 api.add_router("/quickfind/", quickfind_router, tags=["quickfind"])
 api.add_router("/studies/", study_router, tags=["studies"])