From 800bcd8e2019a5451b7bae89242bcdaab7b81cfd Mon Sep 17 00:00:00 2001
From: Clinton Blackburn <cblackburn@edx.org>
Date: Wed, 24 May 2017 15:46:00 -0400
Subject: [PATCH] Updated CredentialsApiConfig to pull URLs from settings
Pulling URLs from settings allows us to rely on site configuration overrides in order to support multi-tenancy.
LEARNER-1103
---
lms/envs/aws.py | 3 +
lms/envs/common.py | 3 +
lms/envs/devstack.py | 3 +
lms/envs/devstack_docker.py | 4 +
lms/envs/test.py | 3 +
.../migrations/0003_auto_20170525_1109.py | 24 +++++
openedx/core/djangoapps/credentials/models.py | 51 ++++++-----
.../credentials/tests/test_models.py | 35 +++-----
.../programs/tasks/v1/tests/test_tasks.py | 88 ++++++++++---------
9 files changed, 126 insertions(+), 88 deletions(-)
create mode 100644 openedx/core/djangoapps/credentials/migrations/0003_auto_20170525_1109.py
diff --git a/lms/envs/aws.py b/lms/envs/aws.py
index 684819de48f..00e42b9d16f 100644
--- a/lms/envs/aws.py
+++ b/lms/envs/aws.py
@@ -799,6 +799,9 @@ ECOMMERCE_API_TIMEOUT = ENV_TOKENS.get('ECOMMERCE_API_TIMEOUT', ECOMMERCE_API_TI
COURSE_CATALOG_API_URL = ENV_TOKENS.get('COURSE_CATALOG_API_URL', COURSE_CATALOG_API_URL)
+CREDENTIALS_INTERNAL_SERVICE_URL = ENV_TOKENS.get('CREDENTIALS_INTERNAL_SERVICE_URL', CREDENTIALS_INTERNAL_SERVICE_URL)
+CREDENTIALS_PUBLIC_SERVICE_URL = ENV_TOKENS.get('CREDENTIALS_PUBLIC_SERVICE_URL', CREDENTIALS_PUBLIC_SERVICE_URL)
+
##### Custom Courses for EdX #####
if FEATURES.get('CUSTOM_COURSES_EDX'):
INSTALLED_APPS += ('lms.djangoapps.ccx', 'openedx.core.djangoapps.ccxcon')
diff --git a/lms/envs/common.py b/lms/envs/common.py
index db159c9f8e5..65074ec1985 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -2908,6 +2908,9 @@ ECOMMERCE_SERVICE_WORKER_USERNAME = 'ecommerce_worker'
COURSE_CATALOG_API_URL = None
+CREDENTIALS_INTERNAL_SERVICE_URL = None
+CREDENTIALS_PUBLIC_SERVICE_URL = None
+
# Reverification checkpoint name pattern
CHECKPOINT_PATTERN = r'(?P<checkpoint_name>[^/]+)'
diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py
index a7dcf6c429f..d5a5f49cd32 100644
--- a/lms/envs/devstack.py
+++ b/lms/envs/devstack.py
@@ -219,6 +219,9 @@ if FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and 'third_party_auth.dummy.DummyBack
############## ECOMMERCE API CONFIGURATION SETTINGS ###############
ECOMMERCE_PUBLIC_URL_ROOT = "http://localhost:8002"
+CREDENTIALS_INTERNAL_SERVICE_URL = 'http://localhost:8008'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:8008'
+
###################### Cross-domain requests ######################
FEATURES['ENABLE_CORS_HEADERS'] = True
CORS_ALLOW_CREDENTIALS = True
diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py
index 3973f22a1c7..797bb9d8a8d 100644
--- a/lms/envs/devstack_docker.py
+++ b/lms/envs/devstack_docker.py
@@ -15,8 +15,12 @@ LMS_ROOT_URL = 'http://{}'.format(HOST)
ECOMMERCE_PUBLIC_URL_ROOT = 'http://localhost:18130'
ECOMMERCE_API_URL = 'http://edx.devstack.ecommerce:18130/api/v2'
+
ENTERPRISE_API_URL = '{}/enterprise/api/v1/'.format(LMS_ROOT_URL)
+CREDENTIALS_INTERNAL_SERVICE_URL = 'http://edx.devstack.credentials:18150'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'http://localhost:18150'
+
OAUTH_OIDC_ISSUER = '{}/oauth2'.format(LMS_ROOT_URL)
JWT_AUTH.update({
diff --git a/lms/envs/test.py b/lms/envs/test.py
index bbf519ff05d..5457a398413 100644
--- a/lms/envs/test.py
+++ b/lms/envs/test.py
@@ -591,6 +591,9 @@ JWT_AUTH.update({
COURSE_CATALOG_API_URL = 'https://catalog.example.com/api/v1'
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials-internal.example.com'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'https://credentials.example.com'
+
COMPREHENSIVE_THEME_DIRS = [REPO_ROOT / "themes", REPO_ROOT / "common/test"]
COMPREHENSIVE_THEME_LOCALE_PATHS = [REPO_ROOT / "themes/conf/locale", ]
diff --git a/openedx/core/djangoapps/credentials/migrations/0003_auto_20170525_1109.py b/openedx/core/djangoapps/credentials/migrations/0003_auto_20170525_1109.py
new file mode 100644
index 00000000000..6c5acb79356
--- /dev/null
+++ b/openedx/core/djangoapps/credentials/migrations/0003_auto_20170525_1109.py
@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('credentials', '0002_auto_20160325_0631'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='credentialsapiconfig',
+ name='internal_service_url',
+ field=models.URLField(help_text=b'DEPRECATED: Use the setting CREDENTIALS_INTERNAL_SERVICE_URL.', verbose_name='Internal Service URL'),
+ ),
+ migrations.AlterField(
+ model_name='credentialsapiconfig',
+ name='public_service_url',
+ field=models.URLField(help_text=b'DEPRECATED: Use the setting CREDENTIALS_PUBLIC_SERVICE_URL.', verbose_name='Public Service URL'),
+ ),
+ ]
diff --git a/openedx/core/djangoapps/credentials/models.py b/openedx/core/djangoapps/credentials/models.py
index 25fa2ce931c..e435d20c97d 100644
--- a/openedx/core/djangoapps/credentials/models.py
+++ b/openedx/core/djangoapps/credentials/models.py
@@ -4,10 +4,12 @@ Models for credentials support for the LMS and Studio.
from urlparse import urljoin
-from django.utils.translation import ugettext_lazy as _
+from config_models.models import ConfigurationModel
+from django.conf import settings
from django.db import models
+from django.utils.translation import ugettext_lazy as _
-from config_models.models import ConfigurationModel
+from openedx.core.djangoapps.site_configuration import helpers
API_VERSION = 'v2'
@@ -17,35 +19,42 @@ class CredentialsApiConfig(ConfigurationModel):
Manages configuration for connecting to the Credential service and using its
API.
"""
+
class Meta(object):
- app_label = "credentials"
+ app_label = 'credentials'
OAUTH2_CLIENT_NAME = 'credentials'
API_NAME = 'credentials'
CACHE_KEY = 'credentials.api.data'
- internal_service_url = models.URLField(verbose_name=_("Internal Service URL"))
- public_service_url = models.URLField(verbose_name=_("Public Service URL"))
+ internal_service_url = models.URLField(
+ verbose_name=_('Internal Service URL'),
+ help_text='DEPRECATED: Use the setting CREDENTIALS_INTERNAL_SERVICE_URL.'
+ )
+ public_service_url = models.URLField(
+ verbose_name=_('Public Service URL'),
+ help_text='DEPRECATED: Use the setting CREDENTIALS_PUBLIC_SERVICE_URL.'
+ )
enable_learner_issuance = models.BooleanField(
- verbose_name=_("Enable Learner Issuance"),
+ verbose_name=_('Enable Learner Issuance'),
default=False,
help_text=_(
- "Enable issuance of credentials via Credential Service."
+ 'Enable issuance of credentials via Credential Service.'
)
)
enable_studio_authoring = models.BooleanField(
- verbose_name=_("Enable Authoring of Credential in Studio"),
+ verbose_name=_('Enable Authoring of Credential in Studio'),
default=False,
help_text=_(
- "Enable authoring of Credential Service credentials in Studio."
+ 'Enable authoring of Credential Service credentials in Studio.'
)
)
cache_ttl = models.PositiveIntegerField(
- verbose_name=_("Cache Time To Live"),
+ verbose_name=_('Cache Time To Live'),
default=0,
help_text=_(
- "Specified in seconds. Enable caching by setting this to a value greater than 0."
+ 'Specified in seconds. Enable caching by setting this to a value greater than 0.'
)
)
@@ -55,32 +64,26 @@ class CredentialsApiConfig(ConfigurationModel):
@property
def internal_api_url(self):
"""
- Generate a URL based on internal service URL and API version number.
+ Internally-accessible API URL root.
"""
- return urljoin(self.internal_service_url, '/api/{}/'.format(API_VERSION))
+ root = helpers.get_value('CREDENTIALS_INTERNAL_SERVICE_URL', settings.CREDENTIALS_INTERNAL_SERVICE_URL)
+ return urljoin(root, '/api/{}/'.format(API_VERSION))
@property
def public_api_url(self):
"""
- Generate a URL based on public service URL and API version number.
+ Publicly-accessible API URL root.
"""
- return urljoin(self.public_service_url, '/api/{}/'.format(API_VERSION))
+ root = helpers.get_value('CREDENTIALS_PUBLIC_SERVICE_URL', settings.CREDENTIALS_PUBLIC_SERVICE_URL)
+ return urljoin(root, '/api/{}/'.format(API_VERSION))
@property
def is_learner_issuance_enabled(self):
"""
- Indicates whether the learner credential should be enabled or not.
+ Returns boolean indicating if credentials should be issued.
"""
return self.enabled and self.enable_learner_issuance
- @property
- def is_studio_authoring_enabled(self):
- """
- Indicates whether Studio functionality related to Credential should
- be enabled or not.
- """
- return self.enabled and self.enable_studio_authoring
-
@property
def is_cache_enabled(self):
"""Whether responses from the Credentials API will be cached."""
diff --git a/openedx/core/djangoapps/credentials/tests/test_models.py b/openedx/core/djangoapps/credentials/tests/test_models.py
index 77178e15a74..ed146585f77 100644
--- a/openedx/core/djangoapps/credentials/tests/test_models.py
+++ b/openedx/core/djangoapps/credentials/tests/test_models.py
@@ -1,27 +1,34 @@
"""Tests for models supporting Credentials-related functionality."""
-from django.test import TestCase
+from django.test import TestCase, override_settings
from nose.plugins.attrib import attr
+from openedx.core.djangoapps.credentials.models import API_VERSION
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
from openedx.core.djangolib.testing.utils import skip_unless_lms
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials.example.com'
+CREDENTIALS_PUBLIC_SERVICE_URL = 'https://credentials.example.com'
+
@skip_unless_lms
@attr(shard=2)
class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase):
"""Tests covering the CredentialsApiConfig model."""
+
+ @override_settings(
+ CREDENTIALS_INTERNAL_SERVICE_URL=CREDENTIALS_INTERNAL_SERVICE_URL,
+ CREDENTIALS_PUBLIC_SERVICE_URL=CREDENTIALS_PUBLIC_SERVICE_URL
+ )
def test_url_construction(self):
"""Verify that URLs returned by the model are constructed correctly."""
credentials_config = self.create_credentials_config()
- self.assertEqual(
- credentials_config.internal_api_url,
- credentials_config.internal_service_url.strip('/') + '/api/v2/')
+ expected = '{root}/api/{version}/'.format(root=CREDENTIALS_INTERNAL_SERVICE_URL.strip('/'), version=API_VERSION)
+ self.assertEqual(credentials_config.internal_api_url, expected)
- self.assertEqual(
- credentials_config.public_api_url,
- credentials_config.public_service_url.strip('/') + '/api/v2/')
+ expected = '{root}/api/{version}/'.format(root=CREDENTIALS_PUBLIC_SERVICE_URL.strip('/'), version=API_VERSION)
+ self.assertEqual(credentials_config.public_api_url, expected)
def test_is_learner_issuance_enabled(self):
"""
@@ -36,17 +43,3 @@ class TestCredentialsApiConfig(CredentialsApiConfigMixin, TestCase):
credentials_config = self.create_credentials_config()
self.assertTrue(credentials_config.is_learner_issuance_enabled)
-
- def test_is_studio_authoring_enabled(self):
- """
- Verify that the property controlling display in the Studio authoring is only True
- when configuration is enabled and all required configuration is provided.
- """
- credentials_config = self.create_credentials_config(enabled=False)
- self.assertFalse(credentials_config.is_studio_authoring_enabled)
-
- credentials_config = self.create_credentials_config(enable_studio_authoring=False)
- self.assertFalse(credentials_config.is_studio_authoring_enabled)
-
- credentials_config = self.create_credentials_config()
- self.assertTrue(credentials_config.is_studio_authoring_enabled)
diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py
index f8a0c748c09..1960bb4e42f 100644
--- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py
+++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py
@@ -3,15 +3,15 @@ Tests for programs celery tasks.
"""
import json
-from celery.exceptions import MaxRetriesExceededError
import ddt
+import httpretty
+import mock
+from celery.exceptions import MaxRetriesExceededError
from django.conf import settings
from django.test import override_settings, TestCase
+from edx_oauth2_provider.tests.factories import ClientFactory
from edx_rest_api_client import exceptions
from edx_rest_api_client.client import EdxRestApiClient
-from edx_oauth2_provider.tests.factories import ClientFactory
-import httpretty
-import mock
from openedx.core.djangoapps.catalog.tests.mixins import CatalogIntegrationMixin
from openedx.core.djangoapps.credentials.tests.mixins import CredentialsApiConfigMixin
@@ -19,7 +19,7 @@ from openedx.core.djangoapps.programs.tasks.v1 import tasks
from openedx.core.djangolib.testing.utils import skip_unless_lms
from student.tests.factories import UserFactory
-
+CREDENTIALS_INTERNAL_SERVICE_URL = 'https://credentials.example.com'
TASKS_MODULE = 'openedx.core.djangoapps.programs.tasks.v1.tasks'
@@ -29,6 +29,7 @@ class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase):
Test the get_api_client function
"""
+ @override_settings(CREDENTIALS_INTERNAL_SERVICE_URL=CREDENTIALS_INTERNAL_SERVICE_URL)
@mock.patch(TASKS_MODULE + '.JwtBuilder.build_token')
def test_get_api_client(self, mock_build_token):
"""
@@ -36,13 +37,12 @@ class GetApiClientTestCase(CredentialsApiConfigMixin, TestCase):
"""
student = UserFactory()
ClientFactory.create(name='credentials')
- api_config = self.create_credentials_config(
- internal_service_url='http://foo'
- )
+ api_config = self.create_credentials_config()
mock_build_token.return_value = 'test-token'
api_client = tasks.get_api_client(api_config, student)
- self.assertEqual(api_client._store['base_url'], 'http://foo/api/v2/') # pylint: disable=protected-access
+ expected = CREDENTIALS_INTERNAL_SERVICE_URL.strip('/') + '/api/v2/'
+ self.assertEqual(api_client._store['base_url'], expected) # pylint: disable=protected-access
self.assertEqual(api_client._store['session'].auth.token, 'test-token') # pylint: disable=protected-access
@@ -82,7 +82,7 @@ class GetAwardedCertificateProgramsTestCase(TestCase):
]
result = tasks.get_certified_programs(student)
- self.assertEqual(mock_get_credentials.call_args[0], (student, ))
+ self.assertEqual(mock_get_credentials.call_args[0], (student,))
self.assertEqual(result, [1])
@@ -136,10 +136,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
UserFactory.create(username=settings.CREDENTIALS_SERVICE_USERNAME) # pylint: disable=no-member
def test_completion_check(
- self,
- mock_get_completed_programs,
- mock_get_certified_programs, # pylint: disable=unused-argument
- mock_award_program_certificate, # pylint: disable=unused-argument
+ self,
+ mock_get_completed_programs,
+ mock_get_certified_programs, # pylint: disable=unused-argument
+ mock_award_program_certificate, # pylint: disable=unused-argument
):
"""
Checks that the Programs API is used correctly to determine completed
@@ -155,12 +155,12 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
)
@ddt.unpack
def test_awarding_certs(
- self,
- already_awarded_program_uuids,
- expected_awarded_program_uuids,
- mock_get_completed_programs,
- mock_get_certified_programs,
- mock_award_program_certificate,
+ self,
+ already_awarded_program_uuids,
+ expected_awarded_program_uuids,
+ mock_get_completed_programs,
+ mock_get_certified_programs,
+ mock_award_program_certificate,
):
"""
Checks that the Credentials API is used to award certificates for
@@ -179,10 +179,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
)
@ddt.unpack
def test_retry_if_config_disabled(
- self,
- disabled_config_type,
- disabled_config_attribute,
- *mock_helpers
+ self,
+ disabled_config_type,
+ disabled_config_attribute,
+ *mock_helpers
):
"""
Checks that the task is aborted if any relevant api configs are
@@ -208,10 +208,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
self.assertFalse(mock_helper.called)
def test_abort_if_no_completed_programs(
- self,
- mock_get_completed_programs,
- mock_get_certified_programs,
- mock_award_program_certificate,
+ self,
+ mock_get_completed_programs,
+ mock_get_certified_programs,
+ mock_award_program_certificate,
):
"""
Checks that the task will be aborted without further action if there
@@ -234,19 +234,21 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.side_effect
"""
+
def side_effect(*_a): # pylint: disable=missing-docstring
if side_effects:
exc = side_effects.pop(0)
if exc:
raise exc
return mock.DEFAULT
+
return side_effect
def test_continue_awarding_certs_if_error(
- self,
- mock_get_completed_programs,
- mock_get_certified_programs,
- mock_award_program_certificate,
+ self,
+ mock_get_completed_programs,
+ mock_get_certified_programs,
+ mock_award_program_certificate,
):
"""
Checks that a single failure to award one of several certificates
@@ -268,9 +270,9 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
mock_info.assert_any_call(mock.ANY, 2, self.student.username)
def test_retry_on_programs_api_errors(
- self,
- mock_get_completed_programs,
- *_mock_helpers # pylint: disable=unused-argument
+ self,
+ mock_get_completed_programs,
+ *_mock_helpers # pylint: disable=unused-argument
):
"""
Ensures that any otherwise-unhandled errors that arise while trying
@@ -283,10 +285,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
self.assertEqual(mock_get_completed_programs.call_count, 2)
def test_retry_on_credentials_api_errors(
- self,
- mock_get_completed_programs,
- mock_get_certified_programs,
- mock_award_program_certificate,
+ self,
+ mock_get_completed_programs,
+ mock_get_certified_programs,
+ mock_award_program_certificate,
):
"""
Ensures that any otherwise-unhandled errors that arise while trying
@@ -302,10 +304,10 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo
self.assertEqual(mock_award_program_certificate.call_count, 1)
def test_no_retry_on_credentials_api_not_found_errors(
- self,
- mock_get_completed_programs,
- mock_get_certified_programs,
- mock_award_program_certificate,
+ self,
+ mock_get_completed_programs,
+ mock_get_certified_programs,
+ mock_award_program_certificate,
):
mock_get_completed_programs.return_value = [1, 2]
mock_get_certified_programs.side_effect = [[], [2]]
--
GitLab