diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index a24f482755..aadbf9fee6 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -338,7 +338,7 @@ def get_user_orders(user): cache_key = commerce_configuration.CACHE_KEY + '.' + str(user.id) if use_cache else None api = ecommerce_api_client(user) commerce_user_orders = get_edx_api_data( - commerce_configuration, user, 'orders', api=api, querystring=user_query, cache_key=cache_key + commerce_configuration, 'orders', api=api, querystring=user_query, cache_key=cache_key ) for order in commerce_user_orders: diff --git a/openedx/core/djangoapps/catalog/tests/test_utils.py b/openedx/core/djangoapps/catalog/tests/test_utils.py index 8c28dfd9ae..791d2aae71 100644 --- a/openedx/core/djangoapps/catalog/tests/test_utils.py +++ b/openedx/core/djangoapps/catalog/tests/test_utils.py @@ -198,7 +198,7 @@ class TestGetCourseRuns(CatalogIntegrationMixin, TestCase): """ args, kwargs = call_args - for arg in (self.catalog_integration, self.user, 'course_runs'): + for arg in (self.catalog_integration, 'course_runs'): self.assertIn(arg, args) self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.internal_api_url) # pylint: disable=protected-access diff --git a/openedx/core/djangoapps/catalog/utils.py b/openedx/core/djangoapps/catalog/utils.py index f77ad42165..dc7d6c94aa 100644 --- a/openedx/core/djangoapps/catalog/utils.py +++ b/openedx/core/djangoapps/catalog/utils.py @@ -2,7 +2,6 @@ import copy import logging -import waffle from django.conf import settings from django.contrib.auth import get_user_model from edx_rest_api_client.client import EdxRestApiClient @@ -11,7 +10,6 @@ from openedx.core.djangoapps.catalog.models import CatalogIntegration from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.token_utils import JwtBuilder - log = logging.getLogger(__name__) User = get_user_model() # pylint: disable=invalid-name @@ -67,11 +65,10 @@ def get_programs(uuid=None, types=None): # pylint: disable=redefined-builtin return get_edx_api_data( catalog_integration, - user, 'programs', + api=api, resource_id=uuid, cache_key=cache_key if catalog_integration.is_cache_enabled else None, - api=api, querystring=querystring, ) else: @@ -98,13 +95,8 @@ def get_program_types(name=None): api = create_catalog_api_client(user, catalog_integration) cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY) - data = get_edx_api_data( - catalog_integration, - user, - 'program_types', - cache_key=cache_key if catalog_integration.is_cache_enabled else None, - api=api - ) + data = get_edx_api_data(catalog_integration, 'program_types', api=api, + cache_key=cache_key if catalog_integration.is_cache_enabled else None) # Filter by name if a name was provided if name: @@ -169,12 +161,6 @@ def get_course_runs(): 'exclude_utm': 1, } - course_runs = get_edx_api_data( - catalog_integration, - user, - 'course_runs', - api=api, - querystring=querystring, - ) + course_runs = get_edx_api_data(catalog_integration, 'course_runs', api=api, querystring=querystring) return course_runs diff --git a/openedx/core/djangoapps/credentials/utils.py b/openedx/core/djangoapps/credentials/utils.py index 9e381492d6..38eced303c 100644 --- a/openedx/core/djangoapps/credentials/utils.py +++ b/openedx/core/djangoapps/credentials/utils.py @@ -1,15 +1,28 @@ """Helper functions for working with Credentials.""" from __future__ import unicode_literals + import logging +from django.conf import settings +from edx_rest_api_client.client import EdxRestApiClient + from openedx.core.djangoapps.catalog.utils import get_programs from openedx.core.djangoapps.credentials.models import CredentialsApiConfig from openedx.core.lib.edx_api_utils import get_edx_api_data - +from openedx.core.lib.token_utils import JwtBuilder log = logging.getLogger(__name__) +def get_credentials_api_client(user): + """ Returns an authenticated Credentials API client. """ + + scopes = ['email', 'profile'] + expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION + jwt = JwtBuilder(user).build_token(scopes, expires_in) + return EdxRestApiClient(CredentialsApiConfig.current().internal_api_url, jwt=jwt) + + def get_credentials(user, program_uuid=None): """ Given a user, get credentials earned from the credentials service. @@ -35,9 +48,10 @@ def get_credentials(user, program_uuid=None): # want to see them displayed immediately. use_cache = credential_configuration.is_cache_enabled and not user.is_staff cache_key = credential_configuration.CACHE_KEY + '.' + user.username if use_cache else None + api = get_credentials_api_client(user) return get_edx_api_data( - credential_configuration, user, 'credentials', querystring=querystring, cache_key=cache_key + credential_configuration, 'credentials', api=api, querystring=querystring, cache_key=cache_key ) diff --git a/openedx/core/lib/edx_api_utils.py b/openedx/core/lib/edx_api_utils.py index 172cf8e6d2..fc330ee622 100644 --- a/openedx/core/lib/edx_api_utils.py +++ b/openedx/core/lib/edx_api_utils.py @@ -14,15 +14,14 @@ from openedx.core.lib.token_utils import JwtBuilder log = logging.getLogger(__name__) -def get_edx_api_data(api_config, user, resource, api=None, resource_id=None, - querystring=None, cache_key=None, many=True, traverse_pagination=True): +def get_edx_api_data(api_config, resource, api, resource_id=None, querystring=None, cache_key=None, many=True, + traverse_pagination=True): """GET data from an edX REST API. DRY utility for handling caching and pagination. Arguments: api_config (ConfigurationModel): The configuration model governing interaction with the API. - user (User): The user to authenticate as when requesting data. resource (str): Name of the API resource being requested. Keyword Arguments: @@ -52,27 +51,6 @@ def get_edx_api_data(api_config, user, resource, api=None, resource_id=None, if cached: return cached - try: - if not api: - # TODO: Use the system's JWT_AUDIENCE and JWT_SECRET_KEY instead of client ID and name. - client_name = api_config.OAUTH2_CLIENT_NAME - - try: - client = Client.objects.get(name=client_name) - except Client.DoesNotExist: - raise ImproperlyConfigured( - 'OAuth2 Client with name [{}] does not exist.'.format(client_name) - ) - - scopes = ['email', 'profile'] - expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION - jwt = JwtBuilder(user, secret=client.client_secret).build_token(scopes, expires_in, aud=client.client_id) - - api = EdxRestApiClient(api_config.internal_api_url, jwt=jwt) - except: # pylint: disable=bare-except - log.exception('Failed to initialize the %s API client.', api_config.API_NAME) - return no_data - try: endpoint = getattr(api, resource) querystring = querystring if querystring else {} diff --git a/openedx/core/lib/tests/test_edx_api_utils.py b/openedx/core/lib/tests/test_edx_api_utils.py index 9031cb2c4e..de7093de55 100644 --- a/openedx/core/lib/tests/test_edx_api_utils.py +++ b/openedx/core/lib/tests/test_edx_api_utils.py @@ -65,7 +65,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach ) with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init: - actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) + actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api) # Verify that the helper function didn't initialize its own client. self.assertFalse(mock_init.called) @@ -96,7 +96,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach self._mock_catalog_api(responses) - actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) + actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api) self.assertEqual(actual_collection, expected_collection) self._assert_num_requests(len(expected_collection)) @@ -125,9 +125,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach [httpretty.Response(body=json.dumps(body), content_type='application/json') for body in responses] ) - actual_collection = get_edx_api_data( - catalog_integration, self.user, 'programs', api=api, traverse_pagination=False, - ) + actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api, traverse_pagination=False) self.assertEqual(actual_collection, expected_response) self._assert_num_requests(1) @@ -149,7 +147,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach url=url ) - actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id) + actual_resource = get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id) self.assertEqual(actual_resource, expected_resource) self._assert_num_requests(1) @@ -178,7 +176,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach url=url ) - actual_resource = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, resource_id=resource_id) + actual_resource = get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id) self.assertEqual(actual_resource, expected_resource) self._assert_num_requests(1) @@ -214,17 +212,15 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach cache_key = CatalogIntegration.current().CACHE_KEY # Warm up the cache. - get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key) - get_edx_api_data( - catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key - ) + get_edx_api_data(catalog_integration, 'programs', api=api, cache_key=cache_key) + get_edx_api_data(catalog_integration, 'programs', api=api, resource_id=resource_id, cache_key=cache_key) # Hit the cache. - actual_collection = get_edx_api_data(catalog_integration, self.user, 'programs', api=api, cache_key=cache_key) + actual_collection = get_edx_api_data(catalog_integration, 'programs', api=api, cache_key=cache_key) self.assertEqual(actual_collection, expected_collection) actual_resource = get_edx_api_data( - catalog_integration, self.user, 'programs', api=api, resource_id=resource_id, cache_key=cache_key + catalog_integration, 'programs', api=api, resource_id=resource_id, cache_key=cache_key ) self.assertEqual(actual_resource, expected_resource) @@ -236,24 +232,11 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach """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') + actual = get_edx_api_data(catalog_integration, 'programs', api=None) 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(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') - - self.assertTrue(mock_exception.called) - self.assertEqual(actual, []) - @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.""" @@ -264,7 +247,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach [httpretty.Response(body='clunk', content_type='application/json', status_code=500)] ) - actual = get_edx_api_data(catalog_integration, self.user, 'programs', api=api) + actual = get_edx_api_data(catalog_integration, 'programs', api=api) self.assertTrue(mock_exception.called) self.assertEqual(actual, []) @@ -276,33 +259,15 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach actual = get_edx_api_data( catalog_integration, - self.user, 'programs', + api=None, resource_id=100, - many=False, + 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.""" @@ -315,21 +280,10 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach actual = get_edx_api_data( catalog_integration, - self.user, 'programs', api=api, resource_id=100, - many=False, + 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) - - credentials_api_config = self.create_credentials_config() - - with mock.patch('openedx.core.lib.edx_api_utils.EdxRestApiClient.__init__') as mock_init: - get_edx_api_data(credentials_api_config, self.user, 'credentials') - self.assertTrue(mock_init.called)