From 44d5e80aeeaa1db42bdb048eff20ae0abac5ad8d Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 11 Sep 2020 17:27:41 +0200 Subject: [PATCH] Added alarms for domain/mailboxes. (#2009) * Added alarms for domain/mailboxes. * Try to close connections in thread. * Make sure to close database connections in threads. * Use TransactionTestCase for policyd because we use threads. * Added unit test. --- modoboa/admin/constants.py | 8 +++ modoboa/admin/factories.py | 10 +++ modoboa/admin/migrations/0017_alarm.py | 31 +++++++++ modoboa/admin/models/__init__.py | 2 + modoboa/admin/models/alarm.py | 55 +++++++++++++++ .../admin/templates/admin/domain_alarms.html | 46 +++++++++++++ modoboa/admin/templatetags/admin_tags.py | 9 ++- modoboa/admin/tests/test_domain.py | 3 + modoboa/admin/urls.py | 2 + modoboa/admin/views/__init__.py | 3 +- modoboa/admin/views/domain.py | 16 +++++ modoboa/core/models.py | 6 +- modoboa/policyd/core.py | 68 +++++++++++++++++-- modoboa/policyd/tests.py | 22 +++--- 14 files changed, 255 insertions(+), 26 deletions(-) create mode 100644 modoboa/admin/migrations/0017_alarm.py create mode 100644 modoboa/admin/models/alarm.py create mode 100644 modoboa/admin/templates/admin/domain_alarms.html diff --git a/modoboa/admin/constants.py b/modoboa/admin/constants.py index bfedf1eed..18480ab29 100644 --- a/modoboa/admin/constants.py +++ b/modoboa/admin/constants.py @@ -34,3 +34,11 @@ DKIM_KEY_LENGTHS = [ (2048, "2048"), (4096, "4096"), ] + +ALARM_OPENED = 1 +ALARM_CLOSED = 2 + +ALARM_STATUSES = [ + (ALARM_OPENED, _("Opened")), + (ALARM_CLOSED, _("Closed")), +] diff --git a/modoboa/admin/factories.py b/modoboa/admin/factories.py index cadd4907f..2d839c7ad 100644 --- a/modoboa/admin/factories.py +++ b/modoboa/admin/factories.py @@ -70,6 +70,16 @@ class SenderAddressFactory(factory.django.DjangoModelFactory): model = models.SenderAddress +class AlarmFactory(factory.django.DjangoModelFactory): + """Factory for Alarm model.""" + + domain = factory.SubFactory(DomainFactory) + mailbox = factory.SubFactory(MailboxFactory) + + class Meta: + model = models.Alarm + + def populate_database(): """Create test data. diff --git a/modoboa/admin/migrations/0017_alarm.py b/modoboa/admin/migrations/0017_alarm.py new file mode 100644 index 000000000..91afe96eb --- /dev/null +++ b/modoboa/admin/migrations/0017_alarm.py @@ -0,0 +1,31 @@ +# Generated by Django 2.2.12 on 2020-09-11 07:46 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('admin', '0016_auto_20200602_1201'), + ] + + operations = [ + migrations.CreateModel( + name='Alarm', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(default=django.utils.timezone.now)), + ('closed', models.DateTimeField(blank=True, null=True)), + ('status', models.IntegerField(choices=[(1, 'Opened'), (2, 'Closed')], db_index=True, default=1)), + ('title', models.CharField(max_length=150)), + ('internal_name', models.CharField(max_length=15)), + ('domain', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='alarms', to='admin.Domain')), + ('mailbox', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='alarms', to='admin.Mailbox')), + ], + options={ + 'ordering': ['created'], + }, + ), + ] diff --git a/modoboa/admin/models/__init__.py b/modoboa/admin/models/__init__.py index 29ac0b270..67e40e208 100644 --- a/modoboa/admin/models/__init__.py +++ b/modoboa/admin/models/__init__.py @@ -1,5 +1,6 @@ """Admin models.""" +from .alarm import Alarm from .alias import Alias, AliasRecipient from .base import AdminObject from .domain import Domain @@ -8,6 +9,7 @@ from .mailbox import Mailbox, MailboxOperation, Quota, SenderAddress from .mxrecord import DNSBLResult, MXRecord __all__ = [ + "Alarm", "AdminObject", "Alias", "AliasRecipient", diff --git a/modoboa/admin/models/alarm.py b/modoboa/admin/models/alarm.py new file mode 100644 index 000000000..e99c74fd4 --- /dev/null +++ b/modoboa/admin/models/alarm.py @@ -0,0 +1,55 @@ +"""Alert related models.""" + +from django.db import models +from django.utils import timezone + +from reversion import revisions as reversion + +from .. import constants + + +class AlarmQuerySet(models.QuerySet): + """Custom queryset for Alarm.""" + + def opened(self): + """Return opened alarms.""" + return self.filter(status=constants.ALARM_OPENED) + + +class Alarm(models.Model): + """A simple alarm to attach to a domain and/or mailbox.""" + + domain = models.ForeignKey( + "admin.Domain", on_delete=models.CASCADE, related_name="alarms") + mailbox = models.ForeignKey( + "admin.Mailbox", on_delete=models.SET_NULL, + related_name="alarms", + null=True, blank=True, + ) + created = models.DateTimeField(default=timezone.now) + closed = models.DateTimeField(null=True, blank=True) + status = models.IntegerField( + default=constants.ALARM_OPENED, choices=constants.ALARM_STATUSES, + db_index=True + ) + + title = models.CharField(max_length=150) + internal_name = models.CharField(max_length=15) + + objects = models.Manager.from_queryset(AlarmQuerySet)() + + class Meta: + ordering = ["created"] + + def __str__(self): + return "[{}] {} - {}".format( + self.created, self.domain, self.get_status_display()) + + def close(self): + """Close this alarm.""" + self.status = constants.ALARM_CLOSED + self.closed = timezone.now() + self.save() + + +reversion.register(Alarm) diff --git a/modoboa/admin/templates/admin/domain_alarms.html b/modoboa/admin/templates/admin/domain_alarms.html new file mode 100644 index 000000000..a9db261a9 --- /dev/null +++ b/modoboa/admin/templates/admin/domain_alarms.html @@ -0,0 +1,46 @@ +{% extends "admin/header.html" %} + +{% load admin_tags i18n lib_tags static %} + +{% block extra_css %} + {{ block.super }} + {% extra_static_content "domains" "css" user %} +{% endblock %} + +{% block leftcol %} + + + {% get_extra_admin_content user "leftcol" selection %} +{% endblock %} + +{% block apparea %} +

{% trans "Opened alarms for" %} {{ object }}

+
+ + + + + + + + + + + {% for alarm in object.alarms.opened %} + + + + + + {% empty %} + + + + {% endfor %} + +
{% trans "Date" %}{% trans "Message" %}{% trans "Mailbox" %}
{{ alarm.created|date:"SHORT_DATETIME_FORMAT" }}{{ alarm.title }}{{ alarm.mailbox|default:"" }}
+
{% trans "No opened alarm" %}
+
+{% endblock %} diff --git a/modoboa/admin/templatetags/admin_tags.py b/modoboa/admin/templatetags/admin_tags.py index cc2d36aed..abdc01466 100644 --- a/modoboa/admin/templatetags/admin_tags.py +++ b/modoboa/admin/templatetags/admin_tags.py @@ -133,8 +133,15 @@ def domain_actions(user, domain): "url": u"{0}#list/?searchquery=@{1}".format( reverse("admin:identity_list"), domain.name), "title": _("View the domain's identities"), - "img": "fa fa-user"} + "img": "fa fa-user"}, ] + if domain.alarms.opened().exists(): + actions.append({ + "name": "listalarms", + "url": reverse("admin:domain_alarms", args=[domain.pk]), + "title": _("View domain's alarms"), + "img": "fa fa-bell" + }) if user.has_perm("admin.change_domain"): actions.append({ "name": "editdomain", diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py index 593a772a9..28bebccb4 100644 --- a/modoboa/admin/tests/test_domain.py +++ b/modoboa/admin/tests/test_domain.py @@ -353,9 +353,12 @@ class DomainTestCase(ModoTestCase): def test_page_loader(self): """Test page loader view.""" + factories.AlarmFactory( + domain__name="test.com", mailbox=None, title="Test alarm") url = reverse("admin:domain_page") response = self.ajax_get(url) self.assertIn("handle_mailboxes", response) + self.assertIn("listalarms", response["rows"]) response = self.ajax_get("{}?objtype=quota".format(url)) self.assertIn("progress-bar", response["rows"]) diff --git a/modoboa/admin/urls.py b/modoboa/admin/urls.py index 1f10a8520..681313f00 100644 --- a/modoboa/admin/urls.py +++ b/modoboa/admin/urls.py @@ -15,6 +15,8 @@ urlpatterns = [ path('domains/', domain_views.domains, name="domain_list"), path('domains//', views.DomainDetailView.as_view(), name="domain_detail"), + path('domains//alarms/', views.DomainAlarmsView.as_view(), + name="domain_alarms"), path('domains//dnsbl/', views.DNSBLDomainDetailView.as_view(), name="dnsbl_domain_detail"), path('domains//mx/', views.MXDomainDetailView.as_view(), diff --git a/modoboa/admin/views/__init__.py b/modoboa/admin/views/__init__.py index eb2db5aa2..5f3ad4bcb 100644 --- a/modoboa/admin/views/__init__.py +++ b/modoboa/admin/views/__init__.py @@ -2,13 +2,14 @@ from .alias import AliasDetailView from .dns import DNSBLDomainDetailView, MXDomainDetailView -from .domain import DomainDetailView +from .domain import DomainAlarmsView, DomainDetailView from .identity import AccountDetailView __all__ = [ "AccountDetailView", "AliasDetailView", "DNSBLDomainDetailView", + "DomainAlarmsView", "DomainDetailView", "MXDomainDetailView", ] diff --git a/modoboa/admin/views/domain.py b/modoboa/admin/views/domain.py index 89b151cb5..6251b48bf 100644 --- a/modoboa/admin/views/domain.py +++ b/modoboa/admin/views/domain.py @@ -244,3 +244,19 @@ class DomainDetailView( context.update(widget["context"]) return context + + +class DomainAlarmsView( + auth_mixins.PermissionRequiredMixin, generic.DetailView): + """A view to list domain alarms.""" + + model = Domain + permission_required = "admin.view_domain" + template_name = "admin/domain_alarms.html" + + def get_queryset(self): + """Add some prefetching.""" + return ( + Domain.objects.get_for_admin(self.request.user) + .prefetch_related("alarms") + ) diff --git a/modoboa/core/models.py b/modoboa/core/models.py index 0db1ccf24..970d049cf 100644 --- a/modoboa/core/models.py +++ b/modoboa/core/models.py @@ -6,9 +6,7 @@ from email.header import Header from django.conf import settings from django.db import models from django.urls import reverse -from django.utils.encoding import ( - force_str, python_2_unicode_compatible, smart_bytes, smart_text -) +from django.utils.encoding import force_str, smart_bytes, smart_text from django.utils.functional import cached_property from django.utils.translation import ugettext as _, ugettext_lazy @@ -34,7 +32,6 @@ except ImportError: ldap_available = False -@python_2_unicode_compatible class User(AbstractUser): """Custom User model. @@ -409,7 +406,6 @@ def populate_callback(user, group="SimpleUsers"): sender="populate_callback", user=user) -@python_2_unicode_compatible class ObjectAccess(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE) content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) diff --git a/modoboa/policyd/core.py b/modoboa/policyd/core.py index 757f57dd8..110514893 100644 --- a/modoboa/policyd/core.py +++ b/modoboa/policyd/core.py @@ -10,11 +10,13 @@ from dateutil.relativedelta import relativedelta import aioredis from django.conf import settings +from django.db import connections from django.template.loader import render_to_string from django.utils import timezone from django.utils import translation from django.utils.translation import ugettext as _, ugettext_lazy +from modoboa.admin import constants as admin_constants from modoboa.admin import models as admin_models from modoboa.core import models as core_models from modoboa.lib.email_utils import split_mailbox @@ -28,6 +30,23 @@ SUCCESS_ACTION = b"dunno" FAILURE_ACTION = b"defer_if_permit Daily limit reached, retry later" +def close_db_connections(func, *args, **kwargs): + """ + Make sure to close all connections to DB. + + To use in threads. + """ + def _close_db_connections(*args, **kwargs): + ret = None + try: + ret = func(*args, **kwargs) + finally: + for conn in connections.all(): + conn.close() + return ret + return _close_db_connections + + async def wait_for(dt): """sleep until the specified datetime.""" one_day = 86400 @@ -48,11 +67,13 @@ async def run_at(dt, coro, *args): return await coro(*args) +@close_db_connections def get_local_config(): """Return local configuration.""" return core_models.LocalConfig.objects.first() +@close_db_connections def get_notification_recipients(): """Return superadmins with a mailbox.""" return ( @@ -61,6 +82,22 @@ def get_notification_recipients(): ) +@close_db_connections +def create_alarm(ltype, name): + """Create a new alarm.""" + title = _("Daily sending limit reached") + internal_name = "sending_limit" + if ltype == "domain": + domain = admin_models.Domain.objects.get(name=name) + domain.alarms.create(title=title, internal_name=internal_name) + else: + localpart, domain = split_mailbox(name) + mailbox = admin_models.Mailbox.objects.get( + address=localpart, domain__name=domain) + mailbox.alarms.create( + domain=mailbox.domain, title=title, internal_name=internal_name) + + async def notify_limit_reached(ltype, name): """Send a notification to super admins about item.""" ltype_translations = { @@ -72,9 +109,10 @@ async def notify_limit_reached(ltype, name): loop = asyncio.get_event_loop() futures = [ loop.run_in_executor(executor, get_local_config), - loop.run_in_executor(executor, get_notification_recipients) + loop.run_in_executor(executor, get_notification_recipients), + loop.run_in_executor(executor, create_alarm, ltype, name), ] - lc, recipients = await asyncio.gather(*futures) + lc, recipients, junk = await asyncio.gather(*futures) sender = lc.parameters.get_value("sender_address", app="core") for recipient in recipients: with translation.override(recipient.language): @@ -178,17 +216,35 @@ def get_next_execution_dt(): hour=0, minute=0, second=0) +@close_db_connections def get_domains_to_reset(): - """Return a list of domain to reset.""" - return admin_models.Domain.objects.filter(message_limit__isnull=False) + """ + Return a list of domain to reset. + + We also close all associated alarms. + """ + qset = admin_models.Domain.objects.filter(message_limit__isnull=False) + admin_models.Alarm.objects.filter( + internal_name="limit_reached", domain__in=qset).update( + status=admin_constants.ALARM_CLOSED, closed=timezone.now()) + return qset +@close_db_connections def get_mailboxes_to_reset(): - """Return a list of mailboxes to reset.""" - return ( + """ + Return a list of mailboxes to reset. + + We also close all associated alarms. + """ + qset = ( admin_models.Mailbox.objects.filter(message_limit__isnull=False) .select_related("domain") ) + admin_models.Alarm.objects.filter( + internal_name="limit_reached", mailbox__in=qset).update( + status=admin_constants.ALARM_CLOSED, closed=timezone.now()) + return qset async def reset_counters(): diff --git a/modoboa/policyd/tests.py b/modoboa/policyd/tests.py index f3cb18062..a53b5b790 100644 --- a/modoboa/policyd/tests.py +++ b/modoboa/policyd/tests.py @@ -11,12 +11,13 @@ import redis from django.conf import settings from django.core.management import call_command +from django.test import TransactionTestCase from django.urls import reverse from modoboa.admin import factories as admin_factories from modoboa.admin import models as admin_models from modoboa.core import models as core_models -from modoboa.lib.tests import ModoTestCase +from modoboa.lib.tests import ModoTestCase, ParametersMixin from modoboa.policyd import core as policyd_core from . import constants @@ -40,25 +41,20 @@ class RedisTestCaseMixin: self.rclient.delete(constants.REDIS_HASHNAME) -class PolicyDaemonTestCase(RedisTestCaseMixin, ModoTestCase): +class PolicyDaemonTestCase( + RedisTestCaseMixin, ParametersMixin, TransactionTestCase): """Test cases for policy daemon. A redis instance is required to run those tests. """ - @classmethod - def setUpTestData(cls): # NOQA:N802 - """Create test data.""" - super().setUpTestData() - admin_factories.populate_database() - # FIXME: all database modifications must be made before the - # daemon is started otherwise they won't be seen by the other - # process. I think it's just a sqlite issue... - cls.admin = core_models.User.objects.get(username="user@test2.com") - cls.admin.role = "SuperAdmins" - def setUp(self): super().setUp() + call_command("load_initial_data") + admin_factories.populate_database() + self.admin = core_models.User.objects.get(username="user@test2.com") + self.admin.role = "SuperAdmins" + patcher = patch("aiosmtplib.send") self.send_mock = patcher.start() self.process = Process(target=start_policy_daemon)