From fb88654d84424ebc95ae07d6c56fe8e5c1eaa028 Mon Sep 17 00:00:00 2001 From: Son Nguyen Kim Date: Tue, 13 Jul 2021 17:06:58 +0200 Subject: [PATCH] disable should_disable() for now --- app/email_utils.py | 187 +++++++++++++-------------- tests/test_email_utils.py | 261 +++++++++++++++++++------------------- 2 files changed, 223 insertions(+), 225 deletions(-) diff --git a/app/email_utils.py b/app/email_utils.py index 3583bfc8..70656f3a 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -13,13 +13,12 @@ from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText from email.utils import make_msgid, formatdate, parseaddr from smtplib import SMTP, SMTPServerDisconnected -from typing import Tuple, List, Optional +from typing import Optional import arrow import dkim import spf from jinja2 import Environment, FileSystemLoader -from sqlalchemy import func from validate_email import validate_email from app.config import ( @@ -56,7 +55,6 @@ from app.models import ( SLDomain, Contact, Alias, - EmailLog, TransactionalEmail, ) from app.utils import ( @@ -975,98 +973,101 @@ def should_disable(alias: Alias) -> bool: LOG.warning("%s cannot be disabled", alias) return False - yesterday = arrow.now().shift(days=-1) - nb_bounced_last_24h = ( - db.session.query(EmailLog) - .join(Contact, EmailLog.contact_id == Contact.id) - .filter( - EmailLog.bounced.is_(True), - EmailLog.is_reply.is_(False), - EmailLog.created_at > yesterday, - ) - .filter(Contact.alias_id == alias.id) - .count() - ) - # if more than 12 bounces in 24h -> disable alias - if nb_bounced_last_24h > 12: - LOG.d("more than 12 bounces in the last 24h, disable alias %s", alias) - return True - - # if more than 5 bounces but has bounces last week -> disable alias - elif nb_bounced_last_24h > 5: - one_week_ago = arrow.now().shift(days=-8) - nb_bounced_7d_1d = ( - db.session.query(EmailLog) - .join(Contact, EmailLog.contact_id == Contact.id) - .filter( - EmailLog.bounced.is_(True), - EmailLog.is_reply.is_(False), - EmailLog.created_at > one_week_ago, - EmailLog.created_at < yesterday, - ) - .filter(Contact.alias_id == alias.id) - .count() - ) - if nb_bounced_7d_1d > 1: - LOG.debug( - "more than 5 bounces in the last 24h and more than 1 bounces in the last 7 days, " - "disable alias %s", - alias, - ) - return True - else: - # alias level - # if bounces at least 9 days in the last 10 days -> disable alias - query = ( - db.session.query( - func.date(EmailLog.created_at).label("date"), - func.count(EmailLog.id).label("count"), - ) - .join(Contact, EmailLog.contact_id == Contact.id) - .filter(Contact.alias_id == alias.id) - .filter( - EmailLog.created_at > arrow.now().shift(days=-10), - EmailLog.bounced.is_(True), - EmailLog.is_reply.is_(False), - ) - .group_by("date") - ) - - if query.count() >= 9: - LOG.d( - "Bounces every day for at least 9 days in the last 10 days, disable alias %s", - alias, - ) - return True - - # account level - query = ( - db.session.query( - func.date(EmailLog.created_at).label("date"), - func.count(EmailLog.id).label("count"), - ) - .filter(EmailLog.user_id == alias.user_id) - .filter( - EmailLog.created_at > arrow.now().shift(days=-10), - EmailLog.bounced.is_(True), - EmailLog.is_reply.is_(False), - ) - .group_by("date") - ) - - # if an account has more than 10 bounces every day for at least 4 days in the last 10 days, disable alias - date_bounces: List[Tuple[arrow.Arrow, int]] = list(query) - if len(date_bounces) > 4: - if all([v > 10 for _, v in date_bounces]): - LOG.d( - "+10 bounces for +4 days in the last 10 days on %s, disable alias %s", - alias.user, - alias, - ) - return True - + # todo: remove return False + # yesterday = arrow.now().shift(days=-1) + # nb_bounced_last_24h = ( + # db.session.query(EmailLog) + # .join(Contact, EmailLog.contact_id == Contact.id) + # .filter( + # EmailLog.bounced.is_(True), + # EmailLog.is_reply.is_(False), + # EmailLog.created_at > yesterday, + # ) + # .filter(Contact.alias_id == alias.id) + # .count() + # ) + # # if more than 12 bounces in 24h -> disable alias + # if nb_bounced_last_24h > 12: + # LOG.d("more than 12 bounces in the last 24h, disable alias %s", alias) + # return True + # + # # if more than 5 bounces but has bounces last week -> disable alias + # elif nb_bounced_last_24h > 5: + # one_week_ago = arrow.now().shift(days=-8) + # nb_bounced_7d_1d = ( + # db.session.query(EmailLog) + # .join(Contact, EmailLog.contact_id == Contact.id) + # .filter( + # EmailLog.bounced.is_(True), + # EmailLog.is_reply.is_(False), + # EmailLog.created_at > one_week_ago, + # EmailLog.created_at < yesterday, + # ) + # .filter(Contact.alias_id == alias.id) + # .count() + # ) + # if nb_bounced_7d_1d > 1: + # LOG.debug( + # "more than 5 bounces in the last 24h and more than 1 bounces in the last 7 days, " + # "disable alias %s", + # alias, + # ) + # return True + # else: + # # alias level + # # if bounces at least 9 days in the last 10 days -> disable alias + # query = ( + # db.session.query( + # func.date(EmailLog.created_at).label("date"), + # func.count(EmailLog.id).label("count"), + # ) + # .join(Contact, EmailLog.contact_id == Contact.id) + # .filter(Contact.alias_id == alias.id) + # .filter( + # EmailLog.created_at > arrow.now().shift(days=-10), + # EmailLog.bounced.is_(True), + # EmailLog.is_reply.is_(False), + # ) + # .group_by("date") + # ) + # + # if query.count() >= 9: + # LOG.d( + # "Bounces every day for at least 9 days in the last 10 days, disable alias %s", + # alias, + # ) + # return True + # + # # account level + # query = ( + # db.session.query( + # func.date(EmailLog.created_at).label("date"), + # func.count(EmailLog.id).label("count"), + # ) + # .filter(EmailLog.user_id == alias.user_id) + # .filter( + # EmailLog.created_at > arrow.now().shift(days=-10), + # EmailLog.bounced.is_(True), + # EmailLog.is_reply.is_(False), + # ) + # .group_by("date") + # ) + # + # # if an account has more than 10 bounces every day for at least 4 days in the last 10 days, disable alias + # date_bounces: List[Tuple[arrow.Arrow, int]] = list(query) + # if len(date_bounces) > 4: + # if all([v > 10 for _, v in date_bounces]): + # LOG.d( + # "+10 bounces for +4 days in the last 10 days on %s, disable alias %s", + # alias.user, + # alias, + # ) + # return True + # + # return False + def parse_id_from_bounce(email_address: str) -> int: return int(email_address[email_address.find("+") : email_address.rfind("+")]) diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index 036ea7bd..a501e576 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -1,8 +1,6 @@ import email from email.message import EmailMessage -import arrow - from app.config import MAX_ALERT_24H, EMAIL_DOMAIN, BOUNCE_EMAIL from app.email_utils import ( get_email_domain_part, @@ -24,16 +22,15 @@ from app.email_utils import ( encode_text, EmailEncoding, replace, - should_disable, decode_text, parse_id_from_bounce, get_queue_id, ) from app.extensions import db -from app.models import User, CustomDomain, Alias, Contact, EmailLog +from app.models import User, CustomDomain + # flake8: noqa: E101, W191 -from tests.utils import login def test_get_email_domain_part(): @@ -593,139 +590,139 @@ def test_decode_text(): ) -def test_should_disable(flask_client): - user = User.create( - email="a@b.c", - password="password", - name="Test User", - activated=True, - include_sender_in_reverse_alias=True, - ) - alias = Alias.create_new_random(user) - db.session.commit() - - assert not should_disable(alias) - - # create a lot of bounce on this alias - contact = Contact.create( - user_id=user.id, - alias_id=alias.id, - website_email="contact@example.com", - reply_email="rep@sl.local", - commit=True, - ) - for _ in range(20): - EmailLog.create( - user_id=user.id, - contact_id=contact.id, - alias_id=contact.alias_id, - commit=True, - bounced=True, - ) - - assert should_disable(alias) - - # should not affect another alias - alias2 = Alias.create_new_random(user) - db.session.commit() - assert not should_disable(alias2) +# def test_should_disable(flask_client): +# user = User.create( +# email="a@b.c", +# password="password", +# name="Test User", +# activated=True, +# include_sender_in_reverse_alias=True, +# ) +# alias = Alias.create_new_random(user) +# db.session.commit() +# +# assert not should_disable(alias) +# +# # create a lot of bounce on this alias +# contact = Contact.create( +# user_id=user.id, +# alias_id=alias.id, +# website_email="contact@example.com", +# reply_email="rep@sl.local", +# commit=True, +# ) +# for _ in range(20): +# EmailLog.create( +# user_id=user.id, +# contact_id=contact.id, +# alias_id=contact.alias_id, +# commit=True, +# bounced=True, +# ) +# +# assert should_disable(alias) +# +# # should not affect another alias +# alias2 = Alias.create_new_random(user) +# db.session.commit() +# assert not should_disable(alias2) -def test_should_disable_bounces_every_day(flask_client): - """if an alias has bounces every day at least 9 days in the last 10 days, disable alias""" - user = login(flask_client) - alias = Alias.create_new_random(user) - db.session.commit() - - assert not should_disable(alias) - - # create a lot of bounce on this alias - contact = Contact.create( - user_id=user.id, - alias_id=alias.id, - website_email="contact@example.com", - reply_email="rep@sl.local", - commit=True, - ) - for i in range(9): - EmailLog.create( - user_id=user.id, - contact_id=contact.id, - alias_id=contact.alias_id, - commit=True, - bounced=True, - created_at=arrow.now().shift(days=-i), - ) - - assert should_disable(alias) +# def test_should_disable_bounces_every_day(flask_client): +# """if an alias has bounces every day at least 9 days in the last 10 days, disable alias""" +# user = login(flask_client) +# alias = Alias.create_new_random(user) +# db.session.commit() +# +# assert not should_disable(alias) +# +# # create a lot of bounce on this alias +# contact = Contact.create( +# user_id=user.id, +# alias_id=alias.id, +# website_email="contact@example.com", +# reply_email="rep@sl.local", +# commit=True, +# ) +# for i in range(9): +# EmailLog.create( +# user_id=user.id, +# contact_id=contact.id, +# alias_id=contact.alias_id, +# commit=True, +# bounced=True, +# created_at=arrow.now().shift(days=-i), +# ) +# +# assert should_disable(alias) -def test_should_disable_bounces_account(flask_client): - """if an account has more than 10 bounces every day for at least 5 days in the last 10 days, disable alias""" - user = login(flask_client) - alias = Alias.create_new_random(user) - - db.session.commit() - - # create a lot of bounces on alias - contact = Contact.create( - user_id=user.id, - alias_id=alias.id, - website_email="contact@example.com", - reply_email="rep@sl.local", - commit=True, - ) - - for day in range(6): - for _ in range(10): - EmailLog.create( - user_id=user.id, - contact_id=contact.id, - alias_id=contact.alias_id, - commit=True, - bounced=True, - created_at=arrow.now().shift(days=-day), - ) - - alias2 = Alias.create_new_random(user) - assert should_disable(alias2) +# def test_should_disable_bounces_account(flask_client): +# """if an account has more than 10 bounces every day for at least 5 days in the last 10 days, disable alias""" +# user = login(flask_client) +# alias = Alias.create_new_random(user) +# +# db.session.commit() +# +# # create a lot of bounces on alias +# contact = Contact.create( +# user_id=user.id, +# alias_id=alias.id, +# website_email="contact@example.com", +# reply_email="rep@sl.local", +# commit=True, +# ) +# +# for day in range(6): +# for _ in range(10): +# EmailLog.create( +# user_id=user.id, +# contact_id=contact.id, +# alias_id=contact.alias_id, +# commit=True, +# bounced=True, +# created_at=arrow.now().shift(days=-day), +# ) +# +# alias2 = Alias.create_new_random(user) +# assert should_disable(alias2) -def test_should_disable_bounce_consecutive_days(flask_client): - user = login(flask_client) - alias = Alias.create_new_random(user) - db.session.commit() - - contact = Contact.create( - user_id=user.id, - alias_id=alias.id, - website_email="contact@example.com", - reply_email="rep@sl.local", - commit=True, - ) - - # create 6 bounce on this alias in the last 24h: alias is not disabled - for _ in range(6): - EmailLog.create( - user_id=user.id, - contact_id=contact.id, - alias_id=contact.alias_id, - commit=True, - bounced=True, - ) - assert not should_disable(alias) - - # create 2 bounces in the last 7 days: alias should be disabled - for _ in range(2): - EmailLog.create( - user_id=user.id, - contact_id=contact.id, - alias_id=contact.alias_id, - commit=True, - bounced=True, - created_at=arrow.now().shift(days=-3), - ) - assert should_disable(alias) +# def test_should_disable_bounce_consecutive_days(flask_client): +# user = login(flask_client) +# alias = Alias.create_new_random(user) +# db.session.commit() +# +# contact = Contact.create( +# user_id=user.id, +# alias_id=alias.id, +# website_email="contact@example.com", +# reply_email="rep@sl.local", +# commit=True, +# ) +# +# # create 6 bounce on this alias in the last 24h: alias is not disabled +# for _ in range(6): +# EmailLog.create( +# user_id=user.id, +# contact_id=contact.id, +# alias_id=contact.alias_id, +# commit=True, +# bounced=True, +# ) +# assert not should_disable(alias) +# +# # create 2 bounces in the last 7 days: alias should be disabled +# for _ in range(2): +# EmailLog.create( +# user_id=user.id, +# contact_id=contact.id, +# alias_id=contact.alias_id, +# commit=True, +# bounced=True, +# created_at=arrow.now().shift(days=-3), +# ) +# assert should_disable(alias) def test_parse_id_from_bounce():