diff --git a/ietf/checks.py b/ietf/checks.py index f66182a02..a015460ff 100644 --- a/ietf/checks.py +++ b/ietf/checks.py @@ -56,33 +56,6 @@ def check_group_email_aliases_exists(app_configs, **kwargs): return errors -@checks.register('files') -def check_doc_email_aliases_exists(app_configs, **kwargs): - from ietf.doc.views_doc import check_doc_email_aliases - # - if already_ran(): - return [] - # - errors = [] - try: - ok = check_doc_email_aliases() - if not ok: - errors.append(checks.Error( - "Found no aliases in the document email aliases file\n'%s'."%settings.DRAFT_VIRTUAL_PATH, - hint="These should be created by the infrastructure using ietf/bin/aliases-from-json.py.", - obj=None, - id="datatracker.E0004", - )) - except IOError as e: - errors.append(checks.Error( - "Could not read document email aliases:\n %s" % e, - hint="These should be created by the infrastructure using ietf/bin/aliases-from-json.py.", - obj=None, - id="datatracker.E0005", - )) - - return errors - @checks.register('directories') def check_id_submission_directories(app_configs, **kwargs): # diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index e6a50937a..5d0bb9c4f 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -16,7 +16,6 @@ from http.cookies import SimpleCookie from pathlib import Path from pyquery import PyQuery from urllib.parse import urlparse, parse_qs -from tempfile import NamedTemporaryFile from collections import defaultdict from zoneinfo import ZoneInfo @@ -51,6 +50,7 @@ from ietf.doc.utils import ( DraftAliasGenerator, generate_idnits2_rfc_status, generate_idnits2_rfcs_obsoleted, + get_doc_email_aliases, ) from ietf.group.models import Group, Role from ietf.group.factories import GroupFactory, RoleFactory @@ -2169,24 +2169,6 @@ class ReferencesTest(TestCase): self.assertContains(r, doc1.name) class GenerateDraftAliasesTests(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.DRAFT_ALIASES_PATH - self.saved_draft_virtual_path = settings.DRAFT_VIRTUAL_PATH - settings.DRAFT_ALIASES_PATH = self.doc_aliases_file.name - settings.DRAFT_VIRTUAL_PATH = self.doc_virtual_file.name - - def tearDown(self): - settings.DRAFT_ALIASES_PATH = self.saved_draft_aliases_path - settings.DRAFT_VIRTUAL_PATH = self.saved_draft_virtual_path - os.unlink(self.doc_aliases_file.name) - os.unlink(self.doc_virtual_file.name) - super().tearDown() - @override_settings(TOOLS_SERVER="tools.example.org", DRAFT_ALIAS_DOMAIN="draft.example.org") def test_generator_class(self): """The DraftAliasGenerator should generate the same lists as the old mgmt cmd""" @@ -2286,6 +2268,28 @@ class GenerateDraftAliasesTests(TestCase): {k: sorted(v) for k, v in expected_dict.items()}, ) + # check single name + output = [(alias, alist) for alias, alist in DraftAliasGenerator(Document.objects.filter(name=doc1.name))] + alias_dict = dict(output) + self.assertEqual(len(alias_dict), len(output)) # no duplicate aliases + expected_dict = { + doc1.name: [author1.email_address()], + doc1.name + ".ad": [ad.email_address()], + doc1.name + ".authors": [author1.email_address()], + doc1.name + ".shepherd": [shepherd.email_address()], + doc1.name + + ".all": [ + author1.email_address(), + ad.email_address(), + shepherd.email_address(), + ], + } + # Sort lists for comparison + self.assertEqual( + {k: sorted(v) for k, v in alias_dict.items()}, + {k: sorted(v) for k, v in expected_dict.items()}, + ) + @override_settings(TOOLS_SERVER="tools.example.org", DRAFT_ALIAS_DOMAIN="draft.example.org") def test_get_draft_notify_emails(self): ad = PersonFactory() @@ -2336,37 +2340,20 @@ class EmailAliasesTests(TestCase): WgDraftFactory(name='draft-ietf-mars-test',group__acronym='mars') WgDraftFactory(name='draft-ietf-ames-test',group__acronym='ames') RoleFactory(group__type_id='review', group__acronym='yangdoctors', name_id='secr') - self.doc_alias_file = NamedTemporaryFile(delete=False, mode='w+') - self.doc_alias_file.write("""# Generated by hand at 2015-02-12_16:26:45 -virtual.ietf.org anything -draft-ietf-mars-test@ietf.org xfilter-draft-ietf-mars-test -expand-draft-ietf-mars-test@virtual.ietf.org mars-author@example.com, mars-collaborator@example.com -draft-ietf-mars-test.authors@ietf.org xfilter-draft-ietf-mars-test.authors -expand-draft-ietf-mars-test.authors@virtual.ietf.org mars-author@example.mars, mars-collaborator@example.mars -draft-ietf-mars-test.chairs@ietf.org xfilter-draft-ietf-mars-test.chairs -expand-draft-ietf-mars-test.chairs@virtual.ietf.org mars-chair@example.mars -draft-ietf-mars-test.all@ietf.org xfilter-draft-ietf-mars-test.all -expand-draft-ietf-mars-test.all@virtual.ietf.org mars-author@example.mars, mars-collaborator@example.mars, mars-chair@example.mars -draft-ietf-ames-test@ietf.org xfilter-draft-ietf-ames-test -expand-draft-ietf-ames-test@virtual.ietf.org ames-author@example.com, ames-collaborator@example.com -draft-ietf-ames-test.authors@ietf.org xfilter-draft-ietf-ames-test.authors -expand-draft-ietf-ames-test.authors@virtual.ietf.org ames-author@example.ames, ames-collaborator@example.ames -draft-ietf-ames-test.chairs@ietf.org xfilter-draft-ietf-ames-test.chairs -expand-draft-ietf-ames-test.chairs@virtual.ietf.org ames-chair@example.ames -draft-ietf-ames-test.all@ietf.org xfilter-draft-ietf-ames-test.all -expand-draft-ietf-ames-test.all@virtual.ietf.org ames-author@example.ames, ames-collaborator@example.ames, ames-chair@example.ames -""") - self.doc_alias_file.close() - self.saved_draft_virtual_path = settings.DRAFT_VIRTUAL_PATH - settings.DRAFT_VIRTUAL_PATH = self.doc_alias_file.name - def tearDown(self): - settings.DRAFT_VIRTUAL_PATH = self.saved_draft_virtual_path - os.unlink(self.doc_alias_file.name) - super().tearDown() - - def testAliases(self): + @mock.patch("ietf.doc.views_doc.get_doc_email_aliases") + def testAliases(self, mock_get_aliases): + mock_get_aliases.return_value = [ + {"doc_name": "draft-ietf-mars-test", "alias_type": "", "expansion": "mars-author@example.mars, mars-collaborator@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".authors", "expansion": "mars-author@example.mars, mars-collaborator@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".chairs", "expansion": "mars-chair@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".all", "expansion": "mars-author@example.mars, mars-collaborator@example.mars, mars-chair@example.mars"}, + {"doc_name": "draft-ietf-ames-test", "alias_type": "", "expansion": "ames-author@example.ames, ames-collaborator@example.ames"}, + {"doc_name": "draft-ietf-ames-test", "alias_type": ".authors", "expansion": "ames-author@example.ames, ames-collaborator@example.ames"}, + {"doc_name": "draft-ietf-ames-test", "alias_type": ".chairs", "expansion": "ames-chair@example.ames"}, + {"doc_name": "draft-ietf-ames-test", "alias_type": ".all", "expansion": "ames-author@example.ames, ames-collaborator@example.ames, ames-chair@example.ames"}, + ] PersonFactory(user__username='plain') url = urlreverse('ietf.doc.urls.redirect.document_email', kwargs=dict(name="draft-ietf-mars-test")) r = self.client.get(url) @@ -2376,16 +2363,70 @@ expand-draft-ietf-ames-test.all@virtual.ietf.org ames-author@example.ames, ames login_testing_unauthorized(self, "plain", url) r = self.client.get(url) self.assertEqual(r.status_code, 200) + self.assertEqual(mock_get_aliases.call_args, mock.call()) self.assertTrue(all([x in unicontent(r) for x in ['mars-test@','mars-test.authors@','mars-test.chairs@']])) self.assertTrue(all([x in unicontent(r) for x in ['ames-test@','ames-test.authors@','ames-test.chairs@']])) - def testExpansions(self): + + @mock.patch("ietf.doc.views_doc.get_doc_email_aliases") + def testExpansions(self, mock_get_aliases): + mock_get_aliases.return_value = [ + {"doc_name": "draft-ietf-mars-test", "alias_type": "", "expansion": "mars-author@example.mars, mars-collaborator@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".authors", "expansion": "mars-author@example.mars, mars-collaborator@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".chairs", "expansion": "mars-chair@example.mars"}, + {"doc_name": "draft-ietf-mars-test", "alias_type": ".all", "expansion": "mars-author@example.mars, mars-collaborator@example.mars, mars-chair@example.mars"}, + ] url = urlreverse('ietf.doc.views_doc.document_email', kwargs=dict(name="draft-ietf-mars-test")) r = self.client.get(url) + self.assertEqual(mock_get_aliases.call_args, mock.call("draft-ietf-mars-test")) self.assertEqual(r.status_code, 200) self.assertContains(r, 'draft-ietf-mars-test.all@ietf.org') self.assertContains(r, 'iesg_ballot_saved') + + @mock.patch("ietf.doc.utils.DraftAliasGenerator") + def test_get_doc_email_aliases(self, mock_alias_gen_cls): + mock_alias_gen_cls.return_value = [ + ("draft-something-or-other.some-type", ["somebody@example.com"]), + ("draft-something-or-other", ["somebody@example.com"]), + ("draft-nothing-at-all", ["nobody@example.com"]), + ("draft-nothing-at-all.some-type", ["nobody@example.com"]), + ] + # order is important in the response - should be sorted by doc name and otherwise left + # in order + self.assertEqual( + get_doc_email_aliases(), + [ + { + "doc_name": "draft-nothing-at-all", + "alias_type": "", + "expansion": "nobody@example.com", + }, + { + "doc_name": "draft-nothing-at-all", + "alias_type": ".some-type", + "expansion": "nobody@example.com", + }, + { + "doc_name": "draft-something-or-other", + "alias_type": ".some-type", + "expansion": "somebody@example.com", + }, + { + "doc_name": "draft-something-or-other", + "alias_type": "", + "expansion": "somebody@example.com", + }, + ], + ) + self.assertEqual(mock_alias_gen_cls.call_args, mock.call(None)) + # Repeat with a name, no need to re-test that the alias list is actually passed through, just + # check that the DraftAliasGenerator is called correctly + draft = WgDraftFactory() + get_doc_email_aliases(draft.name) + self.assertQuerySetEqual(mock_alias_gen_cls.call_args[0][0], Document.objects.filter(pk=draft.pk)) + + class DocumentMeetingTests(TestCase): def setUp(self): diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index c3d7552f2..dd3286925 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -14,7 +14,7 @@ import textwrap from collections import defaultdict, namedtuple, Counter from dataclasses import dataclass from pathlib import Path -from typing import Iterator, Union +from typing import Iterator, Optional, Union from zoneinfo import ZoneInfo from django.conf import settings @@ -1265,6 +1265,12 @@ def bibxml_for_draft(doc, rev=None): class DraftAliasGenerator: days = 2 * 365 + def __init__(self, draft_queryset=None): + if draft_queryset is not None: + self.draft_queryset = draft_queryset.filter(type_id="draft") # only drafts allowed + else: + self.draft_queryset = Document.objects.filter(type_id="draft") + def get_draft_ad_emails(self, doc): """Get AD email addresses for the given draft, if any.""" from ietf.group.utils import get_group_ad_emails # avoid circular import @@ -1333,7 +1339,7 @@ class DraftAliasGenerator: def __iter__(self) -> Iterator[tuple[str, list[str]]]: # Internet-Drafts with active status or expired within self.days show_since = timezone.now() - datetime.timedelta(days=self.days) - drafts = Document.objects.filter(type_id="draft") + drafts = self.draft_queryset active_drafts = drafts.filter(states__slug='active') inactive_recent_drafts = drafts.exclude(states__slug='active').filter(expires__gte=show_since) interesting_drafts = active_drafts | inactive_recent_drafts @@ -1384,6 +1390,22 @@ class DraftAliasGenerator: if all: yield alias + ".all", list(all) + +def get_doc_email_aliases(name: Optional[str] = None): + aliases = [] + for (alias, alist) in DraftAliasGenerator( + Document.objects.filter(type_id="draft", name=name) if name else None + ): + # alias is draft-name.alias_type + doc_name, _dot, alias_type = alias.partition(".") + aliases.append({ + "doc_name": doc_name, + "alias_type": f".{alias_type}" if alias_type else "", + "expansion": ", ".join(sorted(alist)), + }) + return sorted(aliases, key=lambda a: (a["doc_name"])) + + def investigate_fragment(name_fragment): can_verify = set() for root in [settings.INTERNET_DRAFT_PATH, settings.INTERNET_DRAFT_ARCHIVE_DIR]: diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 021d5645d..42898d209 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -35,13 +35,13 @@ import glob -import io import json import os import re from pathlib import Path +from django.core.cache import caches from django.db.models import Max from django.http import HttpResponse, Http404 from django.shortcuts import render, get_object_or_404, redirect @@ -49,6 +49,7 @@ from django.template.loader import render_to_string from django.urls import reverse as urlreverse from django.conf import settings from django import forms +from django.contrib.auth.decorators import login_required from django.contrib.staticfiles import finders import debug # pyflakes:ignore @@ -64,7 +65,7 @@ from ietf.doc.utils import (augment_events_with_revision, add_events_message_info, get_unicode_document_content, augment_docs_and_person_with_person_info, irsg_needed_ballot_positions, add_action_holder_change_event, build_file_urls, update_documentauthors, fuzzy_find_documents, - bibxml_for_draft) + bibxml_for_draft, get_doc_email_aliases) from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible from ietf.group.models import Role, Group from ietf.group.utils import can_manage_all_groups_of_type, can_manage_materials, group_features_role_filter @@ -1071,32 +1072,6 @@ def document_pdfized(request, name, rev=None, ext=None): else: raise Http404 -def check_doc_email_aliases(): - pattern = re.compile(r'^expand-(.*?)(\..*?)?@.*? +(.*)$') - good_count = 0 - tot_count = 0 - with io.open(settings.DRAFT_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 get_doc_email_aliases(name): - if name: - pattern = re.compile(r'^expand-(%s)(\..*?)?@.*? +(.*)$'%name) - else: - pattern = re.compile(r'^expand-(.*?)(\..*?)?@.*? +(.*)$') - aliases = [] - with io.open(settings.DRAFT_VIRTUAL_PATH,"r") as virtual_file: - for line in virtual_file.readlines(): - m = pattern.match(line) - if m: - aliases.append({'doc_name':m.group(1),'alias_type':m.group(2),'expansion':m.group(3)}) - return aliases def document_email(request,name): doc = get_object_or_404(Document, name=name) @@ -2021,16 +1996,26 @@ def remind_action_holders(request, name): ) -def email_aliases(request,name=''): - doc = get_object_or_404(Document, name=name) if name else None - if not name: - # require login for the overview page, but not for the - # document-specific pages - if not request.user.is_authenticated: - return redirect('%s?next=%s' % (settings.LOGIN_URL, request.path)) - aliases = get_doc_email_aliases(name) - - return render(request,'doc/email_aliases.html',{'aliases':aliases,'ietf_domain':settings.IETF_DOMAIN,'doc':doc}) +@login_required +def email_aliases(request): + """List of all email aliases + + This is currently slow except when cached + """ + slowcache = caches["slowpages"] + cache_key = "emailaliasesview" + aliases = slowcache.get(cache_key) + if not aliases: + aliases = get_doc_email_aliases() # gets all aliases + slowcache.set(cache_key, aliases, 3600) + return render( + request, + "doc/email_aliases.html", + { + "aliases": aliases, + "ietf_domain": settings.IETF_DOMAIN, + }, + ) class VersionForm(forms.Form): diff --git a/ietf/settings.py b/ietf/settings.py index f04dd15bc..7dadd881e 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -1065,11 +1065,6 @@ GROUP_ALIAS_DOMAIN = IETF_DOMAIN TEST_DATA_DIR = os.path.abspath(BASE_DIR + "/../test/data") -# Path to the email alias lists. Used by ietf.utils.aliases -DRAFT_ALIASES_PATH = os.path.join(TEST_DATA_DIR, "draft-aliases") -DRAFT_VIRTUAL_PATH = os.path.join(TEST_DATA_DIR, "draft-virtual") -DRAFT_VIRTUAL_DOMAIN = "virtual.ietf.org" - 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" diff --git a/ietf/templates/doc/email_aliases.html b/ietf/templates/doc/email_aliases.html index 3311f2e28..111d31b0b 100644 --- a/ietf/templates/doc/email_aliases.html +++ b/ietf/templates/doc/email_aliases.html @@ -3,13 +3,11 @@ {% load origin %} {% block title %} Document email aliases - {% if doc %}for {{ doc.name }}{% endif %} {% endblock %} {% block content %} {% origin %}