Ensure we count the tasks as well as the API calls.

After we have written to the database and placed it on a deliver queue we count it in the cache against the service.

This is the equivalent of doing it at the end of the API call.
This commit is contained in:
Martyn Inglis
2016-11-11 17:36:38 +00:00
parent 033a3e530b
commit 4c0c30bb2e
3 changed files with 117 additions and 64 deletions

View File

@@ -28,6 +28,8 @@ from app.models import (
) )
from app.service.utils import service_allowed_to_send_to from app.service.utils import service_allowed_to_send_to
from app.statsd_decorators import statsd from app.statsd_decorators import statsd
from app import redis_store
from app.clients.redis import cache_key
@notify_celery.task(name="process-job") @notify_celery.task(name="process-job")
@@ -148,6 +150,8 @@ def send_sms(self,
"RETRY FAILED: task send_sms failed for notification {}".format(notification.id), "RETRY FAILED: task send_sms failed for notification {}".format(notification.id),
e e
) )
else:
redis_store.inc(cache_key(service_id))
@notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=300) @notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=300)
@@ -187,3 +191,5 @@ def send_email(self, service_id,
"RETRY FAILED: task send_email failed for notification {}".format(notification.id), "RETRY FAILED: task send_email failed for notification {}".format(notification.id),
e e
) )
else:
redis_store.inc(cache_key(service_id))

View File

@@ -132,7 +132,7 @@ class Config(object):
STATSD_HOST = "statsd.hostedgraphite.com" STATSD_HOST = "statsd.hostedgraphite.com"
STATSD_PORT = 8125 STATSD_PORT = 8125
REDIS_ENABLED = True REDIS_ENABLED = False
REDIS_URL = "redis://localhost:6379/0" REDIS_URL = "redis://localhost:6379/0"
SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200 SENDING_NOTIFICATIONS_TIMEOUT_PERIOD = 259200
@@ -166,7 +166,6 @@ class Development(Config):
class Test(Config): class Test(Config):
REDIS_ENABLED = False
NOTIFY_EMAIL_DOMAIN = 'test.notify.com' NOTIFY_EMAIL_DOMAIN = 'test.notify.com'
FROM_NUMBER = 'testing' FROM_NUMBER = 'testing'
NOTIFY_ENVIRONMENT = 'test' NOTIFY_ENVIRONMENT = 'test'

View File

@@ -334,10 +334,12 @@ def test_should_process_all_sms_job(sample_job,
def test_should_send_template_to_correct_sms_task_and_persist(sample_template_with_placeholders, mocker): def test_should_send_template_to_correct_sms_task_and_persist(sample_template_with_placeholders, mocker):
with freeze_time("2016-01-01 12:00:00.000000"):
notification = _notification_json(sample_template_with_placeholders, notification = _notification_json(sample_template_with_placeholders,
to="+447234123123", personalisation={"name": "Jo"}) to="+447234123123", personalisation={"name": "Jo"})
mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
redis_mock = mocker.patch('app.celery.tasks.redis_store.inc')
notification_id = uuid.uuid4() notification_id = uuid.uuid4()
@@ -366,6 +368,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi
assert persisted_notification.personalisation == {'name': 'Jo'} assert persisted_notification.personalisation == {'name': 'Jo'}
assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"}) assert persisted_notification._personalisation == encryption.encrypt({"name": "Jo"})
assert persisted_notification.notification_type == 'sms' assert persisted_notification.notification_type == 'sms'
redis_mock.assert_called_with(str(sample_template_with_placeholders.service_id) + '-2016-01-01-count')
def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker):
@@ -528,6 +531,48 @@ def test_should_not_send_email_if_restricted_service_and_invalid_email_address(n
Notification.query.filter_by(id=notification_id).one() Notification.query.filter_by(id=notification_id).one()
def test_should_not_not_increment_counter_if_not_sending_sms(notify_db, notify_db_session, mocker):
redis_mock = mocker.patch('app.celery.tasks.redis_store.inc')
user = sample_user(notify_db, notify_db_session)
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
template = sample_template(
notify_db, notify_db_session, service=service, template_type='sms'
)
notification = _notification_json(template, to="+447878787878")
notification_id = uuid.uuid4()
send_sms(
service.id,
notification_id,
encryption.encrypt(notification),
datetime.utcnow().strftime(DATETIME_FORMAT)
)
redis_mock.assert_not_called()
def test_should_not_not_increment_counter_if_not_sending_email(notify_db, notify_db_session, mocker):
redis_mock = mocker.patch('app.celery.tasks.redis_store.inc')
user = sample_user(notify_db, notify_db_session)
service = sample_service(notify_db, notify_db_session, user=user, restricted=True)
template = sample_template(
notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello'
)
notification = _notification_json(template, to="test@example.com")
notification_id = uuid.uuid4()
send_email(
service.id,
notification_id,
encryption.encrypt(notification),
datetime.utcnow().strftime(DATETIME_FORMAT)
)
redis_mock.assert_not_called()
def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_service( def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_service(
notify_db, notify_db_session, mocker notify_db, notify_db_session, mocker
): ):
@@ -649,12 +694,14 @@ def test_should_not_send_sms_if_team_key_and_recipient_not_in_team(notify_db, no
def test_should_use_email_template_and_persist(sample_email_template_with_placeholders, sample_api_key, mocker): def test_should_use_email_template_and_persist(sample_email_template_with_placeholders, sample_api_key, mocker):
with freeze_time("2016-01-01 12:00:00.000000"):
notification = _notification_json( notification = _notification_json(
sample_email_template_with_placeholders, sample_email_template_with_placeholders,
'my_email@my_email.com', 'my_email@my_email.com',
{"name": "Jo"}, {"name": "Jo"},
row_number=1) row_number=1)
mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
redis_mock = mocker.patch('app.celery.tasks.redis_store.inc')
notification_id = uuid.uuid4() notification_id = uuid.uuid4()
@@ -689,6 +736,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
assert persisted_notification.api_key_id == sample_api_key.id assert persisted_notification.api_key_id == sample_api_key.id
assert persisted_notification.key_type == KEY_TYPE_NORMAL assert persisted_notification.key_type == KEY_TYPE_NORMAL
assert persisted_notification.notification_type == 'email' assert persisted_notification.notification_type == 'email'
redis_mock.assert_called_with(str(sample_email_template_with_placeholders.service_id) + '-2016-01-01-count')
def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker): def test_send_email_should_use_template_version_from_job_not_latest(sample_email_template, mocker):