From 09249cfd22479ef9b478250cd6adcbaec6ea94d0 Mon Sep 17 00:00:00 2001 From: Guillaume Fieni Date: Thu, 15 Oct 2026 12:52:51 +0200 Subject: [PATCH 1/2] refactor(actor)!: Resolve and cache handlers by message type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve handlers through the message class’s MRO, giving specific handlers precedence over catch-all handlers regardless of registration order. Support later-loaded and deeper subclasses, and use class keys to avoid name collisions. Cache successful resolutions and clear the cache on every registration. BREAKING CHANGE: Remove `include_subclasses` argument to `State.add_handler` method. Inheritance matching is unconditional. `State.handlers` now uses class objects instead of class names as keys. --- src/powerapi/actor/state.py | 42 ++++-- tests/unit/actor/test_actor.py | 77 +---------- tests/unit/actor/test_state.py | 237 +++++++++++++++++++++++++++++++++ 3 files changed, 271 insertions(+), 85 deletions(-) create mode 100644 tests/unit/actor/test_state.py diff --git a/src/powerapi/actor/state.py b/src/powerapi/actor/state.py index 38192923..2ea9c89f 100644 --- a/src/powerapi/actor/state.py +++ b/src/powerapi/actor/state.py @@ -54,9 +54,11 @@ def __init__(self, actor): self.initialized = False self.alive = True - self.handlers: dict[str, Handler] = {} + self.handlers: dict[type[Message], Handler] = {} self.supervisor = Supervisor() + self._resolved_handlers: dict[type[Message], Handler] = {} + def initialize(self) -> None: """ Initialize resources after the actor receives a start message. @@ -69,25 +71,43 @@ def teardown(self) -> None: def get_corresponding_handler(self, msg: Message) -> Handler: """ - Return the corresponding handler for the given message type. + Return the most specific registered handler, caching successful lookups. :param msg: The message :return: The handler for the given message type :raises KeyError: If the message type does not have a corresponding handler """ - return self.handlers[msg.__class__.__name__] + message_type = type(msg) + try: + return self._resolved_handlers[message_type] + except KeyError: + return self._resolve_handler(message_type) + + def _resolve_handler(self, message_type: type[Message]) -> Handler: + """ + Resolve a handler in method resolution order and cache the successful lookup. + :param message_type: The message class to resolve + :return: The most specific registered handler + :raises KeyError: If no handler is registered for the class or its bases + """ + for base in message_type.__mro__: + handler = self.handlers.get(base) + if handler is not None: + self._resolved_handlers[message_type] = handler + return handler - def add_handler(self, message_type: type[Message], handler: Handler, include_subclasses: bool = True) -> None: + raise KeyError(message_type) + + def add_handler(self, message_type: type[Message], handler: Handler) -> None: """ - Add a handler for the given message type. + Register a handler for a message class and its subclasses. + Exact registrations take precedence over base classes in method resolution order. + Registering the same class again replaces its handler. + Every registration clears the handler resolution cache. :param message_type: The message type :param handler: The corresponding handler - :param include_subclasses: Whether to include subclasses of the message type """ - self.handlers[message_type.__name__] = handler - - if include_subclasses: - for child_type in message_type.__subclasses__(): - self.handlers[child_type.__name__] = handler + self.handlers[message_type] = handler + self._resolved_handlers.clear() def dispatch_message(self, msg: Message) -> None: """ diff --git a/tests/unit/actor/test_actor.py b/tests/unit/actor/test_actor.py index 61fd67c1..60f1fb8e 100644 --- a/tests/unit/actor/test_actor.py +++ b/tests/unit/actor/test_actor.py @@ -68,12 +68,6 @@ def __eq__(self, other): return self.value == other.value -class DummyMessageSubtype(DummyMessage): - """ - Subtype of a dummy message class. - """ - - class FailingStartMessageHandler(Handler[StartMessage]): """ Failing start message handler. @@ -96,17 +90,12 @@ class LoopbackMessageHandler(Handler[DummyMessage]): Set the processed flag to received messages and sends them back to the control channel. """ - def handle(self, message: DummyMessage) -> None: + def handle(self, msg: DummyMessage) -> None: """ Handle a message. """ - message.processed = True - self.state.actor.send_control(message) - - -class KeyErrorMessageHandler(Handler[DummyMessage]): - def handle(self, message: DummyMessage) -> None: - raise KeyError('handler failure') + msg.processed = True + self.state.actor.send_control(msg) class DummyActorState(State): @@ -232,66 +221,6 @@ def test_send_data_message_to_actor(started_loopback_actor): assert ack_msg.processed is True -def test_retrieve_handler_for_known_message_type(): - """ - Test retrieving the corresponding handler for a known message type. - """ - state = DummyActorState(None) - dummy_handler = LoopbackMessageHandler(state) - state.add_handler(DummyMessage, dummy_handler) - - msg_handler = state.get_corresponding_handler(DummyMessage('test-dummy')) - assert msg_handler is dummy_handler - - -def test_retrieve_handler_for_known_message_subtype(): - """ - Test retrieving the corresponding handler for a subtype of a known message type. - """ - state = DummyActorState(None) - dummy_handler = LoopbackMessageHandler(state) - state.add_handler(DummyMessage, dummy_handler, include_subclasses=True) - - msg_handler = state.get_corresponding_handler(DummyMessageSubtype('test-dummy-subtype')) - assert msg_handler is dummy_handler - - -def test_retrieve_handler_for_known_message_with_subtype_excluded(): - """ - Test that retrieving the corresponding handler for an excluded subtype of a known message type raises an error. - """ - state = DummyActorState(None) - dummy_handler = LoopbackMessageHandler(state) - state.add_handler(DummyMessage, dummy_handler, include_subclasses=False) - - with pytest.raises(KeyError, match='DummyMessageSubtype'): - state.get_corresponding_handler(DummyMessageSubtype('test-dummy-subtype')) - - -def test_retrieve_handler_for_unknown_message_type(): - """ - Test that retrieving the corresponding handler for an unknown message type raises an error. - """ - state = DummyActorState(None) - - with pytest.raises(KeyError, match='DummyMessage'): - state.get_corresponding_handler(DummyMessage('test-dummy')) - - -def test_handler_key_error_is_not_mistaken_for_an_unknown_message(): - """ - Exceptions raised by handlers should not be treated as unknown messages. - """ - actor = Actor('pytest') - state = DummyActorState(actor) - actor.state = state - handler = KeyErrorMessageHandler(state) - state.add_handler(DummyMessage, handler) - - with pytest.raises(KeyError, match='handler failure'): - state.dispatch_message(DummyMessage('test-dummy')) - - def test_start_message_handler_initializes_state(): """ The start-message handler should initialize the actor state. diff --git a/tests/unit/actor/test_state.py b/tests/unit/actor/test_state.py new file mode 100644 index 00000000..04d71e61 --- /dev/null +++ b/tests/unit/actor/test_state.py @@ -0,0 +1,237 @@ +# Copyright (c) 2026, Inria +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from datetime import UTC, datetime +from unittest.mock import Mock + +import pytest + +from powerapi.actor import Handler, Message, State +from powerapi.report import HWPCReport, PowerReport, Report +from .test_actor import DummyMessage, LoopbackMessageHandler + + +class DummyMessageSubtype(DummyMessage): + """ + Subtype of a dummy message class. + """ + + +class KeyErrorMessageHandler(Handler[DummyMessage]): + """ + Handler that always raises KeyError. + """ + def handle(self, msg: DummyMessage) -> None: + raise KeyError('handler failure') + + +def test_retrieve_handler_for_known_message_type(): + """ + Test retrieving the corresponding handler for a known message type. + """ + state = State(None) + dummy_handler = LoopbackMessageHandler(state) + state.add_handler(DummyMessage, dummy_handler) + + msg_handler = state.get_corresponding_handler(DummyMessage('test-dummy')) + assert msg_handler is dummy_handler + + +def test_retrieve_handler_for_known_message_subtype(): + """ + Test retrieving the corresponding handler for a subtype of a known message type. + """ + state = State(None) + dummy_handler = LoopbackMessageHandler(state) + state.add_handler(DummyMessage, dummy_handler) + + msg_handler = state.get_corresponding_handler(DummyMessageSubtype('test-dummy-subtype')) + assert msg_handler is dummy_handler + + +def test_retrieve_handler_for_message_subtype_defined_after_registration(): + """ + Test retrieving a handler for a subtype defined after its base handler is registered. + """ + state = State(None) + handler = Mock() + state.add_handler(DummyMessage, handler) + + class LateMessage(DummyMessage): + pass + + assert state.get_corresponding_handler(LateMessage('late')) is handler + + +def test_retrieve_handler_for_deep_subtype_with_only_ancestor_registered(): + """ + Test retrieving a handler through multiple unregistered subclass levels. + """ + class DeepMessage(DummyMessageSubtype): + pass + + state = State(None) + handler = Mock() + state.add_handler(DummyMessage, handler) + + assert state.get_corresponding_handler(DeepMessage('deep')) is handler + + +def test_retrieve_handler_for_deep_subtype_uses_nearest_registered_base(): + """ + Test that a deeper subtype uses the handler of its nearest registered base class. + """ + class DeepMessage(DummyMessageSubtype): + pass + + state = State(None) + subtype_handler = Mock() + state.add_handler(DummyMessageSubtype, subtype_handler) + state.add_handler(DummyMessage, Mock()) + + assert state.get_corresponding_handler(DeepMessage('deep')) is subtype_handler + + +@pytest.mark.parametrize('registration_order', [(Report, HWPCReport), (HWPCReport, Report)]) +def test_specific_report_handler_precedes_catch_all(registration_order: tuple[type[Report], type[Report]]): + """ + Test that a specific report handler takes precedence regardless of registration order. + """ + state = State(None) + catch_all_handler = Mock() + hwpc_handler = Mock() + handlers = {Report: catch_all_handler, HWPCReport: hwpc_handler} + for report_type in registration_order: + state.add_handler(report_type, handlers[report_type]) + + timestamp = datetime.now(UTC) + hwpc_report = HWPCReport(timestamp, 'sensor', 'target', {}) + power_report = PowerReport(timestamp, 'sensor', 'target', 10.0) + + assert state.get_corresponding_handler(hwpc_report) is hwpc_handler + assert state.get_corresponding_handler(power_report) is catch_all_handler + + +def test_handler_resolution_follows_multiple_inheritance_order(): + """ + Test that multiple inheritance uses Python's method resolution order. + """ + class LeftMessage(Message): + pass + + class RightMessage(Message): + pass + + class CombinedMessage(LeftMessage, RightMessage): + pass + + state = State(None) + left_handler = Mock() + state.add_handler(RightMessage, Mock()) + state.add_handler(LeftMessage, left_handler) + + assert state.get_corresponding_handler(CombinedMessage()) is left_handler + + +def test_handler_resolution_is_cached(): + """ + Test that repeated messages of the same type reuse the resolved handler. + """ + state = State(None) + handler = Mock() + state.add_handler(DummyMessage, handler) + state._resolve_handler = Mock(wraps=state._resolve_handler) + + assert state.get_corresponding_handler(DummyMessageSubtype('first')) is handler + assert state.get_corresponding_handler(DummyMessageSubtype('second')) is handler + state._resolve_handler.assert_called_once_with(DummyMessageSubtype) + + +@pytest.mark.parametrize(('message_type', 'registration_type'), [ + (DummyMessage, DummyMessage), + (DummyMessageSubtype, DummyMessage), + (DummyMessageSubtype, DummyMessageSubtype), +]) +def test_registration_invalidates_cached_handler(message_type, registration_type): + """ + Test that adding or replacing a registration invalidates previously resolved handlers. + """ + state = State(None) + original_handler = Mock() + replacement_handler = Mock() + state.add_handler(DummyMessage, original_handler) + message = message_type('message') + assert state.get_corresponding_handler(message) is original_handler + + state.add_handler(registration_type, replacement_handler) + + assert state.get_corresponding_handler(message) is replacement_handler + + +def test_unrelated_message_classes_with_same_name_have_separate_handlers(): + """ + Test that unrelated message classes with the same name retain separate handlers. + """ + def make_message_type() -> type[Message]: + class SameNameMessage(Message): + pass + + return SameNameMessage + + first_type = make_message_type() + second_type = make_message_type() + state = State(None) + first_handler = Mock() + second_handler = Mock() + state.add_handler(first_type, first_handler) + state.add_handler(second_type, second_handler) + + assert state.get_corresponding_handler(first_type()) is first_handler + assert state.get_corresponding_handler(second_type()) is second_handler + + +def test_retrieve_handler_for_unknown_message_type(): + """ + Test that retrieving the corresponding handler for an unknown message type raises an error. + """ + state = State(None) + + with pytest.raises(KeyError, match='DummyMessage'): + state.get_corresponding_handler(DummyMessage('test-dummy')) + + +def test_handler_key_error_is_not_mistaken_for_an_unknown_message(): + """ + Exceptions raised by handlers should not be treated as unknown messages. + """ + state = State(None) + handler = KeyErrorMessageHandler(state) + state.add_handler(DummyMessage, handler) + + with pytest.raises(KeyError, match='handler failure'): + state.dispatch_message(DummyMessage('test-dummy')) From 269fa3b1449b572d1b50246e27f3e81d219695a0 Mon Sep 17 00:00:00 2001 From: Guillaume Fieni Date: Thu, 15 Oct 2026 15:01:53 +0200 Subject: [PATCH 2/2] refactor(actor)!: Rename `dispatch_message` method to `handle_message` --- src/powerapi/actor/actor.py | 2 +- src/powerapi/actor/handler.py | 2 +- src/powerapi/actor/state.py | 6 +++--- tests/unit/actor/test_actor.py | 2 +- tests/unit/actor/test_state.py | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/powerapi/actor/actor.py b/src/powerapi/actor/actor.py index 9ea588e4..733f6322 100644 --- a/src/powerapi/actor/actor.py +++ b/src/powerapi/actor/actor.py @@ -137,7 +137,7 @@ def _process_received_messages(self) -> None: return # Timeout logging.debug('Received message: %s', msg) - self.state.dispatch_message(msg) + self.state.handle_message(msg) def _teardown_actor(self) -> None: """ diff --git a/src/powerapi/actor/handler.py b/src/powerapi/actor/handler.py index 6e91d6c6..2c2ba7d5 100644 --- a/src/powerapi/actor/handler.py +++ b/src/powerapi/actor/handler.py @@ -119,4 +119,4 @@ def handle(self, msg: PoisonPillMessage) -> None: self.state.alive = False while (pending_msg := self.state.actor.socket_interface.receive(timeout=100)) is not None: - self.state.dispatch_message(pending_msg) + self.state.handle_message(pending_msg) diff --git a/src/powerapi/actor/state.py b/src/powerapi/actor/state.py index 2ea9c89f..c730a89c 100644 --- a/src/powerapi/actor/state.py +++ b/src/powerapi/actor/state.py @@ -109,10 +109,10 @@ def add_handler(self, message_type: type[Message], handler: Handler) -> None: self.handlers[message_type] = handler self._resolved_handlers.clear() - def dispatch_message(self, msg: Message) -> None: + def handle_message(self, msg: Message) -> None: """ - Dispatch a message to its registered handler. - :param msg: Message to dispatch + Handle a message using its registered handler. + :param msg: Message to handle """ try: handler = self.get_corresponding_handler(msg) diff --git a/tests/unit/actor/test_actor.py b/tests/unit/actor/test_actor.py index 60f1fb8e..672144b3 100644 --- a/tests/unit/actor/test_actor.py +++ b/tests/unit/actor/test_actor.py @@ -248,7 +248,7 @@ def test_poison_pill_dispatches_pending_messages_before_state_teardown(): handler.handle(PoisonPillMessage()) - state.dispatch_message.assert_called_once_with(pending_message) + state.handle_message.assert_called_once_with(pending_message) state.teardown.assert_not_called() assert state.alive is False diff --git a/tests/unit/actor/test_state.py b/tests/unit/actor/test_state.py index 04d71e61..6438c714 100644 --- a/tests/unit/actor/test_state.py +++ b/tests/unit/actor/test_state.py @@ -234,4 +234,4 @@ def test_handler_key_error_is_not_mistaken_for_an_unknown_message(): state.add_handler(DummyMessage, handler) with pytest.raises(KeyError, match='handler failure'): - state.dispatch_message(DummyMessage('test-dummy')) + state.handle_message(DummyMessage('test-dummy'))