From 913d4326beb17ad6d9a05c11ea89f626a537f764 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 12:50:11 -0400
Subject: [PATCH 01/32] refactor: Change import style for clarity

---
 ietf/sync/tasks.py | 10 +++++-----
 ietf/sync/tests.py |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index 1e4cfe0772..c0904cb133 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -9,7 +9,7 @@
 
 from django.conf import settings
 
-from ietf.sync.rfceditor import MIN_ERRATA_RESULTS, MIN_INDEX_RESULTS, parse_index, update_docs_from_rfc_index
+from ietf.sync import rfceditor
 from ietf.utils import log
 from ietf.utils.timezone import date_today
 
@@ -44,7 +44,7 @@ def rfc_editor_index_update_task(full_index=False):
         log.log(f'GET request timed out retrieving RFC editor index: {exc}')
         return  # failed
     rfc_index_xml = response.text
-    index_data = parse_index(io.StringIO(rfc_index_xml))
+    index_data = rfceditor.parse_index(io.StringIO(rfc_index_xml))
     try:
         response = requests.get(
             settings.RFC_EDITOR_ERRATA_JSON_URL,
@@ -54,13 +54,13 @@ def rfc_editor_index_update_task(full_index=False):
         log.log(f'GET request timed out retrieving RFC editor errata: {exc}')
         return  # failed
     errata_data = response.json()   
-    if len(index_data) < MIN_INDEX_RESULTS:
+    if len(index_data) < rfceditor.MIN_INDEX_RESULTS:
         log.log("Not enough index entries, only %s" % len(index_data))
         return  # failed
-    if len(errata_data) < MIN_ERRATA_RESULTS:
+    if len(errata_data) < rfceditor.MIN_ERRATA_RESULTS:
         log.log("Not enough errata entries, only %s" % len(errata_data))
         return  # failed
-    for rfc_number, changes, doc, rfc_published in update_docs_from_rfc_index(
+    for rfc_number, changes, doc, rfc_published in rfceditor.update_docs_from_rfc_index(
         index_data, errata_data, skip_older_than_date=skip_date
     ):
         for c in changes:
diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py
index 6ce1d12521..e100172a4f 100644
--- a/ietf/sync/tests.py
+++ b/ietf/sync/tests.py
@@ -685,8 +685,8 @@ class TaskTests(TestCase):
         RFC_EDITOR_INDEX_URL="https://rfc-editor.example.com/index/",
         RFC_EDITOR_ERRATA_JSON_URL="https://rfc-editor.example.com/errata/",
     )
-    @mock.patch("ietf.sync.tasks.update_docs_from_rfc_index")
-    @mock.patch("ietf.sync.tasks.parse_index")
+    @mock.patch("ietf.sync.tasks.rfceditor.update_docs_from_rfc_index")
+    @mock.patch("ietf.sync.tasks.rfceditor.parse_index")
     @mock.patch("ietf.sync.tasks.requests.get")
     def test_rfc_editor_index_update_task(
         self, requests_get_mock, parse_index_mock, update_docs_mock

From 2a2992fbbbfba0b15716acc804848545e9c5947f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 13:20:59 -0400
Subject: [PATCH 02/32] feat: Add iana_changes_updates_task()

---
 ietf/sync/tasks.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index c0904cb133..561f705d08 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -8,7 +8,9 @@
 from celery import shared_task
 
 from django.conf import settings
+from django.utils import timezone
 
+from ietf.sync import iana
 from ietf.sync import rfceditor
 from ietf.utils import log
 from ietf.utils.timezone import date_today
@@ -65,3 +67,47 @@ def rfc_editor_index_update_task(full_index=False):
     ):
         for c in changes:
             log.log("RFC%s, %s: %s" % (rfc_number, doc.name, c))
+
+
+@shared_task
+def iana_changes_updates_task():
+    # compensate to avoid we ask for something that happened now and then
+    # don't get it back because our request interval is slightly off
+    CLOCK_SKEW_COMPENSATION = 5  # seconds
+
+    # actually the interface accepts 24 hours, but then we get into
+    # trouble with daylights savings - meh
+    MAX_INTERVAL_ACCEPTED_BY_IANA = datetime.timedelta(hours=23)
+
+    start = (
+        timezone.now() 
+        - datetime.timedelta(hours=23) 
+        + datetime.timedelta(seconds=CLOCK_SKEW_COMPENSATION,)
+    )
+    end = start + datetime.timedelta(hours=23)
+
+    t = start
+    while t < end:
+        # the IANA server doesn't allow us to fetch more than a certain
+        # period, so loop over the requested period and make multiple
+        # requests if necessary
+
+        text = iana.fetch_changes_json(
+            settings.IANA_SYNC_CHANGES_URL, t, min(end, t + MAX_INTERVAL_ACCEPTED_BY_IANA)
+        )
+        log.log(f"Retrieved the JSON: {text}")
+
+        changes = iana.parse_changes_json(text)
+        added_events, warnings = iana.update_history_with_changes(
+            changes, send_email=True
+        )
+
+        for e in added_events:
+            log.log(
+                f"Added event for {e.doc_id} {e.time}: {e.desc} (parsed json: {e.json})"
+            )
+
+        for w in warnings:
+            log.log(f"WARNING: {w}")
+
+        t += MAX_INTERVAL_ACCEPTED_BY_IANA

From 8571c3f638c7f852156585a6b9535adb6c7e3f5e Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 16:42:35 -0400
Subject: [PATCH 03/32] chore: Squelch lint warning

My linter does not like variables defined outside
of __init__()
---
 ietf/utils/management/commands/periodic_tasks.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index 5dc891cfb4..49ba60af0c 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -31,6 +31,7 @@
 
 class Command(BaseCommand):
     """Manage periodic tasks"""
+    crontabs = None
 
     def add_arguments(self, parser):
         parser.add_argument("--create-default", action="store_true")

From 88738ba84f669ddfcb27543c90a8fa0dd1482d41 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 16:45:52 -0400
Subject: [PATCH 04/32] feat: Add PeriodicTask for iana_changes_updates_task

---
 ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index 49ba60af0c..db99af5d04 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -113,6 +113,16 @@ def create_default_tasks(self):
             ),
         )
 
+        PeriodicTask.objects.get_or_create(
+            name="Sync with IANA changes",
+            task="ietf.sync.tasks.iana_changes_update_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["hourly"],
+                description="Fetch change list from IANA and apply to documents",
+            ),
+        )
+
     def show_tasks(self):
         for label, crontab in self.crontabs.items():
             tasks = PeriodicTask.objects.filter(crontab=crontab).order_by(

From 428a22aa1dd814434b4c7341ab18e12225656fd8 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 17:01:56 -0400
Subject: [PATCH 05/32] refactor: tasks instead of scripts on
 sync.views.notify()

---
 ietf/sync/views.py | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/ietf/sync/views.py b/ietf/sync/views.py
index 431dd0a8fc..9cbfe369f1 100644
--- a/ietf/sync/views.py
+++ b/ietf/sync/views.py
@@ -17,6 +17,7 @@
 
 from ietf.doc.models import DeletedEvent, StateDocEvent, DocEvent
 from ietf.ietfauth.utils import role_required, has_role
+from ietf.sync import tasks
 from ietf.sync.discrepancies import find_discrepancies
 from ietf.utils.serialize import object_as_shallow_dict
 from ietf.utils.log import log
@@ -91,19 +92,20 @@ def runscript(name):
                 log("Subprocess error %s when running '%s': %s %s" % (p.returncode, cmd, err, out))
                 raise subprocess.CalledProcessError(p.returncode, cmdstring, "\n".join([err, out]))
 
-        log("Running sync script from notify view POST")
-
-        if notification == "protocols":
-            runscript("iana-protocols-updates")
-
-        if notification == "changes":
-            runscript("iana-changes-updates")
-
-        if notification == "queue":
-            runscript("rfc-editor-queue-updates")
-
         if notification == "index":
-            runscript("rfc-editor-index-updates")
+            log("Queuing RFC Editor index sync from notify view POST")
+            tasks.rfc_editor_index_update_task.delay()
+        elif notification == "changes":
+            log("Queuing IANA changes sync from notify view POST")
+            tasks.iana_changes_updates_task.delay()
+        else:
+            log("Running sync script from notify view POST")
+    
+            if notification == "protocols":
+                runscript("iana-protocols-updates")
+    
+            if notification == "queue":
+                runscript("rfc-editor-queue-updates")
 
         return HttpResponse("OK", content_type="text/plain; charset=%s"%settings.DEFAULT_CHARSET)
 

From 65d1da0ae8235e789cdbcf6195f05ee4a845037f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 17:42:09 -0400
Subject: [PATCH 06/32] test: Test iana_changes_updates_task

---
 ietf/sync/tests.py | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py
index e100172a4f..6448a82952 100644
--- a/ietf/sync/tests.py
+++ b/ietf/sync/tests.py
@@ -19,7 +19,7 @@
 
 import debug                            # pyflakes:ignore
 
-from ietf.doc.factories import WgDraftFactory, RfcFactory, DocumentAuthorFactory
+from ietf.doc.factories import WgDraftFactory, RfcFactory, DocumentAuthorFactory, DocEventFactory
 from ietf.doc.models import Document, DocEvent, DeletedEvent, DocTagName, RelatedDocument, State, StateDocEvent
 from ietf.doc.utils import add_state_change_event
 from ietf.group.factories import GroupFactory
@@ -804,3 +804,42 @@ def json(self):
         parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS)
         tasks.rfc_editor_index_update_task(full_index=False)
         self.assertFalse(update_docs_mock.called)
+
+    @override_settings(IANA_SYNC_CHANGES_URL="https://iana.example.com/sync/")
+    @mock.patch("ietf.sync.tasks.iana.update_history_with_changes")
+    @mock.patch("ietf.sync.tasks.iana.parse_changes_json")
+    @mock.patch("ietf.sync.tasks.iana.fetch_changes_json")
+    def test_iana_changes_updates_task(
+        self, 
+        fetch_changes_mock,
+        parse_changes_mock,
+        update_history_mock,
+    ):
+        # set up mocks
+        fetch_return_val = object()
+        fetch_changes_mock.return_value = fetch_return_val
+        parse_return_val = object()
+        parse_changes_mock.return_value = parse_return_val
+        event_with_json = DocEventFactory()
+        event_with_json.json = "hi I'm json"
+        update_history_mock.return_value = [
+            [event_with_json],  # events
+            ["oh no!"],  # warnings
+        ]
+        
+        tasks.iana_changes_updates_task()
+        self.assertEqual(fetch_changes_mock.call_count, 1)
+        self.assertEqual(
+            fetch_changes_mock.call_args[0][0],
+            "https://iana.example.com/sync/",
+        )
+        self.assertTrue(parse_changes_mock.called)
+        self.assertEqual(
+            parse_changes_mock.call_args,
+            ((fetch_return_val,), {}),
+        )
+        self.assertTrue(update_history_mock.called)
+        self.assertEqual(
+            update_history_mock.call_args,
+            ((parse_return_val,), {"send_email": True}),
+        )

From c646da0e371e7e2d9127e526d33326fe88ace3dd Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Wed, 24 Jan 2024 17:42:47 -0400
Subject: [PATCH 07/32] refactor: rename task for consistency

---
 ietf/sync/tasks.py | 2 +-
 ietf/sync/tests.py | 4 ++--
 ietf/sync/views.py | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index 561f705d08..5395b4a1ed 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -70,7 +70,7 @@ def rfc_editor_index_update_task(full_index=False):
 
 
 @shared_task
-def iana_changes_updates_task():
+def iana_changes_update_task():
     # compensate to avoid we ask for something that happened now and then
     # don't get it back because our request interval is slightly off
     CLOCK_SKEW_COMPENSATION = 5  # seconds
diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py
index 6448a82952..f5c5ee2836 100644
--- a/ietf/sync/tests.py
+++ b/ietf/sync/tests.py
@@ -809,7 +809,7 @@ def json(self):
     @mock.patch("ietf.sync.tasks.iana.update_history_with_changes")
     @mock.patch("ietf.sync.tasks.iana.parse_changes_json")
     @mock.patch("ietf.sync.tasks.iana.fetch_changes_json")
-    def test_iana_changes_updates_task(
+    def test_iana_changes_update_task(
         self, 
         fetch_changes_mock,
         parse_changes_mock,
@@ -827,7 +827,7 @@ def test_iana_changes_updates_task(
             ["oh no!"],  # warnings
         ]
         
-        tasks.iana_changes_updates_task()
+        tasks.iana_changes_update_task()
         self.assertEqual(fetch_changes_mock.call_count, 1)
         self.assertEqual(
             fetch_changes_mock.call_args[0][0],
diff --git a/ietf/sync/views.py b/ietf/sync/views.py
index 9cbfe369f1..2beefa5782 100644
--- a/ietf/sync/views.py
+++ b/ietf/sync/views.py
@@ -97,7 +97,7 @@ def runscript(name):
             tasks.rfc_editor_index_update_task.delay()
         elif notification == "changes":
             log("Queuing IANA changes sync from notify view POST")
-            tasks.iana_changes_updates_task.delay()
+            tasks.iana_changes_update_task.delay()
         else:
             log("Running sync script from notify view POST")
     

From 533c435452b4271631e9686efaa2c7dc814c9379 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 10:58:22 -0400
Subject: [PATCH 08/32] feat: Add iana_protocols_update_task

---
 ietf/sync/tasks.py | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index 5395b4a1ed..00bf73ea53 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -5,7 +5,9 @@
 import datetime
 import io
 import requests
+
 from celery import shared_task
+from itertools import batched
 
 from django.conf import settings
 from django.utils import timezone
@@ -111,3 +113,35 @@ def iana_changes_update_task():
             log.log(f"WARNING: {w}")
 
         t += MAX_INTERVAL_ACCEPTED_BY_IANA
+
+
+@shared_task
+def iana_protocols_update_task():
+    # Earliest date for which we have data suitable to update (was described as
+    # "this needs to be the date where this tool is first deployed" in the original
+    # iana-protocols-updates script)"
+    rfc_must_published_later_than = datetime.datetime(
+        2012, 
+        11, 
+        26, 
+        tzinfo=datetime.timezone.utc,
+    )
+
+    try:
+        response = requests.get(
+            settings.IANA_SYNC_PROTOCOLS_URL,
+            timeout=30,
+        )
+    except requests.Timeout as exc:
+        log.log(f'GET request timed out retrieving IANA protocols page: {exc}')
+        raise
+
+    rfc_numbers = iana.parse_protocol_page(response.text)
+    for batch in batched(rfc_numbers, 100):
+        updated = iana.update_rfc_log_from_protocol_page(
+            batch,
+            rfc_must_published_later_than,
+        )
+
+        for d in updated:
+            log.log("Added history entry for %s" % d.display_name())

From a60d68f519e789afa9f1d32fca935a9c2ef8ff8b Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 11:11:51 -0400
Subject: [PATCH 09/32] feat: Add PeriodicTask for iana protocols sync

---
 ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index db99af5d04..51eaa1179e 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -123,6 +123,16 @@ def create_default_tasks(self):
             ),
         )
 
+        PeriodicTask.objects.get_or_create(
+            name="Sync with IANA protocols page",
+            task="ietf.sync.tasks.iana_changes_update_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["hourly"],
+                description="Fetch protocols page from IANA and update document event logs",
+            ),
+        )
+
     def show_tasks(self):
         for label, crontab in self.crontabs.items():
             tasks = PeriodicTask.objects.filter(crontab=crontab).order_by(

From 309f92dd1feb29cd35c69e8cbbd8b28837fbb1ee Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 11:13:32 -0400
Subject: [PATCH 10/32] refactor: Use protocol sync task instead of script in
 view

---
 ietf/sync/views.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/ietf/sync/views.py b/ietf/sync/views.py
index 2beefa5782..30b2a928e5 100644
--- a/ietf/sync/views.py
+++ b/ietf/sync/views.py
@@ -98,14 +98,12 @@ def runscript(name):
         elif notification == "changes":
             log("Queuing IANA changes sync from notify view POST")
             tasks.iana_changes_update_task.delay()
-        else:
+        elif notification == "protocols":
+            log("Queuing IANA protocols sync from notify view POST")
+            tasks.iana_protocols_update_task.delay()
+        elif notification == "queue":
             log("Running sync script from notify view POST")
-    
-            if notification == "protocols":
-                runscript("iana-protocols-updates")
-    
-            if notification == "queue":
-                runscript("rfc-editor-queue-updates")
+            runscript("rfc-editor-queue-updates")
 
         return HttpResponse("OK", content_type="text/plain; charset=%s"%settings.DEFAULT_CHARSET)
 

From 0d119015f6d7e29f336cea898366a93b8b7bd536 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 12:08:03 -0400
Subject: [PATCH 11/32] refactor: itertools.batched() not available until py312

---
 ietf/sync/tasks.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index 00bf73ea53..95277dfbd1 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -7,7 +7,6 @@
 import requests
 
 from celery import shared_task
-from itertools import batched
 
 from django.conf import settings
 from django.utils import timezone
@@ -137,6 +136,14 @@ def iana_protocols_update_task():
         raise
 
     rfc_numbers = iana.parse_protocol_page(response.text)
+
+    def batched(l, n):
+        """Split list l up in batches of max size n.
+        
+        For Python 3.12 or later, replace this with itertools.batched()
+        """
+        return (l[i:i + n] for i in range(0, len(l), n))
+
     for batch in batched(rfc_numbers, 100):
         updated = iana.update_rfc_log_from_protocol_page(
             batch,

From 51584a5151eefec26b8bbd9e4561d5548770886d Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 12:08:20 -0400
Subject: [PATCH 12/32] test: test iana_protocols_update_task

---
 ietf/sync/tests.py | 49 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py
index f5c5ee2836..c4d8150661 100644
--- a/ietf/sync/tests.py
+++ b/ietf/sync/tests.py
@@ -843,3 +843,52 @@ def test_iana_changes_update_task(
             update_history_mock.call_args,
             ((parse_return_val,), {"send_email": True}),
         )
+
+    @override_settings(IANA_SYNC_PROTOCOLS_URL="https://iana.example.com/proto/")
+    @mock.patch("ietf.sync.tasks.iana.update_rfc_log_from_protocol_page")
+    @mock.patch("ietf.sync.tasks.iana.parse_protocol_page")
+    @mock.patch("ietf.sync.tasks.requests.get")
+    def test_iana_protocols_update_task(
+        self,
+        requests_get_mock,
+        parse_protocols_mock,
+        update_rfc_log_mock,
+    ):
+        # set up mocks
+        requests_get_mock.return_value = mock.Mock(text="fetched response")
+        parse_protocols_mock.return_value = range(110)  # larger than batch size of 100
+        update_rfc_log_mock.return_value = [
+            mock.Mock(display_name=mock.Mock(return_value="name"))
+        ]
+        
+        # call the task
+        tasks.iana_protocols_update_task()
+        
+        # check that it did the right things
+        self.assertTrue(requests_get_mock.called)
+        self.assertEqual(
+            requests_get_mock.call_args[0], 
+            ("https://iana.example.com/proto/",),
+        )
+        self.assertTrue(parse_protocols_mock.called)
+        self.assertEqual(
+            parse_protocols_mock.call_args[0],
+            ("fetched response",),
+        )
+        self.assertEqual(update_rfc_log_mock.call_count, 2)
+        self.assertEqual(
+            update_rfc_log_mock.call_args_list[0][0][0],
+            range(100),  # first batch
+        )
+        self.assertEqual(
+            update_rfc_log_mock.call_args_list[1][0][0],
+            range(100, 110),  # second batch
+        )
+        # make sure the calls use the same later_than date and that it's the expected one
+        published_later_than = set(
+            update_rfc_log_mock.call_args_list[n][0][1] for n in (0, 1)
+        )
+        self.assertEqual(
+            published_later_than, 
+            {datetime.datetime(2012,11,26,tzinfo=datetime.timezone.utc)}
+        )

From 0273cf056a1b0271b09e9a5da68c86e4c370b158 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 13:12:15 -0400
Subject: [PATCH 13/32] feat: Add idindex_update_task()

Calls idindex generation functions and does the file
update dance to put them in place.
---
 ietf/idindex/tasks.py | 77 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 ietf/idindex/tasks.py

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
new file mode 100644
index 0000000000..9934cfcbe7
--- /dev/null
+++ b/ietf/idindex/tasks.py
@@ -0,0 +1,77 @@
+# Copyright The IETF Trust 2024, All Rights Reserved
+#
+# Celery task definitions
+#
+import shutil
+
+from celery import shared_task
+from pathlib import Path
+from tempfile import NamedTemporaryFile
+
+
+from .index import all_id_txt, all_id2_txt, id_index_txt
+
+@shared_task
+def idindex_update_task():
+    """Update I-D indexes"""
+    cleanup_list = set()
+    
+    def _make_temp_file(content):
+        with NamedTemporaryFile(delete=False, dir="/a/tmp") as tf:
+            cleanup_list.add(tf)
+            tf_path = Path(tf.name)
+            tf_path.chmod(0o644)
+            tf.write(content)
+        return tf_path
+
+    def _move_into_place(tf_path, dest):
+        shutil.move(tf_path, dest)
+        cleanup_list.remove(tf_path)
+
+    def _cleanup():
+        for tf_path in cleanup_list:
+            tf_path.unlink(missing_ok=True)
+
+    id_path = Path("/a/ietfdata/doc/draft/repository")
+    derived_path = Path("/a/ietfdata/derived")
+    download_path = Path("/a/www/www6s/download")
+
+    # all_id
+    try:
+        # Generate copies of new contents
+        all_id_content = all_id_txt()
+        all_id_tmpfile = _make_temp_file(all_id_content)
+        derived_all_id_tmpfile = _make_temp_file(all_id_content)
+        download_all_id_tmpfile = _make_temp_file(all_id_content)
+
+        id_index_content = id_index_txt()
+        id_index_tmpfile = _make_temp_file(id_index_content)
+        derived_id_index_tmpfile = _make_temp_file(id_index_content)
+        download_id_index_tmpfile = _make_temp_file(id_index_content)
+        
+        id_abstracts_content = id_index_txt(with_abstracts=True)
+        id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
+        derived_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
+        download_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
+        
+        all_id2_content = all_id2_txt()
+        all_id2_tmpfile = _make_temp_file(all_id2_content)
+        derived_all_id2_tmpfile = _make_temp_file(all_id2_content)
+        
+        # Move temp files as-atomically-as-possible into place
+        _move_into_place(all_id_tmpfile, id_path / "all_id.txt")
+        _move_into_place(derived_all_id_tmpfile, derived_path / "all_id.txt")
+        _move_into_place(download_all_id_tmpfile, download_path / "id-all.txt")
+        
+        _move_into_place(id_index_tmpfile, id_path / "1id-index.txt")
+        _move_into_place(derived_id_index_tmpfile, derived_path / "1id-index.txt")
+        _move_into_place(download_id_index_tmpfile, download_path / "id-index.txt")
+        
+        _move_into_place(id_abstracts_tmpfile, id_path / "1id-abstracts.txt")
+        _move_into_place(derived_id_abstracts_tmpfile, derived_path / "1id-abstracts.txt")
+        _move_into_place(download_id_abstracts_tmpfile, download_path / "id-abstract.txt")
+        
+        _move_into_place(all_id2_tmpfile, id_path / "all_id2.txt")
+        _move_into_place(derived_all_id2_tmpfile, derived_path / "all_id2.txt")
+    finally:
+        _cleanup()

From db51bd1793ba457b68daea8322b62d7d573a439d Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 13:13:54 -0400
Subject: [PATCH 14/32] chore: Add comments to bin/hourly

---
 bin/hourly | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/bin/hourly b/bin/hourly
index 77310302ce..a89d22901c 100755
--- a/bin/hourly
+++ b/bin/hourly
@@ -45,6 +45,7 @@ ID=/a/ietfdata/doc/draft/repository
 DERIVED=/a/ietfdata/derived
 DOWNLOAD=/a/www/www6s/download
 
+## Start of script refactored into idindex_update_task() ===
 export TMPDIR=/a/tmp
 
 TMPFILE1=`mktemp` || exit 1
@@ -85,6 +86,8 @@ mv $TMPFILE9 $DERIVED/1id-index.txt
 mv $TMPFILEA $DERIVED/1id-abstracts.txt
 mv $TMPFILEB $DERIVED/all_id2.txt
 
+## End of script refactored into idindex_update_task() ===
+
 $DTDIR/ietf/manage.py generate_idnits2_rfc_status
 $DTDIR/ietf/manage.py generate_idnits2_rfcs_obsoleted
 

From 9cc66b4af595e7432af5c986358b18ff8b21c2c5 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 13:18:59 -0400
Subject: [PATCH 15/32] fix: annotate types and fix bug

---
 ietf/idindex/tasks.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
index 9934cfcbe7..356c4edcbb 100644
--- a/ietf/idindex/tasks.py
+++ b/ietf/idindex/tasks.py
@@ -8,25 +8,25 @@
 from pathlib import Path
 from tempfile import NamedTemporaryFile
 
-
 from .index import all_id_txt, all_id2_txt, id_index_txt
 
+
 @shared_task
 def idindex_update_task():
     """Update I-D indexes"""
-    cleanup_list = set()
-    
+    cleanup_list: set[Path] = set()
+
     def _make_temp_file(content):
         with NamedTemporaryFile(delete=False, dir="/a/tmp") as tf:
-            cleanup_list.add(tf)
             tf_path = Path(tf.name)
-            tf_path.chmod(0o644)
+            cleanup_list.add(tf_path)
             tf.write(content)
         return tf_path
 
-    def _move_into_place(tf_path, dest):
-        shutil.move(tf_path, dest)
-        cleanup_list.remove(tf_path)
+    def _move_into_place(src_path: Path, dest_path: Path):
+        shutil.move(src_path, dest_path)
+        dest_path.chmod(0o644)
+        cleanup_list.remove(src_path)
 
     def _cleanup():
         for tf_path in cleanup_list:
@@ -48,29 +48,29 @@ def _cleanup():
         id_index_tmpfile = _make_temp_file(id_index_content)
         derived_id_index_tmpfile = _make_temp_file(id_index_content)
         download_id_index_tmpfile = _make_temp_file(id_index_content)
-        
+
         id_abstracts_content = id_index_txt(with_abstracts=True)
         id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
         derived_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
         download_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
-        
+
         all_id2_content = all_id2_txt()
         all_id2_tmpfile = _make_temp_file(all_id2_content)
         derived_all_id2_tmpfile = _make_temp_file(all_id2_content)
-        
+
         # Move temp files as-atomically-as-possible into place
         _move_into_place(all_id_tmpfile, id_path / "all_id.txt")
         _move_into_place(derived_all_id_tmpfile, derived_path / "all_id.txt")
         _move_into_place(download_all_id_tmpfile, download_path / "id-all.txt")
-        
+
         _move_into_place(id_index_tmpfile, id_path / "1id-index.txt")
         _move_into_place(derived_id_index_tmpfile, derived_path / "1id-index.txt")
         _move_into_place(download_id_index_tmpfile, download_path / "id-index.txt")
-        
+
         _move_into_place(id_abstracts_tmpfile, id_path / "1id-abstracts.txt")
         _move_into_place(derived_id_abstracts_tmpfile, derived_path / "1id-abstracts.txt")
         _move_into_place(download_id_abstracts_tmpfile, download_path / "id-abstract.txt")
-        
+
         _move_into_place(all_id2_tmpfile, id_path / "all_id2.txt")
         _move_into_place(derived_all_id2_tmpfile, derived_path / "all_id2.txt")
     finally:

From 6aa4426ea65879f3ac60d09c61f59c3b1698f6d7 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 13:21:05 -0400
Subject: [PATCH 16/32] feat: Create PeriodicTask for idindex_update_task

---
 ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index 51eaa1179e..c86e43ba04 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -133,6 +133,16 @@ def create_default_tasks(self):
             ),
         )
 
+        PeriodicTask.objects.get_or_create(
+            name="Update I-D index files",
+            task="ietf.idindex.tasks.idindex_update_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["hourly"],
+                description="Update I-D index files",
+            ),
+        )
+
     def show_tasks(self):
         for label, crontab in self.crontabs.items():
             tasks = PeriodicTask.objects.filter(crontab=crontab).order_by(

From 2465906a54912e0742048868ad16d80acf4a85bf Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 13:33:19 -0400
Subject: [PATCH 17/32] refactor: Move helpers into a class

More testable this way
---
 ietf/idindex/tasks.py | 84 ++++++++++++++++++++++++-------------------
 ietf/idindex/tests.py |  6 ++++
 2 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
index 356c4edcbb..e410d0d964 100644
--- a/ietf/idindex/tasks.py
+++ b/ietf/idindex/tasks.py
@@ -5,73 +5,83 @@
 import shutil
 
 from celery import shared_task
+from contextlib import contextmanager
 from pathlib import Path
 from tempfile import NamedTemporaryFile
 
 from .index import all_id_txt, all_id2_txt, id_index_txt
 
 
-@shared_task
-def idindex_update_task():
-    """Update I-D indexes"""
-    cleanup_list: set[Path] = set()
+class TempFileManager:
+    def __init__(self, tmpdir=None):
+        self.cleanup_list: set[Path] = set()
+        self.dir = tmpdir
 
-    def _make_temp_file(content):
-        with NamedTemporaryFile(delete=False, dir="/a/tmp") as tf:
+    def make_temp_file(self, content):
+        with NamedTemporaryFile(delete=False, dir=self.dir) as tf:
             tf_path = Path(tf.name)
-            cleanup_list.add(tf_path)
+            self.cleanup_list.add(tf_path)
             tf.write(content)
         return tf_path
 
-    def _move_into_place(src_path: Path, dest_path: Path):
+    def move_into_place(self, src_path: Path, dest_path: Path):
         shutil.move(src_path, dest_path)
         dest_path.chmod(0o644)
-        cleanup_list.remove(src_path)
+        self.cleanup_list.remove(src_path)
 
-    def _cleanup():
-        for tf_path in cleanup_list:
+    def cleanup(self):
+        for tf_path in self.cleanup_list:
             tf_path.unlink(missing_ok=True)
 
+@contextmanager
+def get_tempfile_manager(tmpdir):
+    mgr = TempFileManager(tmpdir)
+    try:
+        yield mgr
+    finally:
+        mgr.cleanup()
+
+
+@shared_task
+def idindex_update_task():
+    """Update I-D indexes"""
     id_path = Path("/a/ietfdata/doc/draft/repository")
     derived_path = Path("/a/ietfdata/derived")
     download_path = Path("/a/www/www6s/download")
 
-    # all_id
-    try:
+    with get_tempfile_manager("/a/tmp") as tmp_mgr:
         # Generate copies of new contents
         all_id_content = all_id_txt()
-        all_id_tmpfile = _make_temp_file(all_id_content)
-        derived_all_id_tmpfile = _make_temp_file(all_id_content)
-        download_all_id_tmpfile = _make_temp_file(all_id_content)
+        all_id_tmpfile = tmp_mgr.make_temp_file(all_id_content)
+        derived_all_id_tmpfile = tmp_mgr.make_temp_file(all_id_content)
+        download_all_id_tmpfile = tmp_mgr.make_temp_file(all_id_content)
 
         id_index_content = id_index_txt()
-        id_index_tmpfile = _make_temp_file(id_index_content)
-        derived_id_index_tmpfile = _make_temp_file(id_index_content)
-        download_id_index_tmpfile = _make_temp_file(id_index_content)
+        id_index_tmpfile = tmp_mgr.make_temp_file(id_index_content)
+        derived_id_index_tmpfile = tmp_mgr.make_temp_file(id_index_content)
+        download_id_index_tmpfile = tmp_mgr.make_temp_file(id_index_content)
 
         id_abstracts_content = id_index_txt(with_abstracts=True)
-        id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
-        derived_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
-        download_id_abstracts_tmpfile = _make_temp_file(id_abstracts_content)
+        id_abstracts_tmpfile = tmp_mgr.make_temp_file(id_abstracts_content)
+        derived_id_abstracts_tmpfile = tmp_mgr.make_temp_file(id_abstracts_content)
+        download_id_abstracts_tmpfile = tmp_mgr.make_temp_file(id_abstracts_content)
 
         all_id2_content = all_id2_txt()
-        all_id2_tmpfile = _make_temp_file(all_id2_content)
-        derived_all_id2_tmpfile = _make_temp_file(all_id2_content)
+        all_id2_tmpfile = tmp_mgr.make_temp_file(all_id2_content)
+        derived_all_id2_tmpfile = tmp_mgr.make_temp_file(all_id2_content)
 
         # Move temp files as-atomically-as-possible into place
-        _move_into_place(all_id_tmpfile, id_path / "all_id.txt")
-        _move_into_place(derived_all_id_tmpfile, derived_path / "all_id.txt")
-        _move_into_place(download_all_id_tmpfile, download_path / "id-all.txt")
+        tmp_mgr.move_into_place(all_id_tmpfile, id_path / "all_id.txt")
+        tmp_mgr.move_into_place(derived_all_id_tmpfile, derived_path / "all_id.txt")
+        tmp_mgr.move_into_place(download_all_id_tmpfile, download_path / "id-all.txt")
 
-        _move_into_place(id_index_tmpfile, id_path / "1id-index.txt")
-        _move_into_place(derived_id_index_tmpfile, derived_path / "1id-index.txt")
-        _move_into_place(download_id_index_tmpfile, download_path / "id-index.txt")
+        tmp_mgr.move_into_place(id_index_tmpfile, id_path / "1id-index.txt")
+        tmp_mgr.move_into_place(derived_id_index_tmpfile, derived_path / "1id-index.txt")
+        tmp_mgr.move_into_place(download_id_index_tmpfile, download_path / "id-index.txt")
 
-        _move_into_place(id_abstracts_tmpfile, id_path / "1id-abstracts.txt")
-        _move_into_place(derived_id_abstracts_tmpfile, derived_path / "1id-abstracts.txt")
-        _move_into_place(download_id_abstracts_tmpfile, download_path / "id-abstract.txt")
+        tmp_mgr.move_into_place(id_abstracts_tmpfile, id_path / "1id-abstracts.txt")
+        tmp_mgr.move_into_place(derived_id_abstracts_tmpfile, derived_path / "1id-abstracts.txt")
+        tmp_mgr.move_into_place(download_id_abstracts_tmpfile, download_path / "id-abstract.txt")
 
-        _move_into_place(all_id2_tmpfile, id_path / "all_id2.txt")
-        _move_into_place(derived_all_id2_tmpfile, derived_path / "all_id2.txt")
-    finally:
-        _cleanup()
+        tmp_mgr.move_into_place(all_id2_tmpfile, id_path / "all_id2.txt")
+        tmp_mgr.move_into_place(derived_all_id2_tmpfile, derived_path / "all_id2.txt")
diff --git a/ietf/idindex/tests.py b/ietf/idindex/tests.py
index c558783789..e62e8947c1 100644
--- a/ietf/idindex/tests.py
+++ b/ietf/idindex/tests.py
@@ -16,6 +16,7 @@
 from ietf.group.factories import GroupFactory
 from ietf.name.models import DocRelationshipName
 from ietf.idindex.index import all_id_txt, all_id2_txt, id_index_txt
+from ietf.idindex.tasks import idindex_update_task
 from ietf.person.factories import PersonFactory, EmailFactory
 from ietf.utils.test_utils import TestCase
 
@@ -151,3 +152,8 @@ def test_id_index_txt(self):
         txt = id_index_txt(with_abstracts=True)
 
         self.assertTrue(draft.abstract[:20] in txt)
+
+
+class TaskTests(TestCase):
+    def test_idindex_update_task(self):
+        

From 49cf0587e5a75d8748d4ae0e57feb3720621cac5 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 15:45:12 -0400
Subject: [PATCH 18/32] refactor: Make TempFileManager a context mgr

---
 ietf/idindex/tasks.py | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
index e410d0d964..23850ad23d 100644
--- a/ietf/idindex/tasks.py
+++ b/ietf/idindex/tasks.py
@@ -5,14 +5,14 @@
 import shutil
 
 from celery import shared_task
-from contextlib import contextmanager
+from contextlib import AbstractContextManager
 from pathlib import Path
 from tempfile import NamedTemporaryFile
 
 from .index import all_id_txt, all_id2_txt, id_index_txt
 
 
-class TempFileManager:
+class TempFileManager(AbstractContextManager):
     def __init__(self, tmpdir=None):
         self.cleanup_list: set[Path] = set()
         self.dir = tmpdir
@@ -33,13 +33,11 @@ def cleanup(self):
         for tf_path in self.cleanup_list:
             tf_path.unlink(missing_ok=True)
 
-@contextmanager
-def get_tempfile_manager(tmpdir):
-    mgr = TempFileManager(tmpdir)
-    try:
-        yield mgr
-    finally:
-        mgr.cleanup()
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        if exc_type is not None:
+            # exception occurred
+            self.cleanup()
+        return False  # False: do not suppress the exception
 
 
 @shared_task
@@ -49,7 +47,7 @@ def idindex_update_task():
     derived_path = Path("/a/ietfdata/derived")
     download_path = Path("/a/www/www6s/download")
 
-    with get_tempfile_manager("/a/tmp") as tmp_mgr:
+    with TempFileManager("/a/tmp") as tmp_mgr:
         # Generate copies of new contents
         all_id_content = all_id_txt()
         all_id_tmpfile = tmp_mgr.make_temp_file(all_id_content)

From 19295d648a040fa8794b7208c52af1cabf6c83ff Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 16:07:23 -0400
Subject: [PATCH 19/32] test: Test idindex_update_task

---
 ietf/idindex/tests.py | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/ietf/idindex/tests.py b/ietf/idindex/tests.py
index e62e8947c1..130acf93d9 100644
--- a/ietf/idindex/tests.py
+++ b/ietf/idindex/tests.py
@@ -3,6 +3,7 @@
 
 
 import datetime
+import mock
 
 from pathlib import Path
 
@@ -16,7 +17,7 @@
 from ietf.group.factories import GroupFactory
 from ietf.name.models import DocRelationshipName
 from ietf.idindex.index import all_id_txt, all_id2_txt, id_index_txt
-from ietf.idindex.tasks import idindex_update_task
+from ietf.idindex.tasks import idindex_update_task, TempFileManager
 from ietf.person.factories import PersonFactory, EmailFactory
 from ietf.utils.test_utils import TestCase
 
@@ -155,5 +156,32 @@ def test_id_index_txt(self):
 
 
 class TaskTests(TestCase):
-    def test_idindex_update_task(self):
+    @mock.patch("ietf.idindex.tasks.all_id_txt")
+    @mock.patch("ietf.idindex.tasks.all_id2_txt")
+    @mock.patch("ietf.idindex.tasks.id_index_txt")
+    @mock.patch.object(TempFileManager, "__enter__")
+    def test_idindex_update_task(
+        self,
+        temp_file_mgr_enter_mock,
+        id_index_mock,
+        all_id2_mock,
+        all_id_mock,
+    ):
+        # Replace TempFileManager's __enter__() method with one that returns a mock.
+        # Pass a spec to the mock so we validate that only actual methods are called.
+        mgr_mock = mock.Mock(spec=TempFileManager)
+        temp_file_mgr_enter_mock.return_value = mgr_mock
+        
+        idindex_update_task()
+
+        self.assertEqual(all_id_mock.call_count, 1)
+        self.assertEqual(all_id2_mock.call_count, 1)
+        self.assertEqual(id_index_mock.call_count, 2)
+        self.assertEqual(id_index_mock.call_args_list[0], (tuple(), dict()))
+        self.assertEqual(
+            id_index_mock.call_args_list[1], 
+            (tuple(), {"with_abstracts": True}),
+        )
+        self.assertEqual(mgr_mock.make_temp_file.call_count, 11)
+        self.assertEqual(mgr_mock.move_into_place.call_count, 11)
         

From a4d69b4893f019fb60b2d8491c05ff4187f6e68f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 16:38:16 -0400
Subject: [PATCH 20/32] test: Test TempFileManager

---
 ietf/idindex/tests.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/ietf/idindex/tests.py b/ietf/idindex/tests.py
index 130acf93d9..31c3aaafbf 100644
--- a/ietf/idindex/tests.py
+++ b/ietf/idindex/tests.py
@@ -6,6 +6,7 @@
 import mock
 
 from pathlib import Path
+from tempfile import TemporaryDirectory
 
 from django.conf import settings
 from django.utils import timezone
@@ -184,4 +185,20 @@ def test_idindex_update_task(
         )
         self.assertEqual(mgr_mock.make_temp_file.call_count, 11)
         self.assertEqual(mgr_mock.move_into_place.call_count, 11)
-        
+
+    def test_temp_file_manager(self):
+        with TemporaryDirectory() as temp_dir:
+            temp_path = Path(temp_dir)
+            with TempFileManager(temp_path) as tfm:
+                path1 = tfm.make_temp_file("yay")
+                path2 = tfm.make_temp_file("boo")  # do not keep this one
+                self.assertTrue(path1.exists())
+                self.assertTrue(path2.exists())
+                dest = temp_path / "yay.txt"
+                tfm.move_into_place(path1, dest)
+            # make sure things were cleaned up...
+            self.assertFalse(path1.exists())  # moved to dest
+            self.assertFalse(path2.exists())  # left behind
+            # check destination contents and permissions
+            self.assertEqual(dest.read_text(), "yay")
+            self.assertEqual(dest.stat().st_mode & 0o777, 0o644)

From 4009301f236e430e3a83e5fa0fa72f0b098fb628 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 16:38:28 -0400
Subject: [PATCH 21/32] fix: Fix bug in TestFileManager

yay testing
---
 ietf/idindex/tasks.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
index 23850ad23d..3cac33a90b 100644
--- a/ietf/idindex/tasks.py
+++ b/ietf/idindex/tasks.py
@@ -4,6 +4,8 @@
 #
 import shutil
 
+import debug    # pyflakes:ignore
+
 from celery import shared_task
 from contextlib import AbstractContextManager
 from pathlib import Path
@@ -18,7 +20,7 @@ def __init__(self, tmpdir=None):
         self.dir = tmpdir
 
     def make_temp_file(self, content):
-        with NamedTemporaryFile(delete=False, dir=self.dir) as tf:
+        with NamedTemporaryFile(mode="wt", delete=False, dir=self.dir) as tf:
             tf_path = Path(tf.name)
             self.cleanup_list.add(tf_path)
             tf.write(content)
@@ -34,9 +36,7 @@ def cleanup(self):
             tf_path.unlink(missing_ok=True)
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        if exc_type is not None:
-            # exception occurred
-            self.cleanup()
+        self.cleanup()
         return False  # False: do not suppress the exception
 
 

From 5882faebc74932558b2462b5812720e199cae9d4 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 17:29:47 -0400
Subject: [PATCH 22/32] feat: Add expire_ids_task()

---
 ietf/doc/tasks.py | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 ietf/doc/tasks.py

diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py
new file mode 100644
index 0000000000..dc125590a1
--- /dev/null
+++ b/ietf/doc/tasks.py
@@ -0,0 +1,48 @@
+# Copyright The IETF Trust 2024, All Rights Reserved
+#
+# Celery task definitions
+#
+import datetime
+import debug  # pyflakes:ignore
+
+from celery import shared_task
+
+from ietf.utils import log
+from ietf.utils.timezone import datetime_today
+
+from .expire import (
+    in_draft_expire_freeze,
+    get_expired_drafts,
+    expirable_drafts,
+    send_expire_notice_for_draft,
+    expire_draft,
+    clean_up_draft_files,
+)
+from .models import Document
+
+
+@shared_task
+def expire_ids_task():
+    try:
+        if not in_draft_expire_freeze():
+            log.log("Expiring drafts ...")
+            for doc in get_expired_drafts():
+                # verify expirability -- it might have changed after get_expired_drafts() was run
+                # (this whole loop took about 2 minutes on 04 Jan 2018)
+                # N.B., re-running expirable_drafts() repeatedly is fairly expensive. Where possible,
+                # it's much faster to run it once on a superset query of the objects you are going
+                # to test and keep its results. That's not desirable here because it would defeat
+                # the purpose of double-checking that a document is still expirable when it is actually
+                # being marked as expired.
+                if expirable_drafts(
+                    Document.objects.filter(pk=doc.pk)
+                ).exists() and doc.expires < datetime_today() + datetime.timedelta(1):
+                    send_expire_notice_for_draft(doc)
+                    expire_draft(doc)
+                    log.log(f"  Expired draft {doc.name}-{doc.rev}")
+
+        log.log("Cleaning up draft files")
+        clean_up_draft_files()
+    except Exception as e:
+        log.log("Exception in expire-ids: %s" % e)
+        raise

From 900489ed103c550b741bb082fd399210cd07b3ee Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 17:31:40 -0400
Subject: [PATCH 23/32] feat: Create PeriodicTask for expire_ids_task

---
 ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index c86e43ba04..2360c84d9b 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -113,6 +113,16 @@ def create_default_tasks(self):
             ),
         )
 
+        PeriodicTask.objects.get_or_create(
+            name="Expire I-Ds",
+            task="ietf.doc.tasks.expire_ids_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["daily"],
+                description="Create expiration notices for expired I-Ds",
+            ),
+        )
+
         PeriodicTask.objects.get_or_create(
             name="Sync with IANA changes",
             task="ietf.sync.tasks.iana_changes_update_task",

From a34785f10b6c7b366fc34af1a04387778a0a0784 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 17:48:55 -0400
Subject: [PATCH 24/32] test: Test expire_ids_task

---
 ietf/doc/tests_tasks.py | 54 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 ietf/doc/tests_tasks.py

diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py
new file mode 100644
index 0000000000..634c10b25a
--- /dev/null
+++ b/ietf/doc/tests_tasks.py
@@ -0,0 +1,54 @@
+# Copyright The IETF Trust 2024, All Rights Reserved
+import mock
+
+from ietf.utils.test_utils import TestCase
+from ietf.utils.timezone import datetime_today
+
+from .factories import DocumentFactory
+from .models import Document
+from .tasks import expire_ids_task
+
+
+class TaskTests(TestCase):
+
+    @mock.patch("ietf.doc.tasks.in_draft_expire_freeze")
+    @mock.patch("ietf.doc.tasks.get_expired_drafts")
+    @mock.patch("ietf.doc.tasks.expirable_drafts")
+    @mock.patch("ietf.doc.tasks.send_expire_notice_for_draft")
+    @mock.patch("ietf.doc.tasks.expire_draft")
+    @mock.patch("ietf.doc.tasks.clean_up_draft_files")
+    def test_expire_ids_task(
+        self,
+        clean_up_draft_files_mock,
+        expire_draft_mock,
+        send_expire_notice_for_draft_mock,
+        expirable_drafts_mock,
+        get_expired_drafts_mock,
+        in_draft_expire_freeze_mock,
+    ):
+        # set up mocks
+        in_draft_expire_freeze_mock.return_value = False
+        doc, other_doc = DocumentFactory.create_batch(2)
+        doc.expires = datetime_today()
+        get_expired_drafts_mock.return_value = [doc, other_doc]
+        expirable_drafts_mock.side_effect = [
+            Document.objects.filter(pk=doc.pk),
+            Document.objects.filter(pk=other_doc.pk),
+        ]
+        
+        # call task
+        expire_ids_task()
+        
+        # check results
+        self.assertTrue(in_draft_expire_freeze_mock.called)
+        self.assertEqual(expirable_drafts_mock.call_count, 2)
+        self.assertEqual(send_expire_notice_for_draft_mock.call_count, 1)
+        self.assertEqual(send_expire_notice_for_draft_mock.call_args[0], (doc,))
+        self.assertEqual(expire_draft_mock.call_count, 1)
+        self.assertEqual(expire_draft_mock.call_args[0], (doc,))
+        self.assertTrue(clean_up_draft_files_mock.called)
+
+        # test that an exception is raised
+        in_draft_expire_freeze_mock.side_effect = RuntimeError
+        with self.assertRaises(RuntimeError):(
+            expire_ids_task())

From 44ea308e9a595aa7a7b75e433951adb866da048f Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 19:59:08 -0400
Subject: [PATCH 25/32] test: Test request timeout in
 iana_protocols_update_task

---
 ietf/sync/tests.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py
index c4d8150661..fec353a97c 100644
--- a/ietf/sync/tests.py
+++ b/ietf/sync/tests.py
@@ -892,3 +892,14 @@ def test_iana_protocols_update_task(
             published_later_than, 
             {datetime.datetime(2012,11,26,tzinfo=datetime.timezone.utc)}
         )
+
+        # try with an exception
+        requests_get_mock.reset_mock()
+        parse_protocols_mock.reset_mock()
+        update_rfc_log_mock.reset_mock()
+        requests_get_mock.side_effect = requests.Timeout
+
+        tasks.iana_protocols_update_task()
+        self.assertTrue(requests_get_mock.called)
+        self.assertFalse(parse_protocols_mock.called)
+        self.assertFalse(update_rfc_log_mock.called)

From 961608476814f6233273ef7f8b238335e36af61b Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:01:58 -0400
Subject: [PATCH 26/32] refactor: do not re-raise timeout exception

Not sure this is the right thing to do, but it's the
same as rfc_editor_index_update_task
---
 ietf/sync/tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py
index 95277dfbd1..bc1218601f 100644
--- a/ietf/sync/tasks.py
+++ b/ietf/sync/tasks.py
@@ -133,7 +133,7 @@ def iana_protocols_update_task():
         )
     except requests.Timeout as exc:
         log.log(f'GET request timed out retrieving IANA protocols page: {exc}')
-        raise
+        return
 
     rfc_numbers = iana.parse_protocol_page(response.text)
 

From 1084d8bcc44b468af0111209d347a0adbff81b56 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:05:23 -0400
Subject: [PATCH 27/32] feat: Add notify_expirations_task

---
 ietf/doc/tasks.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py
index dc125590a1..a2e83e9e26 100644
--- a/ietf/doc/tasks.py
+++ b/ietf/doc/tasks.py
@@ -17,6 +17,8 @@
     send_expire_notice_for_draft,
     expire_draft,
     clean_up_draft_files,
+    get_soon_to_expire_drafts,
+    send_expire_warning_for_draft,
 )
 from .models import Document
 
@@ -46,3 +48,9 @@ def expire_ids_task():
     except Exception as e:
         log.log("Exception in expire-ids: %s" % e)
         raise
+
+
+@shared_task
+def notify_expirations_task(notify_days=14):
+    for doc in get_soon_to_expire_drafts(notify_days):
+        send_expire_warning_for_draft(doc)

From 3920b96edc554eb6f41e41a5e0d9567ebf2b8a5d Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:09:25 -0400
Subject: [PATCH 28/32] feat: Add "weekly" celery beat crontab

---
 ietf/utils/management/commands/periodic_tasks.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index 2360c84d9b..d6e55d5e6e 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -5,6 +5,14 @@
 from django.core.management.base import BaseCommand
 
 CRONTAB_DEFS = {
+    # same as "@weekly" in a crontab
+    "weekly": {
+        "minute": "0",
+        "hour": "0",
+        "day_of_week": "0",
+        "day_of_month": "*",
+        "month_of_year": "*",
+    },
     "daily": {
         "minute": "5",
         "hour": "0",

From 36794c148d0f971216b3f46e315565b3971d021e Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:10:19 -0400
Subject: [PATCH 29/32] refactor: Reorder crontab fields

This matches the crontab file field order
---
 ietf/utils/management/commands/periodic_tasks.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index d6e55d5e6e..c55845d0cb 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -9,30 +9,30 @@
     "weekly": {
         "minute": "0",
         "hour": "0",
-        "day_of_week": "0",
         "day_of_month": "*",
         "month_of_year": "*",
+        "day_of_week": "0",
     },
     "daily": {
         "minute": "5",
         "hour": "0",
-        "day_of_week": "*",
         "day_of_month": "*",
         "month_of_year": "*",
+        "day_of_week": "*",
     },
     "hourly": {
         "minute": "5",
         "hour": "*",
-        "day_of_week": "*",
         "day_of_month": "*",
         "month_of_year": "*",
+        "day_of_week": "*",
     },
     "every_15m": {
         "minute": "*/15",
         "hour": "*",
-        "day_of_week": "*",
         "day_of_month": "*",
         "month_of_year": "*",
+        "day_of_week": "*",
     },
 }
 

From 46f2ba4f41ed83ff41ce95588210d6c19a20b031 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:12:43 -0400
Subject: [PATCH 30/32] feat: Add PeriodicTask for notify_expirations

---
 ietf/utils/management/commands/periodic_tasks.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py
index c55845d0cb..14a21fe964 100644
--- a/ietf/utils/management/commands/periodic_tasks.py
+++ b/ietf/utils/management/commands/periodic_tasks.py
@@ -160,6 +160,16 @@ def create_default_tasks(self):
                 description="Update I-D index files",
             ),
         )
+        
+        PeriodicTask.objects.get_or_create(
+            name="Send expiration notifications",
+            task="ietf.doc.tasks.notify_expirations_task",
+            defaults=dict(
+                enabled=False,
+                crontab=self.crontabs["weekly"],
+                description="Send notifications about I-Ds that will expire in the next 14 days",
+            )
+        )
 
     def show_tasks(self):
         for label, crontab in self.crontabs.items():

From 7daa99ccaff09b311d15f533c4c9b61a22d75d11 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Thu, 25 Jan 2024 20:16:15 -0400
Subject: [PATCH 31/32] test: Test notify_expirations_task

---
 ietf/doc/tests_tasks.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py
index 634c10b25a..931ed438dc 100644
--- a/ietf/doc/tests_tasks.py
+++ b/ietf/doc/tests_tasks.py
@@ -6,7 +6,7 @@
 
 from .factories import DocumentFactory
 from .models import Document
-from .tasks import expire_ids_task
+from .tasks import expire_ids_task, notify_expirations_task
 
 
 class TaskTests(TestCase):
@@ -52,3 +52,12 @@ def test_expire_ids_task(
         in_draft_expire_freeze_mock.side_effect = RuntimeError
         with self.assertRaises(RuntimeError):(
             expire_ids_task())
+
+    @mock.patch("ietf.doc.tasks.send_expire_warning_for_draft")
+    @mock.patch("ietf.doc.tasks.get_soon_to_expire_drafts")
+    def test_notify_expirations_task(self, get_drafts_mock, send_warning_mock):
+        # Set up mocks
+        get_drafts_mock.return_value = ["sentinel"]
+        notify_expirations_task()
+        self.assertEqual(send_warning_mock.call_count, 1)
+        self.assertEqual(send_warning_mock.call_args[0], ("sentinel",))

From 9fa518d357a74cc6f57031d67d1df2e544f0226e Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Fri, 26 Jan 2024 10:29:11 -0400
Subject: [PATCH 32/32] test: Add annotation to satisfy mypy

---
 ietf/idindex/tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ietf/idindex/tasks.py b/ietf/idindex/tasks.py
index 3cac33a90b..c01d50cf5d 100644
--- a/ietf/idindex/tasks.py
+++ b/ietf/idindex/tasks.py
@@ -15,7 +15,7 @@
 
 
 class TempFileManager(AbstractContextManager):
-    def __init__(self, tmpdir=None):
+    def __init__(self, tmpdir=None) -> None:
         self.cleanup_list: set[Path] = set()
         self.dir = tmpdir