From 0dc0b4c56ff6ec2e16c2af315972b41f3f65f34c Mon Sep 17 00:00:00 2001 From: andrey-git Date: Wed, 24 Jan 2018 18:34:59 +0200 Subject: [PATCH 1/3] Add API to write error log --- homeassistant/components/api.py | 26 ++++++++++++++- homeassistant/const.py | 1 + tests/components/test_api.py | 59 ++++++++++++++++++++++++++++++++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/api.py b/homeassistant/components/api.py index f25b0cc130c7cd..0df7a2101cd9db 100644 --- a/homeassistant/components/api.py +++ b/homeassistant/components/api.py @@ -19,7 +19,7 @@ HTTP_BAD_REQUEST, HTTP_CREATED, HTTP_NOT_FOUND, MATCH_ALL, URL_API, URL_API_COMPONENTS, URL_API_CONFIG, URL_API_DISCOVERY_INFO, URL_API_ERROR_LOG, - URL_API_EVENTS, URL_API_SERVICES, + URL_API_EVENTS, URL_API_SERVICES, URL_API_WRITE_ERROR_LOG, URL_API_STATES, URL_API_STATES_ENTITY, URL_API_STREAM, URL_API_TEMPLATE, __version__) from homeassistant.exceptions import TemplateError @@ -51,6 +51,7 @@ def setup(hass, config): hass.http.register_view(APIDomainServicesView) hass.http.register_view(APIComponentsView) hass.http.register_view(APITemplateView) + hass.http.register_view(APIWriteErrorLogView) log_path = hass.data.get(DATA_LOGGING, None) if log_path: @@ -357,6 +358,29 @@ def post(self, request): HTTP_BAD_REQUEST) +class APIWriteErrorLogView(HomeAssistantView): + """View write errors to log.""" + + url = URL_API_WRITE_ERROR_LOG + name = 'api:write_error_log' + + @asyncio.coroutine + def post(self, request): + """Write to error log.""" + try: + data = yield from request.json() + except ValueError: + return self.json_message('Invalid JSON', HTTP_BAD_REQUEST) + if 'message' not in data: + return self.json_message('Missing message', HTTP_BAD_REQUEST) + + logger = logging.getLogger( + data.get('logger', '{}.external'.format(__name__))) + logger.error(data['message']) + + return self.json_message('ok') + + @asyncio.coroutine def async_services_json(hass): """Generate services data to JSONify.""" diff --git a/homeassistant/const.py b/homeassistant/const.py index 560c99bb653bca..8d5bcba6907d38 100644 --- a/homeassistant/const.py +++ b/homeassistant/const.py @@ -389,6 +389,7 @@ URL_API_ERROR_LOG = '/api/error_log' URL_API_LOG_OUT = '/api/log_out' URL_API_TEMPLATE = '/api/template' +URL_API_WRITE_ERROR_LOG = '/api/write_error_log' HTTP_OK = 200 HTTP_CREATED = 201 diff --git a/tests/components/test_api.py b/tests/components/test_api.py index 69b9bfa69decf0..c4b6a215740f45 100644 --- a/tests/components/test_api.py +++ b/tests/components/test_api.py @@ -2,9 +2,9 @@ # pylint: disable=protected-access import asyncio import json +from unittest.mock import patch, MagicMock import pytest - from homeassistant import const import homeassistant.core as ha from homeassistant.setup import async_setup_component @@ -374,6 +374,63 @@ def test_stream_with_restricted(hass, mock_api_client): assert data['event_type'] == 'test_event3' +@asyncio.coroutine +def test_write_error_log_logs_error(mock_api_client): + """Test that error propagates to logger.""" + logger = MagicMock() + with patch('logging.getLogger', return_value=logger): + resp = yield from mock_api_client.post( + const.URL_API_WRITE_ERROR_LOG, + json={"message": "test_message"}) + assert resp.status == 200 + body = yield from resp.json() + assert body.get('message') == 'ok' + assert logger.method_calls[0] == ('error', ('test_message',)) + + +@asyncio.coroutine +def test_write_error_log_logger_name(mock_api_client): + """Test that correct logger name is used.""" + with patch('logging.getLogger') as mock_logging: + resp = yield from mock_api_client.post( + const.URL_API_WRITE_ERROR_LOG, + json={"message": "test_message"}) + assert resp.status == 200 + body = yield from resp.json() + assert body.get('message') == 'ok' + mock_logging.assert_called_once_with( + 'homeassistant.components.api.external') + + with patch('logging.getLogger') as mock_logging: + resp = yield from mock_api_client.post( + const.URL_API_WRITE_ERROR_LOG, + json={"message": "test_message", "logger": "myLogger"}) + assert resp.status == 200 + body = yield from resp.json() + assert body.get('message') == 'ok' + mock_logging.assert_called_once_with('myLogger') + + +@asyncio.coroutine +def test_write_error_log_bad_json(mock_api_client): + """Test bad json returns 500.""" + resp = yield from mock_api_client.post(const.URL_API_WRITE_ERROR_LOG) + assert resp.status == 400 + body = yield from resp.json() + assert body.get('message') == 'Invalid JSON' + + +@asyncio.coroutine +def test_write_error_log_missing_message(mock_api_client): + """Test bad json returns 500.""" + resp = yield from mock_api_client.post( + const.URL_API_WRITE_ERROR_LOG, + json={}) + assert resp.status == 400 + body = yield from resp.json() + assert body.get('message') == 'Missing message' + + @asyncio.coroutine def _stream_next_event(stream): """Read the stream for next event while ignoring ping.""" From 979a7a272cac4b0196d83641887096e1a19b377b Mon Sep 17 00:00:00 2001 From: andrey-git Date: Fri, 26 Jan 2018 11:21:18 +0200 Subject: [PATCH 2/3] Move write_error api to system_log.write service call --- homeassistant/components/api.py | 26 +------- .../components/system_log/__init__.py | 23 ++++++- .../components/system_log/services.yaml | 12 ++++ homeassistant/const.py | 1 - tests/components/test_api.py | 58 ----------------- tests/components/test_system_log.py | 63 ++++++++++++++++--- 6 files changed, 87 insertions(+), 96 deletions(-) diff --git a/homeassistant/components/api.py b/homeassistant/components/api.py index 0df7a2101cd9db..f25b0cc130c7cd 100644 --- a/homeassistant/components/api.py +++ b/homeassistant/components/api.py @@ -19,7 +19,7 @@ HTTP_BAD_REQUEST, HTTP_CREATED, HTTP_NOT_FOUND, MATCH_ALL, URL_API, URL_API_COMPONENTS, URL_API_CONFIG, URL_API_DISCOVERY_INFO, URL_API_ERROR_LOG, - URL_API_EVENTS, URL_API_SERVICES, URL_API_WRITE_ERROR_LOG, + URL_API_EVENTS, URL_API_SERVICES, URL_API_STATES, URL_API_STATES_ENTITY, URL_API_STREAM, URL_API_TEMPLATE, __version__) from homeassistant.exceptions import TemplateError @@ -51,7 +51,6 @@ def setup(hass, config): hass.http.register_view(APIDomainServicesView) hass.http.register_view(APIComponentsView) hass.http.register_view(APITemplateView) - hass.http.register_view(APIWriteErrorLogView) log_path = hass.data.get(DATA_LOGGING, None) if log_path: @@ -358,29 +357,6 @@ def post(self, request): HTTP_BAD_REQUEST) -class APIWriteErrorLogView(HomeAssistantView): - """View write errors to log.""" - - url = URL_API_WRITE_ERROR_LOG - name = 'api:write_error_log' - - @asyncio.coroutine - def post(self, request): - """Write to error log.""" - try: - data = yield from request.json() - except ValueError: - return self.json_message('Invalid JSON', HTTP_BAD_REQUEST) - if 'message' not in data: - return self.json_message('Missing message', HTTP_BAD_REQUEST) - - logger = logging.getLogger( - data.get('logger', '{}.external'.format(__name__))) - logger.error(data['message']) - - return self.json_message('ok') - - @asyncio.coroutine def async_services_json(hass): """Generate services data to JSONify.""" diff --git a/homeassistant/components/system_log/__init__.py b/homeassistant/components/system_log/__init__.py index 0d478ac9316f94..5c8fe3109a6585 100644 --- a/homeassistant/components/system_log/__init__.py +++ b/homeassistant/components/system_log/__init__.py @@ -18,6 +18,9 @@ import homeassistant.helpers.config_validation as cv CONF_MAX_ENTRIES = 'max_entries' +CONF_MESSAGE = 'message' +CONF_LEVEL = 'level' +CONF_LOGGER = 'logger' DATA_SYSTEM_LOG = 'system_log' DEFAULT_MAX_ENTRIES = 50 @@ -25,6 +28,7 @@ DOMAIN = 'system_log' SERVICE_CLEAR = 'clear' +SERVICE_WRITE = 'write' CONFIG_SCHEMA = vol.Schema({ DOMAIN: vol.Schema({ @@ -34,6 +38,12 @@ }, extra=vol.ALLOW_EXTRA) SERVICE_CLEAR_SCHEMA = vol.Schema({}) +SERVICE_WRITE_SCHEMA = vol.Schema({ + vol.Required(CONF_MESSAGE): cv.string, + vol.Optional(CONF_LEVEL, default='error'): + vol.In(['debug', 'info', 'warning', 'error', 'critical']), + vol.Optional(CONF_LOGGER): cv.string, +}) class LogErrorHandler(logging.Handler): @@ -78,12 +88,21 @@ def async_setup(hass, config): @asyncio.coroutine def async_service_handler(service): """Handle logger services.""" - # Only one service so far - handler.records.clear() + if service.service == 'clear': + handler.records.clear() + return + if service.service == 'write': + logger = logging.getLogger( + service.data.get(CONF_LOGGER, '{}.external'.format(__name__))) + level = service.data[CONF_LEVEL] + getattr(logger, level)(service.data[CONF_MESSAGE]) hass.services.async_register( DOMAIN, SERVICE_CLEAR, async_service_handler, schema=SERVICE_CLEAR_SCHEMA) + hass.services.async_register( + DOMAIN, SERVICE_WRITE, async_service_handler, + schema=SERVICE_WRITE_SCHEMA) return True diff --git a/homeassistant/components/system_log/services.yaml b/homeassistant/components/system_log/services.yaml index 98f86e12f8ce27..c168185c9b3c38 100644 --- a/homeassistant/components/system_log/services.yaml +++ b/homeassistant/components/system_log/services.yaml @@ -1,3 +1,15 @@ system_log: clear: description: Clear all log entries. + write: + description: Write log entry. + fields: + message: + description: Message to log. [Required] + example: Something went wrong + level: + description: "Log level: debug, info, warning, error, critical. Defaults to 'error'." + example: debug + logger: + description: Logger name under which to log the message. Defaults to 'system_log.external'. + example: mycomponent.myplatform diff --git a/homeassistant/const.py b/homeassistant/const.py index 8d5bcba6907d38..560c99bb653bca 100644 --- a/homeassistant/const.py +++ b/homeassistant/const.py @@ -389,7 +389,6 @@ URL_API_ERROR_LOG = '/api/error_log' URL_API_LOG_OUT = '/api/log_out' URL_API_TEMPLATE = '/api/template' -URL_API_WRITE_ERROR_LOG = '/api/write_error_log' HTTP_OK = 200 HTTP_CREATED = 201 diff --git a/tests/components/test_api.py b/tests/components/test_api.py index c4b6a215740f45..8f02cab5afa13f 100644 --- a/tests/components/test_api.py +++ b/tests/components/test_api.py @@ -2,7 +2,6 @@ # pylint: disable=protected-access import asyncio import json -from unittest.mock import patch, MagicMock import pytest from homeassistant import const @@ -374,63 +373,6 @@ def test_stream_with_restricted(hass, mock_api_client): assert data['event_type'] == 'test_event3' -@asyncio.coroutine -def test_write_error_log_logs_error(mock_api_client): - """Test that error propagates to logger.""" - logger = MagicMock() - with patch('logging.getLogger', return_value=logger): - resp = yield from mock_api_client.post( - const.URL_API_WRITE_ERROR_LOG, - json={"message": "test_message"}) - assert resp.status == 200 - body = yield from resp.json() - assert body.get('message') == 'ok' - assert logger.method_calls[0] == ('error', ('test_message',)) - - -@asyncio.coroutine -def test_write_error_log_logger_name(mock_api_client): - """Test that correct logger name is used.""" - with patch('logging.getLogger') as mock_logging: - resp = yield from mock_api_client.post( - const.URL_API_WRITE_ERROR_LOG, - json={"message": "test_message"}) - assert resp.status == 200 - body = yield from resp.json() - assert body.get('message') == 'ok' - mock_logging.assert_called_once_with( - 'homeassistant.components.api.external') - - with patch('logging.getLogger') as mock_logging: - resp = yield from mock_api_client.post( - const.URL_API_WRITE_ERROR_LOG, - json={"message": "test_message", "logger": "myLogger"}) - assert resp.status == 200 - body = yield from resp.json() - assert body.get('message') == 'ok' - mock_logging.assert_called_once_with('myLogger') - - -@asyncio.coroutine -def test_write_error_log_bad_json(mock_api_client): - """Test bad json returns 500.""" - resp = yield from mock_api_client.post(const.URL_API_WRITE_ERROR_LOG) - assert resp.status == 400 - body = yield from resp.json() - assert body.get('message') == 'Invalid JSON' - - -@asyncio.coroutine -def test_write_error_log_missing_message(mock_api_client): - """Test bad json returns 500.""" - resp = yield from mock_api_client.post( - const.URL_API_WRITE_ERROR_LOG, - json={}) - assert resp.status == 400 - body = yield from resp.json() - assert body.get('message') == 'Missing message' - - @asyncio.coroutine def _stream_next_event(stream): """Read the stream for next event while ignoring ping.""" diff --git a/tests/components/test_system_log.py b/tests/components/test_system_log.py index 0f61986cf474f6..a3e7d662483cde 100644 --- a/tests/components/test_system_log.py +++ b/tests/components/test_system_log.py @@ -1,11 +1,12 @@ """Test system log component.""" import asyncio import logging +from unittest.mock import MagicMock, patch + import pytest from homeassistant.bootstrap import async_setup_component from homeassistant.components import system_log -from unittest.mock import MagicMock, patch _LOGGER = logging.getLogger('test_logger') @@ -117,11 +118,54 @@ def test_clear_logs(hass, test_client): yield from get_error_log(hass, test_client, 0) +@asyncio.coroutine +def test_write_log(hass): + """Test that error propagates to logger.""" + logger = MagicMock() + with patch('logging.getLogger', return_value=logger) as mock_logging: + hass.async_add_job( + hass.services.async_call( + system_log.DOMAIN, system_log.SERVICE_WRITE, + {'message': 'test_message'})) + yield from hass.async_block_till_done() + mock_logging.assert_called_once_with( + 'homeassistant.components.system_log.external') + assert logger.method_calls[0] == ('error', ('test_message',)) + + +@asyncio.coroutine +def test_write_choose_logger(hass): + """Test that correct logger is chosen.""" + with patch('logging.getLogger') as mock_logging: + hass.async_add_job( + hass.services.async_call( + system_log.DOMAIN, system_log.SERVICE_WRITE, + {'message': 'test_message', + 'logger': 'myLogger'})) + yield from hass.async_block_till_done() + mock_logging.assert_called_once_with( + 'myLogger') + + +@asyncio.coroutine +def test_write_choose_level(hass): + """Test that correct logger is chosen.""" + logger = MagicMock() + with patch('logging.getLogger', return_value=logger): + hass.async_add_job( + hass.services.async_call( + system_log.DOMAIN, system_log.SERVICE_WRITE, + {'message': 'test_message', + 'level': 'debug'})) + yield from hass.async_block_till_done() + assert logger.method_calls[0] == ('debug', ('test_message',)) + + @asyncio.coroutine def test_unknown_path(hass, test_client): """Test error logged from unknown path.""" _LOGGER.findCaller = MagicMock( - return_value=('unknown_path', 0, None, None)) + return_value=('unknown_path', 0, None, None)) _LOGGER.error('error message') log = (yield from get_error_log(hass, test_client, 1))[0] assert log['source'] == 'unknown_path' @@ -130,16 +174,15 @@ def test_unknown_path(hass, test_client): def log_error_from_test_path(path): """Log error while mocking the path.""" call_path = 'internal_path.py' - with patch.object( - _LOGGER, - 'findCaller', - MagicMock(return_value=(call_path, 0, None, None))): + with patch.object(_LOGGER, + 'findCaller', + MagicMock(return_value=(call_path, 0, None, None))): with patch('traceback.extract_stack', MagicMock(return_value=[ - get_frame('main_path/main.py'), - get_frame(path), - get_frame(call_path), - get_frame('venv_path/logging/log.py')])): + get_frame('main_path/main.py'), + get_frame(path), + get_frame(call_path), + get_frame('venv_path/logging/log.py')])): _LOGGER.error('error message') From 92129ea093a8a7034eb27fee92dcda453f4757b4 Mon Sep 17 00:00:00 2001 From: andrey-git Date: Fri, 26 Jan 2018 11:22:24 +0200 Subject: [PATCH 3/3] Restore empty line --- tests/components/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/components/test_api.py b/tests/components/test_api.py index 8f02cab5afa13f..69b9bfa69decf0 100644 --- a/tests/components/test_api.py +++ b/tests/components/test_api.py @@ -4,6 +4,7 @@ import json import pytest + from homeassistant import const import homeassistant.core as ha from homeassistant.setup import async_setup_component