From 7dfff7c46b0112872a5fa156a86c82080d410c71 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 25 Nov 2025 13:10:21 +0100 Subject: [PATCH 1/7] Add failing tests demonstrating inconsistency of add vs update --- .../Changes/AddAntonymReferenceChange.cs | 10 +- .../Changes/NewWordChange.cs | 5 +- src/SIL.Harmony.Sample/CrdtSampleKernel.cs | 6 +- src/SIL.Harmony.Sample/Models/Word.cs | 12 +- .../DataModelReferenceTests.cs | 170 +++++++++++++++++- .../DbContextTests.VerifyModel.verified.txt | 6 +- 6 files changed, 198 insertions(+), 11 deletions(-) diff --git a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs b/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs index 400c063..b4812db 100644 --- a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs +++ b/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs @@ -4,10 +4,11 @@ namespace SIL.Harmony.Sample.Changes; -public class AddAntonymReferenceChange(Guid entityId, Guid antonymId) +public class AddAntonymReferenceChange(Guid entityId, Guid antonymId, bool setObject = true) : EditChange(entityId), ISelfNamedType { public Guid AntonymId { get; set; } = antonymId; + public bool SetObject { get; set; } = setObject; public override async ValueTask ApplyChange(Word entity, IChangeContext context) { @@ -15,7 +16,10 @@ public override async ValueTask ApplyChange(Word entity, IChangeContext context) //then we don't want to apply the change //if the change was already applied, //then this reference is removed via Word.RemoveReference after the change which deletes the Antonym, see SnapshotWorker.MarkDeleted - if (!await context.IsObjectDeleted(AntonymId)) - entity.AntonymId = AntonymId; + var antonym = await context.GetCurrent(AntonymId); + if (antonym is null or { DeletedAt: not null }) return; + + if (SetObject) entity.Antonym = antonym; + entity.AntonymId = AntonymId; } } diff --git a/src/SIL.Harmony.Sample/Changes/NewWordChange.cs b/src/SIL.Harmony.Sample/Changes/NewWordChange.cs index 80f1641..2cf7c48 100644 --- a/src/SIL.Harmony.Sample/Changes/NewWordChange.cs +++ b/src/SIL.Harmony.Sample/Changes/NewWordChange.cs @@ -12,7 +12,8 @@ public class NewWordChange(Guid entityId, string text, string? note = null, Guid public override async ValueTask NewEntity(Commit commit, IChangeContext context) { - var antonymShouldBeNull = AntonymId is null || (await context.IsObjectDeleted(AntonymId.Value)); - return (new Word { Text = Text, Note = Note, Id = EntityId, AntonymId = antonymShouldBeNull ? null : AntonymId }); + var antonym = AntonymId is null ? null : await context.GetCurrent(AntonymId.Value); + antonym = antonym is { DeletedAt: null } ? antonym : null; + return new Word { Text = Text, Note = Note, Id = EntityId, Antonym = antonym, AntonymId = antonym?.Id }; } } diff --git a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs index a430f68..4ad48f9 100644 --- a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs +++ b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs @@ -60,6 +60,10 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi builder.HasMany(w => w.Tags) .WithMany() .UsingEntity(); + builder.HasOne((w) => w.Antonym) + .WithOne() + .HasForeignKey(w => w.AntonymId) + .OnDelete(DeleteBehavior.SetNull); }) .Add(builder => { @@ -81,4 +85,4 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi }); return services; } -} \ No newline at end of file +} diff --git a/src/SIL.Harmony.Sample/Models/Word.cs b/src/SIL.Harmony.Sample/Models/Word.cs index 955c24c..5a8fe05 100644 --- a/src/SIL.Harmony.Sample/Models/Word.cs +++ b/src/SIL.Harmony.Sample/Models/Word.cs @@ -9,6 +9,7 @@ public class Word : IObjectBase public Guid Id { get; init; } public DateTimeOffset? DeletedAt { get; set; } + public Word? Antonym { get; set; } public Guid? AntonymId { get; set; } public Guid? ImageResourceId { get; set; } public List Tags { get; set; } = new(); @@ -26,7 +27,11 @@ IEnumerable Refs() public void RemoveReference(Guid id, CommitBase commit) { - if (AntonymId == id) AntonymId = null; + if (AntonymId == id) + { + AntonymId = null; + Antonym = null; + } } public IObjectBase Copy() @@ -36,6 +41,7 @@ public IObjectBase Copy() Id = Id, Text = Text, Note = Note, + Antonym = Antonym, AntonymId = AntonymId, DeletedAt = DeletedAt, ImageResourceId = ImageResourceId, @@ -46,7 +52,7 @@ public IObjectBase Copy() public override string ToString() { return - $"{nameof(Text)}: {Text}, {nameof(Id)}: {Id}, {nameof(Note)}: {Note}, {nameof(DeletedAt)}: {DeletedAt}, {nameof(AntonymId)}: {AntonymId}, {nameof(ImageResourceId)}: {ImageResourceId}" + + $"{nameof(Text)}: {Text}, {nameof(Id)}: {Id}, {nameof(Note)}: {Note}, {nameof(DeletedAt)}: {DeletedAt}, {nameof(Antonym)}: {Antonym}, {nameof(AntonymId)}: {AntonymId}, {nameof(ImageResourceId)}: {ImageResourceId}" + $", {nameof(Tags)}: {string.Join(", ", Tags.Select(t => t.Text))}"; } -} \ No newline at end of file +} diff --git a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs index 5072362..a059a4c 100644 --- a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs @@ -17,6 +17,174 @@ public override async Task InitializeAsync() await WriteNextChange(SetWord(_word2Id, "entity2")); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddReferenceWorks(bool includeObjectInSnapshot) + { + // act + await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id, setObject: includeObjectInSnapshot)); + + // assert - snapshot + var entryWithRef = await DataModel.GetLatest(_word1Id); + entryWithRef.Should().NotBeNull(); + if (includeObjectInSnapshot) + { + entryWithRef.Antonym.Should().NotBeNull(); + entryWithRef.Antonym.Text.Should().Be("entity2"); + } + entryWithRef.AntonymId.Should().Be(_word2Id); + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == _word1Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity2"); + entityWord.AntonymId.Should().Be(_word2Id); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddEntityAndReferenceInSameCommitWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + + // act + await WriteNextChange( + [ + new NewWordChange(word3Id, "entity3"), + new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(word3Id); + word.Should().NotBeNull(); + word.Text.Should().Be("entity3"); + word.AntonymId.Should().Be(_word1Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity1"); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == word3Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.Text.Should().Be("entity3"); + entityWord.AntonymId.Should().Be(_word1Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity1"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddEntityAndReverseReferenceInSameCommitWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + + // act + await WriteNextChange( + [ + new NewWordChange(word3Id, "entity3"), + new AddAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(_word1Id); + word.Should().NotBeNull(); + word.Text.Should().Be("entity1"); + word.AntonymId.Should().Be(word3Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity3"); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == _word1Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.Text.Should().Be("entity1"); + entityWord.AntonymId.Should().Be(word3Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity3"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddEntityAndReferenceInSameSyncWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + + // act + await AddCommitsViaSync([ + await WriteNextChange(new NewWordChange(word3Id, "entity3"), add: false), + await WriteNextChange(new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), add: false), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(word3Id); + word.Should().NotBeNull(); + word.Text.Should().Be("entity3"); + word.AntonymId.Should().Be(_word1Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity1"); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == word3Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.Text.Should().Be("entity3"); + entityWord.AntonymId.Should().Be(_word1Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity1"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task AddEntityAndReverseReferenceInSameSyncWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + + // act + await AddCommitsViaSync([ + await WriteNextChange(new NewWordChange(word3Id, "entity3"), add: false), + await WriteNextChange(new AddAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), add: false), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(_word1Id); + word.Should().NotBeNull(); + word.Text.Should().Be("entity1"); + word.AntonymId.Should().Be(word3Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity3"); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == _word1Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.Text.Should().Be("entity1"); + entityWord.AntonymId.Should().Be(word3Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity3"); + } [Fact] public async Task DeleteAfterTheFactRewritesReferences() @@ -171,4 +339,4 @@ public async Task CanUpdateTagWithTheSameNameOutOfOrder() await WriteNextChange(SetTag(renameTagId, tagText)); DataModel.QueryLatest().ToBlockingEnumerable().Where(t => t.Text == tagText).Should().ContainSingle(); } -} \ No newline at end of file +} diff --git a/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt b/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt index 7ad2c2d..742ba7d 100644 --- a/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt +++ b/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt @@ -189,19 +189,23 @@ EntityType: Word Properties: Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd - AntonymId (Guid?) + AntonymId (Guid?) FK Index DeletedAt (DateTimeOffset?) ImageResourceId (Guid?) Note (string) SnapshotId (no field, Guid?) Shadow FK Index Text (string) Required + Navigations: + Antonym (Word) ToPrincipal Word Skip navigations: Tags (List) CollectionTag Inverse: Word Keys: Id PK Foreign keys: + Word {'AntonymId'} -> Word {'Id'} Unique SetNull ToPrincipal: Antonym Word {'SnapshotId'} -> ObjectSnapshot {'Id'} Unique SetNull Indexes: + AntonymId Unique SnapshotId Unique Annotations: DiscriminatorProperty: From 6961cdce40b3e3247c777f9e2d489136385dfe31 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 25 Nov 2025 13:24:32 +0100 Subject: [PATCH 2/7] Tweak work-antonym relationship --- src/SIL.Harmony.Sample/CrdtSampleKernel.cs | 4 ++-- src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs index 4ad48f9..8b31e32 100644 --- a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs +++ b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs @@ -61,8 +61,8 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi .WithMany() .UsingEntity(); builder.HasOne((w) => w.Antonym) - .WithOne() - .HasForeignKey(w => w.AntonymId) + .WithMany() + .HasForeignKey(w => w.AntonymId) .OnDelete(DeleteBehavior.SetNull); }) .Add(builder => diff --git a/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt b/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt index 742ba7d..79ccc2f 100644 --- a/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt +++ b/src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt @@ -202,10 +202,10 @@ Keys: Id PK Foreign keys: - Word {'AntonymId'} -> Word {'Id'} Unique SetNull ToPrincipal: Antonym + Word {'AntonymId'} -> Word {'Id'} SetNull ToPrincipal: Antonym Word {'SnapshotId'} -> ObjectSnapshot {'Id'} Unique SetNull Indexes: - AntonymId Unique + AntonymId SnapshotId Unique Annotations: DiscriminatorProperty: From 615a7a5597cb9cf938dd86eb912dfebe7f049465 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 25 Nov 2025 13:25:29 +0100 Subject: [PATCH 3/7] Prevent persisting an Antonym that does not match AntonymId --- src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs b/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs index b4812db..e966814 100644 --- a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs +++ b/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs @@ -19,7 +19,7 @@ public override async ValueTask ApplyChange(Word entity, IChangeContext context) var antonym = await context.GetCurrent(AntonymId); if (antonym is null or { DeletedAt: not null }) return; - if (SetObject) entity.Antonym = antonym; + entity.Antonym = SetObject ? antonym : null; entity.AntonymId = AntonymId; } } From 42209dea552fabd332451c7e8d46fd0feaab2e03 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 25 Nov 2025 13:27:07 +0100 Subject: [PATCH 4/7] Stop marking whole entity graph as added --- src/SIL.Harmony/Db/CrdtRepository.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 98d1210..5bab4f2 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -362,8 +362,11 @@ private async ValueTask ProjectSnapshot(ObjectSnapshot objectSnapshot) //if we don't make a copy first then the entity will be tracked by the context and be modified //by future changes in the same session var entity = objectSnapshot.Entity.Copy().DbObject; - _dbContext.Add(entity) - .Property(ObjectSnapshot.ShadowRefName).CurrentValue = objectSnapshot.Id; + + var entry = _dbContext.Entry(entity); + // only mark this single entry as added, rather than the whole graph (this matches the update behaviour below) + entry.State = EntityState.Added; + entry.Property(ObjectSnapshot.ShadowRefName).CurrentValue = objectSnapshot.Id; } else if (objectSnapshot.EntityIsDeleted) // delete { From e64c2ff9ca79a04874278a9ec19f4ac2e207ec00 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 28 Nov 2025 12:04:52 +0100 Subject: [PATCH 5/7] PR feedback --- .../DataModelReferenceTests.cs | 108 +++++++++++++++++- src/SIL.Harmony/Db/CrdtRepository.cs | 6 +- 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs index a059a4c..f4f3cab 100644 --- a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs @@ -26,22 +26,104 @@ public async Task AddReferenceWorks(bool includeObjectInSnapshot) await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id, setObject: includeObjectInSnapshot)); // assert - snapshot - var entryWithRef = await DataModel.GetLatest(_word1Id); - entryWithRef.Should().NotBeNull(); + var word = await DataModel.GetLatest(_word1Id); + word.Should().NotBeNull(); + word.AntonymId.Should().Be(_word2Id); if (includeObjectInSnapshot) { - entryWithRef.Antonym.Should().NotBeNull(); - entryWithRef.Antonym.Text.Should().Be("entity2"); + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity2"); + } + else + { + word.Antonym.Should().BeNull(); } - entryWithRef.AntonymId.Should().Be(_word2Id); // assert - projected entity var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) .Where(w => w.Id == _word1Id).SingleOrDefaultAsync(); entityWord.Should().NotBeNull(); + entityWord.AntonymId.Should().Be(_word2Id); entityWord.Antonym.Should().NotBeNull(); entityWord.Antonym.Text.Should().Be("entity2"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UpdateReferenceTwiceInSameCommitWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + await WriteNextChange(new NewWordChange(word3Id, "entity3")); + + // act + await WriteNextChange( + [ + new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + new AddAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(word3Id); + word.Should().NotBeNull(); + word.AntonymId.Should().Be(_word2Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity2"); + } + else + { + word.Antonym.Should().BeNull(); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == word3Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); entityWord.AntonymId.Should().Be(_word2Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity2"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UpdateReferenceTwiceInSameSyncWorks(bool includeObjectInSnapshot) + { + // arrange + var word3Id = Guid.NewGuid(); + await WriteNextChange(new NewWordChange(word3Id, "entity3")); + + // act + await WriteNextChange( + [ + new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + new AddAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), + ]); + + // assert - snapshot + var word = await DataModel.GetLatest(word3Id); + word.Should().NotBeNull(); + word.AntonymId.Should().Be(_word2Id); + if (includeObjectInSnapshot) + { + word.Antonym.Should().NotBeNull(); + word.Antonym.Text.Should().Be("entity2"); + } + else + { + word.Antonym.Should().BeNull(); + } + + // assert - projected entity + var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) + .Where(w => w.Id == word3Id).SingleOrDefaultAsync(); + entityWord.Should().NotBeNull(); + entityWord.AntonymId.Should().Be(_word2Id); + entityWord.Antonym.Should().NotBeNull(); + entityWord.Antonym.Text.Should().Be("entity2"); } [Theory] @@ -69,6 +151,10 @@ await WriteNextChange( word.Antonym.Should().NotBeNull(); word.Antonym.Text.Should().Be("entity1"); } + else + { + word.Antonym.Should().BeNull(); + } // assert - projected entity var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) @@ -105,6 +191,10 @@ await WriteNextChange( word.Antonym.Should().NotBeNull(); word.Antonym.Text.Should().Be("entity3"); } + else + { + word.Antonym.Should().BeNull(); + } // assert - projected entity var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) @@ -140,6 +230,10 @@ await WriteNextChange(new AddAntonymReferenceChange(word3Id, _word1Id, setObject word.Antonym.Should().NotBeNull(); word.Antonym.Text.Should().Be("entity1"); } + else + { + word.Antonym.Should().BeNull(); + } // assert - projected entity var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) @@ -175,6 +269,10 @@ await WriteNextChange(new AddAntonymReferenceChange(_word1Id, word3Id, setObject word.Antonym.Should().NotBeNull(); word.Antonym.Text.Should().Be("entity3"); } + else + { + word.Antonym.Should().BeNull(); + } // assert - projected entity var entityWord = await DataModel.QueryLatest(w => w.Include(w => w.Antonym)) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 5bab4f2..1fcae0e 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -363,10 +363,10 @@ private async ValueTask ProjectSnapshot(ObjectSnapshot objectSnapshot) //by future changes in the same session var entity = objectSnapshot.Entity.Copy().DbObject; - var entry = _dbContext.Entry(entity); + var newEntry = _dbContext.Entry(entity); // only mark this single entry as added, rather than the whole graph (this matches the update behaviour below) - entry.State = EntityState.Added; - entry.Property(ObjectSnapshot.ShadowRefName).CurrentValue = objectSnapshot.Id; + newEntry.State = EntityState.Added; + newEntry.Property(ObjectSnapshot.ShadowRefName).CurrentValue = objectSnapshot.Id; } else if (objectSnapshot.EntityIsDeleted) // delete { From c40bf8e6829d9b152fac2c0effa1d0f855e1b300 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 28 Nov 2025 12:06:14 +0100 Subject: [PATCH 6/7] Rename AddAntonymReferenceChange, because it sets a single object. It does not add to a collection. --- ...Change.cs => SetAntonymReferenceChange.cs} | 4 +-- src/SIL.Harmony.Sample/CrdtSampleKernel.cs | 2 +- .../DataModelReferenceTests.cs | 28 +++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) rename src/SIL.Harmony.Sample/Changes/{AddAntonymReferenceChange.cs => SetAntonymReferenceChange.cs} (88%) diff --git a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs b/src/SIL.Harmony.Sample/Changes/SetAntonymReferenceChange.cs similarity index 88% rename from src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs rename to src/SIL.Harmony.Sample/Changes/SetAntonymReferenceChange.cs index e966814..4e02ed1 100644 --- a/src/SIL.Harmony.Sample/Changes/AddAntonymReferenceChange.cs +++ b/src/SIL.Harmony.Sample/Changes/SetAntonymReferenceChange.cs @@ -4,8 +4,8 @@ namespace SIL.Harmony.Sample.Changes; -public class AddAntonymReferenceChange(Guid entityId, Guid antonymId, bool setObject = true) - : EditChange(entityId), ISelfNamedType +public class SetAntonymReferenceChange(Guid entityId, Guid antonymId, bool setObject = true) + : EditChange(entityId), ISelfNamedType { public Guid AntonymId { get; set; } = antonymId; public bool SetObject { get; set; } = setObject; diff --git a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs index 8b31e32..761af81 100644 --- a/src/SIL.Harmony.Sample/CrdtSampleKernel.cs +++ b/src/SIL.Harmony.Sample/CrdtSampleKernel.cs @@ -43,7 +43,7 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi .Add() .Add() .Add() - .Add() + .Add() .Add() .Add>() .Add() diff --git a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs index f4f3cab..ed70888 100644 --- a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs @@ -23,7 +23,7 @@ public override async Task InitializeAsync() public async Task AddReferenceWorks(bool includeObjectInSnapshot) { // act - await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id, setObject: includeObjectInSnapshot)); + await WriteNextChange(new SetAntonymReferenceChange(_word1Id, _word2Id, setObject: includeObjectInSnapshot)); // assert - snapshot var word = await DataModel.GetLatest(_word1Id); @@ -60,8 +60,8 @@ public async Task UpdateReferenceTwiceInSameCommitWorks(bool includeObjectInSnap // act await WriteNextChange( [ - new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), - new AddAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), ]); // assert - snapshot @@ -99,8 +99,8 @@ public async Task UpdateReferenceTwiceInSameSyncWorks(bool includeObjectInSnapsh // act await WriteNextChange( [ - new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), - new AddAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), ]); // assert - snapshot @@ -138,7 +138,7 @@ public async Task AddEntityAndReferenceInSameCommitWorks(bool includeObjectInSna await WriteNextChange( [ new NewWordChange(word3Id, "entity3"), - new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), ]); // assert - snapshot @@ -178,7 +178,7 @@ public async Task AddEntityAndReverseReferenceInSameCommitWorks(bool includeObje await WriteNextChange( [ new NewWordChange(word3Id, "entity3"), - new AddAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), + new SetAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), ]); // assert - snapshot @@ -217,7 +217,7 @@ public async Task AddEntityAndReferenceInSameSyncWorks(bool includeObjectInSnaps // act await AddCommitsViaSync([ await WriteNextChange(new NewWordChange(word3Id, "entity3"), add: false), - await WriteNextChange(new AddAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), add: false), + await WriteNextChange(new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), add: false), ]); // assert - snapshot @@ -256,7 +256,7 @@ public async Task AddEntityAndReverseReferenceInSameSyncWorks(bool includeObject // act await AddCommitsViaSync([ await WriteNextChange(new NewWordChange(word3Id, "entity3"), add: false), - await WriteNextChange(new AddAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), add: false), + await WriteNextChange(new SetAntonymReferenceChange(_word1Id, word3Id, setObject: includeObjectInSnapshot), add: false), ]); // assert - snapshot @@ -287,7 +287,7 @@ await WriteNextChange(new AddAntonymReferenceChange(_word1Id, word3Id, setObject [Fact] public async Task DeleteAfterTheFactRewritesReferences() { - var addRef = await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id)); + var addRef = await WriteNextChange(new SetAntonymReferenceChange(_word1Id, _word2Id)); var entryWithRef = await DataModel.GetLatest(_word1Id); entryWithRef!.AntonymId.Should().Be(_word2Id); @@ -299,7 +299,7 @@ public async Task DeleteAfterTheFactRewritesReferences() [Fact] public async Task DeleteRemovesAllReferences() { - await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id)); + await WriteNextChange(new SetAntonymReferenceChange(_word1Id, _word2Id)); var entryWithRef = await DataModel.GetLatest(_word1Id); entryWithRef!.AntonymId.Should().Be(_word2Id); @@ -311,7 +311,7 @@ public async Task DeleteRemovesAllReferences() [Fact] public async Task SnapshotsDontGetMutatedByADelete() { - var refAdd = await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id)); + var refAdd = await WriteNextChange(new SetAntonymReferenceChange(_word1Id, _word2Id)); await WriteNextChange(new DeleteChange(_word2Id)); var word = await DataModel.GetAtCommit(refAdd.Id, _word1Id); word.Should().NotBeNull(); @@ -323,11 +323,11 @@ public async Task DeleteRetroactivelyRemovesRefs() { var entityId3 = Guid.NewGuid(); await WriteNextChange(SetWord(entityId3, "entity3")); - await WriteNextChange(new AddAntonymReferenceChange(_word1Id, _word2Id)); + await WriteNextChange(new SetAntonymReferenceChange(_word1Id, _word2Id)); var delete = await WriteNextChange(new DeleteChange(_word2Id)); //a ref was synced in the past, it happened before the delete, the reference should be retroactively removed - await WriteChangeBefore(delete, new AddAntonymReferenceChange(entityId3, _word2Id)); + await WriteChangeBefore(delete, new SetAntonymReferenceChange(entityId3, _word2Id)); var entry = await DataModel.GetLatest(entityId3); entry!.AntonymId.Should().BeNull(); } From e28052b0bd9ae9aba643e31ceba1aee7bdd7a02f Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Fri, 28 Nov 2025 13:50:16 +0100 Subject: [PATCH 7/7] Fix test logic --- src/SIL.Harmony.Tests/DataModelReferenceTests.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs index ed70888..cad2545 100644 --- a/src/SIL.Harmony.Tests/DataModelReferenceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelReferenceTests.cs @@ -97,11 +97,10 @@ public async Task UpdateReferenceTwiceInSameSyncWorks(bool includeObjectInSnapsh await WriteNextChange(new NewWordChange(word3Id, "entity3")); // act - await WriteNextChange( - [ - new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), - new SetAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), - ]); + await AddCommitsViaSync([ + await WriteNextChange(new SetAntonymReferenceChange(word3Id, _word1Id, setObject: includeObjectInSnapshot), add: false), + await WriteNextChange(new SetAntonymReferenceChange(word3Id, _word2Id, setObject: includeObjectInSnapshot), add: false), + ]); // assert - snapshot var word = await DataModel.GetLatest(word3Id);