Remove abstraction level MergeByIdentity and make it root instead

Signed-off-by: Mathias L. Baumann <[email protected]>

Author: Marenz

src/frequenz/dispatch/__init__.py

@@ -20,7 +20,7 @@
 from ._dispatch import Dispatch
 from ._dispatcher import Dispatcher
 from ._event import Created, Deleted, DispatchEvent, Updated
-from ._merge_strategies import MergeByIdentity, MergeByType, MergeByTypeTarget
+from ._merge_strategies import MergeByType, MergeByTypeTarget
 
 __all__ = [
     "Created",
@@ -32,7 +32,6 @@
     "ActorDispatcher",
     "DispatchInfo",
     "MergeStrategy",
-    "MergeByIdentity",
     "MergeByType",
     "MergeByTypeTarget",
 ]

src/frequenz/dispatch/_bg_service.py

@@ -31,6 +31,10 @@
 class MergeStrategy(ABC):
     """Base class for strategies to merge running intervals."""
 
+    @abstractmethod
+    def identity(self, dispatch: Dispatch) -> int:
+        """Identity function for the merge criteria."""
+
     @abstractmethod
     def filter(self, dispatches: Mapping[int, Dispatch], dispatch: Dispatch) -> bool:
         """Filter dispatches based on the strategy.
@@ -154,12 +158,9 @@ async def new_running_state_event_receiver(
           dispatches of the same type and target
         * `None` — no merging, just send all events
 
-        You can make your own strategy by subclassing:
-
-        * [`MergeByIdentity`][frequenz.dispatch.MergeByIdentity] — Merges
-          dispatches based on a user defined identity function
-        * [`MergeStrategy`][frequenz.dispatch.MergeStrategy] — Merges based
-          on a user defined filter function
+        You can make your own identity-based strategy by subclassing `MergeByType` and overriding
+        the `identity()` method. If you require a more complex strategy, you can subclass
+        `MergeStrategy` directly and implement both the `identity()` and `filter()` methods.
 
         Running intervals from multiple dispatches will be merged, according to
         the chosen strategy.

src/frequenz/dispatch/_dispatcher.py

@@ -19,7 +19,6 @@
 from ._bg_service import DispatchScheduler, MergeStrategy
 from ._dispatch import Dispatch
 from ._event import DispatchEvent
-from ._merge_strategies import MergeByIdentity
 
 _logger = logging.getLogger(__name__)
 
@@ -227,7 +226,7 @@ async def start_dispatching(
         dispatch_type: str,
         *,
         actor_factory: Callable[[DispatchInfo, Receiver[DispatchInfo]], Actor],
-        merge_strategy: MergeByIdentity | None = None,
+        merge_strategy: MergeStrategy | None = None,
     ) -> None:
         """Manage actors for a given dispatch type.
 

src/frequenz/dispatch/_merge_strategies.py

@@ -4,7 +4,6 @@
 """Different merge strategies for dispatch running state events."""
 
 import logging
-from abc import abstractmethod
 from collections.abc import Mapping
 
 from typing_extensions import override
@@ -13,12 +12,13 @@
 from ._dispatch import Dispatch
 
 
-class MergeByIdentity(MergeStrategy):
-    """Merge running intervals based on a dispatch configuration."""
+class MergeByType(MergeStrategy):
+    """Merge running intervals based on the dispatch type."""
 
-    @abstractmethod
+    @override
     def identity(self, dispatch: Dispatch) -> int:
         """Identity function for the merge criteria."""
+        return hash(dispatch.type)
 
     @override
     def filter(self, dispatches: Mapping[int, Dispatch], dispatch: Dispatch) -> bool:
@@ -49,15 +49,6 @@ def filter(self, dispatches: Mapping[int, Dispatch], dispatch: Dispatch) -> bool
         return not other_dispatches_running
 
 
-class MergeByType(MergeByIdentity):
-    """Merge running intervals based on the dispatch type."""
-
-    @override
-    def identity(self, dispatch: Dispatch) -> int:
-        """Identity function for the merge criteria."""
-        return hash(dispatch.type)
-
-
 class MergeByTypeTarget(MergeByType):
     """Merge running intervals based on the dispatch type and target."""
 

tests/test_frequenz_dispatch.py

@@ -24,7 +24,6 @@
     Deleted,
     Dispatch,
     DispatchEvent,
-    MergeByIdentity,
     MergeByType,
     MergeByTypeTarget,
     MergeStrategy,
@@ -679,7 +678,7 @@ async def test_multiple_dispatches_sequential_intervals_merge(
 async def test_at_least_one_running_filter(
     fake_time: time_machine.Coordinates,
     generator: DispatchGenerator,
-    merge_strategy: MergeByIdentity,
+    merge_strategy: MergeStrategy,
 ) -> None:
     """Test scenarios directly tied to the _at_least_one_running logic."""
     microgrid_id = randint(1, 100)

tests/test_mananging_actor.py

@@ -27,7 +27,6 @@
     Dispatch,
     Dispatcher,
     DispatchInfo,
-    MergeByIdentity,
     MergeByType,
     MergeByTypeTarget,
     MergeStrategy,
@@ -257,7 +256,7 @@ async def test_dry_run(test_env: TestEnv, fake_time: time_machine.Coordinates) -
 async def test_manage_abstraction(
     fake_time: time_machine.Coordinates,
     generator: DispatchGenerator,
-    strategy: MergeByIdentity | None,
+    strategy: MergeStrategy | None,
 ) -> None:
     """Test Dispatcher.start_dispatching sets up correctly."""
     identity: Callable[[Dispatch], int] = (