Merge pull request #12565 from edx/renzo/program-by-id

Extend edX API utility to support retrieval of specific resources

openedx/core/lib/edx_api_utils.py

@@ -11,23 +11,25 @@ from openedx.core.lib.token_utils import get_id_token
 log = logging.getLogger(__name__)
 
 
-def get_edx_api_data(api_config, user, resource, querystring=None, cache_key=None):
+def get_edx_api_data(api_config, user, resource, resource_id=None, querystring=None, cache_key=None):
-    """Fetch data from an API using provided API configuration and resource
+    """GET data from an edX REST API.
-        name.
+
+    DRY utility for handling caching and pagination.
 
     Arguments:
-        api_config (ConfigurationModel): The configuration model governing
+        api_config (ConfigurationModel): The configuration model governing interaction with the API.
-            interaction with the API.
         user (User): The user to authenticate as when requesting data.
-        resource(str): Name of the API resource for which data is being
+        resource (str): Name of the API resource being requested.
-            requested.
+
-        querystring(dict): Querystring parameters that might be required to
+    Keyword Arguments:
-            request data.
+        resource_id (int or str): Identifies a specific resource to be retrieved.
-        cache_key(str): Where to cache retrieved data. Omitting this will cause the
+        querystring (dict): Optional query string parameters.
-            cache to be bypassed.
+        cache_key (str): Where to cache retrieved data. The cache will be ignored if this is omitted
+            (neither inspected nor updated).
 
     Returns:
-        list of dict, representing data returned by the API.
+        Data returned by the API. When hitting a list endpoint, extracts "results" (list of dict)
+        returned by DRF-powered APIs.
     """
     no_data = []
 
@@ -36,30 +38,29 @@ def get_edx_api_data(api_config, user, resource, querystring=None, cache_key=Non
         return no_data
 
     if cache_key:
+        cache_key = '{}.{}'.format(cache_key, resource_id) if resource_id else cache_key
+
         cached = cache.get(cache_key)
-        if cached is not None:
+        if cached:
             return cached
 
     try:
         jwt = get_id_token(user, api_config.OAUTH2_CLIENT_NAME)
         api = EdxRestApiClient(api_config.internal_api_url, jwt=jwt)
-    except Exception:  # pylint: disable=broad-except
+    except:  # pylint: disable=bare-except
         log.exception('Failed to initialize the %s API client.', api_config.API_NAME)
         return no_data
 
     try:
-        querystring = {} if not querystring else querystring
+        endpoint = getattr(api, resource)
-        response = getattr(api, resource).get(**querystring)
+        querystring = querystring if querystring else {}
-        results = response.get('results', no_data)
+        response = endpoint(resource_id).get(**querystring)
-        page = 1
+
-        next_page = response.get('next', None)
+        if resource_id:
-        while next_page:
+            results = response
-            page += 1
+        else:
-            querystring['page'] = page
+            results = _traverse_pagination(response, endpoint, querystring, no_data)
-            response = getattr(api, resource).get(**querystring)
+    except:  # pylint: disable=bare-except
-            results += response.get('results', no_data)
-            next_page = response.get('next', None)
-    except Exception:  # pylint: disable=broad-except
         log.exception('Failed to retrieve data from the %s API.', api_config.API_NAME)
         return no_data
 
@@ -67,3 +68,22 @@ def get_edx_api_data(api_config, user, resource, querystring=None, cache_key=Non
         cache.set(cache_key, results, api_config.cache_ttl)
 
     return results
+
+
+def _traverse_pagination(response, endpoint, querystring, no_data):
+    """Traverse a paginated API response.
+
+    Extracts and concatenates "results" (list of dict) returned by DRF-powered APIs.
+    """
+    results = response.get('results', no_data)
+
+    page = 1
+    next_page = response.get('next')
+    while next_page:
+        page += 1
+        querystring['page'] = page
+        response = endpoint.get(**querystring)
+        results += response.get('results', no_data)
+        next_page = response.get('next')
+
+    return results

openedx/core/lib/tests/test_edx_api_utils.py

@@ -1,117 +1,197 @@
-"""Tests covering Api utils."""
+"""Tests covering edX API utilities."""
+import json
 import unittest
 
-from django.conf import settings
 from django.core.cache import cache
-from django.test import TestCase
 import httpretty
 import mock
 from nose.plugins.attrib import attr
 from edx_oauth2_provider.tests.factories import ClientFactory
 from provider.constants import CONFIDENTIAL
-from testfixtures import LogCapture
 
-from openedx.core.djangoapps.credentials.models import CredentialsApiConfig
-from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin, CredentialsDataMixin
 from openedx.core.djangoapps.programs.models import ProgramsApiConfig
-from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin, ProgramsDataMixin
+from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin
 from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
 from openedx.core.lib.edx_api_utils import get_edx_api_data
 from student.tests.factories import UserFactory
 
 
-LOGGER_NAME = 'openedx.core.lib.edx_api_utils'
+UTILITY_MODULE = 'openedx.core.lib.edx_api_utils'
 
 
 @attr('shard_2')
-class TestApiDataRetrieval(CredentialsApiConfigMixin, CredentialsDataMixin, ProgramsApiConfigMixin, ProgramsDataMixin,
+@httpretty.activate
-                           CacheIsolationTestCase):
+class TestGetEdxApiData(ProgramsApiConfigMixin, CacheIsolationTestCase):
-    """Test utility for API data retrieval."""
+    """Tests for edX API data retrieval utility."""
 
     ENABLED_CACHES = ['default']
 
     def setUp(self):
-        super(TestApiDataRetrieval, self).setUp()
+        super(TestGetEdxApiData, self).setUp()
-        ClientFactory(name=CredentialsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL)
+
-        ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL)
         self.user = UserFactory()
+        ClientFactory(name=ProgramsApiConfig.OAUTH2_CLIENT_NAME, client_type=CONFIDENTIAL)
 
         cache.clear()
 
-    @httpretty.activate
+    def _mock_programs_api(self, responses, url=None):
-    def test_get_edx_api_data_programs(self):
+        """Helper for mocking out Programs API URLs."""
-        """Verify programs data can be retrieved using get_edx_api_data."""
+        self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Programs API calls.')
-        program_config = self.create_programs_config()
-        self.mock_programs_api()
 
-        actual = get_edx_api_data(program_config, self.user, 'programs')
+        url = url if url else ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/'
-        self.assertEqual(
+
-            actual,
+        httpretty.register_uri(httpretty.GET, url, responses=responses)
-            self.PROGRAMS_API_RESPONSE['results']
+
+    def _assert_num_requests(self, count):
+        """DRY helper for verifying request counts."""
+        self.assertEqual(len(httpretty.httpretty.latest_requests), count)
+
+    def test_get_unpaginated_data(self):
+        """Verify that unpaginated data can be retrieved."""
+        program_config = self.create_programs_config()
+
+        expected_collection = ['some', 'test', 'data']
+        data = {
+            'next': None,
+            'results': expected_collection,
+        }
+
+        self._mock_programs_api(
+            [httpretty.Response(body=json.dumps(data), content_type='application/json')]
         )
 
-        # Verify the API was actually hit (not the cache).
+        actual_collection = get_edx_api_data(program_config, self.user, 'programs')
-        self.assertEqual(len(httpretty.httpretty.latest_requests), 1)
+        self.assertEqual(actual_collection, expected_collection)
 
-    def test_get_edx_api_data_disable_config(self):
+        # Verify the API was actually hit (not the cache)
-        """Verify no data is retrieved if configuration is disabled."""
+        self._assert_num_requests(1)
+
+    def test_get_paginated_data(self):
+        """Verify that paginated data can be retrieved."""
+        program_config = self.create_programs_config()
+
+        expected_collection = ['some', 'test', 'data']
+        url = ProgramsApiConfig.current().internal_api_url.strip('/') + '/programs/?page={}'
+
+        responses = []
+        for page, record in enumerate(expected_collection, start=1):
+            data = {
+                'next': url.format(page + 1) if page < len(expected_collection) else None,
+                'results': [record],
+            }
+
+            body = json.dumps(data)
+            responses.append(
+                httpretty.Response(body=body, content_type='application/json')
+            )
+
+        self._mock_programs_api(responses)
+
+        actual_collection = get_edx_api_data(program_config, self.user, 'programs')
+        self.assertEqual(actual_collection, expected_collection)
+
+        self._assert_num_requests(len(expected_collection))
+
+    def test_get_specific_resource(self):
+        """Verify that a specific resource can be retrieved."""
+        program_config = self.create_programs_config()
+
+        resource_id = 1
+        url = '{api_root}/programs/{resource_id}/'.format(
+            api_root=ProgramsApiConfig.current().internal_api_url.strip('/'),
+            resource_id=resource_id,
+        )
+
+        expected_resource = {'key': 'value'}
+
+        self._mock_programs_api(
+            [httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
+            url=url
+        )
+
+        actual_resource = get_edx_api_data(program_config, self.user, 'programs', 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."""
+        program_config = self.create_programs_config(cache_ttl=5)
+
+        expected_collection = ['some', 'test', 'data']
+        data = {
+            'next': None,
+            'results': expected_collection,
+        }
+
+        self._mock_programs_api(
+            [httpretty.Response(body=json.dumps(data), content_type='application/json')],
+        )
+
+        resource_id = 1
+        url = '{api_root}/programs/{resource_id}/'.format(
+            api_root=ProgramsApiConfig.current().internal_api_url.strip('/'),
+            resource_id=resource_id,
+        )
+
+        expected_resource = {'key': 'value'}
+
+        self._mock_programs_api(
+            [httpretty.Response(body=json.dumps(expected_resource), content_type='application/json')],
+            url=url
+        )
+
+        cache_key = ProgramsApiConfig.current().CACHE_KEY
+
+        # Warm up the cache.
+        get_edx_api_data(program_config, self.user, 'programs', cache_key=cache_key)
+        get_edx_api_data(program_config, self.user, 'programs', resource_id=resource_id, cache_key=cache_key)
+
+        # Hit the cache.
+        actual_collection = get_edx_api_data(program_config, self.user, 'programs', cache_key=cache_key)
+        self.assertEqual(actual_collection, expected_collection)
+
+        actual_resource = get_edx_api_data(
+            program_config, self.user, 'programs', resource_id=resource_id, cache_key=cache_key
+        )
+        self.assertEqual(actual_resource, expected_resource)
+
+        # Verify that only two requests were made, not four.
+        self._assert_num_requests(2)
+
+    @mock.patch(UTILITY_MODULE + '.log.warning')
+    def test_api_config_disabled(self, mock_warning):
+        """Verify that no data is retrieved if the provided config model is disabled."""
         program_config = self.create_programs_config(enabled=False)
 
         actual = get_edx_api_data(program_config, self.user, 'programs')
+
+        self.assertTrue(mock_warning.called)
         self.assertEqual(actual, [])
 
-    @httpretty.activate
-    def test_get_edx_api_data_cache(self):
-        """Verify that when enabled, the cache is used."""
-        program_config = self.create_programs_config(cache_ttl=1)
-        self.mock_programs_api()
-
-        # Warm up the cache.
-        get_edx_api_data(program_config, self.user, 'programs', cache_key='test.key')
-
-        # Hit the cache.
-        get_edx_api_data(program_config, self.user, 'programs', cache_key='test.key')
-
-        # Verify only one request was made.
-        self.assertEqual(len(httpretty.httpretty.latest_requests), 1)
-
     @mock.patch('edx_rest_api_client.client.EdxRestApiClient.__init__')
-    def test_get_edx_api_data_client_initialization_failure(self, mock_init):
+    @mock.patch(UTILITY_MODULE + '.log.exception')
-        """Verify no data is retrieved and exception logged when API client
+    def test_client_initialization_failure(self, mock_exception, mock_init):
-        fails to initialize.
+        """Verify that an exception is logged when the API client fails to initialize."""
-        """
-        program_config = self.create_programs_config()
         mock_init.side_effect = Exception
 
-        with LogCapture(LOGGER_NAME) as logger:
-            actual = get_edx_api_data(program_config, self.user, 'programs')
-            logger.check(
-                (LOGGER_NAME, 'ERROR', u'Failed to initialize the programs API client.')
-            )
-            self.assertEqual(actual, [])
-            self.assertTrue(mock_init.called)
-
-    @httpretty.activate
-    def test_get_edx_api_data_retrieval_failure(self):
-        """Verify exception is logged when data can't be retrieved from API."""
         program_config = self.create_programs_config()
-        self.mock_programs_api(status_code=500)
-        with LogCapture(LOGGER_NAME) as logger:
-            actual = get_edx_api_data(program_config, self.user, 'programs')
-            logger.check(
-                (LOGGER_NAME, 'ERROR', u'Failed to retrieve data from the programs API.')
-            )
-            self.assertEqual(actual, [])
 
-    # this test is skipped under cms because the credentials app is only installed under LMS.
+        actual = get_edx_api_data(program_config, self.user, 'programs')
-    @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
-    @httpretty.activate
-    def test_get_edx_api_data_multiple_page(self):
-        """Verify that all data is retrieve for multiple page response."""
-        credentials_config = self.create_credentials_config()
-        self.mock_credentials_api(self.user, is_next_page=True)
-        querystring = {'username': self.user.username}
 
-        actual = get_edx_api_data(credentials_config, self.user, 'user_credentials', querystring=querystring)
+        self.assertTrue(mock_exception.called)
-        expected_data = self.CREDENTIALS_NEXT_API_RESPONSE['results'] + self.CREDENTIALS_API_RESPONSE['results']
+        self.assertEqual(actual, [])
-        self.assertEqual(actual, expected_data)
+
+    @mock.patch(UTILITY_MODULE + '.log.exception')
+    def test_data_retrieval_failure(self, mock_exception):
+        """Verify that an exception is logged when data can't be retrieved."""
+        program_config = self.create_programs_config()
+
+        self._mock_programs_api(
+            [httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
+        )
+
+        actual = get_edx_api_data(program_config, self.user, 'programs')
+
+        self.assertTrue(mock_exception.called)
+        self.assertEqual(actual, [])