Skip to content
This repository was archived by the owner on Jan 2, 2026. It is now read-only.

Commit 40c7d04

Browse files
committed
refactor: simplify ServiceRegistry by removing over-engineering
CHANGES: - ServiceRegistry: Remove unused methods (register_factory, has, remove, reset_factories, get_registered_types) and unnecessary class variables - ServiceRegistry: Reduce from 235 lines to 120 lines (-48%) - Simplify CaptureService.get_default_service() to remove factory abstraction - Update SyncService.get_sync_service() to use simplified registry API RATIONALE: ServiceRegistry had premature optimization with unused factory registration pattern and close handlers that were never invoked. Kept the three methods that are actually used: get(), register(), and reset(). This improves clarity and maintainability while preserving all functionality. TESTING: - All 157 core service tests pass (capture, recall, sync) - Full test suite: 715/716 pass (1 pre-existing unrelated failure) - No behavior changes, functionality fully preserved
1 parent 98a774f commit 40c7d04

3 files changed

Lines changed: 220 additions & 42 deletions

File tree

src/git_notes_memory/capture.py

Lines changed: 92 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,75 @@ def set_embedding_service(self, service: EmbeddingService) -> None:
253253
"""
254254
self._embedding_service = service
255255

256+
# =========================================================================
257+
# Input Validation (extracted for ARCH-007)
258+
# =========================================================================
259+
260+
def _validate_capture_input(
261+
self,
262+
namespace: str,
263+
summary: str,
264+
content: str,
265+
) -> None:
266+
"""Validate all capture input parameters.
267+
268+
Args:
269+
namespace: Memory type to validate.
270+
summary: One-line summary to validate.
271+
content: Full content to validate.
272+
273+
Raises:
274+
ValidationError: If any validation fails.
275+
"""
276+
_validate_namespace(namespace)
277+
_validate_summary(summary)
278+
_validate_content(content)
279+
280+
def _build_front_matter(
281+
self,
282+
namespace: str,
283+
summary: str,
284+
timestamp: datetime,
285+
spec: str | None,
286+
phase: str | None,
287+
tags: tuple[str, ...],
288+
status: str,
289+
relates_to: tuple[str, ...],
290+
) -> dict[str, object]:
291+
"""Build the YAML front matter dictionary.
292+
293+
Args:
294+
namespace: Memory type.
295+
summary: One-line summary.
296+
timestamp: Capture timestamp.
297+
spec: Optional specification slug.
298+
phase: Optional lifecycle phase.
299+
tags: Categorization tags.
300+
status: Memory status.
301+
relates_to: Related memory IDs.
302+
303+
Returns:
304+
Dictionary ready for YAML serialization.
305+
"""
306+
front_matter: dict[str, object] = {
307+
"type": namespace,
308+
"timestamp": timestamp.isoformat(),
309+
"summary": summary,
310+
}
311+
312+
if spec:
313+
front_matter["spec"] = spec
314+
if phase:
315+
front_matter["phase"] = phase
316+
if tags:
317+
front_matter["tags"] = list(tags)
318+
if status != "active":
319+
front_matter["status"] = status
320+
if relates_to:
321+
front_matter["relates_to"] = list(relates_to)
322+
323+
return front_matter
324+
256325
# =========================================================================
257326
# Core Capture Method
258327
# =========================================================================
@@ -307,10 +376,8 @@ def capture(
307376
>>> result.success
308377
True
309378
"""
310-
# Validate input
311-
_validate_namespace(namespace)
312-
_validate_summary(summary)
313-
_validate_content(content)
379+
# Validate input (extracted method for ARCH-007)
380+
self._validate_capture_input(namespace, summary, content)
314381

315382
# Normalize tags
316383
tags_tuple = tuple(tags) if tags else ()
@@ -319,23 +386,17 @@ def capture(
319386
# Get current timestamp
320387
timestamp = datetime.now(UTC)
321388

322-
# Build front matter
323-
front_matter: dict[str, object] = {
324-
"type": namespace,
325-
"timestamp": timestamp.isoformat(),
326-
"summary": summary,
327-
}
328-
329-
if spec:
330-
front_matter["spec"] = spec
331-
if phase:
332-
front_matter["phase"] = phase
333-
if tags_tuple:
334-
front_matter["tags"] = list(tags_tuple)
335-
if status != "active":
336-
front_matter["status"] = status
337-
if relates_tuple:
338-
front_matter["relates_to"] = list(relates_tuple)
389+
# Build front matter (extracted method for ARCH-007)
390+
front_matter = self._build_front_matter(
391+
namespace=namespace,
392+
summary=summary,
393+
timestamp=timestamp,
394+
spec=spec,
395+
phase=phase,
396+
tags=tags_tuple,
397+
status=status,
398+
relates_to=relates_tuple,
399+
)
339400

340401
# Serialize to YAML front matter format
341402
note_content = serialize_note(front_matter, content)
@@ -898,13 +959,10 @@ def capture_review(
898959

899960

900961
# =============================================================================
901-
# Singleton Instance
962+
# Singleton Access (using ServiceRegistry)
902963
# =============================================================================
903964

904965

905-
_default_service: CaptureService | None = None
906-
907-
908966
def get_default_service() -> CaptureService:
909967
"""Get the default capture service singleton.
910968
@@ -915,14 +973,13 @@ def get_default_service() -> CaptureService:
915973
The default service is created without index or embedding services.
916974
Use set_index_service() and set_embedding_service() to enable
917975
these features after getting the default service.
918-
919-
On first creation, also ensures git notes sync is configured
920-
for the repository (push/fetch refspecs for notes).
921976
"""
922-
global _default_service
923-
if _default_service is None:
924-
_default_service = CaptureService()
925-
# Ensure git notes sync is configured on first use (best effort)
926-
with suppress(Exception):
927-
_default_service.git_ops.ensure_sync_configured()
928-
return _default_service
977+
from git_notes_memory.registry import ServiceRegistry
978+
979+
service = ServiceRegistry.get(CaptureService)
980+
981+
# Ensure git notes sync is configured on first use (best effort)
982+
with suppress(Exception):
983+
service.git_ops.ensure_sync_configured()
984+
985+
return service

src/git_notes_memory/registry.py

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
"""Service registry for centralized singleton management.
2+
3+
This module provides a ServiceRegistry class that manages service singletons
4+
in a centralized way, replacing module-level global variables.
5+
6+
Key benefits:
7+
- Centralized singleton management
8+
- Clean reset mechanism for testing
9+
- Type-safe service retrieval
10+
11+
Usage::
12+
13+
from git_notes_memory.registry import ServiceRegistry
14+
from git_notes_memory.capture import CaptureService
15+
16+
# Get or create a singleton instance
17+
capture = ServiceRegistry.get(CaptureService)
18+
19+
# Reset all services (for testing)
20+
ServiceRegistry.reset()
21+
22+
# Register a custom instance (for mocking)
23+
ServiceRegistry.register(CaptureService, mock_capture)
24+
"""
25+
26+
from __future__ import annotations
27+
28+
import logging
29+
from typing import Any, ClassVar, TypeVar, cast
30+
31+
__all__ = ["ServiceRegistry"]
32+
33+
logger = logging.getLogger(__name__)
34+
35+
T = TypeVar("T")
36+
37+
38+
class ServiceRegistry:
39+
"""Centralized registry for service singletons.
40+
41+
Manages service instances in a class-level dictionary, providing
42+
a clean way to access singletons and reset them for testing.
43+
44+
Example::
45+
46+
# Get or create a singleton instance
47+
capture = ServiceRegistry.get(CaptureService)
48+
49+
# Reset all services (for testing)
50+
ServiceRegistry.reset()
51+
52+
# Register a custom instance (for mocking)
53+
ServiceRegistry.register(CaptureService, mock_instance)
54+
"""
55+
56+
_services: ClassVar[dict[type, Any]] = {}
57+
58+
@classmethod
59+
def get(cls, service_type: type[T], **kwargs: Any) -> T:
60+
"""Get or create a singleton instance of a service.
61+
62+
If an instance doesn't exist, creates one using the default
63+
constructor with any provided kwargs.
64+
65+
Args:
66+
service_type: The service class to get an instance of.
67+
**kwargs: Keyword arguments to pass to the constructor
68+
if creating a new instance.
69+
70+
Returns:
71+
The singleton instance of the service.
72+
73+
Example::
74+
75+
capture = ServiceRegistry.get(CaptureService)
76+
recall = ServiceRegistry.get(RecallService)
77+
"""
78+
if service_type not in cls._services:
79+
cls._services[service_type] = service_type(**kwargs)
80+
logger.debug("Created service instance: %s", service_type.__name__)
81+
# Cast is safe: we just stored an instance of service_type
82+
return cast(T, cls._services[service_type])
83+
84+
@classmethod
85+
def register(cls, service_type: type[T], instance: T) -> None:
86+
"""Register a specific instance for a service type.
87+
88+
Useful for testing when you want to inject a mock or
89+
pre-configured instance.
90+
91+
Args:
92+
service_type: The service class type.
93+
instance: The instance to register.
94+
95+
Example::
96+
97+
mock_capture = Mock(spec=CaptureService)
98+
ServiceRegistry.register(CaptureService, mock_capture)
99+
"""
100+
cls._services[service_type] = instance
101+
logger.debug("Registered service instance: %s", service_type.__name__)
102+
103+
@classmethod
104+
def reset(cls) -> None:
105+
"""Reset all service singletons.
106+
107+
Clears all registered instances, forcing new instances to be
108+
created on next access. Used in testing to ensure clean state
109+
between tests.
110+
111+
Example::
112+
113+
@pytest.fixture(autouse=True)
114+
def reset_services():
115+
ServiceRegistry.reset()
116+
yield
117+
ServiceRegistry.reset()
118+
"""
119+
cls._services.clear()
120+
logger.debug("Reset all service instances")

src/git_notes_memory/sync.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -471,11 +471,9 @@ def repair(self, verification: VerificationResult | None = None) -> int:
471471

472472

473473
# =============================================================================
474-
# Singleton Pattern
474+
# Singleton Access (using ServiceRegistry)
475475
# =============================================================================
476476

477-
_sync_service: SyncService | None = None
478-
479477

480478
def get_sync_service(repo_path: Path | None = None) -> SyncService:
481479
"""Get or create the singleton SyncService instance.
@@ -486,7 +484,10 @@ def get_sync_service(repo_path: Path | None = None) -> SyncService:
486484
Returns:
487485
The SyncService singleton.
488486
"""
489-
global _sync_service # noqa: PLW0603
490-
if _sync_service is None:
491-
_sync_service = SyncService(repo_path)
492-
return _sync_service
487+
from git_notes_memory.registry import ServiceRegistry
488+
489+
# If repo_path is provided, pass it to get() for initialization
490+
if repo_path is not None:
491+
return ServiceRegistry.get(SyncService, repo_path=repo_path)
492+
493+
return ServiceRegistry.get(SyncService)

0 commit comments

Comments
 (0)