6.4 KiB
6.4 KiB
Service Dependency Injection Unification - Implementation Plan
Issue #8867: Refactor - Unify Service Initialization and Dependency Injection
Overview
This refactoring unifies service initialization and dependency injection patterns across the codebase, establishing a consistent DI container pattern for all services as specified in ADR-003.
Current State Analysis
PlanService (Inconsistent Pattern)
def __init__(
self,
settings: Settings,
unit_of_work: UnitOfWork,
ai_provider: AIProviderInterface | None = None,
llm: BaseLanguageModel | None = None,
provider_registry: ProviderRegistry | None = None, # Optional - lazy initialized
actor_service: ActorService | None = None, # Optional - lazy initialized
):
# Lazy initialization pattern (ANTI-PATTERN)
self.actor_service = actor_service or ActorService(settings, unit_of_work)
self._provider_registry = provider_registry
Issues:
- Optional dependencies with lazy initialization
- Service locator pattern (creates ActorService internally)
- Lazy initialization of ProviderRegistry via
_get_provider_registry() - Violates ADR-003 explicit dependency injection principle
ProjectService (Canonical Pattern)
def __init__(
self,
settings: Settings,
unit_of_work: UnitOfWork,
event_bus: EventBus | None = None,
):
self.settings = settings
self.unit_of_work = unit_of_work
self._event_bus = event_bus
Strengths:
- Explicit constructor parameters
- No lazy initialization
- Clear dependency declaration
- Follows ADR-003 pattern
Refactoring Plan
Phase 1: PlanService Refactoring
Changes to Constructor:
def __init__(
self,
settings: Settings,
unit_of_work: UnitOfWork,
provider_registry: ProviderRegistry, # Now required
actor_service: ActorService, # Now required
ai_provider: AIProviderInterface | None = None,
llm: BaseLanguageModel | None = None,
):
self.settings = settings
self.unit_of_work = unit_of_work
self._provider_registry: ProviderRegistry = provider_registry # Direct assignment
self.actor_service = actor_service # Direct assignment
self.ai_provider = ai_provider
self._llm = llm
Code Changes:
- Make
provider_registryandactor_servicerequired parameters - Remove lazy initialization:
self.actor_service = actor_service or ActorService(...) - Remove
_get_provider_registry()method - Replace
self._get_provider_registry()calls withself._provider_registry - Update type annotation:
ProviderRegistry | None→ProviderRegistry
Files Modified:
src/cleveragents/application/services/plan_service.py
Phase 2: DI Container Updates
Container Registration Changes:
# In ApplicationContainer or service registration
plan_service = providers.Factory(
PlanService,
settings=settings,
unit_of_work=unit_of_work,
provider_registry=provider_registry, # Explicitly wired
actor_service=actor_service, # Explicitly wired
ai_provider=ai_provider,
llm=llm,
)
Files Modified:
src/cleveragents/application/container.py
Phase 3: BDD Tests
Test Scenarios:
- PlanService receives all dependencies through constructor
- No dependencies are lazily initialized within the service
- All parameters have explicit type annotations
- ActorService is injected as explicit constructor parameter
- ProviderRegistry is injected as explicit constructor parameter
- DI container wires all service dependencies correctly
- Services can be tested with mock dependencies
Files Created:
features/service_dependency_injection.featurefeatures/steps/service_dependency_injection_steps.py
Quality Gates
All changes must pass:
- Linting:
nox -e lint- Code style and formatting - Type Checking:
nox -e typecheck- Pyright static analysis - Unit Tests:
nox -e unit_tests- Behave BDD tests - Integration Tests:
nox -e integration_tests- Robot Framework tests - Coverage:
nox -e coverage_report- >= 97% coverage
Acceptance Criteria
- All services in
src/cleveragents/application/services/have consistent constructor-based DI pattern - PlanService constructor is refactored to remove optional/lazy dependencies
- All required dependencies are injected explicitly
- No service creates its own dependencies internally
- DI container registration is updated to wire all dependencies
- All existing BDD tests pass after refactor
- New BDD scenarios added to cover DI wiring
- Test coverage remains >= 97%
- All nox quality gates pass
Implementation Notes
Key Decisions
- Required vs Optional: Dependencies that were previously optional with lazy initialization are now required, forcing explicit wiring at composition root
- Parameter Order: Required dependencies listed first, optional dependencies last (Python convention)
- Type Annotations: All parameters have explicit type annotations (no
Anyor# type: ignore) - Backward Compatibility: This is a breaking change for direct instantiation, but the DI container handles all wiring
Challenges & Mitigations
-
Circular Dependencies: May arise when making dependencies explicit
- Mitigation: Use interfaces/protocols to break cycles
-
Test Fixtures: Tests that manually instantiate PlanService need updating
- Mitigation: Update test fixtures to provide all required dependencies
-
Legacy Code: Code that relies on lazy initialization patterns
- Mitigation: Update all call sites to use DI container
Related Documentation
- ADR-003: Dependency Injection - Architecture Decision Record
- CONTRIBUTING.md: Commit format and testing requirements
- specification.md: Product specification sections on service initialization
Commit Message
refactor(services): unify service initialization and dependency injection pattern
- Refactor PlanService to use explicit constructor injection
- Make actor_service and provider_registry required dependencies
- Remove lazy initialization patterns (_get_provider_registry method)
- Update DI container to wire all service dependencies
- Add BDD tests to verify DI pattern compliance
- Ensure all services follow consistent DI pattern per ADR-003
Success Criteria
✅ All quality gates passing
✅ All BDD tests passing
✅ Coverage >= 97%
✅ No # type: ignore comments
✅ All services follow canonical DI pattern
✅ PR created and merged