Refactor job creation code

app/grandchallenge/cases/models.py

@@ -154,6 +154,7 @@ def update_status(self, *, status, error_message=None, linked_object=None):
 
     def process_images(
         self,
+        *,
         linked_app_label=None,
         linked_model_name=None,
         linked_object_pk=None,

app/grandchallenge/components/form_fields.py

@@ -1,5 +1,6 @@
 from django import forms
 from django.forms import ModelChoiceField
+from django.utils.functional import cached_property
 
 from grandchallenge.cases.widgets import (
     FlexibleImageField,
@@ -11,6 +12,9 @@
 from grandchallenge.core.guardian import get_objects_for_user
 from grandchallenge.core.validators import JSONValidator
 from grandchallenge.core.widgets import JSONEditorWidget
+from grandchallenge.serving.models import (
+    get_component_interface_values_for_user,
+)
 from grandchallenge.subdomains.utils import reverse
 from grandchallenge.uploads.models import UserUpload
 from grandchallenge.uploads.widgets import (
@@ -32,7 +36,7 @@ def _join_with_br(a, b):
         return b
 
 
-INTERFACE_FORM_FIELD_PREFIX = "interface_field_"
+INTERFACE_FORM_FIELD_PREFIX = "__INTERFACE_FIELD__"
 
 
 class InterfaceFormField:
@@ -137,11 +141,7 @@ def get_file_field(self):
         key = f"value_type_{INTERFACE_FORM_FIELD_PREFIX}{self.instance.slug}"
         if key in self.form_data.keys():
             type = self.form_data[key]
-        elif (
-            self.user.user_profile.file_civs_user_has_permission_to_use.filter(
-                interface=self.instance
-            ).exists()
-        ):
+        elif self.civs_for_user_for_interface.exists():
             type = "civ"
         else:
             type = "uuid"
@@ -173,15 +173,19 @@ def get_file_field(self):
             )
 
             return ModelChoiceField(
-                queryset=self.user.user_profile.file_civs_user_has_permission_to_use.filter(
-                    interface=self.instance
-                ),
+                queryset=self.civs_for_user_for_interface,
                 widget=SelectUploadWidget(
                     attrs={"upload_link": file_upload_link}
                 ),
                 **self.kwargs,
             )
 
+    @cached_property
+    def civs_for_user_for_interface(self):
+        return get_component_interface_values_for_user(user=self.user).filter(
+            interface=self.instance
+        )
+
     @property
     def field(self):
         return self._field

app/grandchallenge/components/forms.py

@@ -113,7 +113,7 @@ def __init__(self, *args, instance, base_obj, user, **kwargs):
 
         # add fields for all interfaces that already exist on
         # other display sets / archive items
-        for slug, _ in base_obj.values_for_interfaces.items():
+        for slug in base_obj.values_for_interfaces.keys():
             current_value = None
 
             if instance:

app/grandchallenge/components/tasks.py

@@ -1165,7 +1165,9 @@ def validate_voxel_values(*, civ_pk):
     civ.interface._validate_voxel_values(civ.image)
 
 
-@acks_late_micro_short_task(retry_on=(LockNotAcquiredException,))
+@acks_late_micro_short_task(
+    retry_on=(LockNotAcquiredException,), delayed_retry=False
+)
 @transaction.atomic
 def add_image_to_object(
     *,
@@ -1237,7 +1239,9 @@ def add_image_to_object(
         on_commit(signature(linked_task).apply_async)
 
 
-@acks_late_micro_short_task(retry_on=(LockNotAcquiredException,))
+@acks_late_micro_short_task(
+    retry_on=(LockNotAcquiredException,), delayed_retry=False
+)
 @transaction.atomic
 def add_file_to_object(
     *,

app/grandchallenge/profiles/models.py

@@ -5,7 +5,7 @@
 from django.contrib.auth import get_user_model
 from django.core.signing import Signer
 from django.db import models
-from django.db.models import Exists, OuterRef, Q, TextChoices
+from django.db.models import TextChoices
 from django.db.models.signals import post_save
 from django.template.defaultfilters import pluralize
 from django.utils.functional import cached_property
@@ -18,7 +18,6 @@
 from guardian.utils import get_anonymous_user
 from stdimage import JPEGField
 
-from grandchallenge.core.guardian import get_objects_for_user
 from grandchallenge.core.models import UUIDModel
 from grandchallenge.core.storage import get_mugshot_path
 from grandchallenge.core.templatetags.remove_whitespace import oxford_comma
@@ -245,41 +244,6 @@ def dispatch_unread_direct_messages_email(
             subscription_type=EmailSubscriptionTypes.NOTIFICATION,
         )
 
-    @cached_property
-    def file_civs_user_has_permission_to_use(self):
-        # local import to avoid circular dependency
-        from grandchallenge.components.models import (
-            ComponentInterfaceValue,
-            InterfaceKind,
-        )
-
-        job_query = get_objects_for_user(
-            self.user, "algorithms.view_job"
-        ).filter(inputs__pk__in=OuterRef("pk"))
-        display_set_query = get_objects_for_user(
-            self.user, "reader_studies.change_displayset"
-        ).filter(values__pk__in=OuterRef("pk"))
-        archive_item_query = get_objects_for_user(
-            self.user, "archives.change_archiveitem"
-        ).filter(values__pk__in=OuterRef("pk"))
-        return (
-            ComponentInterfaceValue.objects.filter(
-                Q(interface__kind__in=InterfaceKind.interface_type_file())
-                | (
-                    Q(interface__kind__in=InterfaceKind.interface_type_json())
-                    & Q(interface__store_in_database=False)
-                )
-            )
-            .annotate(has_view_job_perm=Exists(job_query))
-            .annotate(has_change_ds_perm=Exists(display_set_query))
-            .annotate(has_change_ai_perm=Exists(archive_item_query))
-            .filter(
-                Q(has_view_job_perm=True)
-                | Q(has_change_ds_perm=True)
-                | Q(has_change_ai_perm=True)
-            )
-        )
-
 
 class UserProfileUserObjectPermission(UserObjectPermissionBase):
     content_object = models.ForeignKey(UserProfile, on_delete=models.CASCADE)

app/grandchallenge/serving/models.py

@@ -1,10 +1,12 @@
 from django.conf import settings
 from django.db import models
+from django.db.models import Exists, OuterRef, Q
 
 from grandchallenge.algorithms.models import AlgorithmImage, AlgorithmModel
 from grandchallenge.cases.models import Image
 from grandchallenge.challenges.models import ChallengeRequest
 from grandchallenge.components.models import ComponentInterfaceValue
+from grandchallenge.core.guardian import get_objects_for_user
 from grandchallenge.evaluation.models import Submission
 from grandchallenge.workstations.models import Feedback
 
@@ -43,3 +45,36 @@ class Download(models.Model):
     algorithm_image = models.ForeignKey(
         AlgorithmImage, null=True, on_delete=models.CASCADE, editable=False
     )
+
+
+def get_component_interface_values_for_user(*, user):
+    job_inputs_query = get_objects_for_user(
+        user=user, perms="algorithms.view_job"
+    ).filter(inputs__pk__in=OuterRef("pk"))
+
+    job_outputs_query = get_objects_for_user(
+        user=user, perms="algorithms.view_job"
+    ).filter(outputs__pk__in=OuterRef("pk"))
+
+    display_set_query = get_objects_for_user(
+        user=user, perms="reader_studies.view_displayset"
+    ).filter(values__pk__in=OuterRef("pk"))
+
+    archive_item_query = get_objects_for_user(
+        user=user, perms="archives.view_archiveitem"
+    ).filter(values__pk__in=OuterRef("pk"))
+
+    return (
+        ComponentInterfaceValue.objects.annotate(
+            has_view_job_inputs_perm=Exists(job_inputs_query)
+        )
+        .annotate(has_view_job_outputs_perm=Exists(job_outputs_query))
+        .annotate(has_view_ds_perm=Exists(display_set_query))
+        .annotate(has_view_ai_perm=Exists(archive_item_query))
+        .filter(
+            Q(has_view_job_inputs_perm=True)
+            | Q(has_view_job_outputs_perm=True)
+            | Q(has_view_ds_perm=True)
+            | Q(has_view_ai_perm=True)
+        )
+    )

app/grandchallenge/serving/views.py

@@ -12,10 +12,12 @@
 from grandchallenge.cases.models import Image
 from grandchallenge.challenges.models import ChallengeRequest
 from grandchallenge.components.models import ComponentInterfaceValue
-from grandchallenge.core.guardian import get_objects_for_user
 from grandchallenge.core.storage import internal_protected_s3_storage
 from grandchallenge.evaluation.models import Submission
-from grandchallenge.serving.models import Download
+from grandchallenge.serving.models import (
+    Download,
+    get_component_interface_values_for_user,
+)
 from grandchallenge.workstations.models import Feedback
 
 
@@ -137,25 +139,10 @@ def serve_component_interface_value(
     except (MultipleObjectsReturned, ComponentInterfaceValue.DoesNotExist):
         raise Http404("No ComponentInterfaceValue found.")
 
-    for perm, lookup in (
-        ("algorithms.view_job", "outputs"),
-        ("algorithms.view_job", "inputs"),
-        ("archives.view_archiveitem", "values"),
-        ("reader_studies.view_displayset", "values"),
-    ):
-        # Q | Q filters are very slow, this potentially does several db calls
-        # but each is quite performant. Could be optimised later.
-        if (
-            get_objects_for_user(
-                user=user,
-                perms=perm,
-            )
-            .filter(**{lookup: civ})
-            .exists()
-        ):
-            return protected_storage_redirect(
-                name=civ.file.name, creator=user, component_interface_value=civ
-            )
+    if civ in get_component_interface_values_for_user(user=user):
+        return protected_storage_redirect(
+            name=civ.file.name, creator=user, component_interface_value=civ
+        )
 
     raise PermissionDenied
 

app/tests/profiles_tests/test_models.py

@@ -5,25 +5,14 @@
 from django.contrib.sites.models import Site
 from django.core import mail
 
-from grandchallenge.components.models import ComponentInterface
 from grandchallenge.notifications.models import Notification
 from grandchallenge.profiles.models import (
     EmailSubscriptionTypes,
     NotificationEmailOptions,
 )
 from grandchallenge.subdomains.utils import reverse
-from tests.algorithms_tests.factories import AlgorithmJobFactory
-from tests.archives_tests.factories import ArchiveFactory, ArchiveItemFactory
-from tests.components_tests.factories import (
-    ComponentInterfaceFactory,
-    ComponentInterfaceValueFactory,
-)
 from tests.factories import UserFactory
 from tests.notifications_tests.factories import NotificationFactory
-from tests.reader_studies_tests.factories import (
-    DisplaySetFactory,
-    ReaderStudyFactory,
-)
 from tests.utils import get_view_for_user
 
 
@@ -199,52 +188,3 @@ def test_unsubscribe_link(
         unsubscribe_viewname,
         kwargs={"token": user.user_profile.unsubscribe_token},
     )
-
-
-@pytest.mark.django_db
-def test_file_civs_user_has_permission_to_use():
-    user = UserFactory()
-    assert list(user.user_profile.file_civs_user_has_permission_to_use) == []
-
-    ci_file = ComponentInterfaceFactory(
-        kind=ComponentInterface.Kind.ANY, store_in_database=False
-    )
-    ci_str = ComponentInterfaceFactory(kind=ComponentInterface.Kind.STRING)
-
-    civ1, civ2, civ3, civ4, civ5, civ6 = (
-        ComponentInterfaceValueFactory.create_batch(6, interface=ci_file)
-    )
-    civ_str = ComponentInterfaceValueFactory(interface=ci_str)
-
-    job_with_perm = AlgorithmJobFactory(creator=user, time_limit=60)
-    job_without_perm = AlgorithmJobFactory(time_limit=60)
-
-    job_with_perm.inputs.set([civ1])
-    job_without_perm.inputs.set([civ2])
-
-    rs = ReaderStudyFactory()
-    rs.add_editor(user)
-    ds_with_perm = DisplaySetFactory(reader_study=rs)
-    ds_without_perm = DisplaySetFactory()
-
-    ds_with_perm.values.set([civ3])
-    ds_without_perm.values.set([civ4])
-
-    archive = ArchiveFactory()
-    archive.add_editor(user)
-    ai_with_perm = ArchiveItemFactory(archive=archive)
-    ai_without_perm = ArchiveItemFactory()
-
-    ai_with_perm.values.set([civ5])
-    ai_without_perm.values.set([civ6])
-
-    del user.user_profile.file_civs_user_has_permission_to_use
-    assert civ1 in user.user_profile.file_civs_user_has_permission_to_use
-    assert civ3 in user.user_profile.file_civs_user_has_permission_to_use
-    assert civ5 in user.user_profile.file_civs_user_has_permission_to_use
-    assert civ2 not in user.user_profile.file_civs_user_has_permission_to_use
-    assert civ4 not in user.user_profile.file_civs_user_has_permission_to_use
-    assert civ6 not in user.user_profile.file_civs_user_has_permission_to_use
-    assert (
-        civ_str not in user.user_profile.file_civs_user_has_permission_to_use
-    )

app/tests/reader_studies_tests/test_forms.py

@@ -1344,8 +1344,8 @@ def test_display_set_add_interface_form():
         auto_id="1",
     )
     assert sorted(form.fields.keys()) == [
-        "interface",
         f"{INTERFACE_FORM_FIELD_PREFIX}{ci_file.slug}",
+        "interface",
     ]
     assert isinstance(
         form.fields[f"{INTERFACE_FORM_FIELD_PREFIX}{ci_file.slug}"].widget,
@@ -1361,8 +1361,8 @@ def test_display_set_add_interface_form():
         auto_id="1",
     )
     assert sorted(form.fields.keys()) == [
-        "interface",
         f"{INTERFACE_FORM_FIELD_PREFIX}{ci_value.slug}",
+        "interface",
     ]
     assert isinstance(
         form.fields[f"{INTERFACE_FORM_FIELD_PREFIX}{ci_value.slug}"].widget,
@@ -1378,8 +1378,8 @@ def test_display_set_add_interface_form():
         auto_id="1",
     )
     assert sorted(form.fields.keys()) == [
-        "interface",
         f"{INTERFACE_FORM_FIELD_PREFIX}{ci_image.slug}",
+        "interface",
     ]
     assert isinstance(
         form.fields[f"{INTERFACE_FORM_FIELD_PREFIX}{ci_image.slug}"].widget,