diff --git a/unstract/core/src/unstract/core/plugins/plugin_manager.py b/unstract/core/src/unstract/core/plugins/plugin_manager.py index 1b091d9cf4..7b06d1bb06 100644 --- a/unstract/core/src/unstract/core/plugins/plugin_manager.py +++ b/unstract/core/src/unstract/core/plugins/plugin_manager.py @@ -11,6 +11,11 @@ from pathlib import Path from typing import Any +# Metadata keys ending in either suffix are treated as class/entrypoint handles +# and promoted onto the registered plugin dict (e.g. entrypoint_cls, +# exception_cls, service_class). +_CLASS_HANDLE_SUFFIXES = ("_cls", "_class") + class PluginManager: """Generic plugin manager for loading and managing application plugins. @@ -215,13 +220,16 @@ def load_plugins(self) -> None: "metadata": metadata, } - # Add optional fields if present - if "entrypoint_cls" in metadata: - plugin_data["entrypoint_cls"] = metadata["entrypoint_cls"] - if "exception_cls" in metadata: - plugin_data["exception_cls"] = metadata["exception_cls"] - if "service_class" in metadata: - plugin_data["service_class"] = metadata["service_class"] + # Add optional class/entrypoint fields if present, so new plugin + # metadata fields don't need this loader updated every time. + for key, value in metadata.items(): + if not isinstance(key, str): + continue + # `key not in plugin_data` is defensive: none of the fixed + # keys above (version/module/metadata) end in a class-handle + # suffix, so this never actually excludes anything today. + if key.endswith(_CLASS_HANDLE_SUFFIXES) and key not in plugin_data: + plugin_data[key] = value self.plugins[plugin_name] = plugin_data diff --git a/unstract/core/tests/test_plugin_manager.py b/unstract/core/tests/test_plugin_manager.py new file mode 100644 index 0000000000..407f5de468 --- /dev/null +++ b/unstract/core/tests/test_plugin_manager.py @@ -0,0 +1,123 @@ +import importlib +import sys + +import pytest +from unstract.core.plugins.plugin_manager import PluginManager + + +def _write_plugin(plugins_pkg_dir, plugin_name, metadata_body): + plugin_dir = plugins_pkg_dir / plugin_name + plugin_dir.mkdir() + (plugin_dir / "__init__.py").write_text(f"metadata = {metadata_body}\n") + + +@pytest.fixture +def plugins_pkg_dir(tmp_path, monkeypatch): + """Create an importable package directory to hold fake plugins.""" + pkg_dir = tmp_path / "fake_plugins_pkg" + pkg_dir.mkdir() + (pkg_dir / "__init__.py").write_text("") + + monkeypatch.syspath_prepend(str(tmp_path)) + importlib.invalidate_caches() + + yield pkg_dir + + sys.modules.pop("fake_plugins_pkg", None) + for name in list(sys.modules): + if name.startswith("fake_plugins_pkg."): + sys.modules.pop(name) + + +def _load(plugins_pkg_dir): + manager = PluginManager( + plugins_dir=plugins_pkg_dir, + plugins_pkg="fake_plugins_pkg", + use_singleton=False, + ) + manager.load_plugins() + return manager + + +def test_known_and_unknown_class_fields_are_both_registered(plugins_pkg_dir): + """A plugin's *_class/*_cls metadata fields should surface on the plugin dict, + including fields the loader has never seen before (e.g. headers_cache_class) - + not just the historically hardcoded entrypoint_cls/exception_cls/service_class. + """ + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "service_class": str, "headers_cache_class": dict, }', + ) + + plugin = _load(plugins_pkg_dir).get_plugin("sample_plugin") + + assert plugin["service_class"] is str + assert plugin["headers_cache_class"] is dict + assert plugin["metadata"]["name"] == "sample_plugin" + + +def test_entrypoint_and_exception_cls_still_supported(plugins_pkg_dir): + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "entrypoint_cls": int, "exception_cls": ValueError, }', + ) + + plugin = _load(plugins_pkg_dir).get_plugin("sample_plugin") + + assert plugin["entrypoint_cls"] is int + assert plugin["exception_cls"] is ValueError + + +def test_non_class_metadata_fields_are_not_duplicated_at_top_level(plugins_pkg_dir): + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "description": "just a description", }', + ) + + plugin = _load(plugins_pkg_dir).get_plugin("sample_plugin") + + assert "description" not in plugin + assert plugin["metadata"]["description"] == "just a description" + + +def test_disabled_plugin_is_not_loaded(plugins_pkg_dir): + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "disable": True}', + ) + + manager = _load(plugins_pkg_dir) + + assert not manager.has_plugin("sample_plugin") + + +def test_inactive_plugin_is_not_loaded(plugins_pkg_dir): + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "is_active": False}', + ) + + manager = _load(plugins_pkg_dir) + + assert not manager.has_plugin("sample_plugin") + + +def test_class_field_propagates_regardless_of_value(plugins_pkg_dir): + """A *_class/*_cls field is propagated by name, not filtered by its value - + a plugin can legitimately set one to None before it's populated at runtime. + """ + _write_plugin( + plugins_pkg_dir, + "sample_plugin", + '{"name": "sample_plugin", "headers_cache_class": None}', + ) + + plugin = _load(plugins_pkg_dir).get_plugin("sample_plugin") + + assert "headers_cache_class" in plugin + assert plugin["headers_cache_class"] is None