Merge pull request #88 from OnroerendErfgoed/feature/86_accept_header…

…_filtering

Add support for accept header filtering.

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,40 @@ 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"
+        with pytest.raises(HTTPNotAcceptable):
+            urihandler.handle("http://test.urihandler.org/foobar/18", req)
+
+    def test_redirect_default_mime(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_mime(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)
+
     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,7 +32,20 @@ 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):
+                    if isinstance(request.accept, AcceptNoHeader):
+                        redirect = redirect.get("default")
+                        if not redirect:
+                            raise HTTPNotAcceptable()
+                    else:
+                        for mime, redirect in redirect.items():
+                            if mime in request.accept:
+                                break
+                        else:
+                            # No matching mime was found.
+                            raise HTTPNotAcceptable()
+                redirect = redirect.format(**m.groupdict())
                 log.debug(f"Match found. Redirecting to {redirect}.")
                 return redirect
         return None