From 58c3c0c2d7b62d70d18b4e30b5431728cdf169f9 Mon Sep 17 00:00:00 2001 From: Daniel Friedman Date: Wed, 3 Jun 2015 11:57:53 -0400 Subject: [PATCH] Expose EDXNOTES_PUBLIC_API/EDXNOTES_INTERNAL_API --- lms/djangoapps/edxnotes/decorators.py | 4 +-- lms/djangoapps/edxnotes/helpers.py | 27 ++++++++++---- lms/djangoapps/edxnotes/tests.py | 51 +++++++++++++++++++-------- lms/envs/aws.py | 3 +- lms/envs/bok_choy.py | 3 +- lms/envs/common.py | 5 ++- 6 files changed, 65 insertions(+), 28 deletions(-) diff --git a/lms/djangoapps/edxnotes/decorators.py b/lms/djangoapps/edxnotes/decorators.py index e5dd354e1a..8abb30a715 100644 --- a/lms/djangoapps/edxnotes/decorators.py +++ b/lms/djangoapps/edxnotes/decorators.py @@ -4,7 +4,7 @@ Decorators related to edXNotes. from django.conf import settings import json from edxnotes.helpers import ( - get_endpoint, + get_public_endpoint, get_id_token, get_token_url, generate_uid, @@ -45,7 +45,7 @@ def edxnotes(cls): "courseId": unicode(self.runtime.course_id).encode("utf-8"), "token": get_id_token(self.runtime.get_real_user(self.runtime.anonymous_student_id)), "tokenUrl": get_token_url(self.runtime.course_id), - "endpoint": get_endpoint(), + "endpoint": get_public_endpoint(), "debug": settings.DEBUG, "eventStringLimit": settings.TRACK_MAX_EVENT / 6, }, diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 30d70de7be..2e1044981b 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -79,7 +79,7 @@ def send_request(user, course_id, path="", query_string=None): """ Sends a request with appropriate parameters and headers. """ - url = get_endpoint(path) + url = get_internal_endpoint(path) params = { "user": anonymous_id_for_user(user, None), "course_id": unicode(course_id).encode("utf-8"), @@ -286,14 +286,19 @@ def get_notes(user, course): return json.dumps(preprocess_collection(user, course, collection), cls=NoteJSONEncoder) -def get_endpoint(path=""): +def get_endpoint(api_url, path=""): """ Returns edx-notes-api endpoint. + + Arguments: + api_url (str): base url to the notes api + path (str): path to the resource + Returns: + str: full endpoint to the notes api """ try: - url = settings.EDXNOTES_INTERFACE['url'] - if not url.endswith("/"): - url += "/" + if not api_url.endswith("/"): + api_url += "/" if path: if path.startswith("/"): @@ -301,11 +306,21 @@ def get_endpoint(path=""): if not path.endswith("/"): path += "/" - return url + path + return api_url + path except (AttributeError, KeyError): raise ImproperlyConfigured(_("No endpoint was provided for EdxNotes.")) +def get_public_endpoint(path=""): + """Get the full path to a resource on the public notes API.""" + return get_endpoint(settings.EDXNOTES_PUBLIC_API, path) + + +def get_internal_endpoint(path=""): + """Get the full path to a resource on the private notes API.""" + return get_endpoint(settings.EDXNOTES_INTERNAL_API, path) + + def get_course_position(course_module): """ Return the user's current place in the course. diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 5d5be0fd9d..20b02cb9cb 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -1,6 +1,8 @@ """ Tests for the EdxNotes app. """ +from contextlib import contextmanager +import ddt import json import jwt from mock import patch, MagicMock @@ -15,6 +17,7 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.core.exceptions import ImproperlyConfigured from django.test.client import RequestFactory +from django.test.utils import override_settings from oauth2_provider.tests.factories import ClientFactory from provider.oauth2.models import Client from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -76,7 +79,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase): self.problem = TestProblem(self.course) @patch.dict("django.conf.settings.FEATURES", {'ENABLE_EDXNOTES': True}) - @patch("edxnotes.decorators.get_endpoint") + @patch("edxnotes.decorators.get_public_endpoint") @patch("edxnotes.decorators.get_token_url") @patch("edxnotes.decorators.get_id_token") @patch("edxnotes.decorators.generate_uid") @@ -141,6 +144,7 @@ class EdxNotesDecoratorTest(ModuleStoreTestCase): @skipUnless(settings.FEATURES["ENABLE_EDXNOTES"], "EdxNotes feature needs to be enabled.") +@ddt.ddt class EdxNotesHelpersTest(ModuleStoreTestCase): """ Tests for EdxNotes helpers. @@ -232,29 +236,44 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): {"type": "bar"}] self.assertTrue(helpers.is_feature_enabled(self.course)) - def test_get_endpoint(self): + @ddt.data( + helpers.get_public_endpoint, + helpers.get_internal_endpoint, + ) + def test_get_endpoints(self, get_endpoint_function): """ - Tests that storage_url method returns appropriate values. + Test that the get_public_endpoint and get_internal_endpoint functions + return appropriate values. """ + @contextmanager + def patch_edxnotes_api_settings(url): + """ + Convenience function for patching both EDXNOTES_PUBLIC_API and + EDXNOTES_INTERNAL_API. + """ + with override_settings(EDXNOTES_PUBLIC_API=url): + with override_settings(EDXNOTES_INTERNAL_API=url): + yield + # url ends with "/" - with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com/"}): - self.assertEqual("http://example.com/", helpers.get_endpoint()) + with patch_edxnotes_api_settings("http://example.com/"): + self.assertEqual("http://example.com/", get_endpoint_function()) # url doesn't have "/" at the end - with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}): - self.assertEqual("http://example.com/", helpers.get_endpoint()) + with patch_edxnotes_api_settings("http://example.com"): + self.assertEqual("http://example.com/", get_endpoint_function()) # url with path that starts with "/" - with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}): - self.assertEqual("http://example.com/some_path/", helpers.get_endpoint("/some_path")) + with patch_edxnotes_api_settings("http://example.com"): + self.assertEqual("http://example.com/some_path/", get_endpoint_function("/some_path")) # url with path without "/" - with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}): - self.assertEqual("http://example.com/some_path/", helpers.get_endpoint("some_path/")) + with patch_edxnotes_api_settings("http://example.com"): + self.assertEqual("http://example.com/some_path/", get_endpoint_function("some_path/")) # url is not configured - with patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": None}): - self.assertRaises(ImproperlyConfigured, helpers.get_endpoint) + with patch_edxnotes_api_settings(None): + self.assertRaises(ImproperlyConfigured, get_endpoint_function) @patch("edxnotes.helpers.requests.get") def test_get_notes_correct_data(self, mock_get): @@ -669,7 +688,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): helpers.get_module_context(self.course, self.chapter_2) ) - @patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}) + @override_settings(EDXNOTES_PUBLIC_API="http://example.com") + @override_settings(EDXNOTES_INTERNAL_API="http://example.com") @patch("edxnotes.helpers.anonymous_id_for_user") @patch("edxnotes.helpers.get_id_token") @patch("edxnotes.helpers.requests.get") @@ -697,7 +717,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): } ) - @patch.dict("django.conf.settings.EDXNOTES_INTERFACE", {"url": "http://example.com"}) + @override_settings(EDXNOTES_PUBLIC_API="http://example.com") + @override_settings(EDXNOTES_INTERNAL_API="http://example.com") @patch("edxnotes.helpers.anonymous_id_for_user") @patch("edxnotes.helpers.get_id_token") @patch("edxnotes.helpers.requests.get") diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 4928a20d4a..8b4601bb4a 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -629,4 +629,5 @@ else: # EdxNotes config -EDXNOTES_INTERFACE = ENV_TOKENS.get('EDXNOTES_INTERFACE', EDXNOTES_INTERFACE) +EDXNOTES_PUBLIC_API = ENV_TOKENS.get('EDXNOTES_PUBLIC_API', EDXNOTES_PUBLIC_API) +EDXNOTES_INTERNAL_API = ENV_TOKENS.get('EDXNOTES_INTERNAL_API', EDXNOTES_INTERNAL_API) diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index ce93aac8da..4eacabf257 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -71,7 +71,8 @@ XQUEUE_INTERFACE['url'] = 'http://localhost:8040' OPEN_ENDED_GRADING_INTERFACE['url'] = 'http://localhost:8041/' # Configure the LMS to use our stub EdxNotes implementation -EDXNOTES_INTERFACE['url'] = 'http://localhost:8042/api/v1' +EDXNOTES_PUBLIC_API = 'http://localhost:8042/api/v1' +EDXNOTES_INTERNAL_API = 'http://localhost:8042/api/v1' # Enable django-pipeline and staticfiles STATIC_ROOT = (TEST_ROOT / "staticfiles").abspath() diff --git a/lms/envs/common.py b/lms/envs/common.py index 4e5b1e6914..e461c05a29 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1038,9 +1038,8 @@ MOCK_STAFF_GRADING = False ################################# EdxNotes config ######################### # Configure the LMS to use our stub EdxNotes implementation -EDXNOTES_INTERFACE = { - 'url': 'http://localhost:8120/api/v1', -} +EDXNOTES_PUBLIC_API = 'http://localhost:8120/api/v1' +EDXNOTES_INTERNAL_API = 'http://localhost:8120/api/v1' ########################## Parental controls config #######################