diff --git a/ietf/checks.py b/ietf/checks.py index a015460ff..f911d081f 100644 --- a/ietf/checks.py +++ b/ietf/checks.py @@ -28,34 +28,6 @@ def already_ran(): checks_run.append(name) return False - -@checks.register('files') -def check_group_email_aliases_exists(app_configs, **kwargs): - from ietf.group.views import check_group_email_aliases - # - if already_ran(): - return [] - # - errors = [] - try: - ok = check_group_email_aliases() - if not ok: - errors.append(checks.Error( - "Found no aliases in the group email aliases file\n'%s'."%settings.GROUP_ALIASES_PATH, - hint="These should be created by the infrastructure using ietf/bin/aliases-from-json.py.", - obj=None, - id="datatracker.E0002", - )) - except IOError as e: - errors.append(checks.Error( - "Could not read group email aliases:\n %s" % e, - hint="These should be created by the infrastructure using ietf/bin/aliases-from-json.py.", - obj=None, - id="datatracker.E0003", - )) - - return errors - @checks.register('directories') def check_id_submission_directories(app_configs, **kwargs): # diff --git a/ietf/group/tests.py b/ietf/group/tests.py index 0d5cddf10..130c68b3f 100644 --- a/ietf/group/tests.py +++ b/ietf/group/tests.py @@ -1,13 +1,10 @@ # Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- -import os import datetime import json +import mock -from tempfile import NamedTemporaryFile - -from django.conf import settings from django.urls import reverse as urlreverse from django.db.models import Q from django.test import Client @@ -22,6 +19,7 @@ from ietf.group.utils import ( get_group_role_emails, get_child_group_role_emails, get_group_ad_emails, + get_group_email_aliases, GroupAliasGenerator, role_holder_emails, ) @@ -132,24 +130,6 @@ class GroupDocDependencyTests(TestCase): class GenerateGroupAliasesTests(TestCase): - def setUp(self): - super().setUp() - self.doc_aliases_file = NamedTemporaryFile(delete=False, mode='w+') - self.doc_aliases_file.close() - self.doc_virtual_file = NamedTemporaryFile(delete=False, mode='w+') - self.doc_virtual_file.close() - self.saved_draft_aliases_path = settings.GROUP_ALIASES_PATH - self.saved_draft_virtual_path = settings.GROUP_VIRTUAL_PATH - settings.GROUP_ALIASES_PATH = self.doc_aliases_file.name - settings.GROUP_VIRTUAL_PATH = self.doc_virtual_file.name - - def tearDown(self): - settings.GROUP_ALIASES_PATH = self.saved_draft_aliases_path - settings.GROUP_VIRTUAL_PATH = self.saved_draft_virtual_path - os.unlink(self.doc_aliases_file.name) - os.unlink(self.doc_virtual_file.name) - super().tearDown() - def test_generator_class(self): """The GroupAliasGenerator should generate the same lists as the old mgmt cmd""" # clean out test fixture group roles we don't need for this test @@ -208,6 +188,66 @@ class GenerateGroupAliasesTests(TestCase): {k: (sorted(doms), sorted(addrs)) for k, (doms, addrs) in expected_dict.items()}, ) + @mock.patch("ietf.group.utils.GroupAliasGenerator") + def test_get_group_email_aliases(self, mock_alias_gen_cls): + GroupFactory(name="agroup", type_id="rg") + GroupFactory(name="bgroup") + GroupFactory(name="cgroup", type_id="rg") + GroupFactory(name="dgroup") + + mock_alias_gen_cls.return_value = [ + ("bgroup-chairs", ["ietf"], ["c1@example.com", "c2@example.com"]), + ("agroup-ads", ["ietf", "irtf"], ["ad@example.com"]), + ("bgroup-ads", ["ietf"], ["ad@example.com"]), + ] + # order is important - should be by acronym, otherwise left in order returned by generator + self.assertEqual( + get_group_email_aliases(None, None), + [ + { + "acronym": "agroup", + "alias_type": "-ads", + "expansion": "ad@example.com", + }, + { + "acronym": "bgroup", + "alias_type": "-chairs", + "expansion": "c1@example.com, c2@example.com", + }, + { + "acronym": "bgroup", + "alias_type": "-ads", + "expansion": "ad@example.com", + }, + ], + ) + self.assertQuerySetEqual( + mock_alias_gen_cls.call_args[0][0], + Group.objects.all(), + ordered=False, + ) + + # test other parameter combinations but we already checked that the alias generator's + # output will be passed through, so don't re-test the processing + get_group_email_aliases("agroup", None) + self.assertQuerySetEqual( + mock_alias_gen_cls.call_args[0][0], + Group.objects.filter(acronym="agroup"), + ordered=False, + ) + get_group_email_aliases(None, "wg") + self.assertQuerySetEqual( + mock_alias_gen_cls.call_args[0][0], + Group.objects.filter(type_id="wg"), + ordered=False, + ) + get_group_email_aliases("agroup", "wg") + self.assertQuerySetEqual( + mock_alias_gen_cls.call_args[0][0], + Group.objects.none(), + ordered=False, + ) + class GroupRoleEmailTests(TestCase): diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 29a45a32e..561f542f4 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -2,16 +2,15 @@ # -*- coding: utf-8 -*- -import os import calendar import datetime import io import bleach +import mock from unittest.mock import call, patch from pathlib import Path from pyquery import PyQuery -from tempfile import NamedTemporaryFile import debug # pyflakes:ignore @@ -1925,58 +1924,72 @@ class EmailAliasesTests(TestCase): PersonFactory(user__username='plain') GroupFactory(acronym='mars',parent=GroupFactory(type_id='area')) GroupFactory(acronym='ames',parent=GroupFactory(type_id='area')) - self.group_alias_file = NamedTemporaryFile(delete=False) - self.group_alias_file.write(b"""# Generated by hand at 2015-02-12_16:30:52 -virtual.ietf.org anything -mars-ads@ietf.org xfilter-mars-ads -expand-mars-ads@virtual.ietf.org aread@example.org -mars-chairs@ietf.org xfilter-mars-chairs -expand-mars-chairs@virtual.ietf.org mars_chair@ietf.org -ames-ads@ietf.org xfilter-mars-ads -expand-ames-ads@virtual.ietf.org aread@example.org -ames-chairs@ietf.org xfilter-mars-chairs -expand-ames-chairs@virtual.ietf.org mars_chair@ietf.org -""") - self.group_alias_file.close() - self.saved_group_virtual_path = settings.GROUP_VIRTUAL_PATH - settings.GROUP_VIRTUAL_PATH = self.group_alias_file.name - def tearDown(self): - settings.GROUP_VIRTUAL_PATH = self.saved_group_virtual_path - os.unlink(self.group_alias_file.name) - super().tearDown() - - def testAliases(self): + @mock.patch("ietf.group.views.get_group_email_aliases") + def testAliases(self, mock_get_aliases): url = urlreverse('ietf.group.urls_info_details.redirect.email', kwargs=dict(acronym="mars")) r = self.client.get(url) self.assertEqual(r.status_code, 302) + mock_get_aliases.return_value = [ + {"acronym": "mars", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "mars", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + ] for testdict in [dict(acronym="mars"),dict(acronym="mars",group_type="wg")]: url = urlreverse('ietf.group.urls_info_details.redirect.email', kwargs=testdict) r = self.client.get(url,follow=True) + self.assertEqual( + mock_get_aliases.call_args, + mock.call(testdict.get("acronym", None), testdict.get("group_type", None)), + ) self.assertTrue(all([x in unicontent(r) for x in ['mars-ads@','mars-chairs@']])) self.assertFalse(any([x in unicontent(r) for x in ['ames-ads@','ames-chairs@']])) url = urlreverse('ietf.group.views.email_aliases', kwargs=dict()) login_testing_unauthorized(self, "plain", url) + + mock_get_aliases.return_value = [ + {"acronym": "mars", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "mars", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + {"acronym": "ames", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "ames", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + ] r = self.client.get(url) self.assertTrue(r.status_code,200) + self.assertEqual(mock_get_aliases.call_args, mock.call(None, None)) self.assertTrue(all([x in unicontent(r) for x in ['mars-ads@','mars-chairs@','ames-ads@','ames-chairs@']])) url = urlreverse('ietf.group.views.email_aliases', kwargs=dict(group_type="wg")) + mock_get_aliases.return_value = [ + {"acronym": "mars", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "mars", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + {"acronym": "ames", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "ames", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + ] r = self.client.get(url) self.assertEqual(r.status_code,200) + self.assertEqual(mock_get_aliases.call_args, mock.call(None, "wg")) self.assertContains(r, 'mars-ads@') url = urlreverse('ietf.group.views.email_aliases', kwargs=dict(group_type="rg")) + mock_get_aliases.return_value = [] r = self.client.get(url) self.assertEqual(r.status_code,200) + self.assertEqual(mock_get_aliases.call_args, mock.call(None, "rg")) self.assertNotContains(r, 'mars-ads@') - def testExpansions(self): + @mock.patch("ietf.group.views.get_group_email_aliases") + def testExpansions(self, mock_get_aliases): + mock_get_aliases.return_value = [ + {"acronym": "mars", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "mars", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + {"acronym": "ames", "alias_type": "-ads", "expansion": "aread@example.org"}, + {"acronym": "ames", "alias_type": "-chairs", "expansion": "mars_chair@ietf.org"}, + ] url = urlreverse('ietf.group.views.email', kwargs=dict(acronym="mars")) r = self.client.get(url) self.assertEqual(r.status_code,200) + self.assertEqual(mock_get_aliases.call_args, mock.call("mars", None)) self.assertContains(r, 'Email aliases') self.assertContains(r, 'mars-ads@ietf.org') self.assertContains(r, 'group_personnel_change') diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 51696eb39..68b618120 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -373,6 +373,12 @@ class GroupAliasGenerator: ] # This should become groupfeature driven... no_ad_group_types = ["rg", "rag", "team", "program", "rfcedtyp", "edappr", "edwg"] + def __init__(self, group_queryset=None): + if group_queryset is None: + self.group_queryset = Group.objects.all() + else: + self.group_queryset = group_queryset + def __iter__(self): show_since = timezone.now() - datetime.timedelta(days=self.days) @@ -384,7 +390,7 @@ class GroupAliasGenerator: if g == "program": domains.append("iab") - entries = Group.objects.filter(type=g).all() + entries = self.group_queryset.filter(type=g).all() active_entries = entries.filter(state__in=self.active_states) inactive_recent_entries = entries.exclude( state__in=self.active_states @@ -405,7 +411,7 @@ class GroupAliasGenerator: yield name + "-chairs", domains, list(chair_emails) # The area lists include every chair in active working groups in the area - areas = Group.objects.filter(type="area").all() + areas = self.group_queryset.filter(type="area").all() active_areas = areas.filter(state__in=self.active_states) for area in active_areas: name = area.acronym @@ -418,7 +424,7 @@ class GroupAliasGenerator: # Other groups with chairs that require Internet-Draft submission approval gtypes = GroupTypeName.objects.values_list("slug", flat=True) - special_groups = Group.objects.filter( + special_groups = self.group_queryset.filter( type__features__req_subm_approval=True, acronym__in=gtypes, state="active" ) for group in special_groups: @@ -427,6 +433,24 @@ class GroupAliasGenerator: yield group.acronym + "-chairs", ["ietf"], list(chair_emails) +def get_group_email_aliases(acronym, group_type): + aliases = [] + group_queryset = Group.objects.all() + if acronym: + group_queryset = group_queryset.filter(acronym=acronym) + if group_type: + group_queryset = group_queryset.filter(type__slug=group_type) + for (alias, _, alist) in GroupAliasGenerator(group_queryset): + acro, _hyphen, alias_type = alias.partition("-") + expansion = ", ".join(sorted(alist)) + aliases.append({ + "acronym": acro, + "alias_type": f"-{alias_type}" if alias_type else "", + "expansion": expansion, + }) + return sorted(aliases, key=lambda a: a["acronym"]) + + def role_holder_emails(): """Get queryset of active Emails for group role holders""" group_types_of_interest = [ diff --git a/ietf/group/views.py b/ietf/group/views.py index 09b1ac933..f909a31b6 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -37,9 +37,7 @@ import copy import datetime import itertools -import io import math -import re import json import types @@ -81,7 +79,8 @@ from ietf.group.utils import (can_manage_all_groups_of_type, can_manage_materials, group_attribute_change_desc, construct_group_menu_context, get_group_materials, save_group_in_history, can_manage_group, update_role_set, - get_group_or_404, setup_default_community_list_for_group, fill_in_charter_info) + get_group_or_404, setup_default_community_list_for_group, fill_in_charter_info, + get_group_email_aliases) # from ietf.ietfauth.utils import has_role, is_authorized_in_group from ietf.mailtrigger.utils import gather_relevant_expansions @@ -137,21 +136,6 @@ def extract_last_name(role): return role.person.name_parts()[3] -def check_group_email_aliases(): - pattern = re.compile(r'expand-(.*?)(-\w+)@.*? +(.*)$') - tot_count = 0 - good_count = 0 - with io.open(settings.GROUP_VIRTUAL_PATH,"r") as virtual_file: - for line in virtual_file.readlines(): - m = pattern.match(line) - tot_count += 1 - if m: - good_count += 1 - if good_count > 50 and tot_count < 3*good_count: - return True - return False - - def response_from_file(fpath: Path) -> HttpResponse: """Helper to shovel a file back in an HttpResponse""" try: @@ -580,21 +564,6 @@ def group_about_status_edit(request, acronym, group_type=None): } ) -def get_group_email_aliases(acronym, group_type): - if acronym: - pattern = re.compile(r'expand-(%s)(-\w+)@.*? +(.*)$'%acronym) - else: - pattern = re.compile(r'expand-(.*?)(-\w+)@.*? +(.*)$') - - aliases = [] - with io.open(settings.GROUP_VIRTUAL_PATH,"r") as virtual_file: - for line in virtual_file.readlines(): - m = pattern.match(line) - if m: - if acronym or not group_type or Group.objects.filter(acronym=m.group(1),type__slug=group_type): - aliases.append({'acronym':m.group(1),'alias_type':m.group(2),'expansion':m.group(3)}) - return aliases - def email(request, acronym, group_type=None): group = get_group_or_404(acronym, group_type) diff --git a/ietf/settings.py b/ietf/settings.py index 7dadd881e..b341e3e0e 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -1065,9 +1065,6 @@ GROUP_ALIAS_DOMAIN = IETF_DOMAIN TEST_DATA_DIR = os.path.abspath(BASE_DIR + "/../test/data") -GROUP_ALIASES_PATH = os.path.join(TEST_DATA_DIR, "group-aliases") -GROUP_VIRTUAL_PATH = os.path.join(TEST_DATA_DIR, "group-virtual") -GROUP_VIRTUAL_DOMAIN = "virtual.ietf.org" POSTCONFIRM_PATH = "/a/postconfirm/wrapper" diff --git a/k8s/settings_local.py b/k8s/settings_local.py index 0b37df544..8fd9530d7 100644 --- a/k8s/settings_local.py +++ b/k8s/settings_local.py @@ -192,10 +192,6 @@ if _SCOUT_KEY is not None: SCOUT_CORE_AGENT_LAUNCH = False SCOUT_REVISION_SHA = __release_hash__[:7] -# Path to the email alias lists. Used by ietf.utils.aliases -GROUP_ALIASES_PATH = "/a/postfix/group-aliases" -GROUP_VIRTUAL_PATH = "/a/postfix/group-virtual" - STATIC_URL = os.environ.get("DATATRACKER_STATIC_URL", None) if STATIC_URL is None: from ietf import __version__