From f84216a5acb3ac540480eba55c5838ecda0ec02d Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 12 Dec 2022 10:10:05 +0100 Subject: [PATCH] base_adresse: set result id to lookup id (#72263) base_adresse data source ids are not canonical, many can match the same adress. But clients of the API does not handle this well, so it's better to always return the same id event if we know it has changed. --- passerelle/apps/base_adresse/models.py | 17 ++++++++++++++++- tests/test_base_adresse.py | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/passerelle/apps/base_adresse/models.py b/passerelle/apps/base_adresse/models.py index 9a3d5308..42cd34d2 100644 --- a/passerelle/apps/base_adresse/models.py +++ b/passerelle/apps/base_adresse/models.py @@ -192,12 +192,27 @@ class BaseAdresse(AddressResource): else: self.sectorize(address.data) # if sectors have been updated since caching address.update_timestamp() - return {'data': [address.data]} + result = address.data + # Keep the original id if the client revalidate the + # response before accepting it (like w.c.s. does). + # id can change if street name changes (case change for + # example). + # See https://dev.entrouvert.org/issues/72263 + result = result.copy() + result['id'] = id + return {'data': [result]} # Use search with label as q and lat/lon as geographic hint if q and lat and lon: results = self.addresses(request, q=q, lat=lat, lon=lon, citycode=citycode)['data'] for result in results: # match by id if possible if result['ban_id'] == ban_id: + # Keep the original id if the client revalidate the + # response before accepting it (like w.c.s. does). + # id can change if street name changes (case change for + # example). + # See https://dev.entrouvert.org/issues/72263 + result = result.copy() + result['id'] = id return {'data': [result]} self.logger.error('get_by_id: id %s was not found', id) return {'err': _('Address ID not found')} diff --git a/tests/test_base_adresse.py b/tests/test_base_adresse.py index e5dfc596..91c313a1 100644 --- a/tests/test_base_adresse.py +++ b/tests/test_base_adresse.py @@ -1053,6 +1053,7 @@ def test_base_adresse_reverse_cache( first_timestamp = AddressCacheModel.objects.get().timestamp resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, api_id)) + data = resp.json['data'][0] assert mock_api_adresse_data_gouv_fr_search.call['count'] == 0 assert data['text'] == 'Rue Roger Halope 49000 Angers' assert 'address' in data @@ -1062,3 +1063,18 @@ def test_base_adresse_reverse_cache( resp = app.get('/base-adresse/%s/reverse?lon=-0.593775&lat=47.474633' % base_adresse.slug) assert mock_api_adresse_data_gouv_fr_reverse.call['count'] == 2 assert AddressCacheModel.objects.get().timestamp > first_timestamp + + # check lookup id is kept + resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, api_id.lower())) + data = resp.json['data'][0] + assert mock_api_adresse_data_gouv_fr_search.call['count'] == 0 + assert data['id'] != api_id + assert data['id'] == api_id.lower() + + # without cache + assert AddressCacheModel.objects.all().delete() + resp = app.get('/base-adresse/%s/addresses?id=%s' % (base_adresse.slug, api_id.lower())) + data = resp.json['data'][0] + assert mock_api_adresse_data_gouv_fr_search.call['count'] == 1 + assert data['id'] != api_id + assert data['id'] == api_id.lower()