From 5e2c2b1e939169be94bbe46bf97e0a1037c90798 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Wed, 5 Aug 2015 18:37:54 +0500 Subject: [PATCH] Fixed course_id pattern regex. PLAT-776 --- .../djangoapps/track/tests/test_contexts.py | 42 +++++++++++++++---- lms/envs/common.py | 2 +- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/track/tests/test_contexts.py b/common/djangoapps/track/tests/test_contexts.py index 49e4e4fc50..e3cc91759e 100644 --- a/common/djangoapps/track/tests/test_contexts.py +++ b/common/djangoapps/track/tests/test_contexts.py @@ -1,23 +1,36 @@ # pylint: disable=missing-docstring +import ddt from unittest import TestCase from track import contexts +@ddt.ddt class TestContexts(TestCase): COURSE_ID = 'test/course_name/course_run' + SPLIT_COURSE_ID = 'course-v1:test+course_name+course_run' ORG_ID = 'test' - def test_course_id_from_url(self): - self.assert_parses_course_id_from_url('http://foo.bar.com/courses/{course_id}/more/stuff') + @ddt.data( + (COURSE_ID, ''), + (COURSE_ID, '/more/stuff'), + (COURSE_ID, '?format=json'), + (SPLIT_COURSE_ID, ''), + (SPLIT_COURSE_ID, '/more/stuff'), + (SPLIT_COURSE_ID, '?format=json') + ) + @ddt.unpack + def test_course_id_from_url(self, course_id, postfix): + url = 'http://foo.bar.com/courses/{}{}'.format(course_id, postfix) + self.assert_parses_course_id_from_url(url, course_id) - def assert_parses_course_id_from_url(self, format_string): + def assert_parses_course_id_from_url(self, format_string, course_id): self.assertEquals( - contexts.course_context_from_url(format_string.format(course_id=self.COURSE_ID)), + contexts.course_context_from_url(format_string.format(course_id=course_id)), { - 'course_id': self.COURSE_ID, + 'course_id': course_id, 'org_id': self.ORG_ID } ) @@ -34,11 +47,22 @@ class TestContexts(TestCase): } ) - def test_malformed_course_id(self): - self.assert_empty_context_for_url('http://foo.bar.com/courses/test') + @ddt.data('', '/', '/?', '?format=json') + def test_malformed_course_id(self, postfix): + self.assert_empty_context_for_url('http://foo.bar.com/courses/test/course_name{}'.format(postfix)) - def test_course_id_later_in_url(self): - self.assert_parses_course_id_from_url('http://foo.bar.com/x/y/z/courses/{course_id}') + @ddt.data( + (COURSE_ID, ''), + (COURSE_ID, '/more/stuff'), + (COURSE_ID, '?format=json'), + (SPLIT_COURSE_ID, ''), + (SPLIT_COURSE_ID, '/more/stuff'), + (SPLIT_COURSE_ID, '?format=json') + ) + @ddt.unpack + def test_course_id_later_in_url(self, course_id, postfix): + url = 'http://foo.bar.com/x/y/z/courses/{}{}'.format(course_id, postfix) + self.assert_parses_course_id_from_url(url, course_id) def test_no_url(self): self.assert_empty_context_for_url(None) diff --git a/lms/envs/common.py b/lms/envs/common.py index a60fbad51e..efc358446f 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -593,7 +593,7 @@ LMS_MIGRATION_ALLOWED_IPS = [] # Note: these intentionally greedily grab all chars up to the next slash including any pluses # DHM: I really wanted to ensure the separators were the same (+ or /) but all patts I tried had # too many inadvertent side effects :-( -COURSE_KEY_PATTERN = r'(?P[^/+]+(/|\+)[^/+]+(/|\+)[^/]+)' +COURSE_KEY_PATTERN = r'(?P[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)' COURSE_ID_PATTERN = COURSE_KEY_PATTERN.replace('course_key_string', 'course_id') COURSE_KEY_REGEX = COURSE_KEY_PATTERN.replace('P', ':')