diff --git a/arkindex/documents/api/elements.py b/arkindex/documents/api/elements.py index 4ea56749530880c1ac640a243d779f92b7a67ed6..2018fa094655d6a397fe40f818ed38cd7ee29e4c 100644 --- a/arkindex/documents/api/elements.py +++ b/arkindex/documents/api/elements.py @@ -816,32 +816,22 @@ class ElementChildren(ElementsListBase): Requires a **read** access to the element's corpus. """ + @property + def is_recursive(self): + recursive_param = self.clean_params.get('recursive') + return recursive_param is not None and recursive_param.lower() not in ('false', '0') + @property def selected_corpus(self): return self.element.corpus def get_filters(self): filters = super().get_filters() - recursive_param = self.clean_params.get('recursive') - if recursive_param is None or recursive_param.lower() in ('false', '0'): - # Only list direct children, by listing all the direct paths from the current element - # This is faster for direct children than : - # - Element.objects.filter(paths__path__last=XXX) - # - Element.objects.get_descending() - # This code path is extremely used, as it's the basis for navigation amongst elements - # Be extremely careful when changing this behaviour. - paths = self.element.paths.values_list('path', flat=True) - if not paths: - # Support top level children - paths = [[]] - - filters['paths__path__in'] = [ - path + [self.kwargs['pk']] - for path in paths - ] - else: + if self.is_recursive: filters['paths__path__overlap'] = [self.kwargs['pk']] + else: + filters['paths__path__last'] = self.kwargs['pk'] return filters diff --git a/arkindex/documents/migrations/0047_elementpath_element_path_last.py b/arkindex/documents/migrations/0047_elementpath_element_path_last.py new file mode 100644 index 0000000000000000000000000000000000000000..e52cadae8ecc3282089e098502547468a8ce6b49 --- /dev/null +++ b/arkindex/documents/migrations/0047_elementpath_element_path_last.py @@ -0,0 +1,24 @@ +# Generated by Django 4.0.1 on 2022-01-05 16:29 + +import django.db.models.expressions +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('documents', '0046_transcription_orientation'), + ] + + operations = [ + migrations.AddIndex( + model_name='elementpath', + index=models.Index(django.db.models.expressions.F('path__last'), name='element_path_last'), + ), + # After this index is created, we need to analyze the table again to make its query planner see it; + # otherwise, it will keep its old way of querying and no performance improvement is made. + migrations.RunSQL( + "ANALYZE documents_elementpath", + reverse_sql=migrations.RunSQL.noop, + ) + ] diff --git a/arkindex/documents/models.py b/arkindex/documents/models.py index 713103e9e562aac1dbf1ced413278d8ee63cefcc..0fb62f4036c0a0b7093bab8a8d599b311a92fb1c 100644 --- a/arkindex/documents/models.py +++ b/arkindex/documents/models.py @@ -121,6 +121,7 @@ class ElementPath(models.Model): class Meta: indexes = [ GinIndex(fields=['path']), + models.Index('path__last', name='element_path_last'), ] constraints = [ models.UniqueConstraint(fields=('element', 'path'), name='unique_element_paths'), diff --git a/arkindex/documents/tests/test_children_elements.py b/arkindex/documents/tests/test_children_elements.py index 6d225e1a42badce3789344bbd63904de11b63250..551560563c99d8dca4031f1b494c949b10c64532 100644 --- a/arkindex/documents/tests/test_children_elements.py +++ b/arkindex/documents/tests/test_children_elements.py @@ -248,7 +248,7 @@ class TestChildrenElements(FixtureAPITestCase): def test_element_children_worker_version(self): self.corpus.elements.filter(name__contains='page 1r').update(worker_version=self.worker_version) - with self.assertNumQueries(7): + with self.assertNumQueries(6): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={'worker_version': str(self.worker_version.id)} @@ -307,7 +307,7 @@ class TestChildrenElements(FixtureAPITestCase): ) element.add_parent(self.vol) - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={'with_has_children': True}, @@ -329,7 +329,7 @@ class TestChildrenElements(FixtureAPITestCase): ) def test_children_modified_since_bad_format(self): - with self.assertNumQueries(2): + with self.assertNumQueries(1): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), HTTP_IF_MODIFIED_SINCE='AAAAAAAAAAAAAAAAA', @@ -338,7 +338,7 @@ class TestChildrenElements(FixtureAPITestCase): self.assertDictEqual(response.json(), {'If-Modified-Since': ['Bad date format']}) def test_children_modified_since_not_modified(self): - with self.assertNumQueries(3): + with self.assertNumQueries(2): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT', @@ -347,7 +347,7 @@ class TestChildrenElements(FixtureAPITestCase): def test_children_modified_since(self): self.corpus.elements.filter(name='Volume 1, page 1r').update(updated='2099-04-02T13:37:43Z') - with self.assertNumQueries(7): + with self.assertNumQueries(6): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT', @@ -361,7 +361,7 @@ class TestChildrenElements(FixtureAPITestCase): ) def test_children_filter_metadata_name(self): - with self.assertNumQueries(6): + with self.assertNumQueries(5): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={'metadata_name': 'folio'} @@ -388,7 +388,7 @@ class TestChildrenElements(FixtureAPITestCase): }) def test_children_filter_metadata_name_value(self): - with self.assertNumQueries(6): + with self.assertNumQueries(5): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={'metadata_name': 'folio', 'metadata_value': '1v'} @@ -464,7 +464,7 @@ class TestChildrenElements(FixtureAPITestCase): for operator, value, expected_elements in parameters: with self.subTest(operator=operator, value=value): - with self.assertNumQueries(5): + with self.assertNumQueries(4): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={ @@ -480,7 +480,7 @@ class TestChildrenElements(FixtureAPITestCase): ) def test_children_filter_metadata_contains(self): - with self.assertNumQueries(6): + with self.assertNumQueries(5): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={ @@ -513,8 +513,8 @@ class TestChildrenElements(FixtureAPITestCase): ] for rotation_angle, expected_elements in cases: with self.subTest(rotation_angle=rotation_angle): - # Requests that return no elements only make 3 SQL queries - num_queries = 5 if len(expected_elements) else 3 + # Requests that return no elements only make 2 SQL queries + num_queries = 4 if len(expected_elements) else 2 with self.assertNumQueries(num_queries): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), @@ -564,7 +564,7 @@ class TestChildrenElements(FixtureAPITestCase): ] for mirrored, expected_elements in cases: with self.subTest(mirrored=mirrored): - with self.assertNumQueries(5): + with self.assertNumQueries(4): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}), data={ diff --git a/arkindex/documents/tests/test_classes.py b/arkindex/documents/tests/test_classes.py index 3ef288e0c3c74d6fc5cfeefda5e9515ae91b5e06..b204fac81c5211e176c87ffd526ddb957683e513 100644 --- a/arkindex/documents/tests/test_classes.py +++ b/arkindex/documents/tests/test_classes.py @@ -330,7 +330,7 @@ class TestClasses(FixtureAPITestCase): ) def test_element_children_classes(self): - with self.assertNumQueries(7): + with self.assertNumQueries(6): response = self.client.get( reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}), data={'type': self.classified.slug, 'with_classes': 'yes'} diff --git a/arkindex/documents/tests/test_destroy_elements.py b/arkindex/documents/tests/test_destroy_elements.py index 5348e927cd32d8ac00787e15db03ec29ef7c6c10..34be212608692331e46a39e1837409e9b22f822d 100644 --- a/arkindex/documents/tests/test_destroy_elements.py +++ b/arkindex/documents/tests/test_destroy_elements.py @@ -340,7 +340,7 @@ class TestDestroyElements(FixtureAPITestCase): self.client.force_login(self.user) element = self.corpus.elements.create(type=self.volume_type, name='Lonely element') - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.delete(reverse('api:elements-children', kwargs={'pk': element.id})) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) @@ -349,7 +349,7 @@ class TestDestroyElements(FixtureAPITestCase): @patch('arkindex.project.triggers.documents_tasks.element_trash.delay') def test_destroy_element_children(self, delay_mock): self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.delete(reverse('api:elements-children', kwargs={'pk': self.vol.id})) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -370,7 +370,7 @@ class TestDestroyElements(FixtureAPITestCase): @patch('arkindex.project.triggers.documents_tasks.element_trash.delay') def test_destroy_element_children_delete_children(self, delay_mock): self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.delete( reverse('api:elements-children', kwargs={'pk': self.vol.id}), QUERY_STRING='delete_children=false'