From 8c077409f9ab31e6be83c9bfdfb69913ddac88ba Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 14:31:06 -0400 Subject: [PATCH 1/9] feat: investigate docs asynchronously --- ietf/doc/forms.py | 1 + ietf/doc/tasks.py | 9 + ietf/doc/views_doc.py | 79 +++++++-- ietf/settings.py | 15 +- ietf/templates/doc/investigate.html | 249 +++++++++++++++++----------- requirements.txt | 1 + 6 files changed, 246 insertions(+), 108 deletions(-) diff --git a/ietf/doc/forms.py b/ietf/doc/forms.py index f77b2183186..8a1e9ecb986 100644 --- a/ietf/doc/forms.py +++ b/ietf/doc/forms.py @@ -276,6 +276,7 @@ class InvestigateForm(forms.Form): ), min_length=8, ) + task_id = forms.CharField(required=False, widget=forms.HiddenInput) def clean_name_fragment(self): disallowed_characters = ["%", "/", "\\", "*"] diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py index f1de459dd84..5b1b4f0b6bc 100644 --- a/ietf/doc/tasks.py +++ b/ietf/doc/tasks.py @@ -31,6 +31,7 @@ generate_idnits2_rfcs_obsoleted, update_or_create_draft_bibxml_file, ensure_draft_bibxml_path_exists, + investigate_fragment, ) @@ -119,3 +120,11 @@ def generate_draft_bibxml_files_task(days=7, process_all=False): update_or_create_draft_bibxml_file(event.doc, event.rev) except Exception as err: log.log(f"Error generating bibxml for {event.doc.name}-{event.rev}: {err}") + + +@shared_task(ignore_result=False) +def investigate_fragment_task(name_fragment: str): + return { + "name_fragment": name_fragment, + "results": investigate_fragment(name_fragment) + } diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 9f7cf12bcb5..f504e050b34 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -41,10 +41,11 @@ from pathlib import Path +from celery.result import AsyncResult from django.core.cache import caches from django.core.exceptions import PermissionDenied from django.db.models import Max -from django.http import HttpResponse, Http404, HttpResponseBadRequest +from django.http import HttpResponse, Http404, HttpResponseBadRequest, JsonResponse from django.shortcuts import render, get_object_or_404, redirect from django.template.loader import render_to_string from django.urls import reverse as urlreverse @@ -57,21 +58,22 @@ from ietf.doc.models import ( Document, DocHistory, DocEvent, BallotDocEvent, BallotType, ConsensusDocEvent, NewRevisionDocEvent, TelechatDocEvent, WriteupDocEvent, IanaExpertDocEvent, - IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder, DocumentAuthor, - RelatedDocument, RelatedDocHistory) + IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder, DocumentAuthor, + RelatedDocument, RelatedDocHistory) +from ietf.doc.tasks import investigate_fragment_task from ietf.doc.utils import (augment_events_with_revision, - can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, investigate_fragment, - needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, - get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, - add_events_message_info, get_unicode_document_content, + can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, + needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, + get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, + add_events_message_info, get_unicode_document_content, augment_docs_and_person_with_person_info, irsg_needed_ballot_positions, add_action_holder_change_event, - build_file_urls, update_documentauthors, fuzzy_find_documents, - bibxml_for_draft, get_doc_email_aliases) + build_file_urls, update_documentauthors, fuzzy_find_documents, + bibxml_for_draft, get_doc_email_aliases) from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible from ietf.group.models import Role, Group from ietf.group.utils import can_manage_all_groups_of_type, can_manage_materials, group_features_role_filter from ietf.ietfauth.utils import ( has_role, is_authorized_in_doc_stream, user_is_person, - role_required, is_individual_draft_author, can_request_rfc_publication) + role_required, is_individual_draft_author, can_request_rfc_publication) from ietf.name.models import StreamName, BallotPositionName from ietf.utils.history import find_history_active_at from ietf.doc.views_ballot import parse_ballot_edit_return_point @@ -2275,16 +2277,65 @@ def idnits2_state(request, name, rev=None): content_type="text/plain;charset=utf-8", ) + @role_required("Secretariat") def investigate(request): + """Investigate a fragment + + A plain GET with no querystring returns the UI page. + + POST with the task_id field empty starts an async task and returns a JSON response with + the ID needed to monitor the task for results. + + GET with a querystring parameter "id" will poll the status of the async task and return "ready" + or "notready". + + POST with the task_id field set to the id of a "ready" task will return its results or an error + if the task failed or the id is invalid (expired, never exited, etc). + """ results = None + # Start an investigation or retrieve a result on a POST if request.method == "POST": form = InvestigateForm(request.POST) if form.is_valid(): - name_fragment = form.cleaned_data["name_fragment"] - results = investigate_fragment(name_fragment) + task_id = form.cleaned_data["task_id"] + if task_id: + # Ignore the rest of the form and retrieve the result + task_result = AsyncResult(task_id) + if task_result.successful(): + retval = task_result.get() + results = retval["results"] + form.data = form.data.copy() + form.data["name_fragment"] = retval["name_fragment"] # ensure consistency + del form.data["task_id"] # do not request the task result again + else: + form.add_error( + None, + "The investigation task failed. Please try again and ask for help if this recurs.", + ) + # Falls through to the render at the end! + else: + name_fragment = form.cleaned_data["name_fragment"] + task_result = investigate_fragment_task.delay(name_fragment) + return JsonResponse({"id": task_result.id}) else: - form = InvestigateForm() + task_id = request.GET.get("id", None) + if task_id is not None: + # Check status if we got the "id" parameter + task_result = AsyncResult(task_id) + return JsonResponse({ + "status": "ready" if task_result.ready() else "notready" + }) + else: + # Serve up an empty form + form = InvestigateForm() + + # If we get here, it is just a plain GET - serve the UI return render( - request, "doc/investigate.html", context=dict(form=form, results=results) + request, + "doc/investigate.html", + context={ + "form": form, + "results": results, + }, ) diff --git a/ietf/settings.py b/ietf/settings.py index b452864be67..a1dc9ffe215 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -452,6 +452,7 @@ def skip_unreadable_post(record): 'django_vite', 'django_bootstrap5', 'django_celery_beat', + 'django_celery_results', 'corsheaders', 'django_markup', 'oidc_provider', @@ -1226,7 +1227,9 @@ def skip_unreadable_post(record): # https://docs.celeryq.dev/en/stable/userguide/tasks.html#rpc-result-backend-rabbitmq-qpid # Results can be retrieved only once and only by the caller of the task. Results will be # lost if the message broker restarts. -CELERY_RESULT_BACKEND = 'rpc://' # sends a msg via the msg broker +CELERY_RESULT_BACKEND = 'django-cache' # use a Django cache for results +CELERY_CACHE_BACKEND = 'celery-results' # which Django cache to use +CELERY_RESULT_EXPIRES = datetime.timedelta(minutes=5) # how long are results valid? (Default is 1 day) CELERY_TASK_IGNORE_RESULT = True # ignore results unless specifically enabled for a task # Meetecho API setup: Uncomment this and provide real credentials to enable @@ -1309,6 +1312,11 @@ def skip_unreadable_post(record): "MAX_ENTRIES": 5000, }, }, + "celery-results": { + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", + "LOCATION": f"{MEMCACHED_HOST}:{MEMCACHED_PORT}", + "KEY_PREFIX": "ietf:celery", + }, } else: CACHES = { @@ -1347,6 +1355,11 @@ def skip_unreadable_post(record): "MAX_ENTRIES": 5000, }, }, + "celery-results": { + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", + "LOCATION": "app:11211", + "KEY_PREFIX": "ietf:celery", + }, } PUBLISH_IPR_STATES = ['posted', 'removed', 'removed_objfalse'] diff --git a/ietf/templates/doc/investigate.html b/ietf/templates/doc/investigate.html index bdcf644406b..ac8e0d26a78 100644 --- a/ietf/templates/doc/investigate.html +++ b/ietf/templates/doc/investigate.html @@ -6,112 +6,175 @@ {% endblock %} {% block content %} - {% origin %} -

Investigate

-
- {% csrf_token %} - {% bootstrap_form form %} - -
- {% if results %} -
- {% if results.can_verify %} -

These can be authenticated

- - - - - - - - - - - {% for path in results.can_verify %} - {% with url=path|url_for_path %} - + {% origin %} +

Investigate

+
+
+ {% csrf_token %} + {% bootstrap_form form %} + + +
+ {% if results %} +
+ {% if results.can_verify %} +

These can be authenticated

+
NameLast Modified OnLinkSource
+ + + + + + + + + + {% for path in results.can_verify %} + {% with url=path|url_for_path %} + - + {% endif %} + - - {% endwith %} - {% endfor %} - -
NameLast Modified OnLinkSource
{{path.name}} - {% if path|mtime_is_epoch %} - Timestamp has been lost (is Unix Epoch) - {% else %} + + {% if path|mtime_is_epoch %} + Timestamp has been lost (is Unix Epoch) + {% else %} {{path|mtime|date:"DATETIME_FORMAT"}} - {% endif %} - {{url}} {{path}}
- {% else %} -

Nothing with this name fragment can be authenticated

- {% endif %} -
- {% if results.unverifiable_collections %} -

These are in the archive, but cannot be authenticated

- - - - - - - - - - - {% for path in results.unverifiable_collections %} - {% with url=path|url_for_path %} - + + {% endwith %} + {% endfor %} + +
NameLast Modified OnLinkSource
+ {% else %} +

Nothing with this name fragment can be authenticated

+ {% endif %} +
+ {% if results.unverifiable_collections %} +

These are in the archive, but cannot be authenticated

+ + + + + + + + + + + {% for path in results.unverifiable_collections %} + {% with url=path|url_for_path %} + - + {% endif %} + - - {% endwith %} - {% endfor %} - -
NameLast Modified OnLinkSource
{{path.name}} - {% if path|mtime_is_epoch %} - Timestamp has been lost (is Unix Epoch) - {% else %} + + {% if path|mtime_is_epoch %} + Timestamp has been lost (is Unix Epoch) + {% else %} {{path|mtime|date:"DATETIME_FORMAT"}} - {% endif %} - {{url}} {{path}}
- {% endif %} - {% if results.unexpected %} -

These are unexpected and we do not know what their origin is. These cannot be authenticated

- - - - - - - - - - {% for path in results.unexpected %} - {% with url=path|url_for_path %} - + + {% endwith %} + {% endfor %} + +
NameLast Modified OnLink
+ {% endif %} + {% if results.unexpected %} +

These are unexpected and we do not know what their origin is. These cannot be authenticated

+ + + + + + + + + + {% for path in results.unexpected %} + {% with url=path|url_for_path %} + - + {% endif %} + - - {% endwith %} - {% endfor %} - -
NameLast Modified OnLink
{{path.name}} - {% if path|mtime_is_epoch %} - Timestamp has been lost (is Unix Epoch) - {% else %} + + {% if path|mtime_is_epoch %} + Timestamp has been lost (is Unix Epoch) + {% else %} {{path|mtime|date:"DATETIME_FORMAT"}} - {% endif %} - {{url}}
- {% endif %} + + {% endwith %} + {% endfor %} + + + {% endif %} +
+ {% endif %} - {% endif %} {% endblock %} {% block js %} + {% endblock %} \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index ec5fc60b5fa..ae8b06faeed 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,6 +13,7 @@ Django>4.2,<5 django-analytical>=3.1.0 django-bootstrap5>=21.3 django-celery-beat>=2.3.0 +django-celery-results>=2.5.1 django-csp>=3.7 django-cors-headers>=3.11.0 django-debug-toolbar>=3.2.4 From 8a7a022c2f27037c45b7684a5c5bffa870497c2f Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 14:36:25 -0400 Subject: [PATCH 2/9] refactor: move script to its own js file --- ietf/static/js/investigate.js | 52 +++++++++++++++++++++++++++ ietf/templates/doc/investigate.html | 54 +---------------------------- package.json | 1 + 3 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 ietf/static/js/investigate.js diff --git a/ietf/static/js/investigate.js b/ietf/static/js/investigate.js new file mode 100644 index 00000000000..8e5fe52c78f --- /dev/null +++ b/ietf/static/js/investigate.js @@ -0,0 +1,52 @@ +// Copyright The IETF Trust 2025, All Rights Reserved +document.addEventListener('DOMContentLoaded', () => { + const investigateForm = document.forms['investigate'] + investigateForm.addEventListener('submit', (event) => { + // Intercept submission unless we've filled in the task_id field + if (!investigateForm.elements['id_task_id'].value) { + event.preventDefault() + runInvestigation() + } + }) + + const runInvestigation = async () => { + // Submit the request + const response = await fetch('', { + method: investigateForm.method, + body: new FormData(investigateForm) + }) + if (!response.ok) { + loadResultsFromTask('bogus-task-id') // bad task id will generate an error from Django + } + const taskId = (await response.json()).id + waitForResults(taskId, 10) // Poll for completion of the investigation + } + + const waitForResults = async (taskId, retries) => { + // indicate that investigation is in progress + document.getElementById('spinner').classList.remove('d-none') + document.getElementById('investigate-button').disabled = true + investigateForm.elements['id_name_fragment'].disabled = true + + const response = await fetch('?' + new URLSearchParams({ id: taskId })) + if (!response.ok) { + loadResultsFromTask('bogus-task-id') // bad task id will generate an error from Django + } + const result = await response.json() + if (result.status !== 'ready' && retries > 0) { + setTimeout(waitForResults, 5000, taskId, retries - 1) + } else { + /* Either the response is ready or we timed out waiting. In either case, submit + the task_id via POST and let Django display an error if it's not ready. Before + submitting, re-enable the form fields so the POST is valid. Other in-progress + indicators will be reset when the POST response is loaded. */ + loadResultsFromTask(taskId) + } + } + + const loadResultsFromTask = (taskId) => { + investigateForm.elements['id_name_fragment'].disabled = false + investigateForm.elements['id_task_id'].value = taskId + investigateForm.submit() + } +}) diff --git a/ietf/templates/doc/investigate.html b/ietf/templates/doc/investigate.html index ac8e0d26a78..f53f227dcbd 100644 --- a/ietf/templates/doc/investigate.html +++ b/ietf/templates/doc/investigate.html @@ -124,57 +124,5 @@

These are unexpected and we do not know what their origin is. These cannot b {% endblock %} {% block js %} - + {% endblock %} \ No newline at end of file diff --git a/package.json b/package.json index afcabd13edb..b3d36b349c2 100644 --- a/package.json +++ b/package.json @@ -132,6 +132,7 @@ "ietf/static/js/highcharts.js", "ietf/static/js/highstock.js", "ietf/static/js/ietf.js", + "ietf/static/js/investigate.js", "ietf/static/js/ipr-edit.js", "ietf/static/js/ipr-search.js", "ietf/static/js/js-cookie.js", From 521def0680bf30d987b1494eecff949043c9f34e Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 14:38:41 -0400 Subject: [PATCH 3/9] fix: adjust polling interval/duration --- ietf/static/js/investigate.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ietf/static/js/investigate.js b/ietf/static/js/investigate.js index 8e5fe52c78f..3737ee35f50 100644 --- a/ietf/static/js/investigate.js +++ b/ietf/static/js/investigate.js @@ -19,7 +19,8 @@ document.addEventListener('DOMContentLoaded', () => { loadResultsFromTask('bogus-task-id') // bad task id will generate an error from Django } const taskId = (await response.json()).id - waitForResults(taskId, 10) // Poll for completion of the investigation + // Poll for completion of the investigation up to 18*10 = 180 seconds + waitForResults(taskId, 18) } const waitForResults = async (taskId, retries) => { @@ -34,7 +35,8 @@ document.addEventListener('DOMContentLoaded', () => { } const result = await response.json() if (result.status !== 'ready' && retries > 0) { - setTimeout(waitForResults, 5000, taskId, retries - 1) + // 10 seconds per retry + setTimeout(waitForResults, 10000, taskId, retries - 1) } else { /* Either the response is ready or we timed out waiting. In either case, submit the task_id via POST and let Django display an error if it's not ready. Before From a0510fc29dad6df8818089f99fa7be92161e4be4 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 14:54:25 -0400 Subject: [PATCH 4/9] test: test new task --- ietf/doc/tests_tasks.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py index 67997acd859..ae7d9f36fb3 100644 --- a/ietf/doc/tests_tasks.py +++ b/ietf/doc/tests_tasks.py @@ -20,6 +20,7 @@ generate_draft_bibxml_files_task, generate_idnits2_rfcs_obsoleted_task, generate_idnits2_rfc_status_task, + investigate_fragment_task, notify_expirations_task, ) @@ -98,6 +99,20 @@ def test_expire_last_calls_task(self, mock_get_expired, mock_expire): self.assertEqual(mock_expire.call_args_list[1], mock.call(docs[1])) self.assertEqual(mock_expire.call_args_list[2], mock.call(docs[2])) + def test_investigate_fragment_task(self): + investigation_results = object() # singleton + with mock.patch("ietf.doc.tasks.investigate_fragment", return_value=investigation_results) as mock_inv: + retval = investigate_fragment_task("some fragment") + self.assertTrue(mock_inv.called) + self.assertEqual(mock_inv.call_args, mock.call("some fragment")) + self.assertEqual( + retval, + { + "name_fragment": "some fragment", + "results": investigation_results + } + ) + class Idnits2SupportTests(TestCase): settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['DERIVED_DIR'] From 8eae5fb61df0f27b35ea044cdc7e4a61b5539684 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 16:22:52 -0400 Subject: [PATCH 5/9] fix: extra tag/fix whitespace --- ietf/templates/doc/investigate.html | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ietf/templates/doc/investigate.html b/ietf/templates/doc/investigate.html index f53f227dcbd..436a8ce91ae 100644 --- a/ietf/templates/doc/investigate.html +++ b/ietf/templates/doc/investigate.html @@ -39,16 +39,16 @@

These can be authenticated

{% for path in results.can_verify %} {% with url=path|url_for_path %} - {{path.name}} + {{ path.name }} {% if path|mtime_is_epoch %} Timestamp has been lost (is Unix Epoch) {% else %} - {{path|mtime|date:"DATETIME_FORMAT"}} + {{ path|mtime|date:"DATETIME_FORMAT" }} {% endif %} - {{url}} - {{path}} + {{ url }} + {{ path }} {% endwith %} {% endfor %} @@ -73,16 +73,16 @@

These are in the archive, but cannot be authenticated

{% for path in results.unverifiable_collections %} {% with url=path|url_for_path %} - {{path.name}} + {{ path.name }} {% if path|mtime_is_epoch %} Timestamp has been lost (is Unix Epoch) {% else %} - {{path|mtime|date:"DATETIME_FORMAT"}} + {{ path|mtime|date:"DATETIME_FORMAT" }} {% endif %} - {{url}} - {{path}} + {{ url }} + {{ path }} {% endwith %} {% endfor %} @@ -103,15 +103,15 @@

These are unexpected and we do not know what their origin is. These cannot b {% for path in results.unexpected %} {% with url=path|url_for_path %} - {{path.name}} + {{ path.name }} {% if path|mtime_is_epoch %} Timestamp has been lost (is Unix Epoch) {% else %} - {{path|mtime|date:"DATETIME_FORMAT"}} + {{ path|mtime|date:"DATETIME_FORMAT" }} {% endif %} - {{url}} + {{ url }} {% endwith %} {% endfor %} @@ -120,7 +120,6 @@

These are unexpected and we do not know what their origin is. These cannot b {% endif %} {% endif %} - {% endblock %} {% block js %} From 718d254ff2273014b4b417f25d8b07ef46d67af9 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 16:26:53 -0400 Subject: [PATCH 6/9] style: restore whitespace (I hope) --- ietf/doc/views_doc.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index f504e050b34..2723994789c 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -58,22 +58,22 @@ from ietf.doc.models import ( Document, DocHistory, DocEvent, BallotDocEvent, BallotType, ConsensusDocEvent, NewRevisionDocEvent, TelechatDocEvent, WriteupDocEvent, IanaExpertDocEvent, - IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder, DocumentAuthor, - RelatedDocument, RelatedDocHistory) + IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS, DocumentActionHolder, DocumentAuthor, + RelatedDocument, RelatedDocHistory) from ietf.doc.tasks import investigate_fragment_task from ietf.doc.utils import (augment_events_with_revision, - can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, - needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, - get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, - add_events_message_info, get_unicode_document_content, + can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, + needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, + get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, + add_events_message_info, get_unicode_document_content, augment_docs_and_person_with_person_info, irsg_needed_ballot_positions, add_action_holder_change_event, - build_file_urls, update_documentauthors, fuzzy_find_documents, - bibxml_for_draft, get_doc_email_aliases) + build_file_urls, update_documentauthors, fuzzy_find_documents, + bibxml_for_draft, get_doc_email_aliases) from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible from ietf.group.models import Role, Group from ietf.group.utils import can_manage_all_groups_of_type, can_manage_materials, group_features_role_filter from ietf.ietfauth.utils import ( has_role, is_authorized_in_doc_stream, user_is_person, - role_required, is_individual_draft_author, can_request_rfc_publication) + role_required, is_individual_draft_author, can_request_rfc_publication) from ietf.name.models import StreamName, BallotPositionName from ietf.utils.history import find_history_active_at from ietf.doc.views_ballot import parse_ballot_edit_return_point From 732f4532c88cffbefd3817329abcbb4cef0ef668 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 16:31:18 -0400 Subject: [PATCH 7/9] style: black/standard styling --- ietf/doc/tasks.py | 2 +- ietf/doc/tests_tasks.py | 10 ++++------ ietf/doc/views_doc.py | 14 ++++++++------ ietf/static/js/investigate.js | 3 +-- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/ietf/doc/tasks.py b/ietf/doc/tasks.py index 5b1b4f0b6bc..6eb901e6c70 100644 --- a/ietf/doc/tasks.py +++ b/ietf/doc/tasks.py @@ -126,5 +126,5 @@ def generate_draft_bibxml_files_task(days=7, process_all=False): def investigate_fragment_task(name_fragment: str): return { "name_fragment": name_fragment, - "results": investigate_fragment(name_fragment) + "results": investigate_fragment(name_fragment), } diff --git a/ietf/doc/tests_tasks.py b/ietf/doc/tests_tasks.py index ae7d9f36fb3..8a6ffa8be1e 100644 --- a/ietf/doc/tests_tasks.py +++ b/ietf/doc/tests_tasks.py @@ -101,16 +101,14 @@ def test_expire_last_calls_task(self, mock_get_expired, mock_expire): def test_investigate_fragment_task(self): investigation_results = object() # singleton - with mock.patch("ietf.doc.tasks.investigate_fragment", return_value=investigation_results) as mock_inv: + with mock.patch( + "ietf.doc.tasks.investigate_fragment", return_value=investigation_results + ) as mock_inv: retval = investigate_fragment_task("some fragment") self.assertTrue(mock_inv.called) self.assertEqual(mock_inv.call_args, mock.call("some fragment")) self.assertEqual( - retval, - { - "name_fragment": "some fragment", - "results": investigation_results - } + retval, {"name_fragment": "some fragment", "results": investigation_results} ) diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 2723994789c..591a72d907e 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -2286,10 +2286,10 @@ def investigate(request): POST with the task_id field empty starts an async task and returns a JSON response with the ID needed to monitor the task for results. - + GET with a querystring parameter "id" will poll the status of the async task and return "ready" or "notready". - + POST with the task_id field set to the id of a "ready" task will return its results or an error if the task failed or the id is invalid (expired, never exited, etc). """ @@ -2306,7 +2306,9 @@ def investigate(request): retval = task_result.get() results = retval["results"] form.data = form.data.copy() - form.data["name_fragment"] = retval["name_fragment"] # ensure consistency + form.data["name_fragment"] = retval[ + "name_fragment" + ] # ensure consistency del form.data["task_id"] # do not request the task result again else: form.add_error( @@ -2323,9 +2325,9 @@ def investigate(request): if task_id is not None: # Check status if we got the "id" parameter task_result = AsyncResult(task_id) - return JsonResponse({ - "status": "ready" if task_result.ready() else "notready" - }) + return JsonResponse( + {"status": "ready" if task_result.ready() else "notready"} + ) else: # Serve up an empty form form = InvestigateForm() diff --git a/ietf/static/js/investigate.js b/ietf/static/js/investigate.js index 3737ee35f50..b22e099b1ee 100644 --- a/ietf/static/js/investigate.js +++ b/ietf/static/js/investigate.js @@ -12,8 +12,7 @@ document.addEventListener('DOMContentLoaded', () => { const runInvestigation = async () => { // Submit the request const response = await fetch('', { - method: investigateForm.method, - body: new FormData(investigateForm) + method: investigateForm.method, body: new FormData(investigateForm) }) if (!response.ok) { loadResultsFromTask('bogus-task-id') // bad task id will generate an error from Django From 9b03cc2a7e9f79b2e82faff0c0974a344f04d0c0 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 18:13:40 -0400 Subject: [PATCH 8/9] test: fix test of investigate view --- ietf/doc/tests.py | 118 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 105 insertions(+), 13 deletions(-) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 0630fcd8d49..ce5239353b2 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -3280,7 +3280,8 @@ def test_investigate_fragment(self): "draft-this-should-not-be-possible-00.txt", ) - def test_investigate(self): + def test_investigate_get(self): + """GET with no querystring should retrieve the investigate UI""" url = urlreverse("ietf.doc.views_doc.investigate") login_testing_unauthorized(self, "secretary", url) r = self.client.get(url) @@ -3288,36 +3289,127 @@ def test_investigate(self): q = PyQuery(r.content) self.assertEqual(len(q("form#investigate")), 1) self.assertEqual(len(q("div#results")), 0) - r = self.client.post(url, dict(name_fragment="this-is-not-found")) + + @mock.patch("ietf.doc.views_doc.AsyncResult") + def test_investgate_get_task_id(self, mock_asyncresult): + """GET with querystring should lookup task status""" + url = urlreverse("ietf.doc.views_doc.investigate") + login_testing_unauthorized(self, "secretary", url) + mock_asyncresult.return_value.ready.return_value = True + r = self.client.get(url + "?id=a-task-id") + self.assertEqual(r.status_code, 200) + self.assertEqual(r.json(), {"status": "ready"}) + + mock_asyncresult.return_value.ready.return_value = False + r = self.client.get(url + "?id=a-task-id") + self.assertEqual(r.status_code, 200) + self.assertEqual(r.json(), {"status": "notready"}) + + @mock.patch("ietf.doc.views_doc.investigate_fragment_task") + def test_investigate_post(self, mock_investigate_fragment_task): + """POST with a name_fragment should start a celery task""" + url = urlreverse("ietf.doc.views_doc.investigate") + login_testing_unauthorized(self, "secretary", url) + + # test some invalid cases + r = self.client.post(url, {"name_fragment": "short"}) # limit is >= 8 characters self.assertEqual(r.status_code, 200) q = PyQuery(r.content) + self.assertEqual(len(q("#id_name_fragment.is-invalid")), 1) + self.assertFalse(mock_investigate_fragment_task.delay.called) + for char in ["*", "%", "/", "\\"]: + r = self.client.post(url, {"name_fragment": f"bad{char}character"}) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertEqual(len(q("#id_name_fragment.is-invalid")), 1) + self.assertFalse(mock_investigate_fragment_task.delay.called) + + # now a valid one + mock_investigate_fragment_task.delay.return_value.id = "a-task-id" + r = self.client.post(url, {"name_fragment": "this-is-a-valid-fragment"}) + self.assertEqual(r.status_code, 200) + self.assertTrue(mock_investigate_fragment_task.delay.called) + self.assertEqual(mock_investigate_fragment_task.delay.call_args, mock.call("this-is-a-valid-fragment")) + self.assertEqual(r.json(), {"id": "a-task-id"}) + + @mock.patch("ietf.doc.views_doc.AsyncResult") + def test_investigate_post_task_id(self, mock_asyncresult): + url = urlreverse("ietf.doc.views_doc.investigate") + login_testing_unauthorized(self, "secretary", url) + + # First, test a non-successful result - this could be a failure or non-existent task id + mock_result = mock_asyncresult.return_value + mock_result.successful.return_value = False + r = self.client.post(url, {"name_fragment": "some-fragment", "task_id": "a-task-id"}) + self.assertContains(r, "The investigation task failed.", status_code=200) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) + self.assertFalse(mock_result.get.called) + mock_asyncresult.reset_mock() + q = PyQuery(r.content) + + # now the various successful result mixes + mock_result = mock_asyncresult.return_value + mock_result.successful.return_value = True + mock_result.get.return_value = { + "name_fragment": "some-fragment", + "results": { + "can_verify": set(), + "unverifiable_collections": set(), + "unexpected": set(), + } + } + r = self.client.post(url, {"name_fragment": "some-fragment", "task_id": "a-task-id"}) + self.assertEqual(r.status_code, 200) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) + mock_asyncresult.reset_mock() + q = PyQuery(r.content) self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 0) self.assertEqual(len(q("table#unverifiable")), 0) self.assertEqual(len(q("table#unexpected")), 0) - r = self.client.post(url, dict(name_fragment="mixed-provenance")) + + a_file_that_exists = Path(settings.INTERNET_DRAFT_PATH) / "draft-this-is-active-00.txt" + + mock_result.get.return_value = { + "name_fragment": "some-fragment", + "results": { + "can_verify": {a_file_that_exists}, + "unverifiable_collections": {a_file_that_exists}, + "unexpected": set(), + } + } + r = self.client.post(url, {"name_fragment": "some-fragment", "task_id": "a-task-id"}) self.assertEqual(r.status_code, 200) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) + mock_asyncresult.reset_mock() q = PyQuery(r.content) self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 1) self.assertEqual(len(q("table#unverifiable")), 1) self.assertEqual(len(q("table#unexpected")), 0) - r = self.client.post(url, dict(name_fragment="not-be-possible")) + + mock_result.get.return_value = { + "name_fragment": "some-fragment", + "results": { + "can_verify": set(), + "unverifiable_collections": set(), + "unexpected": {a_file_that_exists}, + } + } + r = self.client.post(url, {"name_fragment": "some-fragment", "task_id": "a-task-id"}) self.assertEqual(r.status_code, 200) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) + mock_asyncresult.reset_mock() q = PyQuery(r.content) self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 0) self.assertEqual(len(q("table#unverifiable")), 0) self.assertEqual(len(q("table#unexpected")), 1) - r = self.client.post(url, dict(name_fragment="short")) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - self.assertEqual(len(q("#id_name_fragment.is-invalid")), 1) - for char in ["*", "%", "/", "\\"]: - r = self.client.post(url, dict(name_fragment=f"bad{char}character")) - self.assertEqual(r.status_code, 200) - q = PyQuery(r.content) - self.assertEqual(len(q("#id_name_fragment.is-invalid")), 1) + class LogIOErrorTests(TestCase): From ba7f2d9d09b5aba7eafe0e8f3768fc2469228a54 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 16 Jan 2025 20:26:24 -0400 Subject: [PATCH 9/9] test: improve/delint tests --- ietf/doc/tests.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index ce5239353b2..f5af7bb48b2 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -3299,15 +3299,20 @@ def test_investgate_get_task_id(self, mock_asyncresult): r = self.client.get(url + "?id=a-task-id") self.assertEqual(r.status_code, 200) self.assertEqual(r.json(), {"status": "ready"}) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) + mock_asyncresult.reset_mock() mock_asyncresult.return_value.ready.return_value = False r = self.client.get(url + "?id=a-task-id") self.assertEqual(r.status_code, 200) self.assertEqual(r.json(), {"status": "notready"}) + self.assertTrue(mock_asyncresult.called) + self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) @mock.patch("ietf.doc.views_doc.investigate_fragment_task") def test_investigate_post(self, mock_investigate_fragment_task): - """POST with a name_fragment should start a celery task""" + """POST with a name_fragment and no task_id should start a celery task""" url = urlreverse("ietf.doc.views_doc.investigate") login_testing_unauthorized(self, "secretary", url) @@ -3334,6 +3339,7 @@ def test_investigate_post(self, mock_investigate_fragment_task): @mock.patch("ietf.doc.views_doc.AsyncResult") def test_investigate_post_task_id(self, mock_asyncresult): + """POST with name_fragment and task_id should retrieve results""" url = urlreverse("ietf.doc.views_doc.investigate") login_testing_unauthorized(self, "secretary", url) @@ -3347,12 +3353,14 @@ def test_investigate_post_task_id(self, mock_asyncresult): self.assertFalse(mock_result.get.called) mock_asyncresult.reset_mock() q = PyQuery(r.content) + self.assertEqual(q("#id_name_fragment").val(), "some-fragment") + self.assertEqual(q("#id_task_id").val(), "a-task-id") # now the various successful result mixes mock_result = mock_asyncresult.return_value mock_result.successful.return_value = True mock_result.get.return_value = { - "name_fragment": "some-fragment", + "name_fragment": "different-fragment", "results": { "can_verify": set(), "unverifiable_collections": set(), @@ -3365,15 +3373,19 @@ def test_investigate_post_task_id(self, mock_asyncresult): self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) mock_asyncresult.reset_mock() q = PyQuery(r.content) + self.assertEqual(q("#id_name_fragment").val(), "different-fragment", "name_fragment should be reset") + self.assertEqual(q("#id_task_id").val(), "", "task_id should be cleared") self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 0) self.assertEqual(len(q("table#unverifiable")), 0) self.assertEqual(len(q("table#unexpected")), 0) + # This file was created in setUp. It allows the view to render properly + # but its location / content don't matter for this test otherwise. a_file_that_exists = Path(settings.INTERNET_DRAFT_PATH) / "draft-this-is-active-00.txt" mock_result.get.return_value = { - "name_fragment": "some-fragment", + "name_fragment": "different-fragment", "results": { "can_verify": {a_file_that_exists}, "unverifiable_collections": {a_file_that_exists}, @@ -3386,13 +3398,15 @@ def test_investigate_post_task_id(self, mock_asyncresult): self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) mock_asyncresult.reset_mock() q = PyQuery(r.content) + self.assertEqual(q("#id_name_fragment").val(), "different-fragment", "name_fragment should be reset") + self.assertEqual(q("#id_task_id").val(), "", "task_id should be cleared") self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 1) self.assertEqual(len(q("table#unverifiable")), 1) self.assertEqual(len(q("table#unexpected")), 0) mock_result.get.return_value = { - "name_fragment": "some-fragment", + "name_fragment": "different-fragment", "results": { "can_verify": set(), "unverifiable_collections": set(), @@ -3405,6 +3419,8 @@ def test_investigate_post_task_id(self, mock_asyncresult): self.assertEqual(mock_asyncresult.call_args, mock.call("a-task-id")) mock_asyncresult.reset_mock() q = PyQuery(r.content) + self.assertEqual(q("#id_name_fragment").val(), "different-fragment", "name_fragment should be reset") + self.assertEqual(q("#id_task_id").val(), "", "task_id should be cleared") self.assertEqual(len(q("div#results")), 1) self.assertEqual(len(q("table#authenticated")), 0) self.assertEqual(len(q("table#unverifiable")), 0)