diff --git a/src/SIL.Harmony.Tests/CommitTests.cs b/src/SIL.Harmony.Tests/CommitTests.cs index e24d6c2..660f3ca 100644 --- a/src/SIL.Harmony.Tests/CommitTests.cs +++ b/src/SIL.Harmony.Tests/CommitTests.cs @@ -114,17 +114,14 @@ public void CanRoundTripCommitThroughJson() { ClientId = Guid.NewGuid(), HybridDateTime = Now(), - ChangeEntities = - { - new ChangeEntity - { - Change = change, - Index = 0, - CommitId = change.CommitId, - EntityId = change.EntityId - } - } }; + commit.ChangeEntities.Add(new ChangeEntity + { + Change = change, + Index = 0, + CommitId = commit.Id, + EntityId = change.EntityId + }); commit.SetParentHash(Convert.ToHexString(XxHash64.Hash(Guid.NewGuid().ToByteArray()))); var json = JsonSerializer.Serialize(commit, serializerOptions); var commit2 = JsonSerializer.Deserialize(json, serializerOptions); diff --git a/src/SIL.Harmony.Tests/DataModelIntegrityTests.cs b/src/SIL.Harmony.Tests/DataModelIntegrityTests.cs index 7e9659d..dee531f 100644 --- a/src/SIL.Harmony.Tests/DataModelIntegrityTests.cs +++ b/src/SIL.Harmony.Tests/DataModelIntegrityTests.cs @@ -4,20 +4,6 @@ namespace SIL.Harmony.Tests; public class DataModelIntegrityTests : DataModelTestBase { - [Fact] - public async Task CanAddTheSameCommitMultipleTimes() - { - var entity1Id = Guid.NewGuid(); - var change = SetWord(entity1Id, "entity1"); - var first = await WriteNextChange(change); - await WriteNextChange(SetWord(entity1Id, "entity1.1")); - await DataModel.AddChange(_localClientId, change, first.Id); - await DataModel.AddChange(_localClientId, change, first.Id); - - var entry = await DataModel.GetLatest(entity1Id); - entry!.Text.Should().Be("entity1.1"); - } - [Fact] public async Task CanAddTheSameCommitMultipleTimesVisSync() { diff --git a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs index f4626e2..3e97ac8 100644 --- a/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs +++ b/src/SIL.Harmony.Tests/DataModelPerformanceTests.cs @@ -110,7 +110,7 @@ internal static async Task BulkInsertChanges(DataModelTestBase dataModelTest, in var commit = new Commit(commitId) { ClientId = Guid.NewGuid(), - HybridDateTime = new HybridDateTime(dataModelTest.NextDate(), 0), + HybridDateTime = new HybridDateTime(dataModelTest.NextDate(), 0), ChangeEntities = [ new ChangeEntity() diff --git a/src/SIL.Harmony.Tests/DataModelTestBase.cs b/src/SIL.Harmony.Tests/DataModelTestBase.cs index d195249..80402eb 100644 --- a/src/SIL.Harmony.Tests/DataModelTestBase.cs +++ b/src/SIL.Harmony.Tests/DataModelTestBase.cs @@ -103,15 +103,18 @@ protected async ValueTask WriteChange(Guid clientId, bool add = true) { if (!add) - return new Commit + { + var commit = new Commit { ClientId = clientId, HybridDateTime = new HybridDateTime(dateTime, 0), - ChangeEntities = changes.Select((change, index) => new ChangeEntity - { - Change = change, Index = index, CommitId = change.CommitId, EntityId = change.EntityId - }).ToList() }; + commit.ChangeEntities.AddRange(changes.Select((change, index) => new ChangeEntity + { + Change = change, Index = index, CommitId = commit.Id, EntityId = change.EntityId + })); + return commit; + } MockTimeProvider.SetNextDateTime(dateTime); return await DataModel.AddChanges(clientId, changes); } diff --git a/src/SIL.Harmony.Tests/PersistExtraDataTests.cs b/src/SIL.Harmony.Tests/PersistExtraDataTests.cs index 0de54a8..4182729 100644 --- a/src/SIL.Harmony.Tests/PersistExtraDataTests.cs +++ b/src/SIL.Harmony.Tests/PersistExtraDataTests.cs @@ -23,7 +23,7 @@ public class ExtraDataModel : IObjectBase { public Guid Id { get; set; } public DateTimeOffset? DeletedAt { get; set; } - public Guid CommitId { get; set; } + public Guid LastCommitId { get; set; } public DateTimeOffset? DateTime { get; set; } public long Counter { get; set; } @@ -42,7 +42,7 @@ public IObjectBase Copy() { Id = Id, DeletedAt = DeletedAt, - CommitId = CommitId, + LastCommitId = LastCommitId, DateTime = DateTime, Counter = Counter }; @@ -61,7 +61,7 @@ public PersistExtraDataTests() { if (obj is ExtraDataModel extraDataModel) { - extraDataModel.CommitId = snapshot.CommitId; + extraDataModel.LastCommitId = snapshot.CommitId; extraDataModel.DateTime = snapshot.Commit.HybridDateTime.DateTime; extraDataModel.Counter = snapshot.Commit.HybridDateTime.Counter; } @@ -78,7 +78,7 @@ public async Task CanPersistExtraData() var commit = await _dataModelTestBase.WriteNextChange(new CreateExtraDataModelChange(entityId)); var extraDataModel = _dataModelTestBase.DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken).Should().ContainSingle().Subject; extraDataModel.Id.Should().Be(entityId); - extraDataModel.CommitId.Should().Be(commit.Id); + extraDataModel.LastCommitId.Should().Be(commit.Id); extraDataModel.DateTime.Should().Be(commit.HybridDateTime.DateTime); extraDataModel.Counter.Should().Be(commit.HybridDateTime.Counter); } diff --git a/src/SIL.Harmony/Changes/Change.cs b/src/SIL.Harmony/Changes/Change.cs index 6908536..aad8a0b 100644 --- a/src/SIL.Harmony/Changes/Change.cs +++ b/src/SIL.Harmony/Changes/Change.cs @@ -6,9 +6,6 @@ namespace SIL.Harmony.Changes; [JsonPolymorphic(TypeDiscriminatorPropertyName = CrdtConstants.ChangeDiscriminatorProperty)] public interface IChange { - [JsonIgnore] - Guid CommitId { get; set; } - [JsonIgnore] Guid EntityId { get; set; } @@ -41,9 +38,6 @@ protected Change(Guid entityId) EntityId = entityId; } - [JsonIgnore] - public Guid CommitId { get; set; } - public Guid EntityId { get; set; } async ValueTask IChange.NewEntity(Commit commit, IChangeContext context) diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index e120e1c..5c1e336 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -47,23 +47,14 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, /// if the client id changes too much it could slow down the sync process /// /// change to be applied to the model - /// - /// can be used by the application code to ensure a specific change is only applied once, - /// for example a one time migration or update of pre seeded data in the model, a hard coded guid could be used - /// which will ensure it's only applied once to the model, even if multiple clients update at the same time and all apply the same change. - /// typical changes should not specify the commitId and let a new guid to be generated for each commit. - /// This could also be useful if the application has a flaky connection with the DataModel and needs to retry the same change multiple times but ensure it's only applied once, - /// then the guid would be generated by the application - /// /// used to store metadata on the commit, for example app version or author id /// the newly created commit public async Task AddChange( Guid clientId, IChange change, - Guid commitId = default, CommitMetadata? commitMetadata = null) { - return await AddChanges(clientId, [change], commitId, commitMetadata); + return await AddChanges(clientId, [change], commitMetadata); } public async Task AddManyChanges(Guid clientId, @@ -74,7 +65,7 @@ public async Task AddManyChanges(Guid clientId, await using var repo = await _crdtRepositoryFactory.CreateRepository(); var commits = changes .Chunk(changesPerCommitMax) - .Select(chunk => NewCommit(Guid.NewGuid(), clientId, commitMetadata(), chunk)) + .Select(chunk => NewCommit(clientId, commitMetadata(), chunk)) .ToArray(); if (commits is []) return; using var locked = await repo.Lock(); @@ -91,24 +82,23 @@ public async Task AddManyChanges(Guid clientId, public async Task AddChanges( Guid clientId, IEnumerable changes, - Guid commitId = default, CommitMetadata? commitMetadata = null) { - var commit = NewCommit(commitId, clientId, commitMetadata, changes); + var commit = NewCommit(clientId, commitMetadata, changes); await Add(commit); return commit; } - private Commit NewCommit(Guid commitId, Guid clientId, CommitMetadata? commitMetadata, IEnumerable changes) + private Commit NewCommit(Guid clientId, CommitMetadata? commitMetadata, IEnumerable changes) { - commitId = commitId == default ? Guid.NewGuid() : commitId; - return new Commit(commitId) + var commit = new Commit { ClientId = clientId, HybridDateTime = _timeProvider.GetDateTime(), - ChangeEntities = [..changes.Select(ToChangeEntity)], Metadata = commitMetadata ?? new() }; + commit.ChangeEntities.AddRange(changes.Select((c, i) => ToChangeEntity(c, i, commit.Id))); + return commit; } private async Task Add(Commit commit) @@ -133,11 +123,11 @@ public ValueTask DisposeAsync() return ValueTask.CompletedTask; } - private static ChangeEntity ToChangeEntity(IChange change, int index) + private static ChangeEntity ToChangeEntity(IChange change, int index, Guid commitId) { return new ChangeEntity() { - Change = change, CommitId = change.CommitId, EntityId = change.EntityId, Index = index + Change = change, CommitId = commitId, EntityId = change.EntityId, Index = index }; }