From 2420d53c15b141594492db6dc41cf1431393f1e4 Mon Sep 17 00:00:00 2001
From: Hammad Ahmad Waqas <hammadahmadwaqas1@gmail.com>
Date: Fri, 9 Aug 2019 14:49:50 +0500
Subject: [PATCH] setting cache for get_catalog_results from discovery

---
 enterprise/api_client/discovery.py            | 37 +++++++++++++++----
 .../api_client/test_discovery.py              | 19 ++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/enterprise/api_client/discovery.py b/enterprise/api_client/discovery.py
index f3b9ccae35..5a67f0638f 100644
--- a/enterprise/api_client/discovery.py
+++ b/enterprise/api_client/discovery.py
@@ -13,9 +13,11 @@
 from requests.exceptions import ConnectionError, Timeout  # pylint: disable=redefined-builtin
 
 from django.conf import settings
+from django.core.cache import cache
 from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist
 from django.utils.translation import ugettext_lazy as _
 
+from enterprise import utils
 from enterprise.utils import MultipleProgramMatchError, NotConnectedToOpenEdX, get_configuration_value_for_site
 
 try:
@@ -116,10 +118,20 @@ def traverse_pagination(response, endpoint, content_filter_query, query_params):
 
         return results
 
-    def get_catalog_results(self, content_filter_query, query_params=None, traverse_pagination=False):
+    def get_catalog_results_from_discovery(self, content_filter_query, query_params=None, traverse_pagination=False):
         """
-        Return results from the discovery service's search/all endpoint.
+            Return results from the discovery service's search/all endpoint."""
+
+        endpoint = getattr(self.client, self.SEARCH_ALL_ENDPOINT)
+        response = endpoint().post(data=content_filter_query, **query_params)
+        if traverse_pagination:
+            response['results'] = self.traverse_pagination(response, endpoint, content_filter_query, query_params)
+            response['next'] = response['previous'] = None
+        return response
 
+    def get_catalog_results(self, content_filter_query, query_params=None, traverse_pagination=False):
+        """
+            Return results from the cache or discovery service's search/all endpoint.
         Arguments:
             content_filter_query (dict): query parameters used to filter catalog results.
             query_params (dict): query parameters used to paginate results.
@@ -132,11 +144,22 @@ def get_catalog_results(self, content_filter_query, query_params=None, traverse_
         query_params = query_params or {}
 
         try:
-            endpoint = getattr(self.client, self.SEARCH_ALL_ENDPOINT)
-            response = endpoint().post(data=content_filter_query, **query_params)
-            if traverse_pagination:
-                response['results'] = self.traverse_pagination(response, endpoint, content_filter_query, query_params)
-                response['next'] = response['previous'] = None
+            cache_key = utils.get_cache_key(
+                service='discovery',
+                endpoint=self.SEARCH_ALL_ENDPOINT,
+                query=content_filter_query,
+                traverse_pagination=traverse_pagination,
+                **query_params
+            )
+            response = cache.get(cache_key)
+            if not response:
+                # Response is not cached, so make a call.
+                response = self.get_catalog_results_from_discovery(
+                    content_filter_query,
+                    query_params,
+                    traverse_pagination
+                )
+                cache.set(cache_key, response, settings.ENTERPRISE_API_CACHE_TIMEOUT)
         except Exception as ex:  # pylint: disable=broad-except
             LOGGER.exception(
                 'Attempted to call course-discovery search/all/ endpoint with the following parameters: '
diff --git a/tests/test_enterprise/api_client/test_discovery.py b/tests/test_enterprise/api_client/test_discovery.py
index a9a57ded99..2d0ca5b6e3 100644
--- a/tests/test_enterprise/api_client/test_discovery.py
+++ b/tests/test_enterprise/api_client/test_discovery.py
@@ -387,6 +387,25 @@ def test_get_catalog_results(self):
         )
         assert actual_result == response_dict
 
+    @responses.activate
+    @mock.patch.object(CourseCatalogApiClient, 'get_catalog_results_from_discovery', return_value={'result': 'dummy'})
+    def test_get_catalog_results_cache(self, mocked_get_catalog_results_from_discovery):  # pylint: disable=invalid-name
+        """
+        Verify `get_catalog_results` of CourseCatalogApiClient works as expected.
+        """
+        content_filter_query = {'content_type': 'course', 'aggregation_key': ['course:edX+DemoX']}
+        self.api.get_catalog_results(content_filter_query=content_filter_query)
+        assert mocked_get_catalog_results_from_discovery.call_count == 1
+
+        # searching same query should not hit discovery service again
+        self.api.get_catalog_results(content_filter_query=content_filter_query)
+        assert mocked_get_catalog_results_from_discovery.call_count == 1
+
+        # getting catalog with different params should hit discovery
+        content_filter_query.update({'partner': 'edx'})
+        self.api.get_catalog_results(content_filter_query=content_filter_query)
+        assert mocked_get_catalog_results_from_discovery.call_count == 2
+
     @responses.activate
     def test_get_catalog_results_with_traverse_pagination(self):
         """