From 630fcd4a286ca523be93dee7ac63eaca35ad14f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:25:35 +0000 Subject: [PATCH 01/16] feat(integrations): add IntegrationPlugin Protocol scaffold Refs #22 --- backend/app/integrations/__init__.py | 1 + backend/app/integrations/base.py | 29 ++++++++++++++++++++ backend/tests/unit/integrations/__init__.py | 0 backend/tests/unit/integrations/test_base.py | 24 ++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 backend/app/integrations/__init__.py create mode 100644 backend/app/integrations/base.py create mode 100644 backend/tests/unit/integrations/__init__.py create mode 100644 backend/tests/unit/integrations/test_base.py diff --git a/backend/app/integrations/__init__.py b/backend/app/integrations/__init__.py new file mode 100644 index 0000000..35c8f0c --- /dev/null +++ b/backend/app/integrations/__init__.py @@ -0,0 +1 @@ +"""Integration plugin discovery — populated in Phase 2.""" diff --git a/backend/app/integrations/base.py b/backend/app/integrations/base.py new file mode 100644 index 0000000..a794d2c --- /dev/null +++ b/backend/app/integrations/base.py @@ -0,0 +1,29 @@ +"""Protocol contract for integration-lookup plugins. + +Each external app (Snipe-IT, Spoolman, Grocy, future integrations) lives +in its own module under app.integrations., implements this Protocol, +and registers itself via setuptools entry-points (group +'label_hub.integrations'). The Protocol is @runtime_checkable so the +registry can validate candidates with isinstance() at registration time. +""" + +from __future__ import annotations + +from typing import Protocol, runtime_checkable + +from app.schemas.label_data import LabelData + + +@runtime_checkable +class IntegrationPlugin(Protocol): + """Per-integration lookup contract.""" + + name: str # canonical id, e.g. "snipeit" — matches TemplateSchema.app + display_name: str # UI-friendly, e.g. "Snipe-IT" + + async def lookup(self, identifier: str) -> LabelData: + """Resolve an integration-specific identifier to LabelData. + + Raises AppLookupNotFoundError (or a subclass) if the entity does not + exist on the upstream app. + """ diff --git a/backend/tests/unit/integrations/__init__.py b/backend/tests/unit/integrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/unit/integrations/test_base.py b/backend/tests/unit/integrations/test_base.py new file mode 100644 index 0000000..0e2451a --- /dev/null +++ b/backend/tests/unit/integrations/test_base.py @@ -0,0 +1,24 @@ +"""Smoke tests for the IntegrationPlugin Protocol.""" + +from typing import get_type_hints + +from app.integrations.base import IntegrationPlugin + + +def test_protocol_is_runtime_checkable() -> None: + """isinstance() must work — the registry uses it defensively.""" + class _Fake: + name = "x" + display_name = "X" + + async def lookup(self, identifier: str) -> object: + return object() + + assert isinstance(_Fake(), IntegrationPlugin) + + +def test_protocol_requires_name_display_name_lookup() -> None: + """Type hints declare the contract.""" + hints = get_type_hints(IntegrationPlugin) + assert "name" in hints and hints["name"] is str + assert "display_name" in hints and hints["display_name"] is str From 618b6646879b4f881713da7d1f3ecb4b2f5449eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:26:04 +0000 Subject: [PATCH 02/16] feat(integrations): add IntegrationRegistry with register/get/all/names Refs #22 --- backend/app/integrations/registry.py | 55 +++++++++++++++ .../tests/unit/integrations/test_registry.py | 67 +++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 backend/app/integrations/registry.py create mode 100644 backend/tests/unit/integrations/test_registry.py diff --git a/backend/app/integrations/registry.py b/backend/app/integrations/registry.py new file mode 100644 index 0000000..0892de3 --- /dev/null +++ b/backend/app/integrations/registry.py @@ -0,0 +1,55 @@ +"""Discover integration plugins by name. + +The registry is class-level state, populated either by direct +`register()` calls (used by tests) or by the entry-points discovery in +`app.integrations.__init__` at module import time. +""" + +from __future__ import annotations + +from typing import ClassVar + +from app.integrations.base import IntegrationPlugin + + +class IntegrationNotFoundError(Exception): + """No plugin registered for the given integration name.""" + + +class IntegrationRegistry: + """Class-level registry of IntegrationPlugin instances.""" + + _plugins: ClassVar[dict[str, IntegrationPlugin]] = {} + + @classmethod + def register(cls, plugin: IntegrationPlugin) -> None: + """Add `plugin` under its `.name`. Rejects empty names and duplicates.""" + if not plugin.name: + raise ValueError( + f"IntegrationPlugin requires a non-empty name; got {plugin.name!r}" + ) + if plugin.name in cls._plugins: + raise ValueError( + f"Plugin {plugin.name!r} already registered" + ) + cls._plugins[plugin.name] = plugin + + @classmethod + def get(cls, name: str) -> IntegrationPlugin: + """Return the plugin registered under `name` or raise.""" + plugin = cls._plugins.get(name) + if plugin is None: + raise IntegrationNotFoundError( + f"Unknown integration {name!r}. Registered: {sorted(cls._plugins)}" + ) + return plugin + + @classmethod + def all(cls) -> dict[str, IntegrationPlugin]: + """Return a shallow copy of the registry (callers may mutate safely).""" + return dict(cls._plugins) + + @classmethod + def names(cls) -> list[str]: + """Return registered plugin names, sorted alphabetically.""" + return sorted(cls._plugins) diff --git a/backend/tests/unit/integrations/test_registry.py b/backend/tests/unit/integrations/test_registry.py new file mode 100644 index 0000000..d1d717b --- /dev/null +++ b/backend/tests/unit/integrations/test_registry.py @@ -0,0 +1,67 @@ +"""Unit tests for IntegrationRegistry.""" + +import pytest +from app.integrations.base import IntegrationPlugin +from app.integrations.registry import IntegrationNotFoundError, IntegrationRegistry +from app.schemas.label_data import LabelData + + +class _FakePlugin: + """Minimal IntegrationPlugin-shaped object for tests.""" + + def __init__(self, name: str = "fake", display_name: str = "Fake") -> None: + self.name = name + self.display_name = display_name + + async def lookup(self, identifier: str) -> LabelData: + return LabelData( + title="fake", primary_id=identifier, qr_payload="x", source_app=self.name + ) + + +@pytest.fixture(autouse=True) +def _clear_registry() -> None: + """Each test starts with an empty registry.""" + IntegrationRegistry._plugins.clear() + + +def test_register_stores_plugin() -> None: + p = _FakePlugin() + IntegrationRegistry.register(p) + assert IntegrationRegistry.get("fake") is p + + +def test_register_rejects_empty_name() -> None: + p = _FakePlugin(name="") + with pytest.raises(ValueError, match="non-empty name"): + IntegrationRegistry.register(p) + + +def test_register_rejects_duplicate() -> None: + IntegrationRegistry.register(_FakePlugin()) + with pytest.raises(ValueError, match="already registered"): + IntegrationRegistry.register(_FakePlugin()) + + +def test_get_unknown_raises_not_found() -> None: + with pytest.raises(IntegrationNotFoundError, match="Unknown integration 'nope'"): + IntegrationRegistry.get("nope") + + +def test_all_returns_copy() -> None: + IntegrationRegistry.register(_FakePlugin(name="a")) + IntegrationRegistry.register(_FakePlugin(name="b")) + snapshot = IntegrationRegistry.all() + snapshot.clear() + assert IntegrationRegistry.names() == ["a", "b"] + + +def test_names_returns_sorted() -> None: + IntegrationRegistry.register(_FakePlugin(name="zeta")) + IntegrationRegistry.register(_FakePlugin(name="alpha")) + assert IntegrationRegistry.names() == ["alpha", "zeta"] + + +def test_runtime_protocol_check_accepts_fake() -> None: + """The Protocol is structural — _FakePlugin implements it.""" + assert isinstance(_FakePlugin(), IntegrationPlugin) From b2f0edddb97c4e85e73ec6aa1a0e6b3f6d25a5b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:34:07 +0000 Subject: [PATCH 03/16] feat(integrations): harden IntegrationRegistry name validation - Reject whitespace-only names (was: passed silently) - Reject non-string name types with TypeError (was: crashed later inside sorted()) - Test fixture clears registry on both setup and teardown Refs #22 --- backend/app/integrations/registry.py | 11 ++++++++-- .../tests/unit/integrations/test_registry.py | 21 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/backend/app/integrations/registry.py b/backend/app/integrations/registry.py index 0892de3..45b7c26 100644 --- a/backend/app/integrations/registry.py +++ b/backend/app/integrations/registry.py @@ -23,8 +23,15 @@ class IntegrationRegistry: @classmethod def register(cls, plugin: IntegrationPlugin) -> None: - """Add `plugin` under its `.name`. Rejects empty names and duplicates.""" - if not plugin.name: + """Add `plugin` under its `.name`. + + Rejects empty, whitespace-only, or non-string names and duplicates. + """ + if not isinstance(plugin.name, str): + raise TypeError( + f"IntegrationPlugin name must be a string; got {type(plugin.name).__name__}" + ) + if not plugin.name.strip(): raise ValueError( f"IntegrationPlugin requires a non-empty name; got {plugin.name!r}" ) diff --git a/backend/tests/unit/integrations/test_registry.py b/backend/tests/unit/integrations/test_registry.py index d1d717b..f6637ef 100644 --- a/backend/tests/unit/integrations/test_registry.py +++ b/backend/tests/unit/integrations/test_registry.py @@ -1,5 +1,7 @@ """Unit tests for IntegrationRegistry.""" +from collections.abc import Generator + import pytest from app.integrations.base import IntegrationPlugin from app.integrations.registry import IntegrationNotFoundError, IntegrationRegistry @@ -20,8 +22,10 @@ async def lookup(self, identifier: str) -> LabelData: @pytest.fixture(autouse=True) -def _clear_registry() -> None: - """Each test starts with an empty registry.""" +def _clear_registry() -> Generator[None, None, None]: + """Each test starts with an empty registry; clean up after as well.""" + IntegrationRegistry._plugins.clear() + yield IntegrationRegistry._plugins.clear() @@ -37,6 +41,19 @@ def test_register_rejects_empty_name() -> None: IntegrationRegistry.register(p) +def test_register_rejects_whitespace_only_name() -> None: + p = _FakePlugin(name=" ") + with pytest.raises(ValueError, match="non-empty name"): + IntegrationRegistry.register(p) + + +def test_register_rejects_non_string_name() -> None: + p = _FakePlugin() + p.name = 42 # type: ignore[assignment] + with pytest.raises(TypeError, match="must be a string"): + IntegrationRegistry.register(p) + + def test_register_rejects_duplicate() -> None: IntegrationRegistry.register(_FakePlugin()) with pytest.raises(ValueError, match="already registered"): From 54a118e19cd490569cdf4986efbcbcf1b0488fec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:39:05 +0000 Subject: [PATCH 04/16] feat(integrations): entry_points discovery with defensive plugin loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loads plugins from the 'label_hub.integrations' entry-points group. Each plugin is loaded, instantiated, validated against the IntegrationPlugin Protocol, and registered independently — a single broken third-party plugin logs an error and is skipped without preventing the others from loading. Failure modes covered: load-time exception, non-Plugin export, name collision with an already-registered plugin. Refs #22 --- backend/app/integrations/__init__.py | 71 +++++++- .../tests/unit/integrations/test_discovery.py | 151 ++++++++++++++++++ 2 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 backend/tests/unit/integrations/test_discovery.py diff --git a/backend/app/integrations/__init__.py b/backend/app/integrations/__init__.py index 35c8f0c..f3bbd40 100644 --- a/backend/app/integrations/__init__.py +++ b/backend/app/integrations/__init__.py @@ -1 +1,70 @@ -"""Integration plugin discovery — populated in Phase 2.""" +"""Integration plugin discovery. + +Importing this package triggers `_discover_plugins`, which scans the +`label_hub.integrations` entry-points group and registers every declared +plugin with `IntegrationRegistry`. Bundled plugins (snipeit, spoolman, +grocy) declare their entry-points in this repo's `pyproject.toml`. +Third-party plugins installed via pip register the same way without any +change to the core repo. + +Loading is defensive: a broken third-party package logs an error and is +skipped, the remaining plugins still register. +""" + +from __future__ import annotations + +import importlib.metadata +import logging + +from app.integrations.base import IntegrationPlugin +from app.integrations.registry import IntegrationRegistry + +_logger = logging.getLogger(__name__) + + +def _discover_plugins() -> None: + """Load every plugin under the 'label_hub.integrations' entry-points group. + + Each entry point is loaded independently — a failure in one does not + prevent the others from registering. Three failure modes are handled: + the loaded object is not an IntegrationPlugin, ep.load() itself raises, + or two plugins share a name (collision). + """ + for ep in importlib.metadata.entry_points(group="label_hub.integrations"): + try: + plugin_cls = ep.load() + except Exception as e: # third-party load can raise anything + _logger.error( + "Failed to load entry-point %r: %s", ep.name, e + ) + continue + + try: + instance = plugin_cls() + except Exception as e: + _logger.error( + "Failed to instantiate entry-point %r (class %r): %s", + ep.name, + plugin_cls.__name__ if hasattr(plugin_cls, "__name__") else plugin_cls, + e, + ) + continue + + if not isinstance(instance, IntegrationPlugin): + _logger.error( + "Entry-point %r exports %r which does not satisfy IntegrationPlugin " + "(missing required attributes name/display_name/lookup)", + ep.name, + type(instance).__name__, + ) + continue + + try: + IntegrationRegistry.register(instance) + except ValueError as e: + _logger.error( + "Entry-point %r could not register: %s", ep.name, e + ) + + +_discover_plugins() diff --git a/backend/tests/unit/integrations/test_discovery.py b/backend/tests/unit/integrations/test_discovery.py new file mode 100644 index 0000000..d2533d2 --- /dev/null +++ b/backend/tests/unit/integrations/test_discovery.py @@ -0,0 +1,151 @@ +"""Tests for entry_points-based plugin discovery.""" + +from collections.abc import Generator, Iterable + +import pytest +from app.integrations import _discover_plugins +from app.integrations.registry import IntegrationRegistry + + +class _FakeEntryPoint: + def __init__(self, name: str, plugin_cls: type) -> None: + self.name = name + self.value = f"{plugin_cls.__module__}:{plugin_cls.__name__}" + self._cls = plugin_cls + + def load(self) -> type: + return self._cls + + +class _AlphaPlugin: + name = "alpha" + display_name = "Alpha" + + async def lookup(self, identifier: str) -> object: + raise NotImplementedError + + +class _BetaPlugin: + name = "beta" + display_name = "Beta" + + async def lookup(self, identifier: str) -> object: + raise NotImplementedError + + +class _NotAPlugin: + """Class that doesn't satisfy IntegrationPlugin (missing display_name, lookup).""" + + name = "fake" + + +class _ExplodingEntryPoint: + def __init__(self, name: str) -> None: + self.name = name + self.value = "exploding" + + def load(self) -> type: + raise RuntimeError("simulated package import failure") + + +@pytest.fixture(autouse=True) +def _clear_registry() -> Generator[None, None, None]: + IntegrationRegistry._plugins.clear() + yield + IntegrationRegistry._plugins.clear() + + +def test_discover_loads_all_entry_points(monkeypatch: pytest.MonkeyPatch) -> None: + def fake_entry_points(group: str) -> Iterable[_FakeEntryPoint]: + assert group == "label_hub.integrations" + return [ + _FakeEntryPoint("alpha", _AlphaPlugin), + _FakeEntryPoint("beta", _BetaPlugin), + ] + + monkeypatch.setattr("importlib.metadata.entry_points", fake_entry_points) + _discover_plugins() + assert IntegrationRegistry.names() == ["alpha", "beta"] + + +def test_discover_with_no_entry_points_is_noop(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("importlib.metadata.entry_points", lambda group: []) + _discover_plugins() + assert IntegrationRegistry.names() == [] + + +def test_discover_rejects_non_plugin_class( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """An entry point that loads something not satisfying IntegrationPlugin is logged + skipped.""" + monkeypatch.setattr( + "importlib.metadata.entry_points", + lambda group: [_FakeEntryPoint("bad", _NotAPlugin)], + ) + with caplog.at_level("ERROR"): + _discover_plugins() + assert IntegrationRegistry.names() == [] + assert any("does not satisfy IntegrationPlugin" in r.message for r in caplog.records) + # Error message must name the entry-point so users can find the broken package + assert any("bad" in r.message for r in caplog.records) + + +def test_discover_handles_load_exception( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """If ep.load() raises, the discovery logs the error and continues with the next plugin.""" + monkeypatch.setattr( + "importlib.metadata.entry_points", + lambda group: [ + _ExplodingEntryPoint("broken"), + _FakeEntryPoint("alpha", _AlphaPlugin), + ], + ) + with caplog.at_level("ERROR"): + _discover_plugins() + # The good plugin still registered — bad one didn't kill the process + assert IntegrationRegistry.names() == ["alpha"] + assert any("broken" in r.message for r in caplog.records) + + +def test_discover_handles_duplicate_registration( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Two entry-points exporting plugins with the same name: log + skip the second.""" + + class _AlphaDup: + name = "alpha" + display_name = "Alpha Duplicate" + + async def lookup(self, identifier: str) -> object: + raise NotImplementedError + + monkeypatch.setattr( + "importlib.metadata.entry_points", + lambda group: [ + _FakeEntryPoint("alpha", _AlphaPlugin), + _FakeEntryPoint("alpha-dup", _AlphaDup), + ], + ) + with caplog.at_level("ERROR"): + _discover_plugins() + # Registry has the first one; the second was skipped with an error + assert IntegrationRegistry.names() == ["alpha"] + assert any( + "alpha-dup" in r.message or "already registered" in r.message + for r in caplog.records + ) + + +def test_protocol_rejects_incomplete_class() -> None: + """A class missing required attributes does not satisfy the Protocol. + + Carried forward from Phase 1 review feedback — verifies the negative + half of the @runtime_checkable contract. + """ + from app.integrations.base import IntegrationPlugin + + assert not isinstance(_NotAPlugin(), IntegrationPlugin) From 6a6708ae88a61ff5d73d24652200d4003f42ebd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:39:10 +0000 Subject: [PATCH 05/16] docs(integrations): align IntegrationPlugin Protocol docstring with discovery enforcement The Protocol's @runtime_checkable property is now enforced by _discover_plugins in app.integrations.__init__, not by the registry itself. Update the docstring to reflect where the isinstance check actually lives. Refs #22 --- backend/app/integrations/base.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/integrations/base.py b/backend/app/integrations/base.py index a794d2c..2ee8239 100644 --- a/backend/app/integrations/base.py +++ b/backend/app/integrations/base.py @@ -4,7 +4,9 @@ in its own module under app.integrations., implements this Protocol, and registers itself via setuptools entry-points (group 'label_hub.integrations'). The Protocol is @runtime_checkable so the -registry can validate candidates with isinstance() at registration time. +entry-points discovery in `app.integrations.__init__` can validate each +loaded class with isinstance() before registering it, rejecting broken +third-party plugins with a clear log message. """ from __future__ import annotations From ac4494e1f2488a8329d73b82e6245dfed375f7db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:45:50 +0000 Subject: [PATCH 06/16] feat(integrations): preserve tracebacks + catch TypeError in plugin discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _logger.exception() instead of error() on load/instantiate guards to capture stack traces for production debugging - except (ValueError, TypeError) on register() guard — non-string plugin name no longer escapes and kills the discovery loop - Add regression test for the TypeError-escape path - Docstring updated: four failure modes (was three) Refs #22 --- backend/app/integrations/__init__.py | 28 +++++++++-------- .../tests/unit/integrations/test_discovery.py | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/backend/app/integrations/__init__.py b/backend/app/integrations/__init__.py index f3bbd40..cce82d2 100644 --- a/backend/app/integrations/__init__.py +++ b/backend/app/integrations/__init__.py @@ -26,27 +26,29 @@ def _discover_plugins() -> None: """Load every plugin under the 'label_hub.integrations' entry-points group. Each entry point is loaded independently — a failure in one does not - prevent the others from registering. Three failure modes are handled: - the loaded object is not an IntegrationPlugin, ep.load() itself raises, - or two plugins share a name (collision). + prevent the others from registering. Four failure modes are handled: + + 1. The entry-point's `ep.load()` raises (broken third-party package). + 2. Instantiating the loaded class raises (constructor error). + 3. The loaded object does not satisfy the IntegrationPlugin Protocol + (missing attributes or wrong shape). + 4. The plugin's `name` collides with an already-registered plugin, or + has the wrong type (Registry rejects with ValueError / TypeError). """ for ep in importlib.metadata.entry_points(group="label_hub.integrations"): try: plugin_cls = ep.load() - except Exception as e: # third-party load can raise anything - _logger.error( - "Failed to load entry-point %r: %s", ep.name, e - ) + except Exception: + _logger.exception("Failed to load entry-point %r", ep.name) continue try: instance = plugin_cls() - except Exception as e: - _logger.error( - "Failed to instantiate entry-point %r (class %r): %s", + except Exception: + _logger.exception( + "Failed to instantiate entry-point %r (class %s)", ep.name, - plugin_cls.__name__ if hasattr(plugin_cls, "__name__") else plugin_cls, - e, + getattr(plugin_cls, "__name__", repr(plugin_cls)), ) continue @@ -61,7 +63,7 @@ def _discover_plugins() -> None: try: IntegrationRegistry.register(instance) - except ValueError as e: + except (ValueError, TypeError) as e: _logger.error( "Entry-point %r could not register: %s", ep.name, e ) diff --git a/backend/tests/unit/integrations/test_discovery.py b/backend/tests/unit/integrations/test_discovery.py index d2533d2..2f1f555 100644 --- a/backend/tests/unit/integrations/test_discovery.py +++ b/backend/tests/unit/integrations/test_discovery.py @@ -140,6 +140,36 @@ async def lookup(self, identifier: str) -> object: ) +def test_discover_handles_non_string_name_typeerror( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A plugin with a non-string `name` makes Registry raise TypeError — + discovery must catch it and continue with the next plugin instead of + aborting the loop.""" + + class _BadNamePlugin: + name = 42 # int, not str — Registry rejects this + display_name = "Bad" + + async def lookup(self, identifier: str) -> object: + raise NotImplementedError + + monkeypatch.setattr( + "importlib.metadata.entry_points", + lambda group: [ + _FakeEntryPoint("bad-name", _BadNamePlugin), + _FakeEntryPoint("alpha", _AlphaPlugin), + ], + ) + with caplog.at_level("ERROR"): + _discover_plugins() + # The good plugin still registered — bad one didn't kill the process + assert IntegrationRegistry.names() == ["alpha"] + # Bad plugin's entry-point name appears in error log + assert any("bad-name" in r.message for r in caplog.records) + + def test_protocol_rejects_incomplete_class() -> None: """A class missing required attributes does not satisfy the Protocol. From febd7655b45e3d4b1a3cef80a338100cce813b58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:51:31 +0000 Subject: [PATCH 07/16] refactor(integrations): relocate snipeit client to plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renames SnipeITClient → SnipeITPlugin and adds name/display_name attributes so the class satisfies the IntegrationPlugin Protocol. File moves preserve git history. lookup_service.py has no direct SnipeITClient import — it uses the _LookupClient structural Protocol — so no alias shim is needed there. The test top-level import uses `SnipeITPlugin as SnipeITClient` to keep all call-site names in the test body unchanged. Refs #22 --- backend/app/integrations/snipeit/__init__.py | 1 + .../snipeit/plugin.py} | 16 ++++++++++------ backend/app/services/lookup_service.py | 2 +- .../test_snipeit_plugin.py} | 19 +++++++++---------- 4 files changed, 21 insertions(+), 17 deletions(-) create mode 100644 backend/app/integrations/snipeit/__init__.py rename backend/app/{services/snipeit_client.py => integrations/snipeit/plugin.py} (85%) rename backend/tests/unit/{services/test_snipeit_client.py => integrations/test_snipeit_plugin.py} (88%) diff --git a/backend/app/integrations/snipeit/__init__.py b/backend/app/integrations/snipeit/__init__.py new file mode 100644 index 0000000..4d109df --- /dev/null +++ b/backend/app/integrations/snipeit/__init__.py @@ -0,0 +1 @@ +"""Snipe-IT integration plugin.""" diff --git a/backend/app/services/snipeit_client.py b/backend/app/integrations/snipeit/plugin.py similarity index 85% rename from backend/app/services/snipeit_client.py rename to backend/app/integrations/snipeit/plugin.py index 7290758..6882949 100644 --- a/backend/app/services/snipeit_client.py +++ b/backend/app/integrations/snipeit/plugin.py @@ -21,15 +21,19 @@ class SnipeITNotFoundError(AppLookupNotFoundError): """Raised when no Snipe-IT asset matches the given tag.""" -class SnipeITClient: - """Async client for Snipe-IT's REST API. +class SnipeITPlugin: + """Snipe-IT integration plugin. - Authenticates with a bearer token (Snipe-IT API key). Configuration — - base URL, API key, timeout — is injected so the same class can hit the - user's live instance from production and a respx-mocked endpoint from - tests, with no hidden global state. + Implements the IntegrationPlugin protocol — exposes `name`, + `display_name`, and an async `lookup` resolving asset_tag → LabelData. + Configuration (base URL, API key, timeout) is injected so the same + instance can hit the user's live Snipe-IT from production and a + respx-mocked endpoint from tests, with no hidden global state. """ + name = "snipeit" + display_name = "Snipe-IT" + def __init__( self, *, diff --git a/backend/app/services/lookup_service.py b/backend/app/services/lookup_service.py index e058c86..b9a8697 100644 --- a/backend/app/services/lookup_service.py +++ b/backend/app/services/lookup_service.py @@ -27,7 +27,7 @@ class _LookupClient(Protocol): """Minimal contract every per-app client satisfies. - SnipeITClient.lookup, GrocyClient.lookup, SpoolmanClient.lookup all + SnipeITPlugin.lookup, GrocyPlugin.lookup, SpoolmanPlugin.lookup all match this shape — `Protocol` lets us depend on the method without importing concrete classes (avoids a cycle and keeps tests trivial). """ diff --git a/backend/tests/unit/services/test_snipeit_client.py b/backend/tests/unit/integrations/test_snipeit_plugin.py similarity index 88% rename from backend/tests/unit/services/test_snipeit_client.py rename to backend/tests/unit/integrations/test_snipeit_plugin.py index f6e1e84..68f452c 100644 --- a/backend/tests/unit/services/test_snipeit_client.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -1,7 +1,7 @@ import httpx import pytest import respx -from app.services.snipeit_client import SnipeITClient, SnipeITNotFoundError +from app.integrations.snipeit.plugin import SnipeITNotFoundError, SnipeITPlugin def test_not_found_error_is_app_lookup_not_found() -> None: @@ -11,7 +11,6 @@ def test_not_found_error_is_app_lookup_not_found() -> None: """ from app.services.errors import AppLookupNotFoundError from app.services.grocy_client import GrocyNotFoundError - from app.services.snipeit_client import SnipeITNotFoundError from app.services.spoolman_client import SpoolmanNotFoundError assert issubclass(SnipeITNotFoundError, AppLookupNotFoundError) @@ -34,7 +33,7 @@ async def test_lookup_asset_returns_label_data() -> None: ) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") data = await client.lookup("ASSET-12345") assert data.title == "MacBook Pro 16" @@ -51,7 +50,7 @@ async def test_lookup_asset_404_raises_not_found() -> None: return_value=httpx.Response(404) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") with pytest.raises(SnipeITNotFoundError, match="UNKNOWN"): await client.lookup("UNKNOWN") @@ -68,7 +67,7 @@ async def test_lookup_asset_without_serial_has_no_secondary_line() -> None: ) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") data = await client.lookup("A-1") assert data.secondary == () @@ -81,7 +80,7 @@ async def test_lookup_strips_trailing_slash_from_base_url() -> None: respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(200, json={"id": 1, "asset_tag": "A-1", "name": "Thing"}) ) - client = SnipeITClient(base_url="https://snipe-it.example/", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example/", api_key="test-key") data = await client.lookup("A-1") assert data.qr_payload == "https://snipe-it.example/hardware/1" @@ -99,7 +98,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: json={"asset_tag": "A-1", "name": "Broken Asset"}, # no 'id' ) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("A-1") @@ -111,7 +110,7 @@ async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(500) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") with pytest.raises(httpx.HTTPStatusError): await client.lookup("A-1") @@ -126,7 +125,7 @@ async def test_lookup_url_encodes_asset_tag() -> None: json={"id": 1, "asset_tag": "A/1 test", "name": "Thing"}, ) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") data = await client.lookup("A/1 test") assert data.title == "Thing" @@ -138,7 +137,7 @@ async def test_lookup_sends_bearer_auth_header() -> None: route = respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(200, json={"id": 1, "asset_tag": "A-1", "name": "T"}) ) - client = SnipeITClient(base_url="https://snipe-it.example", api_key="secret-key-42") + client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="secret-key-42") await client.lookup("A-1") assert route.called From 2ec83b11b8dfd813c0e8015be851558152f0c278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 12:59:14 +0000 Subject: [PATCH 08/16] refactor(integrations): relocate spoolman client to plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renames SpoolmanClient → SpoolmanPlugin and adds name/display_name attributes so the class satisfies the IntegrationPlugin Protocol. File moves preserve git history. lookup_service.py has no direct SpoolmanClient import — it uses the _LookupClient structural Protocol — so no alias shim is needed there. Test imports use SpoolmanPlugin directly (clean, no alias) since all call-site names were updated in place. Also fixes test_snipeit_plugin.py: the cross-app inheritance test (test_not_found_error_is_app_lookup_not_found) imported SpoolmanNotFoundError from the now-deleted app.services.spoolman_client path; updated to use app.integrations.spoolman.plugin. Refs #22 --- backend/app/integrations/spoolman/__init__.py | 1 + .../spoolman/plugin.py} | 14 ++++++++++-- .../unit/integrations/test_snipeit_plugin.py | 2 +- .../test_spoolman_plugin.py} | 22 +++++++++---------- 4 files changed, 25 insertions(+), 14 deletions(-) create mode 100644 backend/app/integrations/spoolman/__init__.py rename backend/app/{services/spoolman_client.py => integrations/spoolman/plugin.py} (86%) rename backend/tests/unit/{services/test_spoolman_client.py => integrations/test_spoolman_plugin.py} (85%) diff --git a/backend/app/integrations/spoolman/__init__.py b/backend/app/integrations/spoolman/__init__.py new file mode 100644 index 0000000..65172fb --- /dev/null +++ b/backend/app/integrations/spoolman/__init__.py @@ -0,0 +1 @@ +"""Spoolman integration plugin.""" diff --git a/backend/app/services/spoolman_client.py b/backend/app/integrations/spoolman/plugin.py similarity index 86% rename from backend/app/services/spoolman_client.py rename to backend/app/integrations/spoolman/plugin.py index fe583f1..67df2ea 100644 --- a/backend/app/services/spoolman_client.py +++ b/backend/app/integrations/spoolman/plugin.py @@ -22,8 +22,18 @@ class SpoolmanNotFoundError(AppLookupNotFoundError): """Raised when no Spoolman spool matches the given id.""" -class SpoolmanClient: - """Async client for Spoolman's REST API.""" +class SpoolmanPlugin: + """Spoolman integration plugin. + + Implements the IntegrationPlugin protocol — exposes `name`, + `display_name`, and an async `lookup` resolving spool-id → LabelData. + Configuration (base URL, optional API key, timeout) is injected the + same way as SnipeITPlugin so production and respx-mocked tests use + the same instance without hidden global state. + """ + + name = "spoolman" + display_name = "Spoolman" def __init__( self, diff --git a/backend/tests/unit/integrations/test_snipeit_plugin.py b/backend/tests/unit/integrations/test_snipeit_plugin.py index 68f452c..56a4781 100644 --- a/backend/tests/unit/integrations/test_snipeit_plugin.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -9,9 +9,9 @@ def test_not_found_error_is_app_lookup_not_found() -> None: Ensures the aggregator can catch any client's not-found in a single clause. """ + from app.integrations.spoolman.plugin import SpoolmanNotFoundError from app.services.errors import AppLookupNotFoundError from app.services.grocy_client import GrocyNotFoundError - from app.services.spoolman_client import SpoolmanNotFoundError assert issubclass(SnipeITNotFoundError, AppLookupNotFoundError) assert issubclass(GrocyNotFoundError, AppLookupNotFoundError) diff --git a/backend/tests/unit/services/test_spoolman_client.py b/backend/tests/unit/integrations/test_spoolman_plugin.py similarity index 85% rename from backend/tests/unit/services/test_spoolman_client.py rename to backend/tests/unit/integrations/test_spoolman_plugin.py index 46c45a4..51801ed 100644 --- a/backend/tests/unit/services/test_spoolman_client.py +++ b/backend/tests/unit/integrations/test_spoolman_plugin.py @@ -1,7 +1,7 @@ import httpx import pytest import respx -from app.services.spoolman_client import SpoolmanClient, SpoolmanNotFoundError +from app.integrations.spoolman.plugin import SpoolmanNotFoundError, SpoolmanPlugin @pytest.mark.asyncio @@ -21,7 +21,7 @@ async def test_lookup_spool_returns_label_data() -> None: }, ) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") data = await client.lookup("42") assert data.title == "BambuLab PLA Black" @@ -35,7 +35,7 @@ async def test_lookup_spool_returns_label_data() -> None: @respx.mock async def test_lookup_spool_404_raises() -> None: respx.get("https://spoolman.example/api/v1/spool/999").mock(return_value=httpx.Response(404)) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") with pytest.raises(SpoolmanNotFoundError, match="999"): await client.lookup("999") @@ -49,7 +49,7 @@ async def test_lookup_spool_without_remaining_weight() -> None: json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}}, ) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") data = await client.lookup("1") assert data.secondary == () @@ -63,7 +63,7 @@ async def test_lookup_spool_with_missing_vendor_name() -> None: json={"id": 1, "filament": {"name": "PLA"}}, ) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") data = await client.lookup("1") assert data.title == "Unknown PLA" @@ -76,7 +76,7 @@ async def test_lookup_strips_trailing_slash() -> None: 200, json={"id": 7, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanClient(base_url="https://spoolman.example/") + client = SpoolmanPlugin(base_url="https://spoolman.example/") data = await client.lookup("7") assert data.qr_payload == "https://spoolman.example/spool/show/7" @@ -89,7 +89,7 @@ async def test_lookup_url_encodes_spool_id() -> None: 200, json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") data = await client.lookup("A/1") # If encoding worked the mock matched and we got LabelData back. assert data.source_app == "spoolman" @@ -100,7 +100,7 @@ async def test_lookup_url_encodes_spool_id() -> None: @respx.mock async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock(return_value=httpx.Response(503)) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") with pytest.raises(httpx.HTTPStatusError): await client.lookup("1") @@ -111,7 +111,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock( return_value=httpx.Response(200, json={"filament": {"vendor": {"name": "V"}, "name": "M"}}) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("1") @@ -123,7 +123,7 @@ async def test_lookup_spool_with_null_filament() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock( return_value=httpx.Response(200, json={"id": 1, "filament": None}) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") data = await client.lookup("1") assert data.title == "Unknown Unknown" @@ -137,7 +137,7 @@ async def test_lookup_sends_no_auth_header() -> None: 200, json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanClient(base_url="https://spoolman.example") + client = SpoolmanPlugin(base_url="https://spoolman.example") await client.lookup("1") assert route.called From 103a85a01a136ef03ef4a1fd9224fd31f4bd7622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:03:39 +0000 Subject: [PATCH 09/16] refactor(integrations): relocate grocy client to plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renames GrocyClient → GrocyPlugin and adds name/display_name attributes so the class satisfies the IntegrationPlugin Protocol. File moves preserve git history. With this commit all three integration clients live under app/integrations/ and the cross-app inheritance test in test_snipeit_plugin.py now covers all three plugins through their new import paths. The next step (Phase 6) drops AppLookupService's constructor-injection in favor of IntegrationRegistry lookup. Refs #22 --- backend/app/integrations/grocy/__init__.py | 1 + .../grocy/plugin.py} | 17 +++++++++++++---- .../test_grocy_plugin.py} | 18 +++++++++--------- .../unit/integrations/test_snipeit_plugin.py | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 backend/app/integrations/grocy/__init__.py rename backend/app/{services/grocy_client.py => integrations/grocy/plugin.py} (84%) rename backend/tests/unit/{services/test_grocy_client.py => integrations/test_grocy_plugin.py} (85%) diff --git a/backend/app/integrations/grocy/__init__.py b/backend/app/integrations/grocy/__init__.py new file mode 100644 index 0000000..bb444e4 --- /dev/null +++ b/backend/app/integrations/grocy/__init__.py @@ -0,0 +1 @@ +"""Grocy integration plugin.""" diff --git a/backend/app/services/grocy_client.py b/backend/app/integrations/grocy/plugin.py similarity index 84% rename from backend/app/services/grocy_client.py rename to backend/app/integrations/grocy/plugin.py index 4a8cc3b..90256cb 100644 --- a/backend/app/services/grocy_client.py +++ b/backend/app/integrations/grocy/plugin.py @@ -1,8 +1,8 @@ -"""Grocy REST API client — product lookup by id. +"""Grocy integration plugin — product lookup by id. Grocy uses a custom `GROCY-API-KEY` header (not Bearer) and returns 400 with `{"error_message": "..."}` for missing products instead of 404 — -both quirks are explicit in the client's mapping logic. +both quirks are explicit in the plugin's mapping logic. """ from __future__ import annotations @@ -20,8 +20,17 @@ class GrocyNotFoundError(AppLookupNotFoundError): """Raised when no Grocy product matches the given id.""" -class GrocyClient: - """Async client for Grocy's REST API.""" +class GrocyPlugin: + """Grocy integration plugin. + + Implements the IntegrationPlugin protocol — exposes `name`, + `display_name`, and an async `lookup` resolving product-id → + LabelData. Configuration injection follows the same pattern as + SnipeITPlugin and SpoolmanPlugin. + """ + + name = "grocy" + display_name = "Grocy" def __init__( self, diff --git a/backend/tests/unit/services/test_grocy_client.py b/backend/tests/unit/integrations/test_grocy_plugin.py similarity index 85% rename from backend/tests/unit/services/test_grocy_client.py rename to backend/tests/unit/integrations/test_grocy_plugin.py index 7bef7d0..eae115b 100644 --- a/backend/tests/unit/services/test_grocy_client.py +++ b/backend/tests/unit/integrations/test_grocy_plugin.py @@ -1,7 +1,7 @@ import httpx import pytest import respx -from app.services.grocy_client import GrocyClient, GrocyNotFoundError +from app.integrations.grocy.plugin import GrocyNotFoundError, GrocyPlugin @pytest.mark.asyncio @@ -14,7 +14,7 @@ async def test_lookup_product_returns_label_data() -> None: ) ) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") data = await client.lookup("42") assert data.title == "Milch 1L" @@ -31,7 +31,7 @@ async def test_lookup_product_400_raises_not_found() -> None: respx.get("https://grocy.example/api/objects/products/999").mock( return_value=httpx.Response(400, json={"error_message": "No such product"}) ) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") with pytest.raises(GrocyNotFoundError, match="999"): await client.lookup("999") @@ -43,7 +43,7 @@ async def test_lookup_product_404_also_raises_not_found() -> None: respx.get("https://grocy.example/api/objects/products/999").mock( return_value=httpx.Response(404) ) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") with pytest.raises(GrocyNotFoundError): await client.lookup("999") @@ -54,7 +54,7 @@ async def test_lookup_strips_trailing_slash_from_base_url() -> None: respx.get("https://grocy.example/api/objects/products/7").mock( return_value=httpx.Response(200, json={"id": 7, "name": "X"}) ) - client = GrocyClient(base_url="https://grocy.example/", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example/", api_key="grocy-key") data = await client.lookup("7") assert data.qr_payload == "https://grocy.example/product/7" @@ -65,7 +65,7 @@ async def test_lookup_url_encodes_product_id() -> None: respx.get("https://grocy.example/api/objects/products/A%2F1").mock( return_value=httpx.Response(200, json={"id": 1, "name": "Encoded"}) ) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") data = await client.lookup("A/1") assert data.title == "Encoded" @@ -74,7 +74,7 @@ async def test_lookup_url_encodes_product_id() -> None: @respx.mock async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://grocy.example/api/objects/products/1").mock(return_value=httpx.Response(500)) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") with pytest.raises(httpx.HTTPStatusError): await client.lookup("1") @@ -86,7 +86,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: respx.get("https://grocy.example/api/objects/products/1").mock( return_value=httpx.Response(200, json={"name": "Broken"}) # no id ) - client = GrocyClient(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("1") @@ -98,7 +98,7 @@ async def test_lookup_sends_grocy_api_key_header() -> None: route = respx.get("https://grocy.example/api/objects/products/1").mock( return_value=httpx.Response(200, json={"id": 1, "name": "x"}) ) - client = GrocyClient(base_url="https://grocy.example", api_key="my-grocy-key-42") + client = GrocyPlugin(base_url="https://grocy.example", api_key="my-grocy-key-42") await client.lookup("1") assert route.called diff --git a/backend/tests/unit/integrations/test_snipeit_plugin.py b/backend/tests/unit/integrations/test_snipeit_plugin.py index 56a4781..8f9ec5c 100644 --- a/backend/tests/unit/integrations/test_snipeit_plugin.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -9,9 +9,9 @@ def test_not_found_error_is_app_lookup_not_found() -> None: Ensures the aggregator can catch any client's not-found in a single clause. """ + from app.integrations.grocy.plugin import GrocyNotFoundError from app.integrations.spoolman.plugin import SpoolmanNotFoundError from app.services.errors import AppLookupNotFoundError - from app.services.grocy_client import GrocyNotFoundError assert issubclass(SnipeITNotFoundError, AppLookupNotFoundError) assert issubclass(GrocyNotFoundError, AppLookupNotFoundError) From a496ca7095252fc61f02e66a98242c4710f7d934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:10:54 +0000 Subject: [PATCH 10/16] refactor(integrations): plugins read config from settings module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each plugin's __init__ now takes no arguments and reads its config (base_url, api_key, timeout) from app.config.get_settings(). This is required for the entry_points discovery, which instantiates plugins via plugin_cls() with no args. Three timeout fields were added to Settings (snipeit_timeout, grocy_timeout, spoolman_timeout, all PRINTER_HUB_-prefixed, default 5.0). The snipeit_api_key and grocy_api_key fields are SecretStr — the plugin constructors call .get_secret_value() internally. Test fixtures replace constructor-arg injection with monkeypatch.setenv() + get_settings.cache_clear(). Each module gets an autouse fixture that wires the PRINTER_HUB_* env vars to the fake host used by respx mocks. Tests that need a different value (e.g. trailing-slash, bearer-header) override the env var directly before constructing the plugin. Refs #22 --- backend/app/config.py | 3 ++ backend/app/integrations/grocy/plugin.py | 16 +++----- backend/app/integrations/snipeit/plugin.py | 16 +++----- backend/app/integrations/spoolman/plugin.py | 13 +++--- .../unit/integrations/test_grocy_plugin.py | 38 +++++++++++++---- .../unit/integrations/test_snipeit_plugin.py | 41 +++++++++++++++---- .../unit/integrations/test_spoolman_plugin.py | 36 +++++++++++----- 7 files changed, 109 insertions(+), 54 deletions(-) diff --git a/backend/app/config.py b/backend/app/config.py index f3d8dc4..487d200 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -59,13 +59,16 @@ class Settings(BaseSettings): # Snipe-IT integration (optional) snipeit_url: str = "" snipeit_api_key: SecretStr = SecretStr("") + snipeit_timeout: float = 5.0 # Grocy integration (optional) grocy_url: str = "" grocy_api_key: SecretStr = SecretStr("") + grocy_timeout: float = 5.0 # Spoolman integration (no API key needed) spoolman_url: str = "" + spoolman_timeout: float = 5.0 # Server server_port: int = 8090 diff --git a/backend/app/integrations/grocy/plugin.py b/backend/app/integrations/grocy/plugin.py index 90256cb..4c2244f 100644 --- a/backend/app/integrations/grocy/plugin.py +++ b/backend/app/integrations/grocy/plugin.py @@ -32,16 +32,12 @@ class GrocyPlugin: name = "grocy" display_name = "Grocy" - def __init__( - self, - *, - base_url: str, - api_key: str, - timeout: float = 5.0, - ) -> None: - self._base_url = base_url.rstrip("/") - self._api_key = api_key - self._timeout = timeout + def __init__(self) -> None: + from app.config import get_settings + settings = get_settings() + self._base_url = settings.grocy_url.rstrip("/") + self._api_key = settings.grocy_api_key.get_secret_value() + self._timeout = settings.grocy_timeout async def lookup(self, product_id: str) -> LabelData: """Return LabelData for `product_id`, or raise GrocyNotFoundError.""" diff --git a/backend/app/integrations/snipeit/plugin.py b/backend/app/integrations/snipeit/plugin.py index 6882949..a472ba3 100644 --- a/backend/app/integrations/snipeit/plugin.py +++ b/backend/app/integrations/snipeit/plugin.py @@ -34,16 +34,12 @@ class SnipeITPlugin: name = "snipeit" display_name = "Snipe-IT" - def __init__( - self, - *, - base_url: str, - api_key: str, - timeout: float = 5.0, - ) -> None: - self._base_url = base_url.rstrip("/") - self._api_key = api_key - self._timeout = timeout + def __init__(self) -> None: + from app.config import get_settings + settings = get_settings() + self._base_url = settings.snipeit_url.rstrip("/") + self._api_key = settings.snipeit_api_key.get_secret_value() + self._timeout = settings.snipeit_timeout async def lookup(self, asset_tag: str) -> LabelData: """Return LabelData for `asset_tag`, or raise SnipeITNotFoundError.""" diff --git a/backend/app/integrations/spoolman/plugin.py b/backend/app/integrations/spoolman/plugin.py index 67df2ea..76b5158 100644 --- a/backend/app/integrations/spoolman/plugin.py +++ b/backend/app/integrations/spoolman/plugin.py @@ -35,14 +35,11 @@ class SpoolmanPlugin: name = "spoolman" display_name = "Spoolman" - def __init__( - self, - *, - base_url: str, - timeout: float = 5.0, - ) -> None: - self._base_url = base_url.rstrip("/") - self._timeout = timeout + def __init__(self) -> None: + from app.config import get_settings + settings = get_settings() + self._base_url = settings.spoolman_url.rstrip("/") + self._timeout = settings.spoolman_timeout async def lookup(self, spool_id: str) -> LabelData: """Return LabelData for `spool_id`, or raise SpoolmanNotFoundError.""" diff --git a/backend/tests/unit/integrations/test_grocy_plugin.py b/backend/tests/unit/integrations/test_grocy_plugin.py index eae115b..4aff65d 100644 --- a/backend/tests/unit/integrations/test_grocy_plugin.py +++ b/backend/tests/unit/integrations/test_grocy_plugin.py @@ -3,6 +3,19 @@ import respx from app.integrations.grocy.plugin import GrocyNotFoundError, GrocyPlugin +# --------------------------------------------------------------------------- +# Settings fixture +# --------------------------------------------------------------------------- + +@pytest.fixture(autouse=True) +def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: + """Point the plugin at a fake host. respx mocks the actual HTTP.""" + monkeypatch.setenv("PRINTER_HUB_GROCY_URL", "https://grocy.example") + monkeypatch.setenv("PRINTER_HUB_GROCY_API_KEY", "grocy-key") + monkeypatch.setenv("PRINTER_HUB_GROCY_TIMEOUT", "5.0") + from app.config import get_settings + get_settings.cache_clear() + @pytest.mark.asyncio @respx.mock @@ -14,7 +27,7 @@ async def test_lookup_product_returns_label_data() -> None: ) ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() data = await client.lookup("42") assert data.title == "Milch 1L" @@ -31,7 +44,7 @@ async def test_lookup_product_400_raises_not_found() -> None: respx.get("https://grocy.example/api/objects/products/999").mock( return_value=httpx.Response(400, json={"error_message": "No such product"}) ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() with pytest.raises(GrocyNotFoundError, match="999"): await client.lookup("999") @@ -43,7 +56,7 @@ async def test_lookup_product_404_also_raises_not_found() -> None: respx.get("https://grocy.example/api/objects/products/999").mock( return_value=httpx.Response(404) ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() with pytest.raises(GrocyNotFoundError): await client.lookup("999") @@ -54,7 +67,11 @@ async def test_lookup_strips_trailing_slash_from_base_url() -> None: respx.get("https://grocy.example/api/objects/products/7").mock( return_value=httpx.Response(200, json={"id": 7, "name": "X"}) ) - client = GrocyPlugin(base_url="https://grocy.example/", api_key="grocy-key") + import os + os.environ["PRINTER_HUB_GROCY_URL"] = "https://grocy.example/" + from app.config import get_settings + get_settings.cache_clear() + client = GrocyPlugin() data = await client.lookup("7") assert data.qr_payload == "https://grocy.example/product/7" @@ -65,7 +82,7 @@ async def test_lookup_url_encodes_product_id() -> None: respx.get("https://grocy.example/api/objects/products/A%2F1").mock( return_value=httpx.Response(200, json={"id": 1, "name": "Encoded"}) ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() data = await client.lookup("A/1") assert data.title == "Encoded" @@ -74,7 +91,7 @@ async def test_lookup_url_encodes_product_id() -> None: @respx.mock async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://grocy.example/api/objects/products/1").mock(return_value=httpx.Response(500)) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() with pytest.raises(httpx.HTTPStatusError): await client.lookup("1") @@ -86,7 +103,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: respx.get("https://grocy.example/api/objects/products/1").mock( return_value=httpx.Response(200, json={"name": "Broken"}) # no id ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="grocy-key") + client = GrocyPlugin() with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("1") @@ -95,10 +112,15 @@ async def test_lookup_missing_id_raises_value_error() -> None: @respx.mock async def test_lookup_sends_grocy_api_key_header() -> None: """Outgoing request must carry GROCY-API-KEY header (not Bearer).""" + import os + os.environ["PRINTER_HUB_GROCY_API_KEY"] = "my-grocy-key-42" + from app.config import get_settings + get_settings.cache_clear() + route = respx.get("https://grocy.example/api/objects/products/1").mock( return_value=httpx.Response(200, json={"id": 1, "name": "x"}) ) - client = GrocyPlugin(base_url="https://grocy.example", api_key="my-grocy-key-42") + client = GrocyPlugin() await client.lookup("1") assert route.called diff --git a/backend/tests/unit/integrations/test_snipeit_plugin.py b/backend/tests/unit/integrations/test_snipeit_plugin.py index 8f9ec5c..dfa284b 100644 --- a/backend/tests/unit/integrations/test_snipeit_plugin.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -3,6 +3,20 @@ import respx from app.integrations.snipeit.plugin import SnipeITNotFoundError, SnipeITPlugin +# --------------------------------------------------------------------------- +# Settings fixture — env vars are read by the plugin constructor via get_settings() +# --------------------------------------------------------------------------- + +@pytest.fixture(autouse=True) +def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: + """Point the plugin at a fake host. respx mocks the actual HTTP.""" + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_URL", "https://snipe-it.example") + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_API_KEY", "test-key") + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_TIMEOUT", "5.0") + # Clear the lru_cache so the plugin picks up the patched env vars. + from app.config import get_settings + get_settings.cache_clear() + def test_not_found_error_is_app_lookup_not_found() -> None: """All concrete NotFoundErrors must inherit from AppLookupNotFoundError. @@ -33,7 +47,7 @@ async def test_lookup_asset_returns_label_data() -> None: ) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() data = await client.lookup("ASSET-12345") assert data.title == "MacBook Pro 16" @@ -50,7 +64,7 @@ async def test_lookup_asset_404_raises_not_found() -> None: return_value=httpx.Response(404) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() with pytest.raises(SnipeITNotFoundError, match="UNKNOWN"): await client.lookup("UNKNOWN") @@ -67,7 +81,7 @@ async def test_lookup_asset_without_serial_has_no_secondary_line() -> None: ) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() data = await client.lookup("A-1") assert data.secondary == () @@ -80,7 +94,13 @@ async def test_lookup_strips_trailing_slash_from_base_url() -> None: respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(200, json={"id": 1, "asset_tag": "A-1", "name": "Thing"}) ) - client = SnipeITPlugin(base_url="https://snipe-it.example/", api_key="test-key") + # Trailing slash is injected via env var — the plugin must strip it. + import os + + from app.config import get_settings + os.environ["PRINTER_HUB_SNIPEIT_URL"] = "https://snipe-it.example/" + get_settings.cache_clear() + client = SnipeITPlugin() data = await client.lookup("A-1") assert data.qr_payload == "https://snipe-it.example/hardware/1" @@ -98,7 +118,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: json={"asset_tag": "A-1", "name": "Broken Asset"}, # no 'id' ) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("A-1") @@ -110,7 +130,7 @@ async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(500) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() with pytest.raises(httpx.HTTPStatusError): await client.lookup("A-1") @@ -125,7 +145,7 @@ async def test_lookup_url_encodes_asset_tag() -> None: json={"id": 1, "asset_tag": "A/1 test", "name": "Thing"}, ) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="test-key") + client = SnipeITPlugin() data = await client.lookup("A/1 test") assert data.title == "Thing" @@ -134,10 +154,15 @@ async def test_lookup_url_encodes_asset_tag() -> None: @respx.mock async def test_lookup_sends_bearer_auth_header() -> None: """lookup() must send Authorization: Bearer … and Accept: application/json.""" + import os + os.environ["PRINTER_HUB_SNIPEIT_API_KEY"] = "secret-key-42" + from app.config import get_settings + get_settings.cache_clear() + route = respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(200, json={"id": 1, "asset_tag": "A-1", "name": "T"}) ) - client = SnipeITPlugin(base_url="https://snipe-it.example", api_key="secret-key-42") + client = SnipeITPlugin() await client.lookup("A-1") assert route.called diff --git a/backend/tests/unit/integrations/test_spoolman_plugin.py b/backend/tests/unit/integrations/test_spoolman_plugin.py index 51801ed..2f9c57e 100644 --- a/backend/tests/unit/integrations/test_spoolman_plugin.py +++ b/backend/tests/unit/integrations/test_spoolman_plugin.py @@ -3,6 +3,18 @@ import respx from app.integrations.spoolman.plugin import SpoolmanNotFoundError, SpoolmanPlugin +# --------------------------------------------------------------------------- +# Settings fixture +# --------------------------------------------------------------------------- + +@pytest.fixture(autouse=True) +def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: + """Point the plugin at a fake host. respx mocks the actual HTTP.""" + monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_URL", "https://spoolman.example") + monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_TIMEOUT", "5.0") + from app.config import get_settings + get_settings.cache_clear() + @pytest.mark.asyncio @respx.mock @@ -21,7 +33,7 @@ async def test_lookup_spool_returns_label_data() -> None: }, ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() data = await client.lookup("42") assert data.title == "BambuLab PLA Black" @@ -35,7 +47,7 @@ async def test_lookup_spool_returns_label_data() -> None: @respx.mock async def test_lookup_spool_404_raises() -> None: respx.get("https://spoolman.example/api/v1/spool/999").mock(return_value=httpx.Response(404)) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() with pytest.raises(SpoolmanNotFoundError, match="999"): await client.lookup("999") @@ -49,7 +61,7 @@ async def test_lookup_spool_without_remaining_weight() -> None: json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}}, ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() data = await client.lookup("1") assert data.secondary == () @@ -63,7 +75,7 @@ async def test_lookup_spool_with_missing_vendor_name() -> None: json={"id": 1, "filament": {"name": "PLA"}}, ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() data = await client.lookup("1") assert data.title == "Unknown PLA" @@ -76,7 +88,11 @@ async def test_lookup_strips_trailing_slash() -> None: 200, json={"id": 7, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example/") + import os + os.environ["PRINTER_HUB_SPOOLMAN_URL"] = "https://spoolman.example/" + from app.config import get_settings + get_settings.cache_clear() + client = SpoolmanPlugin() data = await client.lookup("7") assert data.qr_payload == "https://spoolman.example/spool/show/7" @@ -89,7 +105,7 @@ async def test_lookup_url_encodes_spool_id() -> None: 200, json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() data = await client.lookup("A/1") # If encoding worked the mock matched and we got LabelData back. assert data.source_app == "spoolman" @@ -100,7 +116,7 @@ async def test_lookup_url_encodes_spool_id() -> None: @respx.mock async def test_lookup_5xx_raises_httpx_error() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock(return_value=httpx.Response(503)) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() with pytest.raises(httpx.HTTPStatusError): await client.lookup("1") @@ -111,7 +127,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock( return_value=httpx.Response(200, json={"filament": {"vendor": {"name": "V"}, "name": "M"}}) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() with pytest.raises(ValueError, match="missing required field 'id'"): await client.lookup("1") @@ -123,7 +139,7 @@ async def test_lookup_spool_with_null_filament() -> None: respx.get("https://spoolman.example/api/v1/spool/1").mock( return_value=httpx.Response(200, json={"id": 1, "filament": None}) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() data = await client.lookup("1") assert data.title == "Unknown Unknown" @@ -137,7 +153,7 @@ async def test_lookup_sends_no_auth_header() -> None: 200, json={"id": 1, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - client = SpoolmanPlugin(base_url="https://spoolman.example") + client = SpoolmanPlugin() await client.lookup("1") assert route.called From 2ded455e566d05f6764446ee6236a8e0fd38ba34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:11:02 +0000 Subject: [PATCH 11/16] refactor(integrations): replace AppLookupService constructor-injection with Registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the _AppName Literal, AVAILABLE_APPS constant, _LookupClient Protocol, and the three-positional-argument constructor. Service now dispatches through IntegrationRegistry.get() — new plugins registered via entry_points become available without touching this module. UnknownAppError wraps IntegrationNotFoundError and preserves the operationally distinct contract: unknown app = caller bug (not data state). The error message propagates from the Registry so it naturally lists registered plugin names. The available_apps property delegates to IntegrationRegistry.names(), returning a sorted list of currently registered plugin names. Test suite replaces MagicMock constructor injection with a _StubPlugin fixture that populates and cleans the Registry around each test. All seven prior test cases are preserved as equivalent Registry-based tests. Refs #22 --- backend/app/services/lookup_service.py | 78 +++------- .../unit/services/test_lookup_service.py | 143 +++++++----------- 2 files changed, 82 insertions(+), 139 deletions(-) diff --git a/backend/app/services/lookup_service.py b/backend/app/services/lookup_service.py index b9a8697..2417097 100644 --- a/backend/app/services/lookup_service.py +++ b/backend/app/services/lookup_service.py @@ -1,77 +1,47 @@ -"""Aggregator that routes lookup requests to the right per-app client. +"""Aggregator that routes lookup requests to the right integration plugin. -The service does not know any client's internals — it just dispatches by -`source_app` to a registered async lookup callable. New apps (e.g. -OpenFoodFacts) plug in by extending the constructor and the `_AppName` -literal. +The service uses IntegrationRegistry as its source of truth. New apps +register themselves via entry_points (group 'label_hub.integrations') and +become available without touching this module. UnknownAppError signals a configuration mismatch (the caller asked for an app that wasn't registered). It deliberately does NOT inherit from AppLookupNotFoundError — the two failure modes are operationally distinct: - UnknownAppError: "you misconfigured the request" -- AppLookupNotFoundError (from any client): "the entity doesn't exist" +- AppLookupNotFoundError (from any plugin): "the entity doesn't exist" """ from __future__ import annotations -from typing import Literal, Protocol, cast, get_args - +from app.integrations.registry import ( + IntegrationNotFoundError, + IntegrationRegistry, +) from app.schemas.label_data import LabelData -_AppName = Literal["snipeit", "grocy", "spoolman"] - -AVAILABLE_APPS: tuple[_AppName, ...] = get_args(_AppName) - - -class _LookupClient(Protocol): - """Minimal contract every per-app client satisfies. - - SnipeITPlugin.lookup, GrocyPlugin.lookup, SpoolmanPlugin.lookup all - match this shape — `Protocol` lets us depend on the method without - importing concrete classes (avoids a cycle and keeps tests trivial). - """ - - async def lookup(self, identifier: str) -> LabelData: ... - class UnknownAppError(Exception): - """Raised when `source_app` does not match any registered client.""" + """Raised when `source_app` does not match any registered plugin.""" class AppLookupService: - """Route `lookup(source_app, id)` to the right per-app client.""" - - def __init__( - self, - *, - snipeit: _LookupClient, - grocy: _LookupClient, - spoolman: _LookupClient, - ) -> None: - self._clients: dict[_AppName, _LookupClient] = { - "snipeit": snipeit, - "grocy": grocy, - "spoolman": spoolman, - } - # Computed once at construction — _clients never mutates after __init__. - self.available_apps: tuple[_AppName, ...] = tuple(sorted(self._clients)) + """Route `lookup(source_app, id)` through IntegrationRegistry.""" async def lookup(self, source_app: str, identifier: str) -> LabelData: - """Dispatch to `source_app`'s client. - - `source_app` is validated against the registry at runtime. The - `_AppName` Literal exists for static-analysis tooling only and does - NOT restrict what strings callers may pass — UnknownAppError covers - the runtime mismatch case. + """Dispatch to the registered plugin for `source_app`. - Raises UnknownAppError if `source_app` is not registered. Any - AppLookupNotFoundError from the underlying client propagates + Raises UnknownAppError if no plugin is registered. Any + AppLookupNotFoundError raised by the underlying plugin propagates unchanged so callers can catch it uniformly. """ - client = self._clients.get(cast(_AppName, source_app)) - if client is None: - raise UnknownAppError( - f"Unknown app {source_app!r}. Available: {list(self.available_apps)}" - ) - return await client.lookup(identifier) + try: + plugin = IntegrationRegistry.get(source_app) + except IntegrationNotFoundError as e: + raise UnknownAppError(str(e)) from e + return await plugin.lookup(identifier) + + @property + def available_apps(self) -> list[str]: + """Names of currently registered plugins, sorted alphabetically.""" + return IntegrationRegistry.names() diff --git a/backend/tests/unit/services/test_lookup_service.py b/backend/tests/unit/services/test_lookup_service.py index 3ffd838..e3ecff4 100644 --- a/backend/tests/unit/services/test_lookup_service.py +++ b/backend/tests/unit/services/test_lookup_service.py @@ -1,120 +1,93 @@ -from unittest.mock import AsyncMock, MagicMock +"""Tests for AppLookupService — Registry-based dispatch.""" + +from collections.abc import Generator import pytest +from app.integrations.registry import IntegrationRegistry from app.schemas.label_data import LabelData -from app.services.errors import AppLookupNotFoundError -from app.services.lookup_service import ( - AVAILABLE_APPS, - AppLookupService, - UnknownAppError, -) - - -def _make_service( - *, - snipeit: MagicMock | None = None, - grocy: MagicMock | None = None, - spoolman: MagicMock | None = None, -) -> AppLookupService: - """Build a service with MagicMock defaults — tests override what they need.""" - return AppLookupService( - snipeit=snipeit or MagicMock(), - grocy=grocy or MagicMock(), - spoolman=spoolman or MagicMock(), - ) +from app.services.lookup_service import AppLookupService, UnknownAppError -@pytest.mark.asyncio -async def test_lookup_routes_snipeit() -> None: - snipeit = MagicMock() - snipeit.lookup = AsyncMock( - return_value=LabelData( - title="MacBook", - primary_id="ASSET-1", - qr_payload="https://snipe.example/h/1", - source_app="snipeit", +class _StubPlugin: + def __init__(self, name: str) -> None: + self.name = name + self.display_name = name.title() + + async def lookup(self, identifier: str) -> LabelData: + return LabelData( + title=f"stub-{self.name}", + primary_id=identifier, + qr_payload=f"https://example/{identifier}", + source_app=self.name, ) - ) - service = _make_service(snipeit=snipeit) - data = await service.lookup("snipeit", "ASSET-1") - snipeit.lookup.assert_awaited_once_with("ASSET-1") - assert data.title == "MacBook" +@pytest.fixture(autouse=True) +def _populate_registry() -> Generator[None, None, None]: + IntegrationRegistry._plugins.clear() + IntegrationRegistry.register(_StubPlugin("snipeit")) + IntegrationRegistry.register(_StubPlugin("spoolman")) + IntegrationRegistry.register(_StubPlugin("grocy")) + yield + IntegrationRegistry._plugins.clear() @pytest.mark.asyncio -async def test_lookup_routes_grocy() -> None: - grocy = MagicMock() - grocy.lookup = AsyncMock( - return_value=LabelData(title="Milch", primary_id="42", qr_payload="x", source_app="grocy") - ) - service = _make_service(grocy=grocy) +async def test_lookup_dispatches_to_registered_plugin() -> None: + svc = AppLookupService() + result = await svc.lookup("snipeit", "ASSET-1") + assert result.source_app == "snipeit" + assert result.primary_id == "ASSET-1" - data = await service.lookup("grocy", "42") - grocy.lookup.assert_awaited_once_with("42") - assert data.title == "Milch" +@pytest.mark.asyncio +async def test_lookup_routes_grocy() -> None: + svc = AppLookupService() + result = await svc.lookup("grocy", "42") + assert result.source_app == "grocy" + assert result.primary_id == "42" @pytest.mark.asyncio async def test_lookup_routes_spoolman() -> None: - spoolman = MagicMock() - spoolman.lookup = AsyncMock( - return_value=LabelData( - title="BambuLab PLA", primary_id="#7", qr_payload="x", source_app="spoolman" - ) - ) - service = _make_service(spoolman=spoolman) - - data = await service.lookup("spoolman", "7") - - spoolman.lookup.assert_awaited_once_with("7") - assert data.title == "BambuLab PLA" + svc = AppLookupService() + result = await svc.lookup("spoolman", "7") + assert result.source_app == "spoolman" + assert result.primary_id == "7" @pytest.mark.asyncio -async def test_lookup_unknown_app_raises_unknown_app_error() -> None: - service = _make_service() - - with pytest.raises(UnknownAppError, match="bogus"): - await service.lookup("bogus", "x") +async def test_lookup_unknown_app_raises_unknownapperror() -> None: + svc = AppLookupService() + with pytest.raises(UnknownAppError, match="nope"): + await svc.lookup("nope", "x") @pytest.mark.asyncio -async def test_unknown_app_error_message_lists_available_apps() -> None: - service = _make_service() - - with pytest.raises(UnknownAppError) as excinfo: - await service.lookup("bogus", "x") +async def test_lookup_propagates_app_lookup_not_found_unchanged() -> None: + """AppLookupNotFoundError from a plugin must propagate — the service does not swallow it.""" + from app.services.errors import AppLookupNotFoundError - msg = str(excinfo.value) - for app in AVAILABLE_APPS: - assert app in msg, f"Expected {app} in error message, got: {msg}" + class _RaisingPlugin: + name = "failing" + display_name = "Failing" + async def lookup(self, identifier: str) -> LabelData: + raise AppLookupNotFoundError(f"Entity {identifier!r} not found") -@pytest.mark.asyncio -async def test_lookup_propagates_app_lookup_not_found_unchanged() -> None: - """AppLookupNotFoundError from a client must propagate — the aggregator does not swallow it.""" - snipeit = MagicMock() - snipeit.lookup = AsyncMock(side_effect=AppLookupNotFoundError("Asset 'X' not found")) - service = _make_service(snipeit=snipeit) + IntegrationRegistry.register(_RaisingPlugin()) + svc = AppLookupService() with pytest.raises(AppLookupNotFoundError, match="X"): - await service.lookup("snipeit", "X") + await svc.lookup("failing", "X") -def test_available_apps_constant_matches_registered_clients() -> None: - """The exported AVAILABLE_APPS constant must agree with the actual registry.""" - service = _make_service() - assert set(service.available_apps) == set(AVAILABLE_APPS) +def test_available_apps_reflects_registry() -> None: + assert AppLookupService().available_apps == ["grocy", "snipeit", "spoolman"] def test_unknown_app_error_does_not_inherit_from_app_lookup_not_found() -> None: - """UnknownAppError is a configuration mismatch, NOT an entity-not-found. + """UnknownAppError is a configuration mismatch, NOT an entity-not-found.""" + from app.services.errors import AppLookupNotFoundError - The aggregator's clients raise AppLookupNotFoundError for missing entities. - UnknownAppError is operationally distinct (caller bug, not data state) and - must not be confused with it. - """ assert not issubclass(UnknownAppError, AppLookupNotFoundError) From 97e993d13753a5d9eb94fb1cd10e59c281b823fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:16:54 +0000 Subject: [PATCH 12/16] test(integrations): use monkeypatch.setenv instead of direct os.environ writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests previously wrote to os.environ[...] directly. The mutations were not undone at test-end; they only happened to work because the _stub_settings autouse fixture + cache_clear() pattern restored consistent state before the next test. Under pytest-xdist or any test-reordering this would become flaky. Switching to monkeypatch.setenv() makes each test fully self-contained — pytest restores the previous env-var state automatically at teardown. Also removes the in-function `import os` statements that are no longer needed. Refs #22 --- backend/tests/unit/integrations/test_grocy_plugin.py | 10 ++++------ .../tests/unit/integrations/test_snipeit_plugin.py | 11 ++++------- .../tests/unit/integrations/test_spoolman_plugin.py | 5 ++--- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/backend/tests/unit/integrations/test_grocy_plugin.py b/backend/tests/unit/integrations/test_grocy_plugin.py index 4aff65d..56114a4 100644 --- a/backend/tests/unit/integrations/test_grocy_plugin.py +++ b/backend/tests/unit/integrations/test_grocy_plugin.py @@ -63,13 +63,12 @@ async def test_lookup_product_404_also_raises_not_found() -> None: @pytest.mark.asyncio @respx.mock -async def test_lookup_strips_trailing_slash_from_base_url() -> None: +async def test_lookup_strips_trailing_slash_from_base_url(monkeypatch: pytest.MonkeyPatch) -> None: respx.get("https://grocy.example/api/objects/products/7").mock( return_value=httpx.Response(200, json={"id": 7, "name": "X"}) ) - import os - os.environ["PRINTER_HUB_GROCY_URL"] = "https://grocy.example/" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_GROCY_URL", "https://grocy.example/") get_settings.cache_clear() client = GrocyPlugin() data = await client.lookup("7") @@ -110,11 +109,10 @@ async def test_lookup_missing_id_raises_value_error() -> None: @pytest.mark.asyncio @respx.mock -async def test_lookup_sends_grocy_api_key_header() -> None: +async def test_lookup_sends_grocy_api_key_header(monkeypatch: pytest.MonkeyPatch) -> None: """Outgoing request must carry GROCY-API-KEY header (not Bearer).""" - import os - os.environ["PRINTER_HUB_GROCY_API_KEY"] = "my-grocy-key-42" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_GROCY_API_KEY", "my-grocy-key-42") get_settings.cache_clear() route = respx.get("https://grocy.example/api/objects/products/1").mock( diff --git a/backend/tests/unit/integrations/test_snipeit_plugin.py b/backend/tests/unit/integrations/test_snipeit_plugin.py index dfa284b..0eb6c3f 100644 --- a/backend/tests/unit/integrations/test_snipeit_plugin.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -89,16 +89,14 @@ async def test_lookup_asset_without_serial_has_no_secondary_line() -> None: @pytest.mark.asyncio @respx.mock -async def test_lookup_strips_trailing_slash_from_base_url() -> None: +async def test_lookup_strips_trailing_slash_from_base_url(monkeypatch: pytest.MonkeyPatch) -> None: """base_url='https://snipe-it.example/' must not produce a double slash.""" respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( return_value=httpx.Response(200, json={"id": 1, "asset_tag": "A-1", "name": "Thing"}) ) # Trailing slash is injected via env var — the plugin must strip it. - import os - from app.config import get_settings - os.environ["PRINTER_HUB_SNIPEIT_URL"] = "https://snipe-it.example/" + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_URL", "https://snipe-it.example/") get_settings.cache_clear() client = SnipeITPlugin() data = await client.lookup("A-1") @@ -152,11 +150,10 @@ async def test_lookup_url_encodes_asset_tag() -> None: @pytest.mark.asyncio @respx.mock -async def test_lookup_sends_bearer_auth_header() -> None: +async def test_lookup_sends_bearer_auth_header(monkeypatch: pytest.MonkeyPatch) -> None: """lookup() must send Authorization: Bearer … and Accept: application/json.""" - import os - os.environ["PRINTER_HUB_SNIPEIT_API_KEY"] = "secret-key-42" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_API_KEY", "secret-key-42") get_settings.cache_clear() route = respx.get("https://snipe-it.example/api/v1/hardware/bytag/A-1").mock( diff --git a/backend/tests/unit/integrations/test_spoolman_plugin.py b/backend/tests/unit/integrations/test_spoolman_plugin.py index 2f9c57e..010dead 100644 --- a/backend/tests/unit/integrations/test_spoolman_plugin.py +++ b/backend/tests/unit/integrations/test_spoolman_plugin.py @@ -82,15 +82,14 @@ async def test_lookup_spool_with_missing_vendor_name() -> None: @pytest.mark.asyncio @respx.mock -async def test_lookup_strips_trailing_slash() -> None: +async def test_lookup_strips_trailing_slash(monkeypatch: pytest.MonkeyPatch) -> None: respx.get("https://spoolman.example/api/v1/spool/7").mock( return_value=httpx.Response( 200, json={"id": 7, "filament": {"vendor": {"name": "V"}, "name": "M"}} ) ) - import os - os.environ["PRINTER_HUB_SPOOLMAN_URL"] = "https://spoolman.example/" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_URL", "https://spoolman.example/") get_settings.cache_clear() client = SpoolmanPlugin() data = await client.lookup("7") From 75499575c7b05875714d99d052c6b7d93763d14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:18:41 +0000 Subject: [PATCH 13/16] build: register bundled integration plugins as entry_points Snipe-IT, Spoolman, and Grocy plugins are declared under the label_hub.integrations entry-points group, so importing app.integrations at app start triggers automatic discovery and registration via IntegrationRegistry. Third-party plugins follow the same pattern in their own pyproject.toml. Refs #22 --- backend/pyproject.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 0e9723d..810ed62 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -44,6 +44,11 @@ dev = [ "mypy>=1.13", ] +[project.entry-points."label_hub.integrations"] +snipeit = "app.integrations.snipeit.plugin:SnipeITPlugin" +spoolman = "app.integrations.spoolman.plugin:SpoolmanPlugin" +grocy = "app.integrations.grocy.plugin:GrocyPlugin" + [build-system] requires = ["hatchling"] build-backend = "hatchling.build" From 3d9358ae8fcc03bd37a68d6d100a48f8fe1683c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:19:05 +0000 Subject: [PATCH 14/16] docs(integrations): explain plugin contract and how to add integrations README covers three areas: - Adding a bundled plugin (in this repo) - Adding a third-party plugin (external pip package) - Plugin contract, defensive loading, and the test fixture pattern Refs #22 --- backend/app/integrations/README.md | 99 ++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 backend/app/integrations/README.md diff --git a/backend/app/integrations/README.md b/backend/app/integrations/README.md new file mode 100644 index 0000000..0636250 --- /dev/null +++ b/backend/app/integrations/README.md @@ -0,0 +1,99 @@ +# Integration Plugins + +Lookup clients for external apps (Snipe-IT, Spoolman, Grocy, +third-party) live here. Each plugin implements the `IntegrationPlugin` +protocol from `base.py` and registers itself via setuptools +entry-points. + +## Adding a bundled plugin + +1. Create `backend/app/integrations//plugin.py` with a class that + implements `IntegrationPlugin`: + + ```python + from app.schemas.label_data import LabelData + + + class MyAppPlugin: + name = "myapp" + display_name = "My App" + + def __init__(self) -> None: + # Local import avoids a load-time cycle. + from app.config import get_settings + settings = get_settings() + self._base_url = settings.myapp_url.rstrip("/") + + async def lookup(self, identifier: str) -> LabelData: + # Call upstream, build LabelData. + ... + ``` + +2. Register the entry-point in `backend/pyproject.toml`: + + ```toml + [project.entry-points."label_hub.integrations"] + myapp = "app.integrations.myapp.plugin:MyAppPlugin" + ``` + +3. Re-install the package (`pip install -e backend`) and the plugin + loads at app start. `IntegrationRegistry.names()` will include + `"myapp"`. + +## Adding a third-party plugin (external repo) + +External plugins are standalone Python packages. Their own +`pyproject.toml` declares the same entry-points group: + +```toml +[project.entry-points."label_hub.integrations"] +openfoodfacts = "label_hub_openfoodfacts.plugin:OpenFoodFactsPlugin" +``` + +After `pip install label-hub-openfoodfacts` the plugin is registered +the same way bundled plugins are — no Label-Hub repo change needed. + +## Plugin contract + +| Attribute / method | Type | Purpose | +|---|---|---| +| `name` | `str` | Canonical id used in templates (`TemplateSchema.app`) and audit logs. Must be unique across all registered plugins. | +| `display_name` | `str` | UI label, e.g. shown in template-picker dropdowns. | +| `__init__(self)` | `None` | Must accept no positional or keyword arguments — entry-points discovery instantiates plugins with `plugin_cls()`. Read configuration from `app.config.get_settings()` via a local import. | +| `lookup(identifier)` | `async (str) -> LabelData` | Resolves the integration's identifier to a `LabelData`. Raise `AppLookupNotFoundError` (or a subclass) when the entity does not exist. | + +## Defensive loading + +Plugin discovery in `app/integrations/__init__.py` catches and logs +four failure modes so a single broken third-party package cannot +prevent the rest of the application from starting: + +1. `entry_point.load()` raises an exception. +2. The plugin class's `__init__` raises. +3. The loaded object does not satisfy `IntegrationPlugin` (missing + required attributes). +4. The plugin's `name` collides with an already-registered plugin, or + has the wrong type (the registry rejects with + `ValueError`/`TypeError`). + +Failures are logged via `logging` (level: ERROR) with the entry-point +name. Production sysadmins find the broken plugin in their log +aggregator without losing any well-behaved plugins. + +## Testing + +Plugin tests live in +`backend/tests/unit/integrations/test__plugin.py`. Use `respx` +to mock the upstream HTTP layer (`respx` is already a dev dependency +in `pyproject.toml`). + +To exercise plugin configuration via environment variables in tests, +use the `monkeypatch.setenv` pattern + `get_settings.cache_clear()` +in an `autouse=True` fixture: + +```python +@pytest.fixture(autouse=True) +def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PRINTER_HUB_MYAPP_URL", "https://example.test") + get_settings.cache_clear() +``` From 7d75806d206c514127473693b8b6c90c6cae07a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 13:48:03 +0000 Subject: [PATCH 15/16] style(integrations): apply ruff format to all Phase-3.5 files CI's ruff format --check step caught formatting drift in the 12 files added/modified during Phase 3.5. Local pre-checks ran ruff check (lint) but not ruff format (formatting); fixed by running ruff format on the affected files. No logic changes. Refs #22 --- backend/app/integrations/__init__.py | 4 +--- backend/app/integrations/base.py | 2 +- backend/app/integrations/grocy/plugin.py | 1 + backend/app/integrations/registry.py | 8 ++------ backend/app/integrations/snipeit/plugin.py | 1 + backend/app/integrations/spoolman/plugin.py | 1 + backend/tests/unit/integrations/test_base.py | 1 + backend/tests/unit/integrations/test_discovery.py | 3 +-- backend/tests/unit/integrations/test_grocy_plugin.py | 4 ++++ backend/tests/unit/integrations/test_registry.py | 4 +--- backend/tests/unit/integrations/test_snipeit_plugin.py | 4 ++++ backend/tests/unit/integrations/test_spoolman_plugin.py | 3 +++ 12 files changed, 21 insertions(+), 15 deletions(-) diff --git a/backend/app/integrations/__init__.py b/backend/app/integrations/__init__.py index cce82d2..3eb470c 100644 --- a/backend/app/integrations/__init__.py +++ b/backend/app/integrations/__init__.py @@ -64,9 +64,7 @@ def _discover_plugins() -> None: try: IntegrationRegistry.register(instance) except (ValueError, TypeError) as e: - _logger.error( - "Entry-point %r could not register: %s", ep.name, e - ) + _logger.error("Entry-point %r could not register: %s", ep.name, e) _discover_plugins() diff --git a/backend/app/integrations/base.py b/backend/app/integrations/base.py index 2ee8239..9ca94b9 100644 --- a/backend/app/integrations/base.py +++ b/backend/app/integrations/base.py @@ -20,7 +20,7 @@ class IntegrationPlugin(Protocol): """Per-integration lookup contract.""" - name: str # canonical id, e.g. "snipeit" — matches TemplateSchema.app + name: str # canonical id, e.g. "snipeit" — matches TemplateSchema.app display_name: str # UI-friendly, e.g. "Snipe-IT" async def lookup(self, identifier: str) -> LabelData: diff --git a/backend/app/integrations/grocy/plugin.py b/backend/app/integrations/grocy/plugin.py index 4c2244f..8f59fa1 100644 --- a/backend/app/integrations/grocy/plugin.py +++ b/backend/app/integrations/grocy/plugin.py @@ -34,6 +34,7 @@ class GrocyPlugin: def __init__(self) -> None: from app.config import get_settings + settings = get_settings() self._base_url = settings.grocy_url.rstrip("/") self._api_key = settings.grocy_api_key.get_secret_value() diff --git a/backend/app/integrations/registry.py b/backend/app/integrations/registry.py index 45b7c26..ccbc886 100644 --- a/backend/app/integrations/registry.py +++ b/backend/app/integrations/registry.py @@ -32,13 +32,9 @@ def register(cls, plugin: IntegrationPlugin) -> None: f"IntegrationPlugin name must be a string; got {type(plugin.name).__name__}" ) if not plugin.name.strip(): - raise ValueError( - f"IntegrationPlugin requires a non-empty name; got {plugin.name!r}" - ) + raise ValueError(f"IntegrationPlugin requires a non-empty name; got {plugin.name!r}") if plugin.name in cls._plugins: - raise ValueError( - f"Plugin {plugin.name!r} already registered" - ) + raise ValueError(f"Plugin {plugin.name!r} already registered") cls._plugins[plugin.name] = plugin @classmethod diff --git a/backend/app/integrations/snipeit/plugin.py b/backend/app/integrations/snipeit/plugin.py index a472ba3..acb6302 100644 --- a/backend/app/integrations/snipeit/plugin.py +++ b/backend/app/integrations/snipeit/plugin.py @@ -36,6 +36,7 @@ class SnipeITPlugin: def __init__(self) -> None: from app.config import get_settings + settings = get_settings() self._base_url = settings.snipeit_url.rstrip("/") self._api_key = settings.snipeit_api_key.get_secret_value() diff --git a/backend/app/integrations/spoolman/plugin.py b/backend/app/integrations/spoolman/plugin.py index 76b5158..1d73012 100644 --- a/backend/app/integrations/spoolman/plugin.py +++ b/backend/app/integrations/spoolman/plugin.py @@ -37,6 +37,7 @@ class SpoolmanPlugin: def __init__(self) -> None: from app.config import get_settings + settings = get_settings() self._base_url = settings.spoolman_url.rstrip("/") self._timeout = settings.spoolman_timeout diff --git a/backend/tests/unit/integrations/test_base.py b/backend/tests/unit/integrations/test_base.py index 0e2451a..79bde3a 100644 --- a/backend/tests/unit/integrations/test_base.py +++ b/backend/tests/unit/integrations/test_base.py @@ -7,6 +7,7 @@ def test_protocol_is_runtime_checkable() -> None: """isinstance() must work — the registry uses it defensively.""" + class _Fake: name = "x" display_name = "X" diff --git a/backend/tests/unit/integrations/test_discovery.py b/backend/tests/unit/integrations/test_discovery.py index 2f1f555..b773e64 100644 --- a/backend/tests/unit/integrations/test_discovery.py +++ b/backend/tests/unit/integrations/test_discovery.py @@ -135,8 +135,7 @@ async def lookup(self, identifier: str) -> object: # Registry has the first one; the second was skipped with an error assert IntegrationRegistry.names() == ["alpha"] assert any( - "alpha-dup" in r.message or "already registered" in r.message - for r in caplog.records + "alpha-dup" in r.message or "already registered" in r.message for r in caplog.records ) diff --git a/backend/tests/unit/integrations/test_grocy_plugin.py b/backend/tests/unit/integrations/test_grocy_plugin.py index 56114a4..0f51cf8 100644 --- a/backend/tests/unit/integrations/test_grocy_plugin.py +++ b/backend/tests/unit/integrations/test_grocy_plugin.py @@ -7,6 +7,7 @@ # Settings fixture # --------------------------------------------------------------------------- + @pytest.fixture(autouse=True) def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: """Point the plugin at a fake host. respx mocks the actual HTTP.""" @@ -14,6 +15,7 @@ def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("PRINTER_HUB_GROCY_API_KEY", "grocy-key") monkeypatch.setenv("PRINTER_HUB_GROCY_TIMEOUT", "5.0") from app.config import get_settings + get_settings.cache_clear() @@ -68,6 +70,7 @@ async def test_lookup_strips_trailing_slash_from_base_url(monkeypatch: pytest.Mo return_value=httpx.Response(200, json={"id": 7, "name": "X"}) ) from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_GROCY_URL", "https://grocy.example/") get_settings.cache_clear() client = GrocyPlugin() @@ -112,6 +115,7 @@ async def test_lookup_missing_id_raises_value_error() -> None: async def test_lookup_sends_grocy_api_key_header(monkeypatch: pytest.MonkeyPatch) -> None: """Outgoing request must carry GROCY-API-KEY header (not Bearer).""" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_GROCY_API_KEY", "my-grocy-key-42") get_settings.cache_clear() diff --git a/backend/tests/unit/integrations/test_registry.py b/backend/tests/unit/integrations/test_registry.py index f6637ef..610ce9a 100644 --- a/backend/tests/unit/integrations/test_registry.py +++ b/backend/tests/unit/integrations/test_registry.py @@ -16,9 +16,7 @@ def __init__(self, name: str = "fake", display_name: str = "Fake") -> None: self.display_name = display_name async def lookup(self, identifier: str) -> LabelData: - return LabelData( - title="fake", primary_id=identifier, qr_payload="x", source_app=self.name - ) + return LabelData(title="fake", primary_id=identifier, qr_payload="x", source_app=self.name) @pytest.fixture(autouse=True) diff --git a/backend/tests/unit/integrations/test_snipeit_plugin.py b/backend/tests/unit/integrations/test_snipeit_plugin.py index 0eb6c3f..1bdb21f 100644 --- a/backend/tests/unit/integrations/test_snipeit_plugin.py +++ b/backend/tests/unit/integrations/test_snipeit_plugin.py @@ -7,6 +7,7 @@ # Settings fixture — env vars are read by the plugin constructor via get_settings() # --------------------------------------------------------------------------- + @pytest.fixture(autouse=True) def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: """Point the plugin at a fake host. respx mocks the actual HTTP.""" @@ -15,6 +16,7 @@ def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("PRINTER_HUB_SNIPEIT_TIMEOUT", "5.0") # Clear the lru_cache so the plugin picks up the patched env vars. from app.config import get_settings + get_settings.cache_clear() @@ -96,6 +98,7 @@ async def test_lookup_strips_trailing_slash_from_base_url(monkeypatch: pytest.Mo ) # Trailing slash is injected via env var — the plugin must strip it. from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_URL", "https://snipe-it.example/") get_settings.cache_clear() client = SnipeITPlugin() @@ -153,6 +156,7 @@ async def test_lookup_url_encodes_asset_tag() -> None: async def test_lookup_sends_bearer_auth_header(monkeypatch: pytest.MonkeyPatch) -> None: """lookup() must send Authorization: Bearer … and Accept: application/json.""" from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_SNIPEIT_API_KEY", "secret-key-42") get_settings.cache_clear() diff --git a/backend/tests/unit/integrations/test_spoolman_plugin.py b/backend/tests/unit/integrations/test_spoolman_plugin.py index 010dead..6dda4a1 100644 --- a/backend/tests/unit/integrations/test_spoolman_plugin.py +++ b/backend/tests/unit/integrations/test_spoolman_plugin.py @@ -7,12 +7,14 @@ # Settings fixture # --------------------------------------------------------------------------- + @pytest.fixture(autouse=True) def _stub_settings(monkeypatch: pytest.MonkeyPatch) -> None: """Point the plugin at a fake host. respx mocks the actual HTTP.""" monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_URL", "https://spoolman.example") monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_TIMEOUT", "5.0") from app.config import get_settings + get_settings.cache_clear() @@ -89,6 +91,7 @@ async def test_lookup_strips_trailing_slash(monkeypatch: pytest.MonkeyPatch) -> ) ) from app.config import get_settings + monkeypatch.setenv("PRINTER_HUB_SPOOLMAN_URL", "https://spoolman.example/") get_settings.cache_clear() client = SpoolmanPlugin() From e938ae807b7d02a6d461987da492f0fb29f11db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Strausmann?= Date: Fri, 15 May 2026 14:00:43 +0000 Subject: [PATCH 16/16] fix(integrations): address PR #55 review feedback - README: document the config.py step for bundled plugins and the external-plugin-config pattern for third-party plugins (Gemini) - lookup_service.py: side-effect-import app.integrations to ensure the registry is populated even when main.py is not on the call path (Copilot) - registry.py: reject plugin names with leading/trailing whitespace in addition to whitespace-only; whitespace in a canonical id is always wrong (Copilot, plus regression test) - spoolman/plugin.py: docstring no longer claims an api_key field that does not exist (Copilot suppressed) Refs #22 --- backend/app/integrations/README.md | 29 +++++++++++++++++-- backend/app/integrations/registry.py | 10 +++++-- backend/app/integrations/spoolman/plugin.py | 5 ++-- backend/app/services/lookup_service.py | 5 ++++ .../tests/unit/integrations/test_registry.py | 7 +++++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/backend/app/integrations/README.md b/backend/app/integrations/README.md index 0636250..d52ab5c 100644 --- a/backend/app/integrations/README.md +++ b/backend/app/integrations/README.md @@ -29,14 +29,28 @@ entry-points. ... ``` -2. Register the entry-point in `backend/pyproject.toml`: +2. Add the plugin's configuration fields to `backend/app/config.py`: + + ```python + class Settings(BaseSettings): + # ... existing fields ... + myapp_url: str = "" + myapp_api_key: SecretStr = SecretStr("") + myapp_timeout: float = 5.0 + ``` + + Settings reads them from environment variables prefixed with + `PRINTER_HUB_` (e.g. `PRINTER_HUB_MYAPP_URL`). Match the field + names you read in the plugin constructor. + +3. Register the entry-point in `backend/pyproject.toml`: ```toml [project.entry-points."label_hub.integrations"] myapp = "app.integrations.myapp.plugin:MyAppPlugin" ``` -3. Re-install the package (`pip install -e backend`) and the plugin +4. Re-install the package (`pip install -e backend`) and the plugin loads at app start. `IntegrationRegistry.names()` will include `"myapp"`. @@ -53,6 +67,17 @@ openfoodfacts = "label_hub_openfoodfacts.plugin:OpenFoodFactsPlugin" After `pip install label-hub-openfoodfacts` the plugin is registered the same way bundled plugins are — no Label-Hub repo change needed. +Third-party plugins cannot add fields to the core `app.config.Settings` +class — it uses `extra="ignore"`, and pip-installed packages cannot +modify the host application's source. External plugins should manage +their own configuration: read environment variables directly (with +`os.environ` or a local `pydantic_settings.BaseSettings` subclass), +or accept a dedicated env-var prefix (e.g. `LABEL_HUB_OPENFOODFACTS_*`) +that does not collide with the core `PRINTER_HUB_*` namespace. + +Bundled plugins use `app.config.get_settings()`; third-party plugins +must NOT, because adding a field there requires a core repo change. + ## Plugin contract | Attribute / method | Type | Purpose | diff --git a/backend/app/integrations/registry.py b/backend/app/integrations/registry.py index ccbc886..377658b 100644 --- a/backend/app/integrations/registry.py +++ b/backend/app/integrations/registry.py @@ -25,14 +25,20 @@ class IntegrationRegistry: def register(cls, plugin: IntegrationPlugin) -> None: """Add `plugin` under its `.name`. - Rejects empty, whitespace-only, or non-string names and duplicates. + Rejects empty, whitespace-only, padded, or non-string names and duplicates. """ if not isinstance(plugin.name, str): raise TypeError( f"IntegrationPlugin name must be a string; got {type(plugin.name).__name__}" ) - if not plugin.name.strip(): + stripped = plugin.name.strip() + if not stripped: raise ValueError(f"IntegrationPlugin requires a non-empty name; got {plugin.name!r}") + if stripped != plugin.name: + raise ValueError( + "IntegrationPlugin name must not have leading/trailing whitespace;" + f" got {plugin.name!r}" + ) if plugin.name in cls._plugins: raise ValueError(f"Plugin {plugin.name!r} already registered") cls._plugins[plugin.name] = plugin diff --git a/backend/app/integrations/spoolman/plugin.py b/backend/app/integrations/spoolman/plugin.py index 1d73012..02fad77 100644 --- a/backend/app/integrations/spoolman/plugin.py +++ b/backend/app/integrations/spoolman/plugin.py @@ -27,9 +27,8 @@ class SpoolmanPlugin: Implements the IntegrationPlugin protocol — exposes `name`, `display_name`, and an async `lookup` resolving spool-id → LabelData. - Configuration (base URL, optional API key, timeout) is injected the - same way as SnipeITPlugin so production and respx-mocked tests use - the same instance without hidden global state. + Configuration (base URL + timeout) is read from settings; Spoolman + requires no authentication, so there is no api_key field. """ name = "spoolman" diff --git a/backend/app/services/lookup_service.py b/backend/app/services/lookup_service.py index 2417097..76da871 100644 --- a/backend/app/services/lookup_service.py +++ b/backend/app/services/lookup_service.py @@ -14,6 +14,11 @@ from __future__ import annotations +# Trigger plugin discovery — importing this module guarantees the +# registry is populated regardless of whether main.py has imported +# app.integrations yet. The discovery is idempotent (registry +# rejects duplicates), so multiple imports are safe. +import app.integrations # noqa: F401 from app.integrations.registry import ( IntegrationNotFoundError, IntegrationRegistry, diff --git a/backend/tests/unit/integrations/test_registry.py b/backend/tests/unit/integrations/test_registry.py index 610ce9a..8dac3f7 100644 --- a/backend/tests/unit/integrations/test_registry.py +++ b/backend/tests/unit/integrations/test_registry.py @@ -45,6 +45,13 @@ def test_register_rejects_whitespace_only_name() -> None: IntegrationRegistry.register(p) +def test_register_rejects_padded_name() -> None: + """Canonical ids must not have leading/trailing whitespace.""" + p = _FakePlugin(name=" snipeit ") + with pytest.raises(ValueError, match="leading/trailing whitespace"): + IntegrationRegistry.register(p) + + def test_register_rejects_non_string_name() -> None: p = _FakePlugin() p.name = 42 # type: ignore[assignment]