-
-
Notifications
You must be signed in to change notification settings - Fork 6
Read commit timestamps in UTC via linq2db and remove NormalizeTimestamp #2391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
467b4d0
Add cross-ORM date parity test; run FwLite tests under non-UTC zone
myieye 7c97a72
Reuse Harmony's linq2db schema so NormalizeTimestamp is unneeded
myieye ecd7af4
Cover DeletedAt and de-duplicate the timestamp parity test
myieye ad982a5
Trim parity test to one representative plain column
myieye 64e8450
Tighten comments per review
myieye a7aba06
Address review feedback on date-parity test
myieye 1d8ffe9
Throw when Harmony's linq2db mapping schema is missing
myieye 37a7d3d
Delete commits after a snapshot target via EF, not linq2db
myieye d82cc33
Set a last user in the comment timestamp parity test
myieye File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
112 changes: 112 additions & 0 deletions
112
backend/FwLite/LcmCrdt.Tests/DateTimeOffsetOrmParityTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<HistoryService>(); | ||
| private DataModel DataModel => _fixture.DataModel; | ||
| private Guid ClientId => _fixture.GetService<CurrentProjectService>().ProjectData.ClientId; | ||
| private Task<LcmCrdtDbContext> NewContext() => | ||
| _fixture.GetService<IDbContextFactory<LcmCrdtDbContext>>().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<Commit>().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<Commit>().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<CurrentProjectService>().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<CommentThread>().AsNoTracking(), t => t.Id == thread.Id)).CreatedAt; | ||
| var l2db = await ctx.Set<CommentThread>().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<Entry>(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<ObjectSnapshot>().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"); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.