From afc5c00fc84c6cc45e7cea4279a9027f9dc8f84e Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 30 Jan 2020 10:26:30 +0100 Subject: [PATCH] base_adresse: add /addresses/ endpoint (#39387) Compatible with wcs API. --- .../migrations/0016_addresscachemodel.py | 25 ++++ passerelle/apps/base_adresse/models.py | 112 +++++++++++++----- tests/conftest.py | 5 +- tests/test_base_adresse.py | 89 +++++++++++++- 4 files changed, 197 insertions(+), 34 deletions(-) create mode 100644 passerelle/apps/base_adresse/migrations/0016_addresscachemodel.py diff --git a/passerelle/apps/base_adresse/migrations/0016_addresscachemodel.py b/passerelle/apps/base_adresse/migrations/0016_addresscachemodel.py new file mode 100644 index 00000000..6cae5a10 --- /dev/null +++ b/passerelle/apps/base_adresse/migrations/0016_addresscachemodel.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2020-01-30 11:34 +from __future__ import unicode_literals + +from django.db import migrations, models +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('base_adresse', '0015_auto_20191206_1244'), + ] + + operations = [ + migrations.CreateModel( + name='AddressCacheModel', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('api_id', models.CharField(max_length=30, unique=True)), + ('data', jsonfield.fields.JSONField(default=dict)), + ('timestamp', models.DateTimeField(auto_now=True)), + ], + ), + ] diff --git a/passerelle/apps/base_adresse/models.py b/passerelle/apps/base_adresse/models.py index 65d70158..15e0a6da 100644 --- a/passerelle/apps/base_adresse/models.py +++ b/passerelle/apps/base_adresse/models.py @@ -1,6 +1,8 @@ import bz2 +import datetime import unicodedata +from jsonfield import JSONField from requests import RequestException from django.db import connection, models @@ -49,21 +51,62 @@ class BaseAdresse(BaseResource): class Meta: verbose_name = _('Base Adresse Web Service') + @staticmethod + def format_address_data(data): + result = {} + result['lon'] = str(data['geometry']['coordinates'][0]) + result['lat'] = str(data['geometry']['coordinates'][1]) + result['address'] = {'country': 'France'} + for prop, value in data['properties'].items(): + if prop in ('city', 'postcode', 'citycode'): + result['address'][prop] = value + elif prop == 'housenumber': + result['address']['house_number'] = value + elif prop == 'label': + result['text'] = result['display_name'] = value + elif prop == 'name': + house_number = data['properties'].get('housenumber') + if house_number and value.startswith(house_number): + value = value[len(house_number):].strip() + result['address']['road'] = value + elif prop == 'id': + result['id'] = value + return result + @endpoint(pattern='(?P.+)?$', - description=_('Geocoding'), + description=_('Addresses list'), parameters={ - 'q': {'description': _('Address'), 'example_value': '169 rue du chateau, paris'} + 'id': {'description': _('Address identifier')}, + 'q': {'description': _('Address'), 'example_value': '169 rue du chateau, paris'}, + 'page_limit': {'description': _('Maximum number of results to return. Must be ' + 'lower than 20.')}, + 'zipcode': {'description': _('Zipcode'), 'example_value': '75014'}, + 'lat': {'description': _('Prioritize results according to coordinates. "lat" ' + 'parameter must be present.')}, + 'lon': {'description': _('Prioritize results according to coordinates. "lon" ' + 'parameter must be present.')}, }) - def search(self, request, q, zipcode='', lat=None, lon=None, **kwargs): - if kwargs.get('format', 'json') != 'json': - raise NotImplementedError() + def addresses(self, request, id=None, q=None, zipcode='', lat=None, lon=None, page_limit=5): + if id is not None: + try: + address = AddressCacheModel.objects.get(api_id=id) + except AddressCacheModel.DoesNotExist: + return {'err': _('Address ID not found')} + address.update_timestamp() + return {'data': [address.data]} if not q: - return [] + return {'data': []} + + try: + if int(page_limit) > 20: + page_limit = 20 + except ValueError: + page_limit = 5 scheme, netloc, path, params, query, fragment = urlparse.urlparse(self.service_url) path = urlparse.urljoin(path, 'search/') - query_args = {'q': q, 'limit': 1} + query_args = {'q': q, 'limit': page_limit} if zipcode: query_args['postcode'] = zipcode if lat and lon: @@ -78,14 +121,21 @@ class BaseAdresse(BaseResource): for feature in result_response.json().get('features'): if not feature['geometry']['type'] == 'Point': continue # skip unknown - result.append({ - 'lon': str(feature['geometry']['coordinates'][0]), - 'lat': str(feature['geometry']['coordinates'][1]), - 'display_name': feature['properties']['label'], - }) - break + data = self.format_address_data(feature) + result.append(data) + AddressCacheModel.objects.update_or_create(api_id=data['id'], data=data) - return result + return {'data': result} + + @endpoint(pattern='(?P.+)?$', description=_('Geocoding (Nominatim API)'), + parameters={ + 'q': {'description': _('Address'), 'example_value': '169 rue du chateau, paris'}, + }) + def search(self, request, q, zipcode='', lat=None, lon=None, **kwargs): + if kwargs.get('format', 'json') != 'json': + raise NotImplementedError() + result = self.addresses(request, q=q, zipcode=zipcode, lat=lat, lon=lon, page_limit=1) + return result['data'] @endpoint(description=_('Reverse geocoding'), parameters={ @@ -107,23 +157,7 @@ class BaseAdresse(BaseResource): for feature in result_response.json().get('features'): if not feature['geometry']['type'] == 'Point': continue # skip unknown - result = {} - result['lon'] = str(feature['geometry']['coordinates'][0]) - result['lat'] = str(feature['geometry']['coordinates'][1]) - result['address'] = {'country': 'France'} - for prop in feature['properties']: - if prop in ('city', 'postcode', 'citycode'): - result['address'][prop] = feature['properties'][prop] - elif prop == 'housenumber': - result['address']['house_number'] = feature['properties'][prop] - elif prop == 'label': - result['display_name'] = feature['properties'][prop] - elif prop == 'name': - house_number = feature['properties'].get('housenumber') - value = feature['properties'][prop] - if house_number and value.startswith(house_number): - value = value[len(house_number):].strip() - result['address']['road'] = value + result = self.format_address_data(feature) return result @endpoint(description=_('Streets from zipcode'), @@ -368,8 +402,15 @@ class BaseAdresse(BaseResource): code=data['code'], zipcode=zipcode, defaults=defaults) CityModel.objects.filter(last_update__lt=start_update).delete() + def clean_addresses_cache(self): + old_addresses = AddressCacheModel.objects.filter( + timestamp__lt=timezone.now() - datetime.timedelta(hours=1) + ) + old_addresses.delete() + def hourly(self): super(BaseAdresse, self).hourly() + self.clean_addresses_cache() # don't wait for daily job to grab data if self.get_zipcodes() and not self.get_streets_queryset().exists(): self.update_streets_data() @@ -489,3 +530,12 @@ class CityModel(UnaccentNameMixin, models.Model): def __str__(self): return '%s %s' % (self.zipcode, self.name) + + +class AddressCacheModel(models.Model): + api_id = models.CharField(max_length=30, unique=True) + data = JSONField() + timestamp = models.DateTimeField(auto_now=True) + + def update_timestamp(self): + self.save() diff --git a/tests/conftest.py b/tests/conftest.py index 2b8258b1..117b0f0c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,5 @@ import pytest -from httmock import urlmatch, HTTMock, response +from httmock import urlmatch, HTTMock, response, remember_called import django_webtest @@ -25,6 +25,7 @@ def app(request): @urlmatch(netloc='^api-adresse.data.gouv.fr$', path='^/search/$') +@remember_called def api_adresse_data_gouv_fr_search(url, request): return response(200, { "limit": 1, @@ -95,7 +96,7 @@ def api_adresse_data_gouv_fr_reverse(url, request): @pytest.yield_fixture def mock_api_adresse_data_gouv_fr_search(): with HTTMock(api_adresse_data_gouv_fr_search): - yield None + yield api_adresse_data_gouv_fr_search @pytest.yield_fixture diff --git a/tests/test_base_adresse.py b/tests/test_base_adresse.py index 522ece2b..bc43b8d9 100644 --- a/tests/test_base_adresse.py +++ b/tests/test_base_adresse.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +import datetime import os import pytest import mock @@ -13,7 +14,7 @@ from django.core.management.base import CommandError from django.utils.six.moves.urllib.parse import urljoin from passerelle.apps.base_adresse.models import (BaseAdresse, StreetModel, CityModel, - DepartmentModel, RegionModel) + DepartmentModel, RegionModel, AddressCacheModel) FAKED_CONTENT = json.dumps({ "limit": 1, @@ -604,3 +605,89 @@ def test_base_adresse_command_update_geo_no_connection(mocked_get, db, base_adre call_command('cron', 'daily') assert mocked_get.call_count == 3 assert not RegionModel.objects.exists() + + +@mock.patch('passerelle.utils.Request.get') +def test_base_adresse_addresses(mocked_get, app, base_adresse): + endpoint = utils.generic_endpoint_url('base-adresse', 'addresses', slug=base_adresse.slug) + assert endpoint == '/base-adresse/base-adresse/addresses' + mocked_get.return_value = utils.FakedResponse(content=FAKED_CONTENT, status_code=200) + resp = app.get(endpoint, params={'q': 'plop'}, status=200) + data = resp.json['data'][0] + assert data['lat'] == '47.474633' + assert data['lon'] == '-0.593775' + assert data['display_name'] == 'Rue Roger Halope 49000 Angers' + assert data['text'] == 'Rue Roger Halope 49000 Angers' + assert data['id'] == '49007_6950_be54bd' + assert data['address']['city'] == 'Angers' + assert data['address']['postcode'] == '49000' + assert data['address']['citycode'] == '49007' + assert data['address']['road'] == 'Rue Roger Halope' + + +@mock.patch('passerelle.utils.Request.get') +def test_base_adresse_addresses_qs_page_limit(mocked_get, app, base_adresse): + resp = app.get('/base-adresse/%s/addresses?q=plop&page_limit=1' % base_adresse.slug) + assert 'limit=1' in mocked_get.call_args[0][0] + + resp = app.get('/base-adresse/%s/addresses?q=plop&page_limit=100' % base_adresse.slug) + assert 'limit=20' in mocked_get.call_args[0][0] + + resp = app.get('/base-adresse/%s/addresses?q=plop&page_limit=blabla' % base_adresse.slug) + assert 'limit=5' in mocked_get.call_args[0][0] + + +def test_base_adresse_addresses_cache(app, base_adresse, mock_api_adresse_data_gouv_fr_search): + resp = app.get('/base-adresse/%s/addresses?q=plop' % base_adresse.slug) + assert mock_api_adresse_data_gouv_fr_search.call['count'] == 1 + + data = resp.json['data'][0] + assert data['text'] == 'Rue Roger Halope 49000 Angers' + + api_id = data['id'] + assert AddressCacheModel.objects.filter(api_id=api_id).exists() + assert AddressCacheModel.objects.count() == 1 + + resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, api_id)) + assert mock_api_adresse_data_gouv_fr_search.call['count'] == 1 # no new call + assert data['text'] == 'Rue Roger Halope 49000 Angers' + assert 'address' in data + + resp = app.get('/base-adresse/%s/addresses?q=plop' % base_adresse.slug) + assert AddressCacheModel.objects.count() == 1 # no new object has been created + + +def test_base_adresse_addresses_cache_err(app, base_adresse, + mock_api_adresse_data_gouv_fr_search): + resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, 'wrong_id')) + assert mock_api_adresse_data_gouv_fr_search.call['count'] == 0 + assert 'err' in resp.json + + +@pytest.mark.usefixtures('mock_update_api_geo', 'mock_update_streets') +def test_base_adresse_addresses_clean_cache(app, base_adresse, freezer, + mock_api_adresse_data_gouv_fr_search): + resp = app.get('/base-adresse/%s/addresses?q=plop' % base_adresse.slug) + assert AddressCacheModel.objects.count() == 1 + + freezer.move_to(datetime.timedelta(minutes=30)) + call_command('cron', 'hourly') + assert AddressCacheModel.objects.count() == 1 + + freezer.move_to(datetime.timedelta(minutes=30, seconds=1)) + call_command('cron', 'hourly') + assert AddressCacheModel.objects.count() == 0 + + resp = app.get('/base-adresse/%s/addresses?q=plop' % base_adresse.slug) + assert AddressCacheModel.objects.count() == 1 + + # asking for the address again resets the timestamp + freezer.move_to(datetime.timedelta(hours=1, seconds=1)) + resp = app.get('/base-adresse/%s/addresses?q=plop' % base_adresse.slug) + call_command('cron', 'hourly') + assert AddressCacheModel.objects.count() == 1 + + freezer.move_to(datetime.timedelta(hours=1, seconds=1)) + resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, '49007_6950_be54bd')) + call_command('cron', 'hourly') + assert AddressCacheModel.objects.count() == 1 -- 2.20.1