MongoUnitOfWorkBase Testability & Maintainability Improvements

Summary: Refactor MongoUnitOfWorkBase and MongoContext to improve testability, reduce coupling, and simplify the codebase.

Background

The MongoUnitOfWorkBase class is a core infrastructure component used by both API and Project Management services. Recent work to improve test coverage revealed several design issues that make the class difficult to test, maintain, and reason about.

Current State

• Location: src/libs/mongo/SyRF.Mongo.Common/MongoUnitOfWorkBase.cs
• Lines of Code: ~410
• Test Coverage: ~24% (after recent improvements)
• Consumers: MongoPmUnitOfWork, MongoApiUnitOfWork

Recent Workarounds

To enable testing of the actual class (rather than mock reimplementations), we added:

1. A protected constructor to MongoContext for test subclasses
2. Made GetCollection<T, TId>() virtual for override in tests

These workarounds work but indicate deeper design issues worth addressing.

Problems Identified

1. Tight Coupling to Concrete MongoContext

Current:

public MongoUnitOfWorkBase(MongoContext mongoContext, ...)

Issues:

• Cannot inject a mock without subclassing
• Required workaround: protected constructor + virtual methods
• Violates dependency inversion principle

Proposed:

public interface IMongoContext
{
    IMongoCollection<TAggregateRoot> GetCollection<TAggregateRoot, TId>()
        where TAggregateRoot : IAggregateRoot<TId>
        where TId : IEquatable<TId>, IComparable<TId>;

    IMongoClient MongoClient { get; }
}

public MongoUnitOfWorkBase(IMongoContext mongoContext, ...)

2. Reflection-Based Cache Invalidator Lookup

Current (line 43-49):

protected ICacheInvalidator GetCacheInvalidator<TAggregateRoot, TId>() =>
    (ICacheInvalidator) GetType().GetProperties().Single(
        propertyInfo =>
            typeof(ICrudRepository<TAggregateRoot, TId>).IsAssignableFrom(propertyInfo.PropertyType)
    ).GetValue(this);

Issues:

• Uses reflection on every save operation
• Brittle - fails at runtime if property missing
• Performance overhead
• Hard to understand and debug

Proposed:

// Option A: Store invalidators in dictionary during AddRepository
private readonly Dictionary<Type, ICacheInvalidator> _cacheInvalidators = new();

protected void AddRepository<TAggregateRoot, TId>(ICrudRepository<TAggregateRoot, TId> repository)
    where TAggregateRoot : class, IAggregateRoot<TId>, ICacheInvalidator
{
    _crudRepositoryDictionary.Add(repository);
    _cacheInvalidators[typeof(TAggregateRoot)] = (ICacheInvalidator)repository;
}

protected ICacheInvalidator GetCacheInvalidator<TAggregateRoot, TId>() =>
    _cacheInvalidators[typeof(TAggregateRoot)];

3. Multiple Responsibilities (SRP Violation)

The class currently handles:

• Repository registration and lookup
• Session management
• Single entity save (sync + async)
• Batch entity save (sync + async)
• Batched bulk save with progress
• Event collection and dispatch
• Cache invalidation
• Index initialization
• Mapping creation

Proposed: Extract focused services:

MongoUnitOfWorkBase
├── IEventDispatcher          # Event collection and dispatch
├── ICacheInvalidationService # Cache invalidation logic
└── IBulkWriteService         # Batch save operations

4. Nullable CancellationToken Anti-Pattern

Current:

public async Task BatchedSaveManyAsync<TAggregateRoot, TId>(
    ...,
    CancellationToken? cancellationToken = null,
    ...)
{
    if (cancellationToken != null && cancellationToken.Value.IsCancellationRequested)

Issues:

• Non-standard pattern (should use CancellationToken = default)
• Verbose null checks throughout
• Inconsistent with .NET conventions

Proposed:

public async Task BatchedSaveManyAsync<TAggregateRoot, TId>(
    ...,
    CancellationToken cancellationToken = default,
    ...)
{
    cancellationToken.ThrowIfCancellationRequested();

5. Excessive Method Overloads

Current: 26 public methods, many are <TAggregateRoot> convenience wrappers for <TAggregateRoot, Guid>:

// Pattern repeated many times
public void Save<TAggregateRoot>(...) where TAggregateRoot : class, IAggregateRoot<Guid> =>
    Save<TAggregateRoot, Guid>(...);

public void Save<TAggregateRoot, TId>(...) { /* actual implementation */ }

Issues:

• Code duplication
• Large API surface
• Most usages are Guid anyway

Proposed Options:

1. Keep only generic version - Callers specify <Entity, Guid> explicitly
2. Extension methods - Move convenience overloads to extension methods
3. Default type parameter (C# doesn't support this, but could use different pattern)

6. Sync/Async Code Duplication

Current: Save, SaveAsync, SaveMany, SaveManyAsync, etc. with duplicated logic.

Proposed:

• Keep only async versions (modern .NET best practice)
• Or use sync-over-async carefully if sync is truly needed

7. Dynamic Dispatch in Event Handler

Current (line 407):

_eventManager.DispatchAsync((dynamic) domainEvent, messageSender);

Issues:

• Loses compile-time type safety
• Runtime performance overhead
• Harder to trace through code

Proposed: Use generic dispatch or visitor pattern:

// Option: Generic method with constraint
await _eventManager.DispatchAsync<TEvent>(domainEvent, messageSender)
    where TEvent : IDomainEvent;

// Or: Event handler registry pattern

Implementation Plan

Phase 1: Interface Extraction (Low Risk)

1. Create IMongoContext interface
2. Have MongoContext implement IMongoContext
3. Update MongoUnitOfWorkBase to depend on IMongoContext
4. Update tests to use mock IMongoContext directly

Estimated effort: 1-2 hours Risk: Low - additive change, backward compatible

Phase 2: Remove Reflection (Medium Risk)

1. Add _cacheInvalidators dictionary
2. Populate during AddRepository calls
3. Replace reflection-based lookup
4. Add tests for cache invalidator registration

Estimated effort: 2-3 hours Risk: Medium - changes runtime behavior

Phase 3: Fix CancellationToken Pattern (Low Risk)

1. Change CancellationToken? to CancellationToken = default
2. Update all usages to use standard pattern
3. Use ThrowIfCancellationRequested() instead of manual checks

Estimated effort: 1 hour Risk: Low - mostly mechanical changes

Phase 4: Reduce Method Overloads (Medium Risk)

1. Audit all callers of convenience overloads
2. Decide on approach (remove vs extension methods)
3. Update callers as needed
4. Mark old methods [Obsolete] first if needed

Estimated effort: 2-4 hours (depends on caller count) Risk: Medium - breaking change for consumers

Phase 5: Extract Services (Higher Risk, Optional)

1. Extract IEventDispatcher
2. Extract IBulkWriteService
3. Update DI registration

Estimated effort: 4-6 hours Risk: Higher - significant restructuring

Success Criteria

• Test coverage for MongoUnitOfWorkBase > 60%
• No reflection in hot paths
• IMongoContext interface exists for clean mocking
• Standard CancellationToken pattern used throughout
• Reduced cognitive load when reading the class

Acceptance Criteria for PR

• All existing tests pass
• New tests for refactored code
• No breaking changes to public API (or documented migration path)
• SonarCloud quality gate passes

References

• MongoUnitOfWorkBase.cs
• MongoContext.cs
• MongoUnitOfWorkBaseTests.cs
• MongoPmUnitOfWork.cs