Skip to content

Commit 7949e9f

Browse files
committed
use CallResultHandlerDecorator for webhook response future
Address PR review feedback: instead of adding a new result_future field to DispatchData, route the response future through the existing CallResultHandlerDecorator pipeline.
1 parent 321aa5a commit 7949e9f

4 files changed

Lines changed: 24 additions & 73 deletions

File tree

custom_components/pyscript/decorator.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ async def _call(self, data: DispatchData) -> None:
264264
# notify handlers with "None"
265265
for result_handler_dec in result_handlers:
266266
await result_handler_dec.handle_call_result(data, None)
267-
data.set_result(None)
268267
return
269268
# Fire an event indicating that pyscript is running
270269
# Note: the event must have an entity_id for logbook to work correctly.
@@ -280,9 +279,9 @@ async def _call(self, data: DispatchData) -> None:
280279
result = await data.call_ast_ctx.call_func(self.eval_func, None, **data.func_args)
281280
for result_handler_dec in result_handlers:
282281
await result_handler_dec.handle_call_result(data, result)
283-
data.set_result(result)
284282
except Exception as e:
285-
data.set_result(None)
283+
for result_handler_dec in result_handlers:
284+
await result_handler_dec.handle_call_result(data, None)
286285
await self.handle_exception(e)
287286

288287
async def dispatch(self, data: DispatchData) -> None:
@@ -293,7 +292,8 @@ async def dispatch(self, data: DispatchData) -> None:
293292
for dec in decorators:
294293
if await dec.handle_dispatch(data) is False:
295294
self.logger.debug("Trigger not active due to %s", dec)
296-
data.set_result(None)
295+
for result_handler_dec in self.get_decorators(CallResultHandlerDecorator):
296+
await result_handler_dec.handle_call_result(data, None)
297297
return
298298

299299
action_ast_ctx = AstEval(

custom_components/pyscript/decorator_abc.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import annotations
44

55
from abc import ABC, abstractmethod
6-
import asyncio
76
from dataclasses import dataclass, field
87
from enum import StrEnum
98
import logging
@@ -47,16 +46,6 @@ class DispatchData:
4746
call_ast_ctx: AstEval | None = field(default=None, kw_only=True)
4847
hass_context: Context | None = field(default=None, kw_only=True)
4948

50-
# When set, the dispatch pipeline resolves this future with the
51-
# decorated function's return value. Resolved with None if the
52-
# function is skipped (guard rejection) or raises.
53-
result_future: asyncio.Future[Any] | None = field(default=None, kw_only=True)
54-
55-
def set_result(self, value: Any) -> None:
56-
"""Resolve result_future with value if it is still pending."""
57-
if self.result_future is not None and not self.result_future.done():
58-
self.result_future.set_result(value)
59-
6049

6150
class Decorator(ABC):
6251
"""Generic decorator abstraction."""

custom_components/pyscript/decorators/webhook.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313
from homeassistant.components.webhook import SUPPORTED_METHODS
1414
from homeassistant.helpers import config_validation as cv
1515

16-
from ..decorator_abc import DispatchData, TriggerDecorator
16+
from ..decorator_abc import CallResultHandlerDecorator, DispatchData, TriggerDecorator
1717
from .base import AutoKwargsDecorator, ExpressionDecorator
1818

1919
_LOGGER = logging.getLogger(__name__)
2020

2121

22-
class WebhookTriggerDecorator(TriggerDecorator, ExpressionDecorator, AutoKwargsDecorator):
22+
class WebhookTriggerDecorator(
23+
TriggerDecorator, ExpressionDecorator, AutoKwargsDecorator, CallResultHandlerDecorator
24+
):
2325
"""Implementation for @webhook_trigger."""
2426

2527
name = "webhook_trigger"
@@ -41,6 +43,7 @@ class WebhookTriggerDecorator(TriggerDecorator, ExpressionDecorator, AutoKwargsD
4143
local_only: bool
4244
methods: set[str]
4345
sets_http_response_code: bool
46+
future: asyncio.Future[Any] | None = None
4447

4548
webhook_id2triggers: ClassVar[dict[str, set[WebhookTriggerDecorator]]] = {}
4649

@@ -75,10 +78,11 @@ async def _handler(hass, webhook_id, request):
7578
if not await trigger.check_expression_vars(trigger_args):
7679
continue
7780
future: asyncio.Future[Any] = hass.loop.create_future()
81+
trigger.future = future
7882
if trigger.sets_http_response_code:
7983
response_future = future
8084
futures.append(future)
81-
await trigger.dispatch(DispatchData(trigger_args, result_future=future))
85+
await trigger.dispatch(DispatchData(trigger_args))
8286

8387
if not futures:
8488
return None
@@ -89,6 +93,14 @@ async def _handler(hass, webhook_id, request):
8993
return None
9094
return WebhookTriggerDecorator.coerce_response(response_future.result())
9195

96+
async def handle_call_result(self, data: DispatchData, result: Any) -> None:
97+
"""Resolve the response future with the decorated function's return value."""
98+
if data.trigger is not self:
99+
return
100+
response_future = self.future
101+
if response_future is not None and not response_future.done():
102+
response_future.set_result(result)
103+
92104
@staticmethod
93105
def coerce_response(value: Any) -> web.Response | None:
94106
"""Convert a webhook function return value to an aiohttp Response."""

tests/test_decorator_manager.py

Lines changed: 5 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from __future__ import annotations
44

5-
import asyncio
65
import logging
76
from typing import ClassVar
87
from unittest.mock import patch
@@ -354,14 +353,12 @@ def make_dispatch_data(
354353
*,
355354
call_ast_ctx: DummyCallAstCtx | None = None,
356355
hass_context: Context | None = None,
357-
result_future: asyncio.Future | None = None,
358356
) -> DispatchData:
359357
"""Build DispatchData from test doubles."""
360358
return DispatchData(
361359
func_args,
362360
call_ast_ctx=call_ast_ctx,
363361
hass_context=hass_context,
364-
result_future=result_future,
365362
)
366363

367364

@@ -607,59 +604,14 @@ async def test_function_decorator_manager_logs_call_exception(hass):
607604

608605

609606
@pytest.mark.asyncio
610-
async def test_function_decorator_manager_result_future_success(hass):
611-
"""Successful calls should resolve result_future with the function's return value."""
612-
DecoratorManager.hass = hass
613-
manager = FunctionDecoratorManager(DummyAstCtx(), DummyEvalFuncVar())
614-
call_ast_ctx = DummyCallAstCtx(result=201)
615-
future: asyncio.Future = hass.loop.create_future()
616-
617-
with patch.object(Function, "store_hass_context"):
618-
await call_function_manager(
619-
manager,
620-
make_dispatch_data(
621-
{"arg1": 1},
622-
call_ast_ctx=call_ast_ctx,
623-
hass_context=Context(id="call-parent"),
624-
result_future=future,
625-
),
626-
)
627-
await hass.async_block_till_done()
628-
629-
assert future.done()
630-
assert future.result() == 201
631-
632-
633-
@pytest.mark.asyncio
634-
async def test_function_decorator_manager_result_future_cancel(hass):
635-
"""When a call handler cancels, result_future should resolve to None."""
636-
DecoratorManager.hass = hass
637-
manager = FunctionDecoratorManager(DummyAstCtx(), DummyEvalFuncVar())
638-
manager.add(make_cancel_call_handler())
639-
future: asyncio.Future = hass.loop.create_future()
640-
641-
await call_function_manager(
642-
manager,
643-
make_dispatch_data(
644-
{"arg1": 1},
645-
call_ast_ctx=DummyCallAstCtx(result="unused"),
646-
hass_context=Context(id="call-parent"),
647-
result_future=future,
648-
),
649-
)
650-
651-
assert future.done()
652-
assert future.result() is None
653-
654-
655-
@pytest.mark.asyncio
656-
async def test_function_decorator_manager_result_future_exception(hass):
657-
"""When the decorated function raises, result_future should resolve to None."""
607+
async def test_function_decorator_manager_exception_calls_result_handlers(hass):
608+
"""When the decorated function raises, result handlers should be notified with None."""
658609
DecoratorManager.hass = hass
659610
ast_ctx = DummyAstCtx()
660611
manager = FunctionDecoratorManager(ast_ctx, DummyEvalFuncVar())
612+
result_handler = make_recording_result_handler()
613+
manager.add(result_handler)
661614
call_ast_ctx = DummyCallAstCtx(exc=RuntimeError("boom"))
662-
future: asyncio.Future = hass.loop.create_future()
663615

664616
with patch.object(Function, "store_hass_context"):
665617
await call_function_manager(
@@ -668,12 +620,10 @@ async def test_function_decorator_manager_result_future_exception(hass):
668620
{"arg1": 1},
669621
call_ast_ctx=call_ast_ctx,
670622
hass_context=Context(id="call-parent"),
671-
result_future=future,
672623
),
673624
)
674625

675-
assert future.done()
676-
assert future.result() is None
626+
assert result_handler.results == [None]
677627
assert len(ast_ctx.logged_exceptions) == 1
678628

679629

0 commit comments

Comments
 (0)