diff --git a/cmk/base/plugins/server_side_calls/register.py b/cmk/base/plugins/server_side_calls/register.py index a856ed72302..8d3e9a258b8 100644 --- a/cmk/base/plugins/server_side_calls/register.py +++ b/cmk/base/plugins/server_side_calls/register.py @@ -14,13 +14,11 @@ def load_active_checks() -> tuple[Sequence[str], Mapping[str, ActiveCheckConfig]]: loaded = discover_plugins( "server_side_calls", - "active_check_", - ActiveCheckConfig, + {ActiveCheckConfig: "active_check_"}, raise_errors=cmk.utils.debug.enabled(), ) # TODO: # * see if we really need to return the errors. Maybe we can just either ignore or raise them. - # * deal with duplicate names. return [str(e) for e in loaded.errors], { plugin.name: plugin for plugin in loaded.plugins.values() } @@ -29,13 +27,11 @@ def load_active_checks() -> tuple[Sequence[str], Mapping[str, ActiveCheckConfig] def load_special_agents() -> tuple[Sequence[str], Mapping[str, SpecialAgentConfig]]: loaded = discover_plugins( "server_side_calls", - "special_agent_", - SpecialAgentConfig, + {SpecialAgentConfig: "special_agent_"}, raise_errors=cmk.utils.debug.enabled(), ) # TODO: # * see if we really need to return the errors. Maybe we can just either ignore or raise them. - # * deal with duplicate names. return [str(e) for e in loaded.errors], { plugin.name: plugin for plugin in loaded.plugins.values() } diff --git a/cmk/discover_plugins.py b/cmk/discover_plugins.py index faead930ec0..93813a557e6 100644 --- a/cmk/discover_plugins.py +++ b/cmk/discover_plugins.py @@ -57,9 +57,7 @@ class DiscoveredPlugins(Generic[_PluginType]): def discover_plugins( plugin_group: str, - name_prefix: str, - plugin_type: type[_PluginType], - *, + plugin_prefixes: Mapping[type[_PluginType], str], raise_errors: bool, ) -> DiscoveredPlugins[_PluginType]: """Collect all plugins from well-known locations""" @@ -71,7 +69,7 @@ def discover_plugins( ) namespaces_by_priority = find_namespaces(modules, plugin_group, ls=_ls_defensive) - collector = Collector(plugin_type, name_prefix, raise_errors=raise_errors) + collector = Collector(plugin_prefixes, raise_errors=raise_errors) for mod_name in namespaces_by_priority: collector.add_from_module(mod_name, _import_optionally) @@ -117,7 +115,7 @@ def _import_optionally(module_name: str, raise_errors: bool) -> ModuleType | Non return importlib.import_module(module_name) except ModuleNotFoundError: return None # never choke upon empty/non-existing folders. - except Exception as _exc: + except Exception: if raise_errors: raise return None @@ -133,13 +131,18 @@ def _ls_defensive(path: str) -> Sequence[str]: class Collector(Generic[_PluginType]): def __init__( self, - plugin_type: type[_PluginType], - name_prefix: str, - *, + plugin_prefixes: Mapping[type[_PluginType], str], raise_errors: bool, ) -> None: - self.plugin_type: Final = plugin_type - self.name_prefix: Final = name_prefix + # Transform plugin types / prefixes to the data structure we + # need for this algorithm. + # We pass them differently to help the type inference along. + # Other approaches require even worse explicit type annotations + # on caller side. + self._prefix_to_types = tuple( + (prefix, tuple(t for t, p in plugin_prefixes.items() if p == prefix)) + for prefix in set(plugin_prefixes.values()) + ) self.raise_errors: Final = raise_errors self.errors: list[Exception] = [] @@ -170,11 +173,15 @@ def _collect_module_plugins( ) -> None: """Dispatch valid and invalid well-known objects""" for name, value in objects.items(): - if not name.startswith(self.name_prefix): - continue + try: + plugin_types = next( + types for prefix, types in self._prefix_to_types if name.startswith(prefix) + ) + except StopIteration: + continue # no match location = PluginLocation(module_name, name) - if not isinstance(value, self.plugin_type): + if not isinstance(value, plugin_types): self._handle_error(TypeError(f"{location}: {value!r}")) continue diff --git a/cmk/gui/utils/rulespecs/loader.py b/cmk/gui/utils/rulespecs/loader.py index e335d1b6b59..177853d7018 100644 --- a/cmk/gui/utils/rulespecs/loader.py +++ b/cmk/gui/utils/rulespecs/loader.py @@ -5,15 +5,14 @@ from collections.abc import Mapping, Sequence -from cmk.discover_plugins import discover_plugins, DiscoveredPlugins +from cmk.discover_plugins import discover_plugins from cmk.rulesets.v1 import RuleSpec def load_api_v1_rulespecs(raise_errors: bool) -> tuple[Sequence[str], Mapping[str, RuleSpec]]: - discovered_plugins: DiscoveredPlugins[RuleSpec] = discover_plugins( + discovered_plugins = discover_plugins( "rulesets", - "rulespec_", - RuleSpec, + {RuleSpec: "rulespec_"}, raise_errors=raise_errors, ) errors = [str(e) for e in discovered_plugins.errors] diff --git a/tests/unit/cmk/test_discover_plugins.py b/tests/unit/cmk/test_discover_plugins.py index d87502d2464..e53cd8e2883 100644 --- a/tests/unit/cmk/test_discover_plugins.py +++ b/tests/unit/cmk/test_discover_plugins.py @@ -5,7 +5,7 @@ from collections.abc import Iterable from dataclasses import dataclass -from types import ModuleType +from types import ModuleType, UnionType import pytest @@ -101,16 +101,16 @@ def test_find_namespaces_deduplicate_preserving_order() -> None: @dataclass -class MyPluginBase: +class MyTestPlugin: name: str -class MyTestPlugin(MyPluginBase): - pass +@dataclass +class MyOtherPlugin: + name: str -class MyOtherPlugin(MyPluginBase): - pass +AllMyPlugins: UnionType = MyTestPlugin | MyOtherPlugin class TestCollector: @@ -125,12 +125,14 @@ def _importer(self, module_name: str, raise_errors: bool) -> ModuleType | None: return _make_module("my_other_type", [], my_plugin_2=MyOtherPlugin("herta")) case "my_collision": return _make_module("my_collision", [], my_plugin_3=MyTestPlugin("herta")) + case "your_module": + return _make_module("your_module", [], your_plugin_1=MyOtherPlugin("herbert")) raise ValueError("this test seems broken") def test_sipmle_ok_case(self) -> None: """Load a plugin""" - collector = Collector(MyTestPlugin, "my_", raise_errors=False) + collector = Collector({MyTestPlugin: "my_"}, raise_errors=False) collector.add_from_module("my_module", self._importer) assert not collector.errors assert collector.plugins == { @@ -139,7 +141,7 @@ def test_sipmle_ok_case(self) -> None: def test_mising_ok(self) -> None: """Ignore missing modules""" - collector = Collector(MyTestPlugin, "my_", raise_errors=True) + collector = Collector({MyTestPlugin: "my_"}, raise_errors=True) # missing is ok, even if raise_errors is true. collector.add_from_module("nonexistant", self._importer) assert not collector.errors @@ -147,28 +149,28 @@ def test_mising_ok(self) -> None: def test_unknown_name_ignored(self) -> None: """Do not load a plugin with name prefix missmatch""" - collector = Collector(MyTestPlugin, "your_", raise_errors=False) + collector = Collector({MyTestPlugin: "your_"}, raise_errors=False) collector.add_from_module("my_module", self._importer) assert not collector.errors assert not collector.plugins def test_wrong_type_raise(self) -> None: """Raise if a plugin has the wrong type""" - collector = Collector(MyTestPlugin, "my_", raise_errors=True) + collector = Collector({MyTestPlugin: "my_"}, raise_errors=True) with pytest.raises(TypeError): collector.add_from_module("my_other_type", self._importer) def test_wrong_type_recorded(self) -> None: """Record the error if a plugin has the wrong type""" - collector = Collector(MyTestPlugin, "my_", raise_errors=False) + collector = Collector({MyTestPlugin: "my_"}, raise_errors=False) collector.add_from_module("my_other_type", self._importer) assert len(collector.errors) == 1 assert isinstance(collector.errors[0], TypeError) assert not collector.plugins def test_name_collision_same_type(self) -> None: - collector = Collector(MyTestPlugin, "my_", raise_errors=True) + collector = Collector({MyTestPlugin: "my_"}, raise_errors=True) collector.add_from_module("my_module", self._importer) with pytest.raises(ValueError, match="already defined"): @@ -182,11 +184,27 @@ def test_name_collision_same_type(self) -> None: def test_name_collision_different_type(self) -> None: """Plugins with same name but different type is ok""" - collector = Collector(MyPluginBase, "my_", raise_errors=False) + collector = Collector[MyOtherPlugin | MyTestPlugin]( + {MyTestPlugin: "my_", MyOtherPlugin: "my_"}, raise_errors=False + ) collector.add_from_module("my_module", self._importer) collector.add_from_module("my_other_type", self._importer) + assert not collector.errors assert collector.plugins == { PluginLocation("my_module", "my_plugin_1"): MyTestPlugin("herta"), PluginLocation("my_other_type", "my_plugin_2"): MyOtherPlugin("herta"), } + + def test_different_prefixes(self) -> None: + collector = Collector[MyTestPlugin | MyOtherPlugin]( + {MyTestPlugin: "my_", MyOtherPlugin: "your_"}, raise_errors=False + ) + collector.add_from_module("my_module", self._importer) + collector.add_from_module("your_module", self._importer) + + assert not collector.errors + assert collector.plugins == { + PluginLocation("my_module", "my_plugin_1"): MyTestPlugin("herta"), + PluginLocation("your_module", "your_plugin_1"): MyOtherPlugin("herbert"), + }