Skip to content

Commit

Permalink
Reduce repeated calls to django cache while calculating utm_source fo…
Browse files Browse the repository at this point in the history
…r listing endpoints (#4462)

perf: cache calls for calculating utm_source
  • Loading branch information
zawan-ila authored Oct 14, 2024
1 parent ff9f50b commit 1f1c6ef
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 1 deletion.
5 changes: 5 additions & 0 deletions course_discovery/apps/api/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.conf import settings
from django.core.cache import cache
from django.http.response import HttpResponse
from edx_django_utils.cache import get_cache_key
from rest_framework.renderers import JSONRenderer
from rest_framework_extensions.cache.decorators import CacheResponse
from rest_framework_extensions.key_constructor.bits import KeyBitBase, QueryParamsKeyBit
Expand Down Expand Up @@ -165,3 +166,7 @@ def list(self, request, *args, **kwargs):
)
def retrieve(self, request, *args, **kwargs):
return super().retrieve(request, *args, **kwargs)


def get_utm_source_request_cache_key(partner, user):
return get_cache_key(partner=partner.id, user=user.id)
4 changes: 3 additions & 1 deletion course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
from taxonomy.choices import ProductTypes
from taxonomy.utils import get_whitelisted_serialized_skills

from course_discovery.apps.api.cache import get_utm_source_request_cache_key
from course_discovery.apps.api.fields import (
HtmlField, ImageField, SlugRelatedFieldWithReadSerializer, SlugRelatedTranslatableField, StdImageSerializerField
)
from course_discovery.apps.api.utils import StudioAPI, get_excluded_restriction_types
from course_discovery.apps.api.utils import StudioAPI, get_excluded_restriction_types, use_request_cache
from course_discovery.apps.catalogs.models import Catalog
from course_discovery.apps.core.api_client.lms import LMSAPIClient
from course_discovery.apps.core.utils import update_instance
Expand Down Expand Up @@ -147,6 +148,7 @@ def get_lms_course_url_for_archived(partner, course_key):
return f'{lms_url}/courses/{course_key}/course/'


@use_request_cache("utm_source_cache", get_utm_source_request_cache_key)
def get_utm_source_for_user(partner, user):
"""
Return the utm source for the user.
Expand Down
39 changes: 39 additions & 0 deletions course_discovery/apps/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.core.files.base import ContentFile
from django.db.models.fields.related import ManyToManyField
from django.utils.translation import gettext as _
from edx_django_utils.cache import RequestCache
from opaque_keys.edx.keys import CourseKey
from requests.exceptions import HTTPError
from sortedm2m.fields import SortedManyToManyField
Expand Down Expand Up @@ -352,3 +353,41 @@ def push_to_studio(self, course_run, create=False, old_course_run_key=None, user
self.create_course_run_in_studio(course_run, user=user)
else:
self.update_course_run_details_in_studio(course_run)


def use_request_cache(cache_name, key_func):
"""
This decorator can be used to cache the result of a function per request.
It leverages the RequestCache from edx-django-utils for this purpose
Args:
cache_name (string): The identifier for the cache
key_func (callable): The function used for computing cache keys. This will be called
with the same arguments as the function being decorated
Example:
@use_request_cache("find_factors_cache", lambda x: x)
def find_factors(x):
print("Looking for factors!")
# Some expensive calculation
print("Found the factors")
Over the course of a single request,
find_factors(12345) # This will run the expensive calculation.
find_factors(12345) # This will skip the expensive calculation and return cached value.
"""
def inner(fn):
@functools.wraps(fn)
def wrapper(*args, **kwargs):
cache = RequestCache(cache_name)
cache_key = key_func(*args, **kwargs)
cached_response = cache.get_cached_response(cache_key)
if cached_response.is_found:
return cached_response.value

result = fn(*args, **kwargs)

cache.set(cache_key, result)
return result
return wrapper
return inner
22 changes: 22 additions & 0 deletions course_discovery/apps/api/v1/tests/test_views/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from edx_toggles.toggles.testutils import override_waffle_switch
from rest_framework.reverse import reverse
from testfixtures import LogCapture
from waffle.testutils import override_switch

from course_discovery.apps.api.v1.exceptions import EditableAndQUnsupported
from course_discovery.apps.api.v1.tests.test_views.mixins import APITestCase, OAuth2Mixin, SerializationMixin
Expand Down Expand Up @@ -352,6 +353,27 @@ def test_list(self):
self.serialize_course(Course.objects.all(), many=True)
)

def test_no_repeated_cache_calls_for_utm_calculation(self):
"""
Test that utm source calculation is done only once per request, and not per
serialized object on a listing endpoint. Since each utm source calculation
requires two cache calls, this reduces the number of calls to the django cache.
"""
CourseFactory(
partner=self.partner, title='Fake Test 1', key='edX+Fake102', type=self.audit_type
)
self.partner.lms_url = "http://localhost:8000"
self.partner.save()
with mock.patch('course_discovery.apps.core.api_client.lms.LMSAPIClient.get_api_access_request', return_value={
"company_name": "Example Inc",
"company_address": "Example Address",
}) as mocked_api_access_request:
with override_switch('use_company_name_as_utm_source_value', True):
url = reverse('api:v1:course-list')
response = self.client.get(url)
assert response.data["count"] == 2
mocked_api_access_request.assert_called_once()

@pytest.mark.skip(reason="https://github.com/openedx/course-discovery/issues/4431")
@responses.activate
def test_list_query(self):
Expand Down

0 comments on commit 1f1c6ef

Please sign in to comment.