Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Updated CatalogIntegration to use a site-specific URL

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

LEARNER-1111
This commit is contained in:
Clinton Blackburn
2017-05-23 17:39:55 -04:00
committed by Afzal Wali
parent dc25688096
commit f318661df5
11 changed files with 111 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
openedx/core/djangoapps/api_admin/utils.py
View File

@@ -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)

23
openedx/core/djangoapps/api_admin/views.py
View File

@@ -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)

2
openedx/core/djangoapps/catalog/management/commands/cache_programs.py
View File

@@ -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)

2
openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py
View File

@@ -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)

19
openedx/core/djangoapps/catalog/migrations/0004_auto_20170616_0618.py Normal file
View File

@@ -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'),
),
]

18
openedx/core/djangoapps/catalog/models.py
View File

@@ -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):
return self.internal_api_url
def get_internal_api_url(self):
""" 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)

31
openedx/core/djangoapps/catalog/tests/test_models.py
View File

@@ -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')

5
openedx/core/djangoapps/catalog/tests/test_utils.py
View File

@@ -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,

22
openedx/core/djangoapps/catalog/utils.py
View File

@@ -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):
"""Returns an API client which can be used to make catalog API requests."""
def create_catalog_api_client(user):
"""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)
except User.DoesNotExist:
user = catalog_integration.get_service_user()
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)
except User.DoesNotExist:
user = catalog_integration.get_service_user()
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,

28
openedx/core/lib/tests/test_edx_api_utils.py
View File

@@ -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)]

6
openedx/features/enterprise_support/api.py
View File

@@ -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)