From 38256e002185e074f41fb34f0ecb8e39b51fd69f Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Wed, 14 Dec 2022 13:49:12 +0000
Subject: [PATCH] Sort ListCorpusWorkerVersions by worker name and revision
 hash

---
 arkindex/process/api.py                       |  21 +-
 .../process/tests/test_transkribus_import.py  |   2 +-
 arkindex/process/tests/test_workers.py        | 284 ++++++++++++++----
 3 files changed, 237 insertions(+), 70 deletions(-)

diff --git a/arkindex/process/api.py b/arkindex/process/api.py
index e4071f2722..193808d4c9 100644
--- a/arkindex/process/api.py
+++ b/arkindex/process/api.py
@@ -982,7 +982,6 @@ class CorpusWorkerVersionList(CorpusACLMixin, ListAPIView):
 
     Guest access is required on private corpora. No specific rights are required on the workers.
     """
-    pagination_class = CustomCursorPagination
     permission_classes = (IsVerifiedOrReadOnly, )
     serializer_class = WorkerVersionSerializer
     # For OpenAPI type discovery
@@ -993,10 +992,22 @@ class CorpusWorkerVersionList(CorpusACLMixin, ListAPIView):
         return get_object_or_404(self.readable_corpora, pk=self.kwargs['pk'])
 
     def get_queryset(self):
-        # This queryset does not have any ordering because the CustomCursorPagination always overrides it with .order_by('id')
-        return self.corpus.worker_versions.prefetch_related(
-            Prefetch('revision', queryset=Revision.objects.select_related('repo').prefetch_related('refs')),
-            Prefetch('worker', queryset=Worker.objects.select_related('repository', 'type')),
+        return (
+            self.corpus.worker_versions
+            .select_related(
+                'worker',
+                'revision',
+            )
+            .order_by(
+                'worker__name',
+                'revision__hash',
+            )
+            .prefetch_related(
+                'revision__repo',
+                'revision__refs',
+                'worker__repository',
+                'worker__type',
+            )
         )
 
 
diff --git a/arkindex/process/tests/test_transkribus_import.py b/arkindex/process/tests/test_transkribus_import.py
index d542796bd9..0da526005b 100644
--- a/arkindex/process/tests/test_transkribus_import.py
+++ b/arkindex/process/tests/test_transkribus_import.py
@@ -177,7 +177,7 @@ class TestTranskribusImport(FixtureAPITestCase):
         process = Process.objects.get(id=data["id"])
         self.assertEqual(process.mode, ProcessMode.Transkribus)
         corpus = process.corpus
-        with self.assertNumQueries(8):
+        with self.assertNumQueries(10):
             response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': corpus.id}))
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
diff --git a/arkindex/process/tests/test_workers.py b/arkindex/process/tests/test_workers.py
index 73694c0f2c..fddb349704 100644
--- a/arkindex/process/tests/test_workers.py
+++ b/arkindex/process/tests/test_workers.py
@@ -869,7 +869,6 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
 
     def test_invalid_user_configuration_list_requires_subtype(self):
         self.client.force_login(self.internal_user)
-        self.maxDiff = None
         response = self.client.post(
             reverse("api:worker-versions", kwargs={"pk": str(self.worker_2.id)}),
             data={
@@ -1498,53 +1497,72 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         self.version_1.configuration = {'docker': {'shm_size': '32b'}}
         self.assertEqual(self.version_1.docker_shm_size, '32b')
 
-    def _serialize_worker_version(self, version):
-        return {
-            'id': str(version.id),
-            'configuration': {'test': 42},
-            'revision': {
-                'id': str(version.revision_id),
-                'hash': '1337',
-                'author': 'Test user',
-                'message': 'My w0rk3r',
-                'created': '2020-02-02T01:23:45.678000Z',
-                'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
-                'refs': []
-            },
-            'gpu_usage': version.gpu_usage.value,
-            'model_usage': version.model_usage,
-            'docker_image': str(version.docker_image_id),
-            'docker_image_iid': None,
-            'docker_image_name': version.docker_image_name,
-            'state': 'available',
-            'worker': {
-                'id': str(version.worker.id),
-                'name': version.worker.name,
-                'type': version.worker.type.slug,
-                'slug': version.worker.slug,
-            }
-        }
-
     def test_corpus_worker_version_no_login_public(self):
         self.assertTrue(self.corpus.public)
         # Use multiple versions to ensure we don't get duplicated queries
         self.corpus.worker_versions.set([self.version_1, self.version_2])
 
-        with self.assertNumQueries(5):
+        with self.assertNumQueries(7):
             response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
-        # Versions will be ordered by ID
-        expected_results = sorted([
-            self._serialize_worker_version(self.version_1),
-            self._serialize_worker_version(self.version_2),
-        ], key=lambda version: version['id'])
-
         self.assertDictEqual(response.json(), {
-            'count': None,
+            'count': 2,
+            'number': 1,
             'previous': None,
             'next': None,
-            'results': expected_results
+            'results': [
+                {
+                    'id': str(self.version_2.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_2.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_2.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/dla:{self.version_2.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_2.id),
+                        'name': 'Document layout analyser',
+                        'type': 'dla',
+                        'slug': 'dla',
+                    }
+                },
+                {
+                    'id': str(self.version_1.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_1.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_1.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_1.id),
+                        'name': 'Recognizer',
+                        'type': 'recognizer',
+                        'slug': 'reco',
+                    }
+                },
+            ],
         })
 
     def test_corpus_worker_version_no_login_private(self):
@@ -1565,21 +1583,67 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         # Use multiple versions to ensure we don't get duplicated queries
         self.corpus.worker_versions.set([self.version_1, self.version_2])
 
-        with self.assertNumQueries(9):
+        with self.assertNumQueries(11):
             response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
-        # Versions will be ordered by ID
-        expected_results = sorted([
-            self._serialize_worker_version(self.version_1),
-            self._serialize_worker_version(self.version_2),
-        ], key=lambda version: version['id'])
-
         self.assertDictEqual(response.json(), {
-            'count': None,
+            'count': 2,
+            'number': 1,
             'previous': None,
             'next': None,
-            'results': expected_results
+            'results': [
+                {
+                    'id': str(self.version_2.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_2.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_2.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/dla:{self.version_2.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_2.id),
+                        'name': 'Document layout analyser',
+                        'type': 'dla',
+                        'slug': 'dla',
+                    }
+                },
+                {
+                    'id': str(self.version_1.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_1.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_1.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_1.id),
+                        'name': 'Recognizer',
+                        'type': 'recognizer',
+                        'slug': 'reco',
+                    }
+                },
+            ],
         })
 
     def test_corpus_worker_version_list(self):
@@ -1587,21 +1651,67 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         # Use multiple versions to ensure we don't get duplicated queries
         self.corpus.worker_versions.set([self.version_1, self.version_2])
 
-        with self.assertNumQueries(9):
+        with self.assertNumQueries(11):
             response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
-        # Versions will be ordered by ID
-        expected_results = sorted([
-            self._serialize_worker_version(self.version_1),
-            self._serialize_worker_version(self.version_2),
-        ], key=lambda version: version['id'])
-
         self.assertDictEqual(response.json(), {
-            'count': None,
+            'count': 2,
+            'number': 1,
             'previous': None,
             'next': None,
-            'results': expected_results
+            'results': [
+                {
+                    'id': str(self.version_2.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_2.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_2.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/dla:{self.version_2.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_2.id),
+                        'name': 'Document layout analyser',
+                        'type': 'dla',
+                        'slug': 'dla',
+                    }
+                },
+                {
+                    'id': str(self.version_1.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_1.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_1.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_1.id),
+                        'name': 'Recognizer',
+                        'type': 'recognizer',
+                        'slug': 'reco',
+                    }
+                },
+            ],
         })
 
     def test_corpus_worker_version_no_right(self):
@@ -1610,19 +1720,65 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
         # Use multiple versions to ensure we don't get duplicated queries
         self.corpus.worker_versions.set([self.version_1, self.version_2])
 
-        with self.assertNumQueries(9):
+        with self.assertNumQueries(11):
             response = self.client.get(reverse('api:corpus-versions', kwargs={'pk': self.corpus.id}))
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
-        # Versions will be ordered by ID
-        expected_results = sorted([
-            self._serialize_worker_version(self.version_1),
-            self._serialize_worker_version(self.version_2),
-        ], key=lambda version: version['id'])
-
         self.assertDictEqual(response.json(), {
-            'count': None,
+            'count': 2,
+            'number': 1,
             'previous': None,
             'next': None,
-            'results': expected_results
+            'results': [
+                {
+                    'id': str(self.version_2.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_2.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_2.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/dla:{self.version_2.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_2.id),
+                        'name': 'Document layout analyser',
+                        'type': 'dla',
+                        'slug': 'dla',
+                    }
+                },
+                {
+                    'id': str(self.version_1.id),
+                    'configuration': {'test': 42},
+                    'revision': {
+                        'id': str(self.version_1.revision_id),
+                        'hash': '1337',
+                        'author': 'Test user',
+                        'message': 'My w0rk3r',
+                        'created': '2020-02-02T01:23:45.678000Z',
+                        'commit_url': 'http://my_repo.fake/workers/worker/commit/1337',
+                        'refs': []
+                    },
+                    'gpu_usage': 'disabled',
+                    'model_usage': False,
+                    'docker_image': str(self.version_1.docker_image_id),
+                    'docker_image_iid': None,
+                    'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}',
+                    'state': 'available',
+                    'worker': {
+                        'id': str(self.worker_1.id),
+                        'name': 'Recognizer',
+                        'type': 'recognizer',
+                        'slug': 'reco',
+                    }
+                },
+            ],
         })
-- 
GitLab