From ead022f1f118a034c7173831f23387c8379c6495 Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Fri, 4 Mar 2022 16:02:45 -0800 Subject: [PATCH 1/7] Retry once if get 401/403 --- uw_myplan/__init__.py | 34 ++++++++++++++++++++++------------ uw_myplan/dao.py | 23 ++++++++++++++++------- uw_myplan/tests/test_myplan.py | 15 +++++++-------- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index fc7dedc..47c3b15 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -14,17 +14,32 @@ MyPlan, MyPlanTerm, MyPlanCourse, MyPlanCourseSection) logger = logging.getLogger(__name__) +dao = MyPlan_DAO() -def get_plan(regid, year, quarter, terms=4): - dao = MyPlan_DAO() - url = get_plan_url(regid, year, quarter, terms) +def _get_plan_url(regid, year, quarter, terms): + return "/plan/v1/{year},{quarter},{terms},{uwregid}".format( + year=year, quarter=quarter, terms=terms, uwregid=regid) + + +def _get_resource(regid, year, quarter, terms, clear_cached_token=False): + if clear_cached_token: + dao.clear_access_token() + url = _get_plan_url(regid, year, quarter, terms) + return dao.getURL(url, {"Accept": "application/json"}) + - response = dao.getURL(url, {"Accept": "application/json"}) - logger.debug( - {'url': url, 'status': response.status, 'data': response.data}) +def get_plan(regid, year, quarter, terms=4): + url = _get_plan_url(regid, year, quarter, terms) + response = _get_resource(regid, year, quarter, terms) if response.status != 200: - raise DataFailureException(url, response.status, str(response.data)) + if response.status == 401 or response.status == 403: + # clear cached access token, retry once + response = _get_resource( + regid, year, quarter, terms, clear_cached_token=False) + if response.status != 200: + raise DataFailureException( + url, response.status, str(response.data)) data = json.loads(response.data) @@ -59,8 +74,3 @@ def get_plan(regid, year, quarter, terms=4): term.courses.append(course) plan.terms.append(term) return plan - - -def get_plan_url(regid, year, quarter, terms=4): - return "/plan/v1/{year},{quarter},{terms},{uwregid}".format( - year=year, quarter=quarter, terms=terms, uwregid=regid) diff --git a/uw_myplan/dao.py b/uw_myplan/dao.py index e548a81..ac7fa2e 100644 --- a/uw_myplan/dao.py +++ b/uw_myplan/dao.py @@ -9,6 +9,7 @@ from restclients_core.exceptions import DataFailureException logger = logging.getLogger(__name__) +myplan_access_token_url = "/oauth2/token" class MyPlan_Auth_DAO(DAO): @@ -18,18 +19,23 @@ def service_name(self): def _is_cacheable(self, method, url, headers, body=None): return True + def clear_token_from_cache(self): + self.clear_cached_response(myplan_access_token_url) + def get_auth_token(self, secret): - url = "/oauth2/token" headers = {"Authorization": "Basic {}".format(secret), "Content-type": "application/x-www-form-urlencoded"} - response = self.postURL(url, headers, "grant_type=client_credentials") - logger.debug( - {'url': url, - 'status': response.status, - 'data': response.data}) + response = self.postURL( + myplan_access_token_url, headers, "grant_type=client_credentials") + if response.status != 200: - raise DataFailureException(url, response.status, response.data) + logger.error( + {'url': myplan_access_token_url, + 'status': response.status, + 'data': response.data}) + raise DataFailureException( + myplan_access_token_url, response.status, response.data) data = json.loads(response.data) return data.get("access_token", "") @@ -70,3 +76,6 @@ def _custom_headers(self, method, url, headers, body): secret[:10], secret[-10:],)) headers["Authorization"] = self.auth_dao.get_auth_token(secret) return headers + + def clear_access_token(self): + self.auth_dao.clear_token_from_cache() diff --git a/uw_myplan/tests/test_myplan.py b/uw_myplan/tests/test_myplan.py index 7c1de71..8fab8c6 100644 --- a/uw_myplan/tests/test_myplan.py +++ b/uw_myplan/tests/test_myplan.py @@ -2,23 +2,17 @@ # SPDX-License-Identifier: Apache-2.0 from unittest import TestCase -from uw_myplan import get_plan, get_plan_url +from uw_myplan import get_plan, _get_plan_url, _get_resource class MyPlanTestData(TestCase): def test_plan_url(self): self.assertEquals( - get_plan_url( + _get_plan_url( "9136CCB8F66711D5BE060004AC494FFE", 2013, "spring", 2), ( "/plan/v1/2013,spring,2," "9136CCB8F66711D5BE060004AC494FFE")) - self.assertEquals( - get_plan_url( - "9136CCB8F66711D5BE060004AC494FFE", 2012, "summer"), ( - "/plan/v1/2012,summer,4," - "9136CCB8F66711D5BE060004AC494FFE")) - def test_javerage(self): plan = get_plan(regid="9136CCB8F66711D5BE060004AC494FFE", year=2013, @@ -67,6 +61,11 @@ def test_javerage(self): self.assertEquals(term_data.courses[0].sections[1].section_id, 'AA') self.assertEquals(term_data.courses[0].sections[2].section_id, 'AB') + resp = _get_resource( + "9136CCB8F66711D5BE060004AC494FFE", 2013, "spring", + 4, clear_cached_token=True) + self.assertIsNotNone(resp) + def test_json(self): plan = get_plan(regid="9136CCB8F66711D5BE060004AC494FFE", year=2013, quarter="spring", From c27516b8cd52e188dbf00927053fdeaae391fc1d Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Fri, 4 Mar 2022 16:10:00 -0800 Subject: [PATCH 2/7] tidy --- uw_myplan/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index 47c3b15..7585e5c 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -25,12 +25,12 @@ def _get_plan_url(regid, year, quarter, terms): def _get_resource(regid, year, quarter, terms, clear_cached_token=False): if clear_cached_token: dao.clear_access_token() - url = _get_plan_url(regid, year, quarter, terms) - return dao.getURL(url, {"Accept": "application/json"}) + return dao.getURL( + _get_plan_url(regid, year, quarter, terms), + {"Accept": "application/json"}) def get_plan(regid, year, quarter, terms=4): - url = _get_plan_url(regid, year, quarter, terms) response = _get_resource(regid, year, quarter, terms) if response.status != 200: if response.status == 401 or response.status == 403: @@ -39,7 +39,8 @@ def get_plan(regid, year, quarter, terms=4): regid, year, quarter, terms, clear_cached_token=False) if response.status != 200: raise DataFailureException( - url, response.status, str(response.data)) + _get_plan_url(regid, year, quarter, terms), + response.status, str(response.data)) data = json.loads(response.data) From dafea62cd67a40f08c3471cb27d8289b1eca358d Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Fri, 4 Mar 2022 16:12:52 -0800 Subject: [PATCH 3/7] style fix --- uw_myplan/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index 7585e5c..7ea1c55 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -27,7 +27,7 @@ def _get_resource(regid, year, quarter, terms, clear_cached_token=False): dao.clear_access_token() return dao.getURL( _get_plan_url(regid, year, quarter, terms), - {"Accept": "application/json"}) + {"Accept": "application/json"}) def get_plan(regid, year, quarter, terms=4): From d7ced1ff1467b50e6ed0d93d677d28e14c5212a1 Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Fri, 4 Mar 2022 17:33:22 -0800 Subject: [PATCH 4/7] clean up --- uw_myplan/dao.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/uw_myplan/dao.py b/uw_myplan/dao.py index ac7fa2e..c5a459b 100644 --- a/uw_myplan/dao.py +++ b/uw_myplan/dao.py @@ -72,8 +72,8 @@ def _custom_headers(self, method, url, headers, body): headers = {} secret = self.get_service_setting("AUTH_SECRET", "") if secret: - logger.info("AUTH_SECRET: {}...{}".format( - secret[:10], secret[-10:],)) + logger.info( + "AUTH_SECRET: {}...{}".format(secret[:10], secret[-10:])) headers["Authorization"] = self.auth_dao.get_auth_token(secret) return headers From 3292a1badf05a5147ae23928a1649c62d878501b Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Sun, 6 Mar 2022 19:08:42 -0800 Subject: [PATCH 5/7] add logging --- uw_myplan/__init__.py | 36 ++++++++++++++++++++++++------------ uw_myplan/dao.py | 13 +++++++++---- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index 7ea1c55..3e7fccb 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -32,20 +32,32 @@ def _get_resource(regid, year, quarter, terms, clear_cached_token=False): def get_plan(regid, year, quarter, terms=4): response = _get_resource(regid, year, quarter, terms) - if response.status != 200: - if response.status == 401 or response.status == 403: - # clear cached access token, retry once - response = _get_resource( - regid, year, quarter, terms, clear_cached_token=False) - if response.status != 200: - raise DataFailureException( - _get_plan_url(regid, year, quarter, terms), - response.status, str(response.data)) - - data = json.loads(response.data) + logger.info( + {'url': _get_plan_url(regid, year, quarter, terms), + 'status': response.status, + 'data': response.data}) + if response.status == 200: + return _process_data(json.loads(response.data)) + + if response.status == 401 or response.status == 403: + # clear cached access token, retry once + response = _get_resource( + regid, year, quarter, terms, clear_cached_token=True) + logger.info( + {'url': _get_plan_url(regid, year, quarter, terms), + 'status': response.status, + 'data': response.data}) + if response.status == 200: + return _process_data(json.loads(response.data)) + + raise DataFailureException( + _get_plan_url(regid, year, quarter, terms), + response.status, str(response.data)) + +def _process_data(jdata): plan = MyPlan() - for term_data in data: + for term_data in jdata: term = MyPlanTerm() term.year = term_data["Term"]["Year"] term.quarter = term_data["Term"]["Quarter"] diff --git a/uw_myplan/dao.py b/uw_myplan/dao.py index c5a459b..a966b41 100644 --- a/uw_myplan/dao.py +++ b/uw_myplan/dao.py @@ -28,7 +28,10 @@ def get_auth_token(self, secret): response = self.postURL( myplan_access_token_url, headers, "grant_type=client_credentials") - + logger.info( + {'url': myplan_access_token_url, + 'status': response.status, + 'data': response.data}) if response.status != 200: logger.error( {'url': myplan_access_token_url, @@ -69,11 +72,13 @@ def service_mock_paths(self): return [abspath(os.path.join(dirname(__file__), "resources"))] def _custom_headers(self, method, url, headers, body): - headers = {} + if not headers: + headers = {} secret = self.get_service_setting("AUTH_SECRET", "") + logger.info( + "AUTH_SECRET: {}...{}".format(secret[:10], secret[-10:])) + if secret: - logger.info( - "AUTH_SECRET: {}...{}".format(secret[:10], secret[-10:])) headers["Authorization"] = self.auth_dao.get_auth_token(secret) return headers From af135502ba36ffe8ad1252ffd898e3ca2c6b92e0 Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Sun, 6 Mar 2022 20:26:52 -0800 Subject: [PATCH 6/7] remove debugging --- uw_myplan/__init__.py | 8 -------- uw_myplan/dao.py | 7 ------- 2 files changed, 15 deletions(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index 3e7fccb..a8a160a 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -32,10 +32,6 @@ def _get_resource(regid, year, quarter, terms, clear_cached_token=False): def get_plan(regid, year, quarter, terms=4): response = _get_resource(regid, year, quarter, terms) - logger.info( - {'url': _get_plan_url(regid, year, quarter, terms), - 'status': response.status, - 'data': response.data}) if response.status == 200: return _process_data(json.loads(response.data)) @@ -43,10 +39,6 @@ def get_plan(regid, year, quarter, terms=4): # clear cached access token, retry once response = _get_resource( regid, year, quarter, terms, clear_cached_token=True) - logger.info( - {'url': _get_plan_url(regid, year, quarter, terms), - 'status': response.status, - 'data': response.data}) if response.status == 200: return _process_data(json.loads(response.data)) diff --git a/uw_myplan/dao.py b/uw_myplan/dao.py index a966b41..86b046f 100644 --- a/uw_myplan/dao.py +++ b/uw_myplan/dao.py @@ -28,10 +28,6 @@ def get_auth_token(self, secret): response = self.postURL( myplan_access_token_url, headers, "grant_type=client_credentials") - logger.info( - {'url': myplan_access_token_url, - 'status': response.status, - 'data': response.data}) if response.status != 200: logger.error( {'url': myplan_access_token_url, @@ -75,9 +71,6 @@ def _custom_headers(self, method, url, headers, body): if not headers: headers = {} secret = self.get_service_setting("AUTH_SECRET", "") - logger.info( - "AUTH_SECRET: {}...{}".format(secret[:10], secret[-10:])) - if secret: headers["Authorization"] = self.auth_dao.get_auth_token(secret) return headers From eac37137afd4ddb27b7f6d7db8d8bfcd5ef906db Mon Sep 17 00:00:00 2001 From: Fang Lin Date: Mon, 7 Mar 2022 14:13:28 -0800 Subject: [PATCH 7/7] Refactor to class, add test coverage --- uw_myplan/__init__.py | 118 +++++++++++++++++---------------- uw_myplan/tests/test_dao.py | 11 +++ uw_myplan/tests/test_myplan.py | 42 +++++++++--- 3 files changed, 102 insertions(+), 69 deletions(-) diff --git a/uw_myplan/__init__.py b/uw_myplan/__init__.py index a8a160a..74539f8 100644 --- a/uw_myplan/__init__.py +++ b/uw_myplan/__init__.py @@ -14,68 +14,70 @@ MyPlan, MyPlanTerm, MyPlanCourse, MyPlanCourseSection) logger = logging.getLogger(__name__) -dao = MyPlan_DAO() -def _get_plan_url(regid, year, quarter, terms): - return "/plan/v1/{year},{quarter},{terms},{uwregid}".format( - year=year, quarter=quarter, terms=terms, uwregid=regid) +class Plan(object): + def __init__(self, actas=None): + self.dao = MyPlan_DAO() -def _get_resource(regid, year, quarter, terms, clear_cached_token=False): - if clear_cached_token: - dao.clear_access_token() - return dao.getURL( - _get_plan_url(regid, year, quarter, terms), - {"Accept": "application/json"}) + def _get_plan_url(self, regid, year, quarter, terms): + return "/plan/v1/{year},{quarter},{terms},{uwregid}".format( + year=year, quarter=quarter, terms=terms, uwregid=regid) + def _get_resource(self, regid, year, quarter, terms, + clear_cached_token=False): + if clear_cached_token: + self.dao.clear_access_token() + return self.dao.getURL( + self._get_plan_url(regid, year, quarter, terms), + {"Accept": "application/json"}) -def get_plan(regid, year, quarter, terms=4): - response = _get_resource(regid, year, quarter, terms) - if response.status == 200: - return _process_data(json.loads(response.data)) - - if response.status == 401 or response.status == 403: - # clear cached access token, retry once - response = _get_resource( - regid, year, quarter, terms, clear_cached_token=True) + def get_plan(self, regid, year, quarter, terms=4): + response = self._get_resource(regid, year, quarter, terms) if response.status == 200: - return _process_data(json.loads(response.data)) - - raise DataFailureException( - _get_plan_url(regid, year, quarter, terms), - response.status, str(response.data)) - - -def _process_data(jdata): - plan = MyPlan() - for term_data in jdata: - term = MyPlanTerm() - term.year = term_data["Term"]["Year"] - term.quarter = term_data["Term"]["Quarter"] - - term.course_search_href = term_data["CourseSearchHref"] - term.degree_audit_href = term_data["DegreeAuditHref"] - term.myplan_href = term_data["MyPlanHref"] - term.registration_href = term_data["RegistrationHref"] - term.registered_courses_count = int( - term_data["RegisteredCoursesCount"]) - term.registered_sections_count = int( - term_data["RegisteredSectionsCount"]) - - for course_data in term_data["Courses"]: - course = MyPlanCourse() - course.curriculum_abbr = course_data["CurriculumAbbreviation"] - course.course_number = course_data["CourseNumber"] - - is_available = course_data["RegistrationAvailable"] - course.registrations_available = is_available - - for section_data in course_data["Sections"]: - section = MyPlanCourseSection() - section.section_id = section_data["SectionId"] - course.sections.append(section) - - term.courses.append(course) - plan.terms.append(term) - return plan + return self._process_data(json.loads(response.data)) + + if response.status == 401 or response.status == 403: + # clear cached access token, retry once + response = self._get_resource( + regid, year, quarter, terms, clear_cached_token=True) + if response.status == 200: + return self._process_data(json.loads(response.data)) + + raise DataFailureException( + self._get_plan_url(regid, year, quarter, terms), + response.status, str(response.data)) + + def _process_data(self, jdata): + plan = MyPlan() + for term_data in jdata: + term = MyPlanTerm() + term.year = term_data["Term"]["Year"] + term.quarter = term_data["Term"]["Quarter"] + + term.course_search_href = term_data["CourseSearchHref"] + term.degree_audit_href = term_data["DegreeAuditHref"] + term.myplan_href = term_data["MyPlanHref"] + term.registration_href = term_data["RegistrationHref"] + term.registered_courses_count = int( + term_data["RegisteredCoursesCount"]) + term.registered_sections_count = int( + term_data["RegisteredSectionsCount"]) + + for course_data in term_data["Courses"]: + course = MyPlanCourse() + course.curriculum_abbr = course_data["CurriculumAbbreviation"] + course.course_number = course_data["CourseNumber"] + + is_available = course_data["RegistrationAvailable"] + course.registrations_available = is_available + + for section_data in course_data["Sections"]: + section = MyPlanCourseSection() + section.section_id = section_data["SectionId"] + course.sections.append(section) + + term.courses.append(course) + plan.terms.append(term) + return plan diff --git a/uw_myplan/tests/test_dao.py b/uw_myplan/tests/test_dao.py index 07ed2a7..2641b3c 100644 --- a/uw_myplan/tests/test_dao.py +++ b/uw_myplan/tests/test_dao.py @@ -5,6 +5,7 @@ import mock from commonconf import override_settings from restclients_core.exceptions import DataFailureException +from restclients_core.models import MockHTTP from uw_myplan.dao import MyPlan_Auth_DAO, MyPlan_DAO from uw_myplan.utils import ( fdao_myplan_override, fdao_myplan_auth_override) @@ -22,6 +23,16 @@ def test_get_auth_token(self): self.assertIsNotNone( MyPlan_Auth_DAO().get_auth_token("test1")) + @mock.patch.object(MyPlan_Auth_DAO, "postURL") + def test_get_auth_token(self, mock): + response = MockHTTP() + response.status = 404 + response.data = "Not Found" + mock.return_value = response + self.assertRaises( + DataFailureException, + MyPlan_Auth_DAO().get_auth_token, "test1") + def test_no_auth_header(self): headers = MyPlan_DAO()._custom_headers("GET", "/", {}, "") self.assertFalse("Authorization" in headers) diff --git a/uw_myplan/tests/test_myplan.py b/uw_myplan/tests/test_myplan.py index 8fab8c6..9f071c1 100644 --- a/uw_myplan/tests/test_myplan.py +++ b/uw_myplan/tests/test_myplan.py @@ -2,22 +2,40 @@ # SPDX-License-Identifier: Apache-2.0 from unittest import TestCase -from uw_myplan import get_plan, _get_plan_url, _get_resource +import mock +from restclients_core.models import MockHTTP +from restclients_core.exceptions import DataFailureException +from uw_myplan import Plan -class MyPlanTestData(TestCase): +class PlanTest(TestCase): def test_plan_url(self): self.assertEquals( - _get_plan_url( + Plan()._get_plan_url( "9136CCB8F66711D5BE060004AC494FFE", 2013, "spring", 2), ( "/plan/v1/2013,spring,2," "9136CCB8F66711D5BE060004AC494FFE")) + @mock.patch.object(Plan, "_get_resource") + def test_error_401(self, mock): + response = MockHTTP() + response.status = 403 + response.data = "Not Authorized" + mock.return_value = response + self.assertRaises( + DataFailureException, + Plan().get_plan, + "9136CCB8F66711D5BE060004AC494FFE", + 2013, + "spring", + terms=4) + def test_javerage(self): - plan = get_plan(regid="9136CCB8F66711D5BE060004AC494FFE", - year=2013, - quarter="spring", - terms=4) + plan = Plan().get_plan( + regid="9136CCB8F66711D5BE060004AC494FFE", + year=2013, + quarter="spring", + terms=4) self.assertEquals(len(plan.terms), 4) self.assertEquals(plan.terms[0].year, 2013) @@ -61,15 +79,17 @@ def test_javerage(self): self.assertEquals(term_data.courses[0].sections[1].section_id, 'AA') self.assertEquals(term_data.courses[0].sections[2].section_id, 'AB') - resp = _get_resource( + resp = Plan()._get_resource( "9136CCB8F66711D5BE060004AC494FFE", 2013, "spring", 4, clear_cached_token=True) self.assertIsNotNone(resp) def test_json(self): - plan = get_plan(regid="9136CCB8F66711D5BE060004AC494FFE", - year=2013, quarter="spring", - terms=4) + plan = Plan().get_plan( + regid="9136CCB8F66711D5BE060004AC494FFE", + year=2013, + quarter="spring", + terms=4) json_data = plan.json_data() term_data = json_data["terms"][0] self.assertEquals(