Fix referencing added entries in sync (#1594)

* Fix referencing added entries in sync

* write a test for the changes made to DiffAddThenUpdate

---------

Co-authored-by: Kevin Hahn <[email protected]>

Author: myieye

backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs

@@ -8,7 +8,7 @@
 
 namespace FwLiteProjectSync.Tests;
 
-public class EntrySyncTests : IClassFixture<SyncFixture>, IAsyncLifetime
+public class CrdtEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture)
 {
     private static readonly AutoFaker AutoFaker = new(new AutoFakerConfig()
     {
@@ -22,37 +22,25 @@ public class EntrySyncTests : IClassFixture<SyncFixture>, IAsyncLifetime
         ]
     });
 
-    public EntrySyncTests(SyncFixture fixture)
+    protected override IMiniLcmApi GetApi(SyncFixture fixture)
     {
-        _fixture = fixture;
+        return fixture.CrdtApi;
     }
 
-    public async Task InitializeAsync()
-    {
-        await _fixture.EnsureDefaultVernacularWritingSystemExistsInCrdt();
-    }
-
-    public Task DisposeAsync()
-    {
-        return Task.CompletedTask;
-    }
-
-    private readonly SyncFixture _fixture;
-
     [Fact]
     public async Task CanSyncRandomEntries()
     {
-        var createdEntry = await _fixture.CrdtApi.CreateEntry(await AutoFaker.EntryReadyForCreation(_fixture.CrdtApi));
-        var after = await AutoFaker.EntryReadyForCreation(_fixture.CrdtApi, entryId: createdEntry.Id);
+        var createdEntry = await Api.CreateEntry(await AutoFaker.EntryReadyForCreation(Api));
+        var after = await AutoFaker.EntryReadyForCreation(Api, entryId: createdEntry.Id);
 
         after.Senses = [.. AutoFaker.Faker.Random.Shuffle([
                 // copy some senses over, so moves happen
                 ..AutoFaker.Faker.Random.ListItems(createdEntry.Senses),
                 ..after.Senses
             ])];
 
-        await EntrySync.Sync(createdEntry, after, _fixture.CrdtApi);
-        var actual = await _fixture.CrdtApi.GetEntry(after.Id);
+        await EntrySync.Sync(createdEntry, after, Api);
+        var actual = await Api.GetEntry(after.Id);
         actual.Should().NotBeNull();
         actual.Should().BeEquivalentTo(after, options => options
             .For(e => e.Senses).Exclude(s => s.Order)
@@ -61,14 +49,41 @@ public async Task CanSyncRandomEntries()
             .For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order)
             );
     }
+}
+
+public class FwDataEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture)
+{
+    protected override IMiniLcmApi GetApi(SyncFixture fixture)
+    {
+        return fixture.FwDataApi;
+    }
+}
+
+public abstract class EntrySyncTestsBase(SyncFixture fixture) : IClassFixture<SyncFixture>, IAsyncLifetime
+{
+    public async Task InitializeAsync()
+    {
+        await _fixture.EnsureDefaultVernacularWritingSystemExistsInCrdt();
+        Api = GetApi(_fixture);
+    }
+
+    public Task DisposeAsync()
+    {
+        return Task.CompletedTask;
+    }
+
+    protected abstract IMiniLcmApi GetApi(SyncFixture fixture);
+
+    private readonly SyncFixture _fixture = fixture;
+    protected IMiniLcmApi Api = null!;
 
     [Fact]
     public async Task CanChangeComplexFormViaSync_Components()
     {
-        var component1 = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "component1" } } });
-        var component2 = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "component2" } } });
+        var component1 = await Api.CreateEntry(new() { LexemeForm = { { "en", "component1" } } });
+        var component2 = await Api.CreateEntry(new() { LexemeForm = { { "en", "component2" } } });
         var complexFormId = Guid.NewGuid();
-        var complexForm = await _fixture.CrdtApi.CreateEntry(new()
+        var complexForm = await Api.CreateEntry(new()
         {
             Id = complexFormId,
             LexemeForm = { { "en", "complex form" } },
@@ -83,24 +98,25 @@ public async Task CanChangeComplexFormViaSync_Components()
                 }
             ]
         });
-        Entry after = (Entry) complexForm.Copy();
-        after.Components[0].ComponentEntryId = component2.Id;
-        after.Components[0].ComponentHeadword = component2.Headword();
+        var complexFormAfter = complexForm.Copy();
+        complexFormAfter.Components[0].ComponentEntryId = component2.Id;
+        complexFormAfter.Components[0].ComponentHeadword = component2.Headword();
 
-        await EntrySync.Sync(complexForm, after, _fixture.CrdtApi);
+        await EntrySync.Sync(complexForm, complexFormAfter, Api);
 
-        var actual = await _fixture.CrdtApi.GetEntry(after.Id);
+        var actual = await Api.GetEntry(complexFormAfter.Id);
         actual.Should().NotBeNull();
-        actual.Should().BeEquivalentTo(after, options => options);
+        actual.Should().BeEquivalentTo(complexFormAfter, options => options
+            .For(e => e.Components).Exclude(c => c.Id));
     }
 
     [Fact]
     public async Task CanChangeComplexFormViaSync_ComplexForms()
     {
-        var complexForm1 = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm1" } } });
-        var complexForm2 = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm2" } } });
+        var complexForm1 = await Api.CreateEntry(new() { LexemeForm = { { "en", "complexForm1" } } });
+        var complexForm2 = await Api.CreateEntry(new() { LexemeForm = { { "en", "complexForm2" } } });
         var componentId = Guid.NewGuid();
-        var component = await _fixture.CrdtApi.CreateEntry(new()
+        var component = await Api.CreateEntry(new()
         {
             Id = componentId,
             LexemeForm = { { "en", "component" } },
@@ -115,47 +131,44 @@ public async Task CanChangeComplexFormViaSync_ComplexForms()
                 }
             ]
         });
-        Entry after = (Entry) component.Copy();
-        after.ComplexForms[0].ComplexFormEntryId = complexForm2.Id;
-        after.ComplexForms[0].ComplexFormHeadword = complexForm2.Headword();
+        var componentAter = component.Copy();
+        componentAter.ComplexForms[0].ComplexFormEntryId = complexForm2.Id;
+        componentAter.ComplexForms[0].ComplexFormHeadword = complexForm2.Headword();
 
-        await EntrySync.Sync(component, after, _fixture.CrdtApi);
+        await EntrySync.Sync(component, componentAter, Api);
 
-        var actual = await _fixture.CrdtApi.GetEntry(after.Id);
+        var actual = await Api.GetEntry(componentAter.Id);
         actual.Should().NotBeNull();
-        actual.Should().BeEquivalentTo(after, options => options);
+        actual.Should().BeEquivalentTo(componentAter, options => options
+            .For(e => e.ComplexForms).Exclude(c => c.Id));
     }
 
     [Fact]
     public async Task CanChangeComplexFormTypeViaSync()
     {
-        var complexFormType = await _fixture.CrdtApi.CreateComplexFormType(new() { Name = new() { { "en", "complexFormType" } } });
-        var entry = await _fixture.CrdtApi.CreateEntry(new() { LexemeForm = { { "en", "complexForm1" } } });
-        var after = (Entry) entry.Copy();
+        var complexFormType = await Api.CreateComplexFormType(new() { Name = new() { { "en", "complexFormType" } } });
+        var entry = await Api.CreateEntry(new() { LexemeForm = { { "en", "complexForm1" } } });
+        var after = entry.Copy();
         after.ComplexFormTypes = [complexFormType];
-        await EntrySync.Sync(entry, after, _fixture.CrdtApi);
+        await EntrySync.Sync(entry, after, Api);
 
-        var actual = await _fixture.CrdtApi.GetEntry(after.Id);
+        var actual = await Api.GetEntry(after.Id);
         actual.Should().NotBeNull();
         actual.Should().BeEquivalentTo(after, options => options);
     }
 
     [Theory]
-    [InlineData(true, ProjectDataFormat.Harmony)]
-    [InlineData(true, ProjectDataFormat.FwData)]
-    [InlineData(false, ProjectDataFormat.Harmony)]
-    [InlineData(false, ProjectDataFormat.FwData)]
-    public async Task CanInsertComplexFormComponentViaSync(bool componentThenComplexForm, ProjectDataFormat apiType)
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task CanInsertComplexFormComponentViaSync(bool componentThenComplexForm)
     {
         // arrange
-        IMiniLcmApi api = apiType == ProjectDataFormat.Harmony ? _fixture.CrdtApi : _fixture.FwDataApi;
-
-        var existingComponent = await api.CreateEntry(new() { LexemeForm = { { "en", "existing-component" } } });
-        var complexFormBefore = await api.CreateEntry(new() { LexemeForm = { { "en", "complex-form" } } });
-        await api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(complexFormBefore, existingComponent));
-        complexFormBefore = (await api.GetEntry(complexFormBefore.Id))!;
+        var existingComponent = await Api.CreateEntry(new() { LexemeForm = { { "en", "existing-component" } } });
+        var complexFormBefore = await Api.CreateEntry(new() { LexemeForm = { { "en", "complex-form" } } });
+        await Api.CreateComplexFormComponent(ComplexFormComponent.FromEntries(complexFormBefore, existingComponent));
+        complexFormBefore = (await Api.GetEntry(complexFormBefore.Id))!;
 
-        var newComponentBefore = await api.CreateEntry(new() { LexemeForm = { { "en", "component" } } });
+        var newComponentBefore = await Api.CreateEntry(new() { LexemeForm = { { "en", "component" } } });
         var newComplexFormComponent = ComplexFormComponent.FromEntries(complexFormBefore, newComponentBefore);
 
         var newComponentAfter = newComponentBefore.Copy();
@@ -169,18 +182,18 @@ public async Task CanInsertComplexFormComponentViaSync(bool componentThenComplex
             // this results in 2 crdt changes:
             // (1) add complex-form (i.e. implicitly add component)
             // (2) move component to the right place
-            await EntrySync.Sync([newComponentBefore, complexFormBefore], [newComponentAfter, complexFormAfter], api);
+            await EntrySync.Sync([newComponentBefore, complexFormBefore], [newComponentAfter, complexFormAfter], Api);
         }
         else
         {
             // this results in 1 crdt change:
             // the component is added in the right place
             // (adding the complex-form becomes a no-op, because it already exists and a BetweenPosition is not specified)
-            await EntrySync.Sync([complexFormBefore, newComponentBefore], [complexFormAfter, newComponentAfter], api);
+            await EntrySync.Sync([complexFormBefore, newComponentBefore], [complexFormAfter, newComponentAfter], Api);
         }
 
         // assert
-        var actual = await api.GetEntry(complexFormAfter.Id);
+        var actual = await Api.GetEntry(complexFormAfter.Id);
         actual.Should().NotBeNull();
         actual.Should().BeEquivalentTo(complexFormAfter, options => options
             .WithStrictOrdering()
@@ -191,4 +204,34 @@ public async Task CanInsertComplexFormComponentViaSync(bool componentThenComplex
             .Excluding(c => c.Order)
             .Excluding(c => c.Id));
     }
+
+    [Fact]
+    public async Task CanSyncNewEntryReferencedByExistingEntry()
+    {
+        // arrange
+        // - before
+        var existingEntryBefore = await Api.CreateEntry(new() { LexemeForm = { { "en", "existing-component" } } });
+
+        // - after
+        var existingEntryAfter = existingEntryBefore.Copy();
+        var newEntry = new Entry() { Id = Guid.NewGuid(), LexemeForm = { { "en", "complex-form" } } };
+        var newComplexFormComponent = ComplexFormComponent.FromEntries(newEntry, existingEntryAfter);
+        existingEntryAfter.ComplexForms.Add(newComplexFormComponent);
+        newEntry.Components.Add(newComplexFormComponent);
+
+        // act
+        await EntrySync.Sync([existingEntryBefore], [existingEntryAfter, newEntry], Api);
+
+        // assert
+        var actualExistingEntry = await Api.GetEntry(existingEntryAfter.Id);
+        actualExistingEntry.Should().BeEquivalentTo(existingEntryAfter, options => options
+            .For(e => e.ComplexForms).Exclude(c => c.Id)
+            .For(e => e.ComplexForms).Exclude(c => c.Order));
+
+        var actualNewEntry = await Api.GetEntry(newEntry.Id);
+        actualNewEntry.Should().BeEquivalentTo(newEntry, options => options
+            .Excluding(e => e.ComplexFormTypes) // LibLcm automatically creates a complex form type. Should we?
+            .For(e => e.Components).Exclude(c => c.Id)
+            .For(e => e.Components).Exclude(c => c.Order));
+    }
 }

backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs

@@ -85,7 +85,7 @@ public void DeleteSyncSnapshot()
         if (File.Exists(snapshotPath)) File.Delete(snapshotPath);
     }
 
-    private static readonly SemaphoreSlim _vernacularSemaphore = new(1, 1);
+    private readonly SemaphoreSlim _vernacularSemaphore = new(1, 1);
 
     // a vernacular writing system is required in order to query for entries
     // this is optional setup, because our core sync integration tests benefit from having a CRDT project that's as empty as possible

backend/FwLite/MiniLcm.Tests/DiffCollectionTests.cs

@@ -1,4 +1,6 @@
+using System.Runtime.CompilerServices;
 using MiniLcm.SyncHelpers;
+using Moq;
 
 namespace MiniLcm.Tests;
 
@@ -192,6 +194,121 @@ private static CollectionDiffOperation Remove(TestOrderable value)
     {
         return new CollectionDiffOperation(value, PositionDiffKind.Remove);
     }
+
+
+    public record Entry(Guid Id, string Word);
+
+    private readonly FakeDiffApi _fakeApi = new();
+
+    [Fact]
+    public async Task Diff_CallsAddForNewRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        await DiffCollection.Diff([], [entry], _fakeApi);
+        _fakeApi.VerifyCalls(new FakeDiffApi.MethodCall(entry, nameof(FakeDiffApi.Add)));
+    }
+
+    [Fact]
+    public async Task Diff_CallsRemoveForMissingRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        await DiffCollection.Diff([entry], [], _fakeApi);
+        _fakeApi.VerifyCalls(new FakeDiffApi.MethodCall(entry, nameof(FakeDiffApi.Remove)));
+    }
+
+    [Fact]
+    public async Task Diff_CallsReplaceForMatchingRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        var updated = entry with { Word = "new" };
+        await DiffCollection.Diff([entry], [updated], _fakeApi);
+        _fakeApi.VerifyCalls(new FakeDiffApi.MethodCall((entry, updated), nameof(FakeDiffApi.Replace)));
+    }
+
+    [Fact]
+    public async Task Diff_AddThenUpdate_CallsAddForNewRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        await DiffCollection.DiffAddThenUpdate([], [entry], _fakeApi);
+        _fakeApi.VerifyCalls(
+            new FakeDiffApi.MethodCall(entry, nameof(FakeDiffApi.AddAndGet)),
+            new FakeDiffApi.MethodCall((entry, entry), nameof(FakeDiffApi.Replace))
+        );
+    }
+
+    [Fact]
+    public async Task DiffAddThenUpdate_CallsRemoveForMissingRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        await DiffCollection.DiffAddThenUpdate([entry], [], _fakeApi);
+        _fakeApi.VerifyCalls(new FakeDiffApi.MethodCall(entry, nameof(FakeDiffApi.Remove)));
+    }
+
+    [Fact]
+    public async Task DiffAddThenUpdate_CallsReplaceForMatchingRecords()
+    {
+        var entry = new Entry(Guid.NewGuid(), "test");
+        var updated = entry with { Word = "new" };
+        await DiffCollection.DiffAddThenUpdate([entry], [updated], _fakeApi);
+        _fakeApi.VerifyCalls(new FakeDiffApi.MethodCall((entry, updated), nameof(FakeDiffApi.Replace)));
+    }
+
+    [Fact]
+    public async Task DiffAddThenUpdate_AddAlwaysBeforeReplace()
+    {
+        var newEntry = new Entry(Guid.NewGuid(), "new");
+        var oldEntry = new Entry(Guid.NewGuid(), "test");
+        var updated = oldEntry with { Word = "new" };
+        await DiffCollection.DiffAddThenUpdate([oldEntry], [updated, newEntry], _fakeApi);
+        //this order is required because the new entry must be created before the updated entry is modified.
+        //the updated entry might reference the newEntry and so must be updated after the new entry is created.
+        //the order that the replace calls are made is unimportant.
+        _fakeApi.VerifyCalls(
+            new FakeDiffApi.MethodCall(newEntry, nameof(FakeDiffApi.AddAndGet)),
+            new FakeDiffApi.MethodCall((oldEntry, updated), nameof(FakeDiffApi.Replace)),
+            new FakeDiffApi.MethodCall((newEntry, newEntry), nameof(FakeDiffApi.Replace))
+        );
+    }
+
+    private class FakeDiffApi: CollectionDiffApi<Entry, Guid>
+    {
+        public record MethodCall(object Args, [CallerMemberName] string Name = "");
+
+        public List<MethodCall> Calls { get; set; } = [];
+        public override Task<int> Add(Entry value)
+        {
+            Calls.Add(new(value));
+            return Task.FromResult(1);
+        }
+
+        public override Task<int> Remove(Entry value)
+        {
+            Calls.Add(new(value));
+            return Task.FromResult(1);
+        }
+
+        public override Task<int> Replace(Entry before, Entry after)
+        {
+            Calls.Add(new((before, after)));
+            return Task.FromResult(1);
+        }
+
+        public override Task<(int, Entry)> AddAndGet(Entry value)
+        {
+            Calls.Add(new(value));
+            return Task.FromResult((1, value));
+        }
+
+        public override Guid GetId(Entry value)
+        {
+            return value.Id;
+        }
+
+        public void VerifyCalls(params MethodCall[] expectedCalls)
+        {
+            Calls.Should().BeEquivalentTo(expectedCalls, o => o.WithStrictOrdering());
+        }
+    }
 }
 
 public class CollectionDiffTestCase