refactor: add guards/type hints to build_file_urls() (#4896)

* refactor: add guards/type hints to build_file_urls()

* fix: compare doc.type_id, not doc.type, with string value

* test: add cursory test of build_file_urls()

ietf/doc/tests_utils.py

@@ -5,6 +5,7 @@ import debug  # pyflakes:ignore
 from unittest.mock import patch
 
 from django.db import IntegrityError
+from django.test.utils import override_settings
 from django.utils import timezone
 
 from ietf.group.factories import GroupFactory, RoleFactory
@@ -15,7 +16,7 @@ from ietf.person.models import Person
 from ietf.doc.factories import DocumentFactory, WgRfcFactory, WgDraftFactory
 from ietf.doc.models import State, DocumentActionHolder, DocumentAuthor, Document
 from ietf.doc.utils import (update_action_holders, add_state_change_event, update_documentauthors,
-                            fuzzy_find_documents, rebuild_reference_relations)
+                            fuzzy_find_documents, rebuild_reference_relations, build_file_urls)
 from ietf.utils.draft import Draft, PlaintextDraft
 from ietf.utils.xmldraft import XMLDraft
 
@@ -294,6 +295,30 @@ class MiscTests(TestCase):
         self.do_fuzzy_find_documents_rfc_test('draft-name-that-has-two-02-04')
         self.do_fuzzy_find_documents_rfc_test('draft-wild-01-numbers-0312')
 
+    @override_settings(RFC_FILE_TYPES=['pdf'], IDSUBMIT_FILE_TYPES=['xml'])
+    @patch('ietf.doc.utils.os.path.exists', return_value=True)
+    def test_build_file_urls(self, mocked):
+        # a cursory test only - does not check details of how URLs are constructed
+        self.assertEqual(
+            build_file_urls(DocumentFactory(type_id='statchg')), ([], []),
+            'Non-draft Document should return empty sets'
+        )
+
+        with self.assertRaises(AssertionError):
+            build_file_urls(WgDraftFactory(rev=''))
+
+        urls, types = build_file_urls(WgDraftFactory(rev='23'))
+        self.assertEqual(['xml', 'bibtex'], [t for t, _ in urls])
+        self.assertEqual(types, ['xml'])
+
+        urls, types = build_file_urls(WgRfcFactory(rev=''))
+        self.assertEqual(['pdf', 'bibtex'], [t for t, _ in urls])
+        self.assertEqual(types, ['pdf'])
+
+        urls, types = build_file_urls(WgRfcFactory(rev='23'))
+        self.assertEqual(['pdf', 'bibtex'], [t for t, _ in urls])
+        self.assertEqual(types, ['pdf'])
+
 
 class RebuildReferenceRelationsTests(TestCase):
     def setUp(self):

ietf/doc/utils.py

@@ -12,6 +12,7 @@ import re
 import textwrap
 
 from collections import defaultdict, namedtuple
+from typing import Union
 from urllib.parse import quote
 from zoneinfo import ZoneInfo
 
@@ -995,8 +996,11 @@ def get_search_cache_key(params):
     key = "doc:document:search:" + hashlib.sha512(json.dumps(kwargs, sort_keys=True).encode('utf-8')).hexdigest()
     return key
     
-def build_file_urls(doc):
+def build_file_urls(doc: Union[Document, DocHistory]):
-    if isinstance(doc,Document) and doc.get_state_slug() == "rfc":
+    if doc.type_id != 'draft':
+        return [], []
+
+    if doc.get_state_slug() == "rfc":
         name = doc.canonical_name()
         base_path = os.path.join(settings.RFC_PATH, name + ".")
         possible_types = settings.RFC_FILE_TYPES
@@ -1017,7 +1021,7 @@ def build_file_urls(doc):
             if doc.tags.filter(slug="verified-errata").exists():
                 file_urls.append(("with errata", settings.RFC_EDITOR_INLINE_ERRATA_URL.format(rfc_number=doc.rfc_number())))
         file_urls.append(("bibtex", urlreverse('ietf.doc.views_doc.document_bibtex',kwargs=dict(name=name))))
-    else:
+    elif doc.rev:
         base_path = os.path.join(settings.INTERNET_ALL_DRAFTS_ARCHIVE_DIR, doc.name + "-" + doc.rev + ".")
         possible_types = settings.IDSUBMIT_FILE_TYPES
         found_types = [t for t in possible_types if os.path.exists(base_path + t)]
@@ -1031,6 +1035,12 @@ def build_file_urls(doc):
             file_urls.append(("htmlized", urlreverse('ietf.doc.views_doc.document_html', kwargs=dict(name=doc.name, rev=doc.rev))))
             file_urls.append(("pdfized", urlreverse('ietf.doc.views_doc.document_pdfized', kwargs=dict(name=doc.name, rev=doc.rev))))
         file_urls.append(("bibtex", urlreverse('ietf.doc.views_doc.document_bibtex',kwargs=dict(name=doc.name,rev=doc.rev))))
+    else:
+        # As of 2022-12-14, there are 1463 Document and 3136 DocHistory records with type='draft' and rev=''.
+        # All of these are in the rfc state and are covered by the above cases.
+        log.unreachable('2022-12-14')
+        file_urls = []
+        found_types = []
 
     return file_urls, found_types