From ffb5cfe515943bfb112c07bd83b30191798146d6 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 13:07:22 -0500
Subject: [PATCH 01/10] Add `content_negociation` extension

To reject unacceptable client requests based on the `Accept` header
---
 docs/changelog.md        | 20 ++++++---
 docs/reference.md        | 20 +++++++++
 requirements.txt         |  1 +
 roll/__init__.py         | 21 ++++++----
 roll/extensions.py       | 15 ++++++-
 roll/testing.py          | 14 ++++++-
 tests/test_errors.py     | 12 +++---
 tests/test_extensions.py | 91 +++++++++++++++++++++++++++++++++++++++-
 tests/test_hooks.py      |  6 +--
 tests/test_request.py    | 10 +----
 tests/test_views.py      | 10 ++---
 11 files changed, 179 insertions(+), 41 deletions(-)

diff --git a/docs/changelog.md b/docs/changelog.md
index f8751521..49e2b05d 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -10,13 +10,21 @@ A changelog:
 
 ## dev version
 
-* Changed `Roll.hook` signature to also accept kwargs ([#5](https://github.com/pyrates/roll/pull/5))
-* `json` shorcut sets `utf-8` charset in `Content-Type` header ([#13](https://github.com/pyrates/roll/pull/13))
-* Added `static` extension to serve static files for development ([#16](https://github.com/pyrates/roll/pull/16))
-* `cors` accepts `headers` parameter to control `Access-Control-Allow-Headers` ([#12](https://github.com/pyrates/roll/pull/12))
+* Changed `Roll.hook` signature to also accept kwargs
+  ([#5](https://github.com/pyrates/roll/pull/5))
+* `json` shorcut sets `utf-8` charset in `Content-Type` header
+  ([#13](https://github.com/pyrates/roll/pull/13))
+* Added `static` extension to serve static files for development
+  ([#16](https://github.com/pyrates/roll/pull/16))
+* `cors` accepts `headers` parameter to control `Access-Control-Allow-Headers`
+  ([#12](https://github.com/pyrates/roll/pull/12))
+* Added `content_negociation` extension to reject unacceptable client requests
+  based on the `Accept` header
 * **Breaking changes**:
-    * `options` extension is no more applied by default ([#16](https://github.com/pyrates/roll/pull/16))
-    * deprecated `req` pytest fixture is now removed ([#9](https://github.com/pyrates/roll/pull/9))
+    * `options` extension is no more applied by default
+      ([#16](https://github.com/pyrates/roll/pull/16))
+    * deprecated `req` pytest fixture is now removed
+      ([#9](https://github.com/pyrates/roll/pull/9))
 
 ## 0.5.0 — 2017-09-21
 
diff --git a/docs/reference.md b/docs/reference.md
index ec823a0f..ba2bdc9f 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -152,10 +152,19 @@ Combine it with the `cors` extension to handle the preflight request.
 - **app**: Roll app to register the extension against
 
 
+### content_negociation
+
+Deal with content negociation declared during routes definition.
+Will return a `406 Not Acceptable` response in case of mismatch between
+the `Accept` header from the client and the `accepts` parameter set in
+routes. Useful to reject requests which are not expecting the available
+response.
+
 #### Parameters
 
 - **app**: Roll app to register the extension against
 
+
 ### traceback
 
 Print the traceback on the server side if any. Handy for debugging.
@@ -164,6 +173,7 @@ Print the traceback on the server side if any. Handy for debugging.
 
 - **app**: Roll app to register the extension against
 
+
 ### igniter
 
 Display a BIG message when running the server.
@@ -173,6 +183,7 @@ Quite useless, hence so essential!
 
 - **app**: Roll app to register the extension against
 
+
 ### static
 
 Serve static files. Should not be used in production.
@@ -240,3 +251,12 @@ Fired in case of error, can be at each request.
 Use it to customize HTTP error formatting for instance.
 
 Receives `request`, `response` and `error` parameters.
+
+
+### dispatch
+
+Fired after the route matching at each request.
+Use it to customize routes restrictions for instance.
+
+Receives `request` and `payload` parameters. The latter being the result
+of the routes matching (dict).
diff --git a/requirements.txt b/requirements.txt
index 734d2d84..71b4515d 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,4 @@
 autoroutes==0.2.0
 httptools==0.0.9
+mimetype-match==1.0.4
 uvloop==0.8.1
diff --git a/roll/__init__.py b/roll/__init__.py
index 415141aa..434017cd 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -199,10 +199,12 @@ def write(self, *args):
         payload = b'HTTP/1.1 %a %b\r\n' % (
             self.response.status.value, self.response.status.phrase.encode())
         if not isinstance(self.response.body, bytes):
-            self.response.body = self.response.body.encode()
+            self.response.body = str(self.response.body).encode()
         if 'Content-Length' not in self.response.headers:
             length = len(self.response.body)
             self.response.headers['Content-Length'] = length
+        if 'Content-Type' not in self.response.headers:
+            self.response.headers['Content-Type'] = 'text/html'
         for key, value in self.response.headers.items():
             payload += b'%b: %b\r\n' % (key.encode(), str(value).encode())
         payload += b'\r\n%b' % self.response.body
@@ -242,7 +244,7 @@ async def shutdown(self):
     async def __call__(self, request: Request, response: Response):
         try:
             if not await self.hook('request', request, response):
-                params, handler = self.dispatch(request)
+                params, handler = await self.dispatch(request)
                 await handler(request, response, **params)
         except Exception as error:
             await self.on_error(request, response, error)
@@ -268,22 +270,25 @@ async def on_error(self, request: Request, response: Response, error):
     def factory(self):
         return self.Protocol(self)
 
-    def route(self, path: str, methods: list=None):
+    def route(self, path: str, methods: list=None, **extras):
         if methods is None:
             methods = ['GET']
 
         def wrapper(func):
-            self.routes.add(path, **{m: func for m in methods})
+            handlers = {method: func for method in methods}
+            handlers.update(extras)
+            self.routes.add(path, **handlers)
             return func
 
         return wrapper
 
-    def dispatch(self, request: Request):
-        handlers, params = self.routes.match(request.path)
-        if request.method not in handlers:
+    async def dispatch(self, request: Request):
+        payload, params = self.routes.match(request.path)
+        await self.hook('dispatch', request, payload)
+        if request.method not in payload:
             raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
         request.kwargs.update(params)
-        return params, handlers[request.method]
+        return params, payload[request.method]
 
     def listen(self, name: str):
         def wrapper(func):
diff --git a/roll/extensions.py b/roll/extensions.py
index e65369ad..e9e80279 100644
--- a/roll/extensions.py
+++ b/roll/extensions.py
@@ -5,6 +5,8 @@
 from pathlib import Path
 from traceback import print_exc
 
+from mimetype_match import get_best_match
+
 from . import HttpError
 
 
@@ -48,6 +50,15 @@ async def handle_options(request, response):
         return request.method == 'OPTIONS'
 
 
+def content_negociation(app):
+
+    @app.listen('dispatch')
+    async def reject_unacceptable_requests(request, payload):
+        accept = request.headers.get('Accept', '*/*')
+        if get_best_match(accept, payload['accepts']) is None:
+            raise HttpError(HTTPStatus.NOT_ACCEPTABLE)
+
+
 def traceback(app):
 
     @app.listen('error')
@@ -108,8 +119,8 @@ async def serve(request, response, path):
         content_type, encoding = mimetypes.guess_type(str(abspath))
         with abspath.open('rb') as source:
             response.body = source.read()
-            response.headers['Content-Type'] = (content_type
-                                                or 'application/octet-stream')
+            response.headers['Content-Type'] = (content_type or
+                                                'application/octet-stream')
 
     @app.listen('startup')
     async def register_route():
diff --git a/roll/testing.py b/roll/testing.py
index 25a356ba..218e1b9d 100644
--- a/roll/testing.py
+++ b/roll/testing.py
@@ -4,6 +4,15 @@
 import pytest
 
 
+class Transport:
+
+    def write(self, data):
+        ...
+
+    def close(self):
+        ...
+
+
 class Client:
 
     # Default content type for request body encoding, change it to your own
@@ -36,12 +45,15 @@ async def request(self, path, method='GET', body=b'', headers=None,
             headers['Content-Type'] = content_type
         body, headers = self.encode_body(body, headers)
         protocol = self.app.factory()
+        protocol.connection_made(Transport())
         protocol.on_message_begin()
         protocol.on_url(path.encode())
         protocol.request.body = body
         protocol.request.method = method
         protocol.request.headers = headers
-        return await self.app(protocol.request, protocol.response)
+        await self.app(protocol.request, protocol.response)
+        protocol.write()
+        return protocol.response
 
     async def get(self, path, **kwargs):
         return await self.request(path, method='GET', **kwargs)
diff --git a/tests/test_errors.py b/tests/test_errors.py
index 8507be9c..f14dd279 100644
--- a/tests/test_errors.py
+++ b/tests/test_errors.py
@@ -14,7 +14,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    assert resp.body == 'Oops.'
+    assert resp.body == b'Oops.'
 
 
 async def test_httpstatus_error(client, app):
@@ -25,7 +25,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.BAD_REQUEST
-    assert resp.body == 'Really bad.'
+    assert resp.body == b'Really bad.'
 
 
 async def test_error_only_with_status(client, app):
@@ -36,7 +36,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    assert resp.body == 'Internal Server Error'
+    assert resp.body == b'Internal Server Error'
 
 
 async def test_error_only_with_httpstatus(client, app):
@@ -47,7 +47,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    assert resp.body == 'Internal Server Error'
+    assert resp.body == b'Internal Server Error'
 
 
 async def test_error_subclasses_with_super(client, app):
@@ -63,7 +63,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    assert resp.body == '<h1>Oops.</h1>'
+    assert resp.body == b'<h1>Oops.</h1>'
 
 
 async def test_error_subclasses_without_super(client, app):
@@ -79,4 +79,4 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    assert resp.body == '<h1>Oops.</h1>'
+    assert resp.body == b'<h1>Oops.</h1>'
diff --git a/tests/test_extensions.py b/tests/test_extensions.py
index d1f3c2e6..2445fdf5 100644
--- a/tests/test_extensions.py
+++ b/tests/test_extensions.py
@@ -18,7 +18,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.OK
-    assert resp.body == 'test response'
+    assert resp.body == b'test response'
     assert resp.headers['Access-Control-Allow-Origin'] == '*'
 
 
@@ -167,3 +167,92 @@ async def test_can_change_static_prefix(client, app):
     resp = await client.get('/foo/index.html')
     assert resp.status == HTTPStatus.OK
     assert b'Test' in resp.body
+
+
+async def test_get_accept_content_negociation(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', accepts=['text/html'])
+    async def get(req, resp):
+        resp.body = 'accepted'
+
+    resp = await client.get('/test', headers={'Accept': 'text/html'})
+    assert resp.status == HTTPStatus.OK
+    assert resp.body == b'accepted'
+    assert resp.headers['Content-Type'] == 'text/html'
+
+
+async def test_get_accept_content_negociation_if_many(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', accepts=['text/html', 'application/json'])
+    async def get(req, resp):
+        if req.headers['Accept'] == 'text/html':
+            resp.body = '<h1>accepted</h1>'
+        elif req.headers['Accept'] == 'application/json':
+            resp.json = {'status': 'accepted'}
+
+    resp = await client.get('/test', headers={'Accept': 'text/html'})
+    assert resp.status == HTTPStatus.OK
+    assert resp.body == b'<h1>accepted</h1>'
+    assert resp.headers['Content-Type'] == 'text/html'
+    resp = await client.get('/test', headers={'Accept': 'application/json'})
+    assert resp.status == HTTPStatus.OK
+    assert resp.body == b'{"status":"accepted"}'
+    assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
+
+
+async def test_get_reject_content_negociation(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', accepts=['text/html'])
+    async def get(req, resp):
+        resp.body = 'rejected'
+
+    resp = await client.get('/test', headers={'Accept': 'text/css'})
+    assert resp.status == HTTPStatus.NOT_ACCEPTABLE
+
+
+async def test_get_accept_star_content_negociation(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', accepts=['text/css'])
+    async def get(req, resp):
+        resp.body = 'accepted'
+
+    resp = await client.get('/test', headers={'Accept': 'text/*'})
+    assert resp.status == HTTPStatus.OK
+
+
+async def test_post_accept_content_negociation(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', methods=['POST'], accepts=['application/json'])
+    async def get(req, resp):
+        resp.json = {'status': 'accepted'}
+
+    client.content_type = 'application/x-www-form-urlencoded'
+    resp = await client.post('/test', body={'key': 'value'},
+                             headers={'Accept': 'application/json'})
+    assert resp.status == HTTPStatus.OK
+    assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
+    assert resp.body == b'{"status":"accepted"}'
+
+
+async def test_post_reject_content_negociation(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', methods=['POST'], accepts=['text/html'])
+    async def get(req, resp):
+        resp.json = {'status': 'accepted'}
+
+    client.content_type = 'application/x-www-form-urlencoded'
+    resp = await client.post('/test', body={'key': 'value'},
+                             headers={'Accept': 'application/json'})
+    assert resp.status == HTTPStatus.NOT_ACCEPTABLE
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index db62f684..91ceec6d 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -13,7 +13,7 @@ async def test_request_hook_can_alter_response(client, app):
     @app.listen('request')
     async def listener(request, response):
         response.status = 400
-        response.body = 'another response'
+        response.body = b'another response'
         return True  # Shortcut the response process.
 
     @app.route('/test')
@@ -22,7 +22,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.BAD_REQUEST
-    assert resp.body == 'another response'
+    assert resp.body == b'another response'
 
 
 async def test_response_hook_can_alter_response(client, app):
@@ -39,7 +39,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.BAD_REQUEST
-    assert resp.body == 'another response'
+    assert resp.body == b'another response'
 
 
 async def test_error_with_json_format(client, app):
diff --git a/tests/test_request.py b/tests/test_request.py
index 3a2c1b70..0cb106d0 100644
--- a/tests/test_request.py
+++ b/tests/test_request.py
@@ -2,19 +2,11 @@
 
 import pytest
 from roll import Protocol, HttpError
+from roll.testing import Transport
 
 pytestmark = pytest.mark.asyncio
 
 
-class Transport:
-
-    def write(self, data):
-        ...
-
-    def close(self):
-        ...
-
-
 @pytest.fixture
 def protocol(app):
     protocol = Protocol(app)
diff --git a/tests/test_views.py b/tests/test_views.py
index 81396066..f489cecc 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -13,7 +13,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.OK
-    assert resp.body == 'test response'
+    assert resp.body == b'test response'
 
 
 async def test_simple_non_200_response(client, app):
@@ -53,9 +53,9 @@ async def test_post_json(client, app):
     async def get(req, resp):
         resp.body = req.body
 
-    resp = await client.post('/test', body={"key": "value"})
+    resp = await client.post('/test', body={'key': 'value'})
     assert resp.status == HTTPStatus.OK
-    assert resp.body == '{"key": "value"}'
+    assert resp.body == b'{"key": "value"}'
 
 
 async def test_post_urlencoded(client, app):
@@ -65,9 +65,9 @@ async def get(req, resp):
         resp.body = req.body
 
     client.content_type = 'application/x-www-form-urlencoded'
-    resp = await client.post('/test', body={"key": "value"})
+    resp = await client.post('/test', body={'key': 'value'})
     assert resp.status == HTTPStatus.OK
-    assert resp.body == 'key=value'
+    assert resp.body == b'key=value'
 
 
 async def test_can_define_twice_a_route_with_different_payloads(client, app):

From 7655d0ec3fc4009d6f150d8dda4715388abe6c11 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 14:37:11 -0500
Subject: [PATCH 02/10] Post-review fixes

---
 docs/changelog.md  |  1 +
 docs/reference.md  |  9 ---------
 roll/__init__.py   | 21 +++++++++------------
 roll/extensions.py |  6 +++---
 4 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/docs/changelog.md b/docs/changelog.md
index 49e2b05d..b16781d0 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -20,6 +20,7 @@ A changelog:
   ([#12](https://github.com/pyrates/roll/pull/12))
 * Added `content_negociation` extension to reject unacceptable client requests
   based on the `Accept` header
+  ([#21](https://github.com/pyrates/roll/pull/21))
 * **Breaking changes**:
     * `options` extension is no more applied by default
       ([#16](https://github.com/pyrates/roll/pull/16))
diff --git a/docs/reference.md b/docs/reference.md
index ba2bdc9f..f7694d3e 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -251,12 +251,3 @@ Fired in case of error, can be at each request.
 Use it to customize HTTP error formatting for instance.
 
 Receives `request`, `response` and `error` parameters.
-
-
-### dispatch
-
-Fired after the route matching at each request.
-Use it to customize routes restrictions for instance.
-
-Receives `request` and `payload` parameters. The latter being the result
-of the routes matching (dict).
diff --git a/roll/__init__.py b/roll/__init__.py
index 434017cd..b30c7afb 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -102,7 +102,7 @@ class Request:
     The parsing is made by `httptools.HttpRequestParser`.
     """
     __slots__ = ('url', 'path', 'query_string', 'query', 'method', 'kwargs',
-                 'body', 'headers')
+                 'body', 'headers', 'routing')
 
     def __init__(self):
         self.kwargs = {}
@@ -149,6 +149,7 @@ class Protocol(asyncio.Protocol):
     Request = Request
     Response = Response
     RequestParser = HttpRequestParser
+    DEFAULT_CONTENT_TYPE = 'text/html'
 
     def __init__(self, app):
         self.app = app
@@ -204,7 +205,7 @@ def write(self, *args):
             length = len(self.response.body)
             self.response.headers['Content-Length'] = length
         if 'Content-Type' not in self.response.headers:
-            self.response.headers['Content-Type'] = 'text/html'
+            self.response.headers['Content-Type'] = self.DEFAULT_CONTENT_TYPE
         for key, value in self.response.headers.items():
             payload += b'%b: %b\r\n' % (key.encode(), str(value).encode())
         payload += b'\r\n%b' % self.response.body
@@ -243,8 +244,12 @@ async def shutdown(self):
 
     async def __call__(self, request: Request, response: Response):
         try:
+            request.routing, params = self.routes.match(request.path)
             if not await self.hook('request', request, response):
-                params, handler = await self.dispatch(request)
+                if request.method not in request.routing:
+                    raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
+                request.kwargs.update(params)
+                handler = request.routing[request.method]
                 await handler(request, response, **params)
         except Exception as error:
             await self.on_error(request, response, error)
@@ -270,7 +275,7 @@ async def on_error(self, request: Request, response: Response, error):
     def factory(self):
         return self.Protocol(self)
 
-    def route(self, path: str, methods: list=None, **extras):
+    def route(self, path: str, methods: list=None, **extras: dict):
         if methods is None:
             methods = ['GET']
 
@@ -282,14 +287,6 @@ def wrapper(func):
 
         return wrapper
 
-    async def dispatch(self, request: Request):
-        payload, params = self.routes.match(request.path)
-        await self.hook('dispatch', request, payload)
-        if request.method not in payload:
-            raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
-        request.kwargs.update(params)
-        return params, payload[request.method]
-
     def listen(self, name: str):
         def wrapper(func):
             self.hooks.setdefault(name, [])
diff --git a/roll/extensions.py b/roll/extensions.py
index e9e80279..29b9c035 100644
--- a/roll/extensions.py
+++ b/roll/extensions.py
@@ -52,10 +52,10 @@ async def handle_options(request, response):
 
 def content_negociation(app):
 
-    @app.listen('dispatch')
-    async def reject_unacceptable_requests(request, payload):
+    @app.listen('request')
+    async def reject_unacceptable_requests(request, response):
         accept = request.headers.get('Accept', '*/*')
-        if get_best_match(accept, payload['accepts']) is None:
+        if get_best_match(accept, request.routing['accepts']) is None:
             raise HttpError(HTTPStatus.NOT_ACCEPTABLE)
 
 

From 0bde825398fc0aed0127609f2dc617af756e746d Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 14:49:40 -0500
Subject: [PATCH 03/10] Explicit json loading in tests as ujson differs

---
 tests/test_extensions.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_extensions.py b/tests/test_extensions.py
index 2445fdf5..f9a09424 100644
--- a/tests/test_extensions.py
+++ b/tests/test_extensions.py
@@ -200,7 +200,7 @@ async def get(req, resp):
     assert resp.headers['Content-Type'] == 'text/html'
     resp = await client.get('/test', headers={'Accept': 'application/json'})
     assert resp.status == HTTPStatus.OK
-    assert resp.body == b'{"status":"accepted"}'
+    assert json.loads(resp.body) == {'status': 'accepted'}
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
 
 
@@ -241,7 +241,7 @@ async def get(req, resp):
                              headers={'Accept': 'application/json'})
     assert resp.status == HTTPStatus.OK
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
-    assert resp.body == b'{"status":"accepted"}'
+    assert json.loads(resp.body) == {'status': 'accepted'}
 
 
 async def test_post_reject_content_negociation(client, app):

From 4a9c1078dae19b740b7a06b0efee03aada2ae9aa Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 14:54:17 -0500
Subject: [PATCH 04/10] Compatibility with Python 3.5 and json loading

---
 tests/test_extensions.py | 8 ++++----
 tests/test_hooks.py      | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/test_extensions.py b/tests/test_extensions.py
index f9a09424..633eaf00 100644
--- a/tests/test_extensions.py
+++ b/tests/test_extensions.py
@@ -85,7 +85,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
-    assert json.loads(resp.body) == {'key': 'value'}
+    assert json.loads(resp.body.decode()) == {'key': 'value'}
     assert resp.status == HTTPStatus.OK
 
 
@@ -98,7 +98,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
-    assert json.loads(resp.body) == {'key': 'value'}
+    assert json.loads(resp.body.decode()) == {'key': 'value'}
     assert resp.status == HTTPStatus.BAD_REQUEST
 
 
@@ -200,7 +200,7 @@ async def get(req, resp):
     assert resp.headers['Content-Type'] == 'text/html'
     resp = await client.get('/test', headers={'Accept': 'application/json'})
     assert resp.status == HTTPStatus.OK
-    assert json.loads(resp.body) == {'status': 'accepted'}
+    assert json.loads(resp.body.decode()) == {'status': 'accepted'}
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
 
 
@@ -241,7 +241,7 @@ async def get(req, resp):
                              headers={'Accept': 'application/json'})
     assert resp.status == HTTPStatus.OK
     assert resp.headers['Content-Type'] == 'application/json; charset=utf-8'
-    assert json.loads(resp.body) == {'status': 'accepted'}
+    assert json.loads(resp.body.decode()) == {'status': 'accepted'}
 
 
 async def test_post_reject_content_negociation(client, app):
diff --git a/tests/test_hooks.py b/tests/test_hooks.py
index 91ceec6d..a602f203 100644
--- a/tests/test_hooks.py
+++ b/tests/test_hooks.py
@@ -55,7 +55,7 @@ async def get(req, resp):
 
     resp = await client.get('/test')
     assert resp.status == HTTPStatus.INTERNAL_SERVER_ERROR
-    error = json.loads(resp.body)
+    error = json.loads(resp.body.decode())
     assert error == {"status": 500, "message": "JSON error"}
 
 

From 120245c4700c6aa4a905eee9415aae6aa7e8f3a2 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 17:51:17 -0500
Subject: [PATCH 05/10] Turned matched route payload/vars as a namedtuple

---
 roll/__init__.py   | 18 +++++++++++-------
 roll/extensions.py |  2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/roll/__init__.py b/roll/__init__.py
index b30c7afb..fbefcfdc 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -9,6 +9,7 @@
 a test failing): https://github.com/pyrates/roll/issues/new
 """
 import asyncio
+from collections import namedtuple
 from http import HTTPStatus
 from typing import TypeVar
 from urllib.parse import parse_qs, unquote
@@ -99,10 +100,10 @@ def float(self, key: str, default=...):
 class Request:
     """A container for the result of the parsing on each request.
 
-    The parsing is made by `httptools.HttpRequestParser`.
+    The default parsing is made by `httptools.HttpRequestParser`.
     """
     __slots__ = ('url', 'path', 'query_string', 'query', 'method', 'kwargs',
-                 'body', 'headers', 'routing')
+                 'body', 'headers', 'route')
 
     def __init__(self):
         self.kwargs = {}
@@ -224,6 +225,9 @@ def match(self, url: str):
         return payload, params
 
 
+Route = namedtuple('Route', ['payload', 'vars'])
+
+
 class Roll:
     """Deal with routes dispatching and events listening.
 
@@ -244,13 +248,13 @@ async def shutdown(self):
 
     async def __call__(self, request: Request, response: Response):
         try:
-            request.routing, params = self.routes.match(request.path)
+            request.route = Route(*self.routes.match(request.path))
             if not await self.hook('request', request, response):
-                if request.method not in request.routing:
+                if request.method not in request.route.payload:
                     raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
-                request.kwargs.update(params)
-                handler = request.routing[request.method]
-                await handler(request, response, **params)
+                request.kwargs.update(request.route.vars)
+                handler = request.route.payload[request.method]
+                await handler(request, response, **request.route.vars)
         except Exception as error:
             await self.on_error(request, response, error)
         try:
diff --git a/roll/extensions.py b/roll/extensions.py
index 29b9c035..290213c7 100644
--- a/roll/extensions.py
+++ b/roll/extensions.py
@@ -55,7 +55,7 @@ def content_negociation(app):
     @app.listen('request')
     async def reject_unacceptable_requests(request, response):
         accept = request.headers.get('Accept', '*/*')
-        if get_best_match(accept, request.routing['accepts']) is None:
+        if get_best_match(accept, request.route.payload['accepts']) is None:
             raise HttpError(HTTPStatus.NOT_ACCEPTABLE)
 
 

From 666210b6d8789ab0859d2bbe222b34caefc82a35 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 17:54:00 -0500
Subject: [PATCH 06/10] Remove default content type

---
 roll/__init__.py         | 3 ---
 tests/test_extensions.py | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/roll/__init__.py b/roll/__init__.py
index fbefcfdc..ae043cec 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -150,7 +150,6 @@ class Protocol(asyncio.Protocol):
     Request = Request
     Response = Response
     RequestParser = HttpRequestParser
-    DEFAULT_CONTENT_TYPE = 'text/html'
 
     def __init__(self, app):
         self.app = app
@@ -205,8 +204,6 @@ def write(self, *args):
         if 'Content-Length' not in self.response.headers:
             length = len(self.response.body)
             self.response.headers['Content-Length'] = length
-        if 'Content-Type' not in self.response.headers:
-            self.response.headers['Content-Type'] = self.DEFAULT_CONTENT_TYPE
         for key, value in self.response.headers.items():
             payload += b'%b: %b\r\n' % (key.encode(), str(value).encode())
         payload += b'\r\n%b' % self.response.body
diff --git a/tests/test_extensions.py b/tests/test_extensions.py
index 633eaf00..96a3a200 100644
--- a/tests/test_extensions.py
+++ b/tests/test_extensions.py
@@ -175,6 +175,7 @@ async def test_get_accept_content_negociation(client, app):
 
     @app.route('/test', accepts=['text/html'])
     async def get(req, resp):
+        resp.headers['Content-Type'] = 'text/html'
         resp.body = 'accepted'
 
     resp = await client.get('/test', headers={'Accept': 'text/html'})
@@ -190,6 +191,7 @@ async def test_get_accept_content_negociation_if_many(client, app):
     @app.route('/test', accepts=['text/html', 'application/json'])
     async def get(req, resp):
         if req.headers['Accept'] == 'text/html':
+            resp.headers['Content-Type'] = 'text/html'
             resp.body = '<h1>accepted</h1>'
         elif req.headers['Accept'] == 'application/json':
             resp.json = {'status': 'accepted'}

From 2cd8904ea7ef1b970dbd5c83daa4e2a31947b197 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Tue, 7 Nov 2017 18:03:16 -0500
Subject: [PATCH 07/10] =?UTF-8?q?Let=20Request=E2=80=99s=20kwargs=20empty?=
 =?UTF-8?q?=20by=20default?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 docs/reference.md | 9 +++++++--
 roll/__init__.py  | 5 ++---
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/docs/reference.md b/docs/reference.md
index f7694d3e..ad05873d 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -26,8 +26,12 @@ The `status` can be either a `http.HTTPStatus` instance or an integer.
 
 ### Request
 
-A container for the result of the parsing on each request made by
-`httptools.HttpRequestParser`.
+A container for the result of the parsing on each request.
+The default parsing is made by `httptools.HttpRequestParser`.
+
+You can use the empty `kwargs` dict to attach whatever you want,
+especially useful for extensions.
+
 
 #### Properties
 
@@ -38,6 +42,7 @@ A container for the result of the parsing on each request made by
 - **method** (`str`): HTTP verb
 - **body** (`bytes`): raw body as received by Roll
 - **headers** (`dict`): HTTP headers
+- **route** (`Route`): a namedtuple storing results from URL matching
 - **kwargs** (`dict`): store here any extra data needed in the Request lifetime
 
 
diff --git a/roll/__init__.py b/roll/__init__.py
index ae043cec..7b6d67d4 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -102,8 +102,8 @@ class Request:
 
     The default parsing is made by `httptools.HttpRequestParser`.
     """
-    __slots__ = ('url', 'path', 'query_string', 'query', 'method', 'kwargs',
-                 'body', 'headers', 'route')
+    __slots__ = ('url', 'path', 'query_string', 'query', 'method', 'body',
+                 'headers', 'route', 'kwargs')
 
     def __init__(self):
         self.kwargs = {}
@@ -249,7 +249,6 @@ async def __call__(self, request: Request, response: Response):
             if not await self.hook('request', request, response):
                 if request.method not in request.route.payload:
                     raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
-                request.kwargs.update(request.route.vars)
                 handler = request.route.payload[request.method]
                 await handler(request, response, **request.route.vars)
         except Exception as error:

From 9da2e745c3d2cbb1f2fbab1582889091ced973e7 Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Wed, 8 Nov 2017 11:11:04 -0500
Subject: [PATCH 08/10] Post-review fixes

---
 docs/how-to-guides.md | 21 +++++++++++++++++++++
 docs/index.md         |  1 +
 docs/reference.md     | 16 +++++++++++++++-
 requirements.txt      |  1 -
 roll/__init__.py      |  9 +++++----
 roll/extensions.py    |  9 +++++++--
 6 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/docs/how-to-guides.md b/docs/how-to-guides.md
index 35994c3e..9fd5613b 100644
--- a/docs/how-to-guides.md
+++ b/docs/how-to-guides.md
@@ -46,6 +46,27 @@ always a bonus. The `response` object is modified in place.
 Make sure to check these out!*
 
 
+## How to deal with content negociation
+
+The [`content_negociation` extension](reference.md#content_negociation)
+is made for this purpose, you can use it that way:
+
+```python
+extensions.content_negociation(app)
+
+@app.route('/test', accepts=['text/html', 'application/json'])
+async def get(req, resp):
+    if req.headers['Accept'] == 'text/html':
+        resp.headers['Content-Type'] = 'text/html'
+        resp.body = '<h1>accepted</h1>'
+    elif req.headers['Accept'] == 'application/json':
+        resp.json = {'status': 'accepted'}
+```
+
+Requests with `Accept` header not matching `text/html` or
+`application/json` will be honored with a `406 Not Acceptable` response.
+
+
 ## How to return an HTTP error
 
 There are many reasons to return an HTTP error, with Roll you have to
diff --git a/docs/index.md b/docs/index.md
index ddc1619a..a3f64c7e 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -56,6 +56,7 @@ A how-to guide:
 
 * [How to install Roll](how-to-guides.md#how-to-install-roll)
 * [How to create an extension](how-to-guides.md#how-to-create-an-extension)
+* [How to deal with content negociation](how-to-guides.md#how-to-deal-with-content-negociation)
 * [How to return an HTTP error](how-to-guides.md#how-to-return-an-http-error)
 * [How to return JSON content](how-to-guides.md#how-to-return-json-content)
 * [How to subclass Roll itself](how-to-guides.md#how-to-subclass-roll-itself)
diff --git a/docs/reference.md b/docs/reference.md
index ad05873d..6288b469 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -42,7 +42,7 @@ especially useful for extensions.
 - **method** (`str`): HTTP verb
 - **body** (`bytes`): raw body as received by Roll
 - **headers** (`dict`): HTTP headers
-- **route** (`Route`): a namedtuple storing results from URL matching
+- **route** (`Route`): a [Route instance](#Route) storing results from URL matching
 - **kwargs** (`dict`): store here any extra data needed in the Request lifetime
 
 
@@ -110,6 +110,15 @@ parser. Default routes use [autoroutes](https://github.com/pyrates/autoroutes),
 please refers to that documentation for available patterns.
 
 
+### Route
+
+A namedtuple to collect matched route data with attributes:
+
+* **payload** (`dict`): the data received by the `@app.route` decorator,
+  contains all handlers plus optionnal custom data.
+* **vars** (`dict`): URL placeholders resolved for the current route.
+
+
 ## Extensions
 
 Please read
@@ -170,6 +179,11 @@ response.
 - **app**: Roll app to register the extension against
 
 
+#### Requirements
+
+- mimetype-match>=1.0.4
+
+
 ### traceback
 
 Print the traceback on the server side if any. Handy for debugging.
diff --git a/requirements.txt b/requirements.txt
index 71b4515d..734d2d84 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,3 @@
 autoroutes==0.2.0
 httptools==0.0.9
-mimetype-match==1.0.4
 uvloop==0.8.1
diff --git a/roll/__init__.py b/roll/__init__.py
index 7b6d67d4..28eeed11 100644
--- a/roll/__init__.py
+++ b/roll/__init__.py
@@ -247,7 +247,8 @@ async def __call__(self, request: Request, response: Response):
         try:
             request.route = Route(*self.routes.match(request.path))
             if not await self.hook('request', request, response):
-                if request.method not in request.route.payload:
+                # Uppercased in order to only consider HTTP verbs.
+                if request.method.upper() not in request.route.payload:
                     raise HttpError(HTTPStatus.METHOD_NOT_ALLOWED)
                 handler = request.route.payload[request.method]
                 await handler(request, response, **request.route.vars)
@@ -280,9 +281,9 @@ def route(self, path: str, methods: list=None, **extras: dict):
             methods = ['GET']
 
         def wrapper(func):
-            handlers = {method: func for method in methods}
-            handlers.update(extras)
-            self.routes.add(path, **handlers)
+            payload = {method: func for method in methods}
+            payload.update(extras)
+            self.routes.add(path, **payload)
             return func
 
         return wrapper
diff --git a/roll/extensions.py b/roll/extensions.py
index 290213c7..ac296376 100644
--- a/roll/extensions.py
+++ b/roll/extensions.py
@@ -1,12 +1,11 @@
 import asyncio
 import logging
 import mimetypes
+import sys
 from http import HTTPStatus
 from pathlib import Path
 from traceback import print_exc
 
-from mimetype_match import get_best_match
-
 from . import HttpError
 
 
@@ -52,6 +51,12 @@ async def handle_options(request, response):
 
 def content_negociation(app):
 
+    try:
+        from mimetype_match import get_best_match
+    except ImportError:
+        sys.exit('Please install mimetype-match>=1.0.4 to be able to use the '
+                 'content_negociation extension.')
+
     @app.listen('request')
     async def reject_unacceptable_requests(request, response):
         accept = request.headers.get('Accept', '*/*')

From 4caa40332001113b7b13e453af87273ca20418de Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Wed, 8 Nov 2017 11:37:34 -0500
Subject: [PATCH 09/10] Reject requests if Accept header is absent

---
 roll/extensions.py       |  5 +++--
 tests/test_extensions.py | 12 ++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/roll/extensions.py b/roll/extensions.py
index ac296376..67c4bef9 100644
--- a/roll/extensions.py
+++ b/roll/extensions.py
@@ -59,8 +59,9 @@ def content_negociation(app):
 
     @app.listen('request')
     async def reject_unacceptable_requests(request, response):
-        accept = request.headers.get('Accept', '*/*')
-        if get_best_match(accept, request.route.payload['accepts']) is None:
+        accept = request.headers.get('Accept')
+        accepts = request.route.payload['accepts']
+        if accept is None or get_best_match(accept, accepts) is None:
             raise HttpError(HTTPStatus.NOT_ACCEPTABLE)
 
 
diff --git a/tests/test_extensions.py b/tests/test_extensions.py
index 96a3a200..204a9a3f 100644
--- a/tests/test_extensions.py
+++ b/tests/test_extensions.py
@@ -218,6 +218,18 @@ async def get(req, resp):
     assert resp.status == HTTPStatus.NOT_ACCEPTABLE
 
 
+async def test_get_reject_content_negociation_if_no_accept_header(client, app):
+
+    extensions.content_negociation(app)
+
+    @app.route('/test', accepts=['*/*'])
+    async def get(req, resp):
+        resp.body = 'rejected'
+
+    resp = await client.get('/test')
+    assert resp.status == HTTPStatus.NOT_ACCEPTABLE
+
+
 async def test_get_accept_star_content_negociation(client, app):
 
     extensions.content_negociation(app)

From e737b82129c9ca6316c59461fdb4d665284d4f6b Mon Sep 17 00:00:00 2001
From: David Larlet <david@larlet.fr>
Date: Wed, 8 Nov 2017 11:49:59 -0500
Subject: [PATCH 10/10] Add mimetype-match dependency for CI and dev

---
 requirements-dev.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/requirements-dev.txt b/requirements-dev.txt
index be628831..a882ad69 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,3 +1,4 @@
+mimetype-match==1.0.4
 mkdocs==0.16.3
 pytest==3.1.2
 pytest-asyncio==0.6.0