From 3c68269c003f57d46149bc90c0143b0c02e3c1ef Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 13 Aug 2026 16:22:51 +1000 Subject: [PATCH] Ensure UTC DateTime handling for SQL Server and re-enable retry tests SQL Server `datetime2` columns do not store timezone offsets, leading to `DateTimeKind.Unspecified` when retrieved. This adds EF Core value converters to enforce UTC kind on all persisted timestamps and updates acceptance tests to compare UTC instants. These changes allow several previously deferred retry and ingestion acceptance tests to be re-enabled for both SQL Server and PostgreSQL persistence. --- ...eControl.AcceptanceTests.PostgreSql.csproj | 12 ++------ ...ceControl.AcceptanceTests.SqlServer.csproj | 10 +------ ...ing_failed_message_with_missing_headers.cs | 7 +++-- .../SqlServerServiceControlDbContext.cs | 9 ++++++ .../UtcDateTimeConverters.cs | 14 ++++++++++ .../EFCore/DateTimeKindTests.cs | 28 +++++++++++++++++++ 6 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 src/ServiceControl.Persistence.EFCore.SqlServer/UtcDateTimeConverters.cs create mode 100644 src/ServiceControl.Persistence.Tests/EFCore/DateTimeKindTests.cs diff --git a/src/ServiceControl.AcceptanceTests.PostgreSql/ServiceControl.AcceptanceTests.PostgreSql.csproj b/src/ServiceControl.AcceptanceTests.PostgreSql/ServiceControl.AcceptanceTests.PostgreSql.csproj index 74ed777f0b..c873aa8eb7 100644 --- a/src/ServiceControl.AcceptanceTests.PostgreSql/ServiceControl.AcceptanceTests.PostgreSql.csproj +++ b/src/ServiceControl.AcceptanceTests.PostgreSql/ServiceControl.AcceptanceTests.PostgreSql.csproj @@ -52,23 +52,15 @@ - - - - - - + - + - - - \ No newline at end of file diff --git a/src/ServiceControl.AcceptanceTests.SqlServer/ServiceControl.AcceptanceTests.SqlServer.csproj b/src/ServiceControl.AcceptanceTests.SqlServer/ServiceControl.AcceptanceTests.SqlServer.csproj index 5dda932f0e..6677b0b3dd 100644 --- a/src/ServiceControl.AcceptanceTests.SqlServer/ServiceControl.AcceptanceTests.SqlServer.csproj +++ b/src/ServiceControl.AcceptanceTests.SqlServer/ServiceControl.AcceptanceTests.SqlServer.csproj @@ -52,20 +52,12 @@ - - - - - - + - - - diff --git a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_ingesting_failed_message_with_missing_headers.cs b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_ingesting_failed_message_with_missing_headers.cs index cb0b280106..6196d97536 100644 --- a/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_ingesting_failed_message_with_missing_headers.cs +++ b/src/ServiceControl.AcceptanceTests/Recoverability/MessageFailures/When_ingesting_failed_message_with_missing_headers.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Threading.Tasks; using AcceptanceTesting; using AcceptanceTesting.EndpointTemplates; @@ -67,7 +68,7 @@ public async Task Should_include_headers_required_by_ServicePulse() [Test] public async Task TimeSent_should_not_be_casted() { - var sentTime = DateTime.Parse("2014-11-11T02:26:58.000462Z"); + var sentTime = DateTime.Parse("2014-11-11T02:26:58.000462Z", CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind); var context = await Define(c => { @@ -81,7 +82,9 @@ public async Task TimeSent_should_not_be_casted() var failure = context.Failure; Assert.That(failure, Is.Not.Null); - Assert.That(failure.TimeSent, Is.EqualTo(sentTime)); + + // Raven hands back a local DateTime and EF a UTC one, so compare the instant rather than the wall clock. + Assert.That(failure.TimeSent?.ToUniversalTime(), Is.EqualTo(sentTime)); } async Task TryGetFailureFromApi(TestContext context) diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs index 9fff389d1f..3403cd3c6c 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs @@ -1,5 +1,6 @@ namespace ServiceControl.Persistence.EFCore.SqlServer; +using System; using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore; using ServiceControl.MessageFailures; @@ -8,6 +9,14 @@ namespace ServiceControl.Persistence.EFCore.SqlServer; public class SqlServerServiceControlDbContext(DbContextOptions options) : ServiceControlDbContext(options) { + protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder) + { + base.ConfigureConventions(configurationBuilder); + + configurationBuilder.Properties().HaveConversion(); + configurationBuilder.Properties().HaveConversion(); + } + protected override void OnModelCreating(ModelBuilder modelBuilder) { base.OnModelCreating(modelBuilder); diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/UtcDateTimeConverters.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/UtcDateTimeConverters.cs new file mode 100644 index 0000000000..e464acc5b6 --- /dev/null +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/UtcDateTimeConverters.cs @@ -0,0 +1,14 @@ +namespace ServiceControl.Persistence.EFCore.SqlServer; + +using System; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +// datetime2 stores no offset, so values come back as DateTimeKind.Unspecified and are then serialized +// by the API without the UTC marker. Everything persisted here is UTC. +sealed class UtcDateTimeConverter() : ValueConverter( + value => value.Kind == DateTimeKind.Local ? value.ToUniversalTime() : value, + value => DateTime.SpecifyKind(value, DateTimeKind.Utc)); + +sealed class NullableUtcDateTimeConverter() : ValueConverter( + value => value.HasValue && value.Value.Kind == DateTimeKind.Local ? value.Value.ToUniversalTime() : value, + value => value.HasValue ? DateTime.SpecifyKind(value.Value, DateTimeKind.Utc) : value); diff --git a/src/ServiceControl.Persistence.Tests/EFCore/DateTimeKindTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/DateTimeKindTests.cs new file mode 100644 index 0000000000..9f42206a56 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests/EFCore/DateTimeKindTests.cs @@ -0,0 +1,28 @@ +namespace ServiceControl.Persistence.Tests; + +using System; +using System.Threading.Tasks; +using NUnit.Framework; + +class DateTimeKindTests : ErrorIngestionTestBase +{ + [Test] + public async Task Timestamps_are_read_back_as_utc() + { + var failure = new IngestedFailure(); + + await Ingest(failure); + + var row = await GetFailedMessage(failure.UniqueMessageId); + + using (Assert.EnterMultipleScope()) + { + Assert.That(row.TimeSent.Value.Kind, Is.EqualTo(DateTimeKind.Utc)); + Assert.That(row.LastAttemptedAt.Kind, Is.EqualTo(DateTimeKind.Utc)); + Assert.That(row.FirstTimeOfFailure.Kind, Is.EqualTo(DateTimeKind.Utc)); + Assert.That(row.LastTimeOfFailure.Kind, Is.EqualTo(DateTimeKind.Utc)); + Assert.That(row.LastModified.Kind, Is.EqualTo(DateTimeKind.Utc)); + Assert.That(row.StatusChangedAt.Kind, Is.EqualTo(DateTimeKind.Utc)); + } + } +}