From e11c82a69f26694564fa94eb0f976bb621328e08 Mon Sep 17 00:00:00 2001 From: Victor K Date: Fri, 13 Oct 2017 17:39:57 +0300 Subject: [PATCH] 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 | 11 +++++++++++ aiohttp/helpers.py | 13 +++---------- aiohttp/web.py | 16 ++++++++++++++-- aiohttp/web_protocol.py | 6 +++++- changes/2315.feature | 1 + docs/abc.rst | 24 +++++++++++++++++++++--- docs/logging.rst | 14 ++++++++++++++ docs/web_reference.rst | 3 +++ tests/test_helpers.py | 26 ++++++++++++++++++++++++++ tests/test_web_application.py | 27 ++++++++++++++++++++++++++- 10 files changed, 124 insertions(+), 17 deletions(-) create mode 100644 changes/2315.feature diff --git a/aiohttp/abc.py b/aiohttp/abc.py index 0d2854e66ae..3ca9a5d5b5c 100644 --- a/aiohttp/abc.py +++ b/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""" diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index bbfb82b238f..a672f0183b4 100644 --- a/aiohttp/helpers.py +++ b/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) diff --git a/aiohttp/web.py b/aiohttp/web.py index 199f4d59818..4b5a1de2ff6 100644 --- a/aiohttp/web.py +++ b/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 diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 8bf9149aa00..1bb3eaa3a84 100644 --- a/aiohttp/web_protocol.py +++ b/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 diff --git a/changes/2315.feature b/changes/2315.feature new file mode 100644 index 00000000000..6d39a01e0ce --- /dev/null +++ b/changes/2315.feature @@ -0,0 +1 @@ +aiohttp.wep.Application.make_handler support access_log_class diff --git a/docs/abc.rst b/docs/abc.rst index 4faf7e3f2c4..057d609e268 100644 --- a/docs/abc.rst +++ b/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. diff --git a/docs/logging.rst b/docs/logging.rst index 7695676ebb1..d9fc51155d5 100644 --- a/docs/logging.rst +++ b/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:: diff --git a/docs/web_reference.rst b/docs/web_reference.rst index ece01e3a56a..10b91894b01 100644 --- a/docs/web_reference.rst +++ b/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``. diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c68b317a996..3d94703d605 100644 --- a/tests/test_helpers.py +++ b/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): diff --git a/tests/test_web_application.py b/tests/test_web_application.py index 9e141207702..2805b2babdd 100644 --- a/tests/test_web_application.py +++ b/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()