Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve http handler performance #93324

Merged
merged 3 commits into from
May 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions homeassistant/components/emulated_hue/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,16 @@ async def async_setup(hass: HomeAssistant, yaml_config: ConfigType) -> bool:
app._on_startup.freeze()
await app.startup()

DescriptionXmlView(config).register(app, app.router)
HueUsernameView().register(app, app.router)
HueConfigView(config).register(app, app.router)
HueUnauthorizedUser().register(app, app.router)
HueAllLightsStateView(config).register(app, app.router)
HueOneLightStateView(config).register(app, app.router)
HueOneLightChangeView(config).register(app, app.router)
HueAllGroupsStateView(config).register(app, app.router)
HueGroupView(config).register(app, app.router)
HueFullStateView(config).register(app, app.router)
DescriptionXmlView(config).register(hass, app, app.router)
HueUsernameView().register(hass, app, app.router)
HueConfigView(config).register(hass, app, app.router)
HueUnauthorizedUser().register(hass, app, app.router)
HueAllLightsStateView(config).register(hass, app, app.router)
HueOneLightStateView(config).register(hass, app, app.router)
HueOneLightChangeView(config).register(hass, app, app.router)
HueAllGroupsStateView(config).register(hass, app, app.router)
HueGroupView(config).register(hass, app, app.router)
HueFullStateView(config).register(hass, app, app.router)
Comment on lines +141 to +150
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all legacy. This really should be move to a library.


async def _start(event: Event) -> None:
"""Start the bridge."""
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/http/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ def register_view(self, view: HomeAssistantView | type[HomeAssistantView]) -> No
class_name = view.__class__.__name__
raise AttributeError(f'{class_name} missing required attribute "name"')

view.register(self.app, self.app.router)
view.register(self.hass, self.app, self.app.router)

def register_redirect(
self,
Expand Down
59 changes: 31 additions & 28 deletions homeassistant/components/http/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@

from homeassistant import exceptions
from homeassistant.const import CONTENT_TYPE_JSON
from homeassistant.core import Context, is_callback
from homeassistant.core import Context, HomeAssistant, is_callback
from homeassistant.helpers.json import (
find_paths_unserializable_data,
json_bytes,
json_dumps,
)
from homeassistant.util.json import JSON_ENCODE_EXCEPTIONS, format_unserializable_data

from .const import KEY_AUTHENTICATED, KEY_HASS
from .const import KEY_AUTHENTICATED

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -88,7 +88,9 @@
data["code"] = message_code
return self.json(data, status_code, headers=headers)

def register(self, app: web.Application, router: web.UrlDispatcher) -> None:
def register(
self, hass: HomeAssistant, app: web.Application, router: web.UrlDispatcher
) -> None:
"""Register the view with a router."""
assert self.url is not None, "No url set for view"
urls = [self.url] + self.extra_urls
Expand All @@ -98,7 +100,7 @@
if not (handler := getattr(self, method, None)):
continue

handler = request_handler_factory(self, handler)
handler = request_handler_factory(hass, self, handler)

for url in urls:
routes.append(router.add_route(method, url, handler))
Expand All @@ -115,35 +117,37 @@


def request_handler_factory(
view: HomeAssistantView, handler: Callable
hass: HomeAssistant, view: HomeAssistantView, handler: Callable
) -> Callable[[web.Request], Awaitable[web.StreamResponse]]:
"""Wrap the handler classes."""
assert asyncio.iscoroutinefunction(handler) or is_callback(
is_coroutinefunction = asyncio.iscoroutinefunction(handler)
assert is_coroutinefunction or is_callback(
handler
), "Handler should be a coroutine or a callback."

async def handle(request: web.Request) -> web.StreamResponse:
"""Handle incoming request."""
if request.app[KEY_HASS].is_stopping:
if hass.is_stopping:
return web.Response(status=HTTPStatus.SERVICE_UNAVAILABLE)

authenticated = request.get(KEY_AUTHENTICATED, False)

if view.requires_auth and not authenticated:
raise HTTPUnauthorized()

_LOGGER.debug(
"Serving %s to %s (auth: %s)",
request.path,
request.remote,
authenticated,
)
if _LOGGER.isEnabledFor(logging.DEBUG):
_LOGGER.debug(
"Serving %s to %s (auth: %s)",
request.path,
request.remote,
authenticated,
)

try:
result = handler(request, **request.match_info)

if asyncio.iscoroutine(result):
result = await result
if is_coroutinefunction:
result = await handler(request, **request.match_info)
else:
result = handler(request, **request.match_info)
except vol.Invalid as err:
raise HTTPBadRequest() from err
except exceptions.ServiceNotFound as err:
Expand All @@ -156,21 +160,20 @@
return result

status_code = HTTPStatus.OK

if isinstance(result, tuple):
result, status_code = result

if isinstance(result, bytes):
bresult = result
elif isinstance(result, str):
bresult = result.encode("utf-8")
elif result is None:
bresult = b""
else:
raise TypeError(
f"Result should be None, string, bytes or StreamResponse. Got: {result}"
)
return web.Response(body=result, status=status_code)

if isinstance(result, str):
return web.Response(text=result, status=status_code)

return web.Response(body=bresult, status=status_code)
if result is None:
return web.Response(body=b"", status=status_code)

raise TypeError(

Check warning on line 175 in homeassistant/components/http/view.py

View check run for this annotation

Codecov / codecov/patch

homeassistant/components/http/view.py#L175

Added line #L175 was not covered by tests
f"Result should be None, string, bytes or StreamResponse. Got: {result}"
)

return handle
14 changes: 7 additions & 7 deletions tests/components/emulated_hue/test_hue_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,13 @@ def _mock_hue_endpoints(
web_app = hass.http.app
config = Config(hass, conf, "127.0.0.1")
config.numbers = entity_numbers
HueUsernameView().register(web_app, web_app.router)
HueAllLightsStateView(config).register(web_app, web_app.router)
HueOneLightStateView(config).register(web_app, web_app.router)
HueOneLightChangeView(config).register(web_app, web_app.router)
HueAllGroupsStateView(config).register(web_app, web_app.router)
HueFullStateView(config).register(web_app, web_app.router)
HueConfigView(config).register(web_app, web_app.router)
HueUsernameView().register(hass, web_app, web_app.router)
HueAllLightsStateView(config).register(hass, web_app, web_app.router)
HueOneLightStateView(config).register(hass, web_app, web_app.router)
HueOneLightChangeView(config).register(hass, web_app, web_app.router)
HueAllGroupsStateView(config).register(hass, web_app, web_app.router)
HueFullStateView(config).register(hass, web_app, web_app.router)
HueConfigView(config).register(hass, web_app, web_app.router)


@pytest.fixture
Expand Down
8 changes: 5 additions & 3 deletions tests/components/http/test_ban.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,15 @@ async def auth_handler(request):
return None, 200

app.router.add_get(
"/auth_true", request_handler_factory(Mock(requires_auth=True), auth_handler)
"/auth_true",
request_handler_factory(hass, Mock(requires_auth=True), auth_handler),
)
app.router.add_get(
"/auth_false", request_handler_factory(Mock(requires_auth=True), auth_handler)
"/auth_false",
request_handler_factory(hass, Mock(requires_auth=True), auth_handler),
)
app.router.add_get(
"/", request_handler_factory(Mock(requires_auth=False), auth_handler)
"/", request_handler_factory(hass, Mock(requires_auth=False), auth_handler)
)

setup_bans(hass, app, 5)
Expand Down
2 changes: 1 addition & 1 deletion tests/components/http/test_data_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async def post(self, request, data):
"""Test method."""
return b""

TestView().register(app, app.router)
TestView().register(app["hass"], app, app.router)
client = await aiohttp_client(app)
return client

Expand Down
25 changes: 16 additions & 9 deletions tests/components/http/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@


@pytest.fixture
def mock_request():
def mock_request() -> Mock:
"""Mock a request."""
return Mock(app={"hass": Mock(is_stopping=False)}, match_info={})


@pytest.fixture
def mock_request_with_stopping():
def mock_request_with_stopping() -> Mock:
"""Mock a request."""
return Mock(app={"hass": Mock(is_stopping=True)}, match_info={})

Expand All @@ -48,34 +48,41 @@ async def test_nan_serialized_to_null() -> None:
assert json.loads(response.body.decode("utf-8")) is None


async def test_handling_unauthorized(mock_request) -> None:
async def test_handling_unauthorized(mock_request: Mock) -> None:
"""Test handling unauth exceptions."""
with pytest.raises(HTTPUnauthorized):
await request_handler_factory(
Mock(requires_auth=False), AsyncMock(side_effect=Unauthorized)
mock_request.app["hass"],
Mock(requires_auth=False),
AsyncMock(side_effect=Unauthorized),
)(mock_request)


async def test_handling_invalid_data(mock_request) -> None:
async def test_handling_invalid_data(mock_request: Mock) -> None:
"""Test handling unauth exceptions."""
with pytest.raises(HTTPBadRequest):
await request_handler_factory(
Mock(requires_auth=False), AsyncMock(side_effect=vol.Invalid("yo"))
mock_request.app["hass"],
Mock(requires_auth=False),
AsyncMock(side_effect=vol.Invalid("yo")),
)(mock_request)


async def test_handling_service_not_found(mock_request) -> None:
async def test_handling_service_not_found(mock_request: Mock) -> None:
"""Test handling unauth exceptions."""
with pytest.raises(HTTPInternalServerError):
await request_handler_factory(
mock_request.app["hass"],
Mock(requires_auth=False),
AsyncMock(side_effect=ServiceNotFound("test", "test")),
)(mock_request)


async def test_not_running(mock_request_with_stopping) -> None:
async def test_not_running(mock_request_with_stopping: Mock) -> None:
"""Test we get a 503 when not running."""
response = await request_handler_factory(
Mock(requires_auth=False), AsyncMock(side_effect=Unauthorized)
mock_request_with_stopping.app["hass"],
Mock(requires_auth=False),
AsyncMock(side_effect=Unauthorized),
)(mock_request_with_stopping)
assert response.status == HTTPStatus.SERVICE_UNAVAILABLE