From b962f7f1c18d6cae047cffec1c05fb0473159a6d Mon Sep 17 00:00:00 2001 From: Serghei Mihai Date: Thu, 29 Dec 2016 14:13:22 +0100 Subject: [PATCH] clicrdv: replace urllib2 by connectors requests (#7898) --- passerelle/apps/clicrdv/models.py | 47 ++++++++------------ tests/test_clicrdv.py | 91 ++++++++++++++++++++++++++++----------- 2 files changed, 83 insertions(+), 55 deletions(-) diff --git a/passerelle/apps/clicrdv/models.py b/passerelle/apps/clicrdv/models.py index cd06449..1ad40b6 100644 --- a/passerelle/apps/clicrdv/models.py +++ b/passerelle/apps/clicrdv/models.py @@ -8,6 +8,7 @@ import base64 import datetime import json import urllib2 +import requests from django.conf import settings from django.core.urlresolvers import reverse @@ -18,6 +19,7 @@ from django.utils.translation import ugettext_lazy as _ from passerelle.base.models import BaseResource from passerelle.utils.api import endpoint +from passerelle.utils.jsonresponse import APIError CLICRDV_SERVERS = ( ('www.clicrdv.com', 'Production (www.clicrdv.com)'), @@ -50,21 +52,18 @@ class ClicRdv(BaseResource): def get_icon_class(cls): return 'clock' - def get_request(self, uri): + def request(self, uri, method='get', **kwargs): url = 'https://%s/api/v1/groups/%s/%s' % (self.server, self.group_id, uri) if '?' in url: url = url + '&apikey=%s&format=json' % self.apikey else: url = url + '?apikey=%s&format=json' % self.apikey - req = urllib2.Request(url) - authheader = 'Basic ' + \ - base64.encodestring('%s:%s' % (self.username, self.password))[:-1] - req.add_header('Authorization', authheader) - return req + basic_auth = requests.auth.HTTPBasicAuth(self.username, self.password) + return self.requests.request(method, url, auth=basic_auth, **kwargs) def get_json(self, uri): - req = self.get_request(uri) - return json.load(urllib2.urlopen(req)) + response = self.request(uri) + return response.json() @endpoint(name='interventionsets', serializer_type='json-api') def get_interventionsets(self, request, **kwargs): @@ -139,20 +138,14 @@ class ClicRdv(BaseResource): times.sort(lambda x,y: cmp(x.get('id'), y.get('id'))) return times - def cancel(self, id, **kwargs): - appointment_id = int(id) - req = self.get_request('appointments/%s' % appointment_id) - req.get_method = (lambda: 'DELETE') + def cancel(self, appointment_id, **kwargs): try: - fd = urllib2.urlopen(req) - none = fd.read() - except urllib2.HTTPError as e: + r = self.request('delete', 'appointments/%s' % appointment_id) + except requests.exceptions.HTTPError as e: # clicrdv will return a "Bad Request" (HTTP 400) response # when it's not possible to remove an appointment # (for example because it's too late) - response = e.read() - response = json.loads(response) - return {'success': False, 'error': response} + raise APIError('ClicRDV error: %s' % e) return {'success': True} @@ -192,23 +185,19 @@ class ClicRdv(BaseResource): for fieldname in (fields.keys() + extra.keys() + data.keys()): if fieldname.startswith('clicrdv_fiche_'): appointment['fiche'][fieldname[14:]] = get_data(fieldname) or '' - req = self.get_request('appointments') - req.add_data(json.dumps({'appointment': appointment})) - req.add_header('Content-Type', 'application/json') + data = json.dumps({'appointment': appointment}) try: - fd = urllib2.urlopen(req) - except urllib2.HTTPError, e: + r = self.request('appointments', method='post', data=data, + headers={'Content-Type', 'application/json'}) + except requests.exceptions.HTTPError as e: try: - error = json.load(e.fp)[0].get('error') + error = json.loads(str(e)).get('error') except: error = 'Unknown error (Passerelle)' - return { - 'success': False, - 'error': error, - } + raise APIError('ClicRDV error: %s' % error) else: success = True - response = json.load(fd) + response = r.json() appointment_id = response.get('records')[0].get('id') return { 'success': True, diff --git a/tests/test_clicrdv.py b/tests/test_clicrdv.py index ff27055..c03274b 100644 --- a/tests/test_clicrdv.py +++ b/tests/test_clicrdv.py @@ -1,7 +1,9 @@ import mock import pytest import urlparse -from StringIO import StringIO +from requests.exceptions import HTTPError + +from django.contrib.contenttypes.models import ContentType from passerelle.base.models import ApiUser, AccessRight from clicrdv.models import ClicRdv @@ -13,18 +15,18 @@ def connector(db): password='test') -@mock.patch('urllib2.urlopen') -def test_urlopen_call(urlopen, app, connector): - urlopen.return_value = StringIO('"foo"') +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_urlopen_call(mocked_request, app, connector): + mocked_request.json.return_value = "foo" rjson = connector.get_json('bar') - assert urlopen.call_count == 1 - req = urlopen.call_args[0][0] - assert req.get_full_url().startswith('https://sandbox.clicrdv.com/api/v1/groups/5242/bar') + assert mocked_request.call_count == 1 + req = mocked_request.call_args[0][1] + assert req.startswith('https://sandbox.clicrdv.com/api/v1/groups/5242/bar') @mock.patch('clicrdv.models.ClicRdv.get_json') -def test_interventionsets(mocked_get, app, connector): - mocked_get.return_value = { +def test_interventionsets(mocked_request_json, app, connector): + mocked_request_json.return_value = { "totalRecords": 2, "records": [ { @@ -48,8 +50,8 @@ def test_interventionsets(mocked_get, app, connector): @mock.patch('clicrdv.models.ClicRdv.get_json') -def test_interventionsets_details(mocked_get, app, connector): - mocked_get.return_value = { +def test_interventionsets_details(mocked_request_json, app, connector): + mocked_request_json.return_value = { "totalRecords": 2, "records": [ { @@ -78,14 +80,16 @@ def test_interventionsets_details(mocked_get, app, connector): assert len(resp.json.get('data')) == 2 assert resp.json.get('data')[0]['text'] == 'pour une personne' -@mock.patch('urllib2.urlopen') -def test_interventions_get_datetimes(urlopen, app, connector): - urlopen.return_value = StringIO('{"availabletimeslots": []}') +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_interventions_get_datetimes(mocked_request, app, connector): + mock_response = mock.Mock() + mock_response.json.return_value = {"availabletimeslots": []} + mocked_request.return_value = mock_response resp = app.get('/clicrdv/test/interventions/63258/dates/') assert resp.json.get('data') == [] assert resp.json.get('err') == 0 - assert urlopen.call_count == 1 - url = urlopen.call_args[0][0].get_full_url() + assert mocked_request.call_count == 1 + url = mocked_request.call_args[0][1] # https://sandbox.clicrdv.com/api/v1/groups/5242/availabletimeslots? # intervention_ids[]=63258&start=2016-09-21&end=2017-09-22&apikey=test&format=json scheme, netloc, path, params, query, fragment = urlparse.urlparse(url) @@ -101,35 +105,40 @@ def test_interventions_get_datetimes(urlopen, app, connector): assert query['apikey'] == ['test'] assert query['format'] == ['json'] - urlopen.return_value = StringIO('''{"availabletimeslots": [ + mock_response.json.return_value = {"availabletimeslots": [ { "start": "2016-09-21 12:34:56" }, { "start": "2016-09-22 11:22:33" } - ]}''') + ]} + mocked_request.return_value = mock_response resp = app.get('/clicrdv/test/interventions/63258/dates/').json - assert urlopen.call_count == 2 + assert mocked_request.call_count == 2 assert resp.get('err') == 0 assert len(resp.get('data')) == 2 assert resp['data'][0] == {'id': '2016-09-21', 'text': '21 September 2016'} assert resp['data'][1] == {'id': '2016-09-22', 'text': '22 September 2016'} - urlopen.return_value = StringIO('''{"availabletimeslots": [ + mock_response.json.return_value = {"availabletimeslots": [ { "start": "2016-09-22 11:22:33" }, { "start": "2016-09-21 12:34:56" } - ]}''') # will be sorted + ]} # will be sorted + + mocked_request.return_value = mock_response resp = app.get('/clicrdv/test/interventions/63258/datetimes/').json - assert urlopen.call_count == 3 + assert mocked_request.call_count == 3 assert resp.get('err') == 0 assert len(resp.get('data')) == 2 assert resp['data'][0] == {'id': '2016-09-21-12:34:56', 'text': '21 September 2016 12:34'} assert resp['data'][1] == {'id': '2016-09-22-11:22:33', 'text': '22 September 2016 11:22'} - urlopen.return_value = StringIO('''{"availabletimeslots": [ + mock_response.json.return_value = {"availabletimeslots": [ { "start": "2016-09-21 12:34:56" }, { "start": "2016-09-21 11:22:33" } - ]}''') # will be sorted + ]} # will be sorted + + mocked_request.return_value = mock_response resp = app.get('/clicrdv/test/interventions/63258/2016-09-21/times').json - assert urlopen.call_count == 4 - url = urlopen.call_args[0][0].get_full_url() + assert mocked_request.call_count == 4 + url = mocked_request.call_args[0][1] scheme, netloc, path, params, query, fragment = urlparse.urlparse(url) query = urlparse.parse_qs(query, keep_blank_values=True) assert query['start'] == ['2016-09-21 00:00:00'] @@ -138,3 +147,33 @@ def test_interventions_get_datetimes(urlopen, app, connector): assert len(resp.get('data')) == 2 assert resp['data'][0] == {'id': '11:22:33', 'text': '11:22'} assert resp['data'][1] == {'id': '12:34:56', 'text': '12:34'} + +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_cancel_appointment(mocked_request, app, connector): + obj_type = ContentType.objects.get_for_model(ClicRdv) + apiuser = ApiUser.objects.create(username='apiuser', keytype='API', + key='apiuser') + AccessRight.objects.create(codename='can_manage_appointment', + resource_type=obj_type, resource_pk=connector.pk, + apiuser=apiuser) + mock_response = mock.Mock() + mock_response.json.return_value = {'ok'} + resp = app.get('/clicrdv/test/63258/cancel?apikey=apiuser').json + assert mocked_request.call_count == 1 + assert resp['data']['success'] + +@mock.patch('passerelle.utils.LoggedRequest.request', + side_effect=HTTPError('{"error": 1}')) +def test_failed_cancel_appointment(mocked_request, app, connector): + obj_type = ContentType.objects.get_for_model(ClicRdv) + apiuser = ApiUser.objects.create(username='apiuser', keytype='API', + key='apiuser') + AccessRight.objects.create(codename='can_manage_appointment', + resource_type=obj_type, resource_pk=connector.pk, + apiuser=apiuser) + mock_response = mock.Mock() + mock_response.json.return_value = {'ok'} + resp = app.get('/clicrdv/test/63258/cancel?apikey=apiuser').json + assert mocked_request.call_count == 1 + assert resp.get('err') == 1 + assert not resp['data'] -- 2.11.0