diff --git a/tests/conftest.py b/tests/conftest.py index dbd1687..2b34291 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,7 +10,11 @@ def handlerconfig(): { "match": r"^/foobar/(?P\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\w+)$", @@ -30,6 +34,24 @@ def handlerconfig(): "mount": True, "redirect": "http://localhost:5555/foo/{foo_id}/bar/{bar_id}", }, + { + "match": r"^/pdf_default/(?P\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\d+)$", + "mount": True, + "redirect": { + "text/html": "http://localhost:5555/pdf_default/{id}", + "application/json": "http://localhost:5555/pdf_default/{id}.json", + } + }, ] } return cfg diff --git a/tests/fail.yaml b/tests/fail.yaml new file mode 100644 index 0000000..019bda7 --- /dev/null +++ b/tests/fail.yaml @@ -0,0 +1,6 @@ +uris: + - match: '^/no_default_present/(?P\d+)$' + mount: True + redirect: + text/html: 'http://localhost:5555/foobar/{id}' + application/json: 'http://localhost:5555/foobar/{id}.json' diff --git a/tests/test.yaml b/tests/test.yaml index 22faf8c..789a28b 100644 --- a/tests/test.yaml +++ b/tests/test.yaml @@ -1,9 +1,12 @@ uris: - - match: '^/foobar/(?P\d+)$' - mount: True - redirect: 'http://localhost:5555/foobar/{id}' - - match: '^/bar/(?P\w+)$' - redirect: 'http://localhost:5555/bar/{name}' - - match: '^urn:x-barbar:(?P\w+):(?P\d+)$' - mount: False - redirect: 'http://localhost:2222/{namespace}/{id}' + - match: '^/foobar/(?P\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\w+)$' + redirect: 'http://localhost:5555/bar/{name}' + - match: '^urn:x-barbar:(?P\w+):(?P\d+)$' + mount: False + redirect: 'http://localhost:2222/{namespace}/{id}' diff --git a/tests/test_functional.py b/tests/test_functional.py index 6c7a05c..ecc26d4 100644 --- a/tests/test_functional.py +++ b/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" diff --git a/tests/test_general.py b/tests/test_general.py index 813b0c3..aea56c9 100644 --- a/tests/test_general.py +++ b/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\\d+)$: Having no default mimetype when " + "declaring multiple mime redirect rules will result in a 406 when no " + "accept header is present." + ) diff --git a/tests/test_handler.py b/tests/test_handler.py index 1ee82ca..222841c 100644 --- a/tests/test_handler.py +++ b/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" diff --git a/urihandler/__init__.py b/urihandler/__init__.py index 5c99e5c..23e1a67 100644 --- a/urihandler/__init__.py +++ b/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 diff --git a/urihandler/handler.py b/urihandler/handler.py index fd974c6..b300d39 100644 --- a/urihandler/handler.py +++ b/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