From fa417e108f271fd612bb2bf024407ad8a56ca735 Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Wed, 23 Nov 2016 15:37:19 +0100 Subject: [PATCH 1/3] take care of domain aliases --- modoboa/admin/management/commands/subcommands/_repair.py | 7 ++++++- modoboa/admin/tests/test_repair.py | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/modoboa/admin/management/commands/subcommands/_repair.py b/modoboa/admin/management/commands/subcommands/_repair.py index 213f40313..029e72ae1 100644 --- a/modoboa/admin/management/commands/subcommands/_repair.py +++ b/modoboa/admin/management/commands/subcommands/_repair.py @@ -62,7 +62,12 @@ class Repair(BaseCommand): elif isinstance(obj, models.DomainAlias): admin = obj.target.admins.first() else: - admin = obj.domain.admins.first() + if obj.domain: + admin = obj.domain.admins.first() + else: + # Alias is an internal alias for domain alias + # we don't care if it's not owned + continue if not admin: # domain has no admin. use the first superuser found admin = User.objects.filter(is_superuser=True, diff --git a/modoboa/admin/tests/test_repair.py b/modoboa/admin/tests/test_repair.py index 1ea383015..e93efa932 100644 --- a/modoboa/admin/tests/test_repair.py +++ b/modoboa/admin/tests/test_repair.py @@ -39,3 +39,8 @@ class RepairTestCase(ModoTestCase): management.call_command("modo", "repair", "--quiet", "--dry-run") # assert its not fixed self.assertIs(get_object_owner(mbox), None) + + def test_management_command_with_nul_domain(self): + """ Just assume nothing raise when an alias has no domain""" + models.Alias.objects.create(address='@modoboa.xxx') + management.call_command("modo", "repair", "--quiet") From 8fad817210ad1b763de010e385785babb8139d4c Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Thu, 24 Nov 2016 11:13:59 +0100 Subject: [PATCH 2/3] assume that aliases ar fixed --- modoboa/admin/tests/test_repair.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modoboa/admin/tests/test_repair.py b/modoboa/admin/tests/test_repair.py index e93efa932..c50128556 100644 --- a/modoboa/admin/tests/test_repair.py +++ b/modoboa/admin/tests/test_repair.py @@ -21,18 +21,20 @@ class RepairTestCase(ModoTestCase): def test_management_command(self): """Check that command works fine.""" ObjectAccess.objects.all().delete() - mbox = models.Mailbox.objects.all()[0] + mbox = models.Mailbox.objects.first() + alias = models.Alias.objects.first() # assert mbox has no owner self.assertIs(get_object_owner(mbox), None) # fix it. run in quiet mode because we dont want output in tests management.call_command("modo", "repair", "--quiet") # assert its fixed self.assertIsNot(get_object_owner(mbox), None) + self.assertIsNot(get_object_owner(alias), None) def test_management_command_with_dry_run(self): """Check that command works fine.""" ObjectAccess.objects.all().delete() - mbox = models.Mailbox.objects.all()[0] + mbox = models.Mailbox.objects.first() # assert mbox has no owner self.assertIs(get_object_owner(mbox), None) # show problems. run in quiet mode because we dont want output in tests From b30470ecb7b72a320d8e4355ccddd40771851bf2 Mon Sep 17 00:00:00 2001 From: Gael Pasgrimaud Date: Thu, 24 Nov 2016 15:36:52 +0100 Subject: [PATCH 3/3] refactor using queryset; add coverage report to tox --- .../commands/subcommands/_repair.py | 26 ++++++++----------- modoboa/admin/tests/test_repair.py | 9 ++++--- tox.ini | 4 ++- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/modoboa/admin/management/commands/subcommands/_repair.py b/modoboa/admin/management/commands/subcommands/_repair.py index 029e72ae1..7d8b4ea14 100644 --- a/modoboa/admin/management/commands/subcommands/_repair.py +++ b/modoboa/admin/management/commands/subcommands/_repair.py @@ -42,8 +42,9 @@ class Repair(BaseCommand): if not quiet: print(message) - def fix_owner(self, model, dry_run=False, **options): - for obj in model.objects.all(): + def fix_owner(self, qs, dry_run=False, **options): + model = qs.model + for obj in qs: kw = dict( cls=model.__name__, obj=obj @@ -62,12 +63,7 @@ class Repair(BaseCommand): elif isinstance(obj, models.DomainAlias): admin = obj.target.admins.first() else: - if obj.domain: - admin = obj.domain.admins.first() - else: - # Alias is an internal alias for domain alias - # we don't care if it's not owned - continue + admin = obj.domain.admins.first() if not admin: # domain has no admin. use the first superuser found admin = User.objects.filter(is_superuser=True, @@ -81,11 +77,11 @@ class Repair(BaseCommand): @known_problem def sometimes_objects_have_no_owner(self, **options): owned_models = ( - User, - models.Domain, - models.Mailbox, - models.Alias, - models.DomainAlias + User.objects.all(), + models.Domain.objects.all(), + models.DomainAlias.objects.all(), + models.Alias.objects.filter(domain__isnull=False), + models.Mailbox.objects.filter(domain__isnull=False), ) - for model in owned_models: - self.fix_owner(model, **options) + for qs in owned_models: + self.fix_owner(qs, **options) diff --git a/modoboa/admin/tests/test_repair.py b/modoboa/admin/tests/test_repair.py index c50128556..0674abdfa 100644 --- a/modoboa/admin/tests/test_repair.py +++ b/modoboa/admin/tests/test_repair.py @@ -26,7 +26,8 @@ class RepairTestCase(ModoTestCase): # assert mbox has no owner self.assertIs(get_object_owner(mbox), None) # fix it. run in quiet mode because we dont want output in tests - management.call_command("modo", "repair", "--quiet") + ret = management.call_command("modo", "repair", "--quiet") + assert ret is None # assert its fixed self.assertIsNot(get_object_owner(mbox), None) self.assertIsNot(get_object_owner(alias), None) @@ -38,11 +39,13 @@ class RepairTestCase(ModoTestCase): # assert mbox has no owner self.assertIs(get_object_owner(mbox), None) # show problems. run in quiet mode because we dont want output in tests - management.call_command("modo", "repair", "--quiet", "--dry-run") + ret = management.call_command("modo", "repair", "--quiet", "--dry-run") + assert ret is None # assert its not fixed self.assertIs(get_object_owner(mbox), None) def test_management_command_with_nul_domain(self): """ Just assume nothing raise when an alias has no domain""" models.Alias.objects.create(address='@modoboa.xxx') - management.call_command("modo", "repair", "--quiet") + ret = management.call_command("modo", "repair", "--quiet") + assert ret is None diff --git a/tox.ini b/tox.ini index 185d3e9e0..ed4170764 100644 --- a/tox.ini +++ b/tox.ini @@ -5,11 +5,13 @@ envlist = py27 changedir=test_project commands = python ../tests.py - coverage run --source ../modoboa manage.py test modoboa.core modoboa.lib modoboa.admin.tests modoboa.limits modoboa.relaydomains + coverage run --source modoboa manage.py test modoboa.core modoboa.lib modoboa.admin.tests modoboa.limits modoboa.relaydomains + coverage report --show-missing deps = -r{toxinidir}/test-requirements.txt coverage setenv = + COVERAGE_FILE={envdir}/.coverage DB=SQLITE [testenv:serve]