diff --git a/bin/daily b/bin/daily index b4ea339958..e6e5304654 100755 --- a/bin/daily +++ b/bin/daily @@ -5,7 +5,6 @@ # This script is expected to be triggered by cron from # /etc/cron.d/datatracker export LANG=en_US.UTF-8 -export PYTHONIOENCODING=utf-8 # Make sure we stop if something goes wrong: program=${0##*/} @@ -17,10 +16,6 @@ cd $DTDIR/ logger -p user.info -t cron "Running $DTDIR/bin/daily" -# Set up the virtual environment -source $DTDIR/env/bin/activate - - # Get IANA-registered yang models #YANG_IANA_DIR=$(python -c 'import ietf.settings; print ietf.settings.SUBMIT_YANG_IANA_MODEL_DIR') # Hardcode the rsync target to avoid any unwanted deletes: @@ -30,9 +25,3 @@ rsync -avzq --delete /a/www/ietf-ftp/iana/yang-parameters/ /a/www/ietf-ftp/yang/ # Get Yang models from Yangcatalog. #rsync -avzq rsync://rsync.yangcatalog.org:10873/yangdeps /a/www/ietf-ftp/yang/catalogmod/ /a/www/ietf-datatracker/scripts/sync_to_yangcatalog - -# Populate the yang repositories -$DTDIR/ietf/manage.py populate_yang_model_dirs -v0 - -# Re-run yang checks on active documents -$DTDIR/ietf/manage.py run_yang_model_checks -v0 diff --git a/ietf/checks.py b/ietf/checks.py index f66182a027..f911d081f0 100644 --- a/ietf/checks.py +++ b/ietf/checks.py @@ -28,61 +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('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 e6a50937a5..5d0bb9c4f9 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -16,7 +16,6 @@ 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 @@ 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 @@ def test_references(self): 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 @@ def test_generator_class(self): {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 @@ def setUp(self): 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 @@ def testAliases(self): 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 c3d7552f24..dd32869251 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -14,7 +14,7 @@ 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 @@ def get_draft_notify_emails(self, doc): 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 @@ def __iter__(self) -> Iterator[tuple[str, list[str]]]: 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 021d5645d9..42898d2098 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.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 @@ 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/group/tests.py b/ietf/group/tests.py index 0d5cddf105..130c68b3fc 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 @@ 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 @@ def test_group_document_dependencies(self): 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 @@ def test_generator_class(self): {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 29a45a32eb..561f542f42 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 @@ def setUp(self): 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 51696eb39b..68b618120b 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 @@ def __iter__(self): 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 @@ def __iter__(self): 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 @@ def __iter__(self): # 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 @@ def __iter__(self): 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 09b1ac933f..f909a31b6d 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 @@ 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/ipr/tests.py b/ietf/ipr/tests.py index e6964445dc..ae42f24442 100644 --- a/ietf/ipr/tests.py +++ b/ietf/ipr/tests.py @@ -789,12 +789,7 @@ def test_ingest_response_email(self, mock_process_response_email): mock_process_response_email.side_effect = None mock_process_response_email.return_value = None # rejected message - with self.assertRaises(EmailIngestionError) as context: - ingest_response_email(message) - self.assertIsNone(context.exception.email_recipients) # default recipients - self.assertIsNotNone(context.exception.email_body) # body set - self.assertIsNotNone(context.exception.email_original_message) # original message attached - self.assertEqual(context.exception.email_attach_traceback, True) + ingest_response_email(message) # should _not_ send an exception email on a clean rejection self.assertTrue(mock_process_response_email.called) self.assertEqual(mock_process_response_email.call_args, mock.call(message)) mock_process_response_email.reset_mock() diff --git a/ietf/ipr/utils.py b/ietf/ipr/utils.py index 06af1535f2..8a35927d12 100644 --- a/ietf/ipr/utils.py +++ b/ietf/ipr/utils.py @@ -92,7 +92,7 @@ def generate_draft_recursive_txt(): def ingest_response_email(message: bytes): from ietf.api.views import EmailIngestionError # avoid circular import try: - result = process_response_email(message) + process_response_email(message) except Exception as err: raise EmailIngestionError( "Datatracker IPR email ingestion error", @@ -104,15 +104,3 @@ def ingest_response_email(message: bytes): email_original_message=message, email_attach_traceback=True, ) from err - - if result is None: - raise EmailIngestionError( - "Datatracker IPR email ingestion rejected", - email_body=dedent("""\ - A message was rejected while ingesting IPR email into the Datatracker. The original message is attached. - - {error_summary} - """), - email_original_message=message, - email_attach_traceback=True, - ) diff --git a/ietf/settings.py b/ietf/settings.py index f04dd15bca..b341e3e0e7 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -1065,14 +1065,6 @@ def skip_unreadable_post(record): 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" POSTCONFIRM_PATH = "/a/postconfirm/wrapper" diff --git a/ietf/submit/tasks.py b/ietf/submit/tasks.py index 9a13268bce..9e279fa9f0 100644 --- a/ietf/submit/tasks.py +++ b/ietf/submit/tasks.py @@ -10,7 +10,8 @@ from ietf.submit.models import Submission from ietf.submit.utils import (cancel_submission, create_submission_event, process_uploaded_submission, - process_and_accept_uploaded_submission) + process_and_accept_uploaded_submission, run_all_yang_model_checks, + populate_yang_model_dirs) from ietf.utils import log @@ -66,6 +67,12 @@ def cancel_stale_submissions(): create_submission_event(None, subm, 'Submission canceled: expired without being posted') +@shared_task +def run_yang_model_checks_task(): + populate_yang_model_dirs() + run_all_yang_model_checks() + + @shared_task(bind=True) def poke(self): log.log(f'Poked {self.name}, request id {self.request.id}') diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 618f237e4d..b48168f8a6 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -49,6 +49,7 @@ from ietf.submit.forms import SubmissionBaseUploadForm, SubmissionAutoUploadForm from ietf.submit.models import Submission, Preapproval, SubmissionExtResource from ietf.submit.tasks import cancel_stale_submissions, process_and_accept_uploaded_submission_task +from ietf.submit.utils import apply_yang_checker_to_draft, run_all_yang_model_checks from ietf.utils import tool_version from ietf.utils.accesstoken import generate_access_token from ietf.utils.mail import outbox, get_payload_text @@ -3487,3 +3488,28 @@ def test_submission_checks(self): "Your Internet-Draft failed at least one submission check.", status_code=200, ) + + +class YangCheckerTests(TestCase): + @mock.patch("ietf.submit.utils.apply_yang_checker_to_draft") + def test_run_all_yang_model_checks(self, mock_apply): + active_drafts = WgDraftFactory.create_batch(3) + WgDraftFactory(states=[("draft", "expired")]) + run_all_yang_model_checks() + self.assertEqual(mock_apply.call_count, 3) + self.assertCountEqual( + [args[0][1] for args in mock_apply.call_args_list], + active_drafts, + ) + + def test_apply_yang_checker_to_draft(self): + draft = WgDraftFactory() + submission = SubmissionFactory(name=draft.name, rev=draft.rev) + submission.checks.create(checker="my-checker") + checker = mock.Mock() + checker.name = "my-checker" + checker.symbol = "X" + checker.check_file_txt.return_value = (True, "whee", None, None, {}) + apply_yang_checker_to_draft(checker, draft) + self.assertEqual(checker.check_file_txt.call_args, mock.call(draft.get_file_name())) + diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 770352b4cd..c814b84657 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -4,9 +4,11 @@ import datetime import io +import json import os import pathlib import re +import sys import time import traceback import xml2rfc @@ -15,6 +17,7 @@ from shutil import move from typing import Optional, Union # pyflakes:ignore from unidecode import unidecode +from xym import xym from django.conf import settings from django.core.exceptions import ValidationError @@ -43,6 +46,7 @@ from ietf.community.utils import update_name_contains_indexes_with_new_doc from ietf.submit.mail import ( announce_to_lists, announce_new_version, announce_to_authors, send_approval_request, send_submission_confirmation, announce_new_wg_00, send_manual_post_request ) +from ietf.submit.checkers import DraftYangChecker from ietf.submit.models import ( Submission, SubmissionEvent, Preapproval, DraftSubmissionStateName, SubmissionCheck, SubmissionExtResource ) from ietf.utils import log @@ -1431,3 +1435,133 @@ def process_uploaded_submission(submission): submission.state_id = "uploaded" submission.save() create_submission_event(None, submission, desc="Completed submission validation checks") + + +def apply_yang_checker_to_draft(checker, draft): + submission = Submission.objects.filter(name=draft.name, rev=draft.rev).order_by('-id').first() + if submission: + check = submission.checks.filter(checker=checker.name).order_by('-id').first() + if check: + result = checker.check_file_txt(draft.get_file_name()) + passed, message, errors, warnings, items = result + items = json.loads(json.dumps(items)) + new_res = (passed, errors, warnings, message) + old_res = (check.passed, check.errors, check.warnings, check.message) if check else () + if new_res != old_res: + log.log(f"Saving new yang checker results for {draft.name}-{draft.rev}") + qs = submission.checks.filter(checker=checker.name).order_by('time') + submission.checks.filter(checker=checker.name).exclude(pk=qs.first().pk).delete() + submission.checks.create(submission=submission, checker=checker.name, passed=passed, + message=message, errors=errors, warnings=warnings, items=items, + symbol=checker.symbol) + else: + log.log(f"Could not run yang checker for {draft.name}-{draft.rev}: missing submission object") + + +def run_all_yang_model_checks(): + checker = DraftYangChecker() + for draft in Document.objects.filter( + type_id="draft", + states=State.objects.get(type="draft", slug="active"), + ): + apply_yang_checker_to_draft(checker, draft) + + +def populate_yang_model_dirs(): + """Update the yang model dirs + + * All yang modules from published RFCs should be extracted and be + available in an rfc-yang repository. + + * All valid yang modules from active, not replaced, Internet-Drafts + should be extracted and be available in a draft-valid-yang repository. + + * All, valid and invalid, yang modules from active, not replaced, + Internet-Drafts should be available in a draft-all-yang repository. + (Actually, given precedence ordering, it would be enough to place + non-validating modules in a draft-invalid-yang repository instead). + + * In all cases, example modules should be excluded. + + * Precedence is established by the search order of the repository as + provided to pyang. + + * As drafts expire, models should be removed in order to catch cases + where a module being worked on depends on one which has slipped out + of the work queue. + + """ + def extract_from(file, dir, strict=True): + saved_stdout = sys.stdout + saved_stderr = sys.stderr + xymerr = io.StringIO() + xymout = io.StringIO() + sys.stderr = xymerr + sys.stdout = xymout + model_list = [] + try: + model_list = xym.xym(str(file), str(file.parent), str(dir), strict=strict, debug_level=-2) + for name in model_list: + modfile = moddir / name + mtime = file.stat().st_mtime + os.utime(str(modfile), (mtime, mtime)) + if '"' in name: + name = name.replace('"', '') + modfile.rename(str(moddir / name)) + model_list = [n.replace('"', '') for n in model_list] + except Exception as e: + log.log("Error when extracting from %s: %s" % (file, str(e))) + finally: + sys.stdout = saved_stdout + sys.stderr = saved_stderr + return model_list + + # Extract from new RFCs + + rfcdir = Path(settings.RFC_PATH) + + moddir = Path(settings.SUBMIT_YANG_RFC_MODEL_DIR) + if not moddir.exists(): + moddir.mkdir(parents=True) + + latest = 0 + for item in moddir.iterdir(): + if item.stat().st_mtime > latest: + latest = item.stat().st_mtime + + log.log(f"Extracting RFC Yang models to {moddir} ...") + for item in rfcdir.iterdir(): + if item.is_file() and item.name.startswith('rfc') and item.name.endswith('.txt') and item.name[3:-4].isdigit(): + if item.stat().st_mtime > latest: + model_list = extract_from(item, moddir) + for name in model_list: + if not (name.startswith('ietf') or name.startswith('iana')): + modfile = moddir / name + modfile.unlink() + + # Extract valid modules from drafts + + six_months_ago = time.time() - 6 * 31 * 24 * 60 * 60 + + def active(dirent): + return dirent.stat().st_mtime > six_months_ago + + draftdir = Path(settings.INTERNET_DRAFT_PATH) + moddir = Path(settings.SUBMIT_YANG_DRAFT_MODEL_DIR) + if not moddir.exists(): + moddir.mkdir(parents=True) + log.log(f"Emptying {moddir} ...") + for item in moddir.iterdir(): + item.unlink() + + log.log(f"Extracting draft Yang models to {moddir} ...") + for item in draftdir.iterdir(): + try: + if item.is_file() and item.name.startswith('draft') and item.name.endswith('.txt') and active(item): + model_list = extract_from(item, moddir, strict=False) + for name in model_list: + if name.startswith('example'): + modfile = moddir / name + modfile.unlink() + except UnicodeDecodeError as e: + log.log(f"Error processing {item.name}: {e}") diff --git a/ietf/templates/doc/email_aliases.html b/ietf/templates/doc/email_aliases.html index 3311f2e28e..111d31b0ba 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 %}