From cfea1ebe5b36d41398f3669f2fb7dccdff57bec3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 7 Apr 2023 15:37:52 -0300 Subject: [PATCH 01/32] fix: Use relative URL for submission status link --- ietf/submit/forms.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 5c0023a001..972696b89a 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -262,10 +262,7 @@ def cleanup(): # called when context exited, even in case of exception raise forms.ValidationError( format_html( 'A submission with same name and revision is currently being processed. Check the status here.', - urljoin( - settings.IDTRACKER_BASE_URL, - urlreverse("ietf.submit.views.submission_status", kwargs={'submission_id': existing[0].pk}), - ) + urlreverse("ietf.submit.views.submission_status", kwargs={'submission_id': existing[0].pk}), ) ) From 942657bc3d722239cff30351b2b4026649bd69ed Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 11 Apr 2023 13:17:42 -0300 Subject: [PATCH 02/32] refactor: Refactor/rename process_uploaded_submission async task --- ietf/submit/tasks.py | 6 ++--- ietf/submit/utils.py | 63 ++++++++++++++++++++++++++++++++------------ ietf/submit/views.py | 4 +-- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/ietf/submit/tasks.py b/ietf/submit/tasks.py index 21d4275b75..646b37e732 100644 --- a/ietf/submit/tasks.py +++ b/ietf/submit/tasks.py @@ -9,18 +9,18 @@ from django.utils import timezone from ietf.submit.models import Submission -from ietf.submit.utils import cancel_submission, create_submission_event, process_uploaded_submission +from ietf.submit.utils import cancel_submission, create_submission_event, process_and_accept_uploaded_submission from ietf.utils import log @shared_task -def process_uploaded_submission_task(submission_id): +def process_and_accept_uploaded_submission_task(submission_id): try: submission = Submission.objects.get(pk=submission_id) except Submission.DoesNotExist: log.log(f'process_uploaded_submission_task called for missing submission_id={submission_id}') else: - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) @shared_task diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 99d4e3ce36..6ebdb6efac 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1203,17 +1203,15 @@ def process_submission_text(submission): ) -def process_uploaded_submission(submission): - def abort_submission(error): - cancel_submission(submission) - create_submission_event(None, submission, f'Submission rejected: {error}') +def process_and_validate_submission(submission): + """Process and validate a submission - if submission.state_id != 'validating': - log.log(f'Submission {submission.pk} is not in "validating" state, skipping.') - return # do nothing + Limitation: Only works with XML submissions. Cannot handle additional formats. + Raises SubmissionError if an error is encountered. + """ if submission.file_types != '.xml': - abort_submission('Only XML Internet-Draft submissions can be processed.') + raise SubmissionError('Only XML Internet-Draft submissions can be processed.') try: process_submission_xml(submission) @@ -1235,16 +1233,47 @@ def abort_submission(error): errors = [c.message for c in submission.checks.filter(passed__isnull=False) if not c.passed] if len(errors) > 0: raise SubmissionError('Checks failed: ' + ' / '.join(errors)) - except SubmissionError as err: - abort_submission(err) except Exception: log.log(f'Unexpected exception while processing submission {submission.pk}.') log.log(traceback.format_exc()) - abort_submission('A system error occurred while processing the submission.') + raise SubmissionError('A system error occurred while processing the submission.') - # if we get here and are still "validating", accept the draft - if submission.state_id == 'validating': - submission.state_id = 'uploaded' - submission.save() - create_submission_event(None, submission, desc="Completed submission validation checks") - accept_submission(submission) + +def submitter_is_author(submission): + submitter = get_person_from_name_email(**submission.submitter_parsed()) # the ** expands dict into kwargs + if submitter: + author_emails = [author['email'].strip().lower() for author in submission.authors] + return any( + email.address.lower() in author_emails + for email in submitter.email_set.filter(active=True) + ) + return False + + +def process_and_accept_uploaded_submission(submission): + """Process, validate, and, if valid, accept an uploaded submission + + Requires that the submitter already be set and is an author of the submitted draft. + The submission must be in the "validating" state. On success, it will be in the + "posted" state. On error, it wil be in the "cancel" state. + """ + if submission.state_id != "validating": + log.log(f'Submission {submission.pk} is not in "validating" state, skipping.') + return # do nothing + + try: + process_and_validate_submission(submission) + except SubmissionError as err: + cancel_submission(submission) # changes Submission.state + create_submission_event(None, submission, f"Submission rejected: {err}") + + if not submitter_is_author(submission): + cancel_submission(submission) # changes Submission.state + create_submission_event( + None, + submission, + f"Submission rejected: Submitter ({submission.submitter}) is not one of the document authors", + ) + + create_submission_event(None, submission, desc="Completed submission validation checks") + accept_submission(submission) diff --git a/ietf/submit/views.py b/ietf/submit/views.py index 4aaafbeacc..c9388d3d0e 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -36,7 +36,7 @@ from ietf.submit.mail import send_full_url, send_manual_post_request, add_submission_email, get_reply_to from ietf.submit.models import (Submission, Preapproval, SubmissionExtResource, DraftSubmissionStateName, SubmissionEmailEvent ) -from ietf.submit.tasks import process_uploaded_submission_task, poke +from ietf.submit.tasks import process_and_accept_uploaded_submission_task, poke from ietf.submit.utils import ( approvable_submissions_for_user, preapprovals_for_user, recently_approved_by_user, validate_submission, create_submission_event, docevent_from_submission, post_submission, cancel_submission, rename_submission_files, remove_submission_files, get_draft_meta, @@ -173,7 +173,7 @@ def err(code, error, messages=None): # Wrap in on_commit so the delayed task cannot start until the view is done with the DB transaction.on_commit( - lambda: process_uploaded_submission_task.delay(submission.pk) + lambda: process_and_accept_uploaded_submission_task.delay(submission.pk) ) return JsonResponse( { From df173b6151d2ae82102ec96783735362168fd47f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 11 Apr 2023 13:20:00 -0300 Subject: [PATCH 03/32] feat: Add async task to process but not accept a submission --- ietf/submit/tasks.py | 13 ++++++++++++- ietf/submit/utils.py | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/ietf/submit/tasks.py b/ietf/submit/tasks.py index 646b37e732..382bff7fae 100644 --- a/ietf/submit/tasks.py +++ b/ietf/submit/tasks.py @@ -9,10 +9,21 @@ from django.utils import timezone from ietf.submit.models import Submission -from ietf.submit.utils import cancel_submission, create_submission_event, process_and_accept_uploaded_submission +from ietf.submit.utils import (cancel_submission, create_submission_event, process_uploaded_submission, + process_and_accept_uploaded_submission) from ietf.utils import log +@shared_task +def process_uploaded_submission_task(submission_id): + try: + submission = Submission.objects.get(pk=submission_id) + except Submission.DoesNotExist: + log.log(f'process_uploaded_submission_task called for missing submission_id={submission_id}') + else: + process_uploaded_submission(submission) + + @shared_task def process_and_accept_uploaded_submission_task(submission_id): try: diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 6ebdb6efac..efea28825d 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1277,3 +1277,23 @@ def process_and_accept_uploaded_submission(submission): create_submission_event(None, submission, desc="Completed submission validation checks") accept_submission(submission) + + +def process_uploaded_submission(submission): + """Process and validate an uploaded submission + + The submission must be in the "validating" state. On success, it will be in the "uploaded" + state. On error, it will be in the "cancel" state. + """ + if submission.state_id != "validating": + log.log(f'Submission {submission.pk} is not in "validating" state, skipping.') + return # do nothing + + try: + process_and_validate_submission(submission) + except SubmissionError as err: + cancel_submission(submission) # changes Submission.state + create_submission_event(None, submission, f"Submission rejected: {err}") + submission.state_id = "uploaded" + submission.save() + create_submission_event(None, submission, desc="Completed submission validation checks") From 5be633a65cbd12662191e9297a58de3910b9ec76 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 11 Apr 2023 13:23:44 -0300 Subject: [PATCH 04/32] feat: Replace upload_submission() with an async implementation (WIP) --- ietf/submit/forms.py | 25 +++++--- ietf/submit/views.py | 145 +++++++++++++++++-------------------------- 2 files changed, 71 insertions(+), 99 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 972696b89a..6eb1cc7e92 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -49,6 +49,15 @@ class SubmissionBaseUploadForm(forms.Form): xml = forms.FileField(label='.xml format', required=True) + # No code currently (14 Sep 2017) uses this class directly; it is + # only used through its subclasses. The two assignments below are + # set to trigger an exception if it is used directly only to make + # sure that adequate consideration is made if it is decided to use it + # directly in the future. Feel free to set these appropriately to + # avoid the exceptions in that case: + formats = None # None will raise an exception in clean() if this isn't changed in a subclass + base_formats = None # None will raise an exception in clean() if this isn't changed in a subclass + def __init__(self, request, *args, **kwargs): super(SubmissionBaseUploadForm, self).__init__(*args, **kwargs) @@ -70,14 +79,6 @@ def __init__(self, request, *args, **kwargs): self.file_types = [] self.file_info = {} # indexed by file field name, e.g., 'txt', 'xml', ... self.xml_version = None - # No code currently (14 Sep 2017) uses this class directly; it is - # only used through its subclasses. The two assignments below are - # set to trigger an exception if it is used directly only to make - # sure that adequate consideration is made if it is decided to use it - # directly in the future. Feel free to set these appropriately to - # avoid the exceptions in that case: - self.formats = None # None will raise an exception in clean() if this isn't changed in a subclass - self.base_formats = None # None will raise an exception in clean() if this isn't changed in a subclass def set_cutoff_warnings(self): now = timezone.now() @@ -610,7 +611,7 @@ def cleanup(): # called when context exited, even in case of exception return super().clean() -class SubmissionManualUploadForm(DeprecatedSubmissionBaseUploadForm): +class DeprecatedSubmissionManualUploadForm(DeprecatedSubmissionBaseUploadForm): xml = forms.FileField(label='.xml format', required=False) # xml field with required=False instead of True txt = forms.FileField(label='.txt format', required=False) # We won't permit html upload until we can verify that the content @@ -620,7 +621,7 @@ class SubmissionManualUploadForm(DeprecatedSubmissionBaseUploadForm): pdf = forms.FileField(label='.pdf format', required=False) def __init__(self, request, *args, **kwargs): - super(SubmissionManualUploadForm, self).__init__(request, *args, **kwargs) + super(DeprecatedSubmissionManualUploadForm, self).__init__(request, *args, **kwargs) self.formats = settings.IDSUBMIT_FILE_TYPES self.base_formats = ['txt', 'xml', ] @@ -639,6 +640,10 @@ def __init__(self, request, *args, **kwargs): self.formats = ['xml', ] self.base_formats = ['xml', ] +class SubmissionManualUploadForm(SubmissionBaseUploadForm): + formats = ('xml',) + base_formats = ('xml',) + class SubmissionAutoUploadForm(SubmissionBaseUploadForm): user = forms.EmailField(required=True) replaces = forms.CharField(required=False, max_length=1000, strip=True) diff --git a/ietf/submit/views.py b/ietf/submit/views.py index c9388d3d0e..68de2745c1 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -30,13 +30,13 @@ from ietf.mailtrigger.utils import gather_address_lists from ietf.message.models import Message, MessageAttachment from ietf.person.models import Email -from ietf.submit.forms import ( SubmissionManualUploadForm, SubmissionAutoUploadForm, AuthorForm, - SubmitterForm, EditSubmissionForm, PreapprovalForm, ReplacesForm, SubmissionEmailForm, MessageModelForm, - DeprecatedSubmissionAutoUploadForm ) +from ietf.submit.forms import (SubmissionAutoUploadForm, AuthorForm, SubmitterForm, EditSubmissionForm, + PreapprovalForm, ReplacesForm, SubmissionEmailForm, MessageModelForm, + DeprecatedSubmissionAutoUploadForm, SubmissionManualUploadForm) from ietf.submit.mail import send_full_url, send_manual_post_request, add_submission_email, get_reply_to from ietf.submit.models import (Submission, Preapproval, SubmissionExtResource, DraftSubmissionStateName, SubmissionEmailEvent ) -from ietf.submit.tasks import process_and_accept_uploaded_submission_task, poke +from ietf.submit.tasks import process_uploaded_submission_task, process_and_accept_uploaded_submission_task, poke from ietf.submit.utils import ( approvable_submissions_for_user, preapprovals_for_user, recently_approved_by_user, validate_submission, create_submission_event, docevent_from_submission, post_submission, cancel_submission, rename_submission_files, remove_submission_files, get_draft_meta, @@ -53,57 +53,23 @@ def upload_submission(request): if request.method == 'POST': - try: - form = SubmissionManualUploadForm(request, data=request.POST, files=request.FILES) - if form.is_valid(): - log('got valid submission form for %s' % form.filename) - saved_files = save_files(form) - authors, abstract, file_name, file_size = get_draft_meta(form, saved_files) - - submission = get_submission(form) - try: - fill_in_submission(form, submission, authors, abstract, file_size) - except Exception as e: - log("Exception: %s\n" % e) - if submission and submission.id: - submission.delete() - raise - - apply_checkers(submission, file_name) - - consistency_error = check_submission_revision_consistency(submission) - if consistency_error: - # A data consistency problem diverted this to manual processing - send notification - submission.state = DraftSubmissionStateName.objects.get(slug="manual") - submission.save() - create_submission_event(request, submission, desc="Uploaded submission (diverted to manual process)") - send_manual_post_request(request, submission, errors=dict(consistency=consistency_error)) - else: - # This is the usual case - create_submission_event(request, submission, desc="Uploaded submission") - - # Don't add an "Uploaded new revision doevent yet, in case of cancellation - - return redirect("ietf.submit.views.submission_status", submission_id=submission.pk, access_token=submission.access_token()) - except IOError as e: - if "read error" in str(e): # The server got an IOError when trying to read POST data - form = SubmissionManualUploadForm(request=request) - form._errors = {} - form._errors["__all__"] = form.error_class(["There was a failure receiving the complete form data -- please try again."]) - else: - raise - except ValidationError as e: - form = SubmissionManualUploadForm(request=request) - form._errors = {} - form._errors["__all__"] = form.error_class(["There was a failure converting the xml file to text -- please verify that your xml file is valid. (%s)" % e.message]) - if debug.debug: - raise - except DataError as e: - form = SubmissionManualUploadForm(request=request) - form._errors = {} - form._errors["__all__"] = form.error_class(["There was a failure processing your upload -- please verify that your Internet-Draft passes idnits. (%s)" % e.message]) - if debug.debug: - raise + form = SubmissionManualUploadForm(request, data=request.POST, files=request.FILES) + if form.is_valid(): + submission = get_submission(form) + submission.state = DraftSubmissionStateName.objects.get(slug="validating") + submission.remote_ip = form.remote_ip + submission.file_types = ','.join(form.file_types) + submission.submission_date = date_today() + submission.save() + clear_existing_files(form) + save_files(form) + create_submission_event(request, submission, desc="Uploaded submission") + # Wrap in on_commit so the delayed task cannot start until the view is done with the DB + transaction.on_commit( + lambda: process_uploaded_submission_task.delay(submission.pk) + ) + return redirect("ietf.submit.views.submission_status", submission_id=submission.pk, + access_token=submission.access_token()) else: form = SubmissionManualUploadForm(request=request) @@ -111,6 +77,7 @@ def upload_submission(request): {'selected': 'index', 'form': form}) + @csrf_exempt def api_submission(request): def err(code, error, messages=None): @@ -222,7 +189,7 @@ def api_submit(request): submission = None def err(code, text): return HttpResponse(text, status=code, content_type='text/plain') - + if request.method == 'GET': return render(request, 'submit/api_submit_info.html') elif request.method == 'POST': @@ -301,7 +268,7 @@ def err(code, text): except Exception as e: exception = e raise - return err(500, "Exception: %s" % str(e)) + return err(500, "Exception: %s" % str(e)) finally: if exception and submission: remove_submission_files(submission) @@ -470,7 +437,7 @@ def submission_status(request, submission_id, access_token=None): update_submission_external_resources(submission, extresources) approvals_received = submitter_form.cleaned_data['approvals_received'] - + if submission.rev == '00' and submission.group and not submission.group.is_active: permission_denied(request, 'Posting a new Internet-Draft for an inactive group is not permitted.') @@ -710,7 +677,7 @@ def confirm_submission(request, submission_id, auth_token): messages.error(request, 'The submission is not in a state where it can be cancelled.') return redirect("ietf.submit.views.submission_status", submission_id=submission_id) - + else: raise RuntimeError("Unexpected state in confirm_submission()") @@ -783,7 +750,7 @@ def manualpost(request): ''' manual = Submission.objects.filter(state_id = "manual").distinct() - + for s in manual: s.passes_checks = all([ c.passed!=False for c in s.checks.all() ]) s.errors = validate_submission(s) @@ -799,7 +766,7 @@ def manualpost(request): def cancel_waiting_for_draft(request): if request.method == 'POST': can_cancel = has_role(request.user, "Secretariat") - + if not can_cancel: permission_denied(request, 'You do not have permission to perform this action.') @@ -808,12 +775,12 @@ def cancel_waiting_for_draft(request): submission = get_submission_or_404(submission_id, access_token = access_token) cancel_submission(submission) - + create_submission_event(request, submission, "Cancelled submission") if (submission.rev != "00"): # Add a doc event docevent_from_submission(submission, "Cancelled submission for rev {}".format(submission.rev)) - + return redirect("ietf.submit.views.manualpost") @@ -826,19 +793,19 @@ def add_manualpost_email(request, submission_id=None, access_token=None): button_text = request.POST.get('submit', '') if button_text == 'Cancel': return redirect("submit/manual_post.html") - + form = SubmissionEmailForm(request.POST) if form.is_valid(): submission_pk = form.cleaned_data['submission_pk'] message = form.cleaned_data['message'] #in_reply_to = form.cleaned_data['in_reply_to'] # create Message - + if form.cleaned_data['direction'] == 'incoming': msgtype = 'msgin' else: msgtype = 'msgout' - + submission, submission_email_event = ( add_submission_email(request=request, remote_ip=remote_ip(request), @@ -848,15 +815,15 @@ def add_manualpost_email(request, submission_id=None, access_token=None): message = message, by = request.user.person, msgtype = msgtype) ) - + messages.success(request, 'Email added.') - + try: draft = Document.objects.get(name=submission.name) except Document.DoesNotExist: # Assume this is revision 00 - we'll do this later draft = None - + if (draft != None): e = AddedMessageEvent(type="added_message", doc=draft) e.message = submission_email_event.submissionemailevent.message @@ -866,7 +833,7 @@ def add_manualpost_email(request, submission_id=None, access_token=None): e.desc = submission_email_event.desc e.time = submission_email_event.time e.save() - + return redirect("ietf.submit.views.manualpost") except ValidationError as e: form = SubmissionEmailForm(request.POST) @@ -883,7 +850,7 @@ def add_manualpost_email(request, submission_id=None, access_token=None): initial['submission_pk'] = submission.pk else: initial['direction'] = 'incoming' - + form = SubmissionEmailForm(initial=initial) return render(request, 'submit/add_submit_email.html',dict(form=form)) @@ -914,20 +881,20 @@ def send_submission_email(request, submission_id, message_id=None): reply_to = form.cleaned_data['reply_to'], body = form.cleaned_data['body'] ) - + in_reply_to_id = form.cleaned_data['in_reply_to_id'] in_reply_to = None rp = "" - + if in_reply_to_id: rp = " reply" try: in_reply_to = Message.objects.get(id=in_reply_to_id) except Message.DoesNotExist: log("Unable to retrieve in_reply_to message: %s" % in_reply_to_id) - + desc = "Sent message {} - manual post - {}-{}".format(rp, - submission.name, + submission.name, submission.rev) SubmissionEmailEvent.objects.create( submission = submission, @@ -941,14 +908,14 @@ def send_submission_email(request, submission_id, message_id=None): send_mail_message(None,msg) messages.success(request, 'Email sent.') - return redirect('ietf.submit.views.submission_status', + return redirect('ietf.submit.views.submission_status', submission_id=submission.id, access_token=submission.access_token()) else: reply_to = get_reply_to() msg = None - + if not message_id: addrs = gather_address_lists('sub_confirmation_requested',submission=submission).as_strings(compact=False) to_email = addrs.to @@ -958,7 +925,7 @@ def send_submission_email(request, submission_id, message_id=None): try: submitEmail = SubmissionEmailEvent.objects.get(id=message_id) msg = submitEmail.message - + if msg: to_email = msg.frm cc = msg.cc @@ -979,24 +946,24 @@ def send_submission_email(request, submission_id, message_id=None): 'subject': subject, 'reply_to': reply_to, } - + if msg: initial['in_reply_to_id'] = msg.id - + form = MessageModelForm(initial=initial) return render(request, "submit/email.html", { 'submission': submission, 'access_token': submission.access_token(), 'form':form}) - + def show_submission_email_message(request, submission_id, message_id, access_token=None): submission = get_submission_or_404(submission_id, access_token) - submitEmail = get_object_or_404(SubmissionEmailEvent, pk=message_id) + submitEmail = get_object_or_404(SubmissionEmailEvent, pk=message_id) attachments = submitEmail.message.messageattachment_set.all() - + return render(request, 'submit/submission_email.html', {'submission': submission, 'message': submitEmail, @@ -1007,25 +974,25 @@ def show_submission_email_attachment(request, submission_id, message_id, filenam message = get_object_or_404(SubmissionEmailEvent, pk=message_id) - attach = get_object_or_404(MessageAttachment, - message=message.message, + attach = get_object_or_404(MessageAttachment, + message=message.message, filename=filename) - + if attach.encoding == "base64": body = base64.b64decode(attach.body) else: body = attach.body.encode('utf-8') - + if attach.content_type is None: content_type='text/plain' else: content_type=attach.content_type - + response = HttpResponse(body, content_type=content_type) response['Content-Disposition'] = 'attachment; filename=%s' % attach.filename response['Content-Length'] = len(body) return response - + def get_submission_or_404(submission_id, access_token=None): submission = get_object_or_404(Submission, pk=submission_id) From 55e2dfe4babc6b1fffc0d71bab3c8447e85b96c3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 19 Apr 2023 13:57:26 -0300 Subject: [PATCH 05/32] fix: Do not put Submission in "uploaded" state if an error occured --- ietf/submit/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index efea28825d..5b326a5563 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1294,6 +1294,7 @@ def process_uploaded_submission(submission): except SubmissionError as err: cancel_submission(submission) # changes Submission.state create_submission_event(None, submission, f"Submission rejected: {err}") - submission.state_id = "uploaded" - submission.save() - create_submission_event(None, submission, desc="Completed submission validation checks") + else: + submission.state_id = "uploaded" + submission.save() + create_submission_event(None, submission, desc="Completed submission validation checks") From 8069f42bb3965f9eb86f249157f79ba1a69dd25d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 2 May 2023 14:19:30 +0000 Subject: [PATCH 06/32] refactor: Improve text/XML draft processing flow --- ietf/submit/forms.py | 195 +++++++++---------- ietf/submit/utils.py | 195 ++++++++++++------- ietf/templates/submit/upload_submission.html | 6 - 3 files changed, 218 insertions(+), 178 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 6eb1cc7e92..9466684bb5 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -49,14 +49,8 @@ class SubmissionBaseUploadForm(forms.Form): xml = forms.FileField(label='.xml format', required=True) - # No code currently (14 Sep 2017) uses this class directly; it is - # only used through its subclasses. The two assignments below are - # set to trigger an exception if it is used directly only to make - # sure that adequate consideration is made if it is decided to use it - # directly in the future. Feel free to set these appropriately to - # avoid the exceptions in that case: - formats = None # None will raise an exception in clean() if this isn't changed in a subclass - base_formats = None # None will raise an exception in clean() if this isn't changed in a subclass + formats = ('xml',) # allowed formats + base_formats = ('xml',) # at least one of these is required def __init__(self, request, *args, **kwargs): super(SubmissionBaseUploadForm, self).__init__(*args, **kwargs) @@ -75,11 +69,12 @@ def __init__(self, request, *args, **kwargs): self.title = None self.abstract = None self.authors = [] - self.parsed_draft = None self.file_types = [] self.file_info = {} # indexed by file field name, e.g., 'txt', 'xml', ... self.xml_version = None + self._extracted_filenames_and_revisions = {} + def set_cutoff_warnings(self): now = timezone.now() meeting = Meeting.get_current_meeting() @@ -127,20 +122,17 @@ def set_cutoff_warnings(self): 'The I-D submission tool will be reopened after %s (IETF-meeting local time).' % (cutoff_01_str, reopen_str)) self.shutdown = True - def clean_file(self, field_name, parser_class): + def _clean_file(self, field_name, parser_class): f = self.cleaned_data[field_name] if not f: return f self.file_info[field_name] = parser_class(f).critical_parse() if self.file_info[field_name].errors: - raise forms.ValidationError(self.file_info[field_name].errors) + raise forms.ValidationError(self.file_info[field_name].errors, code="critical_error") return f def clean_xml(self): - return self.clean_file("xml", XMLParser) - - def clean(self): def format_messages(where, e, log_msgs): m = str(e) if m: @@ -149,38 +141,12 @@ def format_messages(where, e, log_msgs): import traceback typ, val, tb = sys.exc_info() m = traceback.format_exception(typ, val, tb) - m = [ l.replace('\n ', ':\n ') for l in m ] - msgs = [s for s in (["Error from xml2rfc (%s):" % (where,)] + m + log_msgs) if s] + m = [l.replace('\n ', ':\n ') for l in m] + msgs = [s for s in ([f"Error from xml2rfc ({where}):"] + m + log_msgs) if s] return msgs - if self.shutdown and not has_role(self.request.user, "Secretariat"): - raise forms.ValidationError('The submission tool is currently shut down') - - # check general submission rate thresholds before doing any more work - today = date_today() - self.check_submissions_thresholds( - "for the same submitter", - dict(remote_ip=self.remote_ip, submission_date=today), - settings.IDSUBMIT_MAX_DAILY_SAME_SUBMITTER, settings.IDSUBMIT_MAX_DAILY_SAME_SUBMITTER_SIZE, - ) - self.check_submissions_thresholds( - "across all submitters", - dict(submission_date=today), - settings.IDSUBMIT_MAX_DAILY_SUBMISSIONS, settings.IDSUBMIT_MAX_DAILY_SUBMISSIONS_SIZE, - ) - - for ext in self.formats: - f = self.cleaned_data.get(ext, None) - if not f: - continue - self.file_types.append('.%s' % ext) - if not ('.txt' in self.file_types or '.xml' in self.file_types): - if not self.errors: - raise forms.ValidationError('Unexpected submission file types; found %s, but %s is required' % (', '.join(self.file_types), ' or '.join(self.base_formats))) - - # Determine the draft name and revision. Try XML first. - if self.cleaned_data.get('xml'): - xml_file = self.cleaned_data.get('xml') + xml_file = self._clean_file("xml", XMLParser) + if xml_file: tfn = None with ExitStack() as stack: @stack.callback @@ -205,27 +171,48 @@ def cleanup(): # called when context exited, even in case of exception xml_draft = XMLDraft(tfn) except XMLParseError as e: msgs = format_messages('xml', e, e.parser_msgs()) - self.add_error('xml', msgs) - return + raise forms.ValidationError(msgs, code="xml_parse_error") except Exception as e: - self.add_error('xml', f'Error parsing XML Internet-Draft: {e}') - return - - self.filename = xml_draft.filename - self.revision = xml_draft.revision - elif self.cleaned_data.get('txt'): - # no XML available, extract from the text if we have it - # n.b., this code path is unused until a subclass with a 'txt' field is created. - txt_file = self.cleaned_data['txt'] - txt_file.seek(0) - bytes = txt_file.read() - try: - text = bytes.decode(self.file_info['txt'].charset) - self.parsed_draft = PlaintextDraft(text, txt_file.name) - self.filename = self.parsed_draft.filename - self.revision = self.parsed_draft.revision - except (UnicodeDecodeError, LookupError) as e: - self.add_error('txt', 'Failed decoding the uploaded file: "%s"' % str(e)) + raise forms.ValidationError(f"Error parsing XML Internet-Draft: {e}", code="parse_exception") + self._extracted_filenames_and_revisions['xml'] = (xml_draft.filename, xml_draft.revision) + return xml_file + + def clean(self): + if self.shutdown and not has_role(self.request.user, "Secretariat"): + raise forms.ValidationError('The submission tool is currently shut down') + + # check general submission rate thresholds before doing any more work + today = date_today() + self.check_submissions_thresholds( + "for the same submitter", + dict(remote_ip=self.remote_ip, submission_date=today), + settings.IDSUBMIT_MAX_DAILY_SAME_SUBMITTER, settings.IDSUBMIT_MAX_DAILY_SAME_SUBMITTER_SIZE, + ) + self.check_submissions_thresholds( + "across all submitters", + dict(submission_date=today), + settings.IDSUBMIT_MAX_DAILY_SUBMISSIONS, settings.IDSUBMIT_MAX_DAILY_SUBMISSIONS_SIZE, + ) + + for ext in self.formats: + f = self.cleaned_data.get(ext, None) + if not f: + continue + self.file_types.append('.%s' % ext) + if not any(f".{bt}" in self.file_types for bt in self.base_formats): + if not self.errors: + raise forms.ValidationError( + "Unexpected submission file types; found {}, but {} is required".format( + ", ".join(ft.lstrip(".") for ft in self.file_types), + " or ".join(self.base_formats), + ) + ) + + # Determine the draft name and revision + for fmt in self.formats: + if fmt in self._extracted_filenames_and_revisions: + self.filename, self.revision = self._extracted_filenames_and_revisions[fmt] + break rev_error = validate_submission_rev(self.filename, self.revision) if rev_error: @@ -254,34 +241,33 @@ def cleanup(): # called when context exited, even in case of exception self.check_for_old_uppercase_collisions(self.filename) - if self.cleaned_data.get('txt') or self.cleaned_data.get('xml'): - # check group - self.group = self.deduce_group(self.filename) - # check existing - existing = Submission.objects.filter(name=self.filename, rev=self.revision).exclude(state__in=("posted", "cancel", "waiting-for-draft")) - if existing: - raise forms.ValidationError( - format_html( - 'A submission with same name and revision is currently being processed. Check the status here.', - urlreverse("ietf.submit.views.submission_status", kwargs={'submission_id': existing[0].pk}), - ) + # check group + self.group = self.deduce_group(self.filename) + # check existing + existing = Submission.objects.filter(name=self.filename, rev=self.revision).exclude(state__in=("posted", "cancel", "waiting-for-draft")) + if existing: + raise forms.ValidationError( + format_html( + 'A submission with same name and revision is currently being processed. Check the status here.', + urlreverse("ietf.submit.views.submission_status", kwargs={'submission_id': existing[0].pk}), ) + ) - # cut-off - if self.revision == '00' and self.in_first_cut_off: - raise forms.ValidationError(mark_safe(self.cutoff_warning)) - # check thresholds that depend on the draft / group + # cut-off + if self.revision == '00' and self.in_first_cut_off: + raise forms.ValidationError(mark_safe(self.cutoff_warning)) + # check thresholds that depend on the draft / group + self.check_submissions_thresholds( + "for the Internet-Draft %s" % self.filename, + dict(name=self.filename, rev=self.revision, submission_date=today), + settings.IDSUBMIT_MAX_DAILY_SAME_DRAFT_NAME, settings.IDSUBMIT_MAX_DAILY_SAME_DRAFT_NAME_SIZE, + ) + if self.group: self.check_submissions_thresholds( - "for the Internet-Draft %s" % self.filename, - dict(name=self.filename, rev=self.revision, submission_date=today), - settings.IDSUBMIT_MAX_DAILY_SAME_DRAFT_NAME, settings.IDSUBMIT_MAX_DAILY_SAME_DRAFT_NAME_SIZE, + "for the group \"%s\"" % (self.group.acronym), + dict(group=self.group, submission_date=today), + settings.IDSUBMIT_MAX_DAILY_SAME_GROUP, settings.IDSUBMIT_MAX_DAILY_SAME_GROUP_SIZE, ) - if self.group: - self.check_submissions_thresholds( - "for the group \"%s\"" % (self.group.acronym), - dict(group=self.group, submission_date=today), - settings.IDSUBMIT_MAX_DAILY_SAME_GROUP, settings.IDSUBMIT_MAX_DAILY_SAME_GROUP_SIZE, - ) return super().clean() @staticmethod @@ -626,10 +612,10 @@ def __init__(self, request, *args, **kwargs): self.base_formats = ['txt', 'xml', ] def clean_txt(self): - return self.clean_file("txt", PlainParser) + return self._clean_file("txt", PlainParser) def clean_pdf(self): - return self.clean_file("pdf", PDFParser) + return self._clean_file("pdf", PDFParser) class DeprecatedSubmissionAutoUploadForm(DeprecatedSubmissionBaseUploadForm): """Full-service upload form, replaced by the asynchronous version""" @@ -640,21 +626,33 @@ def __init__(self, request, *args, **kwargs): self.formats = ['xml', ] self.base_formats = ['xml', ] + class SubmissionManualUploadForm(SubmissionBaseUploadForm): - formats = ('xml',) - base_formats = ('xml',) + txt = forms.FileField(label='.txt format', required=False) + formats = SubmissionBaseUploadForm.formats + ('txt',) + base_formats = SubmissionBaseUploadForm.base_formats + ('txt',) + + def __init__(self, request, *args, **kwargs): + super().__init__(request, *args, **kwargs) + self.fields['xml'].required = False + + def clean_txt(self): + txt_file = self._clean_file("txt", PlainParser) + bytes = txt_file.read() + try: + text = bytes.decode(self.file_info["txt"].charset) + parsed_draft = PlaintextDraft(text, txt_file.name) + self._extracted_filenames_and_revisions["txt"] = (parsed_draft.filename, parsed_draft.revision) + except (UnicodeDecodeError, LookupError) as e: + raise forms.ValidationError(f'Failed decoding the uploaded file: "{str(e)}"', code="decode_failed") + return txt_file class SubmissionAutoUploadForm(SubmissionBaseUploadForm): user = forms.EmailField(required=True) replaces = forms.CharField(required=False, max_length=1000, strip=True) - def __init__(self, request, *args, **kwargs): - super().__init__(request, *args, **kwargs) - self.formats = ['xml', ] - self.base_formats = ['xml', ] - def clean(self): - super().clean() + cleaned_data = super().clean() # Clean the replaces field after the rest of the cleaning so we know the name of the # uploaded draft via self.filename @@ -694,6 +692,7 @@ def clean(self): alias.name + " is approved by the IESG and cannot be replaced" ), ) + return cleaned_data class NameEmailForm(forms.Form): diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 5b326a5563..e834fdd025 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1128,100 +1128,129 @@ def _normalize_title(title): return normalize_text(title) # normalize whitespace -def process_submission_xml(submission): +def process_submission_xml(filename, revision): """Validate and extract info from an uploaded submission""" - xml_path = staging_path(submission.name, submission.rev, '.xml') + xml_path = staging_path(filename, revision, '.xml') xml_draft = XMLDraft(xml_path) - if submission.name != xml_draft.filename: - raise SubmissionError('XML Internet-Draft filename disagrees with submission filename') - if submission.rev != xml_draft.revision: - raise SubmissionError('XML Internet-Draft revision disagrees with submission revision') - - authors = xml_draft.get_author_list() - for a in authors: - if not a['email']: - raise SubmissionError(f'Missing email address for author {a}') - - author_emails = [a['email'].lower() for a in authors] - submitter = get_person_from_name_email(**submission.submitter_parsed()) # the ** expands dict into kwargs - if not any( - email.address.lower() in author_emails - for email in submitter.email_set.filter(active=True) - ): - raise SubmissionError(f'Submitter ({submitter}) is not one of the document authors') - - # Fill in the submission data - submission.title = _normalize_title(xml_draft.get_title()) - if not submission.title: - raise SubmissionError('Could not extract a valid title from the XML') - submission.authors = [ - {key: auth[key] for key in ('name', 'email', 'affiliation', 'country')} - for auth in authors - ] - submission.xml_version = xml_draft.xml_version - submission.save() - - -def process_submission_text(submission): - """Validate/extract data from the text version of a submitted draft - - This assumes the draft was uploaded as XML and extracts data that is not - currently available directly from the XML. Additional processing, e.g. from - get_draft_meta(), would need to be added in order to support direct text - draft uploads. - """ - text_path = staging_path(submission.name, submission.rev, '.txt') + if filename != xml_draft.filename: + raise SubmissionError( + f"XML Internet-Draft filename ({xml_draft.filename}) " + f"disagrees with submission filename ({filename})" + ) + if revision != xml_draft.revision: + raise SubmissionError( + f"XML Internet-Draft revision ({xml_draft.revision})" + f"disagrees with submission revision ({revision})" + ) + title = _normalize_title(xml_draft.get_title()) + if not title: + raise SubmissionError("Could not extract a valid title from the XML") + + return { + "filename": xml_draft.filename, + "rev": xml_draft.revision, + "title": title, + "authors": [ + {key: auth[key] for key in ('name', 'email', 'affiliation', 'country')} + for auth in xml_draft.get_author_list() + ], + "abstract": None, # not supported from XML + "document_date": None, # not supported from XML + "pages": None, # not supported from XML + "words": None, # not supported from XML + "first_two_pages": None, # not supported from XML + "file_size": None, # not supported from XML + "formal_languages": None, # not supported from XML + "xml_version": xml_draft.xml_version, + } + + +def process_submission_text(filename, revision): + """Validate/extract data from the text version of a submitted draft""" + text_path = staging_path(filename, revision, '.txt') text_draft = PlaintextDraft.from_file(text_path) - if submission.name != text_draft.filename: + if filename != text_draft.filename: raise SubmissionError( - f'Text Internet-Draft filename ({text_draft.filename}) disagrees with submission filename ({submission.name})' + f"Text Internet-Draft filename ({text_draft.filename}) " + f"disagrees with submission filename ({filename})" ) - if submission.rev != text_draft.revision: - raise SubmissionError( - f'Text Internet-Draft revision ({text_draft.revision}) disagrees with submission revision ({submission.rev})') - text_title = _normalize_title(text_draft.get_title()) - if not text_title: - raise SubmissionError('Could not extract a valid title from the text') - if text_title != submission.title: + if revision != text_draft.revision: raise SubmissionError( - f'Text Internet-Draft title ({text_title}) disagrees with submission title ({submission.title})') - - submission.abstract = text_draft.get_abstract() - submission.document_date = text_draft.get_creation_date() - submission.pages = text_draft.get_pagecount() - submission.words = text_draft.get_wordcount() - submission.first_two_pages = ''.join(text_draft.pages[:2]) - submission.file_size = os.stat(text_path).st_size - submission.save() - - submission.formal_languages.set( - FormalLanguageName.objects.filter( - slug__in=text_draft.get_formal_languages() + f"Text Internet-Draft revision ({text_draft.revision}) " + f"disagrees with submission revision ({revision})" ) - ) + title = _normalize_title(text_draft.get_title()) + if not title: + raise SubmissionError("Could not extract a valid title from the text") + + return { + "filename": text_draft.filename, + "rev": text_draft.revision, + "title": _normalize_title(text_draft.get_title()), + "authors": None, # not supported from text (todo: fix this!!) + "abstract": text_draft.get_abstract(), + "document_date": text_draft.get_creation_date(), + "pages": text_draft.get_pagecount(), + "words": text_draft.get_wordcount(), + "first_two_pages": ''.join(text_draft.pages[:2]), + "file_size": os.stat(text_path).st_size, + "formal_languages": FormalLanguageName.objects.filter( + slug__in=text_draft.get_formal_languages() + ), + "xml_version": None, # not supported from text + } def process_and_validate_submission(submission): """Process and validate a submission - Limitation: Only works with XML submissions. Cannot handle additional formats. - Raises SubmissionError if an error is encountered. """ - if submission.file_types != '.xml': - raise SubmissionError('Only XML Internet-Draft submissions can be processed.') + if len(set(submission.file_types.split(",")).intersection({".xml", ".txt"})) == 0: + raise SubmissionError("Require XML and/or text format to process an Internet-Draft submission.") try: - process_submission_xml(submission) + xml_metadata = None + # Parse XML first, if we have it + if ".xml" in submission.formats: + xml_metadata = process_submission_xml(submission) + render_missing_formats(submission) # makes HTML and text, unless text was uploaded + # Parse text, whether uploaded or generated from XML + text_metadata = process_submission_text(submission) + + if xml_metadata and xml_metadata["title"] != text_metadata["title"]: + raise SubmissionError( + f"Text Internet-Draft title ({text_metadata['title']}) " + f"disagrees with XML Internet-Draft title ({xml_metadata['title']})" + ) + + # Fill in the submission from the parsed XML/text metadata + if xml_metadata is not None: + # Items preferred / only available from XML + submission.xml_version = xml_metadata["xml_version"] + submission.authors = xml_metadata["authors"] + else: + # Items to get from text only if XML not available + submission.authors = text_metadata["authors"] + + # Items always to get from text, even when XML is available + submission.title = text_metadata["title"] # verified above this agrees with XML, if present + submission.abstract = text_metadata["abstract"] + submission.document_date = text_metadata["document_date"] + submission.pages = text_metadata["pages"] + submission.words = text_metadata["words"] + submission.first_two_pages = text_metadata["first_two_pages"] + submission.file_size = text_metadata["file_size"] + submission.save() + submission.formal_languages.set(text_metadata["formal_languages"]) + if check_submission_revision_consistency(submission): raise SubmissionError( 'Document revision inconsistency error in the database. ' 'Please contact the secretariat for assistance.' ) - render_missing_formats(submission) - process_submission_text(submission) set_extresources_from_existing_draft(submission) apply_checkers( submission, @@ -1240,9 +1269,13 @@ def process_and_validate_submission(submission): def submitter_is_author(submission): - submitter = get_person_from_name_email(**submission.submitter_parsed()) # the ** expands dict into kwargs + submitter = get_person_from_name_email(**submission.submitter_parsed()) if submitter: - author_emails = [author['email'].strip().lower() for author in submission.authors] + author_emails = [ + author["email"].strip().lower() + for author in submission.authors + if "email" in author + ] return any( email.address.lower() in author_emails for email in submitter.email_set.filter(active=True) @@ -1250,6 +1283,10 @@ def submitter_is_author(submission): return False +def all_authors_have_emails(submission): + return all(a["email"] for a in submission.authors) + + def process_and_accept_uploaded_submission(submission): """Process, validate, and, if valid, accept an uploaded submission @@ -1262,11 +1299,21 @@ def process_and_accept_uploaded_submission(submission): return # do nothing try: - process_and_validate_submission(submission) + process_and_validate_submission( + submission, require_author_emails=True, require_submitter_is_author=True + ) except SubmissionError as err: cancel_submission(submission) # changes Submission.state create_submission_event(None, submission, f"Submission rejected: {err}") + if not all_authors_have_emails(submission): + cancel_submission(submission) # changes Submission.state + create_submission_event( + None, + submission, + "Submission rejected: Email address not found for all authors" + ) + if not submitter_is_author(submission): cancel_submission(submission) # changes Submission.state create_submission_event( diff --git a/ietf/templates/submit/upload_submission.html b/ietf/templates/submit/upload_submission.html index 5858da6f9c..31c5e4ca87 100644 --- a/ietf/templates/submit/upload_submission.html +++ b/ietf/templates/submit/upload_submission.html @@ -60,12 +60,6 @@ However, if you cannot for some reason submit XML, you must submit a plaintext rendering of your I-D.

- {% bootstrap_label ' PDF rendering of the I-D' label_class="form-label fw-bold" %} - {% bootstrap_field form.pdf show_label=False %} -

- Optional to submit, will be auto-generated based - on the submitted XML. -

{% bootstrap_form_errors form %} {% bootstrap_button button_type="submit" name="upload" content="Upload" %} From c2cd9a14c8fcc8405eed478654d0cc2ca0e6d5ae Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 2 May 2023 21:01:56 +0000 Subject: [PATCH 07/32] feat: Extract authors from text in async processing --- ietf/submit/utils.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index e834fdd025..f82d87a3c8 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -11,7 +11,7 @@ import traceback import xml2rfc -from typing import Optional # pyflakes:ignore +from typing import Optional, Union # pyflakes:ignore from unidecode import unidecode from django.conf import settings @@ -1166,6 +1166,27 @@ def process_submission_xml(filename, revision): } +def _turn_into_unicode(s: Optional[Union[str, bytes]]): + """Decode a possibly null string-like item as a string + + Copied from ietf.submit.utils.get_draft_meta(), would be nice to + ditch this. + """ + if s is None: + return "" + + if isinstance(s, str): + return s + else: + try: + return s.decode("utf-8") + except UnicodeDecodeError: + try: + return s.decode("latin-1") + except UnicodeDecodeError: + return "" + + def process_submission_text(filename, revision): """Validate/extract data from the text version of a submitted draft""" text_path = staging_path(filename, revision, '.txt') @@ -1185,11 +1206,22 @@ def process_submission_text(filename, revision): if not title: raise SubmissionError("Could not extract a valid title from the text") + # Drops \r, \n, <, >. Based on get_draft_meta() behavior + trans_table = str.maketrans("", "", "\r\n<>") + authors = [ + { + "name": fullname.translate(trans_table).strip(), + "email": _turn_into_unicode(email if validate_email(email) else ""), + "affiliation": _turn_into_unicode(company), + "country": _turn_into_unicode(country), + } + for (fullname, _, _, _, _, email, country, company) in text_draft.get_author_list() + ] return { "filename": text_draft.filename, "rev": text_draft.revision, "title": _normalize_title(text_draft.get_title()), - "authors": None, # not supported from text (todo: fix this!!) + "authors": authors, "abstract": text_draft.get_abstract(), "document_date": text_draft.get_creation_date(), "pages": text_draft.get_pagecount(), From 063a62591ecdd708fb4b45838bf17d6205fa9b35 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 3 May 2023 15:00:13 +0000 Subject: [PATCH 08/32] fix: Fix call signatures and abort submission on failed validation --- ietf/submit/utils.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index f82d87a3c8..8de547071d 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1246,11 +1246,11 @@ def process_and_validate_submission(submission): try: xml_metadata = None # Parse XML first, if we have it - if ".xml" in submission.formats: - xml_metadata = process_submission_xml(submission) + if ".xml" in submission.file_types: + xml_metadata = process_submission_xml(submission.name, submission.rev) render_missing_formats(submission) # makes HTML and text, unless text was uploaded # Parse text, whether uploaded or generated from XML - text_metadata = process_submission_text(submission) + text_metadata = process_submission_text(submission.name, submission.rev) if xml_metadata and xml_metadata["title"] != text_metadata["title"]: raise SubmissionError( @@ -1331,12 +1331,11 @@ def process_and_accept_uploaded_submission(submission): return # do nothing try: - process_and_validate_submission( - submission, require_author_emails=True, require_submitter_is_author=True - ) + process_and_validate_submission(submission) except SubmissionError as err: cancel_submission(submission) # changes Submission.state create_submission_event(None, submission, f"Submission rejected: {err}") + return if not all_authors_have_emails(submission): cancel_submission(submission) # changes Submission.state @@ -1345,6 +1344,7 @@ def process_and_accept_uploaded_submission(submission): submission, "Submission rejected: Email address not found for all authors" ) + return if not submitter_is_author(submission): cancel_submission(submission) # changes Submission.state @@ -1353,6 +1353,7 @@ def process_and_accept_uploaded_submission(submission): submission, f"Submission rejected: Submitter ({submission.submitter}) is not one of the document authors", ) + return create_submission_event(None, submission, desc="Completed submission validation checks") accept_submission(submission) From 927a1642ad1d306270b58308df2f50b7879c843b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 3 May 2023 15:00:37 +0000 Subject: [PATCH 09/32] feat: Validate submission name format --- ietf/submit/forms.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 9466684bb5..7e7501de37 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -213,6 +213,9 @@ def clean(self): if fmt in self._extracted_filenames_and_revisions: self.filename, self.revision = self._extracted_filenames_and_revisions[fmt] break + name_error = validate_submission_name(self.filename) + if name_error: + raise forms.ValidationError(name_error) rev_error = validate_submission_rev(self.filename, self.revision) if rev_error: From 01f2234cf23ee0aefd944d571c4bff989b88280b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 3 May 2023 15:43:05 +0000 Subject: [PATCH 10/32] fix: Correctly validate emails from text submission --- ietf/submit/utils.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 8de547071d..0c41b31807 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1187,6 +1187,14 @@ def _turn_into_unicode(s: Optional[Union[str, bytes]]): return "" +def _is_valid_email(addr): + try: + validate_email(addr) + except ValidationError: + return False + return True + + def process_submission_text(filename, revision): """Validate/extract data from the text version of a submitted draft""" text_path = staging_path(filename, revision, '.txt') @@ -1211,7 +1219,7 @@ def process_submission_text(filename, revision): authors = [ { "name": fullname.translate(trans_table).strip(), - "email": _turn_into_unicode(email if validate_email(email) else ""), + "email": _turn_into_unicode(email if _is_valid_email(email) else ""), "affiliation": _turn_into_unicode(company), "country": _turn_into_unicode(country), } From 60a237ad8e430f98745be79364c972208db646d0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 3 May 2023 17:55:59 +0000 Subject: [PATCH 11/32] fix: Clean up submission validation --- ietf/submit/forms.py | 108 ++++++++++++++++++++++++++++++------------- 1 file changed, 75 insertions(+), 33 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 7e7501de37..54ebcd1f41 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -174,6 +174,22 @@ def cleanup(): # called when context exited, even in case of exception raise forms.ValidationError(msgs, code="xml_parse_error") except Exception as e: raise forms.ValidationError(f"Error parsing XML Internet-Draft: {e}", code="parse_exception") + if not xml_draft.filename: + raise forms.ValidationError( + "Could not extract a valid Internet-Draft name from the XML. " + "Please make sure that the top-level " + "element has a docName attribute which provides the full Internet-Draft name including " + "revision number.", + code="parse_error_filename", + ) + if not xml_draft.revision: + raise forms.ValidationError( + "Could not extract a valid Internet-Draft revision from the XML. " + "Please make sure that the top-level " + "element has a docName attribute which provides the full Internet-Draft name including " + "revision number.", + code="parse_error_revision", + ) self._extracted_filenames_and_revisions['xml'] = (xml_draft.filename, xml_draft.revision) return xml_file @@ -208,39 +224,48 @@ def clean(self): ) ) - # Determine the draft name and revision - for fmt in self.formats: - if fmt in self._extracted_filenames_and_revisions: - self.filename, self.revision = self._extracted_filenames_and_revisions[fmt] - break - name_error = validate_submission_name(self.filename) - if name_error: - raise forms.ValidationError(name_error) - - rev_error = validate_submission_rev(self.filename, self.revision) - if rev_error: - raise forms.ValidationError(rev_error) - # The following errors are likely noise if we have previous field # errors: if self.errors: raise forms.ValidationError('') + # Check that all formats agree on draft name/rev + filename_from = None + for fmt, (extracted_name, extracted_rev) in self._extracted_filenames_and_revisions.items(): + if self.filename is None: + filename_from = fmt + self.filename = extracted_name + self.revision = extracted_rev + elif self.filename != extracted_name: + raise forms.ValidationError( + {fmt: f"Extracted filename '{extracted_name}' does not match filename '{self.filename}' from {filename_from} format"}, + code="filename_mismatch", + ) + elif self.revision != extracted_rev: + raise forms.ValidationError( + {fmt: f"Extracted revision ({extracted_rev}) does not match revision from {filename_from} format ({self.revision})"}, + code="revision_mismatch", + ) + # Not expected to encounter missing filename/revision here because + # the individual fields should fail validation, but just in case if not self.filename: - raise forms.ValidationError("Could not extract a valid Internet-Draft name from the upload. " - "To fix this in a text upload, please make sure that the full Internet-Draft name including " - "revision number appears centered on its own line below the document title on the " - "first page. In an xml upload, please make sure that the top-level " - "element has a docName attribute which provides the full Internet-Draft name including " - "revision number.") - + raise forms.ValidationError( + "Unable to extract a filename from any uploaded format.", + code="no_filename", + ) if not self.revision: - raise forms.ValidationError("Could not extract a valid Internet-Draft revision from the upload. " - "To fix this in a text upload, please make sure that the full Internet-Draft name including " - "revision number appears centered on its own line below the document title on the " - "first page. In an xml upload, please make sure that the top-level " - "element has a docName attribute which provides the full Internet-Draft name including " - "revision number.") + raise forms.ValidationError( + "Unable to extract a revision from any uploaded format.", + code="no_revision", + ) + + name_error = validate_submission_name(self.filename) + if name_error: + raise forms.ValidationError(name_error) + + rev_error = validate_submission_rev(self.filename, self.revision) + if rev_error: + raise forms.ValidationError(rev_error) self.check_for_old_uppercase_collisions(self.filename) @@ -641,13 +666,30 @@ def __init__(self, request, *args, **kwargs): def clean_txt(self): txt_file = self._clean_file("txt", PlainParser) - bytes = txt_file.read() - try: - text = bytes.decode(self.file_info["txt"].charset) - parsed_draft = PlaintextDraft(text, txt_file.name) - self._extracted_filenames_and_revisions["txt"] = (parsed_draft.filename, parsed_draft.revision) - except (UnicodeDecodeError, LookupError) as e: - raise forms.ValidationError(f'Failed decoding the uploaded file: "{str(e)}"', code="decode_failed") + if txt_file is not None: + bytes = txt_file.read() + try: + text = bytes.decode(self.file_info["txt"].charset) + parsed_draft = PlaintextDraft(text, txt_file.name) + self._extracted_filenames_and_revisions["txt"] = (parsed_draft.filename, parsed_draft.revision) + except (UnicodeDecodeError, LookupError) as e: + raise forms.ValidationError(f'Failed decoding the uploaded file: "{str(e)}"', code="decode_failed") + if not parsed_draft.filename: + raise forms.ValidationError( + "Could not extract a valid Internet-Draft name from the plaintext. " + "Please make sure that the full Internet-Draft name including " + "revision number appears centered on its own line below the document title on the " + "first page.", + code="parse_error_filename", + ) + if not parsed_draft.revision: + raise forms.ValidationError( + "Could not extract a valid Internet-Draft revision from the plaintext. " + "Please make sure that the full Internet-Draft name including " + "revision number appears centered on its own line below the document title on the " + "first page.", + code="parse_error_revision", + ) return txt_file class SubmissionAutoUploadForm(SubmissionBaseUploadForm): From 3ece5d95229797884282ce6cb465044b7d883483 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 3 May 2023 17:56:26 +0000 Subject: [PATCH 12/32] fix: Better display errors on upload_submission page --- ietf/templates/submit/upload_submission.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ietf/templates/submit/upload_submission.html b/ietf/templates/submit/upload_submission.html index 31c5e4ca87..7313d8f000 100644 --- a/ietf/templates/submit/upload_submission.html +++ b/ietf/templates/submit/upload_submission.html @@ -47,7 +47,8 @@ aria-controls="other-formats"> + type="checkbox" + {% if form.errors.txt %}checked {% endif %}> Submit other formats @@ -61,7 +62,7 @@ submit a plaintext rendering of your I-D.

- {% bootstrap_form_errors form %} + {% bootstrap_form_errors form type="non_fields" %} {% bootstrap_button button_type="submit" name="upload" content="Upload" %} {% include "submit/problem-reports-footer.html" %} From 9946a020306e0c4c9fea80cc21cccf786ef386f9 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 02:28:03 +0000 Subject: [PATCH 13/32] feat: Reload submission status page when awaiting validation --- ietf/static/js/draft-submit.js | 140 +++++++++++-------- ietf/templates/submit/submission_status.html | 17 ++- 2 files changed, 91 insertions(+), 66 deletions(-) diff --git a/ietf/static/js/draft-submit.js b/ietf/static/js/draft-submit.js index d3657a8377..7643597a42 100644 --- a/ietf/static/js/draft-submit.js +++ b/ietf/static/js/draft-submit.js @@ -1,69 +1,89 @@ -$(document) - .ready(function () { - // fill in submitter info when an author button is clicked - $("form.idsubmit button.author") - .on("click", function () { - var name = $(this) - .data("name"); - var email = $(this) - .data("email"); +$(function () { + // fill in submitter info when an author button is clicked + $("form.idsubmit button.author") + .on("click", function () { + var name = $(this) + .data("name"); + var email = $(this) + .data("email"); - $(this) - .parents("form") - .find("input[name=submitter-name]") - .val(name || ""); - $(this) - .parents("form") - .find("input[name=submitter-email]") - .val(email || ""); - }); + $(this) + .parents("form") + .find("input[name=submitter-name]") + .val(name || ""); + $(this) + .parents("form") + .find("input[name=submitter-email]") + .val(email || ""); + }); - $("form.idsubmit") - .on("submit", function () { - if (this.submittedAlready) - return false; - else { - this.submittedAlready = true; - return true; - } - }); + $("form.idsubmit") + .on("submit", function () { + if (this.submittedAlready) + return false; + else { + this.submittedAlready = true; + return true; + } + }); - $("form.idsubmit #add-author") - .on("click", function () { - // clone the last author block and make it empty - var cloner = $("#cloner"); - var next = cloner.clone(); - next.find('input:not([type=hidden])') - .val(''); + $("form.idsubmit #add-author") + .on("click", function () { + // clone the last author block and make it empty + var cloner = $("#cloner"); + var next = cloner.clone(); + next.find('input:not([type=hidden])') + .val(''); - // find the author number - var t = next.children('h3') - .text(); - var n = parseInt(t.replace(/\D/g, '')); + // find the author number + var t = next.children('h3') + .text(); + var n = parseInt(t.replace(/\D/g, '')); - // change the number in attributes and text - next.find('*') - .each(function () { - var e = this; - $.each(['id', 'for', 'name', 'value'], function (i, v) { - if ($(e) - .attr(v)) { - $(e) - .attr(v, $(e) - .attr(v) - .replace(n - 1, n)); - } - }); + // change the number in attributes and text + next.find('*') + .each(function () { + var e = this; + $.each(['id', 'for', 'name', 'value'], function (i, v) { + if ($(e) + .attr(v)) { + $(e) + .attr(v, $(e) + .attr(v) + .replace(n - 1, n)); + } }); + }); + + t = t.replace(n, n + 1); + next.children('h3') + .text(t); - t = t.replace(n, n + 1); - next.children('h3') - .text(t); + // move the cloner id to next and insert next into the DOM + cloner.removeAttr('id'); + next.attr('id', 'cloner'); + next.insertAfter(cloner); - // move the cloner id to next and insert next into the DOM - cloner.removeAttr('id'); - next.attr('id', 'cloner'); - next.insertAfter(cloner); + }); - }); - }); + // Reload page periodically if the enableAutoReload checkbox is present and checked + const autoReloadSwitch = document.getElementById("enableAutoReload"); + if (autoReloadSwitch) { + const autoReloadTime = 30000; // ms + let autoReloadTimeoutId; + autoReloadSwitch.parentElement.classList.remove("d-none"); + autoReloadTimeoutId = setTimeout(() => location.reload(), autoReloadTime); + autoReloadSwitch.addEventListener("change", (e) => { + if (e.currentTarget.checked) { + if (!autoReloadTimeoutId) { + autoReloadTimeoutId = setTimeout(() => location.reload(), autoReloadTime); + } + } else { + if (autoReloadTimeoutId) { + clearTimeout(autoReloadTimeoutId); + autoReloadTimeoutId = null; + } + } + }); + } +}); diff --git a/ietf/templates/submit/submission_status.html b/ietf/templates/submit/submission_status.html index 80b0c0612d..f77a653e83 100644 --- a/ietf/templates/submit/submission_status.html +++ b/ietf/templates/submit/submission_status.html @@ -133,17 +133,22 @@

Submission checks

This submission is awaiting the first Internet-Draft upload.

{% elif submission.state_id == 'validating' %} -

+

This submission is still being processed and validated. This normally takes a few minutes after submission. {% with earliest_event=submission.submissionevent_set.last %} - {% if earliest_event %} - It has been {{ earliest_event.time|timesince }} since submission. - {% endif %} + {% if earliest_event %} + It has been {{ earliest_event.time|timesince }} since submission. + {% endif %} {% endwith %} Please contact the secretariat for assistance if it has been more than an hour. -

- {% else %} + +
{# hide with d-none unless javascript makes it visible #} + + +
+
+ {% else %}

Meta-data from the submission

{% if errors %}
From d0dab468d65d87fc5f15b9a18e7697922b4b211b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 13:16:19 +0000 Subject: [PATCH 14/32] test: Fix call signatures; remove unused imports --- ietf/submit/forms.py | 1 - ietf/submit/tests.py | 6 +++--- ietf/submit/views.py | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 54ebcd1f41..a4ff178de1 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -14,7 +14,6 @@ from email.utils import formataddr from unidecode import unidecode -from urllib.parse import urljoin from django import forms from django.conf import settings diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 50a58494d3..6ea92a3171 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -3333,7 +3333,7 @@ def test_process_submission_text_consistency_checks(self): ) txt_path.open('w').write(txt.read()) with self.assertRaisesMessage(SubmissionError, 'disagrees with submission filename'): - process_submission_text(submission) + process_submission_text(submission.name, submission.rev) # rev mismatch txt, _ = submission_file( @@ -3345,7 +3345,7 @@ def test_process_submission_text_consistency_checks(self): ) txt_path.open('w').write(txt.read()) with self.assertRaisesMessage(SubmissionError, 'disagrees with submission revision'): - process_submission_text(submission) + process_submission_text(submission.name, submission.rev) # title mismatch txt, _ = submission_file( @@ -3357,7 +3357,7 @@ def test_process_submission_text_consistency_checks(self): ) txt_path.open('w').write(txt.read()) with self.assertRaisesMessage(SubmissionError, 'disagrees with submission title'): - process_submission_text(submission) + process_submission_text(submission.name, submission.rev) def test_status_of_validating_submission(self): s = SubmissionFactory(state_id='validating') diff --git a/ietf/submit/views.py b/ietf/submit/views.py index 68de2745c1..77f386d146 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -12,7 +12,7 @@ from django.conf import settings from django.contrib import messages from django.contrib.auth.models import User -from django.db import DataError, transaction +from django.db import transaction from django.urls import reverse as urlreverse from django.core.exceptions import ValidationError from django.http import HttpResponseRedirect, Http404, HttpResponseForbidden, HttpResponse, JsonResponse From 4f88cb4ce7e00e8dd6e94b7bcd6af6b9d0d86775 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 13:54:21 +0000 Subject: [PATCH 15/32] chore: Add type hint --- ietf/submit/forms.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index a4ff178de1..557478a86b 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -13,6 +13,7 @@ from contextlib import ExitStack from email.utils import formataddr +from typing import Tuple from unidecode import unidecode from django import forms @@ -48,8 +49,8 @@ class SubmissionBaseUploadForm(forms.Form): xml = forms.FileField(label='.xml format', required=True) - formats = ('xml',) # allowed formats - base_formats = ('xml',) # at least one of these is required + formats: Tuple[str, ...] = ('xml',) # allowed formats + base_formats: Tuple[str, ...] = ('xml',) # at least one of these is required def __init__(self, request, *args, **kwargs): super(SubmissionBaseUploadForm, self).__init__(*args, **kwargs) From dd6827e557baba10e951f28379540095c2a6694d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 14:08:54 +0000 Subject: [PATCH 16/32] test: Update tests to match renamed task --- ietf/submit/tests.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 6ea92a3171..94f51992d3 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -47,7 +47,7 @@ from ietf.submit.forms import SubmissionBaseUploadForm, SubmissionAutoUploadForm from ietf.submit.models import Submission, Preapproval, SubmissionExtResource from ietf.submit.mail import add_submission_email, process_response_email -from ietf.submit.tasks import cancel_stale_submissions, process_uploaded_submission_task +from ietf.submit.tasks import cancel_stale_submissions, process_and_accept_uploaded_submission_task from ietf.utils.accesstoken import generate_access_token from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.models import VersionInfo @@ -2742,6 +2742,8 @@ def supply_extra_metadata(self, name, status_url, submitter_name, submitter_emai @mock.patch.object(transaction, 'on_commit', lambda x: x()) @override_settings(IDTRACKER_BASE_URL='https://datatracker.example.com') class ApiSubmissionTests(BaseSubmitTestCase): + TASK_TO_MOCK = "ietf.submit.views.process_and_accept_uploaded_submission_task" + def test_upload_draft(self): """api_submission accepts a submission and queues it for processing""" url = urlreverse('ietf.submit.views.api_submission') @@ -2750,7 +2752,7 @@ def test_upload_draft(self): 'xml': xml, 'user': author.user.username, } - with mock.patch('ietf.submit.views.process_uploaded_submission_task') as mock_task: + with mock.patch(self.TASK_TO_MOCK) as mock_task: r = self.client.post(url, data) self.assertEqual(r.status_code, 200) response = r.json() @@ -2788,7 +2790,7 @@ def test_upload_draft_with_replaces(self): 'replaces': existing_draft.name, } # mock out the task so we don't call to celery during testing! - with mock.patch('ietf.submit.views.process_uploaded_submission_task'): + with mock.patch(self.TASK_TO_MOCK): r = self.client.post(url, data) self.assertEqual(r.status_code, 200) submission = Submission.objects.last() @@ -2806,7 +2808,7 @@ def test_rejects_broken_upload(self): 'xml': xml, 'user': 'i.dont.exist@nowhere.example.com', } - with mock.patch('ietf.submit.views.process_uploaded_submission_task') as mock_task: + with mock.patch(self.TASK_TO_MOCK) as mock_task: r = self.client.post(url, data) self.assertEqual(r.status_code, 400) response = r.json() @@ -2820,7 +2822,7 @@ def test_rejects_broken_upload(self): 'xml': xml, 'user': author.user.username, } - with mock.patch('ietf.submit.views.process_uploaded_submission_task') as mock_task: + with mock.patch(self.TASK_TO_MOCK) as mock_task: r = self.client.post(url, data) self.assertEqual(r.status_code, 400) response = r.json() @@ -2834,7 +2836,7 @@ def test_rejects_broken_upload(self): 'xml': xml, 'user': author.user.username, } - with mock.patch('ietf.submit.views.process_uploaded_submission_task') as mock_task: + with mock.patch(self.TASK_TO_MOCK) as mock_task: r = self.client.post(url, data) self.assertEqual(r.status_code, 400) response = r.json() @@ -2850,7 +2852,7 @@ def test_rejects_broken_upload(self): 'xml': xml, 'user': author.user.username, } - with mock.patch('ietf.submit.views.process_uploaded_submission_task') as mock_task: + with mock.patch(self.TASK_TO_MOCK) as mock_task: r = self.client.post(url, data) self.assertEqual(r.status_code, 400) response = r.json() @@ -3298,20 +3300,20 @@ def test_process_uploaded_submission_invalid(self): @mock.patch('ietf.submit.tasks.process_uploaded_submission') - def test_process_uploaded_submission_task(self, mock_method): - """process_uploaded_submission_task task should properly call its method""" + def test_process_and_accept_uploaded_submission_task(self, mock_method): + """process_and_accept_uploaded_submission_task task should properly call its method""" s = SubmissionFactory() - process_uploaded_submission_task(s.pk) + process_and_accept_uploaded_submission_task(s.pk) self.assertEqual(mock_method.call_count, 1) self.assertEqual(mock_method.call_args.args, (s,)) @mock.patch('ietf.submit.tasks.process_uploaded_submission') - def test_process_uploaded_submission_task_ignores_invalid_id(self, mock_method): - """process_uploaded_submission_task should ignore an invalid submission_id""" + def test_process_and_accept_uploaded_submission_task_ignores_invalid_id(self, mock_method): + """process_and_accept_uploaded_submission_task should ignore an invalid submission_id""" SubmissionFactory() # be sure there is a Submission bad_pk = 9876 self.assertEqual(Submission.objects.filter(pk=bad_pk).count(), 0) - process_uploaded_submission_task(bad_pk) + process_and_accept_uploaded_submission_task(bad_pk) self.assertEqual(mock_method.call_count, 0) def test_process_submission_text_consistency_checks(self): From 5468788f7f61a57fb46094bf961dcfdd4edb85ef Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 14:56:26 +0000 Subject: [PATCH 17/32] fix: Fix typo in error message --- ietf/submit/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 0c41b31807..229cbcd65a 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1140,7 +1140,7 @@ def process_submission_xml(filename, revision): ) if revision != xml_draft.revision: raise SubmissionError( - f"XML Internet-Draft revision ({xml_draft.revision})" + f"XML Internet-Draft revision ({xml_draft.revision}) " f"disagrees with submission revision ({revision})" ) title = _normalize_title(xml_draft.get_title()) @@ -1302,7 +1302,10 @@ def process_and_validate_submission(submission): errors = [c.message for c in submission.checks.filter(passed__isnull=False) if not c.passed] if len(errors) > 0: raise SubmissionError('Checks failed: ' + ' / '.join(errors)) + except SubmissionError: + raise # pass SubmissionErrors up the stack except Exception: + # convert other exceptions into SubmissionErrors log.log(f'Unexpected exception while processing submission {submission.pk}.') log.log(traceback.format_exc()) raise SubmissionError('A system error occurred while processing the submission.') From 35c4aecbbe2bf27dad8b001a25a961173f77fda4 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 14:57:47 +0000 Subject: [PATCH 18/32] test: Fix failing Api- and AsyncSubmissionTests * Rename process_uploaded_submission to process_and_accept_... * Remove outdated tests Does not yet test new behavior. --- ietf/submit/tests.py | 64 +++++++++++++------------------------------- 1 file changed, 18 insertions(+), 46 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 94f51992d3..24bc34f582 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -28,7 +28,7 @@ from ietf.submit.utils import (expirable_submissions, expire_submission, find_submission_filenames, post_submission, validate_submission_name, validate_submission_rev, - process_uploaded_submission, SubmissionError, process_submission_text) + process_and_accept_uploaded_submission, SubmissionError, process_submission_text) from ietf.doc.factories import (DocumentFactory, WgDraftFactory, IndividualDraftFactory, IndividualRfcFactory, ReviewFactory, WgRfcFactory) from ietf.doc.models import ( Document, DocAlias, DocEvent, State, @@ -3107,8 +3107,8 @@ def test_replaces_field(self): class AsyncSubmissionTests(BaseSubmitTestCase): """Tests of async submission-related tasks""" - def test_process_uploaded_submission(self): - """process_uploaded_submission should properly process a submission""" + def test_process_and_accept_uploaded_submission(self): + """process_and_accept_uploaded_submission should properly process a submission""" _today = date_today() xml, author = submission_file('draft-somebody-test-00', 'draft-somebody-test-00.xml', None, 'test_submission.xml') xml_data = xml.read() @@ -3128,7 +3128,7 @@ def test_process_uploaded_submission(self): self.assertFalse(txt_path.exists()) html_path = xml_path.with_suffix('.html') self.assertFalse(html_path.exists()) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'auth', 'accepted submission should be in auth state') @@ -3146,8 +3146,8 @@ def test_process_uploaded_submission(self): self.assertEqual(submission.file_size, os.stat(txt_path).st_size) self.assertIn('Completed submission validation checks', submission.submissionevent_set.last().desc) - def test_process_uploaded_submission_invalid(self): - """process_uploaded_submission should properly process an invalid submission""" + def test_process_and_accept_uploaded_submission_invalid(self): + """process_and_accept_uploaded_submission should properly process an invalid submission""" xml, author = submission_file('draft-somebody-test-00', 'draft-somebody-test-00.xml', None, 'test_submission.xml') xml_data = xml.read() xml.close() @@ -3168,7 +3168,7 @@ def test_process_uploaded_submission_invalid(self): xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.xml' with xml_path.open('w') as f: f.write(xml_data) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') self.assertIn('not one of the document authors', submission.submissionevent_set.last().desc) @@ -3184,10 +3184,10 @@ def test_process_uploaded_submission_invalid(self): xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.xml' with xml_path.open('w') as f: f.write(re.sub(r'.*', '', xml_data)) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') - self.assertIn('Missing email address', submission.submissionevent_set.last().desc) + self.assertIn('Email address not found for all authors', submission.submissionevent_set.last().desc) # no title submission = SubmissionFactory( @@ -3200,7 +3200,7 @@ def test_process_uploaded_submission_invalid(self): xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.xml' with xml_path.open('w') as f: f.write(re.sub(r'.*', '', xml_data)) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') self.assertIn('Could not extract a valid title', submission.submissionevent_set.last().desc) @@ -3216,10 +3216,10 @@ def test_process_uploaded_submission_invalid(self): xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-different-name-00.xml' with xml_path.open('w') as f: f.write(xml_data) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') - self.assertIn('Internet-Draft filename disagrees', submission.submissionevent_set.last().desc) + self.assertIn('Submission rejected: XML Internet-Draft filename', submission.submissionevent_set.last().desc) # rev mismatch submission = SubmissionFactory( @@ -3232,26 +3232,10 @@ def test_process_uploaded_submission_invalid(self): xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-01.xml' with xml_path.open('w') as f: f.write(xml_data) - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') - self.assertIn('revision disagrees', submission.submissionevent_set.last().desc) - - # not xml - submission = SubmissionFactory( - name='draft-somebody-test', - rev='00', - file_types='.txt', - submitter=author.formatted_email(), - state_id='validating', - ) - txt_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.txt' - with txt_path.open('w') as f: - f.write(txt_data) - process_uploaded_submission(submission) - submission = Submission.objects.get(pk=submission.pk) # refresh - self.assertEqual(submission.state_id, 'cancel') - self.assertIn('Only XML Internet-Draft submissions', submission.submissionevent_set.last().desc) + self.assertIn('Submission rejected: XML Internet-Draft revision', submission.submissionevent_set.last().desc) # wrong state submission = SubmissionFactory( @@ -3265,7 +3249,7 @@ def test_process_uploaded_submission_invalid(self): with xml_path.open('w') as f: f.write(xml_data) with mock.patch('ietf.submit.utils.process_submission_xml') as mock_proc_xml: - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertFalse(mock_proc_xml.called, 'Should not process submission not in "validating" state') self.assertEqual(submission.state_id, 'uploaded', 'State should not be changed') @@ -3293,13 +3277,13 @@ def test_process_uploaded_submission_invalid(self): symbol='x', ) ): - process_uploaded_submission(submission) + process_and_accept_uploaded_submission(submission) submission = Submission.objects.get(pk=submission.pk) # refresh self.assertEqual(submission.state_id, 'cancel') self.assertIn('fake failure', submission.submissionevent_set.last().desc) - @mock.patch('ietf.submit.tasks.process_uploaded_submission') + @mock.patch('ietf.submit.tasks.process_and_accept_uploaded_submission') def test_process_and_accept_uploaded_submission_task(self, mock_method): """process_and_accept_uploaded_submission_task task should properly call its method""" s = SubmissionFactory() @@ -3307,7 +3291,7 @@ def test_process_and_accept_uploaded_submission_task(self, mock_method): self.assertEqual(mock_method.call_count, 1) self.assertEqual(mock_method.call_args.args, (s,)) - @mock.patch('ietf.submit.tasks.process_uploaded_submission') + @mock.patch('ietf.submit.tasks.process_and_accept_uploaded_submission') def test_process_and_accept_uploaded_submission_task_ignores_invalid_id(self, mock_method): """process_and_accept_uploaded_submission_task should ignore an invalid submission_id""" SubmissionFactory() # be sure there is a Submission @@ -3349,18 +3333,6 @@ def test_process_submission_text_consistency_checks(self): with self.assertRaisesMessage(SubmissionError, 'disagrees with submission revision'): process_submission_text(submission.name, submission.rev) - # title mismatch - txt, _ = submission_file( - 'draft-somebody-test-00', # name that appears in the file - 'draft-somebody-test-00.xml', - None, - 'test_submission.txt', - title='Not Correct Draft Title', - ) - txt_path.open('w').write(txt.read()) - with self.assertRaisesMessage(SubmissionError, 'disagrees with submission title'): - process_submission_text(submission.name, submission.rev) - def test_status_of_validating_submission(self): s = SubmissionFactory(state_id='validating') url = urlreverse('ietf.submit.views.submission_status', kwargs={'submission_id': s.pk}) From 90035303947ec1531670698c80570ddf5c4f5a00 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 16:15:45 +0000 Subject: [PATCH 19/32] refactor: Break up submission_file() helper --- ietf/submit/tests.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 24bc34f582..ccb9759df9 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -91,7 +91,8 @@ def repository_dir(self): def archive_dir(self): return settings.INTERNET_DRAFT_ARCHIVE_DIR -def submission_file(name_in_doc, name_in_post, group, templatename, author=None, email=None, title=None, year=None, ascii=True): + +def submission_file_contents(name_in_doc, group, templatename, author=None, email=None, title=None, year=None, ascii=True): _today = date_today() # construct appropriate text draft f = io.open(os.path.join(settings.BASE_DIR, "submit", templatename)) @@ -128,10 +129,18 @@ def submission_file(name_in_doc, name_in_post, group, templatename, author=None, email=email, title=title, ) + return submission_text, author + + +def submission_file(name_in_doc, name_in_post, group, templatename, author=None, email=None, title=None, year=None, ascii=True): + submission_text, author = submission_file_contents( + name_in_doc, group, templatename, author, email, title, year, ascii + ) file = StringIO(submission_text) file.name = name_in_post return file, author + def create_draft_submission_with_rev_mismatch(rev='01'): """Create a draft and submission with mismatched version From add179bf328c31a93eccb23885d8eb94a413f4f5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 20:28:34 +0000 Subject: [PATCH 20/32] test: Refactor tests to run the async processing (wip) --- ....nonascii => test_submission.nonascii.txt} | 0 ietf/submit/tests.py | 133 +++++++----------- 2 files changed, 48 insertions(+), 85 deletions(-) rename ietf/submit/{test_submission.nonascii => test_submission.nonascii.txt} (100%) diff --git a/ietf/submit/test_submission.nonascii b/ietf/submit/test_submission.nonascii.txt similarity index 100% rename from ietf/submit/test_submission.nonascii rename to ietf/submit/test_submission.nonascii.txt diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index ccb9759df9..82a9dee04c 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -7,11 +7,12 @@ import io import os import re -import sys import mock +from faker import Faker from io import StringIO from pyquery import PyQuery +from typing import Tuple from pathlib import Path @@ -28,7 +29,8 @@ from ietf.submit.utils import (expirable_submissions, expire_submission, find_submission_filenames, post_submission, validate_submission_name, validate_submission_rev, - process_and_accept_uploaded_submission, SubmissionError, process_submission_text) + process_and_accept_uploaded_submission, SubmissionError, process_submission_text, + process_uploaded_submission) from ietf.doc.factories import (DocumentFactory, WgDraftFactory, IndividualDraftFactory, IndividualRfcFactory, ReviewFactory, WgRfcFactory) from ietf.doc.models import ( Document, DocAlias, DocEvent, State, @@ -181,62 +183,58 @@ def setUp(self): # Submit views assume there is a "next" IETF to look for cutoff dates against MeetingFactory(type_id='ietf', date=date_today()+datetime.timedelta(days=180)) - def create_and_post_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None): - """Helper to create and post a submission - - If base_filename is None, defaults to 'test_submission' + def _create_staged_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None, ascii=True): + """Create staged submission files that would exist following upload_submission() + + If base_filename is None, "test_submission" is used. """ - url = urlreverse('ietf.submit.views.upload_submission') - files = dict() - for format in formats: - fn = '.'.join((base_filename or 'test_submission', format)) - files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author) - - r = self.client.post(url, files) - if r.status_code != 302: - q = PyQuery(r.content) - print(q('div.invalid-feedback').text()) - self.assertNoFormPostErrors(r, ".invalid-feedback,.alert-danger") + submission_contents, _ = submission_file_contents( + name_in_doc=f'{name}-{rev}', + group=Group.objects.get(acronym=group) if group else None, + templatename='.'.join((base_filename or 'test_submission', format)), + author=author, + ascii=ascii, + ) + self._write_staged_file(f'{name}-{rev}.{format}', submission_contents) + return SubmissionFactory( + state_id="validating", + name=name, + rev=rev, + group=group, + remote_ip=Faker().ipv6, + file_types=",".join(f".{fmt}" for fmt in formats), + submission_date=date_today(), + ) - for format in formats: - self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, format)))) - if format == 'xml': - self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, 'html')))) - return r + def _write_staged_file(self, filename, contents): + """Helper to create a staged submission file in the right place""" + with open(Path(self.staging_dir) / filename, "w") as f: + f.write(contents) - def do_submission(self, name, rev, group=None, formats=["txt",], author=None): + def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",), author=None, base_filename=None, + ascii=True): + """Simulate uploading a draft and waiting for validation results + + Returns the "full access" status URL and the author associated with the submitted draft. + """ # break early in case of missing configuration self.assertTrue(os.path.exists(settings.IDSUBMIT_IDNITS_BINARY)) - # get - url = urlreverse('ietf.submit.views.upload_submission') - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - self.assertEqual(len(q('input[type=file][name=txt]')), 1) - self.assertEqual(len(q('input[type=file][name=xml]')), 1) - # submit if author is None: author = PersonFactory() - r = self.create_and_post_submission(name, rev, author, group, formats) - status_url = r["Location"] - self.assertEqual(Submission.objects.filter(name=name).count(), 1) - submission = Submission.objects.get(name=name) - if len(submission.authors) != 1: - sys.stderr.write("\nAuthor extraction failure.\n") - sys.stderr.write(force_str("Author name used in test: %s\n"%author)) - sys.stderr.write("Author ascii name: %s\n" % author.ascii) - sys.stderr.write("Author initials: %s\n" % author.initials()) - self.assertEqual(len(submission.authors), 1) - a = submission.authors[0] - self.assertEqual(a["name"], author.ascii_name()) - self.assertEqual(a["email"], author.email().address.lower()) - self.assertEqual(a["affiliation"], "Test Centre Inc.") - self.assertEqual(a["country"], "UK") + submission = self._create_staged_submission(name, rev, author, group, formats, base_filename, ascii) + process_uploaded_submission(submission) + status_url = urlreverse( + "ietf.submit.views.submission_status", + kwargs={ + "submission_id": submission.pk, + "access_token": submission.access_token(), + }, + ) return status_url, author def supply_extra_metadata(self, name, status_url, submitter_name, submitter_email, replaces, extresources=None): @@ -1731,25 +1729,11 @@ def test_submit_file_in_archive(self): def test_submit_nonascii_name(self): name = "draft-authorname-testing-nonascii" rev = "00" - group = None - # get - url = urlreverse('ietf.submit.views.upload_submission') - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - - # submit - #author = PersonFactory(name=u"Jörgen Nilsson".encode('latin1')) user = UserFactory(first_name="Jörgen", last_name="Nilsson") author = PersonFactory(user=user) - file, __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.nonascii", author=author, ascii=False) - files = {"txt": file } - - r = self.client.post(url, files) - self.assertEqual(r.status_code, 302) - status_url = r["Location"] + status_url, _ = self.do_submission(name=name, rev=rev, author=author, base_filename="test_submission.nonascii", ascii=False) r = self.client.get(status_url) q = PyQuery(r.content) m = q('p.alert-warning').text() @@ -1759,19 +1743,12 @@ def test_submit_nonascii_name(self): def test_submit_missing_author_email(self): name = "draft-authorname-testing-noemail" rev = "00" - group = None author = PersonFactory() for e in author.email_set.all(): e.delete() - files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission.txt", author=author, ascii=True)[0] } - - # submit - url = urlreverse('ietf.submit.views.upload_submission') - r = self.client.post(url, files) - self.assertEqual(r.status_code, 302) - status_url = r["Location"] + status_url, _ = self.do_submission(name=name, rev=rev, author=author) r = self.client.get(status_url) q = PyQuery(r.content) m = q('p.text-danger').text() @@ -1782,20 +1759,13 @@ def test_submit_missing_author_email(self): def test_submit_bad_author_email(self): name = "draft-authorname-testing-bademail" rev = "00" - group = None author = PersonFactory() email = author.email_set.first() email.address = '@bad.email' email.save() - files = {"xml": submission_file(f'{name}-{rev}',f'{name}-{rev}.xml', group, "test_submission.xml", author=author, ascii=False)[0] } - - # submit - url = urlreverse('ietf.submit.views.upload_submission') - r = self.client.post(url, files) - self.assertEqual(r.status_code, 302) - status_url = r["Location"] + status_url, _ = self.do_submission(name=name, rev=rev, author=author, formats=('xml',)) r = self.client.get(status_url) q = PyQuery(r.content) m = q('p.text-danger').text() @@ -1806,15 +1776,8 @@ def test_submit_bad_author_email(self): def test_submit_invalid_yang(self): name = "draft-yang-testing-invalid" rev = "00" - group = None - - # submit - files = {"txt": submission_file(f'{name}-{rev}', f'{name}-{rev}.txt', group, "test_submission_invalid_yang.txt")[0] } - url = urlreverse('ietf.submit.views.upload_submission') - r = self.client.post(url, files) - self.assertEqual(r.status_code, 302) - status_url = r["Location"] + status_url, _ = self.do_submission(name=name, rev=rev, base_filename="test_submission_invalid_yang") r = self.client.get(status_url) q = PyQuery(r.content) # From f621c6237a3ccdf4917765321bf12966345ebe48 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 20:38:30 +0000 Subject: [PATCH 21/32] test: Drop test of bad PDF submission The PDF submission field was removed, so no need to test it. --- ietf/submit/tests.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 82a9dee04c..18675b294c 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -1689,12 +1689,6 @@ def test_submit_bad_file_xml(self): self.assertIn('Expected the XML file to have extension ".xml"', m) self.assertIn('Expected an XML file of type "application/xml"', m) - def test_submit_bad_file_pdf(self): - r, q, m = self.submit_bad_file("some name", ["pdf"]) - self.assertIn('Invalid characters were found in the name', m) - self.assertIn('Expected the PDF file to have extension ".pdf"', m) - self.assertIn('Expected an PDF file of type "application/pdf"', m) - def test_submit_file_in_archive(self): name = "draft-authorname-testing-file-exists" rev = '00' From 70d92ce805df813b96d300922a24717992416887 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 4 May 2023 23:38:00 +0000 Subject: [PATCH 22/32] test: Update more tests --- ietf/submit/tests.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 18675b294c..93346583d6 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -188,10 +188,12 @@ def _create_staged_submission(self, name, rev, author, group=None, formats=("txt If base_filename is None, "test_submission" is used. """ + if isinstance(group, str): + group = Group.objects.get(acronym=group) for format in formats: submission_contents, _ = submission_file_contents( name_in_doc=f'{name}-{rev}', - group=Group.objects.get(acronym=group) if group else None, + group=group, templatename='.'.join((base_filename or 'test_submission', format)), author=author, ascii=ascii, @@ -1657,13 +1659,8 @@ def submit_bad_doc_name_with_ext(self, name_in_doc, name_in_post, formats): files[format].name = name_in_post r = self.client.post(url, files) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - self.assertTrue(len(q("form .invalid-feedback")) > 0) - m = q('div.invalid-feedback').text() - - return r, q, m + return r def test_submit_bad_file_txt(self): r, q, m = self.submit_bad_file("some name", ["txt"]) @@ -1673,15 +1670,15 @@ def test_submit_bad_file_txt(self): self.assertIn('document does not contain a legitimate name', m) def test_submit_bad_doc_name(self): - r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo.dot-bar", name_in_post="draft-foo.dot-bar", formats=["txt"]) - self.assertIn('contains a disallowed character with byte code: 46', m) + r = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo.dot-bar", name_in_post="draft-foo.dot-bar", formats=["txt"]) + self.assertContains(r, "contains a disallowed character with byte code: 46") # This actually is allowed by the existing code. A significant rework of the validation mechanics is needed. # r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo-bar-00.txt", name_in_post="draft-foo-bar-00.txt", formats=["txt"]) # self.assertIn('Did you include a filename extension in the name by mistake?', m) - r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo-bar-00.xml", name_in_post="draft-foo-bar-00.xml", formats=["xml"]) - self.assertIn('Did you include a filename extension in the name by mistake?', m) - r, q, m = self.submit_bad_doc_name_with_ext(name_in_doc="../malicious-name-in-content-00", name_in_post="../malicious-name-in-post-00.xml", formats=["xml"]) - self.assertIn('Did you include a filename extension in the name by mistake?', m) + r = self.submit_bad_doc_name_with_ext(name_in_doc="draft-foo-bar-00.xml", name_in_post="draft-foo-bar-00.xml", formats=["xml"]) + self.assertContains(r, "Could not extract a valid Internet-Draft revision from the XML") + r = self.submit_bad_doc_name_with_ext(name_in_doc="../malicious-name-in-content-00", name_in_post="../malicious-name-in-post-00.xml", formats=["xml"]) + self.assertContains(r, "Did you include a filename extension in the name by mistake?") def test_submit_bad_file_xml(self): r, q, m = self.submit_bad_file("some name", ["xml"]) From fd56a66ed55ee7d13079a11b8d0e3a9c7600672f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 02:26:07 +0000 Subject: [PATCH 23/32] test: Bring back create_and_post_submission() and fix more tests --- ietf/submit/tests.py | 51 +++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 93346583d6..b7bbcfc9b3 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -183,6 +183,38 @@ def setUp(self): # Submit views assume there is a "next" IETF to look for cutoff dates against MeetingFactory(type_id='ietf', date=date_today()+datetime.timedelta(days=180)) + def create_and_post_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None): + """Helper to create and post a submission + + If base_filename is None, defaults to 'test_submission'. + """ + url = urlreverse('ietf.submit.views.upload_submission') + files = dict() + + for format in formats: + fn = '.'.join((base_filename or 'test_submission', format)) + files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author) + + # Mock task so we can check that it's called without actually submitting a celery task. + # Also mock on_commit() because otherwise the test transaction prevents the call from + # ever being made. + with mock.patch("ietf.submit.views.process_uploaded_submission_task") as mocked_task: + with mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()): + r = self.client.post(url, files) + if r.status_code != 302: + q = PyQuery(r.content) + print(q('div.invalid-feedback').text()) + self.assertNoFormPostErrors(r, ".invalid-feedback,.alert-danger") + self.assertTrue(mocked_task.delay.called) + # Now process the submission like the task would do + process_uploaded_submission(Submission.objects.order_by('-pk').first()) + + for format in formats: + self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, format)))) + if format == 'xml': + self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, 'html')))) + return r + def _create_staged_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None, ascii=True): """Create staged submission files that would exist following upload_submission() @@ -1271,11 +1303,8 @@ def test_submit_cancel_confirmation(self): def test_submit_new_wg_with_dash(self): group = Group.objects.create(acronym="mars-special", name="Mars Special", type_id="wg", state_id="active") - name = "draft-ietf-%s-testing-tests" % group.acronym - - self.do_submission(name, "00") - + self.create_and_post_submission(name=name, rev="00", author=PersonFactory()) self.assertEqual(Submission.objects.get(name=name).group.acronym, group.acronym) def test_submit_new_wg_v2_country_only(self): @@ -1301,19 +1330,15 @@ def test_submit_new_wg_v2_country_only(self): def test_submit_new_irtf(self): group = Group.objects.create(acronym="saturnrg", name="Saturn", type_id="rg", state_id="active") - name = "draft-irtf-%s-testing-tests" % group.acronym - - self.do_submission(name, "00") - - self.assertEqual(Submission.objects.get(name=name).group.acronym, group.acronym) - self.assertEqual(Submission.objects.get(name=name).group.type_id, group.type_id) + self.create_and_post_submission(name=name, rev="00", author=PersonFactory()) + submission = Submission.objects.get(name=name) + self.assertEqual(submission.group.acronym, group.acronym) + self.assertEqual(submission.group.type_id, group.type_id) def test_submit_new_iab(self): name = "draft-iab-testing-tests" - - self.do_submission(name, "00") - + self.create_and_post_submission(name=name, rev="00", author=PersonFactory()) self.assertEqual(Submission.objects.get(name=name).group.acronym, "iab") def test_cancel_submission(self): From dc890b42bfd1113522e88b425400d002dc9b15d7 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 02:27:29 +0000 Subject: [PATCH 24/32] fix: Drop to manual, don't cancel, on revision inconsistency Fixes remaining failing SubmitTest tests --- ietf/submit/utils.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index 229cbcd65a..d91a575bf3 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -40,7 +40,7 @@ from ietf.person.models import Person, Email 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_approval_request, send_submission_confirmation, announce_new_wg_00, send_manual_post_request ) from ietf.submit.models import ( Submission, SubmissionEvent, Preapproval, DraftSubmissionStateName, SubmissionCheck, SubmissionExtResource ) from ietf.utils import log @@ -911,6 +911,9 @@ class SubmissionError(Exception): """Exception for errors during submission processing""" pass +class InconsistentRevisionError(SubmissionError): + """SubmissionError caused by an inconsistent revision""" + def staging_path(filename, revision, ext): if len(ext) > 0 and ext[0] != '.': @@ -1286,11 +1289,9 @@ def process_and_validate_submission(submission): submission.save() submission.formal_languages.set(text_metadata["formal_languages"]) - if check_submission_revision_consistency(submission): - raise SubmissionError( - 'Document revision inconsistency error in the database. ' - 'Please contact the secretariat for assistance.' - ) + consistency_error = check_submission_revision_consistency(submission) + if consistency_error: + raise InconsistentRevisionError(consistency_error) set_extresources_from_existing_draft(submission) apply_checkers( submission, @@ -1382,6 +1383,11 @@ def process_uploaded_submission(submission): try: process_and_validate_submission(submission) + except InconsistentRevisionError as consistency_error: + submission.state_id = "manual" + submission.save() + create_submission_event(None, submission, desc="Uploaded submission (diverted to manual process)") + send_manual_post_request(None, submission, errors=dict(consistency=str(consistency_error))) except SubmissionError as err: cancel_submission(submission) # changes Submission.state create_submission_event(None, submission, f"Submission rejected: {err}") From c86fb7e4e0bf0b8b383ae0101f7e4eb06109cfa5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 13:48:24 +0000 Subject: [PATCH 25/32] style: Restyle upload_submission() with black --- ietf/submit/views.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/ietf/submit/views.py b/ietf/submit/views.py index 77f386d146..c2fef14ba0 100644 --- a/ietf/submit/views.py +++ b/ietf/submit/views.py @@ -52,13 +52,15 @@ def upload_submission(request): - if request.method == 'POST': - form = SubmissionManualUploadForm(request, data=request.POST, files=request.FILES) + if request.method == "POST": + form = SubmissionManualUploadForm( + request, data=request.POST, files=request.FILES + ) if form.is_valid(): submission = get_submission(form) submission.state = DraftSubmissionStateName.objects.get(slug="validating") submission.remote_ip = form.remote_ip - submission.file_types = ','.join(form.file_types) + submission.file_types = ",".join(form.file_types) submission.submission_date = date_today() submission.save() clear_existing_files(form) @@ -68,15 +70,17 @@ def upload_submission(request): transaction.on_commit( lambda: process_uploaded_submission_task.delay(submission.pk) ) - return redirect("ietf.submit.views.submission_status", submission_id=submission.pk, - access_token=submission.access_token()) + return redirect( + "ietf.submit.views.submission_status", + submission_id=submission.pk, + access_token=submission.access_token(), + ) else: form = SubmissionManualUploadForm(request=request) - return render(request, 'submit/upload_submission.html', - {'selected': 'index', - 'form': form}) - + return render( + request, "submit/upload_submission.html", {"selected": "index", "form": form} + ) @csrf_exempt def api_submission(request): From 1e96058df85a3eb991b8250b6e3bca692cfd74d1 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 14:31:51 +0000 Subject: [PATCH 26/32] test: Verify that async submission processing is invoked on upload --- ietf/submit/tests.py | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index b7bbcfc9b3..37d9d8f57b 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -93,6 +93,26 @@ def repository_dir(self): def archive_dir(self): return settings.INTERNET_DRAFT_ARCHIVE_DIR + def post_to_upload_submission(self, *args, **kwargs): + """POST to the upload_submission endpoint + + Use this instead of directly POSTing to be sure that the appropriate celery + tasks would be queued (but are not actually queued during testing) + """ + # Mock task so we can check that it's called without actually submitting a celery task. + # Also mock on_commit() because otherwise the test transaction prevents the call from + # ever being made. + with mock.patch("ietf.submit.views.process_uploaded_submission_task") as mocked_task: + with mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()): + response = self.client.post(*args, **kwargs) + if response.status_code == 302: + # A 302 indicates we're being redirected to the status page, meaning the upload + # was accepted. Check that the task would have been queued. + self.assertTrue(mocked_task.delay.called) + else: + self.assertFalse(mocked_task.delay.called) + return response + def submission_file_contents(name_in_doc, group, templatename, author=None, email=None, title=None, year=None, ascii=True): _today = date_today() @@ -195,17 +215,11 @@ def create_and_post_submission(self, name, rev, author, group=None, formats=("tx fn = '.'.join((base_filename or 'test_submission', format)) files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author) - # Mock task so we can check that it's called without actually submitting a celery task. - # Also mock on_commit() because otherwise the test transaction prevents the call from - # ever being made. - with mock.patch("ietf.submit.views.process_uploaded_submission_task") as mocked_task: - with mock.patch("ietf.submit.views.transaction.on_commit", side_effect=lambda x: x()): - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) if r.status_code != 302: q = PyQuery(r.content) print(q('div.invalid-feedback').text()) self.assertNoFormPostErrors(r, ".invalid-feedback,.alert-danger") - self.assertTrue(mocked_task.delay.called) # Now process the submission like the task would do process_uploaded_submission(Submission.objects.order_by('-pk').first()) @@ -1664,7 +1678,7 @@ def submit_bad_file(self, name, formats): for format in formats: files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.bad', group, "test_submission.bad") - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) @@ -1683,7 +1697,7 @@ def submit_bad_doc_name_with_ext(self, name_in_doc, name_in_post, formats): files[format], author = submission_file(name_in_doc, name_in_post, group, "test_submission.%s" % format) files[format].name = name_in_post - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) self.assertEqual(r.status_code, 200) return r @@ -1734,7 +1748,7 @@ def test_submit_file_in_archive(self): with io.open(fn, 'w') as f: f.write("a" * 2000) files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format) - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) self.assertEqual(r.status_code, 200) q = PyQuery(r.content) @@ -2681,7 +2695,7 @@ def do_submission(self, name, rev, group=None, formats=["txt",]): for format in formats: files[format], author = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, "test_submission.%s" % format) - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) if r.status_code != 302: q = PyQuery(r.content) print(q('div.invalid-feedback span.form-text div').text()) @@ -3610,5 +3624,5 @@ def test_submit_case_conflited_name_fails(self): url = urlreverse("ietf.submit.views.upload_submission") files = {} files["xml"], _ = submission_file("draft-something-hascapitalletters-00", "draft-something-hascapitalletters-00.xml", None, "test_submission.xml") - r = self.client.post(url, files) + r = self.post_to_upload_submission(url, files) self.assertContains(r,"Case-conflicting draft name found",status_code=200) From 2743d52b9e489fd6725286e60a491b0df2834289 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 16:03:00 +0000 Subject: [PATCH 27/32] test: Bring back old do_submission and fix tests Properly separating the upload and async processing stages of submission is a bigger refactoring than will fit right now. This better exercises the submission pipeline. --- ietf/submit/test_submission_invalid_yang.txt | 2 +- ietf/submit/tests.py | 102 +++++++++---------- 2 files changed, 48 insertions(+), 56 deletions(-) diff --git a/ietf/submit/test_submission_invalid_yang.txt b/ietf/submit/test_submission_invalid_yang.txt index ef90b3fcb0..7b09706c32 100644 --- a/ietf/submit/test_submission_invalid_yang.txt +++ b/ietf/submit/test_submission_invalid_yang.txt @@ -2,7 +2,7 @@ -Network Working Group A. Name +Network Working Group %(firstpagename)37s Internet-Draft Test Centre Inc. Intended status: Informational %(month)s %(year)s Expires: %(expiration)s diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 37d9d8f57b..391ec3958f 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -5,9 +5,10 @@ import datetime import email import io +import mock import os import re -import mock +import sys from faker import Faker from io import StringIO @@ -203,7 +204,7 @@ def setUp(self): # Submit views assume there is a "next" IETF to look for cutoff dates against MeetingFactory(type_id='ietf', date=date_today()+datetime.timedelta(days=180)) - def create_and_post_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None): + def create_and_post_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None, ascii=True): """Helper to create and post a submission If base_filename is None, defaults to 'test_submission'. @@ -213,55 +214,30 @@ def create_and_post_submission(self, name, rev, author, group=None, formats=("tx for format in formats: fn = '.'.join((base_filename or 'test_submission', format)) - files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author) + files[format], __ = submission_file(f'{name}-{rev}', f'{name}-{rev}.{format}', group, fn, author=author, ascii=ascii) r = self.post_to_upload_submission(url, files) - if r.status_code != 302: + if r.status_code == 302: + # A redirect means the upload was accepted and queued for processing + process_submission = True + last_submission = Submission.objects.order_by("-pk").first() + self.assertEqual(last_submission.state_id, "validating") + else: + process_submission = False q = PyQuery(r.content) print(q('div.invalid-feedback').text()) self.assertNoFormPostErrors(r, ".invalid-feedback,.alert-danger") + # Now process the submission like the task would do - process_uploaded_submission(Submission.objects.order_by('-pk').first()) - - for format in formats: - self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, format)))) - if format == 'xml': - self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, 'html')))) + if process_submission: + process_uploaded_submission(Submission.objects.order_by('-pk').first()) + for format in formats: + self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, format)))) + if format == 'xml': + self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, 'html')))) return r - def _create_staged_submission(self, name, rev, author, group=None, formats=("txt",), base_filename=None, ascii=True): - """Create staged submission files that would exist following upload_submission() - - If base_filename is None, "test_submission" is used. - """ - if isinstance(group, str): - group = Group.objects.get(acronym=group) - for format in formats: - submission_contents, _ = submission_file_contents( - name_in_doc=f'{name}-{rev}', - group=group, - templatename='.'.join((base_filename or 'test_submission', format)), - author=author, - ascii=ascii, - ) - self._write_staged_file(f'{name}-{rev}.{format}', submission_contents) - return SubmissionFactory( - state_id="validating", - name=name, - rev=rev, - group=group, - remote_ip=Faker().ipv6, - file_types=",".join(f".{fmt}" for fmt in formats), - submission_date=date_today(), - ) - - def _write_staged_file(self, filename, contents): - """Helper to create a staged submission file in the right place""" - with open(Path(self.staging_dir) / filename, "w") as f: - f.write(contents) - - def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",), author=None, base_filename=None, - ascii=True): + def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",), author=None, base_filename=None, ascii=True): """Simulate uploading a draft and waiting for validation results Returns the "full access" status URL and the author associated with the submitted draft. @@ -269,20 +245,38 @@ def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",) # break early in case of missing configuration self.assertTrue(os.path.exists(settings.IDSUBMIT_IDNITS_BINARY)) + # get + url = urlreverse('ietf.submit.views.upload_submission') + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertEqual(len(q('input[type=file][name=txt]')), 1) + self.assertEqual(len(q('input[type=file][name=xml]')), 1) + # submit if author is None: author = PersonFactory() + r = self.create_and_post_submission( + name=name, rev=rev, author=author, group=group, formats=formats, base_filename=base_filename, ascii=ascii + ) + status_url = r["Location"] - submission = self._create_staged_submission(name, rev, author, group, formats, base_filename, ascii) - process_uploaded_submission(submission) + self.assertEqual(Submission.objects.filter(name=name).count(), 1) + submission = Submission.objects.get(name=name) + if len(submission.authors) != 1: + sys.stderr.write("\nAuthor extraction failure.\n") + sys.stderr.write(force_str("Author name used in test: %s\n"%author)) + sys.stderr.write("Author ascii name: %s\n" % author.ascii) + sys.stderr.write("Author initials: %s\n" % author.initials()) + self.assertEqual(len(submission.authors), 1) + a = submission.authors[0] + if ascii: + self.assertEqual(a["name"], author.ascii_name()) + if author.email(): + self.assertEqual(a["email"], author.email().address.lower()) + self.assertEqual(a["affiliation"], "Test Centre Inc.") + self.assertEqual(a["country"], "UK") - status_url = urlreverse( - "ietf.submit.views.submission_status", - kwargs={ - "submission_id": submission.pk, - "access_token": submission.access_token(), - }, - ) return status_url, author def supply_extra_metadata(self, name, status_url, submitter_name, submitter_email, replaces, extresources=None): @@ -1562,7 +1556,7 @@ def test_submit_all_file_types(self): rev = "00" group = "mars" - self.do_submission(name, rev, group, ["txt", "xml", "pdf"]) + self.do_submission(name, rev, group, ["txt", "xml"]) self.assertEqual(Submission.objects.filter(name=name).count(), 1) @@ -1571,8 +1565,6 @@ def test_submit_all_file_types(self): self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.xml" % (name, rev)))) self.assertTrue(name in io.open(os.path.join(self.staging_dir, "%s-%s.xml" % (name, rev))).read()) self.assertTrue('' in io.open(os.path.join(self.staging_dir, "%s-%s.xml" % (name, rev))).read()) - self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.pdf" % (name, rev)))) - self.assertTrue('This is PDF' in io.open(os.path.join(self.staging_dir, "%s-%s.pdf" % (name, rev))).read()) def test_expire_submissions(self): s = Submission.objects.create(name="draft-ietf-mars-foo", From 882b1b98965078df074654e4df17cd52ce5ebd7e Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 16:17:17 +0000 Subject: [PATCH 28/32] fix: Accept only XML for API submissions --- ietf/submit/tests.py | 17 ++++++++++++++++- ietf/submit/utils.py | 10 ++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 391ec3958f..47b91d9882 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -10,7 +10,6 @@ import re import sys -from faker import Faker from io import StringIO from pyquery import PyQuery from typing import Tuple @@ -3231,6 +3230,22 @@ def test_process_and_accept_uploaded_submission_invalid(self): self.assertEqual(submission.state_id, 'cancel') self.assertIn('Submission rejected: XML Internet-Draft revision', submission.submissionevent_set.last().desc) + # not xml + submission = SubmissionFactory( + name='draft-somebody-test', + rev='00', + file_types='.txt', + submitter=author.formatted_email(), + state_id='validating', + ) + txt_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.txt' + with txt_path.open('w') as f: + f.write(txt_data) + process_and_accept_uploaded_submission(submission) + submission = Submission.objects.get(pk=submission.pk) # refresh + self.assertEqual(submission.state_id, 'cancel') + self.assertIn('Only XML Internet-Draft submissions', submission.submissionevent_set.last().desc) + # wrong state submission = SubmissionFactory( name='draft-somebody-test', diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index d91a575bf3..c642baf1fc 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1342,6 +1342,16 @@ def process_and_accept_uploaded_submission(submission): log.log(f'Submission {submission.pk} is not in "validating" state, skipping.') return # do nothing + if submission.file_types != '.xml': + # permit only XML uploads for automatic acceptance + cancel_submission(submission) + create_submission_event( + None, + submission, + "Only XML Internet-Draft submissions can be processed.", + ) + return + try: process_and_validate_submission(submission) except SubmissionError as err: From c0fc6d71cf4e82aef91396ba78ded4c5f2e447af Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 5 May 2023 19:18:21 +0000 Subject: [PATCH 29/32] test: Test submission processing utilities --- ietf/submit/tests.py | 169 +++++++++++++++++++++++++++++++++++++------ ietf/submit/utils.py | 4 +- 2 files changed, 150 insertions(+), 23 deletions(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index 47b91d9882..d1009b6c16 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -30,7 +30,8 @@ from ietf.submit.utils import (expirable_submissions, expire_submission, find_submission_filenames, post_submission, validate_submission_name, validate_submission_rev, process_and_accept_uploaded_submission, SubmissionError, process_submission_text, - process_uploaded_submission) + process_submission_xml, process_uploaded_submission, + process_and_validate_submission) from ietf.doc.factories import (DocumentFactory, WgDraftFactory, IndividualDraftFactory, IndividualRfcFactory, ReviewFactory, WgRfcFactory) from ietf.doc.models import ( Document, DocAlias, DocEvent, State, @@ -3309,38 +3310,162 @@ def test_process_and_accept_uploaded_submission_task_ignores_invalid_id(self, mo process_and_accept_uploaded_submission_task(bad_pk) self.assertEqual(mock_method.call_count, 0) - def test_process_submission_text_consistency_checks(self): - """process_submission_text should check draft metadata against submission""" - submission = SubmissionFactory( - name='draft-somebody-test', - rev='00', - title='Correct Draft Title', + def test_process_submission_xml(self): + xml_path = Path(settings.IDSUBMIT_STAGING_PATH) / "draft-somebody-test-00.xml" + xml, _ = submission_file( + "draft-somebody-test-00", + "draft-somebody-test-00.xml", + None, + "test_submission.xml", + title="Correct Draft Title", ) - txt_path = Path(settings.IDSUBMIT_STAGING_PATH) / 'draft-somebody-test-00.txt' + xml_path.write_text(xml.read()) + output = process_submission_xml("draft-somebody-test", "00") + self.assertEqual(output["filename"], "draft-somebody-test") + self.assertEqual(output["rev"], "00") + self.assertEqual(output["title"], "Correct Draft Title") + self.assertIsNone(output["abstract"]) + self.assertEqual(len(output["authors"]), 1) # not checking in detail, parsing is unreliable + self.assertIsNone(output["document_date"]) + self.assertIsNone(output["pages"]) + self.assertIsNone(output["words"]) + self.assertIsNone(output["first_two_pages"]) + self.assertIsNone(output["file_size"]) + self.assertIsNone(output["formal_languages"]) + self.assertEqual(output["xml_version"], "3") # name mismatch + xml, _ = submission_file( + "draft-somebody-wrong-name-00", # name that appears in the file + "draft-somebody-test-00.xml", + None, + "test_submission.xml", + title="Correct Draft Title", + ) + xml_path.write_text(xml.read()) + with self.assertRaisesMessage(SubmissionError, "disagrees with submission filename"): + process_submission_xml("draft-somebody-test", "00") + + # rev mismatch + xml, _ = submission_file( + "draft-somebody-test-01", # name that appears in the file + "draft-somebody-test-00.xml", + None, + "test_submission.xml", + title="Correct Draft Title", + ) + xml_path.write_text(xml.read()) + with self.assertRaisesMessage(SubmissionError, "disagrees with submission revision"): + process_submission_xml("draft-somebody-test", "00") + + # missing title + xml, _ = submission_file( + "draft-somebody-test-00", # name that appears in the file + "draft-somebody-test-00.xml", + None, + "test_submission.xml", + title="", + ) + xml_path.write_text(xml.read()) + with self.assertRaisesMessage(SubmissionError, "Could not extract a valid title"): + process_submission_xml("draft-somebody-test", "00") + + def test_process_submission_text(self): + txt_path = Path(settings.IDSUBMIT_STAGING_PATH) / "draft-somebody-test-00.txt" txt, _ = submission_file( - 'draft-somebody-wrong-name-00', # name that appears in the file - 'draft-somebody-test-00.xml', + "draft-somebody-test-00", + "draft-somebody-test-00.txt", None, - 'test_submission.txt', - title='Correct Draft Title', + "test_submission.txt", + title="Correct Draft Title", ) - txt_path.open('w').write(txt.read()) - with self.assertRaisesMessage(SubmissionError, 'disagrees with submission filename'): - process_submission_text(submission.name, submission.rev) + txt_path.write_text(txt.read()) + output = process_submission_text("draft-somebody-test", "00") + self.assertEqual(output["filename"], "draft-somebody-test") + self.assertEqual(output["rev"], "00") + self.assertEqual(output["title"], "Correct Draft Title") + self.assertEqual(output["abstract"].strip(), "This document describes how to test tests.") + self.assertEqual(len(output["authors"]), 1) # not checking in detail, parsing is unreliable + self.assertLessEqual(output["document_date"] - date_today(), datetime.timedelta(days=1)) + self.assertEqual(output["pages"], 2) + self.assertGreater(output["words"], 0) # make sure it got something + self.assertGreater(len(output["first_two_pages"]), 0) # make sure it got something + self.assertGreater(output["file_size"], 0) # make sure it got something + self.assertEqual(output["formal_languages"].count(), 1) + self.assertIsNone(output["xml_version"]) + + # name mismatch + txt, _ = submission_file( + "draft-somebody-wrong-name-00", # name that appears in the file + "draft-somebody-test-00.txt", + None, + "test_submission.txt", + title="Correct Draft Title", + ) + txt_path.write_text(txt.read()) + with self.assertRaisesMessage(SubmissionError, "disagrees with submission filename"): + process_submission_text("draft-somebody-test", "00") # rev mismatch txt, _ = submission_file( - 'draft-somebody-test-01', # name that appears in the file - 'draft-somebody-test-00.xml', + "draft-somebody-test-01", # name that appears in the file + "draft-somebody-test-00.txt", None, - 'test_submission.txt', - title='Correct Draft Title', + "test_submission.txt", + title="Correct Draft Title", + ) + txt_path.write_text(txt.read()) + with self.assertRaisesMessage(SubmissionError, "disagrees with submission revision"): + process_submission_text("draft-somebody-test", "00") + + def test_process_and_validate_submission(self): + xml_data = { + "title": "The Title", + "authors": [{ + "name": "Jane Doe", + "email": "jdoe@example.com", + "affiliation": "Test Centre", + "country": "UK", + }], + "xml_version": "3", + } + text_data = { + "title": "The Title", + "abstract": "This is an abstract.", + "authors": [{ + "name": "John Doh", + "email": "ignored@example.com", + "affiliation": "Ignored", + "country": "CA", + }], + "document_date": date_today(), + "pages": 25, + "words": 1234, + "first_two_pages": "Pages One and Two", + "file_size": 4321, + "formal_languages": FormalLanguageName.objects.none(), + } + submission = SubmissionFactory( + state_id="validating", + file_types=".xml,.txt", ) - txt_path.open('w').write(txt.read()) - with self.assertRaisesMessage(SubmissionError, 'disagrees with submission revision'): - process_submission_text(submission.name, submission.rev) + with mock.patch("ietf.submit.utils.process_submission_xml", return_value=xml_data): + with mock.patch("ietf.submit.utils.process_submission_text", return_value=text_data): + with mock.patch("ietf.submit.utils.render_missing_formats") as mock_render: + with mock.patch("ietf.submit.utils.apply_checkers") as mock_checkers: + process_and_validate_submission(submission) + self.assertTrue(mock_render.called) + self.assertTrue(mock_checkers.called) + submission = Submission.objects.get(pk=submission.pk) + self.assertEqual(submission.title, text_data["title"]) + self.assertEqual(submission.abstract, text_data["abstract"]) + self.assertEqual(submission.authors, xml_data["authors"]) + self.assertEqual(submission.document_date, text_data["document_date"]) + self.assertEqual(submission.pages, text_data["pages"]) + self.assertEqual(submission.words, text_data["words"]) + self.assertEqual(submission.first_two_pages, text_data["first_two_pages"]) + self.assertEqual(submission.file_size, text_data["file_size"]) + self.assertEqual(submission.xml_version, xml_data["xml_version"]) def test_status_of_validating_submission(self): s = SubmissionFactory(state_id='validating') diff --git a/ietf/submit/utils.py b/ietf/submit/utils.py index c642baf1fc..3f12e50c5e 100644 --- a/ietf/submit/utils.py +++ b/ietf/submit/utils.py @@ -1215,7 +1215,9 @@ def process_submission_text(filename, revision): ) title = _normalize_title(text_draft.get_title()) if not title: - raise SubmissionError("Could not extract a valid title from the text") + # This test doesn't work well - the text_draft parser tends to grab "Abstract" as + # the title if there's an empty title. + raise SubmissionError("Could not extract a title from the text") # Drops \r, \n, <, >. Based on get_draft_meta() behavior trans_table = str.maketrans("", "", "\r\n<>") From 3b340337b349d375a371c6dd759092a946ae5c47 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 9 May 2023 12:28:23 -0300 Subject: [PATCH 30/32] feat: Improve status display for "validating" submissions --- ietf/static/js/draft-submit.js | 4 ++++ ietf/templates/submit/submission_status.html | 13 +++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ietf/static/js/draft-submit.js b/ietf/static/js/draft-submit.js index 7643597a42..4d813a7fbe 100644 --- a/ietf/static/js/draft-submit.js +++ b/ietf/static/js/draft-submit.js @@ -68,20 +68,24 @@ $(function () { // Reload page periodically if the enableAutoReload checkbox is present and checked const autoReloadSwitch = document.getElementById("enableAutoReload"); + const timeSinceDisplay = document.getElementById("time-since-uploaded"); if (autoReloadSwitch) { const autoReloadTime = 30000; // ms let autoReloadTimeoutId; autoReloadSwitch.parentElement.classList.remove("d-none"); + timeSinceDisplay.classList.remove("d-none"); autoReloadTimeoutId = setTimeout(() => location.reload(), autoReloadTime); autoReloadSwitch.addEventListener("change", (e) => { if (e.currentTarget.checked) { if (!autoReloadTimeoutId) { autoReloadTimeoutId = setTimeout(() => location.reload(), autoReloadTime); + timeSinceDisplay.classList.remove("d-none"); } } else { if (autoReloadTimeoutId) { clearTimeout(autoReloadTimeoutId); autoReloadTimeoutId = null; + timeSinceDisplay.classList.add("d-none"); } } }); diff --git a/ietf/templates/submit/submission_status.html b/ietf/templates/submit/submission_status.html index f77a653e83..fba1023c5d 100644 --- a/ietf/templates/submit/submission_status.html +++ b/ietf/templates/submit/submission_status.html @@ -133,12 +133,21 @@

Submission checks

This submission is awaiting the first Internet-Draft upload.

{% elif submission.state_id == 'validating' %} +
+ Notice: The Internet-Draft submission process has changed as of Datatracker version 10.3.0. + Your Internet-Draft is currently being processed and validated asynchronously. Results will be + displayed at this URL when they are available. If JavaScript is enabled in your + browser, this page will refreshed automatically. If JavaScript is not enabled, or if you + disable the automatic refresh with the toggle below, please reload this page in a few + minutes to see the results. +
- This submission is still being processed and validated. This normally takes a few minutes after + This submission is being processed and validated. This normally takes a few minutes after submission. {% with earliest_event=submission.submissionevent_set.last %} {% if earliest_event %} - It has been {{ earliest_event.time|timesince }} since submission. + Your draft was uploaded at {{ earliest_event.time }} + ({{ earliest_event.time|timesince }} ago). {% endif %} {% endwith %} Please contact the secretariat for assistance if it has been more than an hour. From fa9ce913b80b432253a93da42ed1114c51d8c6eb Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 9 May 2023 12:34:39 -0300 Subject: [PATCH 31/32] chore: Remove obsolete code --- ietf/submit/forms.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/ietf/submit/forms.py b/ietf/submit/forms.py index 557478a86b..407a02206d 100644 --- a/ietf/submit/forms.py +++ b/ietf/submit/forms.py @@ -36,7 +36,6 @@ from ietf.name.models import FormalLanguageName, GroupTypeName from ietf.submit.models import Submission, Preapproval from ietf.submit.utils import validate_submission_name, validate_submission_rev, validate_submission_document_date, remote_ip -from ietf.submit.parsers.pdf_parser import PDFParser from ietf.submit.parsers.plain_parser import PlainParser from ietf.submit.parsers.xml_parser import XMLParser from ietf.utils import log @@ -625,26 +624,6 @@ def cleanup(): # called when context exited, even in case of exception return super().clean() -class DeprecatedSubmissionManualUploadForm(DeprecatedSubmissionBaseUploadForm): - xml = forms.FileField(label='.xml format', required=False) # xml field with required=False instead of True - txt = forms.FileField(label='.txt format', required=False) - # We won't permit html upload until we can verify that the content - # reasonably matches the text and/or xml upload. Till then, we generate - # html for version 3 xml submissions. - # html = forms.FileField(label='.html format', required=False) - pdf = forms.FileField(label='.pdf format', required=False) - - def __init__(self, request, *args, **kwargs): - super(DeprecatedSubmissionManualUploadForm, self).__init__(request, *args, **kwargs) - self.formats = settings.IDSUBMIT_FILE_TYPES - self.base_formats = ['txt', 'xml', ] - - def clean_txt(self): - return self._clean_file("txt", PlainParser) - - def clean_pdf(self): - return self._clean_file("pdf", PDFParser) - class DeprecatedSubmissionAutoUploadForm(DeprecatedSubmissionBaseUploadForm): """Full-service upload form, replaced by the asynchronous version""" user = forms.EmailField(required=True) From 7b62682cc16028b497552c65b33d4a948e96d357 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 9 May 2023 13:47:47 -0300 Subject: [PATCH 32/32] test: Update test to match amended text --- ietf/submit/tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/submit/tests.py b/ietf/submit/tests.py index d1009b6c16..9456fa3f93 100644 --- a/ietf/submit/tests.py +++ b/ietf/submit/tests.py @@ -3472,7 +3472,7 @@ def test_status_of_validating_submission(self): url = urlreverse('ietf.submit.views.submission_status', kwargs={'submission_id': s.pk}) r = self.client.get(url) self.assertContains(r, s.name) - self.assertContains(r, 'still being processed and validated', status_code=200) + self.assertContains(r, 'This submission is being processed and validated.', status_code=200) @override_settings(IDSUBMIT_MAX_VALIDATION_TIME=datetime.timedelta(minutes=30)) def test_cancel_stale_submissions(self):