Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENT-2168 | setting cache for get_catalog_results from discovery #545

Merged
merged 1 commit into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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