Files

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:

  1. Make provider_registry and actor_service required parameters
  2. Remove lazy initialization: self.actor_service = actor_service or ActorService(...)
  3. Remove _get_provider_registry() method
  4. Replace self._get_provider_registry() calls with self._provider_registry
  5. Update type annotation: ProviderRegistry | NoneProviderRegistry

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:

  1. PlanService receives all dependencies through constructor
  2. No dependencies are lazily initialized within the service
  3. All parameters have explicit type annotations
  4. ActorService is injected as explicit constructor parameter
  5. ProviderRegistry is injected as explicit constructor parameter
  6. DI container wires all service dependencies correctly
  7. Services can be tested with mock dependencies

Files Created:

  • features/service_dependency_injection.feature
  • features/steps/service_dependency_injection_steps.py

Quality Gates

All changes must pass:

  1. Linting: nox -e lint - Code style and formatting
  2. Type Checking: nox -e typecheck - Pyright static analysis
  3. Unit Tests: nox -e unit_tests - Behave BDD tests
  4. Integration Tests: nox -e integration_tests - Robot Framework tests
  5. 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

  1. Required vs Optional: Dependencies that were previously optional with lazy initialization are now required, forcing explicit wiring at composition root
  2. Parameter Order: Required dependencies listed first, optional dependencies last (Python convention)
  3. Type Annotations: All parameters have explicit type annotations (no Any or # type: ignore)
  4. Backward Compatibility: This is a breaking change for direct instantiation, but the DI container handles all wiring

Challenges & Mitigations

  1. Circular Dependencies: May arise when making dependencies explicit

    • Mitigation: Use interfaces/protocols to break cycles
  2. Test Fixtures: Tests that manually instantiate PlanService need updating

    • Mitigation: Update test fixtures to provide all required dependencies
  3. Legacy Code: Code that relies on lazy initialization patterns

    • Mitigation: Update all call sites to use DI container
  • 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