From 51270046ce65d4f101f2a640a6352342cdceabdf Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Wed, 26 Aug 2020 14:27:15 -0500 Subject: [PATCH 1/9] add appconf --- core/scaife_viewer/core/conf.py | 10 ++++++++++ core/setup.py | 1 + 2 files changed, 11 insertions(+) create mode 100644 core/scaife_viewer/core/conf.py diff --git a/core/scaife_viewer/core/conf.py b/core/scaife_viewer/core/conf.py new file mode 100644 index 0000000..752dc64 --- /dev/null +++ b/core/scaife_viewer/core/conf.py @@ -0,0 +1,10 @@ +from django.conf import settings # noqa + +from appconf import AppConf + + +class CoreAppConf(AppConf): + ALLOW_TRAILING_COLON = False + + class Meta: + prefix = "scaife_viewer_core" diff --git a/core/setup.py b/core/setup.py index d3ded62..0a5e373 100644 --- a/core/setup.py +++ b/core/setup.py @@ -28,6 +28,7 @@ "anytree==2.4.3", "certifi==2018.11.29", "dask[bag]==1.1.0", + "django_appconf>=1.0.4", "Django>=2.2,<3.0", "elasticsearch==6.3.1", "google-auth==1.6.2", From e6720d6d1b62494b43e013e8a873c566c6919d52 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Wed, 26 Aug 2020 14:58:48 -0500 Subject: [PATCH 2/9] redirect library URNs with a trailing colon --- core/scaife_viewer/core/utils.py | 8 ++++++++ core/scaife_viewer/core/views.py | 12 +++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/core/scaife_viewer/core/utils.py b/core/scaife_viewer/core/utils.py index 862f4b8..86a4d02 100644 --- a/core/scaife_viewer/core/utils.py +++ b/core/scaife_viewer/core/utils.py @@ -3,6 +3,7 @@ from django.urls import reverse from . import cts +from .conf import settings def link_collection(urn) -> dict: @@ -117,3 +118,10 @@ def get_pagination_info(total_count, page_num): "has_next": has_next, "num_pages": num_pages, } + + +def normalize_urn(urn): + if not settings.SCAIFE_VIEWER_CORE_ALLOW_TRAILING_COLON and urn.endswith(":"): + # @@@ logging + urn = urn[:-1] + return urn diff --git a/core/scaife_viewer/core/views.py b/core/scaife_viewer/core/views.py index 457676a..6cc30d4 100644 --- a/core/scaife_viewer/core/views.py +++ b/core/scaife_viewer/core/views.py @@ -21,7 +21,13 @@ from .http import ConditionMixin from .precomputed import library_view_json from .search import SearchQuery -from .utils import apify, encode_link_header, get_pagination_info, link_passage +from .utils import ( + apify, + encode_link_header, + get_pagination_info, + link_passage, + normalize_urn, +) class BaseLibraryView(View): @@ -74,6 +80,10 @@ def get_collection(self): raise Http404() def as_html(self): + normalized_urn = normalize_urn(self.kwargs["urn"]) + if normalized_urn != self.kwargs["urn"]: + return redirect("library_collection", urn=normalized_urn) + collection = self.get_collection() collection_name = collection.__class__.__name__.lower() ctx = {collection_name: collection} From c96c645925790695b58bb9131b4480f8d33ce879 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Wed, 26 Aug 2020 15:04:21 -0500 Subject: [PATCH 3/9] redirect to the first passage --- core/scaife_viewer/core/views.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/core/scaife_viewer/core/views.py b/core/scaife_viewer/core/views.py index 6cc30d4..8a43fbc 100644 --- a/core/scaife_viewer/core/views.py +++ b/core/scaife_viewer/core/views.py @@ -211,12 +211,19 @@ class Reader(TemplateView): template_name = "reader/reader.html" + def get(self, request, *args, **kwargs): + self.urn = cts.URN(self.kwargs["urn"]) + if not self.urn.reference: + return redirect("library_text_redirect", urn=self.kwargs["urn"]) + return super().get(request, *args, **kwargs) + def get_text(self): - urn = cts.URN(self.kwargs["urn"]) try: - text = cts.collection(urn.upTo(cts.URN.NO_PASSAGE)) + text = cts.collection(self.urn.upTo(cts.URN.NO_PASSAGE)) except cts.CollectionDoesNotExist: raise Http404() + # @@@ KeyError is a possibility if the URN is for a text group + # or work return text def get_context_data(self, **kwargs): @@ -230,6 +237,9 @@ def library_text_redirect(request, urn): Given a text URN redirect to the first chunk. Required to prevent TOCing on the top-level library page. """ + # @@@ how to indicate that healing has happened + urn = normalize_urn(urn) + try: text = cts.collection(urn) except cts.CollectionDoesNotExist: From 0f3985e866fb217c2d761835ae08171e2aeca8a3 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 10:18:57 -0500 Subject: [PATCH 4/9] redirect from the reader to the library --- core/scaife_viewer/core/views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/scaife_viewer/core/views.py b/core/scaife_viewer/core/views.py index 8a43fbc..535cc6f 100644 --- a/core/scaife_viewer/core/views.py +++ b/core/scaife_viewer/core/views.py @@ -222,8 +222,6 @@ def get_text(self): text = cts.collection(self.urn.upTo(cts.URN.NO_PASSAGE)) except cts.CollectionDoesNotExist: raise Http404() - # @@@ KeyError is a possibility if the URN is for a text group - # or work return text def get_context_data(self, **kwargs): @@ -245,7 +243,7 @@ def library_text_redirect(request, urn): except cts.CollectionDoesNotExist: raise Http404() if not isinstance(text, cts.Text): - raise Http404() + return redirect("library_collection", urn=urn) passage = text.first_passage() if not passage: raise Http404() From 1c3a4545cbd3706a5891f0579a592882b87a00a1 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 10:23:58 -0500 Subject: [PATCH 5/9] add proper logging when normalizing URNs --- core/scaife_viewer/core/utils.py | 10 ++++++++-- core/scaife_viewer/core/views.py | 1 - 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core/scaife_viewer/core/utils.py b/core/scaife_viewer/core/utils.py index 86a4d02..679109b 100644 --- a/core/scaife_viewer/core/utils.py +++ b/core/scaife_viewer/core/utils.py @@ -1,3 +1,4 @@ +import logging import math from django.urls import reverse @@ -6,6 +7,9 @@ from .conf import settings +logger = logging.getLogger(__name__) + + def link_collection(urn) -> dict: return { "url": reverse("library_collection", kwargs={"urn": urn}), @@ -122,6 +126,8 @@ def get_pagination_info(total_count, page_num): def normalize_urn(urn): if not settings.SCAIFE_VIEWER_CORE_ALLOW_TRAILING_COLON and urn.endswith(":"): - # @@@ logging - urn = urn[:-1] + new_urn = urn[:-1] + msg = f'Normalized "{urn}" to "{new_urn}"' + logger.info(msg) + return new_urn return urn diff --git a/core/scaife_viewer/core/views.py b/core/scaife_viewer/core/views.py index 535cc6f..242a879 100644 --- a/core/scaife_viewer/core/views.py +++ b/core/scaife_viewer/core/views.py @@ -235,7 +235,6 @@ def library_text_redirect(request, urn): Given a text URN redirect to the first chunk. Required to prevent TOCing on the top-level library page. """ - # @@@ how to indicate that healing has happened urn = normalize_urn(urn) try: From 7e72b5d00385fcbc641ea9b028c5cfe7fb677872 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 10:24:59 -0500 Subject: [PATCH 6/9] fix linting errors --- core/scaife_viewer/core/cts/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/scaife_viewer/core/cts/__init__.py b/core/scaife_viewer/core/cts/__init__.py index f893a4d..9caa6a8 100644 --- a/core/scaife_viewer/core/cts/__init__.py +++ b/core/scaife_viewer/core/cts/__init__.py @@ -11,13 +11,13 @@ ) from .exceptions import ( # noqa CollectionDoesNotExist, - PassageDoesNotExist, InvalidPassageReference, InvalidURN, + PassageDoesNotExist, ) +from .heal import heal from .passage import Passage from .reference import URN -from .heal import heal def text_inventory() -> TextInventory: From c4315283b49e8a386f004ecb84743c86b2b20d0d Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 10:53:30 -0500 Subject: [PATCH 7/9] add some tests --- .../tests/fixtures/templates/site_base.html | 1 + core/scaife_viewer/core/tests/fixtures/ti.xml | 49 +++++++++++++++++++ core/scaife_viewer/core/tests/settings.py | 15 ++++-- core/scaife_viewer/core/tests/tests.py | 44 +++++++++++++++-- core/scaife_viewer/core/urls.py | 19 ++++++- 5 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 core/scaife_viewer/core/tests/fixtures/templates/site_base.html create mode 100644 core/scaife_viewer/core/tests/fixtures/ti.xml diff --git a/core/scaife_viewer/core/tests/fixtures/templates/site_base.html b/core/scaife_viewer/core/tests/fixtures/templates/site_base.html new file mode 100644 index 0000000..0edd03a --- /dev/null +++ b/core/scaife_viewer/core/tests/fixtures/templates/site_base.html @@ -0,0 +1 @@ +{% comment %} left intentinoally blank {% endcomment %} diff --git a/core/scaife_viewer/core/tests/fixtures/ti.xml b/core/scaife_viewer/core/tests/fixtures/ti.xml new file mode 100644 index 0000000..b1f56e4 --- /dev/null +++ b/core/scaife_viewer/core/tests/fixtures/ti.xml @@ -0,0 +1,49 @@ + + + GetInventory + urn=None + + + + + Aesop + + Fabulae + + + + Fabulae Aesopicae Collectae, Halm, Teubner, 1872 + + + + + + + + + + Eustratius + + In Aristotelis Ethica Nicomachea I Commentaria + + + Eustratius, In Aristotelis Ethica Nicomachea I Commentaria, Commentaria in Aristotelem Graeca 20, + Reimer, Heylbut, 1892 + + + + + + + + + + + diff --git a/core/scaife_viewer/core/tests/settings.py b/core/scaife_viewer/core/tests/settings.py index 9dac4c1..97c4fee 100644 --- a/core/scaife_viewer/core/tests/settings.py +++ b/core/scaife_viewer/core/tests/settings.py @@ -16,9 +16,7 @@ TEMPLATES = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", - "DIRS": [ - # insert your TEMPLATE_DIRS here - ], + "DIRS": ["scaife_viewer/core/tests/fixtures/templates"], "APP_DIRS": True, "OPTIONS": { "context_processors": [ @@ -39,3 +37,14 @@ SITE_ID = 1 ROOT_URLCONF = "scaife_viewer.core.tests.urls" SECRET_KEY = "notasecret" + +CTS_API_ENDPOINT = os.environ.get( + "CTS_API_ENDPOINT", "https://scaife-cts-dev.perseus.org/api/cts" +) +CTS_RESOLVER = { + "type": "api", + "kwargs": {"endpoint": CTS_API_ENDPOINT}, +} +CTS_LOCAL_TEXT_INVENTORY = "scaife_viewer/core/tests/fixtures/ti.xml" + +DEPLOYMENT_TIMESTAMP_VAR_NAME = "foo" diff --git a/core/scaife_viewer/core/tests/tests.py b/core/scaife_viewer/core/tests/tests.py index 4bca570..afdc660 100644 --- a/core/scaife_viewer/core/tests/tests.py +++ b/core/scaife_viewer/core/tests/tests.py @@ -1,6 +1,42 @@ -from django.test import TestCase +from django.test import TestCase, override_settings +from django.urls import reverse +from ..utils import normalize_urn -class Tests(TestCase): - def setUp(self): - pass + +class URNTests(TestCase): + def test_urn_normalized(self): + provided_urn = "urn:cts:greekLit:tlg0096.tlg002.First1K-grc1:" + acceptable_urn = "urn:cts:greekLit:tlg0096.tlg002.First1K-grc1" + result = normalize_urn(provided_urn) + assert result == acceptable_urn + + @override_settings(SCAIFE_VIEWER_CORE_ALLOW_TRAILING_COLON=True) + def test_urn_trailing_colon_not_normalized(self): + provided_urn = "urn:cts:greekLit:tlg0096.tlg002.First1K-grc1:" + result = normalize_urn(provided_urn) + assert result == provided_urn + + def test_urn_unmodified(self): + provided_urn = "urn:cts:greekLit:tlg0096.tlg002.First1K-grc1" + result = normalize_urn(provided_urn) + assert result == provided_urn + + +class ViewTests(TestCase): + def test_reader_version_urn_redirects_to_first_passage(self): + urn = "urn:cts:greekLit:tlg0096.tlg002.First1K-grc1" + reader_url = reverse("reader", kwargs={"urn": urn}) + response = self.client.get(reader_url, follow=True) + assert len(response.redirect_chain) == 2 + assert ( + response.wsgi_request.path + == "/reader/urn:cts:greekLit:tlg0096.tlg002.First1K-grc1:1-4b/" + ) + + def test_reader_work_urn_redirects_to_library(self): + urn = "urn:cts:greekLit:tlg0096.tlg002" + reader_url = reverse("reader", kwargs={"urn": urn}) + response = self.client.get(reader_url, follow=True) + assert len(response.redirect_chain) == 2 + assert response.wsgi_request.path == "/library/urn:cts:greekLit:tlg0096.tlg002/" diff --git a/core/scaife_viewer/core/urls.py b/core/scaife_viewer/core/urls.py index 637600f..527df58 100644 --- a/core/scaife_viewer/core/urls.py +++ b/core/scaife_viewer/core/urls.py @@ -1 +1,18 @@ -urlpatterns = [] +from django.urls import path + +from .views import LibraryCollectionView, Reader, library_text_redirect + + +urlpatterns = [ + path( + "library//", + LibraryCollectionView.as_view(format="html"), + name="library_collection", + ), + path( + "library//redirect/", + library_text_redirect, + name="library_text_redirect", + ), + path("reader//", Reader.as_view(), name="reader"), +] From 24b8f986f2f69e133bdcf28ec2e82a8309872f0f Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 10:53:38 -0500 Subject: [PATCH 8/9] prevent ElasticSearch instantiation --- core/scaife_viewer/core/search.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/core/scaife_viewer/core/search.py b/core/scaife_viewer/core/search.py index b71c94e..cc47a4c 100644 --- a/core/scaife_viewer/core/search.py +++ b/core/scaife_viewer/core/search.py @@ -2,6 +2,7 @@ from django.conf import settings from django.urls import reverse +from django.utils.functional import SimpleLazyObject import regex from elasticsearch import Elasticsearch @@ -10,11 +11,20 @@ from . import cts -es = Elasticsearch( - hosts=settings.ELASTICSEARCH_HOSTS, - sniff_on_start=settings.ELASTICSEARCH_SNIFF_ON_START, - sniff_on_connection_fail=settings.ELASTICSEARCH_SNIFF_ON_CONNECTION_FAIL, -) +def default_es_client_config(): + return dict( + hosts=settings.ELASTICSEARCH_HOSTS, + sniff_on_start=settings.ELASTICSEARCH_SNIFF_ON_START, + sniff_on_connection_fail=settings.ELASTICSEARCH_SNIFF_ON_CONNECTION_FAIL, + ) + + +def get_es_client(): + return Elasticsearch(**default_es_client_config()) + + +es = SimpleLazyObject(get_es_client) + """ From https://www.elastic.co/guide/en/elasticsearch/reference/6.0/search-request-highlighting.html#boundary-scanners: From 51da386ff7ac838103fdac2195076df98a9eb515 Mon Sep 17 00:00:00 2001 From: Jacob Wegner Date: Mon, 31 Aug 2020 11:00:40 -0500 Subject: [PATCH 9/9] update docs for ALLOW_TRAILING_COLON --- core/README.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/core/README.md b/core/README.md index 6e8677e..8e2fdeb 100644 --- a/core/README.md +++ b/core/README.md @@ -1,3 +1,12 @@ # Scaife Viewer :: Core functionality -This package was extracted from [https://github.com/scaife-viewer/scaife-viewer](https://github.com/scaife-viewer/scaife-viewer) +This package was extracted from +[https://github.com/scaife-viewer/scaife-viewer](https://github.com/scaife-viewer/scaife-viewer) + +## Settings + +### ALLOW_TRAILING_COLON +Default: `False` + +When `False`, to maintain compatability with the MyCapitain resolver, +the trailing colon will be stripped from URNs.