Skip to content

Commit

Permalink
setting cache for get_catalog_results from discovery
Browse files Browse the repository at this point in the history
  • Loading branch information
HammadAhmadWaqas committed Aug 9, 2019
1 parent bb09fbc commit 2256e6e
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
31 changes: 26 additions & 5 deletions enterprise/api_client/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -116,6 +118,14 @@ def traverse_pagination(response, endpoint, content_filter_query, query_params):

return results

def _get_catalog_results_from_discovery(self, content_filter_query, query_params=None, traverse_pagination=False):
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 discovery service's search/all endpoint.
Expand All @@ -132,11 +142,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.DISCOVERY_GET_CATALOG_RESULT_CACHE_TIMEOUT)
except Exception as ex: # pylint: disable=broad-except
LOGGER.exception(
'Attempted to call course-discovery search/all/ endpoint with the following parameters: '
Expand Down
2 changes: 2 additions & 0 deletions enterprise/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ def root(*args):

DEFAULT_FROM_EMAIL = '[email protected]'

DISCOVERY_GET_CATALOG_RESULT_CACHE_TIMEOUT = 5 * 60 # 5 minutes

USER_THROTTLE_RATE = '90/minute'
SERVICE_USER_THROTTLE_RATE = '100/minute'
REST_FRAMEWORK = {
Expand Down
19 changes: 19 additions & 0 deletions tests/test_enterprise/api_client/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
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):
"""
Expand Down

0 comments on commit 2256e6e

Please sign in to comment.