From 8749ac516ce09c4c36da81e32831ef2a6aefc290 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 30 Oct 2024 14:28:26 +0700 Subject: [PATCH 01/10] fix bug trying to use Commit.DateTime in a query --- src/SIL.Harmony/Db/CrdtRepository.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index a6ed5e1..997bc1a 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -144,7 +144,7 @@ public async Task GetCommitsAfter(Commit? commit) } public async Task FindSnapshot(Guid id, bool tracking = false) - { + { return await Snapshots .AsTracking(tracking) .Include(s => s.Commit) @@ -155,7 +155,7 @@ public async Task GetCommitsAfter(Commit? commit) { return await Snapshots.AsTracking(tracking).Include(s => s.Commit) .DefaultOrder() - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.DateTime <= ignoreChangesAfter)); + .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter)); } public async Task GetObjectBySnapshotId(Guid snapshotId) @@ -172,7 +172,7 @@ public async Task GetObjectBySnapshotId(Guid snapshotId) { var snapshot = await Snapshots .DefaultOrder() - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.DateTime <= ignoreChangesAfter)); + .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter)); return (T?) snapshot?.Entity.DbObject; } @@ -214,7 +214,7 @@ public async ValueTask AddIfNew(IEnumerable snapshots) { foreach (var snapshot in snapshots) { - + if (_dbContext.Snapshots.Local.FindEntry(snapshot.Id) is not null) continue; _dbContext.Add(snapshot); await SnapshotAdded(snapshot); From fcd3568269b2381f20d4765282ef935097d26ea8 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 30 Oct 2024 18:04:22 +0700 Subject: [PATCH 02/10] introduce DataModel.GetAtCommit --- src/SIL.Harmony.Core/QueryHelpers.cs | 19 ++- src/SIL.Harmony.Tests/Db/QueryHelperTests.cs | 160 ++++++++++++++++++- src/SIL.Harmony.Tests/ModelSnapshotTests.cs | 26 +++ src/SIL.Harmony/DataModel.cs | 35 +++- src/SIL.Harmony/Db/CrdtRepository.cs | 70 ++++---- src/SIL.Harmony/Db/DbSetExtensions.cs | 24 ++- 6 files changed, 290 insertions(+), 44 deletions(-) diff --git a/src/SIL.Harmony.Core/QueryHelpers.cs b/src/SIL.Harmony.Core/QueryHelpers.cs index 6f4d565..6562829 100644 --- a/src/SIL.Harmony.Core/QueryHelpers.cs +++ b/src/SIL.Harmony.Core/QueryHelpers.cs @@ -89,10 +89,21 @@ public static IQueryable WhereAfter(this IQueryable queryable, T after) || (after.HybridDateTime.DateTime == c.HybridDateTime.DateTime && after.HybridDateTime.Counter == c.HybridDateTime.Counter && after.Id < c.Id)); } - public static IQueryable WhereBefore(this IQueryable queryable, T after) where T : CommitBase + public static IQueryable WhereBefore(this IQueryable queryable, T before, bool inclusive = false) where T : CommitBase { - return queryable.Where(c => c.HybridDateTime.DateTime < after.HybridDateTime.DateTime - || (c.HybridDateTime.DateTime == after.HybridDateTime.DateTime && c.HybridDateTime.Counter < after.HybridDateTime.Counter) - || (c.HybridDateTime.DateTime == after.HybridDateTime.DateTime && c.HybridDateTime.Counter == after.HybridDateTime.Counter && c.Id < after.Id)); + if (inclusive) + { + + return queryable.Where(c => c.HybridDateTime.DateTime < before.HybridDateTime.DateTime + || (c.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.HybridDateTime.Counter < before.HybridDateTime.Counter) + || (c.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.HybridDateTime.Counter == before.HybridDateTime.Counter && + c.Id < before.Id) + || c.Id == before.Id); + } + return queryable.Where(c => c.HybridDateTime.DateTime < before.HybridDateTime.DateTime + || (c.HybridDateTime.DateTime == before.HybridDateTime.DateTime && c.HybridDateTime.Counter < before.HybridDateTime.Counter) + || (c.HybridDateTime.DateTime == before.HybridDateTime.DateTime && c.HybridDateTime.Counter == before.HybridDateTime.Counter && c.Id < before.Id)); } } diff --git a/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs b/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs index 89f474d..22b2808 100644 --- a/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs +++ b/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs @@ -15,7 +15,8 @@ public class QueryHelperTests private Commit Commit(Guid id, HybridDateTime hybridDateTime) => new(id) { ClientId = Guid.Empty, HybridDateTime = hybridDateTime }; - private ObjectSnapshot Snapshot(Guid commitId, HybridDateTime hybridDateTime) => ObjectSnapshot.ForTesting(Commit(commitId, hybridDateTime)); + private ObjectSnapshot Snapshot(Guid commitId, HybridDateTime hybridDateTime) => + ObjectSnapshot.ForTesting(Commit(commitId, hybridDateTime)); private IQueryable Commits(IEnumerable commits) => commits.AsQueryable(); private IQueryable Snapshots(IEnumerable snapshots) => snapshots.AsQueryable(); @@ -283,6 +284,108 @@ public void WhereAfterSnapshot_FiltersOnId() ]); } + + + [Fact] + public void WhereBeforeSnapshot_FiltersOnDate() + { + var filterCommit = Commit(Guid.NewGuid(), Time(2, 0)); + var snapshots = Snapshots([ + Snapshot(id1, Time(1, 0)), + Snapshot(id2, Time(3, 0)), + Snapshot(id3, Time(4, 0)), + ]); + snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ + id1 + ]); + } + + [Fact] + public void WhereBeforeSnapshotInclusive_FiltersOnDate() + { + var filterCommit = Commit(Guid.NewGuid(), Time(2, 0)); + var snapshots = Snapshots([ + Snapshot(id1, Time(1, 0)), + Snapshot(id2, Time(3, 0)), + Snapshot(id3, Time(4, 0)), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime) + ]); + snapshots.WhereBefore(filterCommit, inclusive: true).Select(c => c.CommitId).Should().BeEquivalentTo([ + id1, + filterCommit.Id + ]); + } + + [Fact] + public void WhereBeforeSnapshot_FiltersOnCounter() + { + var filterCommit = Commit(Guid.NewGuid(), Time(1, 2)); + var snapshots = Snapshots([ + Snapshot(id1, Time(1, 1)), + Snapshot(id2, Time(1, 3)), + Snapshot(id3, Time(1, 4)), + ]); + snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ + id1, + ]); + } + + [Fact] + public void WhereBeforeSnapshotInclusive_FiltersOnCounter() + { + var filterCommit = Commit(Guid.NewGuid(), Time(1, 2)); + var snapshots = Snapshots([ + Snapshot(id1, Time(1, 1)), + Snapshot(id2, Time(1, 3)), + Snapshot(id3, Time(1, 4)), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime), + ]); + snapshots.WhereBefore(filterCommit, inclusive: true).Select(c => c.CommitId).Should().BeEquivalentTo([ + id1, + filterCommit.Id + ]); + } + + [Fact] + public void WhereBeforeSnapshot_FiltersOnId() + { + var hybridDateTime = Time(1, 1); + Guid[] ids = OrderedIds(4); + var commitId1 = ids[0]; + var filterCommit = Commit(ids[1], hybridDateTime); + var commitId2 = ids[2]; + var commitId3 = ids[3]; + var snapshots = Snapshots([ + Snapshot(commitId1, hybridDateTime), + Snapshot(commitId2, hybridDateTime), + Snapshot(commitId3, hybridDateTime), + ]); + snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ + commitId1 + ]); + } + + [Fact] + public void WhereBeforeSnapshotInclusive_FiltersOnId() + { + var hybridDateTime = Time(1, 1); + Guid[] ids = OrderedIds(4); + var commitId1 = ids[0]; + var filterCommit = Commit(ids[1], hybridDateTime); + var commitId2 = ids[2]; + var commitId3 = ids[3]; + var snapshots = Snapshots([ + Snapshot(commitId1, hybridDateTime), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime), + Snapshot(commitId2, hybridDateTime), + Snapshot(commitId3, hybridDateTime), + ]); + snapshots.WhereBefore(filterCommit, inclusive: true).Select(c => c.CommitId).Should().BeEquivalentTo([ + commitId1, + filterCommit.Id + ]); + } + [Fact] public void WhereBeforeCommit_FiltersOnDate() { @@ -297,6 +400,22 @@ public void WhereBeforeCommit_FiltersOnDate() ]); } + [Fact] + public void WhereBeforeCommitInclusive_FiltersOnDate() + { + var filterCommit = Commit(Guid.NewGuid(), Time(2, 0)); + var commits = Commits([ + Commit(id1, Time(1, 0)), + Commit(id2, Time(3, 0)), + Commit(id3, Time(4, 0)), + filterCommit + ]); + commits.WhereBefore(filterCommit, inclusive: true).Select(c => c.Id).Should().BeEquivalentTo([ + id1, + filterCommit.Id + ]); + } + [Fact] public void WhereBeforeCommit_FiltersOnCounter() { @@ -307,7 +426,23 @@ public void WhereBeforeCommit_FiltersOnCounter() Commit(id3, Time(1, 4)), ]); commits.WhereBefore(filterCommit).Select(c => c.Id).Should().BeEquivalentTo([ - id1 + id1, + ]); + } + + [Fact] + public void WhereBeforeCommitInclusive_FiltersOnCounter() + { + var filterCommit = Commit(Guid.NewGuid(), Time(1, 2)); + var commits = Commits([ + Commit(id1, Time(1, 1)), + Commit(id2, Time(1, 3)), + Commit(id3, Time(1, 4)), + filterCommit + ]); + commits.WhereBefore(filterCommit, inclusive: true).Select(c => c.Id).Should().BeEquivalentTo([ + id1, + filterCommit.Id ]); } @@ -329,4 +464,25 @@ public void WhereBeforeCommit_FiltersOnId() commitId1 ]); } + + [Fact] + public void WhereBeforeCommitInclusive_FiltersOnId() + { + var hybridDateTime = Time(1, 1); + Guid[] ids = OrderedIds(4); + var commitId1 = ids[0]; + var filterCommit = Commit(ids[1], hybridDateTime); + var commitId2 = ids[2]; + var commitId3 = ids[3]; + var commits = Commits([ + Commit(commitId1, hybridDateTime), + filterCommit, + Commit(commitId2, hybridDateTime), + Commit(commitId3, hybridDateTime), + ]); + commits.WhereBefore(filterCommit, inclusive: true).Select(c => c.Id).Should().BeEquivalentTo([ + commitId1, + filterCommit.Id + ]); + } } diff --git a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs index 90bed26..55a8902 100644 --- a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs +++ b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs @@ -33,6 +33,32 @@ public async Task ModelSnapshotShowsMultipleChanges() snapshot.LastChange.Should().Be(secondChange.DateTime); } + [Fact] + public async Task CanGetWordForASpecificCommit() + { + var entityId = Guid.NewGuid(); + var firstCommit = await WriteNextChange(SetWord(entityId, "first")); + var secondCommit = await WriteNextChange(SetWord(entityId, "second")); + var thirdCommit = await WriteNextChange(SetWord(entityId, "third")); + await ClearNonRootSnapshots(); + var firstWord = await DataModel.GetAtCommit(firstCommit.Id, entityId); + firstWord.Should().NotBeNull(); + firstWord.Text.Should().Be("first"); + + var secondWord = await DataModel.GetAtCommit(secondCommit.Id, entityId); + secondWord.Should().NotBeNull(); + secondWord.Text.Should().Be("second"); + + var thirdWord = await DataModel.GetAtCommit(thirdCommit.Id, entityId); + thirdWord.Should().NotBeNull(); + thirdWord.Text.Should().Be("third"); + } + + private Task ClearNonRootSnapshots() + { + return DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(); + } + [Theory] [InlineData(10)] [InlineData(100)] diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 4853a2d..29e4df9 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -77,7 +77,7 @@ public async Task AddChanges( }; await Add(commit, deferCommit); return commit; - } + } private List _deferredCommits = []; private async Task Add(Commit commit, bool deferSnapshotUpdates) @@ -188,11 +188,6 @@ private async Task ValidateCommits() } } - public async Task GetEntitySnapshotAtTime(DateTimeOffset time, Guid entityId) - { - var snapshots = await GetSnapshotsAt(time); - return snapshots.GetValueOrDefault(entityId); - } public async Task GetLatestSnapshotByObjectId(Guid entityId) { @@ -237,6 +232,34 @@ public async Task> GetSnapshotsAt(DateTimeOffse return snapshots; } + public async Task GetEntitySnapshotAtTime(DateTimeOffset time, Guid entityId) + { + var snapshots = await GetSnapshotsAt(time); + return snapshots.GetValueOrDefault(entityId); + } + + + public async Task GetAtCommit(Guid commitId, Guid entityId) + { + var commit = await _crdtRepository.CurrentCommits().SingleAsync(c => c.Id == commitId); + var repository = _crdtRepository.GetScopedRepository(commit); + var snapshot = await repository.GetCurrentSnapshotByObjectId(entityId, false); + ArgumentNullException.ThrowIfNull(snapshot); + var newCommits = await _crdtRepository.CurrentCommits() + .Include(c => c.ChangeEntities) + .WhereAfter(snapshot.Commit) + .ToArrayAsync(); + if (newCommits.Length > 0) + { + var snapshots = await SnapshotWorker.ApplyCommitsToSnapshots(new Dictionary([new KeyValuePair(snapshot.EntityId, snapshot)]), + repository, + newCommits, + _crdtConfig.Value); + snapshot = snapshots[snapshot.EntityId]; + } + + return (T) snapshot.Entity.DbObject; + } public async Task GetSyncState() { return await _crdtRepository.GetCurrentSyncState(); diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 997bc1a..405f70e 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -9,24 +9,38 @@ namespace SIL.Harmony.Db; -internal class CrdtRepository(ICrdtDbContext _dbContext, IOptions crdtConfig, DateTimeOffset? ignoreChangesAfter = null) +internal class CrdtRepository(ICrdtDbContext _dbContext, IOptions crdtConfig, + Commit? ignoreChangesAfter = null + // DateTimeOffset? ignoreChangesAfter = null +) { private IQueryable Snapshots => _dbContext.Snapshots.AsNoTracking(); + private IQueryable Commits + { + get + { + if (ignoreChangesAfter is not null) + { + return _dbContext.Commits.WhereBefore(ignoreChangesAfter); + } + return _dbContext.Commits; + } + } + public Task BeginTransactionAsync() { return _dbContext.Database.BeginTransactionAsync(); } - public async Task HasCommit(Guid commitId) { - return await _dbContext.Commits.AnyAsync(c => c.Id == commitId); + return await Commits.AnyAsync(c => c.Id == commitId); } public async Task<(Commit? oldestChange, Commit[] newCommits)> FilterExistingCommits(ICollection commits) { Commit? oldestChange = null; - var commitIdsToExclude = await _dbContext.Commits + var commitIdsToExclude = await Commits .Where(c => commits.Select(c => c.Id).Contains(c.Id)) .Select(c => c.Id) .ToArrayAsync(); @@ -51,14 +65,13 @@ await Snapshots public IQueryable CurrentCommits() { - var query = _dbContext.Commits.DefaultOrder(); - if (ignoreChangesAfter is not null) query = query.Where(c => c.HybridDateTime.DateTime <= ignoreChangesAfter); - return query; + return Commits.DefaultOrder(); } public IQueryable CurrentSnapshots() { - var ignoreDate = ignoreChangesAfter?.UtcDateTime; + //todo actually filter by commit + var ignoreDate = ignoreChangesAfter?.HybridDateTime.DateTime.UtcDateTime; return _dbContext.Snapshots.FromSql( $""" WITH LatestSnapshots AS (SELECT first_value(s1.Id) @@ -94,17 +107,6 @@ public IAsyncEnumerable CurrenSimpleSnapshots(bool includeDelete return snapshots; } - private IQueryable CurrentSnapshotIds() - { - return Snapshots.GroupBy(s => s.EntityId, - (entityId, snapshots) => snapshots - //unfortunately this can not be extracted into a helper because the whole thing is part of an expression - .OrderByDescending(c => c.Commit.HybridDateTime.DateTime) - .ThenByDescending(c => c.Commit.HybridDateTime.Counter) - .ThenByDescending(c => c.Commit.Id) - .First(s => ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter).Id); - } - public async Task<(Dictionary currentSnapshots, Commit[] pendingCommits)> GetCurrentSnapshotsAndPendingCommits() { var snapshots = await CurrentSnapshots().Include(s => s.Commit).ToDictionaryAsync(s => s.EntityId); @@ -121,21 +123,21 @@ private IQueryable CurrentSnapshotIds() public async Task FindCommitByHash(string hash) { - return await _dbContext.Commits.SingleOrDefaultAsync(c => c.Hash == hash); + return await Commits.SingleOrDefaultAsync(c => c.Hash == hash); } public async Task FindPreviousCommit(Commit commit) { //can't trust the parentHash actually, so we can't do this. // if (!string.IsNullOrWhiteSpace(commit.ParentHash)) return await FindCommitByHash(commit.ParentHash); - return await _dbContext.Commits.WhereBefore(commit) + return await Commits.WhereBefore(commit) .DefaultOrderDescending() .FirstOrDefaultAsync(); } public async Task GetCommitsAfter(Commit? commit) { - var dbContextCommits = _dbContext.Commits.Include(c => c.ChangeEntities); + var dbContextCommits = Commits.Include(c => c.ChangeEntities); if (commit is null) return await dbContextCommits.DefaultOrder().ToArrayAsync(); return await dbContextCommits .WhereAfter(commit) @@ -155,7 +157,8 @@ public async Task GetCommitsAfter(Commit? commit) { return await Snapshots.AsTracking(tracking).Include(s => s.Commit) .DefaultOrder() - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter)); + //todo actually filter by commit + .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter.HybridDateTime.DateTime)); } public async Task GetObjectBySnapshotId(Guid snapshotId) @@ -172,7 +175,8 @@ public async Task GetObjectBySnapshotId(Guid snapshotId) { var snapshot = await Snapshots .DefaultOrder() - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter)); + //todo actually filter by commit + .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter.HybridDateTime.DateTime)); return (T?) snapshot?.Entity.DbObject; } @@ -187,10 +191,7 @@ public IQueryable GetCurrentObjects() where T : class public async Task GetCurrentSyncState() { - var queryable = _dbContext.Commits.AsQueryable(); - if (ignoreChangesAfter is not null) - queryable = queryable.Where(c => c.HybridDateTime.DateTime <= ignoreChangesAfter); - return await queryable.GetSyncState(); + return await Commits.GetSyncState(); } public async Task> GetChanges(SyncState remoteState) @@ -261,7 +262,16 @@ private async ValueTask SnapshotAdded(ObjectSnapshot objectSnapshot) public CrdtRepository GetScopedRepository(DateTimeOffset newCurrentTime) { - return new CrdtRepository(_dbContext, crdtConfig, newCurrentTime); + return GetScopedRepository(new Commit(Guid.Empty) + { + ClientId = Guid.Empty, + HybridDateTime = new HybridDateTime(newCurrentTime, 0) + }); + } + + public CrdtRepository GetScopedRepository(Commit excludeChangesAfterCommit) + { + return new CrdtRepository(_dbContext, crdtConfig, excludeChangesAfterCommit); } public async Task AddCommit(Commit commit) @@ -278,7 +288,7 @@ public async Task AddCommits(IEnumerable commits, bool save = true) public HybridDateTime? GetLatestDateTime() { - return _dbContext.Commits + return Commits .DefaultOrderDescending() .AsNoTracking() .Select(c => c.HybridDateTime) diff --git a/src/SIL.Harmony/Db/DbSetExtensions.cs b/src/SIL.Harmony/Db/DbSetExtensions.cs index ece85de..dce0553 100644 --- a/src/SIL.Harmony/Db/DbSetExtensions.cs +++ b/src/SIL.Harmony/Db/DbSetExtensions.cs @@ -28,9 +28,29 @@ public static IQueryable WhereAfter(this IQueryable WhereBefore(this IQueryable queryable, Commit before, bool inclusive = false) + { + if (inclusive) + { + return queryable.Where(c => c.Commit.HybridDateTime.DateTime < before.HybridDateTime.DateTime + || (c.Commit.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.Commit.HybridDateTime.Counter < before.HybridDateTime.Counter) + || (c.Commit.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.Commit.HybridDateTime.Counter == before.HybridDateTime.Counter && + c.CommitId < before.Id) + || c.CommitId == before.Id); + } + + return queryable.Where(c => c.Commit.HybridDateTime.DateTime < before.HybridDateTime.DateTime + || (c.Commit.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.Commit.HybridDateTime.Counter < before.HybridDateTime.Counter) + || (c.Commit.HybridDateTime.DateTime == before.HybridDateTime.DateTime && + c.Commit.HybridDateTime.Counter == before.HybridDateTime.Counter && c.CommitId < before.Id)); + } + public static IQueryable AsTracking(this IQueryable queryable, bool tracking = true) where T : class { return queryable.AsTracking(tracking ? QueryTrackingBehavior.TrackAll : QueryTrackingBehavior.NoTracking); } -} \ No newline at end of file +} From a11c91d266bbb82cf38b7f92b2bb83ba3820e39c Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 31 Oct 2024 10:57:00 +0700 Subject: [PATCH 03/10] add explicit tests for CurrentSnapshots from a scoped repo --- src/SIL.Harmony.Tests/RepositoryTests.cs | 51 +++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/SIL.Harmony.Tests/RepositoryTests.cs b/src/SIL.Harmony.Tests/RepositoryTests.cs index fcfc972..505d2a1 100644 --- a/src/SIL.Harmony.Tests/RepositoryTests.cs +++ b/src/SIL.Harmony.Tests/RepositoryTests.cs @@ -211,6 +211,55 @@ await _repository.AddSnapshots([ snapshots.Should().ContainSingle().Which.Commit.HybridDateTime.Should().BeEquivalentTo(commit1Time); } + [Fact] + public async Task ScopedRepo_CurrentSnapshots_FiltersByCounter() + { + var entityId = Guid.NewGuid(); + //not sorting as we want to order based on the hybrid date time counter + Guid[] commitIds = [Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid()]; + var snapshot1 = Snapshot(entityId, commitIds[0], Time(1, 0)); + var snapshot2 = Snapshot(entityId, commitIds[1], Time(2, 0)); + var snapshot3 = Snapshot(entityId, commitIds[2], Time(2, 1)); + await _repository.AddSnapshots([ + snapshot3, + snapshot1, + snapshot2, + ]); + + var snapshots = await _repository.CurrentSnapshots().Include(s => s.Commit).ToArrayAsync(); + var commit = snapshots.Should().ContainSingle().Subject.Commit; + commit.Id.Should().Be(commitIds[2]); + + snapshots = await _repository.GetScopedRepository(snapshot2.Commit).CurrentSnapshots().Include(s => s.Commit) + .ToArrayAsync(); + commit = snapshots.Should().ContainSingle().Subject.Commit; + commit.Id.Should().Be(commitIds[1], $"commit order: [{string.Join(", ", commitIds)}]"); + } + + [Fact] + public async Task ScopedRepo_CurrentSnapshots_FiltersByCommitId() + { + var entityId = Guid.NewGuid(); + Guid[] commitIds = [Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid()]; + Array.Sort(commitIds); + var snapshot1 = Snapshot(entityId, commitIds[0], Time(1, 0)); + var snapshot2 = Snapshot(entityId, commitIds[1], Time(2, 0)); + var snapshot3 = Snapshot(entityId, commitIds[2], Time(2, 0)); + await _repository.AddSnapshots([ + snapshot3, + snapshot1, + snapshot2, + ]); + + var snapshots = await _repository.CurrentSnapshots().Include(s => s.Commit).ToArrayAsync(); + var commit = snapshots.Should().ContainSingle().Subject.Commit; + commit.Id.Should().Be(commitIds[2]); + + snapshots = await _repository.GetScopedRepository(snapshot2.Commit).CurrentSnapshots().Include(s => s.Commit).ToArrayAsync(); + commit = snapshots.Should().ContainSingle().Subject.Commit; + commit.Id.Should().Be(commitIds[1], $"commit order: [{string.Join(", ", commitIds)}]"); + } + [Fact] public async Task DeleteStaleSnapshots_Works() { @@ -283,7 +332,7 @@ public async Task AddCommit_RoundTripsData() { var commit = Commit(Guid.NewGuid(), Time(1, 0)); await _repository.AddCommit(commit); - + var queriedCommit = _repository.CurrentCommits() .AsNoTracking()//ensures that the commit which is tracked above is not returned .Include(c => c.ChangeEntities) From 4e49c7becef70f0dce28db9445677ec9c5f061d6 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 31 Oct 2024 14:23:15 +0700 Subject: [PATCH 04/10] implement repo filtering by commit on snapshots --- .../DataModelReferenceTests.cs | 6 +-- src/SIL.Harmony/DataModel.cs | 4 +- src/SIL.Harmony/Db/CrdtRepository.cs | 46 ++++++++++++------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs index c8dfc31..e871630 100644 --- a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs @@ -47,9 +47,9 @@ public async Task SnapshotsDontGetMutatedByADelete() { var refAdd = await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id)); await WriteNextChange(new DeleteChange(_word2Id)); - var entitySnapshot1 = await DataModel.GetEntitySnapshotAtTime(refAdd.DateTime, _word1Id); - entitySnapshot1.Should().NotBeNull(); - entitySnapshot1!.Entity.Is().AntonymId.Should().Be(_word2Id); + var word = await DataModel.GetAtCommit(refAdd.Id, _word1Id); + word.Should().NotBeNull(); + word.AntonymId.Should().Be(_word2Id); } [Fact] diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 29e4df9..8f9495e 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -184,7 +184,7 @@ private async Task ValidateCommits() var actualParentCommit = await _crdtRepository.FindCommitByHash(commit.ParentHash); throw new CommitValidationException( - $"Commit {commit} does not match expected hash, parent hash [{commit.ParentHash}] !== [{parentHash}], expected parent {parentCommit} and actual parent {actualParentCommit}"); + $"Commit {commit} does not match expected hash, parent hash [{commit.ParentHash}] !== [{parentHash}], expected parent {parentCommit?.ToString() ?? "null"} and actual parent {actualParentCommit?.ToString() ?? "null"}"); } } @@ -245,7 +245,7 @@ public async Task GetAtCommit(Guid commitId, Guid entityId) var repository = _crdtRepository.GetScopedRepository(commit); var snapshot = await repository.GetCurrentSnapshotByObjectId(entityId, false); ArgumentNullException.ThrowIfNull(snapshot); - var newCommits = await _crdtRepository.CurrentCommits() + var newCommits = await repository.CurrentCommits() .Include(c => c.ChangeEntities) .WhereAfter(snapshot.Commit) .ToArrayAsync(); diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 405f70e..1996811 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -15,13 +15,25 @@ internal class CrdtRepository(ICrdtDbContext _dbContext, IOptions cr ) { private IQueryable Snapshots => _dbContext.Snapshots.AsNoTracking(); + + private IQueryable SnapshotsFiltered + { + get + { + if (ignoreChangesAfter is not null) + { + return Snapshots.WhereBefore(ignoreChangesAfter, inclusive: true); + } + return Snapshots; + } + } private IQueryable Commits { get { if (ignoreChangesAfter is not null) { - return _dbContext.Commits.WhereBefore(ignoreChangesAfter); + return _dbContext.Commits.WhereBefore(ignoreChangesAfter, inclusive: true); } return _dbContext.Commits; } @@ -70,8 +82,9 @@ public IQueryable CurrentCommits() public IQueryable CurrentSnapshots() { - //todo actually filter by commit - var ignoreDate = ignoreChangesAfter?.HybridDateTime.DateTime.UtcDateTime; + var ignoreAfterDate = ignoreChangesAfter?.HybridDateTime.DateTime.UtcDateTime; + var ignoreAfterCounter = ignoreChangesAfter?.HybridDateTime.Counter; + var ignoreAfterCommitId = ignoreChangesAfter?.Id; return _dbContext.Snapshots.FromSql( $""" WITH LatestSnapshots AS (SELECT first_value(s1.Id) @@ -79,9 +92,11 @@ WITH LatestSnapshots AS (SELECT first_value(s1.Id) PARTITION BY "s1"."EntityId" ORDER BY "c"."DateTime" DESC, "c"."Counter" DESC, "c"."Id" DESC ) AS "LatestSnapshotId" - FROM "Snapshots" AS "s1" - INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" - WHERE "c"."DateTime" < {ignoreDate} OR {ignoreDate} IS NULL) + FROM "Snapshots" AS "s1" + INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" + WHERE {ignoreAfterDate} IS NULL + OR ("c"."DateTime" < {ignoreAfterDate} OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" < {ignoreAfterCounter}) OR + ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" = {ignoreAfterCounter} AND "c"."Id" < {ignoreAfterCommitId}) OR "c"."Id" = {ignoreAfterCommitId})) SELECT * FROM "Snapshots" AS "s" INNER JOIN LatestSnapshots AS "ls" ON "s"."Id" = "ls"."LatestSnapshotId" @@ -130,8 +145,7 @@ public IAsyncEnumerable CurrenSimpleSnapshots(bool includeDelete { //can't trust the parentHash actually, so we can't do this. // if (!string.IsNullOrWhiteSpace(commit.ParentHash)) return await FindCommitByHash(commit.ParentHash); - return await Commits.WhereBefore(commit) - .DefaultOrderDescending() + return await Commits.DefaultOrderDescending() .FirstOrDefaultAsync(); } @@ -147,7 +161,7 @@ public async Task GetCommitsAfter(Commit? commit) public async Task FindSnapshot(Guid id, bool tracking = false) { - return await Snapshots + return await SnapshotsFiltered .AsTracking(tracking) .Include(s => s.Commit) .SingleOrDefaultAsync(s => s.Id == id); @@ -155,15 +169,16 @@ public async Task GetCommitsAfter(Commit? commit) public async Task GetCurrentSnapshotByObjectId(Guid objectId, bool tracking = false) { - return await Snapshots.AsTracking(tracking).Include(s => s.Commit) + return await SnapshotsFiltered + .AsTracking(tracking) + .Include(s => s.Commit) .DefaultOrder() - //todo actually filter by commit - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter.HybridDateTime.DateTime)); + .LastOrDefaultAsync(s => s.EntityId == objectId); } public async Task GetObjectBySnapshotId(Guid snapshotId) { - var entity = await Snapshots + var entity = await SnapshotsFiltered .Where(s => s.Id == snapshotId) .Select(s => s.Entity) .SingleOrDefaultAsync() @@ -173,10 +188,7 @@ public async Task GetObjectBySnapshotId(Guid snapshotId) public async Task GetCurrent(Guid objectId) where T: class { - var snapshot = await Snapshots - .DefaultOrder() - //todo actually filter by commit - .LastOrDefaultAsync(s => s.EntityId == objectId && (ignoreChangesAfter == null || s.Commit.HybridDateTime.DateTime <= ignoreChangesAfter.HybridDateTime.DateTime)); + var snapshot = await GetCurrentSnapshotByObjectId(objectId); return (T?) snapshot?.Entity.DbObject; } From 73d128680455a2d16f1e71e083b5ebf4f6c545df Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 31 Oct 2024 14:32:07 +0700 Subject: [PATCH 05/10] write test for FindPreviousCommit and fix bug --- src/SIL.Harmony.Tests/RepositoryTests.cs | 23 +++++++++++++++++++++++ src/SIL.Harmony/Db/CrdtRepository.cs | 7 ++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/SIL.Harmony.Tests/RepositoryTests.cs b/src/SIL.Harmony.Tests/RepositoryTests.cs index 505d2a1..a0d4067 100644 --- a/src/SIL.Harmony.Tests/RepositoryTests.cs +++ b/src/SIL.Harmony.Tests/RepositoryTests.cs @@ -339,4 +339,27 @@ public async Task AddCommit_RoundTripsData() .Should().ContainSingle().Subject; queriedCommit.Should().NotBeSameAs(commit).And.BeEquivalentTo(commit); } + + [Fact] + public async Task FindPreviousCommit_Works() + { + var commit1 = Commit(Guid.NewGuid(), Time(1, 0)); + var commit2 = Commit(Guid.NewGuid(), Time(2, 0)); + await _repository.AddCommits([commit1, commit2]); + + var previousCommit = await _repository.FindPreviousCommit(commit2); + ArgumentNullException.ThrowIfNull(previousCommit); + previousCommit.Id.Should().Be(commit1.Id); + } + + [Fact] + public async Task FindPreviousCommit_ReturnsNullForFirstCommit() + { + var commit1 = Commit(Guid.NewGuid(), Time(1, 0)); + var commit2 = Commit(Guid.NewGuid(), Time(2, 0)); + await _repository.AddCommits([commit1, commit2]); + + var previousCommit = await _repository.FindPreviousCommit(commit1); + previousCommit.Should().BeNull(); + } } diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 1996811..d1ace4d 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -92,8 +92,8 @@ WITH LatestSnapshots AS (SELECT first_value(s1.Id) PARTITION BY "s1"."EntityId" ORDER BY "c"."DateTime" DESC, "c"."Counter" DESC, "c"."Id" DESC ) AS "LatestSnapshotId" - FROM "Snapshots" AS "s1" - INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" + FROM "Snapshots" AS "s1" + INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" WHERE {ignoreAfterDate} IS NULL OR ("c"."DateTime" < {ignoreAfterDate} OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" < {ignoreAfterCounter}) OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" = {ignoreAfterCounter} AND "c"."Id" < {ignoreAfterCommitId}) OR "c"."Id" = {ignoreAfterCommitId})) @@ -145,7 +145,8 @@ public IAsyncEnumerable CurrenSimpleSnapshots(bool includeDelete { //can't trust the parentHash actually, so we can't do this. // if (!string.IsNullOrWhiteSpace(commit.ParentHash)) return await FindCommitByHash(commit.ParentHash); - return await Commits.DefaultOrderDescending() + return await Commits.WhereBefore(commit) + .DefaultOrderDescending() .FirstOrDefaultAsync(); } From 33b1aba763633e8fc63f97b1d02332d1e1739c5a Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 31 Oct 2024 14:50:43 +0700 Subject: [PATCH 06/10] don't support creating a socped repo with a date time, push that to the data model and add a method for getting an object at a given time --- src/SIL.Harmony.Tests/ModelSnapshotTests.cs | 30 +++++++++++++- src/SIL.Harmony.Tests/RepositoryTests.cs | 19 --------- src/SIL.Harmony/DataModel.cs | 46 +++++++++++++++------ src/SIL.Harmony/Db/CrdtRepository.cs | 9 ---- 4 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs index 55a8902..632af87 100644 --- a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs +++ b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs @@ -54,6 +54,32 @@ public async Task CanGetWordForASpecificCommit() thirdWord.Text.Should().Be("third"); } + [Fact] + public async Task CanGetWordForASpecificTime() + { + var entityId = Guid.NewGuid(); + var firstCommit = await WriteNextChange(SetWord(entityId, "first")); + var secondCommit = await WriteNextChange(SetWord(entityId, "second")); + var thirdCommit = await WriteNextChange(SetWord(entityId, "third")); + await ClearNonRootSnapshots(); + var firstWord = await DataModel.GetAtTime(firstCommit.DateTime.AddMinutes(5), entityId); + firstWord.Should().NotBeNull(); + firstWord.Text.Should().Be("first"); + + var secondWord = await DataModel.GetAtTime(secondCommit.DateTime.AddMinutes(5), entityId); + secondWord.Should().NotBeNull(); + secondWord.Text.Should().Be("second"); + + //just before the 3rd commit should still be second + secondWord = await DataModel.GetAtTime(thirdCommit.DateTime.Subtract(TimeSpan.FromSeconds(5)), entityId); + secondWord.Should().NotBeNull(); + secondWord.Text.Should().Be("second"); + + var thirdWord = await DataModel.GetAtTime(thirdCommit.DateTime.AddMinutes(5), entityId); + thirdWord.Should().NotBeNull(); + thirdWord.Text.Should().Be("third"); + } + private Task ClearNonRootSnapshots() { return DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(); @@ -82,7 +108,7 @@ public async Task CanGetSnapshotFromEarlier(int changeCount) for (int i = 0; i < changeCount; i++) { - var snapshots = await DataModel.GetSnapshotsAt(changes[i].DateTime); + var snapshots = await DataModel.GetSnapshotsAtCommit(changes[i]); var entry = snapshots[entityId].Entity.Is(); entry.Text.Should().Be($"change {i}"); snapshots.Values.Should().HaveCount(1 + i); @@ -103,7 +129,7 @@ await AddCommitsViaSync(Enumerable.Range(0, changeCount) //delete snapshots so when we get at then we need to re-apply await DbContext.Snapshots.Where(s => !s.IsRoot).ExecuteDeleteAsync(); - var computedModelSnapshots = await DataModel.GetSnapshotsAt(latestSnapshot.Commit.DateTime); + var computedModelSnapshots = await DataModel.GetSnapshotsAtCommit(latestSnapshot.Commit); var entitySnapshot = computedModelSnapshots.Should().ContainSingle().Subject.Value; entitySnapshot.Should().BeEquivalentTo(latestSnapshot, options => options.Excluding(snapshot => snapshot.Id).Excluding(snapshot => snapshot.Commit).Excluding(s => s.Entity.DbObject)); diff --git a/src/SIL.Harmony.Tests/RepositoryTests.cs b/src/SIL.Harmony.Tests/RepositoryTests.cs index a0d4067..c3b4ef6 100644 --- a/src/SIL.Harmony.Tests/RepositoryTests.cs +++ b/src/SIL.Harmony.Tests/RepositoryTests.cs @@ -192,25 +192,6 @@ await _repository.AddSnapshots([ snapshots.Should().ContainSingle().Which.Commit.Id.Should().Be(ids[1]); } - [Fact] - public async Task CurrentSnapshots_FiltersByDate() - { - var entityId = Guid.NewGuid(); - var commit1Time = Time(1, 0); - var commit2Time = Time(3, 0); - await _repository.AddSnapshots([ - Snapshot(entityId, Guid.NewGuid(), commit1Time), - Snapshot(entityId, Guid.NewGuid(), commit2Time), - ]); - - var snapshots = await _repository.CurrentSnapshots().Include(s => s.Commit).ToArrayAsync(); - snapshots.Should().ContainSingle().Which.Commit.HybridDateTime.Should().BeEquivalentTo(commit2Time); - - var newCurrentTime = Time(2, 0).DateTime; - snapshots = await _repository.GetScopedRepository(newCurrentTime).CurrentSnapshots().Include(s => s.Commit).ToArrayAsync(); - snapshots.Should().ContainSingle().Which.Commit.HybridDateTime.Should().BeEquivalentTo(commit1Time); - } - [Fact] public async Task ScopedRepo_CurrentSnapshots_FiltersByCounter() { diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 8f9495e..7f34d8a 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -23,7 +23,10 @@ public class DataModel : ISyncable, IAsyncDisposable private readonly IOptions _crdtConfig; //constructor must be internal because CrdtRepository is internal - internal DataModel(CrdtRepository crdtRepository, JsonSerializerOptions serializerOptions, IHybridDateTimeProvider timeProvider, IOptions crdtConfig) + internal DataModel(CrdtRepository crdtRepository, + JsonSerializerOptions serializerOptions, + IHybridDateTimeProvider timeProvider, + IOptions crdtConfig) { _crdtRepository = crdtRepository; _serializerOptions = serializerOptions; @@ -80,6 +83,7 @@ public async Task AddChanges( } private List _deferredCommits = []; + private async Task Add(Commit commit, bool deferSnapshotUpdates) { if (await _crdtRepository.HasCommit(commit.Id)) return; @@ -97,6 +101,7 @@ private async Task Add(Commit commit, bool deferSnapshotUpdates) { _deferredCommits.Add(commit); } + await transaction.CommitAsync(); } @@ -191,7 +196,8 @@ private async Task ValidateCommits() public async Task GetLatestSnapshotByObjectId(Guid entityId) { - return await _crdtRepository.GetCurrentSnapshotByObjectId(entityId) ?? throw new ArgumentException($"unable to find snapshot for entity {entityId}"); + return await _crdtRepository.GetCurrentSnapshotByObjectId(entityId) ?? + throw new ArgumentException($"unable to find snapshot for entity {entityId}"); } public async Task GetLatest(Guid objectId) where T : class @@ -209,8 +215,10 @@ public IQueryable QueryLatest() where T : class var q = _crdtRepository.GetCurrentObjects(); if (q is IQueryable) { - q = q.OrderBy(o => EF.Property(o, nameof(IOrderableCrdt.Order))).ThenBy(o => EF.Property(o, nameof(IOrderableCrdt.Id))); + q = q.OrderBy(o => EF.Property(o, nameof(IOrderableCrdt.Order))) + .ThenBy(o => EF.Property(o, nameof(IOrderableCrdt.Id))); } + return q; } @@ -219,29 +227,37 @@ public async Task GetBySnapshotId(Guid snapshotId) return await _crdtRepository.GetObjectBySnapshotId(snapshotId); } - public async Task> GetSnapshotsAt(DateTimeOffset dateTime) + public async Task> GetSnapshotsAtCommit(Commit commit) { - var repository = _crdtRepository.GetScopedRepository(dateTime); + var repository = _crdtRepository.GetScopedRepository(commit); var (snapshots, pendingCommits) = await repository.GetCurrentSnapshotsAndPendingCommits(); if (pendingCommits.Length != 0) { - snapshots = await SnapshotWorker.ApplyCommitsToSnapshots(snapshots, repository, pendingCommits, _crdtConfig.Value); + snapshots = await SnapshotWorker.ApplyCommitsToSnapshots(snapshots, + repository, + pendingCommits, + _crdtConfig.Value); } return snapshots; } - public async Task GetEntitySnapshotAtTime(DateTimeOffset time, Guid entityId) + public async Task GetAtTime(DateTimeOffset time, Guid entityId) { - var snapshots = await GetSnapshotsAt(time); - return snapshots.GetValueOrDefault(entityId); + var commitBefore = await _crdtRepository.CurrentCommits().LastOrDefaultAsync(c => c.HybridDateTime.DateTime <= time); + if (commitBefore is null) throw new ArgumentException("unable to find any commits"); + return await GetAtCommit(commitBefore, entityId); } - public async Task GetAtCommit(Guid commitId, Guid entityId) { - var commit = await _crdtRepository.CurrentCommits().SingleAsync(c => c.Id == commitId); + return await GetAtCommit(await _crdtRepository.CurrentCommits().SingleAsync(c => c.Id == commitId), + entityId); + } + + public async Task GetAtCommit(Commit commit, Guid entityId) + { var repository = _crdtRepository.GetScopedRepository(commit); var snapshot = await repository.GetCurrentSnapshotByObjectId(entityId, false); ArgumentNullException.ThrowIfNull(snapshot); @@ -251,15 +267,19 @@ public async Task GetAtCommit(Guid commitId, Guid entityId) .ToArrayAsync(); if (newCommits.Length > 0) { - var snapshots = await SnapshotWorker.ApplyCommitsToSnapshots(new Dictionary([new KeyValuePair(snapshot.EntityId, snapshot)]), + var snapshots = await SnapshotWorker.ApplyCommitsToSnapshots( + new Dictionary([ + new KeyValuePair(snapshot.EntityId, snapshot) + ]), repository, newCommits, _crdtConfig.Value); snapshot = snapshots[snapshot.EntityId]; } - return (T) snapshot.Entity.DbObject; + return (T)snapshot.Entity.DbObject; } + public async Task GetSyncState() { return await _crdtRepository.GetCurrentSyncState(); diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index d1ace4d..1b2ebe6 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -273,15 +273,6 @@ private async ValueTask SnapshotAdded(ObjectSnapshot objectSnapshot) return entity is not null ? _dbContext.Entry(entity) : null; } - public CrdtRepository GetScopedRepository(DateTimeOffset newCurrentTime) - { - return GetScopedRepository(new Commit(Guid.Empty) - { - ClientId = Guid.Empty, - HybridDateTime = new HybridDateTime(newCurrentTime, 0) - }); - } - public CrdtRepository GetScopedRepository(Commit excludeChangesAfterCommit) { return new CrdtRepository(_dbContext, crdtConfig, excludeChangesAfterCommit); From 372b57228b4b644844939ea9bd74bb128f0bd915 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 6 Nov 2024 12:09:07 +0700 Subject: [PATCH 07/10] remove left over commented code --- src/SIL.Harmony/Db/CrdtRepository.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 1b2ebe6..85f57dc 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -11,7 +11,6 @@ namespace SIL.Harmony.Db; internal class CrdtRepository(ICrdtDbContext _dbContext, IOptions crdtConfig, Commit? ignoreChangesAfter = null - // DateTimeOffset? ignoreChangesAfter = null ) { private IQueryable Snapshots => _dbContext.Snapshots.AsNoTracking(); From c2238ec57f81830103a10eec0eaa5c7d50cfff43 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 6 Nov 2024 12:16:48 +0700 Subject: [PATCH 08/10] add comment about why we are clearing non root snapshots --- src/SIL.Harmony.Tests/ModelSnapshotTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs index 632af87..23256e7 100644 --- a/src/SIL.Harmony.Tests/ModelSnapshotTests.cs +++ b/src/SIL.Harmony.Tests/ModelSnapshotTests.cs @@ -61,6 +61,7 @@ public async Task CanGetWordForASpecificTime() var firstCommit = await WriteNextChange(SetWord(entityId, "first")); var secondCommit = await WriteNextChange(SetWord(entityId, "second")); var thirdCommit = await WriteNextChange(SetWord(entityId, "third")); + //ensures that SnapshotWorker.ApplyCommitsToSnapshots will be called when getting the snapshots await ClearNonRootSnapshots(); var firstWord = await DataModel.GetAtTime(firstCommit.DateTime.AddMinutes(5), entityId); firstWord.Should().NotBeNull(); From b9d7d7a0cfeb27e37cf6b07fec7b6c1e5b3999d7 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 6 Nov 2024 12:19:55 +0700 Subject: [PATCH 09/10] include filtered commit on non inclusive tests to show that it gets excluded --- src/SIL.Harmony.Tests/Db/QueryHelperTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs b/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs index 22b2808..0903912 100644 --- a/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs +++ b/src/SIL.Harmony.Tests/Db/QueryHelperTests.cs @@ -293,6 +293,7 @@ public void WhereBeforeSnapshot_FiltersOnDate() var snapshots = Snapshots([ Snapshot(id1, Time(1, 0)), Snapshot(id2, Time(3, 0)), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime), Snapshot(id3, Time(4, 0)), ]); snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ @@ -323,6 +324,7 @@ public void WhereBeforeSnapshot_FiltersOnCounter() var snapshots = Snapshots([ Snapshot(id1, Time(1, 1)), Snapshot(id2, Time(1, 3)), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime), Snapshot(id3, Time(1, 4)), ]); snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ @@ -359,6 +361,7 @@ public void WhereBeforeSnapshot_FiltersOnId() Snapshot(commitId1, hybridDateTime), Snapshot(commitId2, hybridDateTime), Snapshot(commitId3, hybridDateTime), + Snapshot(filterCommit.Id, filterCommit.HybridDateTime), ]); snapshots.WhereBefore(filterCommit).Select(c => c.CommitId).Should().BeEquivalentTo([ commitId1 @@ -394,6 +397,7 @@ public void WhereBeforeCommit_FiltersOnDate() Commit(id1, Time(1, 0)), Commit(id2, Time(3, 0)), Commit(id3, Time(4, 0)), + filterCommit ]); commits.WhereBefore(filterCommit).Select(c => c.Id).Should().BeEquivalentTo([ id1 @@ -424,6 +428,7 @@ public void WhereBeforeCommit_FiltersOnCounter() Commit(id1, Time(1, 1)), Commit(id2, Time(1, 3)), Commit(id3, Time(1, 4)), + filterCommit ]); commits.WhereBefore(filterCommit).Select(c => c.Id).Should().BeEquivalentTo([ id1, @@ -459,6 +464,7 @@ public void WhereBeforeCommit_FiltersOnId() Commit(commitId1, hybridDateTime), Commit(commitId2, hybridDateTime), Commit(commitId3, hybridDateTime), + filterCommit ]); commits.WhereBefore(filterCommit).Select(c => c.Id).Should().BeEquivalentTo([ commitId1 From 476f1286803a63de7a0ddfb904df513d78144a21 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 6 Nov 2024 13:25:17 +0700 Subject: [PATCH 10/10] change how CrdtRepository restricts its queries to avoid writing code that won't work in scoped contexts --- .../DataModelPerformanceTests.cs | 4 +- src/SIL.Harmony.Tests/DbContextTests.cs | 6 +- src/SIL.Harmony/Db/CrdtRepository.cs | 156 ++++++++++++------ src/SIL.Harmony/Db/ICrdtDbContext.cs | 5 +- 4 files changed, 109 insertions(+), 62 deletions(-) diff --git a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs index b5b4443..6aaf2af 100644 --- a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs @@ -125,8 +125,8 @@ internal static async Task BulkInsertChanges(DataModelTestBase dataModelTest, in }; commit.SetParentHash(parentHash); parentHash = commit.Hash; - dataModelTest.DbContext.Commits.Add(commit); - dataModelTest.DbContext.Snapshots.Add(new ObjectSnapshot(await change.NewEntity(commit, null!), commit, true)); + dataModelTest.DbContext.Add(commit); + dataModelTest.DbContext.Add(new ObjectSnapshot(await change.NewEntity(commit, null!), commit, true)); } await dataModelTest.DbContext.SaveChangesAsync(); diff --git a/src/SIL.Harmony.Tests/DbContextTests.cs b/src/SIL.Harmony.Tests/DbContextTests.cs index 33cc0a1..8341517 100644 --- a/src/SIL.Harmony.Tests/DbContextTests.cs +++ b/src/SIL.Harmony.Tests/DbContextTests.cs @@ -27,7 +27,7 @@ public async Task CanRoundTripDatesFromEf(int offset) ClientId = Guid.NewGuid(), HybridDateTime = new HybridDateTime(expectedDateTime, 0) }; - DbContext.Commits.Add(commit); + DbContext.Add(commit); await DbContext.SaveChangesAsync(); var actualCommit = await DbContext.Commits.AsNoTracking().SingleOrDefaultAsyncEF(c => c.Id == commitId); actualCommit!.HybridDateTime.DateTime.Should().Be(expectedDateTime, "EF"); @@ -46,7 +46,7 @@ public async Task CanRoundTripDatesFromLinq2Db(int offset) var commitId = Guid.NewGuid(); var expectedDateTime = new DateTimeOffset(2000, 1, 1, 1, 11, 11, TimeSpan.FromHours(offset)); - await DbContext.Commits.ToLinqToDBTable().AsValueInsertable() + await DbContext.Set().ToLinqToDBTable().AsValueInsertable() .Value(c => c.Id, commitId) .Value(c => c.ClientId, Guid.NewGuid()) .Value(c => c.HybridDateTime.DateTime, expectedDateTime) @@ -74,7 +74,7 @@ public async Task CanFilterCommitsByDateTime(double scale) for (int i = 0; i < 50; i++) { var offset = new TimeSpan((long)(i * scale)); - DbContext.Commits.Add(new Commit + DbContext.Add(new Commit { ClientId = Guid.NewGuid(), HybridDateTime = new HybridDateTime(baseDateTime.Add(offset), 0) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 85f57dc..82efc83 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -1,6 +1,7 @@ using SIL.Harmony.Core; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.Options; using SIL.Harmony.Changes; @@ -9,34 +10,25 @@ namespace SIL.Harmony.Db; -internal class CrdtRepository(ICrdtDbContext _dbContext, IOptions crdtConfig, - Commit? ignoreChangesAfter = null -) +internal class CrdtRepository { - private IQueryable Snapshots => _dbContext.Snapshots.AsNoTracking(); + private readonly ICrdtDbContext _dbContext; + private readonly IOptions _crdtConfig; - private IQueryable SnapshotsFiltered + public CrdtRepository(ICrdtDbContext dbContext, IOptions crdtConfig, + Commit? ignoreChangesAfter = null) { - get - { - if (ignoreChangesAfter is not null) - { - return Snapshots.WhereBefore(ignoreChangesAfter, inclusive: true); - } - return Snapshots; - } - } - private IQueryable Commits - { - get - { - if (ignoreChangesAfter is not null) - { - return _dbContext.Commits.WhereBefore(ignoreChangesAfter, inclusive: true); - } - return _dbContext.Commits; - } + _crdtConfig = crdtConfig; + _dbContext = ignoreChangesAfter is not null ? new ScopedDbContext(dbContext, ignoreChangesAfter) : dbContext; + //we can't use the scoped db context is it prevents access to the DbSet for the Snapshots, + //but since we're using a custom query, we can use it directly and apply the scoped filters manually + _currentSnapshotsQueryable = MakeCurrentSnapshotsQuery(dbContext, ignoreChangesAfter); } + + + private IQueryable Snapshots => _dbContext.Snapshots.AsNoTracking(); + + private IQueryable Commits => _dbContext.Commits; public Task BeginTransactionAsync() { @@ -79,28 +71,34 @@ public IQueryable CurrentCommits() return Commits.DefaultOrder(); } - public IQueryable CurrentSnapshots() + private static IQueryable MakeCurrentSnapshotsQuery(ICrdtDbContext dbContext, Commit? ignoreChangesAfter) { var ignoreAfterDate = ignoreChangesAfter?.HybridDateTime.DateTime.UtcDateTime; var ignoreAfterCounter = ignoreChangesAfter?.HybridDateTime.Counter; var ignoreAfterCommitId = ignoreChangesAfter?.Id; - return _dbContext.Snapshots.FromSql( -$""" -WITH LatestSnapshots AS (SELECT first_value(s1.Id) - OVER ( - PARTITION BY "s1"."EntityId" - ORDER BY "c"."DateTime" DESC, "c"."Counter" DESC, "c"."Id" DESC - ) AS "LatestSnapshotId" - FROM "Snapshots" AS "s1" - INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" - WHERE {ignoreAfterDate} IS NULL - OR ("c"."DateTime" < {ignoreAfterDate} OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" < {ignoreAfterCounter}) OR - ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" = {ignoreAfterCounter} AND "c"."Id" < {ignoreAfterCommitId}) OR "c"."Id" = {ignoreAfterCommitId})) -SELECT * -FROM "Snapshots" AS "s" - INNER JOIN LatestSnapshots AS "ls" ON "s"."Id" = "ls"."LatestSnapshotId" -GROUP BY s.EntityId -""").AsNoTracking(); + return dbContext.Set().FromSql( + $""" + WITH LatestSnapshots AS (SELECT first_value(s1.Id) + OVER ( + PARTITION BY "s1"."EntityId" + ORDER BY "c"."DateTime" DESC, "c"."Counter" DESC, "c"."Id" DESC + ) AS "LatestSnapshotId" + FROM "Snapshots" AS "s1" + INNER JOIN "Commits" AS "c" ON "s1"."CommitId" = "c"."Id" + WHERE {ignoreAfterDate} IS NULL + OR ("c"."DateTime" < {ignoreAfterDate} OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" < {ignoreAfterCounter}) OR + ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" = {ignoreAfterCounter} AND "c"."Id" < {ignoreAfterCommitId}) OR "c"."Id" = {ignoreAfterCommitId})) + SELECT * + FROM "Snapshots" AS "s" + INNER JOIN LatestSnapshots AS "ls" ON "s"."Id" = "ls"."LatestSnapshotId" + GROUP BY s.EntityId + """).AsNoTracking(); + } + + private readonly IQueryable _currentSnapshotsQueryable; + public IQueryable CurrentSnapshots() + { + return _currentSnapshotsQueryable; } public IAsyncEnumerable CurrenSimpleSnapshots(bool includeDeleted = false) @@ -161,7 +159,7 @@ public async Task GetCommitsAfter(Commit? commit) public async Task FindSnapshot(Guid id, bool tracking = false) { - return await SnapshotsFiltered + return await Snapshots .AsTracking(tracking) .Include(s => s.Commit) .SingleOrDefaultAsync(s => s.Id == id); @@ -169,7 +167,7 @@ public async Task GetCommitsAfter(Commit? commit) public async Task GetCurrentSnapshotByObjectId(Guid objectId, bool tracking = false) { - return await SnapshotsFiltered + return await Snapshots .AsTracking(tracking) .Include(s => s.Commit) .DefaultOrder() @@ -178,7 +176,7 @@ public async Task GetCommitsAfter(Commit? commit) public async Task GetObjectBySnapshotId(Guid snapshotId) { - var entity = await SnapshotsFiltered + var entity = await Snapshots .Where(s => s.Id == snapshotId) .Select(s => s.Entity) .SingleOrDefaultAsync() @@ -194,7 +192,7 @@ public async Task GetObjectBySnapshotId(Guid snapshotId) public IQueryable GetCurrentObjects() where T : class { - if (crdtConfig.Value.EnableProjectedTables) + if (_crdtConfig.Value.EnableProjectedTables) { return _dbContext.Set(); } @@ -208,15 +206,14 @@ public async Task GetCurrentSyncState() public async Task> GetChanges(SyncState remoteState) { - var dbContextCommits = _dbContext.Commits; - return await dbContextCommits.GetChanges(remoteState); + return await _dbContext.Commits.GetChanges(remoteState); } public async Task AddSnapshots(IEnumerable snapshots) { foreach (var objectSnapshot in snapshots) { - _dbContext.Snapshots.Add(objectSnapshot); + _dbContext.Add(objectSnapshot); await SnapshotAdded(objectSnapshot); } @@ -228,7 +225,7 @@ public async ValueTask AddIfNew(IEnumerable snapshots) foreach (var snapshot in snapshots) { - if (_dbContext.Snapshots.Local.FindEntry(snapshot.Id) is not null) continue; + if (_dbContext.Set().Local.FindEntry(snapshot.Id) is not null) continue; _dbContext.Add(snapshot); await SnapshotAdded(snapshot); } @@ -238,7 +235,7 @@ public async ValueTask AddIfNew(IEnumerable snapshots) private async ValueTask SnapshotAdded(ObjectSnapshot objectSnapshot) { - if (!crdtConfig.Value.EnableProjectedTables) return; + if (!_crdtConfig.Value.EnableProjectedTables) return; if (objectSnapshot.IsRoot && objectSnapshot.EntityIsDeleted) return; //need to check if an entry exists already, even if this is the root commit it may have already been added to the db var existingEntry = await GetEntityEntry(objectSnapshot.Entity.DbObject.GetType(), objectSnapshot.EntityId); @@ -267,25 +264,25 @@ private async ValueTask SnapshotAdded(ObjectSnapshot objectSnapshot) private async ValueTask GetEntityEntry(Type entityType, Guid entityId) { - if (!crdtConfig.Value.EnableProjectedTables) return null; + if (!_crdtConfig.Value.EnableProjectedTables) return null; var entity = await _dbContext.FindAsync(entityType, entityId); return entity is not null ? _dbContext.Entry(entity) : null; } public CrdtRepository GetScopedRepository(Commit excludeChangesAfterCommit) { - return new CrdtRepository(_dbContext, crdtConfig, excludeChangesAfterCommit); + return new CrdtRepository(_dbContext, _crdtConfig, excludeChangesAfterCommit); } public async Task AddCommit(Commit commit) { - _dbContext.Commits.Add(commit); + _dbContext.Add(commit); await _dbContext.SaveChangesAsync(); } public async Task AddCommits(IEnumerable commits, bool save = true) { - _dbContext.Commits.AddRange(commits); + _dbContext.AddRange(commits); if (save) await _dbContext.SaveChangesAsync(); } @@ -298,3 +295,52 @@ public async Task AddCommits(IEnumerable commits, bool save = true) .FirstOrDefault(); } } + +internal class ScopedDbContext(ICrdtDbContext inner, Commit ignoreChangesAfter) : ICrdtDbContext +{ + public IQueryable Commits => inner.Commits.WhereBefore(ignoreChangesAfter, inclusive: true); + + public IQueryable Snapshots => inner.Snapshots.WhereBefore(ignoreChangesAfter, inclusive: true); + + public Task SaveChangesAsync(CancellationToken cancellationToken = default) + { + return inner.SaveChangesAsync(cancellationToken); + } + + public ValueTask FindAsync(Type entityType, params object?[]? keyValues) + { + throw new NotSupportedException("can not support FindAsync when using scoped db context"); + } + + public DbSet Set() where TEntity : class + { + throw new NotSupportedException("can not support Set when using scoped db context"); + } + + public DatabaseFacade Database => inner.Database; + + public EntityEntry Entry(TEntity entity) where TEntity : class + { + return inner.Entry(entity); + } + + public EntityEntry Entry(object entity) + { + return inner.Entry(entity); + } + + public EntityEntry Add(object entity) + { + return inner.Add(entity); + } + + public void AddRange(IEnumerable entities) + { + inner.AddRange(entities); + } + + public EntityEntry Remove(object entity) + { + return inner.Remove(entity); + } +} \ No newline at end of file diff --git a/src/SIL.Harmony/Db/ICrdtDbContext.cs b/src/SIL.Harmony/Db/ICrdtDbContext.cs index 9d40fe2..475e8e6 100644 --- a/src/SIL.Harmony/Db/ICrdtDbContext.cs +++ b/src/SIL.Harmony/Db/ICrdtDbContext.cs @@ -6,8 +6,8 @@ namespace SIL.Harmony.Db; public interface ICrdtDbContext { - DbSet Commits => Set(); - DbSet Snapshots => Set(); + IQueryable Commits => Set(); + IQueryable Snapshots => Set(); Task SaveChangesAsync(CancellationToken cancellationToken = default); ValueTask FindAsync(Type entityType, params object?[]? keyValues); DbSet Set() where TEntity : class; @@ -15,5 +15,6 @@ public interface ICrdtDbContext EntityEntry Entry(TEntity entity) where TEntity : class; EntityEntry Entry(object entity); EntityEntry Add(object entity); + void AddRange(IEnumerable entities); EntityEntry Remove(object entity); } \ No newline at end of file