Merge pull request #4105 from GeotrekCE/hot_fix_sources_perfs

🚑 [HOT] Improve performance on retrieving Sources in APIv2 #4079
GeotrekCE · May 15, 2024 · 6bdd11a · 6bdd11a
2 parents 840538e + 2502144
commit 6bdd11a
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 19 deletions.
diff --git a/docs/changelog.rst b/docs/changelog.rst
@@ -9,6 +9,10 @@ CHANGELOG
 
 - Add sources websites and pictograms to Aggregator (#3569)
 
+**Hotfix**
+
+- Improve performance on retrieving Sources in APIv2 (fixes #4079)
+
 
 2.105.1 (2024-05-07)
 --------------------

diff --git a/geotrek/api/tests/test_v2.py b/geotrek/api/tests/test_v2.py
@@ -2805,7 +2805,7 @@ def setUpTestData(cls):
         cls.source = common_factory.RecordSourceFactory()
         cls.portal = common_factory.TargetPortalFactory()
         cls.page1 = flatpages_factory.FlatPageFactory(
-            title='AAA', published=True, content='Blah',
+            title='AAA', published=True, published_fr=True, content='Blah',
             sources=[cls.source], portals=[cls.portal]
         )
         cls.page2 = flatpages_factory.FlatPageFactory(
@@ -2828,7 +2828,7 @@ def test_list(self):
                 'title': {'en': 'AAA', 'es': None, 'fr': None, 'it': None},
                 'content': {'en': 'Blah', 'es': None, 'fr': None, 'it': None},
                 'portals': [self.portal.pk],
-                'published': {'en': True, 'es': False, 'fr': False, 'it': False},
+                'published': {'en': True, 'es': False, 'fr': True, 'it': False},
                 'source': [self.source.pk],
                 'attachments': [],
                 'parent': None,
@@ -2875,8 +2875,9 @@ def test_list_returns_published_in_specified_language_only(self):
 
         self.assertEqual(response.status_code, 200)
         resp_data = response.json()
-        self.assertEqual(resp_data["count"], 1)
-        self.assertEqual(resp_data["results"][0]["id"], self.page2.id)
+        self.assertEqual(resp_data["count"], 2)
+        self.assertEqual(resp_data["results"][0]["id"], self.page1.id)
+        self.assertEqual(resp_data["results"][1]["id"], self.page2.id)
 
     def test_list_returns_pages_associated_to_portals(self):
         portal1 = common_factory.TargetPortalFactory()
@@ -3089,7 +3090,7 @@ def test_detail(self):
             'title': {'en': 'AAA', 'es': None, 'fr': None, 'it': None},
             'content': {'en': 'Blah', 'es': None, 'fr': None, 'it': None},
             'portals': [self.portal.pk],
-            'published': {'en': True, 'es': False, 'fr': False, 'it': False},
+            'published': {'en': True, 'es': False, 'fr': True, 'it': False},
             'source': [self.source.pk],
             'attachments': [],
             'parent': None,
@@ -3149,10 +3150,24 @@ def test_filter_portals(self):
         self.assertEqual(response.json()['results'][0]['title']['en'], 'AAA')
 
     def test_filter_sources_by_portal(self):
-        response = self.client.get('/api/v2/source/', {'portals': self.portal.pk})
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.json()['count'], 1)
-        self.assertEqual(response.json()['results'][0]['name'], self.source.name)
+        # 5 queries for 5 related objects
+        # 1 query for select on IDs
+        # 1 count query
+        with self.assertNumQueries(7):
+            response = self.client.get('/api/v2/source/', {'portals': self.portal.pk})
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.json()['count'], 1)
+            self.assertEqual(response.json()['results'][0]['name'], self.source.name)
+
+    def test_filter_sources_by_lang(self):
+        # 5 queries for 5 related objects
+        # 1 query for select on IDs
+        # 1 count query
+        with self.assertNumQueries(7):
+            response = self.client.get('/api/v2/source/', {'language': 'fr'})
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.json()['count'], 1)
+            self.assertEqual(response.json()['results'][0]['name'], self.source.name)
 
 
 class MenuItemTestCase(TestCase):

diff --git a/geotrek/api/v2/filters.py b/geotrek/api/v2/filters.py
@@ -1141,19 +1141,39 @@ def filter_queryset(self, request, qs, view):
 
 class TreksAndSitesAndTourismAndFlatpagesRelatedPortalThemeFilter(RelatedObjectsPublishedNotDeletedByPortalFilter):
     def filter_queryset(self, request, qs, view):
+
+        # Prepare language query
+        lang_query = Q()
+        language = request.GET.get('language')
+        if language:
+            # one language is specified
+            lang_query = Q(**{build_localized_fieldname('published', language): True})
+        else:
+            # no language specified. Check for all.
+            for lang in settings.MODELTRANSLATION_LANGUAGES:
+                lang_query |= Q(**{build_localized_fieldname('published', lang): True})
+
+        # Prepare portal queries
         portals = request.GET.get('portals')
-        portals_query = Q()
         if portals:
-            related_portal_in = '{}__portals__in'.format('flatpages')
-            portals_query = Q(**{related_portal_in: portals.split(',')})
-        set_1 = self.filter_queryset_related_objects_published(qs, request, 'flatpages', portals_query)
-        set_2 = self.filter_queryset_related_objects_published_not_deleted_by_portal(qs, request, 'treks')
-        set_3 = self.filter_queryset_related_objects_published_not_deleted_by_portal(qs, request, 'touristiccontents')
-        set_4 = self.filter_queryset_related_objects_published_not_deleted_by_portal(qs, request, 'touristicevents')
-        set_5 = qs.none()
+            portals_query = Q(**{'portals__in': portals.split(',')})
+            portal_query = Q(**{'portal__in': portals.split(',')})
+        else:
+            portal_query = Q()
+            portals_query = Q()
+
+        # Extract distinct sources
+        flatpages_sources = list(FlatPage.objects.filter(lang_query, portals_query).prefetch_related('portals, source').values_list('source__pk', flat=True))
+        treks_sources = list(Trek.objects.filter(lang_query, portal_query, deleted=False).prefetch_related('portal, source').values_list('source__pk', flat=True))
+        touristiccontent_sources = list(TouristicContent.objects.filter(lang_query, portal_query, deleted=False).prefetch_related('portal, source').values_list('source__pk', flat=True))
+        touristicevent_sources = list(TouristicEvent.objects.filter(lang_query, portal_query, deleted=False).prefetch_related('portal, source').values_list('source__pk', flat=True))
+        all_sources_pks = flatpages_sources + treks_sources + touristiccontent_sources + touristicevent_sources
         if 'geotrek.outdoor' in settings.INSTALLED_APPS:
-            set_5 = self.filter_queryset_related_objects_published_by_portal(qs, request, 'sites')
-        return (set_1 | set_2 | set_3 | set_4 | set_5).distinct()
+            sites_sources = list(Site.objects.filter(lang_query, portal_query).prefetch_related('portal, source').values_list('source__pk', flat=True))
+            all_sources_pks += sites_sources
+
+        # Return distinct sources
+        return qs.filter(pk__in=set(all_sources_pks))
 
 
 class TreksAndSitesRelatedPortalFilter(RelatedObjectsPublishedNotDeletedByPortalFilter):