Addresses performance issues for the topics API

channels/migrations/0011_add_topic_detail_related_name.py

@@ -0,0 +1,24 @@
+# Generated by Django 4.2.13 on 2024-07-10 16:43
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("learning_resources", "0055_alter_relationship_options_ordering"),
+        ("channels", "0010_alter_channel_name_regex"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="channeltopicdetail",
+            name="topic",
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.SET_NULL,
+                related_name="channel_topic_details",
+                to="learning_resources.learningresourcetopic",
+            ),
+        ),
+    ]

channels/models.py

@@ -4,6 +4,7 @@
 from django.core.validators import RegexValidator
 from django.db import models
 from django.db.models import JSONField, deletion
+from django.db.models.functions import Concat
 from imagekit.models import ImageSpecField, ProcessedImageField
 from imagekit.processors import ResizeToFit
 
@@ -14,7 +15,7 @@
     LearningResourceOfferor,
     LearningResourceTopic,
 )
-from main.models import TimestampedModel
+from main.models import TimestampedModel, TimestampedModelQuerySet
 from main.utils import frontend_absolute_url
 from profiles.utils import avatar_uri, banner_uri
 from widgets.models import WidgetList
@@ -23,9 +24,27 @@
 AVATAR_MEDIUM_MAX_DIMENSION = 90
 
 
+class ChannelQuerySet(TimestampedModelQuerySet):
+    """Custom queryset for Channels"""
+
+    def annotate_channel_url(self):
+        """Annotate the channel for serialization"""
+        return self.annotate(
+            _channel_url=Concat(
+                models.Value(frontend_absolute_url("/c/")),
+                "channel_type",
+                models.Value("/"),
+                "name",
+                models.Value("/"),
+            )
+        )
+
+
 class Channel(TimestampedModel):
     """Channel for any field/subject"""
 
+    objects = ChannelQuerySet.as_manager()
+
     # Channel configuration
     name = models.CharField(
         max_length=100,
@@ -91,13 +110,17 @@ def __str__(self):
         """Str representation of channel"""
         return self.title
 
-    class Meta:
-        unique_together = ("name", "channel_type")
-
     @property
     def channel_url(self) -> str:
         """Return the channel url"""
-        return frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/")
+        return getattr(
+            self,
+            "_channel_url",
+            frontend_absolute_url(f"/c/{self.channel_type}/{self.name}/"),
+        )
+
+    class Meta:
+        unique_together = ("name", "channel_type")
 
 
 class ChannelTopicDetail(TimestampedModel):
@@ -110,7 +133,10 @@ class ChannelTopicDetail(TimestampedModel):
         related_name="topic_detail",
     )
     topic = models.ForeignKey(
-        LearningResourceTopic, null=True, on_delete=models.SET_NULL
+        LearningResourceTopic,
+        null=True,
+        on_delete=models.SET_NULL,
+        related_name="channel_topic_details",
     )
 
 

channels/serializers.py

@@ -147,12 +147,7 @@ def get_lists(self, instance):
         """Return the channel's list of LearningPaths"""
         return [
             LearningPathPreviewSerializer(channel_list.channel_list).data
-            for channel_list in ChannelList.objects.filter(channel=instance)
-            .prefetch_related(
-                "channel_list", "channel__lists", "channel__featured_list"
-            )
-            .all()
-            .order_by("position")
+            for channel_list in instance.lists.all()
         ]
 
     def get_channel_url(self, instance) -> str:

channels/views.py

@@ -3,6 +3,7 @@
 import logging
 
 from django.contrib.auth.models import User
+from django.db.models import Prefetch
 from django_filters.rest_framework import DjangoFilterBackend
 from drf_spectacular.utils import extend_schema, extend_schema_view
 from rest_framework import mixins, viewsets
@@ -13,7 +14,7 @@
 
 from channels.api import get_group_role_name, remove_user_role
 from channels.constants import CHANNEL_ROLE_MODERATORS
-from channels.models import Channel
+from channels.models import Channel, ChannelList
 from channels.permissions import ChannelModeratorPermissions, HasChannelPermission
 from channels.serializers import (
     ChannelCreateSerializer,
@@ -75,8 +76,19 @@ def get_queryset(self):
         """Return a queryset"""
         return (
             Channel.objects.prefetch_related(
-                "lists", "sub_channels", "sub_channels__channel"
+                Prefetch(
+                    "lists",
+                    queryset=ChannelList.objects.prefetch_related(
+                        "channel_list", "channel__lists", "channel__featured_list"
+                    ).order_by("position"),
+                ),
+                "sub_channels",
+                Prefetch(
+                    "sub_channels__channel",
+                    queryset=Channel.objects.annotate_channel_url(),
+                ),
             )
+            .annotate_channel_url()
             .select_related(
                 "featured_list", "topic_detail", "department_detail", "unit_detail"
             )

channels/views_test.py

@@ -378,7 +378,7 @@ def test_no_excess_queries(user_client, django_assert_num_queries, related_count
     sub_channels / lists.
     """
     # This isn't too important; we care it does not scale with number of related items
-    expected_query_count = 11
+    expected_query_count = 10
 
     topic_channel = ChannelFactory.create(is_topic=True)
     ChannelListFactory.create_batch(related_count, channel=topic_channel)

conftest.py

@@ -20,3 +20,15 @@ def prevent_requests(mocker, request):  # noqa: PT004
         autospec=True,
         side_effect=DoNotUseRequestException,
     )
+
+
+@pytest.fixture(autouse=True)
+def _disable_silky(settings):
+    """Disable django-silky during tests"""
+    settings.SILKY_INTERCEPT_PERCENT = 0
+
+    settings.MIDDLEWARE = tuple(
+        middleware
+        for middleware in settings.MIDDLEWARE
+        if middleware != "silk.middleware.SilkyMiddleware"
+    )

frontends/api/src/generated/v0/api.ts

@@ -1198,11 +1198,11 @@ export interface LearningResourceTopic {
    */
   parent?: number | null
   /**
-   * Get the channel url for the topic if it exists
+   *
    * @type {string}
    * @memberof LearningResourceTopic
    */
-  channel_url: string | null
+  channel_url: string
 }
 /**
  * Serializer for News FeedItem

frontends/api/src/generated/v1/api.ts

@@ -2285,11 +2285,11 @@ export interface LearningResourceTopic {
    */
   parent?: number | null
   /**
-   * Get the channel url for the topic if it exists
+   *
    * @type {string}
    * @memberof LearningResourceTopic
    */
-  channel_url: string | null
+  channel_url: string
 }
 /**
  * SearchResponseSerializer with OpenAPI annotations for Learning Resources search

learning_resources/models.py

@@ -5,7 +5,7 @@
 from django.contrib.auth.models import User
 from django.contrib.postgres.fields import ArrayField
 from django.db import models
-from django.db.models import JSONField, Q
+from django.db.models import JSONField, OuterRef, Q
 from django.db.models.functions import Lower
 from django.utils import timezone
 
@@ -17,7 +17,7 @@
     LearningResourceType,
     PrivacyLevel,
 )
-from main.models import TimestampedModel
+from main.models import TimestampedModel, TimestampedModelQuerySet
 
 
 def default_learning_format():
@@ -38,11 +38,29 @@ def __str__(self):
         return f"{self.code}: {self.name}"
 
 
+class LearningResourceTopicQuerySet(TimestampedModelQuerySet):
+    """QuerySet for LearningResourceTopic"""
+
+    def annotate_channel_url(self):
+        """Annotate with the channel url"""
+        from channels.models import Channel
+
+        return self.annotate(
+            _channel_url=(
+                Channel.objects.filter(topic_detail__topic=OuterRef("pk"))
+                .annotate_channel_url()
+                .values_list("_channel_url", flat=True)[:1]
+            ),
+        )
+
+
 class LearningResourceTopic(TimestampedModel):
     """
     Topics for all learning resources (e.g. "History")
     """
 
+    objects = LearningResourceTopicQuerySet.as_manager()
+
     name = models.CharField(max_length=128)
     parent = models.ForeignKey(
         "LearningResourceTopic",
@@ -53,9 +71,18 @@ class LearningResourceTopic(TimestampedModel):
 
     def __str__(self):
         """Return the topic name."""
-
         return self.name
 
+    @property
+    def channel_url(self):
+        """Return the topic's channel url"""
+        if hasattr(self, "_channel_url"):
+            return self._channel_url
+
+        topic_detail = self.channel_topic_details.first()
+
+        return topic_detail.channel.channel_url if topic_detail is not None else None
+
     class Meta:
         """Meta options for LearningResourceTopic"""
 

learning_resources/serializers.py

@@ -43,12 +43,7 @@ class LearningResourceTopicSerializer(serializers.ModelSerializer):
     Serializer for LearningResourceTopic model
     """
 
-    channel_url = serializers.SerializerMethodField(read_only=True, allow_null=True)
-
-    def get_channel_url(self, instance: models.LearningResourceTopic) -> str or None:
-        """Get the channel url for the topic if it exists"""
-        channel = Channel.objects.filter(topic_detail__topic=instance).first()
-        return channel.channel_url if channel else None
+    channel_url = serializers.CharField()
 
     class Meta:
         """Meta options for the serializer."""
@@ -100,7 +95,7 @@ class LearningResourceOfferorSerializer(serializers.ModelSerializer):
 
     channel_url = serializers.SerializerMethodField(read_only=True, allow_null=True)
 
-    def get_channel_url(self, instance: models.LearningResourceOfferor) -> str or None:
+    def get_channel_url(self, instance: models.LearningResourceOfferor) -> str | None:
         """Get the channel url for the offeror if it exists"""
         channel = Channel.objects.filter(unit_detail__unit=instance).first()
         return channel.channel_url if channel else None
@@ -160,7 +155,7 @@ class LearningResourceBaseDepartmentSerializer(serializers.ModelSerializer):
 
     def get_channel_url(
         self, instance: models.LearningResourceDepartment
-    ) -> str or None:
+    ) -> str | None:
         """Get the channel url for the department if it exists"""
         channel = Channel.objects.filter(department_detail__department=instance).first()
         return channel.channel_url if channel else None

learning_resources/views.py

@@ -429,7 +429,9 @@ class TopicViewSet(viewsets.ReadOnlyModelViewSet):
     Topics covered by learning resources
     """
 
-    queryset = LearningResourceTopic.objects.all().order_by("name")
+    queryset = (
+        LearningResourceTopic.objects.all().order_by("name").annotate_channel_url()
+    )
     serializer_class = LearningResourceTopicSerializer
     pagination_class = LargePagination
     permission_classes = (AnonymousAccessReadonlyPermission,)

learning_resources/views_test.py

@@ -556,20 +556,24 @@ def test_ocw_webhook_endpoint_bad_key(settings, client):
         )
 
 
-def test_topics_list_endpoint(client):
+def test_topics_list_endpoint(client, django_assert_num_queries):
     """Test topics list endpoint"""
     topics = sorted(
-        LearningResourceTopicFactory.create_batch(3),
+        LearningResourceTopicFactory.create_batch(100),
         key=lambda topic: topic.name,
     )
 
-    resp = client.get(reverse("lr:v1:topics_api-list"))
-    assert resp.data.get("count") == 3
-    for i in range(3):
-        assert (
-            resp.data.get("results")[i]
-            == LearningResourceTopicSerializer(instance=topics[i]).data
-        )
+    with django_assert_num_queries(2):
+        resp = client.get(reverse("lr:v1:topics_api-list"))
+
+    assert resp.data == {
+        "count": 100,
+        "next": None,
+        "previous": None,
+        "results": [
+            LearningResourceTopicSerializer(instance=topic).data for topic in topics
+        ],
+    }
 
 
 def test_topics_detail_endpoint(client):

main/settings.py

@@ -110,6 +110,7 @@
     "news_events",
     "testimonials",
     "data_fixtures",
+    "silk",
 )
 
 if not get_bool("RUN_DATA_MIGRATIONS", default=False):
@@ -150,6 +151,7 @@
     "hijack.middleware.HijackUserMiddleware",
     "oauth2_provider.middleware.OAuth2TokenMiddleware",
     "django_scim.middleware.SCIMAuthCheckMiddleware",
+    "silk.middleware.SilkyMiddleware",
 )
 
 # CORS
@@ -713,3 +715,12 @@ def get_all_config_keys():
     name="POSTHOG_PROJECT_ID",
     default=None,
 )
+
+SILKY_INTERCEPT_PERCENT = get_int(name="SILKY_INTERCEPT_PERCENT", default=50)
+SILKY_MAX_RECORDED_REQUESTS = get_int(name="SILKY_MAX_RECORDED_REQUESTS", default=10**3)
+SILKY_MAX_RECORDED_REQUESTS_CHECK_PERCENT = get_int(
+    name="SILKY_MAX_RECORDED_REQUESTS_CHECK_PERCENT", default=10
+)
+SILKY_AUTHENTICATION = True  # User must login
+SILKY_AUTHORISATION = True
+SILKY_PERMISSIONS = lambda user: user.is_superuser  # noqa: E731

main/urls.py

@@ -67,6 +67,7 @@
     re_path(r"^hijack/", include("hijack.urls", namespace="hijack")),
     re_path(r"", include("news_events.urls")),
     re_path(r"^app", RedirectView.as_view(url=settings.APP_BASE_URL)),
+    re_path(r"^silk/", include("silk.urls", namespace="silk")),
 ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
 
 if settings.DEBUG: