From 71524acf9e94764002c70dd3a41846e8d18b4900 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 17 Aug 2021 15:24:29 +0200 Subject: [PATCH] api_particulier: cache svai responses (#44684) Only for 200 and 4xx status codes. --- passerelle/apps/api_particulier/models.py | 49 ++++++++++++++--------- tests/conftest.py | 4 ++ tests/test_api_particulier.py | 41 +++++++++++++------ 3 files changed, 64 insertions(+), 30 deletions(-) diff --git a/passerelle/apps/api_particulier/models.py b/passerelle/apps/api_particulier/models.py index f45302cc..b56de1b0 100644 --- a/passerelle/apps/api_particulier/models.py +++ b/passerelle/apps/api_particulier/models.py @@ -29,6 +29,7 @@ except ImportError: from django.contrib.postgres.fields import ArrayField +from django.core.cache import cache from django.db import models from django.utils import six from django.utils.six.moves.urllib import parse @@ -115,16 +116,20 @@ class APIParticulier(BaseResource): # avoid logging http errors about non-transport failure message = data.get('message', '') if message in KNOWN_ERRORS.get(response.status_code, []): - raise APIError( - message, - data={ - 'code': data.get('error', 'api-error').replace('_', '-'), - 'status_code': response.status_code, - 'platform': self.platform, - 'content': data, - }, - ) - + error_data = { + 'code': data.get('error', 'api-error').replace('_', '-'), + 'status_code': response.status_code, + 'platform': self.platform, + 'content': data, + } + if response.status_code // 100 == 4: + # for 40x errors, allow caching of the response, as it should not change with future requests + return { + 'err': 1, + 'err_desc': message, + 'data': error_data, + } + raise APIError(message, data=error_data) raise APIError( u'API-particulier platform "%s" returned a non 200 status %s: %s' % (self.platform, response.status_code, data), @@ -268,14 +273,22 @@ class APIParticulier(BaseResource): reference_avis = reference_avis.strip()[:13] if len(numero_fiscal) < 13 or len(reference_avis) < 13: raise APIError('bad numero_fiscal or reference_avis, must be 13 chars long', status_code=400) - return self.get( - 'v2/avis-imposition', - params={ - 'numeroFiscal': numero_fiscal, - 'referenceAvis': reference_avis, - }, - user=user, - ) + cache_key = 'svai-%s-%s-%s' % (self.pk, numero_fiscal, reference_avis) + data = cache.get(cache_key) + if data is None: + data = self.get( + 'v2/avis-imposition', + params={ + 'numeroFiscal': numero_fiscal, + 'referenceAvis': reference_avis, + }, + user=user, + ) + # put in cache for two hours + cache.set(cache_key, data, 3600 * 2) + else: + self.logger.info('found response in cache, "%s"', data) + return data @endpoint( perm='can_access', diff --git a/tests/conftest.py b/tests/conftest.py index 5453a2fc..c307ec5c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -198,6 +198,10 @@ def clear_cache(): finally: InMemoryCache._cache = {} + from django.core.cache import cache + + cache.clear() + @pytest.fixture def simple_user(): diff --git a/tests/test_api_particulier.py b/tests/test_api_particulier.py index cfc175d0..5c0ec897 100644 --- a/tests/test_api_particulier.py +++ b/tests/test_api_particulier.py @@ -169,17 +169,19 @@ def resource(db): ) -def test_error(app, resource, mock_api_particulier): - vector = [ - ( - ['impots_svair', 'avis-imposition'], - { - 'numero_fiscal': '1234567890123', - 'reference_avis': '3210987654321', - }, - ), - (['caf_famille', 'situation-familiale'], {'code_postal': 12, 'numero_allocataire': '0000015'}), - ] +vector = [ + ( + ['impots_svair', 'avis-imposition'], + { + 'numero_fiscal': '1234567890123', + 'reference_avis': '3210987654321', + }, + ), + (['caf_famille', 'situation-familiale'], {'code_postal': 12, 'numero_allocataire': '0000015'}), +] + + +def test_error_500(app, resource, mock_api_particulier): with HTTMock(api_particulier_error_500): def do(endpoint, params): @@ -192,6 +194,9 @@ def test_error(app, resource, mock_api_particulier): for endpoints, params in vector: for endpoint in endpoints: do(endpoint, params) + + +def test_not_json(app, resource, mock_api_particulier): with HTTMock(api_particulier_error_not_json): def do(endpoint, params): @@ -204,6 +209,9 @@ def test_error(app, resource, mock_api_particulier): for endpoints, params in vector: for endpoint in endpoints: do(endpoint, params) + + +def test_not_found(app, resource, mock_api_particulier): with HTTMock(api_particulier_error_not_found): def do(endpoint, params): @@ -216,6 +224,9 @@ def test_error(app, resource, mock_api_particulier): for endpoints, params in vector: for endpoint in endpoints: do(endpoint, params) + + +def test_connection_error(app, resource, mock_api_particulier): with HTTMock(api_particulier_connection_error): def do(endpoint, params): @@ -231,6 +242,8 @@ def test_error(app, resource, mock_api_particulier): for endpoint in endpoints: do(endpoint, params) + +def test_numero_fiscal_too_short(app, resource, mock_api_particulier): resp = endpoint_get( '/api-particulier/test/avis-imposition', app, @@ -246,6 +259,9 @@ def test_error(app, resource, mock_api_particulier): assert resp.json['err'] == 1 assert resp.json['data'] is None assert 'must be 13 chars long' in resp.json['err_desc'] + + +def test_reference_avis_too_short(app, resource, mock_api_particulier): resp = endpoint_get( '/api-particulier/test/avis-imposition', app, @@ -370,10 +386,11 @@ def test_api_particulier_dont_log_not_found(app, resource, mock, should_log): }, ) logs = ResourceLog.objects.all() - assert logs.count() == 3 if should_log: + assert logs.count() == 3 assert logs.filter(levelno=logging.ERROR).count() == 1 else: + assert logs.count() == 2 assert not logs.filter(levelno=logging.ERROR).exists() -- 2.32.0.rc0