From 41df1136900edd4507ab0319e1d0671c8b381008 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Sun, 11 Aug 2019 16:54:46 +0200 Subject: [PATCH] arpege_ecp: do not log requests errors (#35358) Add proper conversion of all HTTP errors to APIError. --- passerelle/apps/arpege_ecp/models.py | 31 ++++++++++----- tests/test_arpege_ecp.py | 57 ++++++++++++++-------------- tests/utils.py | 17 ++++++--- 3 files changed, 61 insertions(+), 44 deletions(-) diff --git a/passerelle/apps/arpege_ecp/models.py b/passerelle/apps/arpege_ecp/models.py index 31c216e3..7f7f64eb 100644 --- a/passerelle/apps/arpege_ecp/models.py +++ b/passerelle/apps/arpege_ecp/models.py @@ -17,6 +17,8 @@ import json import urlparse +from requests import RequestException + from django.db import models from django.utils.translation import ugettext_lazy as _ from django.utils.dateparse import parse_date, parse_time @@ -29,6 +31,8 @@ from passerelle.utils.jsonresponse import APIError class ArpegeECP(BaseResource): + log_requests_errors = False + category = _('Business Process Connectors') webservice_base_url = models.URLField(_('Webservice Base URL')) hawk_auth_id = models.CharField(_('Hawk Authentication id'), max_length=64) @@ -39,22 +43,27 @@ class ArpegeECP(BaseResource): def check_status(self): url = urlparse.urljoin(self.webservice_base_url, 'Hello') - response = self.requests.get(url, auth=HawkAuth(self.hawk_auth_id, self.hawk_auth_key)) - response.raise_for_status() + try: + response = self.requests.get(url, auth=HawkAuth(self.hawk_auth_id, self.hawk_auth_key)) + response.raise_for_status() + except RequestException as e: + raise Exception('Arpege server is down: %s' % e) if not response.json().get('Data'): raise Exception('Invalid credentials') return {'data': response.json()['Data']} def get_access_token(self, NameID): url = urlparse.urljoin(self.webservice_base_url, 'LoginParSubOIDC') - response = self.requests.post(url, auth=HawkAuth(self.hawk_auth_id, self.hawk_auth_key), - json={'subOIDC': NameID}) - if response.status_code // 100 != 2: - raise APIError(u'HTTP error: %s' % response.status_code) + try: + response = self.requests.post(url, auth=HawkAuth(self.hawk_auth_id, self.hawk_auth_key), + json={'subOIDC': NameID}) + response.raise_for_status() + except RequestException as e: + raise APIError(u'Arpege server is down: %s' % e) try: result = response.json() except ValueError: - raise APIError(u'No JSON content returned: %r' % response.content[:1000]) + raise APIError(u'Arpege server is down: no JSON content returned, %r' % response.content[:1000]) if result.get('Data'): if 'AccessToken' not in result['Data']: raise APIError(u'Error on LoginParSubOIDC: missing Data/AccessToken') @@ -69,10 +78,12 @@ class ArpegeECP(BaseResource): url = urlparse.urljoin(self.webservice_base_url, 'DemandesUsager') params = {'scope': 'data_administratives'} auth = HawkAuth(self.hawk_auth_id, self.hawk_auth_key, ext=access_token) - response = self.requests.get(url, params=params, auth=auth) + try: + response = self.requests.get(url, params=params, auth=auth) + response.raise_for_status() + except RequestException as e: + raise APIError(u'Arpege server is down: %s' % e) data = [] - if response.status_code // 100 != 2: - raise APIError(u'HTTP error: %s' % response.status_code) try: result = response.json() except ValueError: diff --git a/tests/test_arpege_ecp.py b/tests/test_arpege_ecp.py index dd896a70..f541d652 100644 --- a/tests/test_arpege_ecp.py +++ b/tests/test_arpege_ecp.py @@ -82,32 +82,32 @@ def test_check_status(mocked_get, connector): resp = connector.check_status() assert str(error.value) == 'Invalid credentials' -@mock.patch('passerelle.utils.Request.post') -def test_get_access_token(mocked_post, connector): - mocked_post.return_value = utils.FakedResponse(content=FAKE_LOGIN_OIDC_RESPONSE, status_code=200) - token = connector.get_access_token('nameid') - assert token == '0f86353f2d87b8b78aaaacc2ecc763e287ded44f773289a5e336546a251718b3' - mocked_post.return_value = utils.FakedResponse(content=FAKE_LOGIN_OIDC_RESPONSE, status_code=404) - with pytest.raises(APIError) as error: +def test_get_access_token(connector): + with utils.mock_url(response=FAKE_LOGIN_OIDC_RESPONSE): token = connector.get_access_token('nameid') + assert token == '0f86353f2d87b8b78aaaacc2ecc763e287ded44f773289a5e336546a251718b3' - assert str(error.value) == 'HTTP error: 404' + with utils.mock_url(response=FAKE_LOGIN_OIDC_RESPONSE, status_code=404): + with pytest.raises(APIError) as error: + token = connector.get_access_token('nameid') - mocked_post.return_value = utils.FakedResponse(content="content", status_code=200) - with pytest.raises(APIError) as error: - token = connector.get_access_token('nameid') - assert str(error.value) == 'No JSON content returned: \'content\'' + assert ' 404 ' in str(error.value) - mocked_post.return_value = utils.FakedResponse(content="content", status_code=200) - with pytest.raises(APIError) as error: - token = connector.get_access_token('nameid') - assert str(error.value) == 'No JSON content returned: \'content\'' + with utils.mock_url(response="content", status_code=200): + with pytest.raises(APIError) as error: + token = connector.get_access_token('nameid') + assert 'no JSON content' in str(error.value) - mocked_post.return_value = utils.FakedResponse(content='{"IsSuccess": false, "CodErreur": "Fail", "LibErreur": "Auth FAIL"}', - status_code=200) - with pytest.raises(APIError) as error: - token = connector.get_access_token('nameid') + with utils.mock_url(response="content", status_code=200): + with pytest.raises(APIError) as error: + token = connector.get_access_token('nameid') + assert 'no JSON content' in str(error.value) + + with utils.mock_url(response='{"IsSuccess": false, "CodErreur": "Fail", "LibErreur": "Auth FAIL"}', + status_code=200): + with pytest.raises(APIError) as error: + token = connector.get_access_token('nameid') assert str(error.value) == 'Auth FAIL (Fail)' @@ -149,18 +149,17 @@ def test_get_user_forms_failure(mocked_post, mocked_get, app, connector): assert result['err_desc'] == 'Failed to get demands (Fail)' -@mock.patch('passerelle.utils.Request.get') -@mock.patch('passerelle.utils.Request.post') -def test_get_user_forms_failure_404(mocked_post, mocked_get, app, connector): +def test_get_user_forms_failure_404(app, connector): endpoint = reverse('generic-endpoint', kwargs={ 'connector': 'arpege-ecp', 'slug': connector.slug, 'endpoint': 'api', 'rest': 'users/nameid/forms'}) - mocked_post.return_value = utils.FakedResponse(content=FAKE_LOGIN_OIDC_RESPONSE, status_code=200) - mocked_get.return_value = utils.FakedResponse(content=FAKE_USER_DEMANDS_RESPONSE, status_code=404) - resp = app.get(endpoint) - result = resp.json - assert result['err'] == 1 - assert result['err_desc'] == 'HTTP error: 404' + with utils.mock_url(url='/LoginParSubOIDC', response=FAKE_LOGIN_OIDC_RESPONSE): + with utils.mock_url(url='/DemandesUsager', status_code=404): + resp = app.get(endpoint) + result = resp.json + assert result['err'] == 1 + assert 'Arpege server is down' in result['err_desc'] + assert ' 404 ' in result['err_desc'] @mock.patch('passerelle.utils.Request.get') diff --git a/tests/utils.py b/tests/utils.py index 7a974f72..f9a419bc 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -30,14 +30,21 @@ class FakedResponse(mock.Mock): return json.loads(self.content) -def mock_url(url, response): - parsed = urlparse.urlparse(url) - if not isinstance(response ,str): +def mock_url(url=None, response='', status_code=200): + urlmatch_kwargs = {} + if url: + parsed = urlparse.urlparse(url) + if parsed.netloc: + urlmatch_kwargs['netloc'] = parsed.netloc + if parsed.path: + urlmatch_kwargs['path'] = parsed.path + + if not isinstance(response, str): response = json.dumps(response) - @httmock.urlmatch(netloc=parsed.netloc, path=parsed.path) + @httmock.urlmatch(**urlmatch_kwargs) def mocked(url, request): - return httmock.response(200, response, request=request) + return httmock.response(status_code, response, request=request) return httmock.HTTMock(mocked) -- 2.23.0.rc1