diff --git a/modoboa/admin/management/commands/handle_mailbox_operations.py b/modoboa/admin/management/commands/handle_mailbox_operations.py index 42b605708..16eae65f4 100644 --- a/modoboa/admin/management/commands/handle_mailbox_operations.py +++ b/modoboa/admin/management/commands/handle_mailbox_operations.py @@ -2,10 +2,11 @@ import logging import os +import shutil from django.core.management.base import BaseCommand -from param_tools import tools as param_tools +from modoboa.parameters import tools as param_tools from modoboa.lib.sysutils import exec_cmd from modoboa.lib.exceptions import InternalError @@ -52,13 +53,16 @@ class Command(BaseCommand): raise OperationError(output) def delete_mailbox(self, operation): + """Try to delete a mailbox tree on filesystem.""" if not os.path.exists(operation.argument): return - code, output = exec_cmd( - "rm -r %s" % operation.argument - ) - if code: - raise OperationError(output) + + def onerror(function, path, excinfo): + """Handle errors.""" + self.logger.critical( + "delete failed (reason: {})".format(excinfo)) + + shutil.rmtree(operation.argument, False, onerror) def check_pidfile(self, path): """Check if this command is already running @@ -93,8 +97,8 @@ class Command(BaseCommand): try: f(ope) except (OperationError, InternalError) as e: - self.logger.critical("%s failed (reason: %s)", - ope, str(e).encode("utf-8")) + self.logger.critical( + "%s failed (reason: %s)", ope, str(e).decode("utf-8")) else: self.logger.info("%s succeed", ope) ope.delete() diff --git a/modoboa/admin/models/domain.py b/modoboa/admin/models/domain.py index cfae8f6f3..e580fddae 100644 --- a/modoboa/admin/models/domain.py +++ b/modoboa/admin/models/domain.py @@ -226,8 +226,7 @@ class Domain(AdminObject): if self.alias_set.count(): ungrant_access_to_objects(self.alias_set.all()) if param_tools.get_global_parameter("auto_account_removal"): - for account in User.objects.filter(mailbox__domain=self): - account.delete(fromuser, keepdir) + User.objects.filter(mailbox__domain=self).delete() elif self.mailbox_set.count(): Quota.objects.filter(username__contains="@%s" % self.name).delete() ungrant_access_to_objects(self.mailbox_set.all()) diff --git a/modoboa/admin/models/mailbox.py b/modoboa/admin/models/mailbox.py index 215c5a155..b527ab0d0 100644 --- a/modoboa/admin/models/mailbox.py +++ b/modoboa/admin/models/mailbox.py @@ -350,7 +350,6 @@ reversion.register(SenderAddress) class MailboxOperation(models.Model): - """An operation on a mailbox.""" mailbox = models.ForeignKey(Mailbox, blank=True, null=True) diff --git a/modoboa/admin/tests/dovecot b/modoboa/admin/tests/dovecot new file mode 100755 index 000000000..dbb210754 --- /dev/null +++ b/modoboa/admin/tests/dovecot @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "2.12" diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py index 5165a5722..e71368f4f 100644 --- a/modoboa/admin/tests/test_domain.py +++ b/modoboa/admin/tests/test_domain.py @@ -186,12 +186,18 @@ class DomainTestCase(ModoTestCase): def test_delete(self): """Test the removal of a domain.""" dom = Domain.objects.get(name="test.com") - self.ajax_post( - reverse("admin:domain_delete", args=[dom.id]), - {} - ) + self.ajax_post(reverse("admin:domain_delete", args=[dom.id])) with self.assertRaises(Domain.DoesNotExist): Domain.objects.get(pk=1) + self.assertTrue( + User.objects.filter(username="user@test.com").exists()) + self.set_global_parameter("auto_account_removal", True) + dom = Domain.objects.get(name="test2.com") + self.ajax_post(reverse("admin:domain_delete", args=[dom.id])) + with self.assertRaises(Domain.DoesNotExist): + Domain.objects.get(pk=1) + self.assertFalse( + User.objects.filter(username="admin@test2.com").exists()) def test_domain_counters(self): """Check counters at domain level.""" diff --git a/modoboa/admin/tests/test_mailbox_operations.py b/modoboa/admin/tests/test_mailbox_operations.py new file mode 100644 index 000000000..d27d99233 --- /dev/null +++ b/modoboa/admin/tests/test_mailbox_operations.py @@ -0,0 +1,86 @@ +"""Management command tests.""" + +import os +import shutil +import tempfile + +import mock + +from django.core.management import call_command +from django.core.urlresolvers import reverse +from django.test import override_settings + +from modoboa.lib.tests import ModoTestCase + +from .. import factories +from .. import models + + +@override_settings( + DOVECOT_LOOKUP_PATH=["{}/dovecot".format(os.path.dirname(__file__))]) +class MailboxOperationTestCase(ModoTestCase): + """Test management command.""" + + @classmethod + def setUpTestData(cls): + """Create test data.""" + super(MailboxOperationTestCase, cls).setUpTestData() + factories.populate_database() + + def setUp(self): + """Initiate test env.""" + super(MailboxOperationTestCase, self).setUp() + self.workdir = tempfile.mkdtemp() + path = "{}/test.com/admin".format(self.workdir) + os.makedirs(path) + self.set_global_parameter("handle_mailboxes", True) + self.set_global_parameter("enable_admin_limits", False, app="limits") + + def tearDown(self): + """Reset test env.""" + shutil.rmtree(self.workdir) + + @mock.patch("modoboa.admin.models.Mailbox.mail_home") + def test_delete_account(self, mail_home_mock): + """Check delete operation.""" + path = "{}/test.com/admin".format(self.workdir) + mail_home_mock.__get__ = mock.Mock(return_value=path) + mb = models.Mailbox.objects.select_related("user").get( + address="admin", domain__name="test.com") + self.ajax_post( + reverse("admin:account_delete", args=[mb.user.pk]), {} + ) + call_command("handle_mailbox_operations") + self.assertFalse(models.MailboxOperation.objects.exists()) + self.assertFalse(os.path.exists(mb.mail_home)) + + @mock.patch("modoboa.admin.models.Mailbox.mail_home") + def test_rename_account(self, mail_home_mock): + """Check rename operation.""" + path = "{}/test.com/admin".format(self.workdir) + mail_home_mock.__get__ = mock.Mock(return_value=path) + mb = models.Mailbox.objects.select_related("user").get( + address="admin", domain__name="test.com") + values = { + "username": "admin2@test.com", "role": "DomainAdmins", + "is_active": True, "email": "admin2@test.com" + } + self.ajax_post( + reverse("admin:account_change", args=[mb.user.pk]), values + ) + path = "{}/test.com/admin2".format(self.workdir) + mail_home_mock.__get__ = mock.Mock(return_value=path) + call_command("handle_mailbox_operations") + self.assertFalse(models.MailboxOperation.objects.exists()) + self.assertTrue(os.path.exists(mb.mail_home)) + + @mock.patch("modoboa.admin.models.Mailbox.mail_home") + def test_delete_domain(self, mail_home_mock): + """Check delete operation.""" + path = "{}/test.com/admin".format(self.workdir) + mail_home_mock.__get__ = mock.Mock(return_value=path) + domain = models.Domain.objects.get(name="test.com") + self.ajax_post(reverse("admin:domain_delete", args=[domain.pk])) + call_command("handle_mailbox_operations") + self.assertFalse(models.MailboxOperation.objects.exists()) + self.assertFalse(os.path.exists(path)) diff --git a/modoboa/admin/views/domain.py b/modoboa/admin/views/domain.py index e1141a6ed..3348ec93d 100644 --- a/modoboa/admin/views/domain.py +++ b/modoboa/admin/views/domain.py @@ -135,7 +135,7 @@ def editdomain(request, dom_id): @login_required @permission_required("admin.delete_domain") def deldomain(request, dom_id): - keepdir = True if request.POST.get("keepdir", "false") == "true" else False + keepdir = request.POST.get("keepdir", "false") == "true" try: mb = Mailbox.objects.get(user__id=request.user.id) except Mailbox.DoesNotExist: