diff --git a/enterprise_subsidy/apps/api/v1/tests/test_views.py b/enterprise_subsidy/apps/api/v1/tests/test_views.py index e79478a1..763a6da7 100644 --- a/enterprise_subsidy/apps/api/v1/tests/test_views.py +++ b/enterprise_subsidy/apps/api/v1/tests/test_views.py @@ -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, @@ -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, @@ -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())]) @@ -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, + }, + ) diff --git a/enterprise_subsidy/apps/api/v1/views/content_metadata.py b/enterprise_subsidy/apps/api/v1/views/content_metadata.py index 90a1c81f..76d80dfd 100644 --- a/enterprise_subsidy/apps/api/v1/views/content_metadata.py +++ b/enterprise_subsidy/apps/api/v1/views/content_metadata.py @@ -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}") diff --git a/enterprise_subsidy/apps/api_client/enterprise_catalog.py b/enterprise_subsidy/apps/api_client/enterprise_catalog.py index 721577cd..c8846b49 100644 --- a/enterprise_subsidy/apps/api_client/enterprise_catalog.py +++ b/enterprise_subsidy/apps/api_client/enterprise_catalog.py @@ -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. @@ -40,6 +42,12 @@ 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. This sub-call helps the catalog service + understand the last time a catalog's customer record was modified, and also helps + to construct course and course run enrollment URLs that are usually not needed + in the context of enterprise-subsidy or callers of the EnterpriseCustomerViewSet. + Defaults to False. Returns: response (dict): JSON response object associated with a content metadata record Raises: @@ -48,8 +56,11 @@ 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 = {} + if skip_customer_fetch: + 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: diff --git a/enterprise_subsidy/apps/content_metadata/api.py b/enterprise_subsidy/apps/content_metadata/api.py index df8c603b..bd80b4bc 100644 --- a/enterprise_subsidy/apps/content_metadata/api.py +++ b/enterprise_subsidy/apps/content_metadata/api.py @@ -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) @@ -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, @@ -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( diff --git a/enterprise_subsidy/settings/base.py b/enterprise_subsidy/settings/base.py index 62721087..5ded0a9c 100644 --- a/enterprise_subsidy/settings/base.py +++ b/enterprise_subsidy/settings/base.py @@ -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