From 24ea6f1637c303beeb90c271f32c38767105eb6d Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 25 Apr 2016 16:12:46 +0100 Subject: [PATCH] Use short dates when selection notifications for deletion. This means we will retain notifications for a full week and not delete records that are 7 x 24 hours older than the time of the run of the deletion task. Also the task only needs to run once a day now, so I have changed the celery config for the deletion tasks. --- app/dao/notifications_dao.py | 12 ++++--- app/models.py | 14 +++++++- config.py | 5 +-- tests/app/dao/test_notification_dao.py | 45 +++++++++++++++++--------- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 14964b568..646ecf7f6 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,5 +1,5 @@ import math -from sqlalchemy import desc +from sqlalchemy import desc, func from datetime import ( datetime, @@ -257,18 +257,20 @@ def filter_query(query, filter_dict=None): def delete_notifications_created_more_than_a_day_ago(status): + one_day_ago = date.today() - timedelta(days=1) deleted = db.session.query(Notification).filter( - Notification.created_at < datetime.utcnow() - timedelta(days=1), + func.date(Notification.created_at) < one_day_ago, Notification.status == status - ).delete() + ).delete(synchronize_session='fetch') db.session.commit() return deleted def delete_notifications_created_more_than_a_week_ago(status): + seven_days_ago = date.today() - timedelta(days=7) deleted = db.session.query(Notification).filter( - Notification.created_at < datetime.utcnow() - timedelta(days=7), + func.date(Notification.created_at) < seven_days_ago, Notification.status == status - ).delete() + ).delete(synchronize_session='fetch') db.session.commit() return deleted diff --git a/app/models.py b/app/models.py index a240d8fae..b15baff81 100644 --- a/app/models.py +++ b/app/models.py @@ -1,7 +1,10 @@ import uuid import datetime -from sqlalchemy.dialects.postgresql import UUID +from sqlalchemy.dialects.postgresql import ( + UUID, + JSON +) from sqlalchemy import UniqueConstraint @@ -410,3 +413,12 @@ class TemplateStatistics(db.Model): unique=False, nullable=False, default=datetime.datetime.utcnow) + + +class Event(object): + + __tablename__ = 'events' + + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + type = db.Column(db.String(255), nullable=False) + details = db.Column(JSON, nullable=False) diff --git a/config.py b/config.py index ad6765232..23687f4b5 100644 --- a/config.py +++ b/config.py @@ -1,4 +1,5 @@ from datetime import timedelta +from celery.schedules import crontab from kombu import Exchange, Queue import os @@ -51,12 +52,12 @@ class Config(object): }, 'delete-failed-notifications': { 'task': 'delete-failed-notifications', - 'schedule': timedelta(minutes=60), + 'schedule': crontab(minute=0, hour=0), 'options': {'queue': 'periodic'} }, 'delete-successful-notifications': { 'task': 'delete-successful-notifications', - 'schedule': timedelta(minutes=31), + 'schedule': crontab(minute=0, hour=0), 'options': {'queue': 'periodic'} } } diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index e03ccaf2b..79b57f9a7 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -3,7 +3,6 @@ import uuid import pytest -from app import DATE_FORMAT from freezegun import freeze_time from sqlalchemy.exc import SQLAlchemyError, IntegrityError @@ -767,7 +766,7 @@ def test_update_notification(sample_notification, sample_template): def test_should_delete_notifications_after_one_day(notify_db, notify_db_session): - created_at = datetime.utcnow() - timedelta(hours=24) + created_at = datetime.utcnow() - timedelta(days=2) sample_notification(notify_db, notify_db_session, created_at=created_at) sample_notification(notify_db, notify_db_session, created_at=created_at) assert len(Notification.query.all()) == 2 @@ -775,20 +774,32 @@ def test_should_delete_notifications_after_one_day(notify_db, notify_db_session) assert len(Notification.query.all()) == 0 +@freeze_time("2016-01-10 12:00:00.000000") def test_should_delete_notifications_after_seven_days(notify_db, notify_db_session): - created_at = datetime.utcnow() - timedelta(hours=24 * 7) - sample_notification(notify_db, notify_db_session, created_at=created_at, status="failed") - sample_notification(notify_db, notify_db_session, created_at=created_at, status="failed") - assert len(Notification.query.all()) == 2 - delete_notifications_created_more_than_a_week_ago('failed') + assert len(Notification.query.all()) == 0 + # create one notification a day between 1st and 9th from 11:00 to 19:00 + for i in range(1, 11): + past_date = '2016-01-{0:02d} {0:02d}:00:00.000000'.format(i, i) + with freeze_time(past_date): + sample_notification(notify_db, notify_db_session, created_at=datetime.utcnow(), status="failed") + + all_notifications = Notification.query.all() + assert len(all_notifications) == 10 + + # Records from before 3rd should be deleted + delete_notifications_created_more_than_a_week_ago('failed') + remaining_notifications = Notification.query.all() + assert len(remaining_notifications) == 8 + assert remaining_notifications[0].created_at.date() == date(2016, 1, 3) + def test_should_not_delete_sent_notifications_before_one_day(notify_db, notify_db_session): - expired = datetime.utcnow() - timedelta(hours=24) - valid = datetime.utcnow() - timedelta(hours=23, minutes=59, seconds=59) - sample_notification(notify_db, notify_db_session, created_at=expired, to_field="expired") - sample_notification(notify_db, notify_db_session, created_at=valid, to_field="valid") + should_delete = datetime.utcnow() - timedelta(days=2) + do_not_delete = datetime.utcnow() - timedelta(days=1) + sample_notification(notify_db, notify_db_session, created_at=should_delete, to_field="expired") + sample_notification(notify_db, notify_db_session, created_at=do_not_delete, to_field="valid") assert len(Notification.query.all()) == 2 delete_notifications_created_more_than_a_day_ago('sending') @@ -797,14 +808,16 @@ def test_should_not_delete_sent_notifications_before_one_day(notify_db, notify_d def test_should_not_delete_failed_notifications_before_seven_days(notify_db, notify_db_session): - expired = datetime.utcnow() - timedelta(hours=24 * 7) - valid = datetime.utcnow() - timedelta(hours=(24 * 6) + 23, minutes=59, seconds=59) - sample_notification(notify_db, notify_db_session, created_at=expired, status="failed", to_field="expired") - sample_notification(notify_db, notify_db_session, created_at=valid, status="failed", to_field="valid") + should_delete = datetime.utcnow() - timedelta(days=8) + do_not_delete = datetime.utcnow() - timedelta(days=7) + sample_notification(notify_db, notify_db_session, created_at=should_delete, status="failed", + to_field="should_delete") + sample_notification(notify_db, notify_db_session, created_at=do_not_delete, status="failed", + to_field="do_not_delete") assert len(Notification.query.all()) == 2 delete_notifications_created_more_than_a_week_ago('failed') assert len(Notification.query.all()) == 1 - assert Notification.query.first().to == 'valid' + assert Notification.query.first().to == 'do_not_delete' @freeze_time("2016-03-30")