Make NewEntry override deleted

src/SIL.Harmony.Tests/DataModelSimpleChanges.cs

@@ -190,28 +190,46 @@ 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<Word>(_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>();
-        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<Word>(_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>();
+        word.Text.Should().Be("after-undo-delete");
+        word.DeletedAt.Should().Be(null);
+    }
+
     [Fact]
     public async Task ChangesToSnapshotsAreNotSaved()
     {
         await WriteNextChange(SetWord(_entity1Id, "test-value"));
         var word = await DataModel.GetLatest<Word>(_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<Word>().Text.Should().Be("after-change");

src/SIL.Harmony.Tests/DeleteAndCreateTests.cs

@@ -0,0 +1,215 @@
+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<Word>(wordId),
+                new NewWordChange(wordId, "Undeleted"),
+            ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId), add: false),
+            await WriteNextChange(new NewWordChange(wordId, "Undeleted"), add: false),
+        ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId));
+
+        await WriteNextChange([
+            new SetWordNoteChange(wordId, "overridden-note"),
+            new NewWordChange(wordId, "Undeleted"),
+        ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(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<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId),
+                new NewWordChange(wordId, "Undeleted"),
+            ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId), add: false),
+            await WriteNextChange(new NewWordChange(wordId, "Undeleted"), add: false),
+        ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("Undeleted");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId),
+            ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().NotBeNull();
+        word.Text.Should().Be("original");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId), add: false),
+        ]);
+
+        var word = await DataModel.GetLatest<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().NotBeNull();
+        word.Text.Should().Be("original");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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<Word>(wordId);
+        word.Should().NotBeNull();
+        word.DeletedAt.Should().BeNull();
+        word.Text.Should().Be("original");
+
+        var entityWord = await DataModel.QueryLatest<Word>().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));
+    }
+}

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<Commit>();
-        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<Commit>();
-        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<Word>(q=> q.Include(w => w.Tags)
+        var word = await DataModel.QueryLatest<Word>(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" }]);

src/SIL.Harmony/Changes/Change.cs

@@ -16,6 +16,8 @@ public interface IChange
 
     ValueTask ApplyChange(IObjectBase entity, IChangeContext context);
     ValueTask<IObjectBase> NewEntity(Commit commit, IChangeContext context);
+    bool SupportsApplyChange();
+    bool SupportsNewEntity();
 }
 
 /// <summary>
@@ -44,7 +46,7 @@ async ValueTask<IObjectBase> IChange.NewEntity(Commit commit, IChangeContext con
 
     public async ValueTask ApplyChange(IObjectBase entity, IChangeContext context)
     {
-        if (this is CreateChange<T>)
+        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<T>;
+    }
+
+    public virtual bool SupportsNewEntity()
+    {
+        return this is not EditChange<T>;
+    }
+
     [JsonIgnore]
     public Type EntityType => typeof(T);
 }

src/SIL.Harmony/Db/CrdtRepository.cs

@@ -299,16 +299,24 @@ public async Task<ChangesResult<Commit>> GetChanges(SyncState remoteState)
 
     public async Task AddSnapshots(IEnumerable<ObjectSnapshot> snapshots)
     {
-        var projectedEntityIds = new HashSet<Guid>();
+        var latestProjectByEntityId = new Dictionary<Guid, HybridDateTime>();
         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
@@ -323,6 +331,20 @@ public async Task AddSnapshots(IEnumerable<ObjectSnapshot> snapshots)
                 throw new DbUpdateException(message, e);
             }
         }
+
+        // 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();
+        }
+        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);
+        }
     }
 
     private async ValueTask ProjectSnapshot(ObjectSnapshot objectSnapshot)