Updated CatalogIntegration to use a site-specific URL

The internal_api_url field has been deprecated. All usages have been updated with calls to the method get_internal_api_url().

LEARNER-1111

openedx/core/djangoapps/api_admin/utils.py

@@ -1,14 +0,0 @@
-""" Course Discovery API Service. """
-from django.conf import settings
-from edx_rest_api_client.client import EdxRestApiClient
-
-from openedx.core.lib.token_utils import JwtBuilder
-
-
-def course_discovery_api_client(user):
-    """ Returns a Course Discovery API client setup with authentication for the specified user. """
-    scopes = ['email', 'profile']
-    expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION
-    jwt = JwtBuilder(user).build_token(scopes, expires_in)
-
-    return EdxRestApiClient(settings.COURSE_CATALOG_API_URL, jwt=jwt)

openedx/core/djangoapps/api_admin/views.py

@@ -18,7 +18,7 @@ from edxmako.shortcuts import render_to_response
 from openedx.core.djangoapps.api_admin.decorators import require_api_access
 from openedx.core.djangoapps.api_admin.forms import ApiAccessRequestForm, CatalogForm
 from openedx.core.djangoapps.api_admin.models import ApiAccessRequest, Catalog
-from openedx.core.djangoapps.api_admin.utils import course_discovery_api_client
+from openedx.core.djangoapps.catalog.utils import create_catalog_api_client
 
 log = logging.getLogger(__name__)
 
@@ -119,6 +119,11 @@ class ApiTosView(TemplateView):
     template_name = 'api_admin/terms_of_service.html'
 
 
+class CatalogApiMixin(object):
+    def get_catalog_api_client(self, user):
+        return create_catalog_api_client(user)
+
+
 class CatalogSearchView(View):
     """View to search for catalogs belonging to a user."""
 
@@ -135,7 +140,7 @@ class CatalogSearchView(View):
         return redirect(reverse('api_admin:catalog-list', kwargs={'username': username}))
 
 
-class CatalogListView(View):
+class CatalogListView(CatalogApiMixin, View):
     """View to list existing catalogs and create new ones."""
 
     template = 'api_admin/catalogs/list.html'
@@ -162,14 +167,14 @@ class CatalogListView(View):
 
     def get(self, request, username):
         """Display a list of a user's catalogs."""
-        client = course_discovery_api_client(request.user)
+        client = self.get_catalog_api_client(request.user)
         form = CatalogForm(initial={'viewers': [username]})
         return render_to_response(self.template, self.get_context_data(client, username, form))
 
     def post(self, request, username):
         """Create a new catalog for a user."""
         form = CatalogForm(request.POST)
-        client = course_discovery_api_client(request.user)
+        client = self.get_catalog_api_client(request.user)
         if not form.is_valid():
             return render_to_response(self.template, self.get_context_data(client, username, form), status=400)
 
@@ -178,7 +183,7 @@ class CatalogListView(View):
         return redirect(reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog['id']}))
 
 
-class CatalogEditView(View):
+class CatalogEditView(CatalogApiMixin, View):
     """View to edit an individual catalog."""
 
     template_name = 'api_admin/catalogs/edit.html'
@@ -196,7 +201,7 @@ class CatalogEditView(View):
 
     def get(self, request, catalog_id):
         """Display a form to edit this catalog."""
-        client = course_discovery_api_client(request.user)
+        client = self.get_catalog_api_client(request.user)
         response = client.catalogs(catalog_id).get()
         catalog = Catalog(attributes=response)
         form = CatalogForm(instance=catalog)
@@ -204,7 +209,7 @@ class CatalogEditView(View):
 
     def post(self, request, catalog_id):
         """Update or delete this catalog."""
-        client = course_discovery_api_client(request.user)
+        client = self.get_catalog_api_client(request.user)
         if request.POST.get('delete-catalog') == 'on':
             client.catalogs(catalog_id).delete()
             return redirect(reverse('api_admin:catalog-search'))
@@ -217,7 +222,7 @@ class CatalogEditView(View):
         return redirect(reverse('api_admin:catalog-edit', kwargs={'catalog_id': catalog['id']}))
 
 
-class CatalogPreviewView(View):
+class CatalogPreviewView(CatalogApiMixin, View):
     """Endpoint to preview courses for a query."""
 
     def get(self, request):
@@ -225,7 +230,7 @@ class CatalogPreviewView(View):
         Return the results of a query against the course catalog API. If no
         query parameter is given, returns an empty result set.
         """
-        client = course_discovery_api_client(request.user)
+        client = self.get_catalog_api_client(request.user)
         # Just pass along the request params including limit/offset pagination
         if 'q' in request.GET:
             results = client.courses.get(**request.GET)

openedx/core/djangoapps/catalog/management/commands/cache_programs.py

@@ -30,7 +30,7 @@ class Command(BaseCommand):
 
         try:
             user = User.objects.get(username=username)
-            client = create_catalog_api_client(user, catalog_integration)
+            client = create_catalog_api_client(user)
         except User.DoesNotExist:
             logger.error(
                 'Failed to create API client. Service user {username} does not exist.'.format(username)

openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py

@@ -21,7 +21,7 @@ class TestCachePrograms(CatalogIntegrationMixin, CacheIsolationTestCase):
 
         self.catalog_integration = self.create_catalog_integration()
 
-        self.list_url = self.catalog_integration.internal_api_url.rstrip('/') + '/programs/'
+        self.list_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/programs/'
         self.detail_tpl = self.list_url.rstrip('/') + '/{uuid}/'
 
         self.programs = ProgramFactory.create_batch(3)

openedx/core/djangoapps/catalog/migrations/0004_auto_20170616_0618.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('catalog', '0003_catalogintegration_page_size'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='catalogintegration',
+            name='internal_api_url',
+            field=models.URLField(help_text='DEPRECATED: Use the setting COURSE_CATALOG_API_URL.', verbose_name='Internal API URL'),
+        ),
+    ]

openedx/core/djangoapps/catalog/models.py

@@ -1,18 +1,23 @@
 """Models governing integration with the catalog service."""
 from config_models.models import ConfigurationModel
+from django.conf import settings
+from django.contrib.auth import get_user_model
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 
+from openedx.core.djangoapps.site_configuration import helpers
+
 
 class CatalogIntegration(ConfigurationModel):
     """Manages configuration for connecting to the catalog service and using its API."""
     API_NAME = 'catalog'
     CACHE_KEY = 'catalog.api.data'
 
+    # TODO Replace all usages of this field with a call to get_internal_api_url().
     internal_api_url = models.URLField(
         verbose_name=_('Internal API URL'),
         help_text=_(
-            'API root to be used for server-to-server requests (e.g., https://catalog-internal.example.com/api/v1/).'
+            'DEPRECATED: Use the setting COURSE_CATALOG_API_URL.'
         )
     )
 
@@ -47,5 +52,12 @@ class CatalogIntegration(ConfigurationModel):
         """Whether responses from the catalog API will be cached."""
         return self.cache_ttl > 0
 
-    def __unicode__(self):
+    def get_internal_api_url(self):
-        return self.internal_api_url
+        """ Returns the internal Catalog API URL associated with the request's site. """
+        return helpers.get_value('COURSE_CATALOG_API_URL', settings.COURSE_CATALOG_API_URL)
+
+    def get_service_user(self):
+        # NOTE: We load the user model here to avoid issues at startup time that result from the hacks
+        # in lms/startup.py.
+        User = get_user_model()  # pylint: disable=invalid-name
+        return User.objects.get(username=self.service_username)

openedx/core/djangoapps/catalog/tests/test_models.py

@@ -1,9 +1,12 @@
 """Catalog model tests."""
 import ddt
-from django.test import TestCase
 import mock
+from django.test import TestCase, override_settings
 
 from openedx.core.djangoapps.catalog.tests import mixins
+from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration
+
+COURSE_CATALOG_API_URL = 'https://api.example.com/v1/'
 
 
 @ddt.ddt
@@ -12,6 +15,11 @@ from openedx.core.djangoapps.catalog.tests import mixins
 class TestCatalogIntegration(mixins.CatalogIntegrationMixin, TestCase):
     """Tests covering the CatalogIntegration model."""
 
+    def assert_get_internal_api_url_value(self, expected):
+        """ Asserts the value of get_internal_api_url matches the expected value. """
+        catalog_integration = self.create_catalog_integration()
+        self.assertEqual(catalog_integration.get_internal_api_url(), expected)
+
     @ddt.data(
         (0, False),
         (1, True),
@@ -21,3 +29,24 @@ class TestCatalogIntegration(mixins.CatalogIntegrationMixin, TestCase):
         """Test the behavior of the property controlling whether API responses are cached."""
         catalog_integration = self.create_catalog_integration(cache_ttl=cache_ttl)
         self.assertEqual(catalog_integration.is_cache_enabled, is_cache_enabled)
+
+    @override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
+    def test_get_internal_api_url(self, _mock_cache):
+        """ Requests made without a microsite should return the value from settings. """
+        self.assert_get_internal_api_url_value(COURSE_CATALOG_API_URL)
+        catalog_integration = self.create_catalog_integration()
+        self.assertEqual(catalog_integration.get_internal_api_url(), COURSE_CATALOG_API_URL)
+
+    @override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
+    @with_site_configuration(configuration={})
+    def test_get_internal_api_url_without_microsite_override(self, _mock_cache):
+        """ Requests made to microsites that do not have COURSE_CATALOG_API_URL overridden should
+        return the default value from settings. """
+        self.assert_get_internal_api_url_value(COURSE_CATALOG_API_URL)
+
+    @override_settings(COURSE_CATALOG_API_URL=COURSE_CATALOG_API_URL)
+    @with_site_configuration(configuration={'COURSE_CATALOG_API_URL': 'foo'})
+    def test_get_internal_api_url_with_microsite_override(self, _mock_cache):
+        """ If a microsite has overridden the value of COURSE_CATALOG_API_URL, the overridden
+        value should be returned. """
+        self.assert_get_internal_api_url_value('foo')

openedx/core/djangoapps/catalog/tests/test_utils.py

@@ -7,7 +7,7 @@ import ddt
 import mock
 from django.contrib.auth import get_user_model
 from django.core.cache import cache
-from django.test import TestCase
+from django.test import TestCase, override_settings
 
 from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAM_UUIDS_CACHE_KEY
 from openedx.core.djangoapps.catalog.models import CatalogIntegration
@@ -209,6 +209,7 @@ class TestGetProgramsWithType(TestCase):
 @mock.patch(UTILS_MODULE + '.get_edx_api_data')
 class TestGetProgramTypes(CatalogIntegrationMixin, TestCase):
     """Tests covering retrieval of program types from the catalog service."""
+    @override_settings(COURSE_CATALOG_API_URL='https://api.example.com/v1/')
     def test_get_program_types(self, mock_get_edx_api_data):
         """Verify get_program_types returns the expected list of program types."""
         program_types = ProgramTypeFactory.create_batch(3)
@@ -249,7 +250,7 @@ class TestGetCourseRuns(CatalogIntegrationMixin, TestCase):
         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
+        self.assertEqual(kwargs['api']._store['base_url'], self.catalog_integration.get_internal_api_url())  # pylint: disable=protected-access
 
         querystring = {
             'page_size': 20,

openedx/core/djangoapps/catalog/utils.py

@@ -3,8 +3,8 @@ import copy
 import logging
 
 from django.conf import settings
-from django.contrib.auth import get_user_model
 from django.core.cache import cache
+from django.core.exceptions import ObjectDoesNotExist
 from edx_rest_api_client.client import EdxRestApiClient
 
 from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL, PROGRAM_UUIDS_CACHE_KEY
@@ -13,16 +13,16 @@ from openedx.core.lib.edx_api_utils import get_edx_api_data
 from openedx.core.lib.token_utils import JwtBuilder
 
 logger = logging.getLogger(__name__)
-User = get_user_model()  # pylint: disable=invalid-name
 
 
-def create_catalog_api_client(user, catalog_integration):
+def create_catalog_api_client(user):
-    """Returns an API client which can be used to make catalog API requests."""
+    """Returns an API client which can be used to make Catalog API requests."""
     scopes = ['email', 'profile']
     expires_in = settings.OAUTH_ID_TOKEN_EXPIRATION
     jwt = JwtBuilder(user).build_token(scopes, expires_in)
 
-    return EdxRestApiClient(catalog_integration.internal_api_url, jwt=jwt)
+    url = CatalogIntegration.current().get_internal_api_url()
+    return EdxRestApiClient(url, jwt=jwt)
 
 
 def get_programs(uuid=None):
@@ -90,11 +90,11 @@ def get_program_types(name=None):
     catalog_integration = CatalogIntegration.current()
     if catalog_integration.enabled:
         try:
-            user = User.objects.get(username=catalog_integration.service_username)
+            user = catalog_integration.get_service_user()
-        except User.DoesNotExist:
+        except ObjectDoesNotExist:
             return []
 
-        api = create_catalog_api_client(user, catalog_integration)
+        api = create_catalog_api_client(user)
         cache_key = '{base}.program_types'.format(base=catalog_integration.CACHE_KEY)
 
         data = get_edx_api_data(catalog_integration, 'program_types', api=api,
@@ -161,15 +161,15 @@ def get_course_runs():
     course_runs = []
     if catalog_integration.enabled:
         try:
-            user = User.objects.get(username=catalog_integration.service_username)
+            user = catalog_integration.get_service_user()
-        except User.DoesNotExist:
+        except ObjectDoesNotExist:
             logger.error(
                 'Catalog service user with username [%s] does not exist. Course runs will not be retrieved.',
                 catalog_integration.service_username,
             )
             return course_runs
 
-        api = create_catalog_api_client(user, catalog_integration)
+        api = create_catalog_api_client(user)
 
         querystring = {
             'page_size': catalog_integration.page_size,

openedx/core/lib/tests/test_edx_api_utils.py

@@ -40,7 +40,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def _mock_catalog_api(self, responses, url=None):
         self.assertTrue(httpretty.is_enabled(), msg='httpretty must be enabled to mock Catalog API calls.')
 
-        url = url if url else CatalogIntegration.current().internal_api_url.strip('/') + '/programs/'
+        url = url if url else CatalogIntegration.current().get_internal_api_url().strip('/') + '/programs/'
 
         httpretty.register_uri(httpretty.GET, url, responses=responses)
 
@@ -51,7 +51,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def test_get_unpaginated_data(self):
         """Verify that unpaginated data can be retrieved."""
         catalog_integration = self.create_catalog_integration()
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
         expected_collection = ['some', 'test', 'data']
         data = {
@@ -76,10 +76,10 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def test_get_paginated_data(self):
         """Verify that paginated data can be retrieved."""
         catalog_integration = self.create_catalog_integration()
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
         expected_collection = ['some', 'test', 'data']
-        url = CatalogIntegration.current().internal_api_url.strip('/') + '/programs/?page={}'
+        url = CatalogIntegration.current().get_internal_api_url().strip('/') + '/programs/?page={}'
 
         responses = []
         for page, record in enumerate(expected_collection, start=1):
@@ -105,9 +105,9 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         Verify that pagination is not traversed if traverse_pagination=False is passed as argument.
         """
         catalog_integration = self.create_catalog_integration()
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
-        url = CatalogIntegration.current().internal_api_url.strip('/') + '/programs/?page={}'
+        url = CatalogIntegration.current().get_internal_api_url().strip('/') + '/programs/?page={}'
         responses = [
             {
                 'next': url.format(2),
@@ -131,11 +131,11 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def test_get_specific_resource(self):
         """Verify that a specific resource can be retrieved."""
         catalog_integration = self.create_catalog_integration()
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
         resource_id = 1
         url = '{api_root}/programs/{resource_id}/'.format(
-            api_root=CatalogIntegration.current().internal_api_url.strip('/'),
+            api_root=CatalogIntegration.current().get_internal_api_url().strip('/'),
             resource_id=resource_id,
         )
 
@@ -160,11 +160,11 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
         return the value of that "results" key.
         """
         catalog_integration = self.create_catalog_integration()
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
         resource_id = 0
         url = '{api_root}/programs/{resource_id}/'.format(
-            api_root=CatalogIntegration.current().internal_api_url.strip('/'),
+            api_root=CatalogIntegration.current().get_internal_api_url().strip('/'),
             resource_id=resource_id,
         )
 
@@ -183,7 +183,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def test_cache_utilization(self):
         """Verify that when enabled, the cache is used."""
         catalog_integration = self.create_catalog_integration(cache_ttl=5)
-        api = create_catalog_api_client(self.user, catalog_integration)
+        api = create_catalog_api_client(self.user)
 
         expected_collection = ['some', 'test', 'data']
         data = {
@@ -197,7 +197,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
 
         resource_id = 1
         url = '{api_root}/programs/{resource_id}/'.format(
-            api_root=CatalogIntegration.current().internal_api_url.strip('/'),
+            api_root=CatalogIntegration.current().get_internal_api_url().strip('/'),
             resource_id=resource_id,
         )
 
@@ -240,7 +240,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     def test_data_retrieval_failure(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)
+        api = create_catalog_api_client(self.user)
 
         self._mock_catalog_api(
             [httpretty.Response(body='clunk', content_type='application/json', status_code=500)]
@@ -271,7 +271,7 @@ class TestGetEdxApiData(CatalogIntegrationMixin, CredentialsApiConfigMixin, Cach
     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)
+        api = create_catalog_api_client(self.user)
 
         self._mock_catalog_api(
             [httpretty.Response(body='clunk', content_type='application/json', status_code=500)]

openedx/features/enterprise_support/api.py

@@ -18,8 +18,8 @@ from edx_rest_api_client.client import EdxRestApiClient
 from requests.exceptions import ConnectionError, Timeout
 from slumber.exceptions import HttpClientError, HttpServerError, SlumberBaseException
 
-from openedx.core.djangoapps.api_admin.utils import course_discovery_api_client
 from openedx.core.djangoapps.catalog.models import CatalogIntegration
+from openedx.core.djangoapps.catalog.utils import create_catalog_api_client
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.lib.token_utils import JwtBuilder
 
@@ -31,7 +31,6 @@ try:
 except ImportError:
     pass
 
-
 CONSENT_FAILED_PARAMETER = 'consent_failed'
 LOGGER = logging.getLogger("edx.enterprise_helpers")
 
@@ -201,6 +200,7 @@ def data_sharing_consent_required(view_func):
     After granting consent, the user will be redirected back to the original request.path.
 
     """
+
     @wraps(view_func)
     def inner(request, course_id, *args, **kwargs):
         """
@@ -462,7 +462,7 @@ def is_course_in_enterprise_catalog(site, course_id, enterprise_catalog_id):
 
         try:
             # GET: /api/v1/catalogs/{catalog_id}/contains?course_run_id={course_run_ids}
-            response = course_discovery_api_client(user=user).catalogs(enterprise_catalog_id).contains.get(
+            response = create_catalog_api_client(user=user).catalogs(enterprise_catalog_id).contains.get(
                 course_run_id=course_id
             )
             cache.set(cache_key, response, settings.COURSES_API_CACHE_TIMEOUT)