Merge pull request #42 from GoodbyePlanet/feat/refactor-factory

GoodbyePlanet · web-flow · commit 16d031540191 · 2026-05-16T13:24:51.000+02:00
feat: Refactor factory to use OCP
diff --git a/server/embeddings/__init__.py b/server/embeddings/__init__.py
@@ -0,0 +1,8 @@
+from server.embeddings.factory import close_embedding_provider, get_embedding_provider
+from . import jina as _jina  # noqa: F401
+from . import jina_api as _jina_api  # noqa: F401
+from . import voyage as _voyage  # noqa: F401
+from . import openai as _openai  # noqa: F401
+from . import ollama as _ollama  # noqa: F401
+
+__all__ = ["get_embedding_provider", "close_embedding_provider"]
diff --git a/server/embeddings/factory.py b/server/embeddings/factory.py
@@ -3,40 +3,27 @@
 from server.config import settings
 from server.embeddings.base import EmbeddingProvider
 
+_registry: dict[str, type[EmbeddingProvider]] = {}
 _provider: EmbeddingProvider | None = None
 
 
+def register(name: str, cls: type[EmbeddingProvider]) -> None:
+    _registry[name] = cls
+
+
 def get_embedding_provider() -> EmbeddingProvider:
     global _provider
     if _provider is not None:
         return _provider
 
     name = settings.embeddings_provider
-    if name == "jina":
-        from server.embeddings.jina import JinaEmbeddingProvider
-
-        _provider = JinaEmbeddingProvider()
-    elif name == "jina-api":
-        from server.embeddings.jina_api import JinaApiEmbeddingProvider
-
-        _provider = JinaApiEmbeddingProvider()
-    elif name == "voyage":
-        from server.embeddings.voyage import VoyageEmbeddingProvider
-
-        _provider = VoyageEmbeddingProvider()
-    elif name == "openai":
-        from server.embeddings.openai import OpenAIEmbeddingProvider
-
-        _provider = OpenAIEmbeddingProvider()
-    elif name == "ollama":
-        from server.embeddings.ollama import OllamaEmbeddingProvider
-
-        _provider = OllamaEmbeddingProvider()
-    else:
+    cls = _registry.get(name)
+    if cls is None:
         raise ValueError(
             f"Unknown EMBEDDINGS_PROVIDER {name!r}. "
-            "Expected one of: jina, jina-api, voyage, openai, ollama."
+            f"Expected one of: {', '.join(sorted(_registry))}."
         )
+    _provider = cls()
     return _provider
 
 
diff --git a/server/embeddings/jina.py b/server/embeddings/jina.py
@@ -58,3 +58,8 @@ async def embed_query(self, text: str) -> list[float]:
 
     async def close(self) -> None:
         await self._client.aclose()
+
+
+from server.embeddings.factory import register  # noqa: E402
+
+register("jina", JinaEmbeddingProvider)
diff --git a/server/embeddings/jina_api.py b/server/embeddings/jina_api.py
@@ -152,3 +152,8 @@ async def _embed(self, texts: list[str], task: str) -> list[list[float]]:
 
     async def close(self) -> None:
         await self._client.aclose()
+
+
+from server.embeddings.factory import register  # noqa: E402
+
+register("jina-api", JinaApiEmbeddingProvider)
diff --git a/server/embeddings/ollama.py b/server/embeddings/ollama.py
@@ -71,3 +71,8 @@ async def embed_query(self, text: str) -> list[float]:
 
     async def close(self) -> None:
         await self._client.aclose()
+
+
+from server.embeddings.factory import register  # noqa: E402
+
+register("ollama", OllamaEmbeddingProvider)
diff --git a/server/embeddings/openai.py b/server/embeddings/openai.py
@@ -94,3 +94,8 @@ async def embed_query(self, text: str) -> list[float]:
 
     async def close(self) -> None:
         await self._client.aclose()
+
+
+from server.embeddings.factory import register  # noqa: E402
+
+register("openai", OpenAIEmbeddingProvider)
diff --git a/server/embeddings/voyage.py b/server/embeddings/voyage.py
@@ -107,3 +107,8 @@ async def _embed(self, texts: list[str], input_type: str) -> list[list[float]]:
 
     async def close(self) -> None:
         await self._client.aclose()
+
+
+from server.embeddings.factory import register  # noqa: E402
+
+register("voyage", VoyageEmbeddingProvider)
diff --git a/server/indexer/pipeline.py b/server/indexer/pipeline.py
@@ -13,7 +13,7 @@
 from server.config import settings
 from server.embeddings.base import EmbeddingProvider
 from server.embeddings.bm25 import BM25SparseProvider, get_sparse_embedding_provider
-from server.embeddings.factory import get_embedding_provider
+from server.embeddings import get_embedding_provider
 from server.indexer.github_source import fetch_blob_content, list_github_files
 from server.parser.base import CodeSymbol
 from server.parser.registry import parse_file
diff --git a/server/main.py b/server/main.py
@@ -10,7 +10,7 @@
 
 from server.config import settings
 from server.embeddings.bm25 import BM25SparseProvider, close_sparse_embedding_provider
-from server.embeddings.factory import close_embedding_provider, get_embedding_provider
+from server.embeddings import close_embedding_provider, get_embedding_provider
 from server.state import (
     get_commit_store,
     get_store,
diff --git a/server/tools/search.py b/server/tools/search.py
@@ -6,7 +6,7 @@
 from mcp.server.fastmcp import FastMCP
 
 from server.config import settings
-from server.embeddings.factory import get_embedding_provider
+from server.embeddings import get_embedding_provider
 from server.indexer.github_source import fetch_file_content
 from server.state import get_sparse_provider, get_store
 
diff --git a/tests/embeddings/test_factory.py b/tests/embeddings/test_factory.py
@@ -0,0 +1,103 @@
+from __future__ import annotations
+
+import pytest
+
+import server.embeddings.factory as factory_module
+from server.embeddings.factory import (
+    close_embedding_provider,
+    get_embedding_provider,
+    register,
+)
+
+
+class _StubProvider:
+    dimensions = 64
+
+    async def embed_batch(self, texts):
+        return [[0.0] * 64] * len(texts)
+
+    async def embed_query(self, text):
+        return [0.0] * 64
+
+
+class _StubProviderWithClose(_StubProvider):
+    def __init__(self):
+        self.closed = False
+
+    async def close(self):
+        self.closed = True
+
+
+@pytest.fixture(autouse=True)
+def reset_factory(monkeypatch):
+    monkeypatch.setattr(factory_module, "_registry", {})
+    monkeypatch.setattr(factory_module, "_provider", None)
+
+
+def test_register_adds_to_registry():
+    register("stub", _StubProvider)
+    assert "stub" in factory_module._registry
+    assert factory_module._registry["stub"] is _StubProvider
+
+
+def test_get_embedding_provider_instantiates_registered_class(monkeypatch):
+    register("stub", _StubProvider)
+    monkeypatch.setattr(
+        "server.embeddings.factory.settings.embeddings_provider", "stub"
+    )
+
+    provider = get_embedding_provider()
+
+    assert isinstance(provider, _StubProvider)
+
+
+def test_get_embedding_provider_returns_singleton(monkeypatch):
+    register("stub", _StubProvider)
+    monkeypatch.setattr(
+        "server.embeddings.factory.settings.embeddings_provider", "stub"
+    )
+
+    assert get_embedding_provider() is get_embedding_provider()
+
+
+def test_get_embedding_provider_raises_for_unknown_name(monkeypatch):
+    register("stub", _StubProvider)
+    monkeypatch.setattr(
+        "server.embeddings.factory.settings.embeddings_provider", "unknown"
+    )
+
+    with pytest.raises(ValueError, match="unknown"):
+        get_embedding_provider()
+
+
+def test_error_message_lists_registered_providers(monkeypatch):
+    register("alpha", _StubProvider)
+    register("beta", _StubProvider)
+    monkeypatch.setattr(
+        "server.embeddings.factory.settings.embeddings_provider", "unknown"
+    )
+
+    with pytest.raises(ValueError, match="alpha"):
+        get_embedding_provider()
+
+
+async def test_close_embedding_provider_calls_close_on_provider(monkeypatch):
+    provider = _StubProviderWithClose()
+    monkeypatch.setattr(factory_module, "_provider", provider)
+
+    await close_embedding_provider()
+
+    assert provider.closed
+    assert factory_module._provider is None
+
+
+async def test_close_embedding_provider_is_noop_when_none():
+    await close_embedding_provider()
+
+
+async def test_close_embedding_provider_skips_close_if_not_defined(monkeypatch):
+    monkeypatch.setattr(factory_module, "_provider", _StubProvider())
+
+    await close_embedding_provider()
+
+    assert factory_module._provider is None
