diff --git a/.github/workflows/fw-lite.yaml b/.github/workflows/fw-lite.yaml index 1096b5d193..bd336a4bf6 100644 --- a/.github/workflows/fw-lite.yaml +++ b/.github/workflows/fw-lite.yaml @@ -66,6 +66,13 @@ jobs: - name: Check for stale generated TypeScript types run: task fw-lite:has-stale-generated-types -- --no-build + - name: Set a non-UTC timezone so timezone bugs are observable + # Hosted runners default to UTC, which hides date bugs: the error equals the local UTC + # offset, which is zero on UTC (see #2092). Eastern is non-UTC, observes DST, and has a + # negative offset, so date round-trips run the way real users actually hit them. + shell: pwsh + run: Set-TimeZone -Id "Eastern Standard Time" + - name: Dotnet test # Benchmarks run in a separate Release-mode job — see `benchmark` below. run: dotnet test FwLiteOnly.slnf --logger GitHubActions --no-build -p:BuildAndroid=false --filter "Category!=Benchmark" diff --git a/backend/FwLite/AGENTS.md b/backend/FwLite/AGENTS.md index 4b97232e8d..b935a7c511 100644 --- a/backend/FwLite/AGENTS.md +++ b/backend/FwLite/AGENTS.md @@ -247,6 +247,10 @@ if (entity?.DeletedAt is not null) return; --- +## 🚨 linq2db and timestamps + +linq2db wraps every SQLite timestamp comparison in `strftime('...%f', ...)` — millisecond precision, not configurable. Never filter or compare commit timestamps through `ToLinqToDB()`; use EF for those predicates (full-precision TEXT comparison, like Harmony's own `CrdtRepository`). `OrderBy` on a timestamp column is safe. See `SnapshotAtCommitService.DeleteCommitsAfter`. + ## 🚨 Harmony Projected Tables (`LcmCrdtDbContext`) `LcmCrdtDbContext` DbSets (`Entries`, `Senses`, etc.) are Harmony's **projected snapshot tables**. They contain only the latest, **un-deleted** state. diff --git a/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs new file mode 100644 index 0000000000..5b35773e58 --- /dev/null +++ b/backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs @@ -0,0 +1,112 @@ +using LcmCrdt.Changes; +using LinqToDB; +using LinqToDB.Async; +using LinqToDB.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore; +using SIL.Harmony.Changes; +using SIL.Harmony.Db; +using Xunit.Abstractions; + +namespace LcmCrdt.Tests; + +// Every persisted DateTimeOffset must round-trip to the same UTC instant through BOTH EF Core and +// linq2db. A broken converter shifts the instant by exactly the local UTC offset, so it is invisible +// on UTC (why #2092 shipped); CI runs a non-UTC zone (fw-lite.yaml) and RequireNonUtc() (in setup) +// fails loudly if that regresses. +public class DateTimeOffsetOrmParityTests(ITestOutputHelper output) : IAsyncLifetime, IAsyncDisposable +{ + private MiniLcmApiFixture _fixture = null!; + private HistoryService History => _fixture.GetService(); + private DataModel DataModel => _fixture.DataModel; + private Guid ClientId => _fixture.GetService().ProjectData.ClientId; + private Task NewContext() => + _fixture.GetService>().CreateDbContextAsync(); + + public async Task InitializeAsync() + { + RequireNonUtc(); + _fixture = MiniLcmApiFixture.Create(); + _fixture.LogTo(output); + await _fixture.InitializeAsync(); + } + + public async Task DisposeAsync() => await _fixture.DisposeAsync(); + async ValueTask IAsyncDisposable.DisposeAsync() => await DisposeAsync(); + + [Fact] + public async Task CommitTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() + { + var entryId = Guid.NewGuid(); + var commit = await DataModel.AddChange(ClientId, + new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "hello" } })); + var truthUtc = commit.HybridDateTime.DateTime.UtcDateTime; + + await using var ctx = await NewContext(); + var efTimestamp = (await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), c => c.Id == commit.Id)).HybridDateTime.DateTime; + // Read the column directly through linq2db too: HistoryService uses linq2db today, but if it ever + // switches to EF this keeps the linq2db path (the one that needed the fix) under test. + var l2dbTimestamp = await ctx.Set().Where(c => c.Id == commit.Id) + .ToLinqToDB().Select(c => c.HybridDateTime.DateTime).FirstAsyncLinqToDB(); + var activityTimestamp = (await History.ProjectActivity(0, 1000).ToArrayAsync()).First(a => a.CommitId == commit.Id).Timestamp; + var historyTimestamp = (await History.GetHistory(entryId).ToArrayAsync()).First(h => h.CommitId == commit.Id).Timestamp; + + foreach (var (name, value) in new[] + { ("EF Core", efTimestamp), ("linq2db", l2dbTimestamp), ("ProjectActivity", activityTimestamp), ("GetHistory", historyTimestamp) }) + { + output.WriteLine($"{name}: {value:o}"); + value.UtcDateTime.Should().Be(truthUtc, $"{name} should report the stored UTC instant"); + value.Offset.Should().Be(TimeSpan.Zero, $"{name} should be UTC, not the ambient zone"); + } + } + + [Fact] + public async Task CommentTimestamp_SameUtcInstant_ThroughEfCoreAndLinq2db() + { + // One plain column is representative: every DateTimeOffset column shares the same global converter. + // Set an explicit non-UTC-offset instant so we also verify the write records the value (the API + // return is already round-tripped through the DB) and that it's normalized to UTC. + await _fixture.GetService().UpdateLastUser("tester", Guid.NewGuid().ToString()); + + var created = new DateTimeOffset(2024, 3, 15, 10, 30, 0, TimeSpan.FromHours(5)); + var thread = await _fixture.Api.CreateCommentThread( + new CommentThread { Id = Guid.NewGuid(), SubjectType = SubjectType.Entry, SubjectId = Guid.NewGuid(), CreatedAt = created }, + new UserComment { Id = Guid.NewGuid(), Text = "hi" }); + + await using var ctx = await NewContext(); + var ef = (await EntityFrameworkQueryableExtensions.SingleAsync( + ctx.Set().AsNoTracking(), t => t.Id == thread.Id)).CreatedAt; + var l2db = await ctx.Set().Where(t => t.Id == thread.Id) + .ToLinqToDB().Select(t => t.CreatedAt).FirstAsyncLinqToDB(); + + output.WriteLine($"CommentThread.CreatedAt: EF {ef:o} | linq2db {l2db:o}"); + l2db.UtcDateTime.Should().Be(ef.UtcDateTime, "EF and linq2db must agree"); + ef.UtcDateTime.Should().Be(created.UtcDateTime, "the written instant must be recorded exactly"); + ef.Offset.Should().Be(TimeSpan.Zero, "EF should be UTC"); + l2db.Offset.Should().Be(TimeSpan.Zero, "linq2db should be UTC"); + } + + [Fact] + public async Task DeletedAt_RoundTripsInUtc_ThroughEfCoreAndLinq2db() + { + var entryId = Guid.NewGuid(); + await DataModel.AddChange(ClientId, new CreateEntryChange(new Entry { Id = entryId, LexemeForm = new() { ["en"] = "bye" } })); + var deletedAtUtc = (await DataModel.AddChange(ClientId, new DeleteChange(entryId))).HybridDateTime.DateTime.UtcDateTime; + + await using var ctx = await NewContext(); + // DeletedAt lives in the snapshot's entity JSON; find the deleted snapshot via the EntityIsDeleted column. + var deletedSnapshot = ctx.Set().AsNoTracking().Where(s => s.EntityId == entryId && s.EntityIsDeleted); + var ef = (await EntityFrameworkQueryableExtensions.SingleAsync(deletedSnapshot)).Entity.DeletedAt!.Value; + var l2db = (await deletedSnapshot.ToLinqToDB().FirstAsyncLinqToDB()).Entity.DeletedAt!.Value; + + output.WriteLine($"DeletedAt: EF {ef:o} | linq2db {l2db:o}"); + ef.UtcDateTime.Should().Be(deletedAtUtc); + l2db.UtcDateTime.Should().Be(deletedAtUtc); + ef.Offset.Should().Be(TimeSpan.Zero); + l2db.Offset.Should().Be(TimeSpan.Zero); + } + + private static void RequireNonUtc() => + TimeZoneInfo.Local.GetUtcOffset(DateTimeOffset.UtcNow).Should().NotBe(TimeSpan.Zero, + "this test only observes timezone bugs under a non-UTC zone — CI sets one in fw-lite.yaml; set your local timezone to run it"); +} diff --git a/backend/FwLite/LcmCrdt/HistoryService.cs b/backend/FwLite/LcmCrdt/HistoryService.cs index 2c6407a7c9..3c002a4c18 100644 --- a/backend/FwLite/LcmCrdt/HistoryService.cs +++ b/backend/FwLite/LcmCrdt/HistoryService.cs @@ -140,7 +140,7 @@ public async IAsyncEnumerable ProjectActivity(int skip = 0, int var queryable = from commit in commits.Skip(skip).Take(take) select new ProjectActivity(commit.Id, - NormalizeTimestamp(commit.HybridDateTime.DateTime), + commit.HybridDateTime.DateTime, commit.ChangeEntities.ToList(), commit.Metadata); await foreach (var projectActivity in queryable.ToLinqToDB().AsAsyncEnumerable()) @@ -277,7 +277,7 @@ from change in changeEntities.LeftJoin(c => #pragma warning restore CS8073 // The result of the expression is always the same since a value of this type is never equal to 'null' select new HistoryLineItem(commit, entityId, - NormalizeTimestamp(commit.HybridDateTime.DateTime), + commit.HybridDateTime.DateTime, snapshot.Id, change.Index, change, @@ -361,13 +361,6 @@ private async IAsyncEnumerable GetAffectedEntryIds(ChangeEntity c } } - internal static DateTimeOffset NormalizeTimestamp(DateTimeOffset timestamp) - { - // Linq2DB materializes datetime columns as local time; reinterpret the captured ticks as UTC to avoid DST offsets. - // see: https://github.com/sillsdev/languageforge-lexbox/issues/2092 - return new DateTimeOffset(timestamp.Ticks, TimeSpan.Zero); - } - public static string ChangesNameHelper(List> changeEntities) { return changeEntities switch diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 970116a127..fafb129290 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -130,11 +130,14 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .UseLinqToDbCrdt(provider) .UseLinqToDB(optionsBuilder => { - var mappingSchema = new MappingSchema(); - new FluentMappingBuilder(mappingSchema).HasAttribute(new ColumnAttribute("DateTime", - nameof(Commit.HybridDateTime) + "." + nameof(HybridDateTime.DateTime))) - .HasAttribute(new ColumnAttribute(nameof(HybridDateTime.Counter), - nameof(Commit.HybridDateTime) + "." + nameof(HybridDateTime.Counter))) + // Extend the mapping schema UseLinqToDbCrdt (above) registered: it configures Harmony's + // Commit.HybridDateTime.DateTime UTC conversion there, and a fresh schema would shadow it, + // making linq2db read commit timestamps in local time (issue #2092). A null schema means + // that invariant broke, so fail loudly rather than silently regressing. + var mappingSchema = optionsBuilder.DbContextOptions.GetLinqToDBOptions()?.ConnectionOptions.MappingSchema + ?? throw new InvalidOperationException( + "linq2db mapping schema was not registered by UseLinqToDbCrdt; Harmony's Commit UTC conversion would be missing (issue #2092)."); + new FluentMappingBuilder(mappingSchema) //tells linq2db to rewrite Sense.SemanticDomainRows / Entry.PublishInRows into //Json.Query(). The rewrite lives on the *Rows shadow accessors //rather than the real IList columns; see Entry.PublishInRows for why. @@ -151,7 +154,6 @@ public static void ConfigureDbOptions(IServiceProvider provider, DbContextOption .Build(); mappingSchema.SetConvertExpression((WritingSystemId id) => new DataParameter { Value = id.Code, DataType = DataType.Text }); - optionsBuilder.AddMappingSchema(mappingSchema); optionsBuilder.AddCustomOptions(options => options.UseSQLite()); // Register read-relevant interceptors for LinqToDB diff --git a/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs b/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs index 6130acfa79..ddf2ec2f47 100644 --- a/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs +++ b/backend/FwLite/LcmCrdt/SnapshotAtCommitService.cs @@ -91,16 +91,23 @@ private async Task ForkDatabase(string sourcePath, string destPath) private async Task DeleteCommitsAfter(ICrdtDbContext context, Commit targetCommit, bool preserveAllFieldWorksCommits) { - var commitsToDelete = context.Commits.WhereAfter(targetCommit); + // WhereAfter must run through EF (like Harmony's own CrdtRepository): linq2db wraps SQLite timestamp + // comparisons in strftime, which is millisecond-grained, so it cannot order commits exactly. + // Only the AuthorName lookup needs linq2db (Json.Value has no EF translation), and it filters on + // commit ids, not timestamps. + var idsAfter = await context.Commits.WhereAfter(targetCommit).Select(c => c.Id).ToArrayAsync(); if (preserveAllFieldWorksCommits) { - commitsToDelete = commitsToDelete.ToLinqToDB().Where(c => - // JSON Sqlite gotcha: null != "FieldWorks" == false (apparently) - (Json.Value(c.Metadata, m => m.AuthorName) ?? "") != "FieldWorks"); + var fieldWorksIds = await context.Commits.ToLinqToDB() + .Where(c => idsAfter.Contains(c.Id) && + // JSON Sqlite gotcha: null != "FieldWorks" == false (apparently) + (Json.Value(c.Metadata, m => m.AuthorName) ?? "") == "FieldWorks") + .Select(c => c.Id) + .ToArrayAsyncLinqToDB(); + idsAfter = idsAfter.Except(fieldWorksIds).ToArray(); } - var count = await commitsToDelete.CountAsyncLinqToDB(); - context.Set().RemoveRange(commitsToDelete); + context.Set().RemoveRange(context.Set().Where(c => idsAfter.Contains(c.Id))); await context.SaveChangesAsync(); - return count; + return idsAfter.Length; } }