improve query performance and commit performance in some cases (#3)

* allow deferring snapshot updates, useful when applying many commits at once in the case of an import

* query directly out of projected table without filtering on snapshot ID as the state of the projected table should be latest anyway

* prevent losing deferred commit snapshot updates by applying them if we ever try to add a commit that's not deferred after adding some deferred ones

* move hash fields out of Commit base and into the Crdt specific class.

* fix issue where GetChanges would return commits it should have filtered out due to dates getting truncated when converter to unix time milliseconds

* update some comments to better explain the issues with the code.

* fix verified db snapshots which changed due to moving the Commit hash out of the base class.

src/Crdt.Core/CommitBase.cs

@@ -1,4 +1,5 @@
-using System.Diagnostics.CodeAnalysis;
+using System.ComponentModel.DataAnnotations;
+using System.Diagnostics.CodeAnalysis;
 using System.IO.Hashing;
 using System.Text.Json.Serialization;
 
@@ -12,37 +13,23 @@ public abstract class CommitBase
 {
     public const string NullParentHash = "0000";
     [JsonConstructor]
-    protected internal CommitBase(Guid id, string hash, string parentHash, HybridDateTime hybridDateTime)
+    protected internal CommitBase(Guid id, HybridDateTime hybridDateTime)
     {
         Id = id;
-        Hash = hash;
-        ParentHash = parentHash;
         HybridDateTime = hybridDateTime;
     }
 
     internal CommitBase(Guid id)
     {
         Id = id;
-        Hash = GenerateHash(NullParentHash);
-        ParentHash = NullParentHash;
     }
 
     public (DateTimeOffset, long, Guid) CompareKey => (HybridDateTime.DateTime, HybridDateTime.Counter, Id);
     public Guid Id { get; }
     public required HybridDateTime HybridDateTime { get; init; }
     public DateTimeOffset DateTime => HybridDateTime.DateTime;
-    [JsonIgnore]
-    public string Hash { get; private set; }
-
-    [JsonIgnore]
-    public string ParentHash { get; private set; }
     public CommitMetadata Metadata { get; init; } = new();
 
-    public void SetParentHash(string parentHash)
-    {
-        Hash = GenerateHash(parentHash);
-        ParentHash = parentHash;
-    }
 
     public string GenerateHash(string parentHash)
     {
@@ -65,7 +52,7 @@ public override string ToString()
 /// <inheritdoc cref="CommitBase"/>
 public abstract class CommitBase<TChange> : CommitBase
 {
-    internal CommitBase(Guid id, string hash, string parentHash, HybridDateTime hybridDateTime) : base(id, hash, parentHash, hybridDateTime)
+    internal CommitBase(Guid id, HybridDateTime hybridDateTime) : base(id, hybridDateTime)
     {
     }
 

src/Crdt.Core/QueryHelpers.cs

@@ -19,22 +19,24 @@ public static async Task<ChangesResult<TCommit>> GetChanges<TCommit, TChange>(th
         var localSyncState = await commits.GetSyncState();
         foreach (var (clientId, localTimestamp) in localSyncState.ClientHeads)
         {
+            //client is new to the other history
             if (!remoteState.ClientHeads.TryGetValue(clientId, out var otherTimestamp))
             {
                 //todo slow, it would be better if we could query on client id and get latest changes per client
-                //client is new to the other history
                 newHistory.AddRange(await commits.Include(c => c.ChangeEntities).DefaultOrder()
                     .Where(c => c.ClientId == clientId)
                     .ToArrayAsync());
             }
+            //client has newer history than the other history
             else if (localTimestamp > otherTimestamp)
             {
                 var otherDt = DateTimeOffset.FromUnixTimeMilliseconds(otherTimestamp);
-                //todo even slower we want to also filter out changes that are already in the other history
-                //client has newer history than the other history
-                newHistory.AddRange(await commits.Include(c => c.ChangeEntities).DefaultOrder()
+                //todo even slower because we need to filter out changes that are already in the other history
+                newHistory.AddRange((await commits.Include(c => c.ChangeEntities).DefaultOrder()
                     .Where(c => c.ClientId == clientId && c.HybridDateTime.DateTime > otherDt)
-                    .ToArrayAsync());
+                    .ToArrayAsync())
+                    //fixes an issue where the query would include commits that are already in the other history
+                    .Where(c => c.DateTime.ToUnixTimeMilliseconds() > otherTimestamp));
             }
         }
 

src/Crdt.Core/ServerCommit.cs

@@ -7,9 +7,7 @@ namespace Crdt.Core;
 public class ServerCommit : CommitBase<ServerJsonChange>
 {
     [JsonConstructor]
-    protected ServerCommit(Guid id, string hash, string parentHash, HybridDateTime hybridDateTime) : base(id,
-        hash,
-        parentHash,
+    protected ServerCommit(Guid id, HybridDateTime hybridDateTime) : base(id,
         hybridDateTime)
     {
     }

src/Crdt.Tests/DataModelSimpleChanges.WriteMultipleCommits.verified.txt

@@ -17,6 +17,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_1,
+    ParentHash: Hash_Empty,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -41,8 +43,6 @@
       DateTime: DateTimeOffset_1
     },
     DateTime: DateTimeOffset_1,
-    Hash: Hash_1,
-    ParentHash: Hash_Empty,
     Metadata: {
       $type: CommitMetadata
     },
@@ -66,6 +66,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_2,
+    ParentHash: Hash_1,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -90,8 +92,6 @@
       DateTime: DateTimeOffset_2
     },
     DateTime: DateTimeOffset_2,
-    Hash: Hash_2,
-    ParentHash: Hash_1,
     Metadata: {
       $type: CommitMetadata
     },

src/Crdt.Tests/DataModelSimpleChanges.Writing2ChangesAtOnceWithMergedHistory.verified.txt

@@ -17,6 +17,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_1,
+    ParentHash: Hash_Empty,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -41,8 +43,6 @@
       DateTime: DateTimeOffset_1
     },
     DateTime: DateTimeOffset_1,
-    Hash: Hash_1,
-    ParentHash: Hash_Empty,
     Metadata: {
       $type: CommitMetadata
     },
@@ -67,6 +67,8 @@
         IsRoot: false
       }
     ],
+    Hash: Hash_2,
+    ParentHash: Hash_1,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -91,8 +93,6 @@
       DateTime: DateTimeOffset_2
     },
     DateTime: DateTimeOffset_2,
-    Hash: Hash_2,
-    ParentHash: Hash_1,
     Metadata: {
       $type: CommitMetadata
     },
@@ -116,6 +116,8 @@
         IsRoot: false
       }
     ],
+    Hash: Hash_3,
+    ParentHash: Hash_2,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -140,8 +142,6 @@
       DateTime: DateTimeOffset_3
     },
     DateTime: DateTimeOffset_3,
-    Hash: Hash_3,
-    ParentHash: Hash_2,
     Metadata: {
       $type: CommitMetadata
     },
@@ -166,6 +166,8 @@
         IsRoot: false
       }
     ],
+    Hash: Hash_4,
+    ParentHash: Hash_3,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -190,8 +192,6 @@
       DateTime: DateTimeOffset_4
     },
     DateTime: DateTimeOffset_4,
-    Hash: Hash_4,
-    ParentHash: Hash_3,
     Metadata: {
       $type: CommitMetadata
     },

src/Crdt.Tests/DataModelSimpleChanges.WritingA2ndChangeDoesNotEffectTheFirstSnapshot.verified.txt

@@ -17,6 +17,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_1,
+    ParentHash: Hash_Empty,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -41,8 +43,6 @@
       DateTime: DateTimeOffset_1
     },
     DateTime: DateTimeOffset_1,
-    Hash: Hash_1,
-    ParentHash: Hash_Empty,
     Metadata: {
       $type: CommitMetadata
     },
@@ -66,6 +66,8 @@
         IsRoot: false
       }
     ],
+    Hash: Hash_2,
+    ParentHash: Hash_1,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -90,8 +92,6 @@
       DateTime: DateTimeOffset_2
     },
     DateTime: DateTimeOffset_2,
-    Hash: Hash_2,
-    ParentHash: Hash_1,
     Metadata: {
       $type: CommitMetadata
     },

src/Crdt.Tests/DataModelSimpleChanges.WritingAChangeMakesASnapshot.verified.txt

@@ -17,6 +17,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_1,
+    ParentHash: Hash_Empty,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -41,8 +43,6 @@
       DateTime: DateTimeOffset_1
     },
     DateTime: DateTimeOffset_1,
-    Hash: Hash_1,
-    ParentHash: Hash_Empty,
     Metadata: {
       $type: CommitMetadata
     },

src/Crdt.Tests/DataModelSimpleChanges.WritingACommitWithMultipleChangesWorks.verified.txt

@@ -31,6 +31,8 @@
         IsRoot: true
       }
     ],
+    Hash: Hash_1,
+    ParentHash: Hash_Empty,
     ChangeEntities: [
       {
         $type: ChangeEntity<IChange>,
@@ -67,8 +69,6 @@
       DateTime: DateTimeOffset_1
     },
     DateTime: DateTimeOffset_1,
-    Hash: Hash_1,
-    ParentHash: Hash_Empty,
     Metadata: {
       $type: CommitMetadata
     },

src/Crdt.Tests/RepositoryTests.cs

@@ -238,4 +238,23 @@ await _repository.AddSnapshots([
         _crdtDbContext.Snapshots.Should().ContainSingle()
             .Which.CommitId.Should().Be(ids[0]);
     }
+
+    [Fact]
+    public async Task GetChanges_HandlesExactDateFilters()
+    {
+        var tmpTime = Time(2, 0);
+        //by adding a tick we cause an error and commit 2 will be returned by the query
+        var commit2Time = tmpTime with { DateTime = tmpTime.DateTime.AddTicks(1) };
+        await _repository.AddCommits([
+            Commit(Guid.NewGuid(), Time(1, 0)),
+            Commit(Guid.NewGuid(), commit2Time),
+            Commit(Guid.NewGuid(), Time(3, 0)),
+        ]);
+
+        var changes = await _repository.GetChanges(new SyncState(new()
+        {
+            { Guid.Empty, commit2Time.DateTime.ToUnixTimeMilliseconds() }
+        }));
+        changes.MissingFromClient.Select(c => c.DateTime.ToUnixTimeMilliseconds()).Should().ContainSingle("because {0} is only before the last commit", commit2Time.DateTime.ToUnixTimeMilliseconds());
+    }
 }

src/Crdt/Commit.cs

@@ -9,21 +9,34 @@ public class Commit : CommitBase<IChange>
 {
     [JsonConstructor]
     protected Commit(Guid id, string hash, string parentHash, HybridDateTime hybridDateTime) : base(id,
-        hash,
-        parentHash,
         hybridDateTime)
     {
+        Hash = hash;
+        ParentHash = parentHash;
     }
 
     internal Commit(Guid id) : base(id)
     {
+        Hash = GenerateHash(NullParentHash);
+        ParentHash = NullParentHash;
     }
 
+    public void SetParentHash(string parentHash)
+    {
+        Hash = GenerateHash(parentHash);
+        ParentHash = parentHash;
+    }
     internal Commit() : this(Guid.NewGuid())
     {
 
     }
 
     [JsonIgnore]
     public List<ObjectSnapshot> Snapshots { get; init; } = [];
+
+    [JsonIgnore]
+    public string Hash { get; private set; }
+
+    [JsonIgnore]
+    public string ParentHash { get; private set; }
 }

src/Crdt/CrdtConfig.cs

@@ -9,7 +9,11 @@ namespace Crdt;
 
 public class CrdtConfig
 {
-    public bool EnableProjectedTables { get; set; } = false;
+    /// <summary>
+    /// recommended to increase query performance, as getting objects can just query the table for that object.
+    /// it does however increase database size as now objects are stored both in snapshots and in their projected tables
+    /// </summary>
+    public bool EnableProjectedTables { get; set; } = true;
     public ChangeTypeListBuilder ChangeTypeListBuilder { get; } = new();
     public ObjectTypeListBuilder ObjectTypeListBuilder { get; } = new();