From 693383e88a96d71829fe0b79421558a30d1472ad Mon Sep 17 00:00:00 2001 From: Maxime Belanger Date: Thu, 31 Mar 2016 15:52:02 -0400 Subject: [PATCH] Refactored broken exception handling --- tests/api_test.py | 39 ---------------------- tests/ubersmith_request_test.py | 27 +++++++++++++-- ubersmith_client/ubersmith_api.py | 10 +----- ubersmith_client/ubersmith_request.py | 13 +++++++- ubersmith_client/ubersmith_request_get.py | 13 +++----- ubersmith_client/ubersmith_request_post.py | 9 ++--- 6 files changed, 48 insertions(+), 63 deletions(-) delete mode 100644 tests/api_test.py diff --git a/tests/api_test.py b/tests/api_test.py deleted file mode 100644 index 82e11f8..0000000 --- a/tests/api_test.py +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright 2016 Internap. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -import unittest -from hamcrest import assert_that, raises, calling -from mock import Mock -from requests.exceptions import ConnectionError, Timeout - -import ubersmith_client -from ubersmith_client.exceptions import UbersmithConnectionError, UbersmithTimeout - - -class ApiTest(unittest.TestCase): - def setUp(self): - self.url = 'http://ubersmith.example.com/' - self.username = 'admin' - self.password = 'test' - - def test_api_method_returns_handle_connection_error_exception(self): - ubersmith_api = ubersmith_client.api.init(self.url, self.username, self.password) - ubersmith_api.ubersmith_request = Mock(side_effect=ConnectionError()) - - assert_that(calling(ubersmith_api.__getattr__).with_args("client"), raises(UbersmithConnectionError)) - - def test_api_method_returns_handle_timeout_exception(self): - ubersmith_api = ubersmith_client.api.init(self.url, self.username, self.password) - ubersmith_api.ubersmith_request = Mock(side_effect=Timeout()) - - assert_that(calling(ubersmith_api.__getattr__).with_args("client"), raises(UbersmithTimeout)) diff --git a/tests/ubersmith_request_test.py b/tests/ubersmith_request_test.py index e3bf5a2..ef3d0ee 100644 --- a/tests/ubersmith_request_test.py +++ b/tests/ubersmith_request_test.py @@ -12,16 +12,25 @@ # See the License for the specific language governing permissions and # limitations under the License. import unittest -from mock import Mock + +import ubersmith_client +from mock import Mock, patch from hamcrest import assert_that, raises, calling +from requests.exceptions import ConnectionError, Timeout -from ubersmith_client.exceptions import UbersmithException, BadRequest, UnknownError, Forbidden, NotFound, Unauthorized +from ubersmith_client.exceptions import UbersmithException, BadRequest, UnknownError, Forbidden, NotFound, Unauthorized, UbersmithConnectionError, \ + UbersmithTimeout from tests.ubersmith_json.response_data_structure import a_response_data from ubersmith_client.ubersmith_request import UbersmithRequest class UbersmithRequestTest(unittest.TestCase): + def setUp(self): + self.url = 'http://ubersmith.example.com/' + self.username = 'admin' + self.password = 'test' + def test_process_ubersmith_response(self): response = Mock() response.status_code = 200 @@ -57,3 +66,17 @@ def test_process_ubersmith_response_raise_exception(self): response.json = Mock(return_value={'status': False, 'error_code': 42, 'error_message': 'come and watch tv'}) assert_that(calling(UbersmithRequest.process_ubersmith_response).with_args(response), raises(UbersmithException, "Error code 42 - message: come and watch tv")) + + @patch('ubersmith_client.ubersmith_request_post.requests') + def test_api_method_returns_handle_connection_error_exception(self, requests_mock): + ubersmith_api = ubersmith_client.api.init(self.url, self.username, self.password) + requests_mock.post = Mock(side_effect=ConnectionError()) + + assert_that(calling(ubersmith_api.client.list), raises(UbersmithConnectionError)) + + @patch('ubersmith_client.ubersmith_request_post.requests') + def test_api_method_returns_handle_timeout_exception(self, requests_mock): + ubersmith_api = ubersmith_client.api.init(self.url, self.username, self.password) + requests_mock.post = Mock(side_effect=Timeout()) + + assert_that(calling(ubersmith_api.client.list), raises(UbersmithTimeout)) diff --git a/ubersmith_client/ubersmith_api.py b/ubersmith_client/ubersmith_api.py index aaacf46..4731bc5 100644 --- a/ubersmith_client/ubersmith_api.py +++ b/ubersmith_client/ubersmith_api.py @@ -10,9 +10,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from requests.exceptions import ConnectionError, Timeout - -from ubersmith_client.exceptions import UbersmithConnectionError, UbersmithTimeout from ubersmith_client.ubersmith_request_get import UbersmithRequestGet from ubersmith_client.ubersmith_request_post import UbersmithRequestPost @@ -26,9 +23,4 @@ def __init__(self, url, user, password, timeout, use_http_get): self.ubersmith_request = UbersmithRequestGet if use_http_get else UbersmithRequestPost def __getattr__(self, module): - try: - return self.ubersmith_request(self.url, self.user, self.password, module, self.timeout) - except ConnectionError: - raise UbersmithConnectionError(self.url) - except Timeout: - raise UbersmithTimeout(self.url, self.timeout) + return self.ubersmith_request(self.url, self.user, self.password, module, self.timeout) diff --git a/ubersmith_client/ubersmith_request.py b/ubersmith_client/ubersmith_request.py index d02785d..905ccfc 100644 --- a/ubersmith_client/ubersmith_request.py +++ b/ubersmith_client/ubersmith_request.py @@ -11,8 +11,10 @@ # See the License for the specific language governing permissions and # limitations under the License. from abc import abstractmethod +from requests import Timeout, ConnectionError -from ubersmith_client.exceptions import get_exception_for, UbersmithException +from ubersmith_client.exceptions import get_exception_for, UbersmithException, UbersmithConnectionError, \ + UbersmithTimeout class UbersmithRequest(object): @@ -32,6 +34,15 @@ def __getattr__(self, function): def __call__(self, **kwargs): raise + def _process_request(self, method, **kwargs): + try: + return method(**kwargs) + + except ConnectionError: + raise UbersmithConnectionError(self.url) + except Timeout: + raise UbersmithTimeout(self.url, self.timeout) + def _build_request_params(self, kwargs): _methods = ".".join(self.methods) kwargs['method'] = "{0}.{1}".format(self.module, _methods) diff --git a/ubersmith_client/ubersmith_request_get.py b/ubersmith_client/ubersmith_request_get.py index c7d48e0..246278a 100644 --- a/ubersmith_client/ubersmith_request_get.py +++ b/ubersmith_client/ubersmith_request_get.py @@ -16,16 +16,13 @@ class UbersmithRequestGet(UbersmithRequest): - def __getattr__(self, function): - self.methods.append(function) - return self - def __call__(self, **kwargs): self._build_request_params(kwargs) - response = requests.get(url=self.url, - auth=(self.user, self.password), - timeout=self.timeout, - params=kwargs) + response = self._process_request(method=requests.get, + url=self.url, + auth=(self.user, self.password), + timeout=self.timeout, + params=kwargs) return UbersmithRequest.process_ubersmith_response(response) diff --git a/ubersmith_client/ubersmith_request_post.py b/ubersmith_client/ubersmith_request_post.py index 73ad2a8..5c7e74b 100644 --- a/ubersmith_client/ubersmith_request_post.py +++ b/ubersmith_client/ubersmith_request_post.py @@ -19,9 +19,10 @@ class UbersmithRequestPost(UbersmithRequest): def __call__(self, **kwargs): self._build_request_params(kwargs) - response = requests.post(url=self.url, - auth=(self.user, self.password), - timeout=self.timeout, - data=kwargs) + response = self._process_request(method=requests.post, + url=self.url, + auth=(self.user, self.password), + timeout=self.timeout, + data=kwargs) return UbersmithRequest.process_ubersmith_response(response)