Return rev used to find doc when heuristics modify the input. Share heuristics between rfcdiff and html views. Fixes ietf-tools#3437. Commit ready for merge.

 - Legacy-Id: 19658

Author: jennifer-richards

ietf/doc/factories.py

@@ -87,6 +87,19 @@ def relations(obj, create, extracted, **kwargs): # pylint: disable=no-self-argum
                     continue
                 obj.relateddocument_set.create(relationship_id=rel_id, target=docalias)
 
+    @factory.post_generation
+    def create_revisions(obj, create, extracted, **kwargs):  # pylint: disable=no-self-argument
+        """Create additional revisions of the document
+
+        Argument should be an iterable of revisions. Remember that range() is exclusive on the end
+        index, so range(1, 10) stops at 9.
+        """
+        if create and extracted:
+            for rev in extracted:
+                e = NewRevisionDocEventFactory(doc=obj, rev=f'{rev:02d}')
+                obj.rev = f'{rev:02d}'
+                obj.save_with_history([e])
+
     @classmethod
     def _after_postgeneration(cls, obj, create, results=None):
         """Save again the instance if creating and at least one hook ran."""

ietf/doc/tests.py

@@ -2486,113 +2486,141 @@ class RfcdiffSupportTests(TestCase):
     def setUp(self):
         super().setUp()
         self.target_view = 'ietf.doc.views_doc.rfcdiff_latest_json'
+        self._last_rfc_num = 8000
 
     def getJson(self, view_args):
         url = urlreverse(self.target_view, kwargs=view_args)
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
         return r.json()
 
-    def test_draft(self):
-        draft = IndividualDraftFactory(name='draft-somebody-did-something',rev='00')
-        for r in range(0,13):
-            e = NewRevisionDocEventFactory(doc=draft,rev=f'{r:02d}')
-            draft.rev = f'{r:02d}'
-            draft.save_with_history([e])
+    def next_rfc_number(self):
+        self._last_rfc_num += 1
+        return self._last_rfc_num
+
+    def do_draft_test(self, name):
+        draft = IndividualDraftFactory(name=name, rev='00', create_revisions=range(0,13))
         draft = reload_db_objects(draft)
 
         received = self.getJson(dict(name=draft.name))
-        self.assertEqual(received, dict(
-            name=draft.name,
-            rev=draft.rev,
-            content_url=draft.get_href(),
-            previous=f'{draft.name}-{(int(draft.rev)-1):02d}'
-        ))
+        self.assertEqual(
+            received,
+            dict(
+                name=draft.name,
+                rev=draft.rev,
+                content_url=draft.get_href(),
+                previous=f'{draft.name}-{(int(draft.rev)-1):02d}'
+            ),
+            'Incorrect JSON when draft revision not specified',
+        )
 
         received = self.getJson(dict(name=draft.name, rev=draft.rev))
-        self.assertEqual(received, dict(
-            name=draft.name,
-            rev=draft.rev,
-            content_url=draft.get_href(),
-            previous=f'{draft.name}-{(int(draft.rev)-1):02d}'
-        ))
+        self.assertEqual(
+            received,
+            dict(
+                name=draft.name,
+                rev=draft.rev,
+                content_url=draft.get_href(),
+                previous=f'{draft.name}-{(int(draft.rev)-1):02d}'
+            ),
+            'Incorrect JSON when latest revision specified',
+        )
 
         received = self.getJson(dict(name=draft.name, rev='10'))
-        self.assertEqual(received, dict(
-            name=draft.name,
-            rev='10',
-            content_url=draft.history_set.get(rev='10').get_href(),
-            previous=f'{draft.name}-09'
-        ))
+        self.assertEqual(
+            received,
+            dict(
+                name=draft.name,
+                rev='10',
+                content_url=draft.history_set.get(rev='10').get_href(),
+                previous=f'{draft.name}-09'
+            ),
+            'Incorrect JSON when historical revision specified',
+        )
 
         received = self.getJson(dict(name=draft.name, rev='00'))
-        self.assertNotIn('previous', received)
+        self.assertNotIn('previous', received, 'Rev 00 has no previous name when not replacing a draft')
 
         replaced = IndividualDraftFactory()
         RelatedDocument.objects.create(relationship_id='replaces',source=draft,target=replaced.docalias.first())
         received = self.getJson(dict(name=draft.name, rev='00'))
-        self.assertEqual(received['previous'], f'{replaced.name}-{replaced.rev}')
-
+        self.assertEqual(received['previous'], f'{replaced.name}-{replaced.rev}',
+                         'Rev 00 has a previous name when replacing a draft')
 
-    def test_draft_with_broken_history(self):
-        draft = IndividualDraftFactory(rev='10')
+    def test_draft(self):
+        # test with typical, straightforward names
+        self.do_draft_test(name='draft-somebody-did-a-thing')
+        # try with different potentially problematic names
+        self.do_draft_test(name='draft-someone-did-something-01-02')
+        self.do_draft_test(name='draft-someone-did-something-else-02')
+        self.do_draft_test(name='draft-someone-did-something-02-weird-01')
+
+    def do_draft_with_broken_history_test(self, name):
+        draft = IndividualDraftFactory(name=name, rev='10')
         received = self.getJson(dict(name=draft.name,rev='09'))
         self.assertEqual(received['rev'],'09')
         self.assertEqual(received['previous'], f'{draft.name}-08')
         self.assertTrue('warning' in received)
 
-
-    def test_draftname_with_numeric_suffix(self):
-        draft = IndividualDraftFactory(name='draft-someone-did-something-01-02',rev='00')
-        for r in range(0,4):
-            e = NewRevisionDocEventFactory(doc=draft,rev=f'{r:02d}')
-            draft.rev = f'{r:02d}'
-            draft.save_with_history([e])
-
-        received = self.getJson(dict(name=draft.name))
-        self.assertEqual(received['rev'],'03')
-        self.assertIn('01-02-03',received['content_url'])
-        self.assertIn('01-02-02',received['previous'])
-
-        received = self.getJson(dict(name=draft.name,rev='02'))
-        self.assertEqual(received['rev'],'02')
-        self.assertIn('01-02-02',received['content_url'])
-
-    def test_rfc(self):
-        draft = WgDraftFactory()
-        for r in range(0,2):
-            e = NewRevisionDocEventFactory(doc=draft,rev=f'{r:02d}')
-            draft.rev = f'{r:02d}'
-            draft.save_with_history([e])
-
-        draft.docalias.create(name='rfc8000')
+    def test_draft_with_broken_history(self):
+        # test with typical, straightforward names
+        self.do_draft_with_broken_history_test(name='draft-somebody-did-something')
+        # try with different potentially problematic names
+        self.do_draft_with_broken_history_test(name='draft-someone-did-something-01-02')
+        self.do_draft_with_broken_history_test(name='draft-someone-did-something-else-02')
+        self.do_draft_with_broken_history_test(name='draft-someone-did-something-02-weird-03')
+
+    def do_rfc_test(self, draft_name):
+        draft = WgDraftFactory(name=draft_name, create_revisions=range(0,2))
+        draft.docalias.create(name=f'rfc{self.next_rfc_number():04}')
         draft.set_state(State.objects.get(type_id='draft',slug='rfc'))
         draft.set_state(State.objects.get(type_id='draft-iesg', slug='pub'))
         draft = reload_db_objects(draft)
         rfc = draft
 
         number = rfc.rfc_number()
         received = self.getJson(dict(name=number))
-        self.assertEqual(received, dict(
-            content_url=rfc.get_href(),
-            name=rfc.canonical_name(),
-            previous=f'{draft.name}-{draft.rev}',
-        ))
+        self.assertEqual(
+            received,
+            dict(
+                content_url=rfc.get_href(),
+                name=rfc.canonical_name(),
+                previous=f'{draft.name}-{draft.rev}',
+            ),
+            'Can look up an RFC by number',
+        )
 
         num_received = received
         received = self.getJson(dict(name=rfc.canonical_name()))
-        self.assertEqual(num_received, received)
+        self.assertEqual(num_received, received, 'RFC by canonical name gives same result as by number')
 
         received = self.getJson(dict(name=f'RfC {number}'))
-        self.assertEqual(num_received, received)
+        self.assertEqual(num_received, received, 'RFC with unusual spacing/caps gives same result as by number')
 
-    def test_rfc_with_tombstone(self):
-        draft = WgDraftFactory()
-        for r in range(0,2):
-            e = NewRevisionDocEventFactory(doc=draft,rev=f'{r:02d}')
-            draft.rev = f'{r:02d}'
-            draft.save_with_history([e])
+        received = self.getJson(dict(name=draft.name))
+        self.assertEqual(num_received, received, 'RFC by draft name and no rev gives same result as by number')
+
+        received = self.getJson(dict(name=draft.name, rev='01'))
+        self.assertEqual(
+            received,
+            dict(
+                content_url=draft.history_set.get(rev='01').get_href(),
+                name=draft.name,
+                rev='01',
+                previous=f'{draft.name}-00',
+            ),
+            'RFC by draft name with rev should give draft name, not canonical name'
+        )
 
+    def test_rfc(self):
+        # simple draft name
+        self.do_rfc_test(draft_name='draft-test-ar-ef-see')
+        # tricky draft names
+        self.do_rfc_test(draft_name='draft-whatever-02')
+        self.do_rfc_test(draft_name='draft-test-me-03-04')
+
+    def test_rfc_with_tombstone(self):
+        draft = WgDraftFactory(create_revisions=range(0,2))
         draft.docalias.create(name='rfc3261') # See views_doc.HAS_TOMBSTONE
         draft.set_state(State.objects.get(type_id='draft',slug='rfc'))
         draft.set_state(State.objects.get(type_id='draft-iesg', slug='pub'))
@@ -2603,4 +2631,46 @@ def test_rfc_with_tombstone(self):
         received = self.getJson(dict(name=rfc.canonical_name()))
         self.assertTrue(received['previous'].endswith('00'))
 
+    def do_rfc_with_broken_history_test(self, draft_name):
+        draft = WgDraftFactory(rev='10', name=draft_name)
+        draft.docalias.create(name=f'rfc{self.next_rfc_number():04}')
+        draft.set_state(State.objects.get(type_id='draft',slug='rfc'))
+        draft.set_state(State.objects.get(type_id='draft-iesg', slug='pub'))
+        draft = reload_db_objects(draft)
+        rfc = draft
+
+        received = self.getJson(dict(name=draft.name))
+        self.assertEqual(
+            received,
+            dict(
+                content_url=rfc.get_href(),
+                name=rfc.canonical_name(),
+                previous=f'{draft.name}-10',
+            ),
+            'RFC by draft name without rev should return canonical RFC name and no rev',
+        )
+
+        received = self.getJson(dict(name=draft.name, rev='10'))
+        self.assertEqual(received['name'], draft.name, 'RFC by draft name with rev should return draft name')
+        self.assertEqual(received['rev'], '10', 'Requested rev should be returned')
+        self.assertEqual(received['previous'], f'{draft.name}-09', 'Previous rev is one less than requested')
+        self.assertIn(f'{draft.name}-10', received['content_url'], 'Returned URL should include requested rev')
+        self.assertNotIn('warning', received, 'No warning when we have the rev requested')
+
+        received = self.getJson(dict(name=f'{draft.name}-09'))
+        self.assertEqual(received['name'], draft.name, 'RFC by draft name with rev should return draft name')
+        self.assertEqual(received['rev'], '09', 'Requested rev should be returned')
+        self.assertEqual(received['previous'], f'{draft.name}-08', 'Previous rev is one less than requested')
+        self.assertIn(f'{draft.name}-09', received['content_url'], 'Returned URL should include requested rev')
+        self.assertEqual(
+            received['warning'],
+            'History for this version not found - these results are speculation',
+            'Warning should be issued when requested rev is not found'
+        )
 
+    def test_rfc_with_broken_history(self):
+        # simple draft name
+        self.do_rfc_with_broken_history_test(draft_name='draft-some-draft')
+        # tricky draft names
+        self.do_rfc_with_broken_history_test(draft_name='draft-gizmo-01')
+        self.do_rfc_with_broken_history_test(draft_name='draft-oh-boy-what-a-draft-02-03')

ietf/doc/tests_utils.py

@@ -8,9 +8,9 @@
 from ietf.person.factories import PersonFactory
 from ietf.utils.test_utils import TestCase
 from ietf.person.models import Person
-from ietf.doc.factories import DocumentFactory
-from ietf.doc.models import State, DocumentActionHolder, DocumentAuthor
-from ietf.doc.utils import update_action_holders, add_state_change_event, update_documentauthors
+from ietf.doc.factories import DocumentFactory, WgRfcFactory
+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
 
 
 class ActionHoldersTests(TestCase):
@@ -239,4 +239,49 @@ def test_update_documentauthors_with_nulls(self):
         self.assertIn('cleared country (was "USA")', events[0].desc)
         docauth = doc.documentauthor_set.first()
         self.assertEqual(docauth.affiliation, '')
-        self.assertEqual(docauth.country, '')
+        self.assertEqual(docauth.country, '')
+
+    def do_fuzzy_find_documents_rfc_test(self, name):
+        rfc = WgRfcFactory(name=name, create_revisions=(0, 1, 2))
+        rfc = Document.objects.get(pk=rfc.pk)  # clear out any cached values
+
+        # by canonical name
+        found = fuzzy_find_documents(rfc.canonical_name(), None)
+        self.assertCountEqual(found.documents, [rfc])
+        self.assertEqual(found.matched_rev, None)
+        self.assertEqual(found.matched_name, rfc.canonical_name())
+
+        # by draft name, no rev
+        found = fuzzy_find_documents(rfc.name, None)
+        self.assertCountEqual(found.documents, [rfc])
+        self.assertEqual(found.matched_rev, None)
+        self.assertEqual(found.matched_name, rfc.name)
+
+        # by draft name, latest rev
+        found = fuzzy_find_documents(rfc.name, '02')
+        self.assertCountEqual(found.documents, [rfc])
+        self.assertEqual(found.matched_rev, '02')
+        self.assertEqual(found.matched_name, rfc.name)
+
+        # by draft name, earlier rev
+        found = fuzzy_find_documents(rfc.name, '01')
+        self.assertCountEqual(found.documents, [rfc])
+        self.assertEqual(found.matched_rev, '01')
+        self.assertEqual(found.matched_name, rfc.name)
+
+        # wrong name or revision
+        found = fuzzy_find_documents(rfc.name + '-incorrect')
+        self.assertCountEqual(found.documents, [], 'Should not find document that does not match')
+        found = fuzzy_find_documents(rfc.name + '-incorrect', '02')
+        self.assertCountEqual(found.documents, [], 'Still should not find document, even with a version')
+        found = fuzzy_find_documents(rfc.name, '22')
+        self.assertCountEqual(found.documents, [rfc],
+                              'Should find document even if rev does not exist')
+
+
+    def test_fuzzy_find_documents(self):
+        # Should add additional tests/test cases for other document types/name formats
+        self.do_fuzzy_find_documents_rfc_test('draft-normal-name')
+        self.do_fuzzy_find_documents_rfc_test('draft-name-with-number-01')
+        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')

ietf/doc/utils.py

@@ -11,7 +11,7 @@
 import re
 import textwrap
 
-from collections import defaultdict
+from collections import defaultdict, namedtuple
 from urllib.parse import quote
 
 from django.conf import settings
@@ -1286,3 +1286,39 @@ def generate_idnits2_rfcs_obsoleted():
         obsdict[k] = sorted(obsdict[k])
     return render_to_string('doc/idnits2-rfcs-obsoleted.txt', context={'obsitems':sorted(obsdict.items())})
 
+
+def fuzzy_find_documents(name, rev=None):
+    """Find a document based on name/rev
+
+    Applies heuristics, assuming the inputs were joined by a '-' that may have been misplaced.
+    If returned documents queryset is empty, matched_rev and and matched_name are meaningless.
+    The rev input is not validated - it is used to find possible names if the name input does
+    not match anything, but matched_rev may not correspond to an actual version of the found
+    document.
+    """
+    # Handle special case name formats
+    if name.startswith('rfc0'):
+        name = "rfc" + name[3:].lstrip('0')
+    if name.startswith('review-') and re.search(r'-\d\d\d\d-\d\d$', name):
+        name = "%s-%s" % (name, rev)
+        rev = None
+    if rev and not name.startswith('charter-') and re.search('[0-9]{1,2}-[0-9]{2}', rev):
+        name = "%s-%s" % (name, rev[:-3])
+        rev = rev[-2:]
+    if re.match("^[0-9]+$", name):
+        name = f'rfc{name}'
+    if re.match("^[Rr][Ff][Cc] [0-9]+$",name):
+        name = f'rfc{name[4:]}'
+
+    # see if we can find a document using this name
+    docs = Document.objects.filter(docalias__name=name, type_id='draft')
+    if rev and not docs.exists():
+        # No document found, see if the name/rev split has been misidentified.
+        # Handles some special cases, like draft-ietf-tsvwg-ieee-802-11.
+        name = '%s-%s' % (name, rev)
+        docs = Document.objects.filter(docalias__name=name, type_id='draft')
+        if docs.exists():
+            rev = None  # found a doc by name with rev = None, so update that
+
+    FoundDocuments = namedtuple('FoundDocuments', 'documents matched_name matched_rev')
+    return FoundDocuments(docs, name, rev)