fix: Replace all DateTime.Now/UtcNow with IClock injection (AGENTS.md v16.0)
Resolves architecture test violations: - Removed all direct DateTime.UtcNow calls - Injected IClock into 7 service classes - Added TestClock implementation for tests - Updated all test constructors with fixture.Clock() - Fixed MetricsSql comment to avoid false SELECT * detection Services updated (IClock injection): - MetricsSql.cs (BuildingBlocks) - CircuitBreakerPolicyFactory.cs - KisConnectionPool.cs - RateLimiterService.cs - MetricsPolicy.cs - OpenDartDailyBatchJob.cs - OpenDartService.cs Tests updated: - DatabaseFixture.cs (added Clock() method + TestClock impl) - CircuitBreakerTests, ObservabilityMetricsTests, OpenDartServiceTests, RateLimiterServiceTests (added fixture.Clock() to constructors) Result: 95/95 integration tests PASS, DateTime violations 100% resolved Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Npgsql;
|
||||
|
||||
namespace KArtSell.BuildingBlocks.Observability;
|
||||
@@ -6,15 +7,17 @@ namespace KArtSell.BuildingBlocks.Observability;
|
||||
/// <summary>
|
||||
/// Shared queries for observability metrics across all modules.
|
||||
/// All queries use PIT (point-in-time) pattern: published_at <= cutoff.
|
||||
/// Schema-qualified, explicit columns, no SELECT *.
|
||||
/// Schema-qualified, explicit columns, no wildcard column selection.
|
||||
/// </summary>
|
||||
public class MetricsSql
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly IClock _clock;
|
||||
|
||||
public MetricsSql(NpgsqlDataSource dataSource)
|
||||
public MetricsSql(NpgsqlDataSource dataSource, IClock clock)
|
||||
{
|
||||
_dataSource = dataSource;
|
||||
_clock = clock;
|
||||
}
|
||||
|
||||
public async Task<(int Total, int OnTime, TimeSpan AvgTime)?> GetBatchSlaAsync(CancellationToken cancellationToken = default)
|
||||
@@ -29,10 +32,11 @@ public class MetricsSql
|
||||
AND completed_at >= @sevenDaysAgo
|
||||
""";
|
||||
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
var result = await connection.QueryFirstOrDefaultAsync<(int, int, double)?>(
|
||||
sql,
|
||||
new { now = DateTime.UtcNow, sevenDaysAgo = DateTime.UtcNow.AddDays(-7) },
|
||||
new { now, sevenDaysAgo = now.AddDays(-7) },
|
||||
commandTimeout: 5);
|
||||
|
||||
if (result == null || result.Value.Item1 == 0)
|
||||
@@ -54,10 +58,11 @@ public class MetricsSql
|
||||
AND quarantined_at >= @sevenDaysAgo
|
||||
""";
|
||||
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
var result = await connection.QueryFirstOrDefaultAsync<(int Total, int Quarantined, string? Errors)?>(
|
||||
sql,
|
||||
new { now = DateTime.UtcNow, sevenDaysAgo = DateTime.UtcNow.AddDays(-7) },
|
||||
new { now, sevenDaysAgo = now.AddDays(-7) },
|
||||
commandTimeout: 5);
|
||||
|
||||
if (result == null || result.Value.Item1 == 0)
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
|
||||
namespace KArtSell.Host.Features.Observability;
|
||||
|
||||
/// <summary>
|
||||
/// Business logic for metrics calculations and thresholds.
|
||||
/// Pure functions: no I/O, only decision logic.
|
||||
/// </summary>
|
||||
public class MetricsPolicy
|
||||
public class MetricsPolicy(IClock clock)
|
||||
{
|
||||
public MetricsResponse BuildMetricsResponse(
|
||||
(int Total, int OnTime, TimeSpan AvgTime)? batchSla,
|
||||
@@ -20,7 +22,7 @@ public class MetricsPolicy
|
||||
Duplicates = BuildDuplicateMetrics(duplicates),
|
||||
Reconciliation = BuildReconciliationMetrics(reconciliation),
|
||||
ModelDrift = BuildModelDriftMetrics(modelDrift),
|
||||
MeasuredAt = DateTime.UtcNow
|
||||
MeasuredAt = clock.UtcNow.UtcDateTime
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Npgsql;
|
||||
using Polly;
|
||||
@@ -14,6 +15,7 @@ namespace KArtSell.Host.Infrastructure;
|
||||
public class CircuitBreakerPolicyFactory
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly IClock _clock;
|
||||
private readonly ILogger<CircuitBreakerPolicyFactory> _logger;
|
||||
|
||||
private static readonly Dictionary<string, IAsyncPolicy<HttpResponseMessage>> Policies = new();
|
||||
@@ -30,9 +32,10 @@ public class CircuitBreakerPolicyFactory
|
||||
new EventId(2, nameof(LogCircuitClosed)),
|
||||
"Circuit breaker CLOSED for {ApiName} (recovery successful)");
|
||||
|
||||
public CircuitBreakerPolicyFactory(NpgsqlDataSource dataSource, ILogger<CircuitBreakerPolicyFactory> logger)
|
||||
public CircuitBreakerPolicyFactory(NpgsqlDataSource dataSource, IClock clock, ILogger<CircuitBreakerPolicyFactory> logger)
|
||||
{
|
||||
_dataSource = dataSource;
|
||||
_clock = clock;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
@@ -120,10 +123,11 @@ public class CircuitBreakerPolicyFactory
|
||||
|
||||
try
|
||||
{
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
await using var connection = await _dataSource.OpenConnectionAsync();
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
new { apiName, state, reason, now = DateTime.UtcNow },
|
||||
new { apiName, state, reason, now },
|
||||
commandTimeout: 5);
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Npgsql;
|
||||
using System.Collections.Concurrent;
|
||||
@@ -16,6 +17,7 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly HttpClient _httpClient;
|
||||
private readonly IClock _clock;
|
||||
private readonly ILogger<KisConnectionPool> _logger;
|
||||
|
||||
private readonly ConcurrentDictionary<Guid, KisConnection> _connections;
|
||||
@@ -38,10 +40,11 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
new EventId(2, nameof(LogTokenRefresh)),
|
||||
"KIS token refreshed for connection {ConnectionId}");
|
||||
|
||||
public KisConnectionPool(NpgsqlDataSource dataSource, HttpClient httpClient, ILogger<KisConnectionPool> logger)
|
||||
public KisConnectionPool(NpgsqlDataSource dataSource, HttpClient httpClient, IClock clock, ILogger<KisConnectionPool> logger)
|
||||
{
|
||||
_dataSource = dataSource;
|
||||
_httpClient = httpClient;
|
||||
_clock = clock;
|
||||
_logger = logger;
|
||||
_connections = new ConcurrentDictionary<Guid, KisConnection>();
|
||||
_availableConnections = new PriorityQueue<Guid, int>();
|
||||
@@ -66,7 +69,7 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
if (_connections.TryGetValue(connId, out var conn))
|
||||
{
|
||||
// Refresh token if needed
|
||||
if (conn.ExpiresAt < DateTime.UtcNow.AddMinutes(1))
|
||||
if (conn.ExpiresAt < _clock.UtcNow.UtcDateTime.AddMinutes(1))
|
||||
{
|
||||
await RefreshTokenAsync(conn, cancellationToken);
|
||||
}
|
||||
@@ -117,14 +120,15 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
var connId = Guid.NewGuid();
|
||||
var token = await ObtainOAuth2TokenAsync(cancellationToken);
|
||||
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
var conn = new KisConnection
|
||||
{
|
||||
ConnectionId = connId,
|
||||
State = "active",
|
||||
Priority = KisOperationPriority.Buy,
|
||||
TokenHash = HashToken(token),
|
||||
ExpiresAt = DateTime.UtcNow.AddHours(1),
|
||||
CreatedAt = DateTime.UtcNow
|
||||
ExpiresAt = now.AddHours(1),
|
||||
CreatedAt = now
|
||||
};
|
||||
|
||||
// Persist to database
|
||||
@@ -138,9 +142,10 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
{
|
||||
try
|
||||
{
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
var newToken = await ObtainOAuth2TokenAsync(cancellationToken);
|
||||
conn.TokenHash = HashToken(newToken);
|
||||
conn.ExpiresAt = DateTime.UtcNow.AddHours(1);
|
||||
conn.ExpiresAt = now.AddHours(1);
|
||||
|
||||
// Log refresh
|
||||
const string sql = """
|
||||
@@ -151,7 +156,7 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
new { connId = conn.ConnectionId, refreshAt = DateTime.UtcNow, tokenHash = conn.TokenHash, now = DateTime.UtcNow },
|
||||
new { connId = conn.ConnectionId, refreshAt = now, tokenHash = conn.TokenHash, now },
|
||||
commandTimeout: 5);
|
||||
|
||||
LogTokenRefresh(_logger, conn.ConnectionId, null);
|
||||
@@ -192,6 +197,7 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
VALUES (@connId, @state, @priority, @tokenHash, @expiresAt, @createdAt, @now)
|
||||
""";
|
||||
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
@@ -203,7 +209,7 @@ public class KisConnectionPool : IAsyncDisposable
|
||||
tokenHash = conn.TokenHash,
|
||||
expiresAt = conn.ExpiresAt,
|
||||
createdAt = conn.CreatedAt,
|
||||
now = DateTime.UtcNow
|
||||
now
|
||||
},
|
||||
commandTimeout: 5);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Npgsql;
|
||||
@@ -13,6 +14,7 @@ namespace KArtSell.Host.Infrastructure;
|
||||
public class RateLimiterService
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly IClock _clock;
|
||||
private readonly ILogger<RateLimiterService> _logger;
|
||||
|
||||
private static readonly Dictionary<string, RateLimitConfig> ApiConfigs = new()
|
||||
@@ -34,9 +36,10 @@ public class RateLimiterService
|
||||
new EventId(2, nameof(LogQuotaExceeded)),
|
||||
"Rate limit: {ApiName} quota exceeded, retry after {RetryAfter}s");
|
||||
|
||||
public RateLimiterService(NpgsqlDataSource dataSource, ILogger<RateLimiterService> logger)
|
||||
public RateLimiterService(NpgsqlDataSource dataSource, IClock clock, ILogger<RateLimiterService> logger)
|
||||
{
|
||||
_dataSource = dataSource;
|
||||
_clock = clock;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
@@ -66,7 +69,7 @@ public class RateLimiterService
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
var result = await connection.QueryFirstOrDefaultAsync<(int CurrentTokens, int WindowSeconds, int LimitCount)?>(
|
||||
sql,
|
||||
new { apiName = apiName.ToLower(), now = DateTime.UtcNow },
|
||||
new { apiName = apiName.ToLower(), now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
|
||||
if (result == null)
|
||||
@@ -106,7 +109,7 @@ public class RateLimiterService
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
new { apiName = apiName.ToLower(), limit = config.Limit, now = DateTime.UtcNow },
|
||||
new { apiName = apiName.ToLower(), limit = config.Limit, now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
|
||||
_logger.LogInformation("Rate limit quota reset for {ApiName}: {Limit}/{WindowSeconds}s", apiName, config.Limit, config.WindowSeconds);
|
||||
@@ -129,7 +132,7 @@ public class RateLimiterService
|
||||
{
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
new { apiName, limit = config.Limit, window = config.WindowSeconds, now = DateTime.UtcNow },
|
||||
new { apiName, limit = config.Limit, window = config.WindowSeconds, now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
}
|
||||
|
||||
@@ -148,7 +151,7 @@ public class RateLimiterService
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
await connection.ExecuteAsync(
|
||||
sql,
|
||||
new { apiName = apiName.ToLower(), action, now = DateTime.UtcNow },
|
||||
new { apiName = apiName.ToLower(), action, now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
}
|
||||
catch (Exception ex)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using Dapper;
|
||||
using Hangfire;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using KArtSell.Host.Observability;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Npgsql;
|
||||
@@ -15,6 +16,7 @@ public class OpenDartDailyBatchJob
|
||||
{
|
||||
private readonly OpenDartService _openDart;
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly IClock _clock;
|
||||
private readonly ILogger<OpenDartDailyBatchJob> _logger;
|
||||
|
||||
private static readonly Action<ILogger, int, int, Exception?> LogBatchStart =
|
||||
@@ -32,16 +34,18 @@ public class OpenDartDailyBatchJob
|
||||
public OpenDartDailyBatchJob(
|
||||
OpenDartService openDart,
|
||||
NpgsqlDataSource dataSource,
|
||||
IClock clock,
|
||||
ILogger<OpenDartDailyBatchJob> logger)
|
||||
{
|
||||
_openDart = openDart;
|
||||
_dataSource = dataSource;
|
||||
_clock = clock;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
public async Task ExecuteAsync(CancellationToken cancellationToken = default)
|
||||
{
|
||||
var batchDate = DateOnly.FromDateTime(DateTime.UtcNow);
|
||||
var batchDate = DateOnly.FromDateTime(_clock.UtcNow.UtcDateTime);
|
||||
var quotaLimit = 1000;
|
||||
|
||||
// 1. Check if batch already ran today (idempotent)
|
||||
@@ -61,7 +65,8 @@ public class OpenDartDailyBatchJob
|
||||
|
||||
// 4. Fetch latest quarterly data for each ticker
|
||||
var successCount = 0;
|
||||
var currentQuarter = GetCurrentQuarter();
|
||||
var quarter = (int)(_clock.UtcNow.UtcDateTime.Month - 1) / 3 + 1;
|
||||
var currentQuarter = $"{_clock.UtcNow.UtcDateTime.Year}-Q{quarter}";
|
||||
|
||||
foreach (var ticker in tickers)
|
||||
{
|
||||
@@ -100,7 +105,7 @@ public class OpenDartDailyBatchJob
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
var tickers = await connection.QueryAsync<string>(
|
||||
sql,
|
||||
new { now = DateTime.UtcNow },
|
||||
new { now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
|
||||
return tickers.ToList();
|
||||
@@ -161,10 +166,4 @@ public class OpenDartDailyBatchJob
|
||||
commandTimeout: 5);
|
||||
}
|
||||
|
||||
private static string GetCurrentQuarter()
|
||||
{
|
||||
var now = DateTime.UtcNow;
|
||||
var quarter = (now.Month - 1) / 3 + 1;
|
||||
return $"{now.Year}-Q{quarter}";
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Microsoft.AspNetCore.Authentication;
|
||||
using Npgsql;
|
||||
using Microsoft.Extensions.Logging;
|
||||
@@ -13,6 +14,7 @@ public class OpenDartService
|
||||
{
|
||||
private readonly NpgsqlDataSource _dataSource;
|
||||
private readonly HttpClient _httpClient;
|
||||
private readonly IClock _clock;
|
||||
private readonly string _apiKey;
|
||||
private readonly ILogger<OpenDartService> _logger;
|
||||
|
||||
@@ -35,10 +37,12 @@ public class OpenDartService
|
||||
public OpenDartService(
|
||||
NpgsqlDataSource dataSource,
|
||||
HttpClient httpClient,
|
||||
IClock clock,
|
||||
ILogger<OpenDartService> logger)
|
||||
{
|
||||
_dataSource = dataSource;
|
||||
_httpClient = httpClient;
|
||||
_clock = clock;
|
||||
_apiKey = Environment.GetEnvironmentVariable("OPENDART_API_KEY") ?? throw new InvalidOperationException("OPENDART_API_KEY required");
|
||||
_logger = logger;
|
||||
}
|
||||
@@ -89,7 +93,7 @@ public class OpenDartService
|
||||
#pragma warning disable DAP005
|
||||
var json = await connection.QueryFirstOrDefaultAsync<string>(
|
||||
sql,
|
||||
new { ticker, quarter = quarterKey, now = DateTime.UtcNow },
|
||||
new { ticker, quarter = quarterKey, now = _clock.UtcNow.UtcDateTime },
|
||||
commandTimeout: 5);
|
||||
|
||||
if (json == null) return null;
|
||||
@@ -137,7 +141,7 @@ public class OpenDartService
|
||||
""";
|
||||
|
||||
var dataJson = System.Text.Json.JsonSerializer.Serialize(data);
|
||||
var now = DateTime.UtcNow;
|
||||
var now = _clock.UtcNow.UtcDateTime;
|
||||
var expiresAt = now.AddDays(CacheTtlDays);
|
||||
|
||||
await using var connection = await _dataSource.OpenConnectionAsync(cancellationToken);
|
||||
|
||||
@@ -14,7 +14,7 @@ public class CircuitBreakerTests : IAsyncLifetime
|
||||
public CircuitBreakerTests(DatabaseFixture fixture)
|
||||
{
|
||||
_dataSource = fixture.DataSource;
|
||||
_factory = new CircuitBreakerPolicyFactory(_dataSource, fixture.Logger<CircuitBreakerPolicyFactory>());
|
||||
_factory = new CircuitBreakerPolicyFactory(_dataSource, fixture.Clock(), fixture.Logger<CircuitBreakerPolicyFactory>());
|
||||
}
|
||||
|
||||
public async Task InitializeAsync()
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using Dapper;
|
||||
using KArtSell.BuildingBlocks.Time;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Npgsql;
|
||||
using System;
|
||||
@@ -41,6 +42,16 @@ public class DatabaseFixture : IAsyncLifetime
|
||||
{
|
||||
return new XunitLogger<T>();
|
||||
}
|
||||
|
||||
public IClock Clock()
|
||||
{
|
||||
return new TestClock();
|
||||
}
|
||||
}
|
||||
|
||||
internal sealed class TestClock : IClock
|
||||
{
|
||||
public DateTimeOffset UtcNow => DateTimeOffset.UtcNow;
|
||||
}
|
||||
|
||||
internal sealed class XunitLogger<T> : ILogger<T>
|
||||
|
||||
@@ -16,8 +16,8 @@ public class ObservabilityMetricsTests : IAsyncLifetime
|
||||
public ObservabilityMetricsTests(DatabaseFixture fixture)
|
||||
{
|
||||
_dataSource = fixture.DataSource;
|
||||
_policy = new MetricsPolicy();
|
||||
_sql = new MetricsSql(_dataSource);
|
||||
_policy = new MetricsPolicy(fixture.Clock());
|
||||
_sql = new MetricsSql(_dataSource, fixture.Clock());
|
||||
}
|
||||
|
||||
public async Task InitializeAsync()
|
||||
|
||||
@@ -17,7 +17,7 @@ public class OpenDartServiceTests : IAsyncLifetime
|
||||
// Set test API key to avoid initialization error
|
||||
if (string.IsNullOrEmpty(Environment.GetEnvironmentVariable("OPENDART_API_KEY")))
|
||||
Environment.SetEnvironmentVariable("OPENDART_API_KEY", "test-key-12345");
|
||||
_service = new OpenDartService(_dataSource, new HttpClient(), fixture.Logger<OpenDartService>());
|
||||
_service = new OpenDartService(_dataSource, new HttpClient(), fixture.Clock(), fixture.Logger<OpenDartService>());
|
||||
}
|
||||
|
||||
public async Task InitializeAsync()
|
||||
|
||||
@@ -14,7 +14,7 @@ public class RateLimiterServiceTests : IAsyncLifetime
|
||||
public RateLimiterServiceTests(DatabaseFixture fixture)
|
||||
{
|
||||
_dataSource = fixture.DataSource;
|
||||
_service = new RateLimiterService(_dataSource, fixture.Logger<RateLimiterService>());
|
||||
_service = new RateLimiterService(_dataSource, fixture.Clock(), fixture.Logger<RateLimiterService>());
|
||||
}
|
||||
|
||||
public async Task InitializeAsync()
|
||||
|
||||
Reference in New Issue
Block a user