From 91be593c824f2584974cf0491751b262bee30ff1 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 24 Feb 2023 14:26:53 -0400 Subject: [PATCH] fix: Use more robust filename/rev matching for meeting materials (#5192) * fix: Use more robust filename/rev matching for meeting materials * chore: Remove debug statement * test: Update test to catch bug fixed by this PR * fix: Be more discerning when parsing revision * test: Update test to exercise revision parsing/validation * style: Apply Black styling to replaced code --- ietf/meeting/tests_views.py | 72 +++++++++++++++++++++++++++++++++---- ietf/meeting/views.py | 45 ++++++++++++----------- 2 files changed, 91 insertions(+), 26 deletions(-) diff --git a/ietf/meeting/tests_views.py b/ietf/meeting/tests_views.py index c24ed34f8..613694cbb 100644 --- a/ietf/meeting/tests_views.py +++ b/ietf/meeting/tests_views.py @@ -35,7 +35,7 @@ from django.utils.text import slugify import debug # pyflakes:ignore -from ietf.doc.models import Document +from ietf.doc.models import Document, NewRevisionDocEvent from ietf.group.models import Group, Role, GroupFeatures from ietf.group.utils import can_manage_group from ietf.person.models import Person @@ -607,13 +607,73 @@ class MeetingTests(BaseMeetingTestCase): @override_settings(MEETING_MATERIALS_SERVE_LOCALLY=True) def test_materials_name_endswith_hyphen_number_number(self): - sp = SessionPresentationFactory(document__name='slides-junk-15',document__type_id='slides',document__states=[('reuse_policy','single')]) - sp.document.uploaded_filename = '%s-%s.pdf'%(sp.document.name,sp.document.rev) + # be sure a shadowed filename without the hyphen does not interfere + shadow = SessionPresentationFactory( + document__name="slides-115-junk", + document__type_id="slides", + document__states=[("reuse_policy", "single")], + ) + shadow.document.uploaded_filename = ( + f"{shadow.document.name}-{shadow.document.rev}.pdf" + ) + shadow.document.save() + # create the material we want to find for the test + sp = SessionPresentationFactory( + document__name="slides-115-junk-15", + document__type_id="slides", + document__states=[("reuse_policy", "single")], + ) + sp.document.uploaded_filename = f"{sp.document.name}-{sp.document.rev}.pdf" sp.document.save() - self.write_materials_file(sp.session.meeting, sp.document, 'Fake slide contents') - url = urlreverse("ietf.meeting.views.materials_document", kwargs=dict(document=sp.document.name,num=sp.session.meeting.number)) + self.write_materials_file( + sp.session.meeting, sp.document, "Fake slide contents rev 00" + ) + + # create rev 01 + sp.document.rev = "01" + sp.document.uploaded_filename = f"{sp.document.name}-{sp.document.rev}.pdf" + sp.document.save_with_history( + [ + NewRevisionDocEvent.objects.create( + type="new_revision", + doc=sp.document, + rev=sp.document.rev, + by=Person.objects.get(name="(System)"), + desc=f"New version available: {sp.document.name}-{sp.document.rev}.txt", + ) + ] + ) + self.write_materials_file( + sp.session.meeting, sp.document, "Fake slide contents rev 01" + ) + url = urlreverse( + "ietf.meeting.views.materials_document", + kwargs=dict(document=sp.document.name, num=sp.session.meeting.number), + ) r = self.client.get(url) - self.assertEqual(r.status_code, 200) + self.assertContains( + r, + "Fake slide contents rev 01", + status_code=200, + msg_prefix="Should return latest rev by default", + ) + url = urlreverse( + "ietf.meeting.views.materials_document", + kwargs=dict(document=sp.document.name + "-00", num=sp.session.meeting.number), + ) + r = self.client.get(url) + self.assertContains( + r, + "Fake slide contents rev 00", + status_code=200, + msg_prefix="Should return existing version on request", + ) + url = urlreverse( + "ietf.meeting.views.materials_document", + kwargs=dict(document=sp.document.name + "-02", num=sp.session.meeting.number), + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 404, "Should not find nonexistent version") def test_important_dates(self): meeting=MeetingFactory(type_id='ietf') diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index 03d6368d5..44193ff9a 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -202,32 +202,37 @@ def current_materials(request): else: raise Http404('No such meeting') + +def _get_materials_doc(meeting, name): + """Get meeting materials document named by name + + Raises Document.DoesNotExist if a match cannot be found. + """ + # try an exact match first + doc = Document.objects.filter(name=name).first() + if doc is not None and doc.get_related_meeting() == meeting: + return doc, None + # try parsing a rev number + if "-" in name: + docname, rev = name.rsplit("-", 1) + if len(rev) == 2 and rev.isdigit(): + doc = Document.objects.get(name=docname) # may raise Document.DoesNotExist + if doc.get_related_meeting() == meeting and rev in doc.revisions(): + return doc, rev + # give up + raise Document.DoesNotExist + + @cache_page(1 * 60) def materials_document(request, document, num=None, ext=None): meeting=get_meeting(num,type_in=['ietf','interim']) num = meeting.number - if (re.search(r'^\w+-\d+-.+-\d\d$', document) or - re.search(r'^\w+-interim-\d+-.+-\d\d-\d\d$', document) or - re.search(r'^\w+-interim-\d+-.+-sess[a-z]-\d\d$', document) or - re.search(r'^(minutes|slides|chatlog|polls)-interim-\d+-.+-\d\d$', document)): - name, rev = document.rsplit('-', 1) - else: - name, rev = document, None # This view does not allow the use of DocAliases. Right now we are probably only creating one (identity) alias, but that may not hold in the future. - doc = Document.objects.filter(name=name).first() - # Handle edge case where the above name, rev splitter misidentifies the end of a document name as a revision number - if not doc: - if rev: - name = name + '-' + rev - rev = None - doc = get_object_or_404(Document, name=name) - else: - raise Http404("No such document") - - if not doc.meeting_related(): - raise Http404("Not a meeting related document") - if doc.get_related_meeting() != meeting: + try: + doc, rev = _get_materials_doc(meeting=meeting, name=document) + except Document.DoesNotExist: raise Http404("No such document for meeting %s" % num) + if not rev: filename = doc.get_file_name() else: