From fe68ca46240e581152855848dc0c8d3af5acd4c0 Mon Sep 17 00:00:00 2001 From: Karan Panchal Date: Mon, 5 Apr 2021 00:00:51 +0530 Subject: [PATCH 1/6] TDL-593: Adding backoff behavior of 429 errors --- tap_xero/client.py | 55 +++++++++++-- tests/unittests/test_exception_handling.py | 93 ++++++++++++++++++++++ 2 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 tests/unittests/test_exception_handling.py diff --git a/tap_xero/client.py b/tap_xero/client.py index 6f06702..183063c 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -8,10 +8,24 @@ from singer.utils import strftime, strptime_to_utc import six import pytz +import backoff BASE_URL = "https://api.xero.com/api.xro/2.0" +class XeroError(Exception): + pass + + +class XeroTooManyError(XeroError): + pass + + +ERROR_CODE_EXCEPTION_MAPPING = { + 429: XeroTooManyError +} + + def parse_date(value): # Xero datetimes can be .NET JSON date strings which look like # "/Date(1419937200000+0000)/" @@ -96,6 +110,7 @@ def refresh_credentials(self, config, config_path): self.access_token = resp["access_token"] self.tenant_id = config['tenant_id'] + @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "") url = join(BASE_URL, xero_resource_name) @@ -109,9 +124,37 @@ def filter(self, tap_stream_id, since=None, **params): request = requests.Request("GET", url, headers=headers, params=params) response = self.session.send(request.prepare()) - response.raise_for_status() - response_meta = json.loads(response.text, - object_hook=_json_load_object_hook, - parse_float=decimal.Decimal) - response_body = response_meta.pop(xero_resource_name) - return response_body + + if response.status_code != 200: + raise_for_error(response) + return None + else: + response_meta = json.loads(response.text, + object_hook=_json_load_object_hook, + parse_float=decimal.Decimal) + response_body = response_meta.pop(xero_resource_name) + return response_body + + +def raise_for_error(resp): + try: + resp.raise_for_status() + except (requests.HTTPError, requests.ConnectionError) as error: + try: + error_code = resp.status_code + + # Response for Status code 429 contains all necessary information in the headers parameter + if error_code == 429: + # Forming a response message for raising custom exception + resp_headers = resp.headers + message = "Error: Too Many Requests. Please retry after {} seconds".format(resp_headers.get("Retry-After")) + else: + # Forming a response message for raising custom exception + response_json = resp.json() + message = "Error: {}".format(response_json.get("error", response_json.get("Title", response_json.get("Detail", "Unknown Error")))) + + exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, XeroError) + raise exc(message) + + except (ValueError, TypeError): + raise XeroError(error) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py new file mode 100644 index 0000000..3b5f7a6 --- /dev/null +++ b/tests/unittests/test_exception_handling.py @@ -0,0 +1,93 @@ +import tap_xero.client as client_ +import unittest +import requests +from unittest import mock +import decimal +import json + + +def mocked_session(*args, **kwargs): + class Mocksession: + def __init__(self, json_data, status_code, content, headers, raise_error): + self.text = json_data + self.status_code = status_code + self.raise_error = raise_error + if headers: + self.headers = headers + + def raise_for_status(self): + if not raise_error: + return self.status_code + + raise requests.HTTPError("sample message") + + arguments_to_session = args[0] + + json_data = arguments_to_session[0] + status_code = arguments_to_session[1] + content = arguments_to_session[2] + headers = arguments_to_session[3] + raise_error = arguments_to_session[4] + return Mocksession(json_data, status_code, content, headers, raise_error) + + +class Mockresponse: + def __init__(self, resp, status_code, content=[], headers=None, raise_error=False): + self.json_data = resp + self.status_code = status_code + self.content = content + self.headers = headers + self.raise_error = raise_error + + def prepare(self): + return (self.json_data, self.status_code, self.content, self.headers, self.raise_error) + + +def mocked_failed_429_request(*args, **kwargs): + json_decode_str = '' + headers = {"Retry-After": 10} + return Mockresponse(json_decode_str, 429, headers=headers, raise_error=True) + + +class TestFilterFunExceptionHandling(unittest.TestCase): + """ + Test cases to verify if the exceptions are handled as expected while communicating with Xero Environment + """ + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_failed_429_request) + def test_too_many_requests_custom_exception(self, mocked_session, mocked_failed_429_request): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + # Verifying if the custom exception 'XeroTooManyError' is raised on receiving status code 429 + self.assertRaises(client_.XeroTooManyError, xero_client.filter(tap_stream_id)) + except (requests.HTTPError, client_.XeroTooManyError) as e: + expected_error_message = "Error: Too Many Requests. Please retry after 10 seconds" + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_failed_429_request) + def test_too_many_requests_backoff_behavior(self, mocked_session, mocked_failed_429_request): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + try: + filter_func_exec = xero_client.filter(tap_stream_id) + except (requests.HTTPError, client_.XeroTooManyError) as e: + pass + + self.assertEqual(mocked_failed_429_request.call_count, 3) + self.assertEqual(mocked_session.call_count, 3) \ No newline at end of file From c6d742adfc7cc053b39fe3b762acf81b4eb6bf34 Mon Sep 17 00:00:00 2001 From: Karan Panchal Date: Mon, 5 Apr 2021 20:24:14 +0530 Subject: [PATCH 2/6] TDL-593: Adding the factor parameter to increase the wait time between two executions --- tap_xero/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 183063c..2d389b8 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -110,7 +110,7 @@ def refresh_credentials(self, config, config_path): self.access_token = resp["access_token"] self.tenant_id = config['tenant_id'] - @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3) + @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3, factor=2) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "") url = join(BASE_URL, xero_resource_name) From d1e74ba85bb95a677a75106bc43e18cc320d08c6 Mon Sep 17 00:00:00 2001 From: Karan Panchal Date: Tue, 6 Apr 2021 00:10:28 +0530 Subject: [PATCH 3/6] TDL-648: Added custom exceptions to handle xero REST API error codes --- tap_xero/client.py | 86 +++++++++-- tests/unittests/test_exception_handling.py | 168 ++++++++++++++++++++- 2 files changed, 239 insertions(+), 15 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 2d389b8..1223d3e 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -17,12 +17,63 @@ class XeroError(Exception): pass +class XeroBadRequestError(XeroError): + pass + + +class XeroUnauthorizedError(XeroError): + pass + + +class XeroForbiddenError(XeroError): + pass + + +class XeroNotFoundError(XeroError): + pass + + class XeroTooManyError(XeroError): pass +class XeroInternalError(XeroError): + pass + + +class XeroNotImplementedError(XeroError): + pass + + ERROR_CODE_EXCEPTION_MAPPING = { - 429: XeroTooManyError + 400: { + "raise_exception": XeroBadRequestError, + "message": "A validation exception has occurred." + }, + 401: { + "raise_exception": XeroUnauthorizedError, + "message": "Invalid authorization credentials." + }, + 403: { + "raise_exception": XeroForbiddenError, + "message": "User doesn't have permission to access the resource." + }, + 404: { + "raise_exception": XeroNotFoundError, + "message": "The resource you have specified cannot be found." + }, + 429: { + "raise_exception": XeroTooManyError, + "message": "The API rate limit for your organisation/application pairing has been exceeded" + }, + 500: { + "raise_exception": XeroInternalError, + "message": "An unhandled error with the Xero API. Contact the Xero API team if problems persist." + }, + 501: { + "raise_exception": XeroNotImplementedError, + "message": "The method you have called has not been implemented." + } } @@ -143,18 +194,33 @@ def raise_for_error(resp): try: error_code = resp.status_code - # Response for Status code 429 contains all necessary information in the headers parameter + # Handling status code 429 specially since the required information is present in the headers if error_code == 429: - # Forming a response message for raising custom exception resp_headers = resp.headers - message = "Error: Too Many Requests. Please retry after {} seconds".format(resp_headers.get("Retry-After")) + api_rate_limit_message = ERROR_CODE_EXCEPTION_MAPPING[429]["message"] + message = "HTTP-error-code: 429, Error: {}. Please retry after {} seconds".format(api_rate_limit_message, resp_headers.get("Retry-After")) + # Handling status code 403 specially since response of API does not contain enough information + elif error_code in (403, 401): + api_message = ERROR_CODE_EXCEPTION_MAPPING[error_code]["message"] + message = "HTTP-error-code: {}, Error: {}".format(error_code, api_message) else: # Forming a response message for raising custom exception - response_json = resp.json() - message = "Error: {}".format(response_json.get("error", response_json.get("Title", response_json.get("Detail", "Unknown Error")))) - - exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, XeroError) - raise exc(message) + try: + response_json = resp.json() + except Exception: + response_json = {} + + message = "HTTP-error-code: {}, Error: {}".format( + error_code, + response_json.get( + "error", response_json.get( + "Title", response_json.get( + "Detail", ERROR_CODE_EXCEPTION_MAPPING.get( + error_code, {}).get("message", "Unknown Error") + )))) + + exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", XeroError) + raise exc(message) from None except (ValueError, TypeError): - raise XeroError(error) + raise XeroError(error) from None diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 3b5f7a6..7ce7ab5 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -21,6 +21,9 @@ def raise_for_status(self): raise requests.HTTPError("sample message") + def json(self): + return self.text + arguments_to_session = args[0] json_data = arguments_to_session[0] @@ -43,20 +46,175 @@ def prepare(self): return (self.json_data, self.status_code, self.content, self.headers, self.raise_error) +def mocked_forbidden_403_exception(*args, **kwargs): + json_decode_str = {"Title": "Forbidden", "Detail": "AuthenticationUnsuccessful"} + + return Mockresponse(json_decode_str, 403, raise_error=True) + + +def mocked_badrequest_400_error(*args, **kwargs): + json_decode_str = {"Message": "Bad Request Error"} + + return Mockresponse(json_decode_str, 400, raise_error=True) + + +def mocked_unauthorized_401_error(*args, **kwargs): + json_decode_str = {"Title": "Unauthorized", "Detail": "AuthenticationUnsuccessful"} + + return Mockresponse(json_decode_str, 401, raise_error=True) + + +def mocked_notfound_404_error(*args, **kwargs): + json_decode_str = {} + + return Mockresponse(json_decode_str, 404, raise_error=True) + + def mocked_failed_429_request(*args, **kwargs): json_decode_str = '' headers = {"Retry-After": 10} return Mockresponse(json_decode_str, 429, headers=headers, raise_error=True) +def mocked_internalservererror_500_error(*args, **kwargs): + json_decode_str = {} + + return Mockresponse(json_decode_str, 500, raise_error=True) + + +def mocked_notimplemented_501_error(*args, **kwargs): + json_decode_str = {} + + return Mockresponse(json_decode_str, 501, raise_error=True) + class TestFilterFunExceptionHandling(unittest.TestCase): """ Test cases to verify if the exceptions are handled as expected while communicating with Xero Environment """ + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_badrequest_400_error) + def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroBadRequestError as e: + expected_error_message = "HTTP-error-code: 400, Error: A validation exception has occurred." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_unauthorized_401_error) + def test_unauthorized_401_error(self, mocked_session, mocked_unauthorized_401_error): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroUnauthorizedError as e: + expected_error_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_forbidden_403_exception) + def test_forbidden_403_exception(self, mocked_session, mocked_forbidden_403_exception): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroForbiddenError as e: + expected_error_message = "HTTP-error-code: 403, Error: User doesn't have permission to access the resource." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_notfound_404_error) + def test_notfound_404_error(self, mocked_session, mocked_notfound_404_error): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroNotFoundError as e: + expected_error_message = "HTTP-error-code: 404, Error: The resource you have specified cannot be found." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_internalservererror_500_error) + def test_internalservererror_500_error(self, mocked_session, mocked_internalservererror_500_error): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroInternalError as e: + expected_error_message = "HTTP-error-code: 500, Error: An unhandled error with the Xero API. Contact the Xero API team if problems persist." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + + @mock.patch('requests.Session.send', side_effect=mocked_session) + @mock.patch('requests.Request', side_effect=mocked_notimplemented_501_error) + def test_notimplemented_501_error(self, mocked_session, mocked_notimplemented_501_error): + config = {} + tap_stream_id = "contacts" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.filter(tap_stream_id) + except client_.XeroNotImplementedError as e: + expected_error_message = "HTTP-error-code: 501, Error: The method you have called has not been implemented." + + # Verifying the message formed for the custom exception + self.assertEquals(str(e), expected_error_message) + pass + + @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_failed_429_request) - def test_too_many_requests_custom_exception(self, mocked_session, mocked_failed_429_request): + def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_request): config = {} tap_stream_id = "contacts" @@ -66,9 +224,9 @@ def test_too_many_requests_custom_exception(self, mocked_session, mocked_failed_ try: # Verifying if the custom exception 'XeroTooManyError' is raised on receiving status code 429 - self.assertRaises(client_.XeroTooManyError, xero_client.filter(tap_stream_id)) - except (requests.HTTPError, client_.XeroTooManyError) as e: - expected_error_message = "Error: Too Many Requests. Please retry after 10 seconds" + filter_func_exec = xero_client.filter(tap_stream_id) + except client_.XeroTooManyError as e: + expected_error_message = "HTTP-error-code: 429, Error: The API rate limit for your organisation/application pairing has been exceeded. Please retry after 10 seconds" # Verifying the message formed for the custom exception self.assertEquals(str(e), expected_error_message) @@ -77,7 +235,7 @@ def test_too_many_requests_custom_exception(self, mocked_session, mocked_failed_ @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_failed_429_request) - def test_too_many_requests_backoff_behavior(self, mocked_session, mocked_failed_429_request): + def test_too_many_requests_429_backoff_behavior(self, mocked_session, mocked_failed_429_request): config = {} tap_stream_id = "contacts" From ff9c22a902fe90b302e8af726556f14319bb50f5 Mon Sep 17 00:00:00 2001 From: Karan Panchal Date: Tue, 6 Apr 2021 23:50:01 +0530 Subject: [PATCH 4/6] TDL-594: Adding the support of validating the authentication and authorization of the credentials passed in the config json file --- tap_xero/__init__.py | 2 +- tap_xero/client.py | 39 +++++++++++--- tap_xero/context.py | 3 ++ tests/unittests/test_exception_handling.py | 63 +++++++++++++++++++++- 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/tap_xero/__init__.py b/tap_xero/__init__.py index c0a1ff9..529790a 100644 --- a/tap_xero/__init__.py +++ b/tap_xero/__init__.py @@ -68,7 +68,7 @@ def ensure_credentials_are_valid(config): XeroClient(config).filter("currencies") def discover(ctx): - ctx.refresh_credentials() + ctx.check_platform_access() catalog = Catalog([]) for stream in streams_.all_streams: schema_dict = load_schema(stream.tap_stream_id) diff --git a/tap_xero/client.py b/tap_xero/client.py index 1223d3e..8fe0972 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -152,14 +152,39 @@ def refresh_credentials(self, config, config_path): "refresh_token": config["refresh_token"], } resp = self.session.post("https://identity.xero.com/connect/token", headers=headers, data=post_body) - resp.raise_for_status() - resp = resp.json() - # Write to config file - config['refresh_token'] = resp["refresh_token"] - update_config_file(config, config_path) - self.access_token = resp["access_token"] - self.tenant_id = config['tenant_id'] + if resp.status_code != 200: + raise_for_error(resp) + else: + resp = resp.json() + + # Write to config file + config['refresh_token'] = resp["refresh_token"] + update_config_file(config, config_path) + self.access_token = resp["access_token"] + self.tenant_id = config['tenant_id'] + + + def check_platform_access(self, config, config_path, check_authentication=True): + + # Validating the authentication of the provided configuration + if check_authentication: + self.refresh_credentials(config, config_path) + + headers = { + "Authorization": "Bearer " + self.access_token, + "Xero-Tenant-Id": self.tenant_id, + "Content-Type": "application/json" + } + + # Validating the authorization of the provided configuration + contacts_url = "https://api.xero.com/api.xro/2.0/Contacts" + request = requests.Request("GET", contacts_url, headers=headers) + response = self.session.send(request.prepare()) + + if response.status_code != 200: + raise_for_error(response) + @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3, factor=2) def filter(self, tap_stream_id, since=None, **params): diff --git a/tap_xero/context.py b/tap_xero/context.py index dc8e589..b2af815 100644 --- a/tap_xero/context.py +++ b/tap_xero/context.py @@ -14,6 +14,9 @@ def __init__(self, config, state, catalog, config_path): def refresh_credentials(self): self.client.refresh_credentials(self.config, self.config_path) + def check_platform_access(self): + self.client.check_platform_access(self.config, self.config_path, check_authentication=True) + def get_bookmark(self, path): return bks_.get_bookmark(self.state, *path) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 7ce7ab5..a01ea2a 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -87,6 +87,13 @@ def mocked_notimplemented_501_error(*args, **kwargs): return Mockresponse(json_decode_str, 501, raise_error=True) + +def mock_successful_request(*args, **kwargs): + json_decode_str = {} + + return Mockresponse(json_decode_str, 200) + + class TestFilterFunExceptionHandling(unittest.TestCase): """ Test cases to verify if the exceptions are handled as expected while communicating with Xero Environment @@ -248,4 +255,58 @@ def test_too_many_requests_429_backoff_behavior(self, mocked_session, mocked_fai pass self.assertEqual(mocked_failed_429_request.call_count, 3) - self.assertEqual(mocked_session.call_count, 3) \ No newline at end of file + self.assertEqual(mocked_session.call_count, 3) + + +@mock.patch("tap_xero.client.XeroClient.refresh_credentials") +@mock.patch('requests.Session.send', side_effect=mocked_session) +class TestCheckPlatformAccessBehavior(unittest.TestCase): + + + @mock.patch('requests.Request', side_effect=mock_successful_request) + def test_check_refresh_credential_call(self, mocked_refresh_credentials, mocked_session, mock_successful_request): + + config = {} + config_path = "" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + # Validating the default value of 'refresh_credentials' argument of check_platform_access function + xero_client.check_platform_access(config, config_path) + self.assertEqual(mocked_refresh_credentials.call_count, 1) + + + @mock.patch('requests.Request', side_effect=mocked_forbidden_403_exception) + def test_check_refresh_credential_function_skip_and_throw_403(self, mocked_refresh_credentials, mocked_session, mocked_forbidden_403_exception): + + config = {} + config_path = "" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.check_platform_access(config, config_path, check_authentication=False) + except client_.XeroForbiddenError as e: + expected_message = "HTTP-error-code: 403, Error: User doesn't have permission to access the resource." + self.assertEqual(str(e) ,expected_message) + + + @mock.patch('requests.Request', side_effect=mocked_unauthorized_401_error) + def test_check_refresh_credential_function_skip_and_throw_401(self, mocked_refresh_credentials, mocked_session, mocked_unauthorized_401_error): + + config = {} + config_path = "" + + xero_client = client_.XeroClient(config) + xero_client.access_token = "123" + xero_client.tenant_id = "123" + + try: + xero_client.check_platform_access(config, config_path, check_authentication=False) + except client_.XeroUnauthorizedError as e: + expected_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." + self.assertEqual(str(e) ,expected_message) \ No newline at end of file From 587956ff086f202bd78b2e0c25d0f30f5e2064db Mon Sep 17 00:00:00 2001 From: Karan Panchal Date: Wed, 7 Apr 2021 00:57:23 +0530 Subject: [PATCH 5/6] TDL-594: Adding a positive scenario where discovery mode passes successfully --- tap_xero/client.py | 5 +- tap_xero/context.py | 2 +- tests/unittests/test_exception_handling.py | 83 +++++++++++++--------- 3 files changed, 54 insertions(+), 36 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 8fe0972..c3cde72 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -165,11 +165,10 @@ def refresh_credentials(self, config, config_path): self.tenant_id = config['tenant_id'] - def check_platform_access(self, config, config_path, check_authentication=True): + def check_platform_access(self, config, config_path): # Validating the authentication of the provided configuration - if check_authentication: - self.refresh_credentials(config, config_path) + self.refresh_credentials(config, config_path) headers = { "Authorization": "Bearer " + self.access_token, diff --git a/tap_xero/context.py b/tap_xero/context.py index b2af815..fabf002 100644 --- a/tap_xero/context.py +++ b/tap_xero/context.py @@ -15,7 +15,7 @@ def refresh_credentials(self): self.client.refresh_credentials(self.config, self.config_path) def check_platform_access(self): - self.client.check_platform_access(self.config, self.config_path, check_authentication=True) + self.client.check_platform_access(self.config, self.config_path) def get_bookmark(self, path): return bks_.get_bookmark(self.state, *path) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index a01ea2a..aa3cdbc 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -16,7 +16,7 @@ def __init__(self, json_data, status_code, content, headers, raise_error): self.headers = headers def raise_for_status(self): - if not raise_error: + if not self.raise_error: return self.status_code raise requests.HTTPError("sample message") @@ -45,6 +45,12 @@ def __init__(self, resp, status_code, content=[], headers=None, raise_error=Fals def prepare(self): return (self.json_data, self.status_code, self.content, self.headers, self.raise_error) + def raise_for_status(self): + if not self.raise_error: + return self.status_code + + raise requests.HTTPError("sample message") + def mocked_forbidden_403_exception(*args, **kwargs): json_decode_str = {"Title": "Forbidden", "Detail": "AuthenticationUnsuccessful"} @@ -94,12 +100,18 @@ def mock_successful_request(*args, **kwargs): return Mockresponse(json_decode_str, 200) +def mock_successful_session_post(*args, **kwargs): + json_decode_str = {"access_token": "123", "refresh_token": "345"} + + return mocked_session((json_decode_str, 200, [], None, False)) + + +@mock.patch('requests.Session.send', side_effect=mocked_session) class TestFilterFunExceptionHandling(unittest.TestCase): """ Test cases to verify if the exceptions are handled as expected while communicating with Xero Environment """ - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_badrequest_400_error) def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error): config = {} @@ -119,7 +131,6 @@ def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error) pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_unauthorized_401_error) def test_unauthorized_401_error(self, mocked_session, mocked_unauthorized_401_error): config = {} @@ -139,7 +150,6 @@ def test_unauthorized_401_error(self, mocked_session, mocked_unauthorized_401_er pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_forbidden_403_exception) def test_forbidden_403_exception(self, mocked_session, mocked_forbidden_403_exception): config = {} @@ -159,7 +169,6 @@ def test_forbidden_403_exception(self, mocked_session, mocked_forbidden_403_exce pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_notfound_404_error) def test_notfound_404_error(self, mocked_session, mocked_notfound_404_error): config = {} @@ -179,7 +188,6 @@ def test_notfound_404_error(self, mocked_session, mocked_notfound_404_error): pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_internalservererror_500_error) def test_internalservererror_500_error(self, mocked_session, mocked_internalservererror_500_error): config = {} @@ -199,7 +207,6 @@ def test_internalservererror_500_error(self, mocked_session, mocked_internalserv pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_notimplemented_501_error) def test_notimplemented_501_error(self, mocked_session, mocked_notimplemented_501_error): config = {} @@ -219,7 +226,6 @@ def test_notimplemented_501_error(self, mocked_session, mocked_notimplemented_50 pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_failed_429_request) def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_request): config = {} @@ -240,7 +246,6 @@ def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_req pass - @mock.patch('requests.Session.send', side_effect=mocked_session) @mock.patch('requests.Request', side_effect=mocked_failed_429_request) def test_too_many_requests_429_backoff_behavior(self, mocked_session, mocked_failed_429_request): config = {} @@ -258,29 +263,34 @@ def test_too_many_requests_429_backoff_behavior(self, mocked_session, mocked_fai self.assertEqual(mocked_session.call_count, 3) -@mock.patch("tap_xero.client.XeroClient.refresh_credentials") + @mock.patch('requests.Session.send', side_effect=mocked_session) class TestCheckPlatformAccessBehavior(unittest.TestCase): - - @mock.patch('requests.Request', side_effect=mock_successful_request) - def test_check_refresh_credential_call(self, mocked_refresh_credentials, mocked_session, mock_successful_request): - - config = {} + @mock.patch('requests.Session.post', side_effect=mocked_unauthorized_401_error) + def test_check_unauthorized_401_error_in_discovery_mode(self, mocked_unauthorized_401_error, mocked_session): + config = { + "client_id": "123", + "client_secret": "123", + "refresh_token": "123", + "tenant_id": "123" + } config_path = "" xero_client = client_.XeroClient(config) - xero_client.access_token = "123" - xero_client.tenant_id = "123" - # Validating the default value of 'refresh_credentials' argument of check_platform_access function - xero_client.check_platform_access(config, config_path) - self.assertEqual(mocked_refresh_credentials.call_count, 1) + try: + xero_client.check_platform_access(config, config_path) + except client_.XeroUnauthorizedError as e: + expected_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." + self.assertEqual(str(e) ,expected_message) + @mock.patch("tap_xero.client.XeroClient.refresh_credentials") @mock.patch('requests.Request', side_effect=mocked_forbidden_403_exception) - def test_check_refresh_credential_function_skip_and_throw_403(self, mocked_refresh_credentials, mocked_session, mocked_forbidden_403_exception): + def test_check_forbidden_403_error_in_discovery_mode(self, mocked_refresh_credentials, mocked_session, mocked_forbidden_403_exception): + mocked_refresh_credentials.return_value = "" config = {} config_path = "" @@ -289,24 +299,33 @@ def test_check_refresh_credential_function_skip_and_throw_403(self, mocked_refre xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path, check_authentication=False) + xero_client.check_platform_access(config, config_path) except client_.XeroForbiddenError as e: expected_message = "HTTP-error-code: 403, Error: User doesn't have permission to access the resource." self.assertEqual(str(e) ,expected_message) - @mock.patch('requests.Request', side_effect=mocked_unauthorized_401_error) - def test_check_refresh_credential_function_skip_and_throw_401(self, mocked_refresh_credentials, mocked_session, mocked_unauthorized_401_error): + @mock.patch('requests.Session.post', side_effect=mock_successful_session_post) + @mock.patch('tap_xero.client.update_config_file') + @mock.patch('requests.Request', side_effect=mock_successful_request) + def test_check_success_200_in_discovery_mode(self, mock_successful_session_post, mocked_update_config_file, mocked_session, mock_successful_request): - config = {} + mocked_update_config_file.return_value = "" + + config = { + "client_id": "123", + "client_secret": "123", + "refresh_token": "123", + "tenant_id": "123" + } config_path = "" xero_client = client_.XeroClient(config) - xero_client.access_token = "123" - xero_client.tenant_id = "123" + expected_access_token = "123" + expected_refresh_token = "345" - try: - xero_client.check_platform_access(config, config_path, check_authentication=False) - except client_.XeroUnauthorizedError as e: - expected_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." - self.assertEqual(str(e) ,expected_message) \ No newline at end of file + xero_client.check_platform_access(config, config_path) + + self.assertEqual(xero_client.access_token, expected_access_token) + self.assertEqual(config["refresh_token"], expected_refresh_token) + self.assertEqual(xero_client.tenant_id, config["tenant_id"]) \ No newline at end of file From 94b7e41bdf6538de263d0217932bc71b3d9259ce Mon Sep 17 00:00:00 2001 From: "Karan Panchal (C)" Date: Fri, 9 Apr 2021 18:05:42 +0530 Subject: [PATCH 6/6] TDL-593: Resolving code review comment --- tap_xero/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 6cf6eff..40f8f14 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -185,8 +185,7 @@ def check_platform_access(self, config, config_path): raise_for_error(response) - @backoff.on_exception(backoff.expo,json.decoder.JSONDecodeError,max_tries=3) - @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3, factor=2) + @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroTooManyError), max_tries=3) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "") url = join(BASE_URL, xero_resource_name)