From 783307c10e9ff1c64c9c09c883583b7a949554db Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Mon, 28 Feb 2022 14:42:26 -0500 Subject: [PATCH 1/9] Add Last-Modified header to document detail --- geniza/corpus/tests/test_corpus_views.py | 6 ++++++ geniza/corpus/views.py | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index bf0b7ecf6..1912770b9 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -96,6 +96,12 @@ def test_get_absolute_url(self, document): document.get_absolute_url() ) + def test_last_modified(self, client, document): + """Ensure that the last modified header is set in the HEAD response""" + SolrClient().update.index([document.index_data()], commit=True) + response = client.head(document.get_absolute_url()) + assert response["Last-Modified"] + @pytest.mark.django_db def test_old_pgp_tabulate_data(): diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 4a129158f..1e68ed569 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -15,6 +15,7 @@ from django.utils.translation import ngettext from django.views.generic import DetailView, FormView, ListView from django.views.generic.edit import FormMixin +from parasolr.django.views import SolrLastModifiedMixin from piffle.presentation import IIIFPresentation from tabular_export.admin import export_to_csv_response @@ -214,7 +215,7 @@ def get(self, request, *args, **kwargs): raise -class DocumentDetailView(DocumentPastIdMixin, DetailView): +class DocumentDetailView(DocumentPastIdMixin, DetailView, SolrLastModifiedMixin): """public display of a single :class:`~geniza.corpus.models.Document`""" model = Document From 775fb3aac97bbf561b4f79f0371e4bbf5ddfbff0 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Mon, 28 Feb 2022 14:49:40 -0500 Subject: [PATCH 2/9] Add Last-Modified header to document search --- geniza/corpus/tests/test_corpus_views.py | 6 ++++++ geniza/corpus/views.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index 1912770b9..e9c38de5e 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -607,6 +607,12 @@ def test_dispatch(self, client): # should not redirect assert response.status_code == 200 + def test_last_modified(self, client, document): + """Ensure that the last modified header is set in the HEAD response""" + SolrClient().update.index([document.index_data()], commit=True) + response = client.head(reverse("corpus:document-search")) + assert response["Last-Modified"] + class TestDocumentScholarshipView: def test_page_title(self, document, client, source): diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 1e68ed569..70d8662e7 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -28,7 +28,7 @@ from geniza.footnotes.models import Footnote -class DocumentSearchView(ListView, FormMixin): +class DocumentSearchView(ListView, FormMixin, SolrLastModifiedMixin): model = Document form_class = DocumentSearchForm context_object_name = "documents" From 2bd1b0577ac31e1df2c4750f82dedaa72f99a049 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 11:32:09 -0500 Subject: [PATCH 3/9] Ensure document suppressed also tirggers the last modified header --- geniza/corpus/tests/test_corpus_views.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index e9c38de5e..a0450100f 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -101,6 +101,7 @@ def test_last_modified(self, client, document): SolrClient().update.index([document.index_data()], commit=True) response = client.head(document.get_absolute_url()) assert response["Last-Modified"] + init_last_modified = response["Last-Modified"] @pytest.mark.django_db @@ -612,6 +613,15 @@ def test_last_modified(self, client, document): SolrClient().update.index([document.index_data()], commit=True) response = client.head(reverse("corpus:document-search")) assert response["Last-Modified"] + init_last_modified = response["Last-Modified"] + + # Ensure that a document being suppressed changes the last modified header + document.status = Document.SUPPRESSED + document.save() + SolrClient().update.index([document.index_data()], commit=True) + response = client.head(reverse("corpus:document-search")) + new_last_modified = response["Last-Modified"] + assert new_last_modified != init_last_modified class TestDocumentScholarshipView: From c5c3a0faca2b1151bbf7d17e0be7b92a160e59ae Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 12:53:56 -0500 Subject: [PATCH 4/9] Ensure that only one doc's lastmodified changes when doc is updated --- geniza/corpus/tests/test_corpus_views.py | 16 +++++++++++++++- geniza/corpus/views.py | 12 +++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index a0450100f..8ec0cc707 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -96,13 +96,24 @@ def test_get_absolute_url(self, document): document.get_absolute_url() ) - def test_last_modified(self, client, document): + def test_last_modified(self, client, document, join): """Ensure that the last modified header is set in the HEAD response""" SolrClient().update.index([document.index_data()], commit=True) response = client.head(document.get_absolute_url()) assert response["Last-Modified"] init_last_modified = response["Last-Modified"] + # Sleeping is required to ensure that the last modified header is different. + sleep(1) + + # Ensure that only one last modified header is updated when a document is updated. + SolrClient().update.index([join.index_data()], commit=True) + updated_doc_response = client.head(join.get_absolute_url()) + other_doc_response = client.head(document.get_absolute_url()) + assert ( + updated_doc_response["Last-Modified"] != other_doc_response["Last-Modified"] + ) + @pytest.mark.django_db def test_old_pgp_tabulate_data(): @@ -615,6 +626,9 @@ def test_last_modified(self, client, document): assert response["Last-Modified"] init_last_modified = response["Last-Modified"] + # Sleep for 1 second to ensure the last modified header is different + sleep(1) + # Ensure that a document being suppressed changes the last modified header document.status = Document.SUPPRESSED document.save() diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 70d8662e7..aec31b4fa 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -193,8 +193,8 @@ def get_context_data(self, **kwargs): return context_data -class DocumentPastIdMixin: - """View mixin to handle redirects for documents with old PGPIDs. +class DocumentDetailBase(SolrLastModifiedMixin): + """View mixin to handle lastmodified and redirects for documents with old PGPIDs. Overrides get request in the case of a 404, looking for any records with passed PGPID in old_pgpids, and if found, redirects to that document with current PGPID.""" @@ -214,8 +214,14 @@ def get(self, request, *args, **kwargs): # otherwise, continue raising the 404 raise + def get_solr_lastmodified_filters(self): + """Query solr for the object that needs their "last_modified" attribute updated. + Overwrites `SolrLastModifiedMixin`'s builtin method. + """ + return {"pgpid_i": self.kwargs["pk"]} -class DocumentDetailView(DocumentPastIdMixin, DetailView, SolrLastModifiedMixin): + +class DocumentDetailView(DocumentDetailBase, DetailView): """public display of a single :class:`~geniza.corpus.models.Document`""" model = Document From e72ba656b9f850979556704f97b8b2347c779521 Mon Sep 17 00:00:00 2001 From: Kevin McElwee <26414213+kmcelwee@users.noreply.github.com> Date: Wed, 2 Mar 2022 14:06:24 -0500 Subject: [PATCH 5/9] Update geniza/corpus/views.py Co-authored-by: Rebecca Sutton Koeser --- geniza/corpus/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index aec31b4fa..5e5539a9a 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -215,8 +215,7 @@ def get(self, request, *args, **kwargs): raise def get_solr_lastmodified_filters(self): - """Query solr for the object that needs their "last_modified" attribute updated. - Overwrites `SolrLastModifiedMixin`'s builtin method. + """Filter solr last modified query by pgpid """ return {"pgpid_i": self.kwargs["pk"]} From 9c9d8e3351e61b91a2db570f73cc5ac31538bdf4 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 14:09:26 -0500 Subject: [PATCH 6/9] Add clarifying documentation --- geniza/corpus/tests/test_corpus_views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index 8ec0cc707..534e48ba8 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -620,13 +620,14 @@ def test_dispatch(self, client): assert response.status_code == 200 def test_last_modified(self, client, document): - """Ensure that the last modified header is set in the HEAD response""" + """Ensure that the last modified header is set in the response""" SolrClient().update.index([document.index_data()], commit=True) response = client.head(reverse("corpus:document-search")) assert response["Last-Modified"] init_last_modified = response["Last-Modified"] - # Sleep for 1 second to ensure the last modified header is different + # Sleep to ensure the last modified header is different. Last-Modified only + # has a resolution of 1 second. sleep(1) # Ensure that a document being suppressed changes the last modified header From c8ed3fddadcc23a67ea3504ca63d501bb1498c38 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 14:12:53 -0500 Subject: [PATCH 7/9] Add item_type filter to solr lastmodified --- geniza/corpus/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 5e5539a9a..25ec12108 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -215,9 +215,8 @@ def get(self, request, *args, **kwargs): raise def get_solr_lastmodified_filters(self): - """Filter solr last modified query by pgpid - """ - return {"pgpid_i": self.kwargs["pk"]} + """Filter solr last modified query by pgpid""" + return {"pgpid_i": self.kwargs["pk"], "item_type_s": "document"} class DocumentDetailView(DocumentDetailBase, DetailView): From 70dfb1d0715c0d7f75485a3740cf311da1d5e628 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 14:13:13 -0500 Subject: [PATCH 8/9] Add test case to ensure lastmodified doesn't change --- geniza/corpus/tests/test_corpus_views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/geniza/corpus/tests/test_corpus_views.py b/geniza/corpus/tests/test_corpus_views.py index 534e48ba8..fbd760eef 100644 --- a/geniza/corpus/tests/test_corpus_views.py +++ b/geniza/corpus/tests/test_corpus_views.py @@ -113,6 +113,7 @@ def test_last_modified(self, client, document, join): assert ( updated_doc_response["Last-Modified"] != other_doc_response["Last-Modified"] ) + assert init_last_modified == other_doc_response["Last-Modified"] @pytest.mark.django_db From 7e57d3fc087966cc4b8e7dfdc1d79654d1f0fef5 Mon Sep 17 00:00:00 2001 From: Kevin McElwee Date: Wed, 2 Mar 2022 16:15:38 -0500 Subject: [PATCH 9/9] Add lastmodified filter to documentsearch --- geniza/corpus/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/geniza/corpus/views.py b/geniza/corpus/views.py index 25ec12108..6dae8a3a6 100644 --- a/geniza/corpus/views.py +++ b/geniza/corpus/views.py @@ -39,6 +39,7 @@ class DocumentSearchView(ListView, FormMixin, SolrLastModifiedMixin): page_description = _("Search and browse Geniza documents.") paginate_by = 50 initial = {"sort": "random"} + solr_lastmodified_filters = {"item_type_s": "document"} # map form sort to solr sort field solr_sort = {