Merge pull request #91 from OnroerendErfgoed/develop

Develop

CHANGES.rst

@@ -1,3 +1,9 @@
+1.0.0 (27-02-2023)
+------------------
+
+- Onderscheid op accept header toelaten (#86)
+
+
 1.0.0 (06-07-2022)
 ------------------
 

requirements-dev.txt

@@ -11,7 +11,7 @@ webtest==3.0.0
 coveralls==3.3.1
 
 # Wheel
-wheel==0.37.1
+wheel==0.38.1
 
 # Linting
 flake8==4.0.1

setup.py

@@ -13,7 +13,7 @@
 
 setup(
     name="urihandler",
-    version="1.0.0",
+    version="1.1.0",
     description="A tiny application that handles (cool) uri's.",
     long_description=README + "\n\n" + CHANGES,
     classifiers=[

tests/conftest.py

@@ -10,7 +10,11 @@ def handlerconfig():
             {
                 "match": r"^/foobar/(?P<id>\d+)$",
                 "mount": True,
-                "redirect": "http://localhost:5555/foobar/{id}",
+                "redirect": {
+                    "default": "http://localhost:5555/foobar/{id}",
+                    "text/html": "http://localhost:5555/foobar/{id}",
+                    "application/json": "http://localhost:5555/foobar/{id}.json",
+                }
             },
             {
                 "match": r"^/bar/(?P<name>\w+)$",
@@ -30,6 +34,24 @@ def handlerconfig():
                 "mount": True,
                 "redirect": "http://localhost:5555/foo/{foo_id}/bar/{bar_id}",
             },
+            {
+                "match": r"^/pdf_default/(?P<id>\d+)$",
+                "mount": True,
+                "redirect": {
+                    "default": "http://localhost:5555/pdf_default/{id}.pdf",
+                    "text/html": "http://localhost:5555/pdf_default/{id}",
+                    "application/json": "http://localhost:5555/pdf_default/{id}.json",
+                    "application/pdf": "http://localhost:5555/pdf_default/{id}.pdf",
+                }
+            },
+            {
+                "match": r"^/mime_no_default/(?P<id>\d+)$",
+                "mount": True,
+                "redirect": {
+                    "text/html": "http://localhost:5555/pdf_default/{id}",
+                    "application/json": "http://localhost:5555/pdf_default/{id}.json",
+                }
+            },
         ]
     }
     return cfg

tests/fail.yaml

@@ -0,0 +1,6 @@
+uris:
+  - match: '^/no_default_present/(?P<id>\d+)$'
+    mount: True
+    redirect:
+      text/html: 'http://localhost:5555/foobar/{id}'
+      application/json: 'http://localhost:5555/foobar/{id}.json'

tests/test.yaml

@@ -1,9 +1,12 @@
 uris:
-    - match: '^/foobar/(?P<id>\d+)$'
-      mount: True
-      redirect: 'http://localhost:5555/foobar/{id}'
-    - match: '^/bar/(?P<name>\w+)$'
-      redirect: 'http://localhost:5555/bar/{name}'
-    - match: '^urn:x-barbar:(?P<namespace>\w+):(?P<id>\d+)$'
-      mount: False
-      redirect: 'http://localhost:2222/{namespace}/{id}'
+  - match: '^/foobar/(?P<id>\d+)$'
+    mount: True
+    redirect:
+      default: 'http://localhost:5555/foobar/{id}'
+      text/html: 'http://localhost:5555/foobar/{id}'
+      application/json: 'http://localhost:5555/foobar/{id}.json'
+  - match: '^/bar/(?P<name>\w+)$'
+    redirect: 'http://localhost:5555/bar/{name}'
+  - match: '^urn:x-barbar:(?P<namespace>\w+):(?P<id>\d+)$'
+    mount: False
+    redirect: 'http://localhost:2222/{namespace}/{id}'

tests/test_functional.py

@@ -22,6 +22,21 @@ def test_redirect(self, app):
         res = app.get("/foobar/18", status=303)
         assert res.status == "303 See Other"
 
+    def test_redirect_accept_header_json(self, app):
+        res = app.get("/foobar/18", headers={"Accept": "application/json"}, status=303)
+        assert res.status == "303 See Other"
+        assert res.location == 'http://localhost:5555/foobar/18.json'
+
+    def test_redirect_accept_header_html(self, app):
+        res = app.get("/foobar/18", headers={"Accept": "text/html"}, status=303)
+        assert res.status == "303 See Other"
+        assert res.location == 'http://localhost:5555/foobar/18'
+
+    def test_redirect_accept_header_wildcard(self, app):
+        res = app.get("/foobar/18", headers={"Accept": "*/*"}, status=303)
+        assert res.status == "303 See Other"
+        assert res.location == 'http://localhost:5555/foobar/18'
+
     def test_redirect_not_allowed(self, app):
         res = app.post("/foobar/18", status=405)
         assert res.status == "405 Method Not Allowed"

tests/test_general.py

@@ -1,3 +1,4 @@
+import logging
 import os
 
 from urihandler import _load_configuration
@@ -22,3 +23,15 @@ def test_load_configuration(self):
             os.path.join(os.path.dirname(os.path.realpath(__file__)), "test.yaml")
         )
         assert "uris" in cfg
+
+    def test_load_configuration_bad_file(self, caplog):
+        with caplog.at_level(logging.WARN):
+            _load_configuration(
+                os.path.join(os.path.dirname(os.path.realpath(__file__)), "fail.yaml")
+            )
+        assert len(caplog.records) == 1
+        assert caplog.records[0].message == (
+            "^/no_default_present/(?P<id>\\d+)$: Having no default mimetype when "
+            "declaring multiple mime redirect rules will result in a 406 when no "
+            "accept header is present."
+        )

tests/test_handler.py

@@ -1,6 +1,8 @@
 import logging
 
+import pytest
 from pyramid import testing
+from pyramid.httpexceptions import HTTPNotAcceptable
 
 from urihandler.handler import IUriHandler
 from urihandler.handler import UriHandler
@@ -11,8 +13,6 @@
 
 
 class TestHandler:
-    def test_urihandler_exists(self, urihandler):
-        assert urihandler
 
     def test_no_match(self, urihandler):
         req = testing.DummyRequest()
@@ -26,6 +26,45 @@ def test_mounted_redirect(self, urihandler):
         res = urihandler.handle("http://test.urihandler.org/foobar/18", req)
         assert res == "http://localhost:5555/foobar/18"
 
+    def test_redirect_with_mime_match(self, urihandler):
+        req = testing.DummyRequest(accept="application/json")
+        req.host_url = "http://test.urihandler.org"
+        res = urihandler.handle("http://test.urihandler.org/foobar/18", req)
+        assert res == "http://localhost:5555/foobar/18.json"
+
+        req = testing.DummyRequest(accept="application/*")
+        req.host_url = "http://test.urihandler.org"
+        res = urihandler.handle("http://test.urihandler.org/foobar/18", req)
+        assert res == "http://localhost:5555/foobar/18.json"
+
+    def test_redirect_with_mime_no_match(self, urihandler):
+        req = testing.DummyRequest(accept="application/pdf")
+        req.host_url = "http://test.urihandler.org"
+        res = urihandler.handle("http://test.urihandler.org/foobar/18", req)
+        assert res == "http://localhost:5555/foobar/18"
+
+    def test_redirect_default_set(self, urihandler):
+        req = testing.DummyRequest()
+        req.host_url = "http://test.urihandler.org"
+        res = urihandler.handle("http://test.urihandler.org/pdf_default/18", req)
+        assert res == "http://localhost:5555/pdf_default/18.pdf"
+
+        req = testing.DummyRequest(accept="application/*")
+        req.host_url = "http://test.urihandler.org"
+        res = urihandler.handle("http://test.urihandler.org/pdf_default/18", req)
+        assert res == "http://localhost:5555/pdf_default/18.json"
+
+    def test_redirect_no_default(self, urihandler):
+        req = testing.DummyRequest()
+        req.host_url = "http://test.urihandler.org"
+        with pytest.raises(HTTPNotAcceptable):
+            urihandler.handle("http://test.urihandler.org/mime_no_default/18", req)
+
+        req = testing.DummyRequest(accept="application/pdf")
+        req.host_url = "http://test.urihandler.org"
+        with pytest.raises(HTTPNotAcceptable):
+            urihandler.handle("http://test.urihandler.org/mime_no_default/18", req)
+
     def test_unanchored_redirect(self, urihandler):
         req = testing.DummyRequest()
         req.host_url = "http://test.urihandler.org"

urihandler/__init__.py

@@ -51,10 +51,20 @@ def _load_configuration(path):
     :returns: A :class:`dict` with the config options.
     """
     log.debug("Loading uriregistry config from %s." % path)
-    f = open(path)
-    content = yaml.safe_load(f.read())
+    with open(path) as f:
+        content = yaml.safe_load(f.read())
+
+    # Perform some validation so we can warn/fail early.
+    for redirect_rule in content["uris"]:
+        if "default" not in redirect_rule:
+            if isinstance(redirect_rule["redirect"], dict):
+                log.warning(
+                    f"{redirect_rule['match']}: Having no default mimetype when "
+                    f"declaring multiple mime redirect rules will result in a 406 "
+                    f"when no accept header is present."
+                )
+            continue
     log.debug(content)
-    f.close()
     return content
 
 

urihandler/handler.py

@@ -2,6 +2,8 @@
 import logging
 import re
 
+from pyramid.httpexceptions import HTTPNotAcceptable
+from webob.acceptparse import AcceptNoHeader
 from zope.interface import Interface
 
 log = logging.getLogger(__name__)
@@ -30,11 +32,41 @@ def handle(self, uri, request):
             log.debug(f"Matching {uri} to {u['match']}.")
             m = re.match(u["match"], uri)
             if m:
-                redirect = u["redirect"].format(**m.groupdict())
+                redirect = u["redirect"]
+                if isinstance(redirect, dict):
+                    redirect = self._get_redirect_based_on_accept_header(
+                        request.accept, redirect
+                    )
+                redirect = redirect.format(**m.groupdict())
                 log.debug(f"Match found. Redirecting to {redirect}.")
                 return redirect
         return None
 
+    def _get_redirect_based_on_accept_header(self, accept_header, redirect_rule):
+        """
+        Return the redirect rule based on accept header.
+
+        At its core it simply looks for a matching mime between accept header
+        and the configured mime type redirects. But exceptions apply.
+
+        If there is no accept header specified or if no matching mime is found,
+        the default will be returned. If default is not set, HTTP 406 gets raised.
+        """
+        default = redirect_rule.get("default")
+        if isinstance(accept_header, AcceptNoHeader):
+            if not default:
+                raise HTTPNotAcceptable()
+            return default
+
+        for mime, redirect in redirect_rule.items():
+            if mime in accept_header:
+                return redirect
+
+        if not default:
+            raise HTTPNotAcceptable()
+
+        return default
+
 
 def _build_uri_handler(registry, handlerconfig):
     """