Allow storing extra data on objects before persisting (#17)

* rename GetLatestObjects to QueryLatest

* allow multiple object adapters

* ensure Simple kernel can be used across multiple tests with different Json configs used by EF

* test `BeforeSaveObject ` to store commit id among other data

* move BeforeSaveObject up into the snapshot worker so it's always applied to objects, not only when using projected tables

* enable benchmarking comments

* change BeforePersistObject to BeforeSaveObject

.github/workflows/test.yaml

@@ -6,7 +6,8 @@ on:
   pull_request: 
     branches:
       - main
-
+permissions: 
+  pull-requests: write #allow benchmark-action to comment on PRs
 jobs:
   build:
     runs-on: ubuntu-latest
@@ -19,4 +20,19 @@ jobs:
         uses: actions/setup-dotnet@v4
 
       - name: Build & test
-        run: dotnet test --configuration Release --logger GitHubActions
+        run: dotnet test --configuration Release --logger GitHubActions
+      - name: Download previous benchmark data
+        uses: actions/cache@v4
+        with:
+          path: ./cache
+          key: ${{ runner.os }}-benchmark
+      - name: Continuous Benchmark
+        uses: benchmark-action/[email protected]
+        with: 
+          tool: benchmarkdotnet
+          output-file-path: src/artifacts/bin/SIL.Harmony.Tests/release/BenchmarkDotNet.Artifacts/results/SIL.Harmony.Tests.DataModelPerformanceBenchmarks-report-full-compressed.json
+          external-data-json-path: ./cache/benchmark-data.json
+          fail-on-alert: true
+          comment-on-alert: true
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+

src/SIL.Harmony.Sample/CrdtSampleKernel.cs

@@ -15,16 +15,16 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi
     {
         return services.AddCrdtDataSample(builder => builder.UseSqlite($"Data Source={dbPath}"));
     }
-    public static IServiceCollection AddCrdtDataSample(this IServiceCollection services, DbConnection connection)
-    {
-        return services.AddCrdtDataSample(builder => builder.UseSqlite(connection, true));
-    }
 
     public static IServiceCollection AddCrdtDataSample(this IServiceCollection services,
-        Action<DbContextOptionsBuilder> optionsBuilder) 
+        Action<DbContextOptionsBuilder> optionsBuilder, bool performanceTest = false)
     {
         services.AddDbContext<SampleDbContext>((provider, builder) =>
         {
+            //this ensures that Ef Conversion methods will not be cached across different IoC containers
+            //this can show up as the second instance using the JsonSerializerOptions from the first container
+            //only needed for testing scenarios
+            builder.EnableServiceProviderCaching(performanceTest);
             builder.UseLinqToDbCrdt(provider);
             optionsBuilder(builder);
             builder.EnableDetailedErrors();

src/SIL.Harmony.Tests/Adapter/CustomObjectAdapterTests.cs

@@ -195,6 +195,6 @@ await dataModel.AddChange(Guid.NewGuid(),
         myClass2.MyNumber.Should().Be(123.45m);
         myClass2.DeletedTime.Should().BeNull();
 
-        dataModel.GetLatestObjects<MyClass>().Should().NotBeEmpty();
+        dataModel.QueryLatest<MyClass>().Should().NotBeEmpty();
     }
 }

src/SIL.Harmony.Tests/DataModelPerformanceTests.cs

@@ -4,6 +4,7 @@
 using BenchmarkDotNet.Columns;
 using BenchmarkDotNet.Configs;
 using BenchmarkDotNet.Engines;
+using BenchmarkDotNet.Exporters.Json;
 using BenchmarkDotNet.Loggers;
 using BenchmarkDotNet.Running;
 using JetBrains.Profiler.SelfApi;
@@ -23,9 +24,13 @@ public class DataModelPerformanceTests(ITestOutputHelper output)
     [Fact]
     public void AddingChangePerformance()
     {
+        #if DEBUG
+        Assert.Fail("This test is disabled in debug builds, not reliable");
+        #endif
         var summary =
             BenchmarkRunner.Run<DataModelPerformanceBenchmarks>(
                 ManualConfig.CreateEmpty()
+                    .AddExporter(JsonExporter.FullCompressed)
                     .AddColumnProvider(DefaultColumnProviders.Instance)
                     .AddLogger(new XUnitBenchmarkLogger(output))
             );
@@ -188,7 +193,7 @@ public class DataModelPerformanceBenchmarks
     [GlobalSetup]
     public void GlobalSetup()
     {
-        _templateModel = new DataModelTestBase(alwaysValidate: false);
+        _templateModel = new DataModelTestBase(alwaysValidate: false, performanceTest: true);
         DataModelPerformanceTests.BulkInsertChanges(_templateModel, StartingSnapshots).GetAwaiter().GetResult();
     }
 
@@ -198,7 +203,7 @@ public void GlobalSetup()
     [IterationSetup]
     public void IterationSetup()
     {
-        _emptyDataModel = new(alwaysValidate: false);
+        _emptyDataModel = new(alwaysValidate: false, performanceTest: true);
         _ = _emptyDataModel.WriteNextChange(_emptyDataModel.SetWord(Guid.NewGuid(), "entity1")).Result;
         _dataModelTestBase = _templateModel.ForkDatabase(false);
     }

src/SIL.Harmony.Tests/DataModelSimpleChanges.cs

@@ -79,7 +79,7 @@ public async Task WriteMultipleCommits()
 
         await WriteNextChange(SetWord(Guid.NewGuid(), "change 3"));
         DbContext.Snapshots.Should().HaveCount(3);
-        DataModel.GetLatestObjects<Word>().Should().HaveCount(3);
+        DataModel.QueryLatest<Word>().Should().HaveCount(3);
     }
 
     [Fact]

src/SIL.Harmony.Tests/DataModelTestBase.cs

@@ -1,3 +1,4 @@
+using System.Data.Common;
 using Microsoft.Data.Sqlite;
 using SIL.Harmony.Changes;
 using SIL.Harmony.Core;
@@ -17,29 +18,34 @@ public class DataModelTestBase : IAsyncLifetime
 {
     protected readonly ServiceProvider _services;
     protected readonly Guid _localClientId = Guid.NewGuid();
+    private readonly bool _performanceTest;
     public readonly DataModel DataModel;
     public readonly SampleDbContext DbContext;
     internal readonly CrdtRepository CrdtRepository;
     protected readonly MockTimeProvider MockTimeProvider = new();
 
-    public DataModelTestBase(bool saveToDisk = false, bool alwaysValidate = true) : this(saveToDisk
+    public DataModelTestBase(bool saveToDisk = false, bool alwaysValidate = true,
+        Action<IServiceCollection>? configure = null, bool performanceTest = false) : this(saveToDisk
         ? new SqliteConnection("Data Source=test.db")
-        : new SqliteConnection("Data Source=:memory:"), alwaysValidate)
+        : new SqliteConnection("Data Source=:memory:"), alwaysValidate, configure, performanceTest)
     {
     }
 
     public DataModelTestBase() : this(new SqliteConnection("Data Source=:memory:"))
     {
     }
 
-    public DataModelTestBase(SqliteConnection connection, bool alwaysValidate = true)
+    public DataModelTestBase(SqliteConnection connection, bool alwaysValidate = true, Action<IServiceCollection>? configure = null, bool performanceTest = false)
     {
-        _services = new ServiceCollection()
-            .AddCrdtDataSample(connection)
-            .AddOptions<CrdtConfig>().Configure(config => config.AlwaysValidateCommits = alwaysValidate)
-            .Services
-            .Replace(ServiceDescriptor.Singleton<IHybridDateTimeProvider>(MockTimeProvider))
-            .BuildServiceProvider();
+        _performanceTest = performanceTest;
+        var serviceCollection = new ServiceCollection().AddCrdtDataSample(builder =>
+            {
+                builder.UseSqlite(connection, true);
+            }, performanceTest)
+            .Configure<CrdtConfig>(config => config.AlwaysValidateCommits = alwaysValidate)
+            .Replace(ServiceDescriptor.Singleton<IHybridDateTimeProvider>(MockTimeProvider));
+        configure?.Invoke(serviceCollection);
+        _services = serviceCollection.BuildServiceProvider();
         DbContext = _services.GetRequiredService<SampleDbContext>();
         DbContext.Database.OpenConnection();
         DbContext.Database.EnsureCreated();
@@ -54,7 +60,7 @@ public DataModelTestBase ForkDatabase(bool alwaysValidate = true)
         var existingConnection = DbContext.Database.GetDbConnection() as SqliteConnection;
         if (existingConnection is null) throw new InvalidOperationException("Database is not SQLite");
         existingConnection.BackupDatabase(connection);
-        var newTestBase = new DataModelTestBase(connection, alwaysValidate);
+        var newTestBase = new DataModelTestBase(connection, alwaysValidate, performanceTest: _performanceTest);
         newTestBase.SetCurrentDate(currentDate.DateTime);
         return newTestBase;
     }

src/SIL.Harmony.Tests/DataQueryTests.cs

@@ -15,7 +15,7 @@ public override async Task InitializeAsync()
     [Fact]
     public async Task CanQueryLatestData()
     {
-        var entries = await DataModel.GetLatestObjects<Word>().ToArrayAsync();
+        var entries = await DataModel.QueryLatest<Word>().ToArrayAsync();
         var entry = entries.Should().ContainSingle().Subject;
         entry.Text.Should().Be("entity1");
     }

src/SIL.Harmony.Tests/DefinitionTests.cs

@@ -50,7 +50,7 @@ public async Task CanGetInOrder()
         await WriteNextChange(NewDefinition(wordId, "greet someone", "verb", 2));
         await WriteNextChange(NewDefinition(wordId, "a greeting", "noun", 1));
 
-        var definitions = await DataModel.GetLatestObjects<Definition>().ToArrayAsync();
+        var definitions = await DataModel.QueryLatest<Definition>().ToArrayAsync();
         definitions.Select(d => d.PartOfSpeech).Should().ContainInConsecutiveOrder(
             "noun",
             "verb"
@@ -69,7 +69,7 @@ public async Task CanChangeOrderBetweenExistingDefinitions()
         await WriteNextChange(NewDefinition(wordId, "greet someone", "verb", 2, definitionBId));
         await WriteNextChange(NewDefinition(wordId, "used as a greeting", "exclamation", 3, definitionCId));
 
-        var definitions = await DataModel.GetLatestObjects<Definition>().ToArrayAsync();
+        var definitions = await DataModel.QueryLatest<Definition>().ToArrayAsync();
         definitions.Select(d => d.PartOfSpeech).Should().ContainInConsecutiveOrder(
             "noun",
             "verb",
@@ -79,7 +79,7 @@ public async Task CanChangeOrderBetweenExistingDefinitions()
         //change the order of the exclamation to be between the noun and verb
         await WriteNextChange(SetOrderChange<Definition>.Between(definitionCId, definitions[0], definitions[1]));
 
-        definitions = await DataModel.GetLatestObjects<Definition>().ToArrayAsync();
+        definitions = await DataModel.QueryLatest<Definition>().ToArrayAsync();
         definitions.Select(d => d.PartOfSpeech).Should().ContainInConsecutiveOrder(
             "noun",
             "exclamation",
@@ -98,7 +98,7 @@ public async Task ConsistentlySortsItems()
         await WriteNextChange(NewDefinition(wordId, "greet someone", "verb", 1, definitionAId));
         await WriteNextChange(NewDefinition(wordId, "a greeting", "noun", 1, definitionBId));
 
-        var definitions = await DataModel.GetLatestObjects<Definition>().ToArrayAsync();
+        var definitions = await DataModel.QueryLatest<Definition>().ToArrayAsync();
         definitions.Select(d => d.Id).Should().ContainInConsecutiveOrder(
             definitionBId,
             definitionAId

src/SIL.Harmony.Tests/PersistExtraDataTests.cs

@@ -0,0 +1,88 @@
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using SIL.Harmony.Changes;
+using SIL.Harmony.Core;
+using SIL.Harmony.Entities;
+using SIL.Harmony.Sample;
+
+namespace SIL.Harmony.Tests;
+
+public class PersistExtraDataTests
+{
+    private DataModelTestBase _dataModelTestBase;
+
+    public class CreateExtraDataModelChange(Guid entityId) : CreateChange<ExtraDataModel>(entityId), ISelfNamedType<CreateExtraDataModelChange>
+    {
+        public override ValueTask<ExtraDataModel> NewEntity(Commit commit, ChangeContext context)
+        {
+            return ValueTask.FromResult(new ExtraDataModel()
+            {
+                Id = EntityId,
+            });
+        }
+    }
+
+    public class ExtraDataModel : IObjectBase<ExtraDataModel>
+    {
+        public Guid Id { get; set; }
+        public DateTimeOffset? DeletedAt { get; set; }
+        public Guid CommitId { get; set; }
+        public DateTimeOffset? DateTime { get; set; }
+        public long Counter { get; set; }
+
+        public Guid[] GetReferences()
+        {
+            return [];
+        }
+
+        public void RemoveReference(Guid id, Commit commit)
+        {
+        }
+
+        public IObjectBase Copy()
+        {
+            return new ExtraDataModel()
+            {
+                Id = Id,
+                DeletedAt = DeletedAt,
+                CommitId = CommitId,
+                DateTime = DateTime,
+                Counter = Counter
+            };
+        }
+    }
+
+    public PersistExtraDataTests()
+    {
+        _dataModelTestBase = new DataModelTestBase(configure: services =>
+        {
+            services.Configure<CrdtConfig>(config =>
+            {
+                config.ObjectTypeListBuilder.DefaultAdapter().Add<ExtraDataModel>();
+                config.ChangeTypeListBuilder.Add<CreateExtraDataModelChange>();
+                config.BeforeSaveObject = (obj, snapshot) =>
+                {
+                    if (obj is ExtraDataModel extraDataModel)
+                    {
+                        extraDataModel.CommitId = snapshot.CommitId;
+                        extraDataModel.DateTime = snapshot.Commit.HybridDateTime.DateTime;
+                        extraDataModel.Counter = snapshot.Commit.HybridDateTime.Counter;
+                    }
+                    return ValueTask.CompletedTask;
+                };
+            });
+        });
+    }
+
+    [Fact]
+    public async Task CanPersistExtraData()
+    {
+        var entityId = Guid.NewGuid();
+        var commit = await _dataModelTestBase.WriteNextChange(new CreateExtraDataModelChange(entityId));
+        var extraDataModel = _dataModelTestBase.DataModel.QueryLatest<ExtraDataModel>().Should().ContainSingle().Subject;
+        extraDataModel.Id.Should().Be(entityId);
+        extraDataModel.CommitId.Should().Be(commit.Id);
+        extraDataModel.DateTime.Should().Be(commit.HybridDateTime.DateTime);
+        extraDataModel.Counter.Should().Be(commit.HybridDateTime.Counter);
+    }
+}

src/SIL.Harmony.Tests/SyncTests.cs

@@ -116,7 +116,7 @@ public async Task CanSync_AddDependentWithMultipleChanges()
 
         await _client2.DataModel.SyncWith(_client1.DataModel);
 
-        _client2.DataModel.GetLatestObjects<Definition>().Should()
-            .BeEquivalentTo(_client1.DataModel.GetLatestObjects<Definition>());
+        _client2.DataModel.QueryLatest<Definition>().Should()
+            .BeEquivalentTo(_client1.DataModel.QueryLatest<Definition>());
     }
 }

src/SIL.Harmony/Adapters/CustomAdapterProvider.cs

@@ -12,8 +12,7 @@ public class CustomAdapterProvider<TCommonInterface, TCustomAdapter> : IObjectAd
 {
     private readonly ObjectTypeListBuilder _objectTypeListBuilder;
     private readonly List<AdapterRegistration> _objectTypes = new();
-    private Dictionary<Type, List<JsonDerivedType>> JsonTypes { get; } = [];
-    Dictionary<Type, List<JsonDerivedType>> IObjectAdapterProvider.JsonTypes => JsonTypes;
+    private Dictionary<Type, List<JsonDerivedType>> JsonTypes => _objectTypeListBuilder.JsonTypes;
 
     public CustomAdapterProvider(ObjectTypeListBuilder objectTypeListBuilder)
     {
@@ -55,6 +54,11 @@ IObjectBase IObjectAdapterProvider.Adapt(object obj)
     {
         return TCustomAdapter.Create((TCommonInterface)obj);
     }
+
+    public bool CanAdapt(object obj)
+    {
+        return obj is TCommonInterface;
+    }
 }
 
 // it's possible to implement this without a Common interface

src/SIL.Harmony/Adapters/DefaultAdapterProvider.cs

@@ -38,6 +38,10 @@ IObjectBase IObjectAdapterProvider.Adapt(object obj)
             $"Object is of type {obj.GetType().Name} which does not implement {nameof(IObjectBase)}");
     }
 
-    private Dictionary<Type, List<JsonDerivedType>> JsonTypes { get; } = [];
-    Dictionary<Type, List<JsonDerivedType>> IObjectAdapterProvider.JsonTypes => JsonTypes;
+    public bool CanAdapt(object obj)
+    {
+        return obj is IObjectBase;
+    }
+
+    private Dictionary<Type, List<JsonDerivedType>> JsonTypes => objectTypeListBuilder.JsonTypes;
 }

src/SIL.Harmony/Adapters/IObjectAdapterProvider.cs

@@ -11,6 +11,5 @@ internal interface IObjectAdapterProvider
 {
     IEnumerable<AdapterRegistration> GetRegistrations();
     IObjectBase Adapt(object obj);
-
-    Dictionary<Type, List<JsonDerivedType>> JsonTypes { get; }
+    bool CanAdapt(object obj);
 }

src/SIL.Harmony/Changes/ChangeContext.cs

@@ -25,5 +25,5 @@ internal ChangeContext(Commit commit, SnapshotWorker worker, CrdtConfig crdtConf
     }
 
     public async ValueTask<bool> IsObjectDeleted(Guid entityId) => (await GetSnapshot(entityId))?.EntityIsDeleted ?? true;
-    internal IObjectBase Adapt(object obj) => _crdtConfig.ObjectTypeListBuilder.AdapterProvider.Adapt(obj);
+    internal IObjectBase Adapt(object obj) => _crdtConfig.ObjectTypeListBuilder.Adapt(obj);
 }