From 0e1e6a9b43ed24948da881603fe8836a14678e24 Mon Sep 17 00:00:00 2001 From: Alexander Dusenbery Date: Tue, 8 Oct 2024 15:22:27 -0400 Subject: [PATCH] refactor: refactor get_metadata_by_query() to allow extra params This will allow us to selective request discovery search/all results that include b2b-restricted results. ENT-9570 --- .../apps/api_client/discovery.py | 63 +++++++++++++------ .../apps/api_client/tests/test_discovery.py | 23 +++++++ 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/enterprise_catalog/apps/api_client/discovery.py b/enterprise_catalog/apps/api_client/discovery.py index 3e4d5d644..658e62ea8 100644 --- a/enterprise_catalog/apps/api_client/discovery.py +++ b/enterprise_catalog/apps/api_client/discovery.py @@ -49,13 +49,14 @@ def _calculate_backoff(self, attempt_count): """ return (self.BACKOFF_FACTOR * (2 ** (attempt_count - 1))) - def _retrieve_metadata_for_content_filter(self, content_filter, page, request_params): + def _retrieve_metadata_page_for_content_filter(self, content_filter, page, request_params): """ Makes a request to discovery's /search/all/ endpoint with the specified content_filter, page, and request_params """ LOGGER.info(f'Retrieving results from course-discovery for page {page}...') attempts = 0 + request_params_with_page = request_params | {'page': page} while True: attempts = attempts + 1 successful = True @@ -64,7 +65,7 @@ def _retrieve_metadata_for_content_filter(self, content_filter, page, request_pa response = self.client.post( DISCOVERY_SEARCH_ALL_ENDPOINT, json=content_filter, - params=request_params, + params=request_params_with_page, timeout=self.HTTP_TIMEOUT, ) successful = response.status_code < 400 @@ -99,6 +100,41 @@ def _retrieve_metadata_for_content_filter(self, content_filter, page, request_pa ) raise err + def retrieve_metadata_for_content_filter(self, content_filter, request_params): + """ + Given a content filter and query params dict, makes one or more requests to the + discovery service to fetch search results based on the filter and concatenates + all results into a returned list. + """ + request_params_customized = request_params | { + # Increase number of results per page for the course-discovery response + 'page_size': 100, + # Ensure paginated results are consistently ordered by `aggregation_key` and `start` + 'ordering': 'aggregation_key,start', + } + page = 1 + results = [] + try: + response = self._retrieve_metadata_page_for_content_filter( + content_filter, page, request_params_customized, + ) + results += response.get('results', []) + # Traverse all pages and concatenate results + while response.get('next'): + page += 1 + response = self._retrieve_metadata_page_for_content_filter( + content_filter, page, request_params_customized, + ) + results += response.get('results', []) + except Exception as exc: + LOGGER.exception( + 'Could not retrieve content items from course-discovery (page %s): %s', + page, + exc, + ) + raise exc + return results + def _retrieve_course_reviews(self, request_params): """ Makes a request to discovery's /api/v1/course_review/ paginated endpoint @@ -251,7 +287,7 @@ def get_video_skills(self, video_usage_key): return video_skills - def get_metadata_by_query(self, catalog_query): + def get_metadata_by_query(self, catalog_query, extra_query_params=None): """ Return results from the discovery service's search/all endpoint. @@ -264,30 +300,17 @@ def get_metadata_by_query(self, catalog_query): request_params = { # Omit non-active course runs from the course-discovery results 'exclude_expired_course_run': True, - # Increase number of results per page for the course-discovery response - 'page_size': 100, - # Ensure paginated results are consistently ordered by `aggregation_key` and `start` - 'ordering': 'aggregation_key,start', # Ensure to fetch learner pathways as part of search/all endpoint response. 'include_learner_pathways': True, - } - - page = 1 + } | (extra_query_params or {}) results = [] + try: content_filter = catalog_query.content_filter - response = self._retrieve_metadata_for_content_filter(content_filter, page, request_params) - results += response.get('results', []) - # Traverse all pages and concatenate results - while response.get('next'): - page += 1 - request_params.update({'page': page}) - response = self._retrieve_metadata_for_content_filter(content_filter, page, request_params) - results += response.get('results', []) + results.extend(self.retrieve_metadata_for_content_filter(content_filter, request_params)) except Exception as exc: LOGGER.exception( - 'Could not retrieve content items from course-discovery (page %s) for catalog query %s: %s', - page, + 'Could not retrieve content items for catalog query %s: %s', catalog_query, exc, ) diff --git a/enterprise_catalog/apps/api_client/tests/test_discovery.py b/enterprise_catalog/apps/api_client/tests/test_discovery.py index a125bc5fc..12f7b95c0 100644 --- a/enterprise_catalog/apps/api_client/tests/test_discovery.py +++ b/enterprise_catalog/apps/api_client/tests/test_discovery.py @@ -36,6 +36,29 @@ def test_get_metadata_by_query_with_results(self, mock_oauth_client): expected_response = [{'key': 'fakeX'}] self.assertEqual(actual_response, expected_response) + @mock.patch('enterprise_catalog.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_metadata_by_query_with_extra_query_params(self, mock_oauth_client): + """ + get_metadata_by_query should call discovery endpoint, but not call + traverse_pagination if traverse_pagination is false. + """ + mock_oauth_client.return_value.post.return_value.status_code = 200 + mock_oauth_client.return_value.post.return_value.json.return_value = { + 'results': [{'key': 'fakeX'}], + } + + catalog_query = CatalogQueryFactory() + client = DiscoveryApiClient() + actual_response = client.get_metadata_by_query(catalog_query, {'do_cool_things': True}) + + post_request = mock_oauth_client.return_value.post + post_request.assert_called_once() + called_params = post_request.call_args_list[0][1]['params'] + self.assertTrue(called_params.get('do_cool_things')) + + expected_response = [{'key': 'fakeX'}] + self.assertEqual(actual_response, expected_response) + @mock.patch('enterprise_catalog.apps.api_client.base_oauth.OAuthAPIClient') def test_get_metadata_by_query_with_retry_and_error(self, mock_oauth_client): """