This commit is contained in:
Kenneth Kehl
2023-07-12 14:52:40 -07:00
parent 599f1fde2c
commit cbed8b8104
8 changed files with 7 additions and 203 deletions

View File

@@ -30,11 +30,9 @@ from app.models import (
SMS_TYPE,
)
from app.notifications.process_notifications import persist_notification
from app.notifications.validators import check_service_over_daily_message_limit
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.service.utils import service_allowed_to_send_to
from app.utils import DATETIME_FORMAT
from app.v2.errors import TooManyRequestsError
@notify_celery.task(name="process-job")
@@ -59,9 +57,6 @@ def process_job(job_id, sender_id=None):
"Job {} has been cancelled, service {} is inactive".format(job_id, service.id))
return
if __sending_limits_for_job_exceeded(service, job, job_id):
return
recipient_csv, template, sender_id = get_recipient_csv_and_template_and_sender_id(job)
current_app.logger.info("Starting job {} processing {} notifications".format(job_id, job.notification_count))
@@ -134,24 +129,6 @@ def process_row(row, template, job, service, sender_id=None):
return notification_id
def __sending_limits_for_job_exceeded(service, job, job_id):
try:
total_sent = check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service)
if total_sent + job.notification_count > service.message_limit:
raise TooManyRequestsError(service.message_limit)
else:
return False
except TooManyRequestsError:
job.job_status = 'sending limits exceeded'
job.processing_finished = datetime.utcnow()
dao_update_job(job)
current_app.logger.info(
"Job {} size {} error. Sending limits {} exceeded".format(
job_id, job.notification_count, service.message_limit)
)
return True
@notify_celery.task(bind=True, name="save-sms", max_retries=5, default_retry_delay=300)
def save_sms(self,
service_id,

View File

@@ -138,21 +138,7 @@ def persist_notification(
dao_create_notification(notification)
if key_type != KEY_TYPE_TEST and current_app.config['REDIS_ENABLED']:
current_app.logger.info('Redis enabled, querying cache key for service id: {}'.format(service.id))
cache_key = redis.daily_limit_cache_key(service.id)
total_key = redis.daily_total_cache_key()
current_app.logger.info('Redis daily limit cache key: {}'.format(cache_key))
if redis_store.get(cache_key) is None:
current_app.logger.info('Redis daily limit cache key does not exist')
# if cache does not exist set the cache to 1 with an expiry of 24 hours,
# The cache should be set by the time we create the notification
# but in case it is this will make sure the expiry is set to 24 hours,
# where if we let the incr method create the cache it will be set a ttl.
redis_store.set(cache_key, 1, ex=86400)
current_app.logger.info('Set redis daily limit cache key to 1')
else:
current_app.logger.info('Redis daily limit cache key does exist')
redis_store.incr(cache_key)
current_app.logger.info('Redis daily limit cache key has been incremented')
if redis_store.get(total_key) is None:
current_app.logger.info('Redis daily total cache key does not exist')
redis_store.set(total_key, 1, ex=86400)

View File

@@ -2,7 +2,6 @@ from flask import current_app
from gds_metrics.metrics import Histogram
from notifications_utils import SMS_CHAR_COUNT_LIMIT
from notifications_utils.clients.redis import (
daily_limit_cache_key,
daily_total_cache_key,
rate_limit_cache_key,
)
@@ -30,12 +29,7 @@ from app.notifications.process_notifications import (
from app.serialised_models import SerialisedTemplate
from app.service.utils import service_allowed_to_send_to
from app.utils import get_public_notify_type_text
from app.v2.errors import (
BadRequestError,
RateLimitError,
TooManyRequestsError,
TotalRequestsError,
)
from app.v2.errors import BadRequestError, RateLimitError, TotalRequestsError
REDIS_EXCEEDED_RATE_LIMIT_DURATION_SECONDS = Histogram(
'redis_exceeded_rate_limit_duration_seconds',
@@ -54,26 +48,6 @@ def check_service_over_api_rate_limit(service, api_key):
raise RateLimitError(rate_limit, interval, api_key.key_type)
def check_service_over_daily_message_limit(key_type, service):
if key_type == KEY_TYPE_TEST or not current_app.config['REDIS_ENABLED']:
return 0
cache_key = daily_limit_cache_key(service.id)
service_stats = redis_store.get(cache_key)
if service_stats is None:
# first message of the day, set the cache to 0 and the expiry to 24 hours
service_stats = 0
redis_store.set(cache_key, service_stats, ex=86400)
return service_stats
if int(service_stats) >= service.message_limit:
current_app.logger.info(
"service {} has been rate limited for daily use sent {} limit {}".format(
service.id, int(service_stats), service.message_limit)
)
raise TooManyRequestsError(service.message_limit)
return int(service_stats)
def check_application_over_daily_message_total(key_type, service):
if key_type == KEY_TYPE_TEST or not current_app.config['REDIS_ENABLED']:
return 0
@@ -98,7 +72,6 @@ def check_application_over_daily_message_total(key_type, service):
def check_rate_limiting(service, api_key):
check_service_over_api_rate_limit(service, api_key)
check_application_over_daily_message_total(api_key.key_type, service)
check_service_over_daily_message_limit(api_key.key_type, service)
def check_template_is_for_notification_type(notification_type, template_type):

View File

@@ -12,7 +12,6 @@ from app.notifications.process_notifications import (
send_notification_to_queue,
)
from app.notifications.validators import (
check_service_over_daily_message_limit,
validate_and_format_recipient,
validate_template,
)
@@ -45,8 +44,6 @@ def send_one_off_notification(service_id, post_data):
validate_template(template.id, personalisation, service, template.template_type)
check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service)
validate_and_format_recipient(
send_to=post_data['to'],
key_type=KEY_TYPE_NORMAL,

View File

@@ -45,7 +45,6 @@ from app.models import (
)
from app.serialised_models import SerialisedService, SerialisedTemplate
from app.utils import DATETIME_FORMAT
from app.v2.errors import TooManyRequestsError
from tests.app import load_example_csv
from tests.app.db import (
create_api_key,
@@ -150,44 +149,6 @@ def test_should_not_process_job_if_already_pending(sample_template, mocker):
assert tasks.process_row.called is False
def test_should_not_process_if_send_limit_is_exceeded(
notify_api, notify_db_session, mocker
):
service = create_service(message_limit=9)
template = create_template(service=service)
job = create_job(template=template, notification_count=10, original_file_name='multiple_sms.csv')
mocker.patch('app.celery.tasks.s3.get_job_and_metadata_from_s3',
return_value=(load_example_csv('multiple_sms'), {'sender_id': None}))
mocker.patch('app.celery.tasks.process_row')
mocker.patch('app.celery.tasks.check_service_over_daily_message_limit',
side_effect=TooManyRequestsError("exceeded limit"))
process_job(job.id)
job = jobs_dao.dao_get_job_by_id(job.id)
assert job.job_status == 'sending limits exceeded'
assert s3.get_job_and_metadata_from_s3.called is False
assert tasks.process_row.called is False
def test_should_not_process_if_send_limit_is_exceeded_by_job_notification_count(
notify_api, notify_db_session, mocker
):
service = create_service(message_limit=9)
template = create_template(service=service)
job = create_job(template=template, notification_count=10, original_file_name='multiple_sms.csv')
mock_s3 = mocker.patch('app.celery.tasks.s3.get_job_and_metadata_from_s3',
return_value=(load_example_csv('multiple_sms'), {'sender_id': None}))
mock_process_row = mocker.patch('app.celery.tasks.process_row')
mocker.patch('app.celery.tasks.check_service_over_daily_message_limit',
return_value=0)
process_job(job.id)
job = jobs_dao.dao_get_job_by_id(job.id)
assert job.job_status == 'sending limits exceeded'
mock_s3.assert_not_called()
mock_process_row.assert_not_called()
def test_should_process_job_if_send_limits_are_not_exceeded(
notify_api, notify_db_session, mocker
):
@@ -200,7 +161,6 @@ def test_should_process_job_if_send_limits_are_not_exceeded(
mocker.patch('app.celery.tasks.save_email.apply_async')
mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
mocker.patch('app.celery.tasks.create_uuid', return_value="uuid")
mocker.patch('app.celery.tasks.check_service_over_daily_message_limit', return_value=0)
process_job(job.id)
s3.get_job_and_metadata_from_s3.assert_called_once_with(

View File

@@ -218,9 +218,9 @@ def test_persist_notification_increments_cache_for_trial_or_live_service(
key_type=api_key.key_type,
reference="ref2")
assert mock_incr.call_count == 2
assert mock_incr.call_count == 1
mock_incr.assert_has_calls([
call(str(service.id) + "-2016-01-01-count", ),
# call(str(service.id) + "-2016-01-01-count", ),
call("2016-01-01-total", )
])
@@ -247,9 +247,9 @@ def test_persist_notification_sets_daily_limit_cache_if_one_does_not_exists(
key_type=api_key.key_type,
reference="ref2")
assert mock_set.call_count == 2
assert mock_set.call_count == 1
mock_set.assert_has_calls([
call(str(service.id) + "-2016-01-01-count", 1, ex=86400),
# call(str(service.id) + "-2016-01-01-count", 1, ex=86400),
call("2016-01-01-total", 1, ex=86400)
])

View File

@@ -1,5 +1,3 @@
from datetime import datetime
import pytest
from flask import current_app
from freezegun import freeze_time
@@ -20,7 +18,6 @@ from app.notifications.validators import (
check_reply_to,
check_service_email_reply_to_id,
check_service_over_api_rate_limit,
check_service_over_daily_message_limit,
check_service_sms_sender_id,
check_template_is_active,
check_template_is_for_notification_type,
@@ -34,12 +31,7 @@ from app.serialised_models import (
SerialisedTemplate,
)
from app.utils import get_template_instance
from app.v2.errors import (
BadRequestError,
RateLimitError,
TooManyRequestsError,
TotalRequestsError,
)
from app.v2.errors import BadRequestError, RateLimitError, TotalRequestsError
from tests.app.db import (
create_api_key,
create_reply_to_email,
@@ -58,67 +50,6 @@ def enable_redis(notify_api):
yield
@pytest.mark.parametrize('key_type', ['team', 'normal'])
def test_check_service_message_limit_in_cache_under_message_limit_passes(
key_type,
sample_service,
mocker):
serialised_service = SerialisedService.from_id(sample_service.id)
mock_get = mocker.patch('app.notifications.validators.redis_store.get', return_value="1")
mock_set = mocker.patch('app.notifications.validators.redis_store.set')
service_stats = check_service_over_daily_message_limit(key_type, serialised_service)
assert service_stats == 1
mock_get.assert_called_once_with(f'{serialised_service.id}-{datetime.utcnow().strftime("%Y-%m-%d")}-count')
mock_set.assert_not_called()
def test_check_service_over_daily_message_limit_should_not_interact_with_cache_for_test_key(sample_service, mocker):
mocker.patch('app.notifications.validators.redis_store')
mock_get = mocker.patch('app.notifications.validators.redis_store.get', side_effect=[None])
serialised_service = SerialisedService.from_id(sample_service.id)
service_stats = check_service_over_daily_message_limit('test', serialised_service)
assert service_stats == 0
mock_get.assert_not_called()
@pytest.mark.parametrize('key_type', ['team', 'normal'])
def test_check_service_over_daily_message_limit_should_set_cache_value_as_zero_if_cache_not_set(
key_type,
sample_template,
sample_service,
mocker
):
serialised_service = SerialisedService.from_id(sample_service.id)
with freeze_time("2016-01-01 12:00:00.000000"):
mocker.patch('app.notifications.validators.redis_store.set')
service_stats = check_service_over_daily_message_limit(key_type, serialised_service)
app.notifications.validators.redis_store.set.assert_called_with(
str(sample_service.id) + "-2016-01-01-count", 0, ex=86400
)
assert service_stats == 0
def test_check_service_over_daily_message_limit_does_nothing_if_redis_disabled(notify_api, sample_service, mocker):
serialised_service = SerialisedService.from_id(sample_service.id)
with set_config(notify_api, 'REDIS_ENABLED', False):
mock_cache_key = mocker.patch('notifications_utils.clients.redis.daily_limit_cache_key')
service_stats = check_service_over_daily_message_limit('normal', serialised_service)
assert service_stats == 0
assert mock_cache_key.method_calls == []
@pytest.mark.parametrize('key_type', ['team', 'normal'])
def test_check_service_message_limit_over_message_limit_fails(key_type, mocker, notify_db_session):
service = create_service(message_limit=4)
mocker.patch('app.redis_store.get', return_value="5")
with pytest.raises(TooManyRequestsError) as e:
check_service_over_daily_message_limit(key_type, service)
assert e.value.status_code == 429
assert e.value.message == 'Exceeded send limits (4) for today'
assert e.value.fields == []
@pytest.mark.parametrize('key_type', ['team', 'normal'])
def test_check_service_message_limit_over_total_limit_fails(key_type, mocker, notify_db_session):
service = create_service()
@@ -479,14 +410,12 @@ def test_check_rate_limiting_validates_api_rate_limit_and_daily_limit(
notify_db_session, mocker
):
mock_rate_limit = mocker.patch('app.notifications.validators.check_service_over_api_rate_limit')
mock_daily_limit = mocker.patch('app.notifications.validators.check_service_over_daily_message_limit')
service = create_service()
api_key = create_api_key(service=service)
check_rate_limiting(service, api_key)
mock_rate_limit.assert_called_once_with(service, api_key)
mock_daily_limit.assert_called_once_with(api_key.key_type, service)
@pytest.mark.parametrize('key_type', ['test', 'normal'])

View File

@@ -19,7 +19,7 @@ from app.models import (
ServiceGuestList,
)
from app.service.send_notification import send_one_off_notification
from app.v2.errors import BadRequestError, TooManyRequestsError
from app.v2.errors import BadRequestError
from tests.app.db import (
create_reply_to_email,
create_service,
@@ -235,24 +235,6 @@ def test_send_one_off_notification_raises_if_cant_send_to_recipient(
assert 'service is in trial mode' in e.value.message
def test_send_one_off_notification_raises_if_over_limit(notify_db_session, mocker):
service = create_service(message_limit=0)
template = create_template(service=service)
mocker.patch(
'app.service.send_notification.check_service_over_daily_message_limit',
side_effect=TooManyRequestsError(1)
)
post_data = {
'template_id': str(template.id),
'to': '07700 900 001',
'created_by': str(service.created_by_id)
}
with pytest.raises(TooManyRequestsError):
send_one_off_notification(service.id, post_data)
def test_send_one_off_notification_raises_if_message_too_long(persist_mock, notify_db_session):
service = create_service()
template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon")