From 92987b0c96e2187dbf3174901f9a65b3b46390b5 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 connector's requests (#7898) --- passerelle/apps/clicrdv/models.py | 57 ++++++++---------- tests/test_clicrdv.py | 118 ++++++++++++++++++++++++++++---------- 2 files changed, 110 insertions(+), 65 deletions(-) diff --git a/passerelle/apps/clicrdv/models.py b/passerelle/apps/clicrdv/models.py index cd06449..44fdc7c 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 @@ -50,25 +51,19 @@ 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 - - def get_json(self, uri): - req = self.get_request(uri) - return json.load(urllib2.urlopen(req)) + basic_auth = requests.auth.HTTPBasicAuth(self.username, self.password) + return self.requests.request(method, url, auth=basic_auth, **kwargs) @endpoint(name='interventionsets', serializer_type='json-api') def get_interventionsets(self, request, **kwargs): - records = self.get_json('interventionsets').get('records') + response = self.request('interventionsets') + records = response.json().get('records') records.sort(lambda x,y: cmp(x['sort'], y['sort'])) ret = [] for record in records: @@ -79,7 +74,8 @@ class ClicRdv(BaseResource): @endpoint(name='interventionsets', pattern='(?P\d+)/', serializer_type='json-api') def get_interventions(self, request, set, **kwargs): ret = [] - records = self.get_json('interventions?interventionset_id=%s' % set).get('records') + response = self.request('interventions?interventionset_id=%s' % set) + records = response.json().get('records') records.sort(lambda x,y: cmp(x['sort'], y['sort'])) for record in records: if record.get('publicname'): @@ -98,7 +94,7 @@ class ClicRdv(BaseResource): request_uri = request_uri + '&start=%s' % urllib2.quote(date_start) if date_end: request_uri = request_uri + '&end=%s' % urllib2.quote(date_end) - for timeslot in self.get_json(request_uri).get('availabletimeslots'): + for timeslot in self.request(request_uri).json().get('availabletimeslots'): timeslots.append(timeslot.get('start')) timeslots.sort() return timeslots @@ -139,20 +135,16 @@ 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) + r.raise_for_status() + 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} + error = json.loads(str(e)).get('error') + return {'success': False, 'error': error} return {'success': True} @@ -192,23 +184,20 @@ 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'}) + r.raise_for_status() + except requests.exceptions.HTTPError as e: try: - error = json.load(e.fp)[0].get('error') + error = json.loads(str(e))[0].get('error') except: error = 'Unknown error (Passerelle)' - return { - 'success': False, - 'error': error, - } + return {'success': False, 'error': 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..0f916cb 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,19 @@ def connector(db): password='test') -@mock.patch('urllib2.urlopen') -def test_urlopen_call(urlopen, app, connector): - urlopen.return_value = StringIO('"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') +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_request_call(mocked_request, app, connector): + mocked_request.json.return_value = "foo" + connector.request('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 = { +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_interventionsets(mocked_request, app, connector): + response = mock.Mock() + response.json.return_value = { "totalRecords": 2, "records": [ { @@ -42,14 +45,16 @@ def test_interventionsets(mocked_get, app, connector): "group_id": 5242, }, ]} + mocked_request.return_value = response resp = app.get('/clicrdv/test/interventionsets/') assert len(resp.json.get('data')) == 2 assert resp.json.get('data')[0]['text'] == 'Une Demande de Passeport' -@mock.patch('clicrdv.models.ClicRdv.get_json') -def test_interventionsets_details(mocked_get, app, connector): - mocked_get.return_value = { +@mock.patch('passerelle.utils.LoggedRequest.request') +def test_interventionsets_details(mocked_request, app, connector): + response = mock.Mock() + response.json.return_value = { "totalRecords": 2, "records": [ { @@ -64,7 +69,7 @@ def test_interventionsets_details(mocked_get, app, connector): }, { "sort": 2, - "publicname": "pour deuxs personnes", + "publicname": "pour deux personnes", "description": None, "name": "2 personnes", "interventionset_id": 7032, @@ -73,19 +78,21 @@ def test_interventionsets_details(mocked_get, app, connector): "abbr": "2 demandes" }, ]} - + mocked_request.return_value = response resp = app.get('/clicrdv/test/interventionsets/7032/') 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 +108,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 +150,47 @@ 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) + 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) + resp = app.get('/clicrdv/test/63258/cancel?apikey=apiuser').json + assert mocked_request.call_count == 1 + assert resp.get('err') == 0 + assert resp['data'] + assert resp['data']['error'] == 1 + + +@mock.patch('passerelle.utils.LoggedRequest.request', + side_effect=HTTPError('[{"error": "creation failed"}]')) +def test_failed_appointment_creation(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) + data = {'fields': {'clicrdv_date_raw': '2017-01-01' , 'clicrdv_time_raw': '12:00:00', + 'firstname': 'Foo', 'lastname': 'Bar', 'email': 'foobar@example.com'}} + resp = app.post_json('/clicrdv/test/interventions/63258/create?apikey=apiuser', data).json + assert resp['data'] + assert not resp['data']['success'] + assert resp['data']['error'] == 'creation failed' -- 2.11.0