From f0d956a313573c6c389a5cfa465bcff9b9c81f88 Mon Sep 17 00:00:00 2001
From: Hamza Shafique <41573849+hamza-56@users.noreply.github.com>
Date: Wed, 9 Oct 2024 12:16:26 +0500
Subject: [PATCH] perf: added caching to XBlockSkillsViewSet list endpoint
 (#208)

* perf: added caching to XBlockSkillsViewSet list endpoint

* test: add tests for XBlockSkillsViewSet caching behavior
---
 CHANGELOG.rst            |  4 ++++
 taxonomy/__init__.py     |  2 +-
 taxonomy/api/v1/views.py | 15 +++++++++++++++
 taxonomy/constants.py    |  2 ++
 tests/test_views.py      | 26 ++++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e622973..992b7a6 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -13,6 +13,10 @@ Change Log
 
 Unreleased
 
+[1.54.0] - 2024-10-02
+---------------------
+* perf: Added caching to `XBlockSkillsViewSet` list endpoint to improve performance and reduce redundant database queries
+
 [1.53.0] - 2024-08-22
 ---------------------
 * perf: Introduced db_index on the `created` and `is_blacklisted` fields in `XBlockSkillData` model 
diff --git a/taxonomy/__init__.py b/taxonomy/__init__.py
index d450b26..738ee11 100644
--- a/taxonomy/__init__.py
+++ b/taxonomy/__init__.py
@@ -15,6 +15,6 @@
 # 2. MINOR version when you add functionality in a backwards compatible manner, and
 # 3. PATCH version when you make backwards compatible bug fixes.
 # More details can be found at https://semver.org/
-__version__ = '1.53.0'
+__version__ = '1.54.0'
 
 default_app_config = 'taxonomy.apps.TaxonomyConfig'  # pylint: disable=invalid-name
diff --git a/taxonomy/api/v1/views.py b/taxonomy/api/v1/views.py
index fd1db4c..0ee1074 100644
--- a/taxonomy/api/v1/views.py
+++ b/taxonomy/api/v1/views.py
@@ -4,6 +4,7 @@
 from collections import OrderedDict
 
 from django_filters.rest_framework import DjangoFilterBackend
+from edx_django_utils.cache import TieredCache, get_cache_key
 from rest_framework import permissions
 from rest_framework.filters import OrderingFilter
 from rest_framework.generics import ListAPIView, RetrieveAPIView
@@ -27,6 +28,7 @@
     SkillsQuizSerializer,
     XBlocksSkillsSerializer,
 )
+from taxonomy.constants import CACHE_TIMEOUT_XBLOCK_SKILLS_SECONDS
 from taxonomy.models import (
     CourseSkills,
     Job,
@@ -320,6 +322,19 @@ def get_queryset(self):
             ),
         ).only('id', 'skills', 'usage_key', 'requires_verification', 'auto_processed', 'hash_content')
 
+    def list(self, request, *args, **kwargs):
+        cache_key = get_cache_key(domain='taxonomy', subdomain='xblock_skills', params=request.query_params)
+
+        cached_response = TieredCache.get_cached_response(cache_key)
+        if cached_response.is_found:
+            return Response(cached_response.value)
+
+        response = super().list(request, *args, **kwargs)
+
+        TieredCache.set_all_tiers(cache_key, response.data, CACHE_TIMEOUT_XBLOCK_SKILLS_SECONDS)
+
+        return response
+
 
 class JobPathAPIView(TaxonomyAPIViewSetMixin, RetrieveAPIView):
     """
diff --git a/taxonomy/constants.py b/taxonomy/constants.py
index fe03b0d..74263e6 100644
--- a/taxonomy/constants.py
+++ b/taxonomy/constants.py
@@ -146,3 +146,5 @@ def get_job_posting_query_filter(jobs=None):
 JOB_SOURCE_COURSE_SKILL = 'course_skill'
 JOB_SOURCE_INDUSTRY = 'industry'
 JOB_SKILLS_URL_NAME = 'job-skills'
+
+CACHE_TIMEOUT_XBLOCK_SKILLS_SECONDS = 60 * 60
diff --git a/tests/test_views.py b/tests/test_views.py
index 388844b..ae27a0c 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -6,6 +6,7 @@
 from random import randint
 
 import mock
+from edx_django_utils.cache import TieredCache
 from mock import patch
 from pytest import mark
 from rest_framework import status
@@ -626,6 +627,31 @@ def test_xblocks_api_with_skill_validation_disabled(self):
             assert api_response.status_code == 200
             assert api_response.json() == []
 
+    def test_list_xblock_skills_cache_hit(self):
+        cached_data = {'results': [{'id': str(self.xblock_skills[0].id)}]}
+        with patch.object(
+            TieredCache,
+            'get_cached_response',
+            wraps=TieredCache.get_cached_response
+        ) as mock_get_cached_response:
+            mock_get_cached_response.return_value = mock.MagicMock(is_found=True, value=cached_data)
+            response = self.client.get(self.view_url)
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            self.assertEqual(response.data, cached_data)
+            mock_get_cached_response.assert_called_once()
+
+    def test_list_xblock_skills_caching(self):
+        with patch.object(TieredCache, 'set_all_tiers') as mock_set_all_tiers, \
+             patch.object(TieredCache, 'get_cached_response', wraps=TieredCache.get_cached_response) \
+                as mock_get_cached_response:
+
+            mock_get_cached_response.return_value = mock.MagicMock(is_found=False)
+            response = self.client.get(self.view_url)
+
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+            mock_get_cached_response.assert_called_once()
+            mock_set_all_tiers.assert_called_once()
+
 
 @mark.django_db
 class TestJobPathAPIView(TestCase):