Add many=True kwarg to get_edx_api_data to modify empty return value; make…

Add many=True kwarg to get_edx_api_data to modify empty return value; make resource_id checks slightly more strict

openedx/core/lib/edx_api_utils.py

@@ -15,7 +15,7 @@ log = logging.getLogger(__name__)
 
 
 def get_edx_api_data(api_config, user, resource,
-                     api=None, resource_id=None, querystring=None, cache_key=None):
+                     api=None, resource_id=None, querystring=None, cache_key=None, many=True):
     """GET data from an edX REST API.
 
     DRY utility for handling caching and pagination.
@@ -31,19 +31,21 @@ def get_edx_api_data(api_config, user, resource,
         querystring (dict): Optional query string parameters.
         cache_key (str): Where to cache retrieved data. The cache will be ignored if this is omitted
             (neither inspected nor updated).
+        many (bool): Whether the resource requested is a collection of objects, or a single object.
+            If false, an empty dict will be returned in cases of failure rather than the default empty list.
 
     Returns:
         Data returned by the API. When hitting a list endpoint, extracts "results" (list of dict)
         returned by DRF-powered APIs.
     """
-    no_data = []
+    no_data = [] if many else {}
 
     if not api_config.enabled:
         log.warning('%s configuration is disabled.', api_config.API_NAME)
         return no_data
 
     if cache_key:
-        cache_key = '{}.{}'.format(cache_key, resource_id) if resource_id else cache_key
+        cache_key = '{}.{}'.format(cache_key, resource_id) if resource_id is not None else cache_key
 
         cached = cache.get(cache_key)
         if cached:
@@ -75,7 +77,7 @@ def get_edx_api_data(api_config, user, resource,
         querystring = querystring if querystring else {}
         response = endpoint(resource_id).get(**querystring)
 
-        if resource_id:
+        if resource_id is not None:
             results = response
         else:
             results = _traverse_pagination(response, endpoint, querystring, no_data)

openedx/core/lib/tests/test_edx_api_utils.py

@@ -124,6 +124,35 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
 
         self._assert_num_requests(1)
 
+    def test_get_specific_resource_with_falsey_id(self):
+        """
+        Verify that a specific resource can be retrieved, and pagination parsing is
+        not attempted, if the ID passed is falsey (e.g., 0). The expected resource contains
+        a "results" key, as a paginatable item would have, so if the function looks for falsey
+        values in the resource_id field, rather than specifically None, the function will
+        return the value of that "results" key.
+        """
+        catalog_integration = self.create_catalog_integration()
+        api = create_catalog_api_client(self.user, catalog_integration)
+
+        resource_id = 0
+        url = '{api_root}/programs/{resource_id}/'.format(
+            api_root=CatalogIntegration.current().internal_api_url.strip('/'),
+            resource_id=resource_id,
+        )
+
+        expected_resource = {'key': 'value', 'results': []}
+
+        self._mock_catalog_api(
+            [httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
+            url=url
+        )
+
+        actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id)
+        self.assertEqual(actual_resource, expected_resource)
+
+        self._assert_num_requests(1)
+
     def test_cache_utilization(self):
         """Verify that when enabled, the cache is used."""
         catalog_integration = self.create_catalog_integration(cache_ttl=5)
@@ -210,6 +239,61 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         self.assertTrue(mock_exception.called)
         self.assertEqual(actual, [])
 
+    @mock.patch(UTILITY_MODULE + '.log.warning')
+    def test_api_config_disabled_with_id_and_not_collection(self, mock_warning):
+        """Verify that no data is retrieved if the provided config model is disabled."""
+        catalog_integration = self.create_catalog_integration(enabled=False)
+
+        actual = get_edx_api_data(
+            catalog_integration,
+            self.user,
+            'programs',
+            resource_id=100,
+            many=False,
+        )
+
+        self.assertTrue(mock_warning.called)
+        self.assertEqual(actual, {})
+
+    @mock.patch('edx_rest_api_client.client.EdxRestApiClient.__init__')
+    @mock.patch(UTILITY_MODULE + '.log.exception')
+    def test_client_initialization_failure_with_id(self, mock_exception, mock_init):
+        """Verify that an exception is logged when the API client fails to initialize."""
+        mock_init.side_effect = Exception
+
+        catalog_integration = self.create_catalog_integration()
+
+        actual = get_edx_api_data(
+            catalog_integration,
+            self.user,
+            'programs',
+            resource_id=100,
+            many=False,
+        )
+        self.assertTrue(mock_exception.called)
+        self.assertEqual(actual, {})
+
+    @mock.patch(UTILITY_MODULE + '.log.exception')
+    def test_data_retrieval_failure_with_id(self, mock_exception):
+        """Verify that an exception is logged when data can't be retrieved."""
+        catalog_integration = self.create_catalog_integration()
+        api = create_catalog_api_client(self.user, catalog_integration)
+
+        self._mock_catalog_api(
+            [httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
+        )
+
+        actual = get_edx_api_data(
+            catalog_integration,
+            self.user,
+            'programs',
+            api=api,
+            resource_id=100,
+            many=False,
+        )
+        self.assertTrue(mock_exception.called)
+        self.assertEqual(actual, {})
+
     def test_api_client_not_provided(self):
         """Verify that an API client doesn't need to be provided."""
         ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL)