From 0fe7a078b34ded4dacd29cf42544b4d9e3a37a70 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Thu, 23 Oct 2025 08:14:03 +0200 Subject: [PATCH 1/9] Make NewEntity override deleted --- .../DataModelSimpleChanges.cs | 28 +++++++++++--- src/SIL.Harmony.Tests/SnapshotTests.cs | 8 ++-- src/SIL.Harmony/Changes/Change.cs | 14 ++++++- src/SIL.Harmony/SnapshotWorker.cs | 37 ++++++++++++------- 4 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/SIL.Harmony.Tests/DataModelSimpleChanges.cs b/src/SIL.Harmony.Tests/DataModelSimpleChanges.cs index 284d2cc..7d989c1 100644 --- a/src/SIL.Harmony.Tests/DataModelSimpleChanges.cs +++ b/src/SIL.Harmony.Tests/DataModelSimpleChanges.cs @@ -190,17 +190,35 @@ public async Task CanDeleteAnEntry() } [Fact] - public async Task CanModifyAnEntryAfterDelete() + public async Task ApplyChange_ModifiesADeletedEntry() { await WriteNextChange(SetWord(_entity1Id, "test-value")); var deleteCommit = await WriteNextChange(new DeleteChange(_entity1Id)); - await WriteNextChange(SetWord(_entity1Id, "after-delete")); + var newNoteChange = new SetWordNoteChange(_entity1Id, "note-after-delete"); + newNoteChange.SupportsApplyChange().Should().BeTrue(); + newNoteChange.SupportsNewEntity().Should().BeFalse(); // otherwise it will override the delete + await WriteNextChange(newNoteChange); var snapshot = await DbContext.Snapshots.DefaultOrder().LastAsync(); var word = snapshot.Entity.Is(); - word.Text.Should().Be("after-delete"); + word.Text.Should().Be("test-value"); + word.Note.Should().Be("note-after-delete"); word.DeletedAt.Should().Be(deleteCommit.DateTime); } + [Fact] + public async Task NewEntity_UndeletesAnEntry() + { + await WriteNextChange(SetWord(_entity1Id, "test-value")); + await WriteNextChange(new DeleteChange(_entity1Id)); + var recreateChange = SetWord(_entity1Id, "after-undo-delete"); + recreateChange.SupportsNewEntity().Should().BeTrue(); + await WriteNextChange(recreateChange); + var snapshot = await DbContext.Snapshots.DefaultOrder().LastAsync(); + var word = snapshot.Entity.Is(); + word.Text.Should().Be("after-undo-delete"); + word.DeletedAt.Should().Be(null); + } + [Fact] public async Task ChangesToSnapshotsAreNotSaved() { @@ -208,10 +226,10 @@ public async Task ChangesToSnapshotsAreNotSaved() var word = await DataModel.GetLatest(_entity1Id); word!.Text.Should().Be("test-value"); word.Note.Should().BeNull(); - + //change made outside the model, should not be saved when writing the next change word.Note = "a note"; - + var commit = await WriteNextChange(SetWord(_entity1Id, "after-change")); var objectSnapshot = commit.Snapshots.Should().ContainSingle().Subject; objectSnapshot.Entity.Is().Text.Should().Be("after-change"); diff --git a/src/SIL.Harmony.Tests/SnapshotTests.cs b/src/SIL.Harmony.Tests/SnapshotTests.cs index 248d574..1a4ff0e 100644 --- a/src/SIL.Harmony.Tests/SnapshotTests.cs +++ b/src/SIL.Harmony.Tests/SnapshotTests.cs @@ -1,4 +1,6 @@ using SIL.Harmony.Sample.Models; +using SIL.Harmony.Sample.Changes; +using SIL.Harmony.Changes; using Microsoft.EntityFrameworkCore; namespace SIL.Harmony.Tests; @@ -19,7 +21,7 @@ public async Task MultipleChangesPreservesRootSnapshot() { var entityId = Guid.NewGuid(); var commits = new List(); - for (int i = 0; i < 4; i++) + for (var i = 0; i < 4; i++) { commits.Add(await WriteChange(_localClientId, new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero).AddHours(i), @@ -39,7 +41,7 @@ public async Task MultipleChangesPreservesSomeIntermediateSnapshots() { var entityId = Guid.NewGuid(); var commits = new List(); - for (int i = 0; i < 6; i++) + for (var i = 0; i < 6; i++) { commits.Add(await WriteChange(_localClientId, new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero).AddHours(i), @@ -135,7 +137,7 @@ await WriteNextChange( var tagCreation = await WriteNextChange(TagWord(wordId, tagId)); await WriteChangeBefore(tagCreation, TagWord(wordId, tagId)); - var word = await DataModel.QueryLatest(q=> q.Include(w => w.Tags) + var word = await DataModel.QueryLatest(q => q.Include(w => w.Tags) .Where(w => w.Id == wordId)).FirstOrDefaultAsync(); word.Should().NotBeNull(); word.Tags.Should().BeEquivalentTo([new Tag { Id = tagId, Text = "tag-1" }]); diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index 3695c95..7cf1794 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -16,6 +16,8 @@ public interface IChange ValueTask ApplyChange(IObjectBase entity, IChangeContext context); ValueTask NewEntity(Commit commit, IChangeContext context); + bool SupportsApplyChange(); + bool SupportsNewEntity(); } /// @@ -44,7 +46,7 @@ async ValueTask IChange.NewEntity(Commit commit, IChangeContext con public async ValueTask ApplyChange(IObjectBase entity, IChangeContext context) { - if (this is CreateChange) + if (!SupportsApplyChange()) return; // skip attempting to apply changes on CreateChange as it does not support apply changes if (entity.DbObject is T entityT) { @@ -56,6 +58,16 @@ public async ValueTask ApplyChange(IObjectBase entity, IChangeContext context) } } + public virtual bool SupportsApplyChange() + { + return this is not CreateChange; + } + + public virtual bool SupportsNewEntity() + { + return this is not EditChange; + } + [JsonIgnore] public Type EntityType => typeof(T); } diff --git a/src/SIL.Harmony/SnapshotWorker.cs b/src/SIL.Harmony/SnapshotWorker.cs index 612346f..ff3cdcb 100644 --- a/src/SIL.Harmony/SnapshotWorker.cs +++ b/src/SIL.Harmony/SnapshotWorker.cs @@ -81,26 +81,37 @@ private async ValueTask ApplyCommitChanges(IEnumerable commits, bool upd IObjectBase entity; var prevSnapshot = await GetSnapshot(commitChange.EntityId); var changeContext = new ChangeContext(commit, commitIndex, intermediateSnapshots, this, _crdtConfig); - bool wasDeleted; - if (prevSnapshot is not null) + + if (prevSnapshot is null) { - entity = prevSnapshot.Entity.Copy(); - wasDeleted = entity.DeletedAt.HasValue; + // create brand new entity + entity = await commitChange.Change.NewEntity(commit, changeContext); } - else + else if (prevSnapshot.EntityIsDeleted && commitChange.Change.SupportsNewEntity()) { + // revive deleted entity entity = await commitChange.Change.NewEntity(commit, changeContext); - wasDeleted = false; } - - await commitChange.Change.ApplyChange(entity, changeContext); - - var deletedByChange = !wasDeleted && entity.DeletedAt.HasValue; - if (deletedByChange) + else if (commitChange.Change.SupportsApplyChange()) { - await MarkDeleted(entity.Id, changeContext); + // update existing entity + entity = prevSnapshot.Entity.Copy(); + var wasDeleted = prevSnapshot.EntityIsDeleted; + await commitChange.Change.ApplyChange(entity, changeContext); + var deletedByChange = !wasDeleted && entity.DeletedAt.HasValue; + if (deletedByChange) + { + await MarkDeleted(entity.Id, changeContext); + } } - + else + { + // entity already exists (and is not deleted) + // and change does not support updating existing entities + // so do nothing + continue; + } + await GenerateSnapshotForEntity(entity, prevSnapshot, changeContext); } _newIntermediateSnapshots.AddRange(intermediateSnapshots.Values); From 00bd31064d1e024755fe1c9538b2d8eb22f9c8f4 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Thu, 23 Oct 2025 09:01:02 +0200 Subject: [PATCH 2/9] Save changes even if no snapshots were created --- src/SIL.Harmony/Db/CrdtRepository.cs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index a7145cb..219321f 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -310,18 +310,18 @@ public async Task AddSnapshots(IEnumerable snapshots) await ProjectSnapshot(snapshot); } } + } - try - { - await _dbContext.SaveChangesAsync(); - } - catch (DbUpdateException e) - { - var entries = string.Join(Environment.NewLine, e.Entries.Select(entry => entry.ToString())); - var message = $"Error saving snapshots: {e.Message}{Environment.NewLine}{entries}"; - _logger.LogError(e, message); - throw new DbUpdateException(message, e); - } + try + { + await _dbContext.SaveChangesAsync(); + } + catch (DbUpdateException e) + { + var entries = string.Join(Environment.NewLine, e.Entries.Select(entry => entry.ToString())); + var message = $"Error saving snapshots: {e.Message}{Environment.NewLine}{entries}"; + _logger.LogError(e, message); + throw new DbUpdateException(message, e); } } From 6755b88e7d900fec78f9bb8e0dae19116e8c5245 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Thu, 23 Oct 2025 09:36:54 +0200 Subject: [PATCH 3/9] Restore save-changes in loop --- src/SIL.Harmony/Db/CrdtRepository.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 219321f..d2231d5 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -310,6 +310,18 @@ public async Task AddSnapshots(IEnumerable snapshots) await ProjectSnapshot(snapshot); } } + + try + { + await _dbContext.SaveChangesAsync(); + } + catch (DbUpdateException e) + { + var entries = string.Join(Environment.NewLine, e.Entries.Select(entry => entry.ToString())); + var message = $"Error saving snapshots: {e.Message}{Environment.NewLine}{entries}"; + _logger.LogError(e, message); + throw new DbUpdateException(message, e); + } } try From d4d2126b1a9f72d91e3ac45d7afa322b4a3f9889 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 24 Oct 2025 09:15:33 +0200 Subject: [PATCH 4/9] Fix projecting deleted and undeleted snapshots of same entity --- src/SIL.Harmony.Tests/DeleteAndCreateTests.cs | 188 ++++++++++++++++++ src/SIL.Harmony/Db/CrdtRepository.cs | 14 +- 2 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 src/SIL.Harmony.Tests/DeleteAndCreateTests.cs diff --git a/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs b/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs new file mode 100644 index 0000000..f1a50c7 --- /dev/null +++ b/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs @@ -0,0 +1,188 @@ +using Microsoft.EntityFrameworkCore; +using SIL.Harmony.Changes; +using SIL.Harmony.Sample.Changes; +using SIL.Harmony.Sample.Models; + +namespace SIL.Harmony.Tests; + +public class DeleteAndCreateTests : DataModelTestBase +{ + [Fact] + public async Task DeleteAndUndeleteInSameCommitWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange(new NewWordChange(wordId, "original")); + + await WriteNextChange( + [ + new DeleteChange(wordId), + new NewWordChange(wordId, "Undeleted"), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task DeleteAndUndeleteInSameSyncWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange(new NewWordChange(wordId, "original")); + + await AddCommitsViaSync([ + await WriteNextChange(new DeleteChange(wordId), add: false), + await WriteNextChange(new NewWordChange(wordId, "Undeleted"), add: false), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task UpdateAndUndeleteInSameCommitWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange(new NewWordChange(wordId, "original")); + await WriteNextChange(new DeleteChange(wordId)); + + await WriteNextChange([ + new SetWordNoteChange(wordId, "overridden-note"), + new NewWordChange(wordId, "Undeleted"), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task UpdateAndUndeleteInSameSyncWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange(new NewWordChange(wordId, "original")); + await WriteNextChange(new DeleteChange(wordId)); + + await AddCommitsViaSync([ + await WriteNextChange(new SetWordNoteChange(wordId, "overridden-note"), add: false), + await WriteNextChange(new NewWordChange(wordId, "Undeleted"), add: false), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task CreateDeleteAndUndeleteInSameCommitWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange( + [ + new NewWordChange(wordId, "original"), + new DeleteChange(wordId), + new NewWordChange(wordId, "Undeleted"), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task CreateDeleteAndUndeleteInSameSyncWorks() + { + var wordId = Guid.NewGuid(); + + await AddCommitsViaSync([ + await WriteNextChange(new NewWordChange(wordId, "original"), add: false), + await WriteNextChange(new DeleteChange(wordId), add: false), + await WriteNextChange(new NewWordChange(wordId, "Undeleted"), add: false), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("Undeleted"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("Undeleted"); + } + + [Fact] + public async Task CreateAndDeleteInSameCommitWorks() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange( + [ + new NewWordChange(wordId, "original"), + new DeleteChange(wordId), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().NotBeNull(); + word.Text.Should().Be("original"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().BeNull(); + } + + [Fact] + public async Task CreateAndDeleteInSameSyncWorks() + { + var wordId = Guid.NewGuid(); + + await AddCommitsViaSync([ + await WriteNextChange(new NewWordChange(wordId, "original"), add: false), + await WriteNextChange(new DeleteChange(wordId), add: false), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().NotBeNull(); + word.Text.Should().Be("original"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().BeNull(); + } +} diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index d2231d5..9e3b983 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -299,16 +299,24 @@ public async Task> GetChanges(SyncState remoteState) public async Task AddSnapshots(IEnumerable snapshots) { - var projectedEntityIds = new HashSet(); + var latestProjectByEntityId = new Dictionary(); foreach (var grouping in snapshots.GroupBy(s => s.EntityIsDeleted).OrderByDescending(g => g.Key))//execute deletes first { foreach (var snapshot in grouping.DefaultOrderDescending()) { _dbContext.Add(snapshot); - if (projectedEntityIds.Add(snapshot.EntityId)) + if (latestProjectByEntityId.TryGetValue(snapshot.EntityId, out var latestProjected)) { - await ProjectSnapshot(snapshot); + // there might be a deleted and un-deleted snapshot for the same entity in the same batch + // in that case there's only a 50% chance that they're in the right order, so we need to explicitly only project the latest one + if (snapshot.Commit.HybridDateTime < latestProjected) + { + continue; + } } + latestProjectByEntityId[snapshot.EntityId] = snapshot.Commit.HybridDateTime; + + await ProjectSnapshot(snapshot); } try From 5986c5634f7da27729aa67c91624e00f971309cf Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 24 Oct 2025 14:49:52 +0200 Subject: [PATCH 5/9] Test no-op NewEntity --- src/SIL.Harmony.Tests/DeleteAndCreateTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs b/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs index f1a50c7..409f8bd 100644 --- a/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs +++ b/src/SIL.Harmony.Tests/DeleteAndCreateTests.cs @@ -185,4 +185,31 @@ await WriteNextChange(new DeleteChange(wordId), add: false), var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); entityWord.Should().BeNull(); } + + [Fact] + public async Task NewEntityOnExistingEntityIsNoOp() + { + var wordId = Guid.NewGuid(); + + await WriteNextChange(new NewWordChange(wordId, "original")); + var snapshotsBefore = await DbContext.Snapshots.Where(s => s.EntityId == wordId).ToArrayAsync(); + + await WriteNextChange( + [ + new NewWordChange(wordId, "Undeleted"), + ]); + + var word = await DataModel.GetLatest(wordId); + word.Should().NotBeNull(); + word.DeletedAt.Should().BeNull(); + word.Text.Should().Be("original"); + + var entityWord = await DataModel.QueryLatest().Where(w => w.Id == wordId).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.DeletedAt.Should().BeNull(); + entityWord.Text.Should().Be("original"); + + var snapshotsAfter = await DbContext.Snapshots.Where(s => s.EntityId == wordId).ToArrayAsync(); + snapshotsAfter.Select(s => s.Id).Should().BeEquivalentTo(snapshotsBefore.Select(s => s.Id)); + } } From 0fde24aad55aebd5a78de83d8fc710424aa1ffcf Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 24 Oct 2025 15:42:40 +0200 Subject: [PATCH 6/9] Add note explaining redundant try/catch --- src/SIL.Harmony/Db/CrdtRepository.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 9e3b983..d8df31b 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -332,6 +332,8 @@ public async Task AddSnapshots(IEnumerable snapshots) } } + // this extra try/catch was added as a quick way to get the NewEntityOnExistingEntityIsNoOp test to pass + // it will be removed again in a larger refactor in https://github.com/sillsdev/harmony/pull/56 try { await _dbContext.SaveChangesAsync(); From 277fc2db82c1887856197e77152eedc59604a914 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 14:35:47 +0100 Subject: [PATCH 7/9] Fix insufficient commit ordering --- src/SIL.Harmony/Db/CrdtRepository.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index d8df31b..98d1210 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -299,7 +299,7 @@ public async Task> GetChanges(SyncState remoteState) public async Task AddSnapshots(IEnumerable snapshots) { - var latestProjectByEntityId = new Dictionary(); + var latestProjectByEntityId = new Dictionary(); foreach (var grouping in snapshots.GroupBy(s => s.EntityIsDeleted).OrderByDescending(g => g.Key))//execute deletes first { foreach (var snapshot in grouping.DefaultOrderDescending()) @@ -309,12 +309,12 @@ public async Task AddSnapshots(IEnumerable snapshots) { // there might be a deleted and un-deleted snapshot for the same entity in the same batch // in that case there's only a 50% chance that they're in the right order, so we need to explicitly only project the latest one - if (snapshot.Commit.HybridDateTime < latestProjected) + if (snapshot.Commit.CompareKey.CompareTo(latestProjected) < 0) { continue; } } - latestProjectByEntityId[snapshot.EntityId] = snapshot.Commit.HybridDateTime; + latestProjectByEntityId[snapshot.EntityId] = snapshot.Commit.CompareKey; await ProjectSnapshot(snapshot); } From 5bbf01842ec267c511f58bafd5adc537ecf625b2 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 14:39:06 +0100 Subject: [PATCH 8/9] Add debug assert for what is now an unexpected code path. --- src/SIL.Harmony/Changes/Change.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index 7cf1794..ac1de0c 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Text.Json.Serialization; namespace SIL.Harmony.Changes; @@ -47,7 +48,11 @@ async ValueTask IChange.NewEntity(Commit commit, IChangeContext con public async ValueTask ApplyChange(IObjectBase entity, IChangeContext context) { if (!SupportsApplyChange()) + { + Debug.Fail("ApplyChange called on a Change that does not support it"); return; // skip attempting to apply changes on CreateChange as it does not support apply changes + } + if (entity.DbObject is T entityT) { await ApplyChange(entityT, context); From 4f0ce0a195cc5cfaf266dabbbdeee894fc1efe74 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 28 Oct 2025 14:39:33 +0100 Subject: [PATCH 9/9] Add comments/docs --- src/SIL.Harmony/Changes/Change.cs | 9 +++++++++ src/SIL.Harmony/SnapshotWorker.cs | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index ac1de0c..6908536 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -17,7 +17,16 @@ public interface IChange ValueTask ApplyChange(IObjectBase entity, IChangeContext context); ValueTask NewEntity(Commit commit, IChangeContext context); + /// + /// Indicates whether this change supports applying changes to an existing entity (whether deleted or not). + /// Essentially just avoids creating redundant snapshots for change objects that won't apply changes. + /// (e.g. redundant change objects intended only for NewEntity) + /// bool SupportsApplyChange(); + /// + /// Indicates whether this change supports creating entities (both creating brand new entities as well as recreating deleted entities). + /// Primarily for differentiating between updating vs recreating deleted entities. + /// bool SupportsNewEntity(); } diff --git a/src/SIL.Harmony/SnapshotWorker.cs b/src/SIL.Harmony/SnapshotWorker.cs index ff3cdcb..0cd822a 100644 --- a/src/SIL.Harmony/SnapshotWorker.cs +++ b/src/SIL.Harmony/SnapshotWorker.cs @@ -84,7 +84,7 @@ private async ValueTask ApplyCommitChanges(IEnumerable commits, bool upd if (prevSnapshot is null) { - // create brand new entity + // create brand new entity - this will (and should) throw if the change doesn't support NewEntity entity = await commitChange.Change.NewEntity(commit, changeContext); } else if (prevSnapshot.EntityIsDeleted && commitChange.Change.SupportsNewEntity()) @@ -106,9 +106,9 @@ private async ValueTask ApplyCommitChanges(IEnumerable commits, bool upd } else { - // entity already exists (and is not deleted) - // and change does not support updating existing entities - // so do nothing + // Entity already exists (and is not deleted) + // and change does not support updating existing entities, + // so do nothing. continue; }