From 6e3576523ee7652a64b092499c3ae6fe1c029f09 Mon Sep 17 00:00:00 2001 From: ml bonhomme <bonhomme@teklia.com> Date: Tue, 9 Jul 2024 09:59:07 +0000 Subject: [PATCH] Remove worker_version from Entity API --- arkindex/documents/api/entities.py | 3 +- arkindex/documents/serializers/entities.py | 27 ---- arkindex/documents/tests/test_entities_api.py | 121 ++++++++++++------ arkindex/documents/tests/test_metadata.py | 7 +- 4 files changed, 86 insertions(+), 72 deletions(-) diff --git a/arkindex/documents/api/entities.py b/arkindex/documents/api/entities.py index a4886d8cf6..68234eeb51 100644 --- a/arkindex/documents/api/entities.py +++ b/arkindex/documents/api/entities.py @@ -132,6 +132,7 @@ class EntityDetails(ACLMixin, RetrieveUpdateDestroyAPIView): return Entity.objects \ .select_related("corpus", "type") \ .filter(corpus__in=Corpus.objects.readable(self.request.user)) \ + .select_related("worker_run") \ .prefetch_related( "corpus", ) @@ -379,7 +380,7 @@ class TranscriptionEntities(ListAPIView): **filters, ) .order_by("offset") - .select_related("entity__type", "worker_run") + .select_related("entity__type", "entity__worker_run", "worker_run") ) diff --git a/arkindex/documents/serializers/entities.py b/arkindex/documents/serializers/entities.py index 66c06cc414..2b5d3fc2be 100644 --- a/arkindex/documents/serializers/entities.py +++ b/arkindex/documents/serializers/entities.py @@ -2,7 +2,6 @@ from collections import defaultdict from textwrap import dedent from django.db import transaction -from drf_spectacular.utils import extend_schema_serializer from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -10,7 +9,6 @@ from arkindex.documents.models import Corpus, Entity, EntityType, TranscriptionE from arkindex.documents.serializers.light import CorpusLightSerializer, EntityTypeLightSerializer from arkindex.documents.serializers.ml import WorkerRunSummarySerializer from arkindex.project.serializer_fields import WorkerRunIDField -from arkindex.project.validators import ForbiddenValidator class EntityTypeSerializer(serializers.ModelSerializer): @@ -56,15 +54,12 @@ class EntityTypeCreateSerializer(EntityTypeSerializer): return self.fields["corpus"].queryset = Corpus.objects.admin(self.context["request"].user) - -@extend_schema_serializer(deprecate_fields=("worker_version_id", )) class BaseEntitySerializer(serializers.ModelSerializer): """ Serializes an entity """ type = EntityTypeLightSerializer() metas = serializers.HStoreField(child=serializers.CharField(), required=False, allow_null=True) - worker_version_id = serializers.PrimaryKeyRelatedField(read_only=True) worker_run = WorkerRunSummarySerializer(read_only=True, allow_null=True) class Meta: @@ -75,19 +70,14 @@ class BaseEntitySerializer(serializers.ModelSerializer): "type", "metas", "validated", - "worker_version_id", "worker_run", ) read_only_fields = ( "id", - "worker_version_id", "worker_run", ) -@extend_schema_serializer( - deprecate_fields=("worker_version_id", ) -) class EntitySerializer(BaseEntitySerializer): """ Serialize an entity with its metadata @@ -129,9 +119,6 @@ class EntitySerializer(BaseEntitySerializer): return data -@extend_schema_serializer( - deprecate_fields=("worker_version", "worker_version_id") -) class EntityCreateSerializer(BaseEntitySerializer): """ Serialize an entity with possible parents and children @@ -141,19 +128,6 @@ class EntityCreateSerializer(BaseEntitySerializer): style={"base_template": "input.html"}, ) metas = serializers.HStoreField(child=serializers.CharField(), required=False) - worker_version = serializers.UUIDField( - allow_null=True, - required=False, - source="worker_version_id", - validators=[ - ForbiddenValidator(), - ], - help_text=dedent(""" - ID of a WorkerVersion that created this entity. - - Creating new entities with a WorkerVersion is forbidden. Use `worker_run_id` instead. - """), - ) worker_run_id = WorkerRunIDField( default=None, help_text=dedent(""" @@ -180,7 +154,6 @@ class EntityCreateSerializer(BaseEntitySerializer): "metas", "validated", "corpus", - "worker_version", "worker_run_id" ) read_only_fields = ( diff --git a/arkindex/documents/tests/test_entities_api.py b/arkindex/documents/tests/test_entities_api.py index f0481fcd9a..70a84274e6 100644 --- a/arkindex/documents/tests/test_entities_api.py +++ b/arkindex/documents/tests/test_entities_api.py @@ -38,12 +38,14 @@ class TestEntitiesAPI(FixtureAPITestCase): corpus=self.corpus, name="entity 1", worker_version=self.worker_version_1, + worker_run=self.worker_run_1, ) self.entity_bis = Entity.objects.create( type=self.location_type, corpus=self.corpus, name="entity 2", worker_version=self.worker_version_2, + worker_run=self.worker_run_2, ) self.element = self.corpus.elements.create( type=self.element_type, @@ -51,10 +53,12 @@ class TestEntitiesAPI(FixtureAPITestCase): image=self.page.image, polygon=LinearRing((0, 0), (0, 42), (42, 42), (42, 0), (0, 0)), worker_version=self.worker_version_1, + worker_run=self.worker_run_1, ) self.transcription = self.element.transcriptions.create( text="Some transcribed text", worker_version=self.worker_version_1, + worker_run=self.worker_run_1, ) self.metadata = self.element.metadatas.create( name="test 1", @@ -68,7 +72,8 @@ class TestEntitiesAPI(FixtureAPITestCase): entity=self.entity_bis, offset=2, length=len(self.entity_bis.name), - worker_version=self.worker_version_1 + worker_version=self.worker_version_1, + worker_run=self.worker_run_1, ) self.tr_entities_sample = { "entity": str(self.entity.id), @@ -125,7 +130,7 @@ class TestEntitiesAPI(FixtureAPITestCase): image=self.imgsrv.images.first(), polygon=LinearRing((0, 0), (0, 200), (200, 200), (200, 0), (0, 0)), ) - elt_tr = elt.transcriptions.create(worker_version=self.worker_version_1, text="goodbye") + elt_tr = elt.transcriptions.create(worker_version=self.worker_version_1, worker_run=self.worker_run_1, text="goodbye") TranscriptionEntity.objects.create(transcription=elt_tr, entity=self.entity, offset=42, length=7) with self.assertNumQueries(6): @@ -253,9 +258,9 @@ class TestEntitiesAPI(FixtureAPITestCase): response = self.client.post(reverse("api:entity-create"), data=data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_create_entity_no_worker_version(self): + def test_create_entity_worker_version_ignored(self): """ - Only a worker run can be used to create an entity, not a worker version + It is not possible to create an entity with a worker version """ data = { "name": "entity", @@ -268,12 +273,19 @@ class TestEntitiesAPI(FixtureAPITestCase): }, } self.client.force_login(self.user) - with self.assertNumQueries(4): + with self.assertNumQueries(5): response = self.client.post(reverse("api:entity-create"), data=data, format="json") - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), { - "worker_version": ["This field is forbidden."], - }) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + data = response.json() + entity = Entity.objects.get(id=data["id"]) + self.assertEqual(entity.name, "entity") + self.assertEqual(entity.type, self.person_type) + self.assertEqual(entity.corpus, self.corpus) + self.assertFalse(entity.validated) + self.assertEqual(entity.metas, {"key": "value", "other key": "other value"}) + self.assertEqual(entity.worker_version_id, None) + self.assertEqual(entity.worker_run, None) def test_create_entity_with_unknown_worker_run(self): data = { @@ -455,8 +467,7 @@ class TestEntitiesAPI(FixtureAPITestCase): "worker_run": { "id": str(self.local_worker_run.id), "summary": self.local_worker_run.summary, - }, - "worker_version_id": str(self.local_worker_run.version_id), + } }) def test_create_entity_task_auth(self): @@ -1033,12 +1044,17 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": self.entity_bis.validated, - "worker_version_id": str(self.worker_version_2.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_2.id), + "summary": self.worker_run_2.summary + }, }, "length": self.transcriptionentity.length, "offset": self.transcriptionentity.offset, - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, "confidence": None }] ) @@ -1063,12 +1079,17 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": self.entity_bis.validated, - "worker_version_id": str(self.worker_version_2.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_2.id), + "summary": self.worker_run_2.summary + }, }, "length": self.transcriptionentity.length, "offset": self.transcriptionentity.offset, - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, "confidence": None }], ) @@ -1185,8 +1206,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, }, "length": 8, "offset": 8, @@ -1224,8 +1247,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, }, "length": 8, "offset": 8, @@ -1248,7 +1273,7 @@ class TestEntitiesAPI(FixtureAPITestCase): with self.assertNumQueries(5): response = self.client.get( reverse("api:transcription-entities", kwargs={"pk": str(self.transcription.id)}), - {"worker_version": False} + {"worker_run": False} ) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertListEqual( @@ -1264,8 +1289,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, }, "length": 8, "offset": 2, @@ -1302,8 +1329,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, }, "length": 8, "offset": 8, @@ -1328,7 +1357,7 @@ class TestEntitiesAPI(FixtureAPITestCase): worker_run=self.worker_run_2, ) self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(6): response = self.client.get( reverse("api:transcription-entities", kwargs={"pk": str(self.transcription.id)}), {"entity_worker_run": str(self.worker_run_1.id)} @@ -1347,7 +1376,6 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), "worker_run": { "id": str(self.worker_run_1.id), "summary": self.worker_run_1.summary, @@ -1396,7 +1424,6 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": None, "worker_run": None, }, "length": 13, @@ -1444,8 +1471,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "metas": None, "validated": False, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary + }, }, "length": 8, "offset": 8, @@ -1455,7 +1484,7 @@ class TestEntitiesAPI(FixtureAPITestCase): ) def test_list_corpus_entities(self): - with self.assertNumQueries(3): + with self.assertNumQueries(5): response = self.client.get(reverse("api:corpus-entities", kwargs={"pk": str(self.corpus.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -1475,8 +1504,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "validated": False, "metas": None, - "worker_version_id": str(self.worker_version_1.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_1.id), + "summary": self.worker_run_1.summary, + }, }, { "id": str(self.entity_bis.id), @@ -1488,8 +1519,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "validated": False, "metas": None, - "worker_version_id": str(self.worker_version_2.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_2.id), + "summary": self.worker_run_2.summary, + }, } ] }) @@ -1504,7 +1537,7 @@ class TestEntitiesAPI(FixtureAPITestCase): self.assertEqual(has_access_mock.call_args, call(AnonymousUser(), self.private_corpus, Role.Guest.value, skip_public=False)) def test_list_corpus_entities_parent(self): - with self.assertNumQueries(4): + with self.assertNumQueries(5): response = self.client.get( reverse("api:corpus-entities", kwargs={"pk": str(self.corpus.id)}), {"parent": str(self.element.id)}, @@ -1526,8 +1559,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "validated": False, "metas": None, - "worker_version_id": str(self.worker_version_2.id), - "worker_run": None + "worker_run": { + "id": str(self.worker_run_2.id), + "summary": self.worker_run_2.summary, + }, }, ] }) @@ -1561,7 +1596,7 @@ class TestEntitiesAPI(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_list_corpus_entities_name(self): - with self.assertNumQueries(3): + with self.assertNumQueries(4): response = self.client.get(reverse("api:corpus-entities", kwargs={"pk": str(self.corpus.id)}), {"name": "2"}) self.assertEqual(response.status_code, status.HTTP_200_OK) data = response.json() @@ -1581,8 +1616,10 @@ class TestEntitiesAPI(FixtureAPITestCase): }, "validated": False, "metas": None, - "worker_version_id": str(self.worker_version_2.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run_2.id), + "summary": self.worker_run_2.summary, + }, } ] }) diff --git a/arkindex/documents/tests/test_metadata.py b/arkindex/documents/tests/test_metadata.py index 569a906247..cff728b7e9 100644 --- a/arkindex/documents/tests/test_metadata.py +++ b/arkindex/documents/tests/test_metadata.py @@ -85,6 +85,7 @@ class TestMetaData(FixtureAPITestCase): type=self.person_type, name="Marc", worker_version=self.worker_version, + worker_run=self.worker_run ) entity_meta = self.vol.metadatas.create( name="meta", @@ -122,8 +123,10 @@ class TestMetaData(FixtureAPITestCase): "color": "ff0000" }, "validated": False, - "worker_version_id": str(self.worker_version.id), - "worker_run": None, + "worker_run": { + "id": str(self.worker_run.id), + "summary": self.worker_run.summary + }, }, "worker_run": None, }, -- GitLab