Skip to content

Commit

Permalink
Merge pull request openedx#545 from edx/hammad/ENT-2168
Browse files Browse the repository at this point in the history
ENT-2168 | setting cache for get_catalog_results from discovery
  • Loading branch information
HammadAhmadWaqas authored Aug 19, 2019
2 parents 55011cd + 2420d53 commit 316da6c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 7 deletions.
37 changes: 30 additions & 7 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,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.
Expand All @@ -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: '
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): # 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):
"""
Expand Down

0 comments on commit 316da6c

Please sign in to comment.