Skip to content

Commit

Permalink
Install django-silk nad fix topics api perf
Browse files Browse the repository at this point in the history
  • Loading branch information
rhysyngsun committed Jul 10, 2024
1 parent 8db7e80 commit eabb23c
Show file tree
Hide file tree
Showing 14 changed files with 180 additions and 43 deletions.
24 changes: 24 additions & 0 deletions channels/migrations/0011_add_topic_detail_related_name.py
Original file line number Diff line number Diff line change
@@ -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",
),
),
]
39 changes: 33 additions & 6 deletions channels/models.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
"""Models for channels"""

from django.conf import settings
from django.contrib.auth.models import Group
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

Expand All @@ -14,7 +16,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
Expand All @@ -23,9 +25,27 @@
AVATAR_MEDIUM_MAX_DIMENSION = 90


class ChannelQuerySet(TimestampedModelQuerySet):
"""Custom queryset for Channels"""

def for_serialization(self):
"""Annotate the channel for serialization"""
return self.annotate(
_channel_url=Concat(
models.Value(f"{settings.APP_BASE_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,
Expand Down Expand Up @@ -91,13 +111,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):
Expand All @@ -110,7 +134,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",
)


Expand Down
4 changes: 2 additions & 2 deletions frontends/api/src/generated/v0/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions frontends/api/src/generated/v1/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 30 additions & 3 deletions learning_resources/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -17,7 +17,7 @@
LearningResourceType,
PrivacyLevel,
)
from main.models import TimestampedModel
from main.models import TimestampedModel, TimestampedModelQuerySet


def default_learning_format():
Expand All @@ -38,11 +38,29 @@ def __str__(self):
return f"{self.code}: {self.name}"


class LearningResourceTopicQuerySet(TimestampedModelQuerySet):
"""QuerySet for LearningResourceTopic"""

def for_serialization(self):
"""Get the default queryset"""
from channels.models import Channel

return self.annotate(
_channel_url=(
Channel.objects.filter(topic_detail__topic=OuterRef("pk"))
.for_serialization()
.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",
Expand All @@ -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"""

Expand Down
11 changes: 3 additions & 8 deletions learning_resources/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion learning_resources/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ class TopicViewSet(viewsets.ReadOnlyModelViewSet):
Topics covered by learning resources
"""

queryset = LearningResourceTopic.objects.all().order_by("name")
queryset = LearningResourceTopic.objects.all().order_by("name").for_serialization()
serializer_class = LearningResourceTopicSerializer
pagination_class = LargePagination
permission_classes = (AnonymousAccessReadonlyPermission,)
Expand Down
22 changes: 13 additions & 9 deletions learning_resources/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
11 changes: 11 additions & 0 deletions main/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"news_events",
"testimonials",
"data_fixtures",
"silk",
)

if not get_bool("RUN_DATA_MIGRATIONS", default=False):
Expand Down Expand Up @@ -150,6 +151,7 @@
"hijack.middleware.HijackUserMiddleware",
"oauth2_provider.middleware.OAuth2TokenMiddleware",
"django_scim.middleware.SCIMAuthCheckMiddleware",
"silk.middleware.SilkyMiddleware",
)

# CORS
Expand Down Expand Up @@ -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
1 change: 1 addition & 0 deletions main/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 1 addition & 4 deletions openapi/specs/v0.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1485,9 +1485,9 @@ components:
maxLength: 12
channel_url:
type: string
nullable: true
description: Get the channel url for the offeror if it exists
readOnly: true
nullable: true
name:
type: string
maxLength: 256
Expand Down Expand Up @@ -1607,9 +1607,6 @@ components:
nullable: true
channel_url:
type: string
description: Get the channel url for the topic if it exists
readOnly: true
nullable: true
required:
- channel_url
- id
Expand Down
11 changes: 4 additions & 7 deletions openapi/specs/v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8076,9 +8076,9 @@ components:
maxLength: 256
channel_url:
type: string
nullable: true
description: Get the channel url for the department if it exists
readOnly: true
nullable: true
required:
- channel_url
- department_id
Expand Down Expand Up @@ -8144,9 +8144,9 @@ components:
maxLength: 256
channel_url:
type: string
nullable: true
description: Get the channel url for the department if it exists
readOnly: true
nullable: true
school:
allOf:
- $ref: '#/components/schemas/LearningResourceBaseSchool'
Expand Down Expand Up @@ -8260,9 +8260,9 @@ components:
maxLength: 256
channel_url:
type: string
nullable: true
description: Get the channel url for the offeror if it exists
readOnly: true
nullable: true
required:
- channel_url
- code
Expand All @@ -8276,9 +8276,9 @@ components:
maxLength: 12
channel_url:
type: string
nullable: true
description: Get the channel url for the offeror if it exists
readOnly: true
nullable: true
name:
type: string
maxLength: 256
Expand Down Expand Up @@ -8670,9 +8670,6 @@ components:
nullable: true
channel_url:
type: string
description: Get the channel url for the topic if it exists
readOnly: true
nullable: true
required:
- channel_url
- id
Expand Down
Loading

0 comments on commit eabb23c

Please sign in to comment.