diff --git a/arkindex/process/api.py b/arkindex/process/api.py index bfd5d73fdc1e0ae9b8a4f33f9e52d10194f30d3c..e2dd6d82b76ec8f1555f48bae5f4ccc4b0318398 100644 --- a/arkindex/process/api.py +++ b/arkindex/process/api.py @@ -105,6 +105,7 @@ from arkindex.process.serializers.workers import ( WorkerConfigurationExistsErrorSerializer, WorkerConfigurationListSerializer, WorkerConfigurationSerializer, + WorkerCreateSerializer, WorkerSerializer, WorkerStatisticsSerializer, WorkerTypeSerializer, @@ -980,7 +981,7 @@ class RevisionRetrieve(RepositoryACLMixin, RetrieveAPIView): ) class WorkerList(ListCreateAPIView): permission_classes = (IsVerified, ) - serializer_class = WorkerSerializer + serializer_class = WorkerCreateSerializer queryset = Worker.objects.none() def filter_queryset(self, qs): @@ -1210,8 +1211,22 @@ class WorkerVersionRetrieve(RetrieveUpdateAPIView): Requires a contributor access to the worker or its repository, except for `public` workers. """), ), + put=extend_schema( + description=dedent(""" + Update a worker. + + Requires a contributor access to the worker or its repository. + """), + ), + patch=extend_schema( + description=dedent(""" + Partially update a worker. + + Requires a contributor access to the worker or its repository. + """), + ), ) -class WorkerRetrieve(RetrieveAPIView): +class WorkerRetrieve(RetrieveUpdateAPIView): permission_classes = (IsVerified, ) serializer_class = WorkerSerializer queryset = Worker.objects.none() @@ -1219,6 +1234,10 @@ class WorkerRetrieve(RetrieveAPIView): def get_queryset(self): return Worker.objects.executable(self.request.user).select_related('repository', 'type') + def check_object_permissions(self, request, worker): + if self.request.method not in permissions.SAFE_METHODS and not worker.is_editable(self.request.user): + raise PermissionDenied(detail='You do not have a contributor access to this worker.') + @extend_schema(tags=['repos']) @extend_schema_view( diff --git a/arkindex/process/serializers/workers.py b/arkindex/process/serializers/workers.py index 7ef377036bb0103e889a249b6cd3a73edc99d61a..db9f41c4a9fdf10835ac1a5b2a85ade95c317e47 100644 --- a/arkindex/process/serializers/workers.py +++ b/arkindex/process/serializers/workers.py @@ -42,17 +42,47 @@ class ExistingWorkerException(Exception): self.worker = worker -class WorkerSerializer(serializers.ModelSerializer): - """ - Serialize a simple repository worker - """ - # We want to only have to specify the type as a slug (char) in stead of the actual workerType id - type = serializers.SlugField() - repository_id = serializers.UUIDField(read_only=True, allow_null=True) +class WorkerLightSerializer(serializers.ModelSerializer): + type = serializers.SlugRelatedField( + slug_field='slug', + queryset=WorkerType.objects.all(), + ) class Meta: model = Worker - fields = ('id', 'name', 'type', 'slug', 'repository_id') + fields = ('id', 'name', 'type', 'slug') + + +class WorkerSerializer(WorkerLightSerializer): + + class Meta(WorkerLightSerializer.Meta): + fields = ('id', 'name', 'description', 'type', 'slug', 'repository_id') + read_only_fields = ('id', 'repository_id') + extra_kwargs = { + 'description': { + 'style': {'base_template': 'textarea.html'}, + }, + } + validators = [ + serializers.UniqueTogetherValidator( + queryset=Worker.objects.all(), + fields=['repository_id', 'slug'], + message='A worker with the same repository and slug already exists.', + ), + ] + + +class WorkerCreateSerializer(WorkerSerializer): + # This cannot use a SlugRelatedField because of some custom handling to automatically create a WorkerType + type = serializers.SlugField(help_text=dedent(""" + Slug of a WorkerType to set on this worker. + + If the WorkerType does not exist, it is created automatically. + """)) + + class Meta(WorkerSerializer.Meta): + # Remove the validator for unique slugs/repos since we do custom handling + validators = [] def create(self, validated_data): """ @@ -108,15 +138,6 @@ class WorkerSerializer(serializers.ModelSerializer): return worker -class WorkerLightSerializer(WorkerSerializer): - """ - Serialize a worker without its repository ID - """ - class Meta: - model = Worker - fields = ('id', 'name', 'type', 'slug') - - class WorkerTypeSerializer(serializers.ModelSerializer): class Meta: model = WorkerType @@ -351,7 +372,7 @@ class RepositorySerializer(serializers.ModelSerializer): """ enabled = serializers.BooleanField(read_only=True) git_clone_url = serializers.SerializerMethodField() - workers = WorkerSerializer(many=True, read_only=True) + workers = WorkerLightSerializer(many=True, read_only=True) authorized_users = serializers.SerializerMethodField(read_only=True) @extend_schema_field(serializers.CharField(allow_null=True)) diff --git a/arkindex/process/tests/test_repos.py b/arkindex/process/tests/test_repos.py index 75b3b6ceb109f66a35630348953aabb178aee914..57c84d4d6d8701de8bda32a70dcb210fbc3c5e03 100644 --- a/arkindex/process/tests/test_repos.py +++ b/arkindex/process/tests/test_repos.py @@ -51,7 +51,6 @@ class TestRepositories(FixtureTestCase): 'name': w.name, 'type': w.type.slug, 'slug': w.slug, - 'repository_id': str(self.repo.id), } for w in self.repo.workers.all() ], 'authorized_users': self.repo.memberships.count(), diff --git a/arkindex/process/tests/test_workers.py b/arkindex/process/tests/test_workers.py index 9ffb00d347d896fa6117b431a37dc01927f9d50b..18fa7aa64dfafd62ac07e69227255aff6a64b8ff 100644 --- a/arkindex/process/tests/test_workers.py +++ b/arkindex/process/tests/test_workers.py @@ -104,6 +104,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_custom.id), 'repository_id': None, 'name': 'Custom worker', + 'description': '', 'slug': 'custom', 'type': 'custom', }, @@ -111,6 +112,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_dla.id), 'repository_id': str(self.repo.id), 'name': 'Document layout analyser', + 'description': '', 'slug': 'dla', 'type': 'dla', }, @@ -118,6 +120,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_file_import.id), 'repository_id': str(self.repo.id), 'name': 'File import', + 'description': '', 'slug': 'file_import', 'type': 'import', }, @@ -125,6 +128,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_generic.id), 'repository_id': str(self.repo.id), 'name': 'Generic worker with a Model', + 'description': '', 'slug': 'generic', 'type': 'recognizer', }, @@ -132,6 +136,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_reco.id), 'repository_id': str(self.repo.id), 'name': 'Recognizer', + 'description': '', 'slug': 'reco', 'type': 'recognizer', }, @@ -139,6 +144,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_gpu.id), 'repository_id': str(self.repo.id), 'name': 'Worker requiring a GPU', + 'description': '', 'slug': 'worker-gpu', 'type': 'worker', }, @@ -163,6 +169,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_dla.id), 'repository_id': str(self.repo.id), 'name': 'Document layout analyser', + 'description': '', 'slug': 'dla', 'type': 'dla', } @@ -197,6 +204,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_dla.id), 'repository_id': str(self.repo.id), 'name': 'Document layout analyser', + 'description': '', 'slug': 'dla', 'type': 'dla', } @@ -243,6 +251,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_reco.id), 'repository_id': str(self.repo.id), 'name': 'Recognizer', + 'description': '', 'slug': 'reco', 'type': 'recognizer', }] @@ -263,6 +272,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(self.worker_dla.id), 'repository_id': str(self.worker_dla.repository.id), 'name': 'Document layout analyser', + 'description': '', 'slug': 'dla', 'type': 'dla', } @@ -296,6 +306,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): 'id': str(worker_2.id), 'repository_id': str(repo2.id), 'name': 'Worker 2', + 'description': '', 'slug': 'worker_2', 'type': self.worker_type_dla.slug } @@ -325,9 +336,10 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): self.assertDictEqual(response.json(), { 'id': str(worker_2.id), 'repository_id': str(repo2.id), - 'name': worker_2.name, - 'slug': worker_2.slug, - 'type': worker_2.type.slug, + 'name': 'Worker 2', + 'description': '', + 'slug': 'worker_2', + 'type': 'dla', }) def test_workers_retrieve_no_revision(self): @@ -338,9 +350,10 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): self.assertDictEqual(response.json(), { 'id': str(self.worker_custom.id), 'repository_id': None, - 'name': self.worker_custom.name, - 'slug': self.worker_custom.slug, - 'type': self.worker_custom.type.slug, + 'name': 'Custom worker', + 'description': '', + 'slug': 'custom', + 'type': 'custom', }) def test_workers_retrieve_no_rights(self): @@ -434,6 +447,7 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): self.assertDictEqual(data, { 'id': str(worker.id), 'name': 'Worker post', + 'description': '', 'repository_id': None, 'slug': 'worker_post', 'type': 'dla', @@ -554,6 +568,266 @@ class TestWorkersWorkerVersions(FixtureAPITestCase): self.assertEqual(data['slug'], 'worker_post') self.assertEqual(data['type'], 'newType') + def test_update_requires_login(self): + with self.assertNumQueries(0): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json(), {'detail': 'Authentication credentials were not provided.'}) + + def test_update_requires_verified(self): + self.user.verified_email = False + self.user.save() + self.client.force_login(self.user) + + with self.assertNumQueries(2): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json(), {'detail': 'You do not have permission to perform this action.'}) + + def test_update_requires_contributor(self): + self.worker_reco.memberships.update_or_create(user=self.user, defaults={'level': Role.Guest.value}) + self.repo.memberships.update_or_create(user=self.user, defaults={'level': Role.Guest.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'Not found.'}) + + def test_update(self): + self.repo.memberships.filter(user=self.user).delete() + self.worker_reco.memberships.update_or_create(user=self.user, defaults={'level': Role.Contributor.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(8): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { + 'id': str(self.worker_reco.id), + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + 'repository_id': str(self.repo.id), + }) + + self.worker_reco.refresh_from_db() + self.assertEqual(self.worker_reco.name, 'New name') + self.assertEqual(self.worker_reco.slug, 'new_slug') + self.assertEqual(self.worker_reco.description, 'New description') + self.assertEqual(self.worker_reco.type, self.worker_type_dla) + self.assertEqual(self.worker_reco.repository_id, self.repo.id) + + def test_update_repository_contributor(self): + self.worker_reco.memberships.filter(user=self.user).delete() + self.repo.memberships.update_or_create(user=self.user, defaults={'level': Role.Contributor.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(9): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { + 'id': str(self.worker_reco.id), + 'name': 'New name', + 'slug': 'new_slug', + 'description': 'New description', + 'type': 'dla', + 'repository_id': str(self.repo.id), + }) + + self.worker_reco.refresh_from_db() + self.assertEqual(self.worker_reco.name, 'New name') + self.assertEqual(self.worker_reco.slug, 'new_slug') + self.assertEqual(self.worker_reco.description, 'New description') + self.assertEqual(self.worker_reco.type, self.worker_type_dla) + self.assertEqual(self.worker_reco.repository_id, self.repo.id) + + def test_update_unique_slug(self): + self.client.force_login(self.user) + + with self.assertNumQueries(10): + response = self.client.put( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'dla', + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), { + 'non_field_errors': ['A worker with the same repository and slug already exists.'], + }) + + def test_partial_update_requires_login(self): + with self.assertNumQueries(0): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'description': 'New description', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json(), {'detail': 'Authentication credentials were not provided.'}) + + def test_partial_update_requires_verified(self): + self.user.verified_email = False + self.user.save() + self.client.force_login(self.user) + + with self.assertNumQueries(2): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'description': 'New description', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.json(), {'detail': 'You do not have permission to perform this action.'}) + + def test_partial_update_requires_contributor(self): + self.worker_reco.memberships.update_or_create(user=self.user, defaults={'level': Role.Guest.value}) + self.repo.memberships.update_or_create(user=self.user, defaults={'level': Role.Guest.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(4): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'description': 'New description', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'Not found.'}) + + def test_partial_update(self): + self.repo.memberships.filter(user=self.user).delete() + self.worker_reco.memberships.update_or_create(user=self.user, defaults={'level': Role.Contributor.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(8): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'description': 'New description', + 'type': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { + 'id': str(self.worker_reco.id), + 'name': 'Recognizer', + 'slug': 'reco', + 'description': 'New description', + 'type': 'dla', + 'repository_id': str(self.repo.id), + }) + + self.worker_reco.refresh_from_db() + self.assertEqual(self.worker_reco.name, 'Recognizer') + self.assertEqual(self.worker_reco.slug, 'reco') + self.assertEqual(self.worker_reco.description, 'New description') + self.assertEqual(self.worker_reco.type, self.worker_type_dla) + self.assertEqual(self.worker_reco.repository_id, self.repo.id) + + def test_partial_update_repository_contributor(self): + self.worker_reco.memberships.filter(user=self.user).delete() + self.repo.memberships.update_or_create(user=self.user, defaults={'level': Role.Contributor.value}) + self.client.force_login(self.user) + + with self.assertNumQueries(8): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'name': 'New name', + 'slug': 'new_slug', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { + 'id': str(self.worker_reco.id), + 'name': 'New name', + 'slug': 'new_slug', + 'description': '', + 'type': 'recognizer', + 'repository_id': str(self.repo.id), + }) + + self.worker_reco.refresh_from_db() + self.assertEqual(self.worker_reco.name, 'New name') + self.assertEqual(self.worker_reco.slug, 'new_slug') + self.assertEqual(self.worker_reco.description, '') + self.assertEqual(self.worker_reco.type.slug, 'recognizer') + self.assertEqual(self.worker_reco.repository_id, self.repo.id) + + def test_partial_update_unique_slug(self): + self.client.force_login(self.user) + + with self.assertNumQueries(9): + response = self.client.patch( + reverse('api:worker-retrieve', kwargs={'pk': str(self.worker_reco.id)}), + { + 'slug': 'dla', + }, + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.json(), { + 'non_field_errors': ['A worker with the same repository and slug already exists.'], + }) + # Tests on get_query_set for WorkerVersionList def test_versions_list_requires_login(self): with self.assertNumQueries(0):