aiohttp.wep.Application.make_handler access_log_class support (#2316)

* Initial access_log_factory support.

* Added changes.

* abstraction level changes for AccessLogger.

* Added RequestHandler docstrings.

* Fix changes naming.

* Added access_log_class check for Application.make_handler

* Added basic tests for access_log_class.

* Added docs for access_log_class

* Fix typo request->response

* _log -> log

* Added more details to TypeError regarding access_log_class.

aiohttp/abc.py

@@ -146,3 +146,14 @@ def write_eof(self, chunk=b''):
     @abstractmethod
     def drain(self):
         """Flush the write buffer."""
+
+
+class AbstractAccessLogger(ABC):
+
+    def __init__(self, logger, log_format):
+        self.logger = logger
+        self.log_format = log_format
+
+    @abstractmethod
+    def log(self, request, response, time):
+        """Emit log to logger"""

aiohttp/helpers.py

@@ -23,6 +23,7 @@
 from yarl import URL
 
 from . import hdrs
+from .abc import AbstractAccessLogger
 from .log import client_logger
 
 
@@ -346,7 +347,7 @@ def content_disposition_header(disptype, quote_fields=True, **params):
     return value
 
 
-class AccessLogger:
+class AccessLogger(AbstractAccessLogger):
     """Helper object to log access.
 
     Usage:
@@ -400,7 +401,7 @@ def __init__(self, logger, log_format=LOG_FORMAT):
         :param log_format: apache compatible log format
 
         """
-        self.logger = logger
+        super().__init__(logger, log_format=log_format)
 
         _compiled_format = AccessLogger._FORMAT_CACHE.get(log_format)
         if not _compiled_format:
@@ -513,14 +514,6 @@ def _format_line(self, request, response, time):
                 for key, method in self._methods)
 
     def log(self, request, response, time):
-        """Log access.
-
-        :param message: Request object. May be None.
-        :param environ: Environment dict. May be None.
-        :param response: Response object.
-        :param transport: Tansport object. May be None
-        :param float time: Time taken to serve the request.
-        """
         try:
             fmt_info = self._format_line(request, response, time)
 

aiohttp/web.py

@@ -15,8 +15,9 @@
 from . import (hdrs, web_exceptions, web_fileresponse, web_middlewares,
                web_protocol, web_request, web_response, web_server,
                web_urldispatcher, web_ws)
-from .abc import AbstractMatchInfo, AbstractRouter
+from .abc import AbstractAccessLogger, AbstractMatchInfo, AbstractRouter
 from .frozenlist import FrozenList
+from .helpers import AccessLogger
 from .http import HttpVersion  # noqa
 from .log import access_logger, web_logger
 from .signals import FuncSignal, PostSignal, PreSignal, Signal
@@ -230,7 +231,17 @@ def router(self):
     def middlewares(self):
         return self._middlewares
 
-    def make_handler(self, *, loop=None, **kwargs):
+    def make_handler(self, *,
+                     loop=None,
+                     access_log_class=AccessLogger,
+                     **kwargs):
+
+        if not issubclass(access_log_class, AbstractAccessLogger):
+            raise TypeError(
+                'access_log_class must be subclass of '
+                'aiohttp.abc.AbstractAccessLogger, got {}'.format(
+                    access_log_class))
+
         self._set_loop(loop)
         self.freeze()
 
@@ -240,6 +251,7 @@ def make_handler(self, *, loop=None, **kwargs):
                 kwargs[k] = v
 
         return Server(self._handle, request_factory=self._make_request,
+                      access_log_class=access_log_class,
                       loop=self.loop, **kwargs)
 
     @asyncio.coroutine

aiohttp/web_protocol.py

@@ -61,6 +61,9 @@ class RequestHandler(asyncio.streams.FlowControlMixin, asyncio.Protocol):
     :param logger: custom logger object
     :type logger: aiohttp.log.server_logger
 
+    :param access_log_class: custom class for access_logger
+    :type access_log_class: aiohttp.abc.AbstractAccessLogger
+
     :param access_log: custom logging object
     :type access_log: aiohttp.log.server_logger
 
@@ -83,6 +86,7 @@ def __init__(self, manager, *, loop=None,
                  tcp_keepalive=True,
                  slow_request_timeout=None,
                  logger=server_logger,
+                 access_log_class=helpers.AccessLogger,
                  access_log=access_logger,
                  access_log_format=helpers.AccessLogger.LOG_FORMAT,
                  debug=False,
@@ -138,7 +142,7 @@ def __init__(self, manager, *, loop=None,
         self.debug = debug
         self.access_log = access_log
         if access_log:
-            self.access_logger = helpers.AccessLogger(
+            self.access_logger = access_log_class(
                 access_log, access_log_format)
         else:
             self.access_logger = None

changes/2315.feature

@@ -0,0 +1 @@
+aiohttp.wep.Application.make_handler support access_log_class

docs/abc.rst

@@ -35,7 +35,7 @@ Not Allowed*. :meth:`AbstractMatchInfo.handler` raises
 :attr:`~AbstractMatchInfo.http_exception` on call.
 
 
-.. class:: AbstractRouter
+.. class:: aiohttp.abc.AbstractRouter
 
    Abstract router, :class:`aiohttp.web.Application` accepts it as
    *router* parameter and returns as
@@ -54,7 +54,7 @@ Not Allowed*. :meth:`AbstractMatchInfo.handler` raises
       :return: :class:`AbstractMatchInfo` instance.
 
 
-.. class:: AbstractMatchInfo
+.. class:: aiohttp.abc.AbstractMatchInfo
 
    Abstract *match info*, returned by :meth:`AbstractRouter.resolve` call.
 
@@ -102,7 +102,7 @@ attribute.
 Abstract Cookie Jar
 -------------------
 
-.. class:: AbstractCookieJar
+.. class:: aiohttp.abc.AbstractCookieJar
 
    The cookie jar instance is available as :attr:`ClientSession.cookie_jar`.
 
@@ -146,3 +146,21 @@ Abstract Cookie Jar
 
       :return: :class:`http.cookies.SimpleCookie` with filtered
          cookies for given URL.
+
+Abstract Abstract Access Logger
+-------------------------------
+
+.. class:: aiohttp.abc.AbstractAccessLogger
+
+   An abstract class, base for all :class:`RequestHandler`
+   ``access_logger`` implementations
+
+   Method ``log`` should be overridden.
+
+   .. method:: log(request, response, time)
+
+      :param request: :class:`aiohttp.web.Request` object.
+
+      :param response: :class:`aiohttp.web.Response` object.
+
+      :param float time: Time taken to serve the request.

docs/logging.rst

@@ -87,6 +87,20 @@ Default access log format is::
 
    '%a %l %u %t "%r" %s %b "%{Referrer}i" "%{User-Agent}i"'
 
+.. versionadded:: 2.3.0
+
+*access_log_class* introduced.
+
+Example of drop-in replacement for :class:`aiohttp.helpers.AccessLogger`::
+
+  from aiohttp.abc import AbstractAccessLogger
+
+  class AccessLogger(AbstractAccessLogger):
+
+      def log(self, request, response, time):
+          self.logger.info(f'{request.remote} '
+                           f'"{request.method} {request.path} '
+                           f'done in {time}s: {response.status}')
 
 .. note::
 

docs/web_reference.rst

@@ -1347,6 +1347,9 @@ duplicated like one using :meth:`Application.copy`.
       :data:`aiohttp.log.server_logger`.
     :param access_log: Custom logging object. Default:
       :data:`aiohttp.log.access_logger`.
+    :param access_log_class: class for `access_logger`. Default:
+      :data:`aiohttp.helpers.AccessLogger`.
+      Must to be a subclass of :class:`aiohttp.abc.AbstractAccessLogger`.
     :param str access_log_format: Access log format string. Default:
       :attr:`helpers.AccessLogger.LOG_FORMAT`.
     :param bool debug: Switches debug mode. Default: ``False``.

tests/test_helpers.py

@@ -10,6 +10,7 @@
 from yarl import URL
 
 from aiohttp import helpers
+from aiohttp.abc import AbstractAccessLogger
 
 
 # -------------------- coro guard --------------------------------
@@ -256,6 +257,31 @@ def test_logger_no_transport():
     mock_logger.info.assert_called_with("-", extra={'remote_address': '-'})
 
 
+def test_logger_abc():
+    class Logger(AbstractAccessLogger):
+        def log(self, request, response, time):
+            1 / 0
+
+    mock_logger = mock.Mock()
+    access_logger = Logger(mock_logger, None)
+
+    with pytest.raises(ZeroDivisionError):
+        access_logger.log(None, None, None)
+
+    class Logger(AbstractAccessLogger):
+        def log(self, request, response, time):
+            self.logger.info(self.log_format.format(
+                request=request,
+                response=response,
+                time=time
+            ))
+
+    mock_logger = mock.Mock()
+    access_logger = Logger(mock_logger, '{request} {response} {time}')
+    access_logger.log('request', 'response', 1)
+    mock_logger.info.assert_called_with('request response 1')
+
+
 class TestReify:
 
     def test_reify(self):

tests/test_web_application.py

@@ -4,7 +4,7 @@
 import pytest
 
 from aiohttp import helpers, log, web
-from aiohttp.abc import AbstractRouter
+from aiohttp.abc import AbstractAccessLogger, AbstractRouter
 
 
 def test_app_ctor(loop):
@@ -65,6 +65,7 @@ def test_app_make_handler_debug_exc(loop, mocker, debug):
     app.make_handler(loop=loop)
     srv.assert_called_with(app._handle,
                            request_factory=app._make_request,
+                           access_log_class=mock.ANY,
                            loop=loop,
                            debug=debug)
 
@@ -76,9 +77,33 @@ def test_app_make_handler_args(loop, mocker):
     app.make_handler(loop=loop)
     srv.assert_called_with(app._handle,
                            request_factory=app._make_request,
+                           access_log_class=mock.ANY,
                            loop=loop, debug=mock.ANY, test=True)
 
 
+def test_app_make_handler_access_log_class(loop, mocker):
+    class Logger:
+        pass
+
+    app = web.Application()
+
+    with pytest.raises(TypeError):
+        app.make_handler(access_log_class=Logger, loop=loop)
+
+    class Logger(AbstractAccessLogger):
+
+        def log(self, request, response, time):
+            self.logger.info('msg')
+
+    srv = mocker.patch('aiohttp.web.Server')
+
+    app.make_handler(access_log_class=Logger, loop=loop)
+    srv.assert_called_with(app._handle,
+                           access_log_class=Logger,
+                           request_factory=app._make_request,
+                           loop=loop, debug=mock.ANY)
+
+
 @asyncio.coroutine
 def test_app_register_on_finish():
     app = web.Application()