From 287eed8f0b95a161bde15ac0c44f7f099b7fb591 Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Fri, 19 Apr 2024 11:25:13 +0000
Subject: [PATCH] Start fixing expected failures
---
arkindex/process/api.py | 24 ++-
arkindex/process/serializers/imports.py | 2 +-
arkindex/process/tests/test_processes.py | 132 +++++++++++------
arkindex/process/tests/test_templates.py | 173 +++++++++++-----------
arkindex/process/tests/test_workerruns.py | 29 ++--
arkindex/project/mixins.py | 46 +-----
arkindex/project/tests/test_acl_mixin.py | 71 ---------
7 files changed, 210 insertions(+), 267 deletions(-)
diff --git a/arkindex/process/api.py b/arkindex/process/api.py
index c0b6131409..e753d3214f 100644
--- a/arkindex/process/api.py
+++ b/arkindex/process/api.py
@@ -2014,8 +2014,15 @@ class CreateProcessTemplate(ProcessACLMixin, WorkerACLMixin, CreateAPIView):
serializer_class = CreateProcessTemplateSerializer
def get_queryset(self):
- return Process.objects \
- .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related("version__worker__type")))
+ return (
+ Process
+ .objects
+ .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related(
+ "version__worker__type",
+ "version__worker__repository",
+ )))
+ .select_related("corpus")
+ )
def check_object_permissions(self, request, process):
access_level = self.process_access_level(process)
@@ -2077,9 +2084,16 @@ class ApplyProcessTemplate(ProcessACLMixin, WorkerACLMixin, CreateAPIView):
serializer_class = ApplyProcessTemplateSerializer
def get_queryset(self):
- return Process.objects \
- .filter(mode=ProcessMode.Template) \
- .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related("version__worker__type", "model_version__model")))
+ return (
+ Process.objects
+ .filter(mode=ProcessMode.Template)
+ .prefetch_related(Prefetch("worker_runs", queryset=WorkerRun.objects.select_related(
+ "version__worker__type",
+ "version__worker__repository",
+ "model_version__model",
+ )))
+ .select_related("corpus")
+ )
def check_object_permissions(self, request, template):
access_level = self.process_access_level(template)
diff --git a/arkindex/process/serializers/imports.py b/arkindex/process/serializers/imports.py
index 830e01541b..2c659da43c 100644
--- a/arkindex/process/serializers/imports.py
+++ b/arkindex/process/serializers/imports.py
@@ -522,7 +522,7 @@ class CreateProcessTemplateSerializer(serializers.ModelSerializer):
class ApplyProcessTemplateSerializer(ProcessACLMixin, serializers.Serializer):
process_id = serializers.PrimaryKeyRelatedField(
- queryset=Process.objects.all(),
+ queryset=Process.objects.select_related("corpus"),
source="process",
required=True,
help_text="ID of the process to apply the template to",
diff --git a/arkindex/process/tests/test_processes.py b/arkindex/process/tests/test_processes.py
index 31d7de782d..f216464246 100644
--- a/arkindex/process/tests/test_processes.py
+++ b/arkindex/process/tests/test_processes.py
@@ -1,5 +1,4 @@
import uuid
-from unittest import expectedFailure
from unittest.mock import call, patch
from django.conf import settings
@@ -753,16 +752,35 @@ class TestProcesses(FixtureAPITestCase):
{"__all__": ["Please wait activities to be initialized before deleting this process"]}
)
- @expectedFailure
- def test_delete_process_no_permission(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=None)
+ def test_delete_process_no_permission(self, get_max_level_mock):
"""
- A user cannot delete a process linked to a corpus they have no admin access to
+ A user cannot delete a process linked to a corpus they have no access to
"""
self.client.force_login(self.user)
- self.assertFalse(self.user_img_process.corpus.memberships.filter(user=self.user).exists())
- response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id}))
- self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
self.assertDictEqual(response.json(), {"detail": "Not found."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus))
+
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value)
+ def test_delete_process_not_admin(self, get_max_level_mock):
+ """
+ A user cannot delete a process linked to a corpus they have no admin access to
+ """
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse("api:process-details", kwargs={"pk": self.user_img_process.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+ self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus))
@patch("arkindex.project.triggers.process_tasks.process_delete.delay")
def test_delete_worker_run_in_use(self, process_delay_mock):
@@ -944,16 +962,20 @@ class TestProcesses(FixtureAPITestCase):
process.refresh_from_db()
self.assertEqual(process.element, element)
- @expectedFailure
- def test_partial_update_no_permission(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=None)
+ def test_partial_update_no_permission(self, get_max_level_mock):
"""
- A user cannot update a process linked to a corpus he has no admin access to
+ A user cannot update a process linked to a corpus they do not have admin access to
"""
self.client.force_login(self.user)
- self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value)
- response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id}))
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
- self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."})
+
+ with self.assertNumQueries(5):
+ response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ self.assertDictEqual(response.json(), {"detail": "Not found."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus))
def test_partial_update_stop(self):
"""
@@ -1374,13 +1396,17 @@ class TestProcesses(FixtureAPITestCase):
process.refresh_from_db()
self.assertEqual(process.name, "newName")
- @expectedFailure
- def test_partial_update_corpus_no_write_right(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value)
+ def test_partial_update_corpus_no_write_right(self, get_max_level_mock):
self.client.force_login(self.user)
- self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value)
- response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id}))
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+ with self.assertNumQueries(5):
+ response = self.client.patch(reverse("api:process-details", kwargs={"pk": self.elts_process.id}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus))
def test_partial_update(self):
"""
@@ -1584,24 +1610,27 @@ class TestProcesses(FixtureAPITestCase):
# Activity initialization runs again
self.assertFalse(delay_mock.called)
- @expectedFailure
- def test_retry_farm_guest(self):
+ @patch("arkindex.ponos.models.Farm.is_available", return_value=False)
+ def test_retry_farm_unavailable(self, is_available_mock):
self.elts_process.run()
self.elts_process.tasks.all().update(state=State.Error)
self.elts_process.finished = timezone.now()
+ self.elts_process.farm = Farm.objects.get(name="Wheat farm")
self.elts_process.save()
self.assertEqual(self.elts_process.state, State.Error)
self.elts_process.farm.memberships.filter(user=self.user).delete()
self.client.force_login(self.user)
- with self.assertNumQueries(10):
+ with self.assertNumQueries(6):
response = self.client.post(reverse("api:process-retry", kwargs={"pk": self.elts_process.id}))
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {
"farm": ["You do not have access to this farm."],
})
+ self.assertEqual(is_available_mock.call_count, 1)
+ self.assertEqual(is_available_mock.call_args, call(self.user))
@override_settings(PUBLIC_HOSTNAME="https://darkindex.lol")
def test_retry_archived_worker(self):
@@ -2047,27 +2076,28 @@ class TestProcesses(FixtureAPITestCase):
process = Process.objects.get(id=data["id"])
self.assertEqual(process.farm, farm)
- @expectedFailure
- def test_from_files_farm_guest(self):
+ @patch("arkindex.ponos.models.Farm.is_available", return_value=False)
+ def test_from_files_farm_guest(self, is_available_mock):
self.client.force_login(self.user)
self.assertEqual(self.version_with_model.worker_runs.count(), 0)
- self.other_farm.memberships.filter(user=self.user).delete()
with (
self.settings(IMPORTS_WORKER_VERSION=str(self.version_with_model.id)),
- self.assertNumQueries(9)
+ self.assertNumQueries(6)
):
response = self.client.post(reverse("api:files-process"), {
"files": [str(self.img_df.id)],
"folder_type": "volume",
"element_type": "page",
- "farm_id": str(self.other_farm.id),
+ "farm_id": str(Farm.objects.first().id),
}, format="json")
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.json(), {
"farm_id": ["You do not have access to this farm."],
})
+ self.assertEqual(is_available_mock.call_count, 1)
+ self.assertEqual(is_available_mock.call_args, call(self.user))
def test_start_process_requires_login(self):
response = self.client.post(
@@ -2442,14 +2472,15 @@ class TestProcesses(FixtureAPITestCase):
self.assertEqual(workers_process.state, State.Unscheduled)
self.assertEqual(workers_process.farm_id, farm.id)
- @expectedFailure
- def test_start_process_default_farm_guest(self):
+ @patch("arkindex.ponos.models.Farm.is_available", return_value=False)
+ @patch("arkindex.process.serializers.imports.get_default_farm")
+ def test_start_process_default_farm_guest(self, get_default_farm_mock, is_available_mock):
+ get_default_farm_mock.return_value = Farm.objects.first()
workers_process = self.corpus.processes.create(creator=self.user, mode=ProcessMode.Workers)
workers_process.worker_runs.create(version=self.recognizer, parents=[], configuration=None)
self.client.force_login(self.user)
- self.default_farm.memberships.filter(user=self.user).delete()
- with self.assertNumQueries(10):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:process-start", kwargs={"pk": str(workers_process.id)})
)
@@ -2461,18 +2492,20 @@ class TestProcesses(FixtureAPITestCase):
workers_process.refresh_from_db()
self.assertEqual(workers_process.state, State.Unscheduled)
self.assertIsNone(workers_process.farm)
+ self.assertEqual(get_default_farm_mock.call_count, 1)
+ self.assertEqual(is_available_mock.call_count, 1)
+ self.assertEqual(is_available_mock.call_args, call(self.user))
- @expectedFailure
- def test_start_process_farm_guest(self):
+ @patch("arkindex.ponos.models.Farm.is_available", return_value=False)
+ def test_start_process_farm_guest(self, is_available_mock):
workers_process = self.corpus.processes.create(creator=self.user, mode=ProcessMode.Workers)
workers_process.worker_runs.create(version=self.recognizer, parents=[], configuration=None)
self.client.force_login(self.user)
- self.other_farm.memberships.filter(user=self.user).delete()
- with self.assertNumQueries(10):
+ with self.assertNumQueries(7):
response = self.client.post(
reverse("api:process-start", kwargs={"pk": str(workers_process.id)}),
- {"farm": str(self.other_farm.id)}
+ {"farm": str(Farm.objects.first().id)}
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
@@ -2482,6 +2515,8 @@ class TestProcesses(FixtureAPITestCase):
workers_process.refresh_from_db()
self.assertEqual(workers_process.state, State.Unscheduled)
self.assertIsNone(workers_process.farm)
+ self.assertEqual(is_available_mock.call_count, 1)
+ self.assertEqual(is_available_mock.call_args, call(self.user))
def test_start_process_wrong_farm_id(self):
"""
@@ -2857,8 +2892,8 @@ class TestProcesses(FixtureAPITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertDictEqual(response.json(), {"__all__": ["A process can only be cleared before getting started."]})
- @expectedFailure
- def test_clear_process_requires_permissions(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value)
+ def test_clear_process_requires_permissions(self, get_max_level_mock):
process = self.corpus.processes.create(
creator=self.user,
mode=ProcessMode.Workers,
@@ -2872,12 +2907,14 @@ class TestProcesses(FixtureAPITestCase):
user2.verified_email = True
user2.save()
- self.corpus.memberships.create(user=user2, level=Role.Contributor.value)
self.client.force_login(user2)
- with self.assertNumQueries(8):
+ with self.assertNumQueries(4):
response = self.client.delete(reverse("api:clear-process", kwargs={"pk": str(process.id)}))
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(response.json(), {"detail": "You do not have a sufficient access level to this process."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(user2, self.corpus))
def test_select_failed_elts_forbidden_methods(self):
self.client.force_login(self.user)
@@ -2905,16 +2942,17 @@ class TestProcesses(FixtureAPITestCase):
response = self.client.post(reverse("api:process-select-failures", kwargs={"pk": str(uuid.uuid4())}))
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
- @expectedFailure
- def test_select_failed_elts_requires_corpus_read_access(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=None)
+ def test_select_failed_elts_requires_corpus_read_access(self, get_max_level_mock):
self.client.force_login(self.user)
- self.elts_process.corpus.memberships.filter(user=self.user).delete()
- self.elts_process.corpus.public = False
- self.elts_process.corpus.save()
- with self.assertNumQueries(5):
+
+ with self.assertNumQueries(3):
response = self.client.post(reverse("api:process-select-failures", kwargs={"pk": str(self.elts_process.id)}))
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(response.json(), {"detail": "You do not have a read access to the corpus of this process."})
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.corpus))
def test_select_failed_elts_requires_workers_mode(self):
self.client.force_login(self.user)
diff --git a/arkindex/process/tests/test_templates.py b/arkindex/process/tests/test_templates.py
index 770b6d2f96..cc32112c7f 100644
--- a/arkindex/process/tests/test_templates.py
+++ b/arkindex/process/tests/test_templates.py
@@ -1,6 +1,6 @@
import json
from datetime import datetime, timezone
-from unittest import expectedFailure
+from unittest.mock import call, patch
from rest_framework import status
from rest_framework.reverse import reverse
@@ -82,7 +82,7 @@ class TestTemplates(FixtureAPITestCase):
def test_create(self):
self.client.force_login(self.user)
- with self.assertNumQueries(9):
+ with self.assertNumQueries(8):
response = self.client.post(
reverse(
"api:create-process-template", kwargs={"pk": str(self.process_template.id)}
@@ -132,38 +132,47 @@ class TestTemplates(FixtureAPITestCase):
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
- @expectedFailure
- def test_create_requires_contributor_access_rights_process(self):
- new_user = User.objects.create(email="new@test.fr", verified_email=True)
- self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value)
- self.private_corpus.memberships.create(user=new_user, level=Role.Guest.value)
- self.client.force_login(new_user)
- response = self.client.post(
- reverse(
- "api:create-process-template",
- kwargs={"pk": str(self.private_process_template.id)},
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value)
+ def test_create_requires_contributor_access_rights_process(self, get_max_level_mock):
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse(
+ "api:create-process-template",
+ kwargs={"pk": str(self.private_process_template.id)},
+ )
)
- )
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(
response.json(),
{"detail": "You do not have a contributor access to this process."},
)
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus))
+
+ @patch("arkindex.project.mixins.has_access", return_value=False)
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Contributor.value)
+ def test_create_requires_access_rights_all_workers(self, get_max_level_mock, has_access_mock):
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse("api:create-process-template", kwargs={"pk": str(self.private_process_template.id)})
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
- @expectedFailure
- def test_create_requires_access_rights_all_workers(self):
- new_user = User.objects.create(email="new@test.fr", verified_email=True)
- self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value)
- self.worker_1.memberships.create(user=new_user, level=Role.Guest.value)
- self.client.force_login(new_user)
- response = self.client.post(
- reverse("api:create-process-template", kwargs={"pk": str(self.private_process_template.id)})
- )
- self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.json(),
["You do not have an execution access to every worker of this process."],
)
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus))
+ self.assertEqual(has_access_mock.call_args_list, [
+ call(self.user, self.worker_1, Role.Contributor.value, skip_public=False),
+ call(self.user, self.worker_1.repository, Role.Contributor.value, skip_public=False),
+ ])
def test_create_unsupported_mode(self):
self.client.force_login(self.user)
@@ -172,7 +181,7 @@ class TestTemplates(FixtureAPITestCase):
self.process.mode = mode
self.process.save()
- with self.assertNumQueries(5):
+ with self.assertNumQueries(4):
response = self.client.post(
reverse("api:create-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -188,7 +197,7 @@ class TestTemplates(FixtureAPITestCase):
self.client.force_login(self.user)
local_process = self.user.processes.get(mode=ProcessMode.Local)
- with self.assertNumQueries(5):
+ with self.assertNumQueries(4):
response = self.client.post(
reverse("api:create-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(local_process.id)}),
@@ -200,74 +209,72 @@ class TestTemplates(FixtureAPITestCase):
local_process.refresh_from_db()
self.assertEqual(local_process.template, None)
- @expectedFailure
- def test_apply_requires_contributor_rights_on_template(self):
+ @patch("arkindex.project.mixins.get_max_level", return_value=Role.Guest.value)
+ def test_apply_requires_contributor_rights_on_template(self, get_max_level_mock):
"""Raise 403 if the user does not have rights on template
"""
- new_user = User.objects.create(email="new@test.fr", verified_email=True)
- # rights on worker of template
- self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value)
- # no rights on template
- self.private_corpus.memberships.create(user=new_user, level=Role.Guest.value)
- # rights on target process
- self.corpus.memberships.create(user=new_user, level=Role.Contributor.value)
- self.client.force_login(new_user)
- response = self.client.post(
- reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
- data=json.dumps({"process_id": str(self.process.id)}),
- content_type="application/json",
- )
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
+ data=json.dumps({"process_id": str(self.process.id)}),
+ content_type="application/json",
+ )
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(
response.json(),
{"detail": "You do not have a contributor access to this process."},
)
+ self.assertEqual(get_max_level_mock.call_count, 1)
+ self.assertEqual(get_max_level_mock.call_args, call(self.user, self.private_corpus))
- @expectedFailure
- def test_apply_requires_contributor_rights_on_process(self):
+ @patch("arkindex.project.mixins.get_max_level", side_effect=[Role.Contributor.value, Role.Guest.value])
+ def test_apply_requires_contributor_rights_on_process(self, get_max_level_mock):
"""Raise 403 if the user does not have rights on the target process
"""
- new_user = User.objects.create(email="new@test.fr", verified_email=True)
- # rights on worker of template
- self.worker_1.memberships.create(user=new_user, level=Role.Contributor.value)
- # rights on template
- self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value)
- # no rights on target process
- self.corpus.memberships.create(user=new_user, level=Role.Guest.value)
- self.client.force_login(new_user)
- response = self.client.post(
- reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
- data=json.dumps({"process_id": str(self.process.id)}),
- content_type="application/json",
- )
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(5):
+ response = self.client.post(
+ reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
+ data=json.dumps({"process_id": str(self.process.id)}),
+ content_type="application/json",
+ )
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(
response.json(),
{"detail": "You do not have a contributor access to this process."},
)
+ self.assertEqual(get_max_level_mock.call_args_list, [
+ call(self.user, self.private_corpus),
+ call(self.user, self.corpus),
+ ])
- @expectedFailure
- def test_apply_requires_access_rights_all_workers(self):
+ @patch("arkindex.project.mixins.has_access", return_value=False)
+ def test_apply_requires_access_rights_all_workers(self, has_access_mock):
"""Raise 403 if the user does not have rights on all workers concerned
"""
- new_user = User.objects.create(email="new@test.fr", verified_email=True)
- # no rights on worker of template
- self.worker_1.memberships.create(user=new_user, level=Role.Guest.value)
- # rights on template
- self.private_corpus.memberships.create(user=new_user, level=Role.Contributor.value)
- # rights on target process
- self.corpus.memberships.create(user=new_user, level=Role.Contributor.value)
- self.client.force_login(new_user)
- response = self.client.post(
- reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
- data=json.dumps({"process_id": str(self.process.id)}),
- content_type="application/json",
- )
- self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.client.force_login(self.user)
+
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse("api:apply-process-template", kwargs={"pk": str(self.private_template.id)}),
+ data=json.dumps({"process_id": str(self.process.id)}),
+ content_type="application/json",
+ )
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
self.assertDictEqual(
response.json(),
{"detail": "You do not have an execution access to this worker."},
)
+ self.assertEqual(has_access_mock.call_args_list, [
+ call(self.user, self.worker_1, Role.Contributor.value, skip_public=False),
+ call(self.user, self.worker_1.repository, Role.Contributor.value, skip_public=False),
+ ])
def test_apply_already_applied(self):
"""Raise 400 if the process already has a template attached
@@ -301,7 +308,7 @@ class TestTemplates(FixtureAPITestCase):
def test_apply(self):
self.assertIsNotNone(self.version_2.docker_image_iid)
self.client.force_login(self.user)
- with self.assertNumQueries(11):
+ with self.assertNumQueries(9):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -334,7 +341,7 @@ class TestTemplates(FixtureAPITestCase):
self.version_2.save()
self.client.force_login(self.user)
- with self.assertNumQueries(11):
+ with self.assertNumQueries(9):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -354,7 +361,7 @@ class TestTemplates(FixtureAPITestCase):
parents=[],
)
# Apply a template that has two other worker runs
- with self.assertNumQueries(13):
+ with self.assertNumQueries(11):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(process.id)}),
@@ -382,7 +389,7 @@ class TestTemplates(FixtureAPITestCase):
self.version_2.state = WorkerVersionState.Error
self.version_2.save()
self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -397,7 +404,7 @@ class TestTemplates(FixtureAPITestCase):
self.worker_2.archived = datetime.now(timezone.utc)
self.worker_2.save()
self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -413,7 +420,7 @@ class TestTemplates(FixtureAPITestCase):
self.model_version.save()
self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -431,7 +438,7 @@ class TestTemplates(FixtureAPITestCase):
self.model.save()
self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -450,7 +457,7 @@ class TestTemplates(FixtureAPITestCase):
self.process.mode = mode
self.process.save()
- with self.assertNumQueries(7):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(self.process.id)}),
@@ -468,7 +475,7 @@ class TestTemplates(FixtureAPITestCase):
self.client.force_login(self.user)
local_process = self.user.processes.get(mode=ProcessMode.Local)
- with self.assertNumQueries(6):
+ with self.assertNumQueries(5):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data=json.dumps({"process_id": str(local_process.id)}),
diff --git a/arkindex/process/tests/test_workerruns.py b/arkindex/process/tests/test_workerruns.py
index cbe72a1c97..58ce01dc83 100644
--- a/arkindex/process/tests/test_workerruns.py
+++ b/arkindex/process/tests/test_workerruns.py
@@ -1,6 +1,5 @@
import uuid
from datetime import datetime, timezone
-from unittest import expectedFailure
from unittest.mock import call, patch
from django.test import override_settings
@@ -97,6 +96,9 @@ class TestWorkerRuns(FixtureAPITestCase):
ram_total=99e9,
last_ping=datetime.now(timezone.utc),
)
+ # Add custom attributes to make the agent usable as an authenticated user
+ cls.agent.is_agent = True
+ cls.agent.is_anonymous = False
def test_list_requires_login(self):
with self.assertNumQueries(0):
@@ -964,8 +966,9 @@ class TestWorkerRuns(FixtureAPITestCase):
"""
self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent)
- self.client.force_login(self.user)
- with self.assertNumQueries(7):
+ # Agent auth is not implemented in CE
+ self.client.force_authenticate(user=self.agent)
+ with self.assertNumQueries(5):
response = self.client.get(
reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}),
)
@@ -1015,7 +1018,6 @@ class TestWorkerRuns(FixtureAPITestCase):
"summary": f"Worker Recognizer @ {str(self.version_1.id)[:6]}",
})
- @expectedFailure
def test_retrieve_agent_unassigned(self):
"""
A Ponos agent cannot retrieve a WorkerRun on a process where it does not have any assigned tasks
@@ -1023,8 +1025,8 @@ class TestWorkerRuns(FixtureAPITestCase):
self.process_1.tasks.create(run=0, depth=0, slug="something", agent=None)
# Agent auth is not implemented in CE
- self.client.force_login(self.user)
- with self.assertNumQueries(3):
+ self.client.force_authenticate(user=self.agent)
+ with self.assertNumQueries(1):
response = self.client.get(
reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}),
)
@@ -2007,7 +2009,6 @@ class TestWorkerRuns(FixtureAPITestCase):
"__all__": ["A WorkerRun already exists on this process with the selected worker version, model version and configuration."],
})
- @expectedFailure
def test_update_agent(self):
"""
Ponos agents cannot update WorkerRuns, even when they can access them
@@ -2015,8 +2016,8 @@ class TestWorkerRuns(FixtureAPITestCase):
self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent)
# Agent auth is not implemented in CE
- self.client.force_login(self.user)
- with self.assertNumQueries(4):
+ self.client.force_authenticate(user=self.agent)
+ with self.assertNumQueries(2):
response = self.client.put(
reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}),
)
@@ -2932,7 +2933,6 @@ class TestWorkerRuns(FixtureAPITestCase):
"__all__": ["A WorkerRun already exists on this process with the selected worker version, model version and configuration."],
})
- @expectedFailure
def test_partial_update_agent(self):
"""
Ponos agents cannot update WorkerRuns, even when they can access them
@@ -2940,8 +2940,8 @@ class TestWorkerRuns(FixtureAPITestCase):
self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent)
# Agent auth is not implemented in CE
- self.client.force_login(self.user)
- with self.assertNumQueries(4):
+ self.client.force_authenticate(user=self.agent)
+ with self.assertNumQueries(2):
response = self.client.patch(
reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}),
)
@@ -3071,7 +3071,6 @@ class TestWorkerRuns(FixtureAPITestCase):
self.assertEqual(response.json(), ["WorkerRuns cannot be deleted from a process that has already started."])
- @expectedFailure
def test_delete_agent(self):
"""
Ponos agents cannot delete WorkerRuns, even when they can access them
@@ -3079,8 +3078,8 @@ class TestWorkerRuns(FixtureAPITestCase):
self.process_1.tasks.create(run=0, depth=0, slug="something", agent=self.agent)
# Agent auth is not implemented in CE
- self.client.force_login(self.user)
- with self.assertNumQueries(3):
+ self.client.force_authenticate(user=self.agent)
+ with self.assertNumQueries(1):
response = self.client.delete(
reverse("api:worker-run-details", kwargs={"pk": str(self.run_1.id)}),
)
diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py
index 99ecfc1c3e..6f04942aa2 100644
--- a/arkindex/project/mixins.py
+++ b/arkindex/project/mixins.py
@@ -8,8 +8,7 @@ from rest_framework.exceptions import APIException, PermissionDenied
from rest_framework.serializers import CharField, Serializer
from arkindex.documents.models import Corpus
-from arkindex.process.models import Process, Repository, Worker
-from arkindex.training.models import Model
+from arkindex.process.models import Process, Repository
from arkindex.users.models import Role
from arkindex.users.utils import filter_rights, get_max_level, has_access
@@ -62,25 +61,6 @@ class WorkerACLMixin(ACLMixin):
A public worker is considered as executable (i.e. any user has a contributor access)
"""
- @property
- def executable_workers(self):
- return Worker.objects.filter(
- Q(public=True)
- | Q(id__in=filter_rights(self.user, Worker, Role.Contributor.value).values("id"))
- | Q(repository_id__in=filter_rights(self.user, Repository, Role.Contributor.value).values("id"))
- ).distinct()
-
- def get_max_level(self, worker):
- # Access right on a worker can be defined by a right on its repository
- worker_level = get_max_level(self.user, worker)
- if not worker.repository:
- return worker_level
- repo_level = get_max_level(self.user, worker.repository)
- return max(
- filter(None, (worker_level, repo_level)),
- default=None
- )
-
def has_worker_access(self, worker, level):
if worker.public and level <= Role.Contributor.value:
return True
@@ -113,18 +93,6 @@ class CorpusACLMixin(ACLMixin):
raise PermissionDenied(detail=f"You do not have {str(role).lower()} access to this corpus.")
return corpus
- @property
- def readable_corpora(self):
- return Corpus.objects.filter(
- id__in=filter_rights(self.user, Corpus, Role.Guest.value).values("id")
- )
-
- @property
- def writable_corpora(self):
- return Corpus.objects.filter(
- id__in=filter_rights(self.user, Corpus, Role.Contributor.value).values("id")
- )
-
def has_read_access(self, corpus):
return self.has_access(corpus, Role.Guest.value)
@@ -140,18 +108,6 @@ class TrainingModelMixin(ACLMixin):
Access control mixin for machine learning models
"""
- @property
- def readable_models(self):
- return Model.objects.filter(
- id__in=filter_rights(self.user, Model, Role.Guest.value).values("id")
- )
-
- @property
- def editable_models(self):
- return Model.objects.filter(
- id__in=filter_rights(self.user, Model, Role.Contributor.value).values("id")
- )
-
def has_read_access(self, model):
return self.has_access(model, Role.Guest.value)
diff --git a/arkindex/project/tests/test_acl_mixin.py b/arkindex/project/tests/test_acl_mixin.py
index 64248aaaa4..8d84f0d1d5 100644
--- a/arkindex/project/tests/test_acl_mixin.py
+++ b/arkindex/project/tests/test_acl_mixin.py
@@ -166,58 +166,6 @@ class TestACLMixin(FixtureTestCase):
self.assertEqual(admin_access, access_check)
ContentType.objects.clear_cache()
- @expectedFailure
- def test_corpus_acl_mixin_writable(self):
- corpus_acl_mixin = CorpusACLMixin(user=self.user1)
- with self.assertNumQueries(1):
- corpora = list(corpus_acl_mixin.writable_corpora)
- self.assertCountEqual(
- list(corpora),
- [self.corpus1]
- )
-
- def test_corpus_readable_orderable(self):
- # Assert corpora retrieved via the mixin are still orderable
- corpus_acl_mixin = CorpusACLMixin(user=self.user3)
- with self.assertNumQueries(1):
- corpora = list(corpus_acl_mixin.readable_corpora.order_by("name"))
- self.assertListEqual(
- [c.name for c in corpora],
- ["Corpus1", "Corpus2", "Unit Tests"]
- )
-
- def test_super_admin_readable_corpora(self):
- # A super admin should retrieve all existing corpora with Admin rights
- corpus_acl_mixin = CorpusACLMixin(user=self.superuser)
- with self.assertNumQueries(1):
- corpora = list(corpus_acl_mixin.readable_corpora)
- self.assertCountEqual(
- list(corpora),
- list(Corpus.objects.all())
- )
-
- @expectedFailure
- def test_anonymous_user_readable_corpora(self):
- # An anonymous user should have guest access to any public corpora
- corpus_acl_mixin = CorpusACLMixin(user=AnonymousUser())
- with self.assertNumQueries(1):
- corpora = list(corpus_acl_mixin.readable_corpora)
- self.assertCountEqual(
- list(corpora),
- list(Corpus.objects.filter(public=True))
- )
-
- def test_corpus_right_and_public(self):
- # User specific rights should be returned instead of the the defaults access for public rights
- Right.objects.create(user=self.user3, content_object=self.corpus, level=42)
- corpus_acl_mixin = CorpusACLMixin(user=self.user3)
- with self.assertNumQueries(1):
- corpora = list(corpus_acl_mixin.readable_corpora)
- self.assertCountEqual(
- list(corpora),
- [self.corpus1, self.corpus2, self.corpus]
- )
-
@expectedFailure
def test_max_level_does_not_exists(self):
with self.assertNumQueries(3):
@@ -306,22 +254,3 @@ class TestACLMixin(FixtureTestCase):
with self.assertNumQueries(0):
admin_access = TrainingModelMixin(user=self.user5).has_admin_access(self.model2)
self.assertTrue(admin_access)
-
- @expectedFailure
- def test_models_readable(self):
- """
- To view a model, a user needs guest access.
- """
- with self.assertNumQueries(1):
- readable_models = list(TrainingModelMixin(user=self.user4).readable_models)
- self.assertListEqual(readable_models, [self.model1])
-
- @expectedFailure
- def test_models_editable(self):
- """
- To edit a model, a user needs contributor access.
- User5 only has that access on model2.
- """
- with self.assertNumQueries(1):
- editable_models = list(TrainingModelMixin(user=self.user5).editable_models)
- self.assertListEqual(editable_models, [self.model2])
--
GitLab