From b07db16cd1dc1b4a134127169acce15b8b875fe7 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 9 Jan 2018 13:24:54 +0000 Subject: [PATCH] Get rate limit from service.rate_limit column (not config) PR #1550 added the rate_limit column to the Service table. This PR removes the rate limits from the config and uses rate_limit from the Service model instead. Rate limits are still separated into 'team', 'normal' and 'test', but these values are the same for a service. Pivotal story https://www.pivotaltracker.com/story/show/153992529 --- app/config.py | 46 ---------------------- app/models.py | 1 + app/notifications/validators.py | 4 +- tests/app/notifications/test_validators.py | 20 ++++------ tests/app/service/test_rest.py | 1 + 5 files changed, 11 insertions(+), 61 deletions(-) diff --git a/app/config.py b/app/config.py index 3368ca68c..42ba4177e 100644 --- a/app/config.py +++ b/app/config.py @@ -7,7 +7,6 @@ from kombu import Exchange, Queue from app.models import ( EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, - KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST ) if os.environ.get('VCAP_SERVICES'): @@ -293,21 +292,6 @@ class Config(object): 'notification': '{}-dvla-letter-api-files'.format(os.getenv('NOTIFY_ENVIRONMENT')) } - API_KEY_LIMITS = { - KEY_TYPE_TEAM: { - "limit": 3000, - "interval": 60 - }, - KEY_TYPE_NORMAL: { - "limit": 3000, - "interval": 60 - }, - KEY_TYPE_TEST: { - "limit": 3000, - "interval": 60 - } - } - FREE_SMS_TIER_FRAGMENT_COUNT = 250000 SMS_INBOUND_WHITELIST = json.loads(os.environ.get('SMS_INBOUND_WHITELIST', '[]')) @@ -375,21 +359,6 @@ class Test(Config): API_RATE_LIMIT_ENABLED = True API_HOST_NAME = "http://localhost:6011" - API_KEY_LIMITS = { - KEY_TYPE_TEAM: { - "limit": 1, - "interval": 2 - }, - KEY_TYPE_NORMAL: { - "limit": 10, - "interval": 20 - }, - KEY_TYPE_TEST: { - "limit": 100, - "interval": 200 - } - } - SMS_INBOUND_WHITELIST = ['203.0.113.195'] FIRETEXT_INBOUND_SMS_AUTH = ['testkey'] MMG_INBOUND_SMS_AUTH = ['testkey'] @@ -420,21 +389,6 @@ class Staging(Config): CHECK_PROXY_HEADER = True REDIS_ENABLED = True - API_KEY_LIMITS = { - KEY_TYPE_TEAM: { - "limit": 24000, - "interval": 60 - }, - KEY_TYPE_NORMAL: { - "limit": 24000, - "interval": 60 - }, - KEY_TYPE_TEST: { - "limit": 24000, - "interval": 60 - } - } - class Live(Config): NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk' diff --git a/app/models.py b/app/models.py index 1a54b19cb..3b2bcbb96 100644 --- a/app/models.py +++ b/app/models.py @@ -250,6 +250,7 @@ class Service(db.Model, Versioned): nullable=True, ) crown = db.Column(db.Boolean, index=False, nullable=False, default=True) + rate_limit = db.Column(db.Integer, index=False, nullable=False, default=3000) association_proxy('permissions', 'service_permission_types') diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 5daaaaa08..921ee350b 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -26,8 +26,8 @@ from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id def check_service_over_api_rate_limit(service, api_key): if current_app.config['API_RATE_LIMIT_ENABLED'] and current_app.config['REDIS_ENABLED']: cache_key = rate_limit_cache_key(service.id, api_key.key_type) - rate_limit = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit'] - interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval'] + rate_limit = service.rate_limit + interval = 60 if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval): current_app.logger.error("service {} has been rate limited for throughput".format(service.id)) raise RateLimitError(rate_limit, interval, api_key.key_type) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 36286ed70..4e426c152 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -256,13 +256,11 @@ def test_check_sms_content_char_count_fails(char_count, notify_api): assert e.value.fields == [] -@pytest.mark.parametrize('key_type, limit, interval', [('team', 1, 2), ('live', 10, 20), ('test', 100, 200)]) +@pytest.mark.parametrize('key_type', ['team', 'live', 'test']) def test_that_when_exceed_rate_limit_request_fails( notify_db, notify_db_session, key_type, - limit, - interval, mocker): with freeze_time("2016-01-01 12:00:00.000000"): @@ -281,36 +279,32 @@ def test_that_when_exceed_rate_limit_request_fails( assert app.redis_store.exceeded_rate_limit.called_with( "{}-{}".format(str(service.id), api_key.key_type), - limit, - interval + service.rate_limit, + 60 ) assert e.value.status_code == 429 assert e.value.message == 'Exceeded rate limit for key type {} of {} requests per {} seconds'.format( - key_type.upper(), limit, interval + key_type.upper(), service.rate_limit, 60 ) assert e.value.fields == [] -@pytest.mark.parametrize('key_type, limit, interval', [('team', 1, 2), ('normal', 10, 20), ('test', 100, 200)]) def test_that_when_not_exceeded_rate_limit_request_succeeds( notify_db, notify_db_session, - key_type, - limit, - interval, mocker): with freeze_time("2016-01-01 12:00:00.000000"): mocker.patch('app.redis_store.exceeded_rate_limit', return_value=False) mocker.patch('app.notifications.validators.services_dao') service = create_service(notify_db, notify_db_session, restricted=True) - api_key = sample_api_key(notify_db, notify_db_session, service=service, key_type=key_type) + api_key = sample_api_key(notify_db, notify_db_session, service=service, key_type='normal') check_service_over_api_rate_limit(service, api_key) assert app.redis_store.exceeded_rate_limit.called_with( "{}-{}".format(str(service.id), api_key.key_type), - limit, - interval + 3000, + 60 ) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 707bffc40..d498a07b6 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -233,6 +233,7 @@ def test_create_service(client, sample_user): assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] + assert json_resp['data']['rate_limit'] == 3000 service_db = Service.query.get(json_resp['data']['id']) assert service_db.name == 'created service'