Skip to content

Commit

Permalink
feat: skip customer fetch in content_metadata view
Browse files Browse the repository at this point in the history
  • Loading branch information
iloveagent57 committed Oct 30, 2023
1 parent 9a7e56b commit 715e8b9
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 10 deletions.
29 changes: 27 additions & 2 deletions enterprise_subsidy/apps/api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,11 @@ def test_successful_get(
self.set_up_admin(enterprise_uuids=[str(customer_uuid)])
mock_oauth_client.return_value.get.return_value = MockResponse(mock_metadata, 200)
url = reverse('api:v1:content-metadata', kwargs={'content_identifier': expected_content_key})

# Make a first call that should not run into a cached response
# from the cached EnterpriseCustomerViewSet.get view.
response = self.client.get(url + f'?enterprise_customer_uuid={str(customer_uuid)}')

assert response.status_code == 200
assert response.json() == {
'content_title': expected_content_title,
Expand All @@ -1444,10 +1448,14 @@ def test_successful_get(
'geag_variant_id': expected_geag_variant_id,
}

# Everything after this line is testing the view's cache
# If this mock response is ever hit, the test will fail, caching prevents it.
# Now make a second call to validate that the view-level cache is utilized.
# This means we won't make a second request via the enterprise catalog API client.
# Adding an exception side effect proves that we don't actually make
# the call with the client.
mock_oauth_client.return_value.get.side_effect = Exception("Does not reach this")

response = self.client.get(url + f'?enterprise_customer_uuid={str(customer_uuid)}')

assert response.status_code == 200
assert response.json() == {
'content_title': expected_content_title,
Expand All @@ -1460,6 +1468,15 @@ def test_successful_get(
'mode': expected_mode,
'geag_variant_id': expected_geag_variant_id,
}
# Validate that, in the first, non-cached request, we call
# the enterprise catalog endpoint via the client, and that
# a `skip_customer_fetch` parameter is included in the request.
mock_oauth_client.return_value.get.assert_called_once_with(
f'api/v1/enterprise-customer/{customer_uuid}/content-metadata/{expected_content_key}/',
params={
'skip_customer_fetch': True,
},
)

def test_failure_no_permission(self):
self.set_up_admin(enterprise_uuids=[str(uuid.uuid4())])
Expand Down Expand Up @@ -1494,6 +1511,14 @@ def test_failure_exception_while_gather_metadata(self, catalog_status_code, expe
url=self.mock_http_error_url
)
url = reverse('api:v1:content-metadata', kwargs={'content_identifier': 'content_key'})

response = self.client.get(url + f'?enterprise_customer_uuid={str(customer_uuid)}')

assert response.status_code == catalog_status_code
assert response.json() == expected_response
mock_oauth_client.return_value.get.assert_called_once_with(
f'api/v1/enterprise-customer/{customer_uuid}/content-metadata/content_key/',
params={
'skip_customer_fetch': True,
},
)
3 changes: 2 additions & 1 deletion enterprise_subsidy/apps/api/v1/views/content_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ def get(self, request, content_identifier, enterprise_customer_uuid):
try:
content_summary = self.content_metadata_api().get_content_summary(
enterprise_customer_uuid[0],
content_identifier
content_identifier,
skip_customer_fetch=True,
)
if not content_summary.get('content_price'):
logger.warning(f"Could not find course price in metadata for {content_identifier}")
Expand Down
9 changes: 7 additions & 2 deletions enterprise_subsidy/apps/api_client/enterprise_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ def content_metadata_url(self, enterprise_customer_uuid, content_identifier):
f'content-metadata/{content_identifier}/'
)

def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_identifier):
def get_content_metadata_for_customer(
self, enterprise_customer_uuid, content_identifier, skip_customer_fetch=False, **kwargs
):
"""
Returns Enterprise Customer related data for a specified piece on content.
Expand All @@ -40,6 +42,8 @@ def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_id
content_identifier (str): **Either** the content UUID or content key identifier for a content record.
Note: the content needs to be owned by a catalog associated with the provided customer else this
method will throw an HTTPError.
skip_customer_fetch (bool): Forces enterprise-catalog to skip a sub-call to an edx-enterprise
API endpoint running in the edx-platform runtime. Defaults to True.
Returns:
response (dict): JSON response object associated with a content metadata record
Raises:
Expand All @@ -48,8 +52,9 @@ def get_content_metadata_for_customer(self, enterprise_customer_uuid, content_id
does not exist, or is not present in a catalog associated with the customer.
"""
content_metadata_url = self.content_metadata_url(enterprise_customer_uuid, content_identifier)
query_params = {'skip_customer_fetch': skip_customer_fetch}
try:
response = self.client.get(content_metadata_url)
response = self.client.get(content_metadata_url, params=query_params)
response.raise_for_status()
return response.json()
except requests.exceptions.HTTPError as exc:
Expand Down
10 changes: 6 additions & 4 deletions enterprise_subsidy/apps/content_metadata/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,14 @@ def get_course_run(self, content_identifier, content_data):
return course_run
return {}

def get_content_summary(self, enterprise_customer_uuid, content_identifier):
def get_content_summary(self, enterprise_customer_uuid, content_identifier, **kwargs):
"""
Returns a summary dict some content metadata, makes the client call
"""
course_details = self.get_content_metadata(
enterprise_customer_uuid,
content_identifier
content_identifier,
**kwargs,
)
return self.summary_data_for_content(content_identifier, course_details)

Expand Down Expand Up @@ -201,7 +202,7 @@ def get_geag_variant_id(self, enterprise_customer_uuid, content_identifier):
return self.get_content_summary(enterprise_customer_uuid, content_identifier).get('geag_variant_id')

@staticmethod
def get_content_metadata(enterprise_customer_uuid, content_identifier):
def get_content_metadata(enterprise_customer_uuid, content_identifier, **kwargs):
"""
Fetches details about the given content from a tiered (request + django) cache;
or it fetches from the enterprise-catalog API if not present in the cache,
Expand All @@ -214,7 +215,8 @@ def get_content_metadata(enterprise_customer_uuid, content_identifier):

course_details = EnterpriseCatalogApiClient().get_content_metadata_for_customer(
enterprise_customer_uuid,
content_identifier
content_identifier,
**kwargs,
)
if course_details:
TieredCache.set_all_tiers(
Expand Down
2 changes: 1 addition & 1 deletion enterprise_subsidy/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ def root(*path_fragments):

# per-view cache timeout settings
# We can disable caching on this view by setting the value below to 0.
CONTENT_METADATA_VIEW_CACHE_TIMEOUT_SECONDS = 60
CONTENT_METADATA_VIEW_CACHE_TIMEOUT_SECONDS = 60 * 15

# disable indexing on history_date
SIMPLE_HISTORY_DATE_INDEX = False

0 comments on commit 715e8b9

Please sign in to comment.