From 82778518af3ee67333f6137bd6e92ddf709b2cf4 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 10 Jul 2026 14:19:20 +0700 Subject: [PATCH 01/18] Add AGENTS.md and gitignore local agent skill files. Co-authored-by: Cursor --- .gitignore | 4 ++++ AGENTS.md | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 AGENTS.md diff --git a/.gitignore b/.gitignore index 7361c97..e7c353e 100644 --- a/.gitignore +++ b/.gitignore @@ -402,3 +402,7 @@ FodyWeavers.xsd *.received.* *.lscache + +# Agent skills (local-only) +.scratch/ +docs/agents/ diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..6ec05a8 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,13 @@ +## Agent skills + +### Issue tracker + +Issues live as local markdown files under `.scratch/` (gitignored). See `docs/agents/issue-tracker.md`. + +### Triage labels + +Default triage role strings (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`. + +### Domain docs + +Single-context layout — `CONTEXT.md` and `docs/adr/` at the repo root. See `docs/agents/domain.md`. From fcc9bc4eeb20c353ff9b185027445b8b348d587d Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 10:58:21 +0700 Subject: [PATCH 02/18] Add pluggable commit materialization filter for scoped visibility. Lets optional refs later exclude commits from snapshots without changing sync storage or forking SnapshotWorker. Co-authored-by: Cursor --- .../CommitMaterializationFilterTests.cs | 108 ++++++++++++++++++ src/SIL.Harmony/CrdtConfig.cs | 6 + src/SIL.Harmony/DataModel.cs | 19 ++- .../ICommitMaterializationFilter.cs | 17 +++ 4 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 src/SIL.Harmony.Tests/CommitMaterializationFilterTests.cs create mode 100644 src/SIL.Harmony/ICommitMaterializationFilter.cs diff --git a/src/SIL.Harmony.Tests/CommitMaterializationFilterTests.cs b/src/SIL.Harmony.Tests/CommitMaterializationFilterTests.cs new file mode 100644 index 0000000..bca227b --- /dev/null +++ b/src/SIL.Harmony.Tests/CommitMaterializationFilterTests.cs @@ -0,0 +1,108 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Core; +using SIL.Harmony.Sample.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class CommitMaterializationFilterTests : DataModelTestBase +{ + public CommitMaterializationFilterTests() : base(configure: services => + { + services.Configure(config => + { + config.CommitMaterializationFilter = new ExcludeMarkedCommitsFilter(); + }); + }) + { + } + + [Fact] + public async Task ExcludedCommitDoesNotAffectGetLatest() + { + var entityId = Guid.NewGuid(); + + await DataModel.AddChange(_localClientId, SetWord(entityId, "included")); + var excluded = await DataModel.AddChange( + _localClientId, + SetWord(entityId, "excluded"), + new CommitMetadata { ["materialize"] = "false" }); + + var word = await DataModel.GetLatest(entityId); + word!.Text.Should().Be("included"); + // Excluded commits are still stored; only materialization skips them + DbContext.Commits.Should().Contain(c => c.Id == excluded.Id); + } + + [Fact] + public async Task LateArrivingIncludedCommitReplaysFilteredHistory() + { + var entityId = Guid.NewGuid(); + + var first = await DataModel.AddChange(_localClientId, SetWord(entityId, "first")); + await DataModel.AddChange( + _localClientId, + SetWord(entityId, "skipped"), + new CommitMetadata { ["materialize"] = "false" }); + + (await DataModel.GetLatest(entityId))!.Text.Should().Be("first"); + + var late = new Commit + { + ClientId = _localClientId, + HybridDateTime = new HybridDateTime(first.DateTime.AddHours(1), 0), + Metadata = new CommitMetadata() + }; + late.ChangeEntities.Add(new ChangeEntity + { + Change = SetWord(entityId, "late"), + Index = 0, + CommitId = late.Id, + EntityId = entityId + }); + await AddCommitsViaSync([late]); + + var word = await DataModel.GetLatest(entityId); + // Included apply order: first → late; skipped never materializes + word!.Text.Should().Be("late"); + } + + [Fact] + public async Task LateArrivingExcludedCommitDoesNotDuplicateLaterSnapshots() + { + var entityId = Guid.NewGuid(); + + var first = await DataModel.AddChange(_localClientId, SetWord(entityId, "first")); + await DataModel.AddChange(_localClientId, SetWord(entityId, "third")); + (await DataModel.GetLatest(entityId))!.Text.Should().Be("third"); + var snapshotCountBefore = DbContext.Snapshots.Count(s => s.EntityId == entityId); + + var excluded = new Commit + { + ClientId = _localClientId, + HybridDateTime = new HybridDateTime(first.DateTime.AddHours(1), 0), + Metadata = new CommitMetadata { ["materialize"] = "false" } + }; + excluded.ChangeEntities.Add(new ChangeEntity + { + Change = SetWord(entityId, "excluded-middle"), + Index = 0, + CommitId = excluded.Id, + EntityId = entityId + }); + await AddCommitsViaSync([excluded]); + + (await DataModel.GetLatest(entityId))!.Text.Should().Be("third"); + DbContext.Snapshots.Count(s => s.EntityId == entityId).Should().Be(snapshotCountBefore); + DbContext.Commits.Should().Contain(c => c.Id == excluded.Id); + } + + /// + /// Excludes commits marked with ExtraMetadata materialize=false. + /// + private sealed class ExcludeMarkedCommitsFilter : ICommitMaterializationFilter + { + public bool Include(Commit commit) => + commit.Metadata["materialize"] != "false"; + } +} diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 2ae0895..851ac98 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -21,6 +21,12 @@ public class CrdtConfig /// after adding any commit validate the commit history, not great for performance but good for testing. /// public bool AlwaysValidateCommits { get; set; } = true; + /// + /// Selects which stored commits are applied when materializing snapshots. + /// Default includes every commit (unchanged behaviour). Commits are still persisted and synced regardless. + /// + public ICommitMaterializationFilter CommitMaterializationFilter { get; set; } = + IncludeAllCommitsFilter.Instance; public ChangeTypeListBuilder ChangeTypeListBuilder { get; } = new(); public IEnumerable ChangeTypes => ChangeTypeListBuilder.Types.Select(t => t.DerivedType); public ObjectTypeListBuilder ObjectTypeListBuilder { get; } = new(); diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 8707fcb..5b36323 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -186,13 +186,22 @@ ValueTask ISyncable.ShouldSync() private async Task UpdateSnapshots(CrdtRepository repo, SortedSet commitsToApply) { if (commitsToApply.Count == 0) return; - var oldestAddedCommit = commitsToApply.First(); - await repo.DeleteStaleSnapshots(oldestAddedCommit); + var filter = _crdtConfig.Value.CommitMaterializationFilter; + var commitsToMaterialize = commitsToApply + .Where(filter.Include) + .ToSortedSet(); + if (commitsToMaterialize.Count == 0) return; + + // Stale boundary is the unfiltered apply-window head. If that head is excluded, + // DeleteStaleSnapshots(filtered.First()) would not clear already-materialized later + // commits (WhereAfter is strict), and re-applying them would duplicate snapshots. + var oldestInApplyWindow = commitsToApply.First(); + await repo.DeleteStaleSnapshots(oldestInApplyWindow); Dictionary snapshotLookup = []; - if (commitsToApply.Count > 10) + if (commitsToMaterialize.Count > 10) { // Bulk-load relevant snapshots to minimize DB queries - var entityIds = commitsToApply + var entityIds = commitsToMaterialize .SelectMany(c => c.ChangeEntities.Select(ce => ce.EntityId)) .Distinct(); @@ -204,7 +213,7 @@ private async Task UpdateSnapshots(CrdtRepository repo, SortedSet commit } var snapshotWorker = new SnapshotWorker(snapshotLookup, repo, _crdtConfig.Value); - await snapshotWorker.UpdateSnapshots(commitsToApply); + await snapshotWorker.UpdateSnapshots(commitsToMaterialize); } private async Task ValidateCommits(CrdtRepository repo) diff --git a/src/SIL.Harmony/ICommitMaterializationFilter.cs b/src/SIL.Harmony/ICommitMaterializationFilter.cs new file mode 100644 index 0000000..e3aca6c --- /dev/null +++ b/src/SIL.Harmony/ICommitMaterializationFilter.cs @@ -0,0 +1,17 @@ +namespace SIL.Harmony; + +/// +/// Decides which commits are applied when materializing snapshots. +/// Commits are still stored and synced regardless of this filter. +/// +public interface ICommitMaterializationFilter +{ + bool Include(Commit commit); +} + +public sealed class IncludeAllCommitsFilter : ICommitMaterializationFilter +{ + public static IncludeAllCommitsFilter Instance { get; } = new(); + + public bool Include(Commit commit) => true; +} From 5fdf45c7258dfe7ba5c2dd13c3cffe092559e99c Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 11:10:07 +0700 Subject: [PATCH 03/18] Add optional SIL.Harmony.Refs package with create-branch support. Lets apps opt in to Branch entities via AddHarmonyRefs without changing core Harmony for clients that do not need refs. Co-authored-by: Cursor --- harmony.sln | 77 ++++++++++++++++++- .../Changes/CreateBranchChange.cs | 21 +++++ src/SIL.Harmony.Refs/Entities/Branch.cs | 26 +++++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 17 ++++ src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj | 17 ++++ src/SIL.Harmony.Tests/CreateBranchTests.cs | 46 +++++++++++ .../SIL.Harmony.Tests.csproj | 1 + 7 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 src/SIL.Harmony.Refs/Changes/CreateBranchChange.cs create mode 100644 src/SIL.Harmony.Refs/Entities/Branch.cs create mode 100644 src/SIL.Harmony.Refs/HarmonyRefsKernel.cs create mode 100644 src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj create mode 100644 src/SIL.Harmony.Tests/CreateBranchTests.cs diff --git a/harmony.sln b/harmony.sln index a44dc0d..62a2cac 100644 --- a/harmony.sln +++ b/harmony.sln @@ -22,38 +22,109 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution src\.editorconfig = src\.editorconfig EndProjectSection EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{827E0CD3-B72D-47B6-A68D-7590B98EB39B}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SIL.Harmony.Refs", "src\SIL.Harmony.Refs\SIL.Harmony.Refs.csproj", "{C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU - EndGlobalSection - GlobalSection(SolutionProperties) = preSolution - HideSolutionNode = FALSE + Release|x64 = Release|x64 + Release|x86 = Release|x86 EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|Any CPU.Build.0 = Debug|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x64.ActiveCfg = Debug|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x64.Build.0 = Debug|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x86.ActiveCfg = Debug|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x86.Build.0 = Debug|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|Any CPU.ActiveCfg = Release|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|Any CPU.Build.0 = Release|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x64.ActiveCfg = Release|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x64.Build.0 = Release|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x86.ActiveCfg = Release|Any CPU + {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x86.Build.0 = Release|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x64.ActiveCfg = Debug|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x64.Build.0 = Debug|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x86.ActiveCfg = Debug|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x86.Build.0 = Debug|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|Any CPU.ActiveCfg = Release|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|Any CPU.Build.0 = Release|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x64.ActiveCfg = Release|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x64.Build.0 = Release|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x86.ActiveCfg = Release|Any CPU + {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x86.Build.0 = Release|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|Any CPU.Build.0 = Debug|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x64.ActiveCfg = Debug|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x64.Build.0 = Debug|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x86.ActiveCfg = Debug|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x86.Build.0 = Debug|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|Any CPU.ActiveCfg = Release|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|Any CPU.Build.0 = Release|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x64.ActiveCfg = Release|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x64.Build.0 = Release|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x86.ActiveCfg = Release|Any CPU + {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x86.Build.0 = Release|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|Any CPU.Build.0 = Debug|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x64.ActiveCfg = Debug|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x64.Build.0 = Debug|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x86.ActiveCfg = Debug|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x86.Build.0 = Debug|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|Any CPU.ActiveCfg = Release|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|Any CPU.Build.0 = Release|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x64.ActiveCfg = Release|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x64.Build.0 = Release|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x86.ActiveCfg = Release|Any CPU + {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x86.Build.0 = Release|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|Any CPU.Build.0 = Debug|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x64.ActiveCfg = Debug|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x64.Build.0 = Debug|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x86.ActiveCfg = Debug|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x86.Build.0 = Debug|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|Any CPU.ActiveCfg = Release|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|Any CPU.Build.0 = Release|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x64.ActiveCfg = Release|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x64.Build.0 = Release|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x86.ActiveCfg = Release|Any CPU + {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x86.Build.0 = Release|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|Any CPU.Build.0 = Debug|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x64.ActiveCfg = Debug|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x64.Build.0 = Debug|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x86.ActiveCfg = Debug|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x86.Build.0 = Debug|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|Any CPU.ActiveCfg = Release|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|Any CPU.Build.0 = Release|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x64.ActiveCfg = Release|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x64.Build.0 = Release|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x86.ActiveCfg = Release|Any CPU + {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x86.Build.0 = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x64.ActiveCfg = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x64.Build.0 = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x86.ActiveCfg = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x86.Build.0 = Debug|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|Any CPU.Build.0 = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x64.ActiveCfg = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x64.Build.0 = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x86.ActiveCfg = Release|Any CPU + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x86.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection + GlobalSection(NestedProjects) = preSolution + {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} EndGlobalSection EndGlobal diff --git a/src/SIL.Harmony.Refs/Changes/CreateBranchChange.cs b/src/SIL.Harmony.Refs/Changes/CreateBranchChange.cs new file mode 100644 index 0000000..f994fc2 --- /dev/null +++ b/src/SIL.Harmony.Refs/Changes/CreateBranchChange.cs @@ -0,0 +1,21 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Entities; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs.Changes; + +public class CreateBranchChange(Guid entityId, string name) : CreateChange(entityId), IPolyType +{ + public string Name { get; } = name; + + public override ValueTask NewEntity(Commit commit, IChangeContext context) + { + return ValueTask.FromResult(new Branch + { + Id = EntityId, + Name = Name + }); + } + + public static string TypeName => "create:branch"; +} diff --git a/src/SIL.Harmony.Refs/Entities/Branch.cs b/src/SIL.Harmony.Refs/Entities/Branch.cs new file mode 100644 index 0000000..46e332e --- /dev/null +++ b/src/SIL.Harmony.Refs/Entities/Branch.cs @@ -0,0 +1,26 @@ +using SIL.Harmony.Entities; + +namespace SIL.Harmony.Refs.Entities; + +/// +/// A named line of work. Display is not unique; identity is . +/// +public class Branch : IObjectBase +{ + public Guid Id { get; init; } + public required string Name { get; set; } + public DateTimeOffset? DeletedAt { get; set; } + + public Guid[] GetReferences() => []; + + public void RemoveReference(Guid id, CommitBase commit) + { + } + + public IObjectBase Copy() => new Branch + { + Id = Id, + Name = Name, + DeletedAt = DeletedAt + }; +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs new file mode 100644 index 0000000..7111b1d --- /dev/null +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -0,0 +1,17 @@ +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs; + +public static class HarmonyRefsKernel +{ + /// + /// Registers branch (and later tag) ref entities and change types on an existing Harmony . + /// + public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) + { + config.ObjectTypeListBuilder.DefaultAdapter().Add(); + config.ChangeTypeListBuilder.Add(); + return config; + } +} diff --git a/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj new file mode 100644 index 0000000..1bebbfd --- /dev/null +++ b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj @@ -0,0 +1,17 @@ + + + + SIL.Harmony.Refs + true + Optional branches and tags layer for SIL.Harmony. + + + + + + + + + + + diff --git a/src/SIL.Harmony.Tests/CreateBranchTests.cs b/src/SIL.Harmony.Tests/CreateBranchTests.cs new file mode 100644 index 0000000..87def89 --- /dev/null +++ b/src/SIL.Harmony.Tests/CreateBranchTests.cs @@ -0,0 +1,46 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Refs.Entities; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class CreateBranchTests : DataModelTestBase +{ + public CreateBranchTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + }) + { + } + + [Fact] + public async Task CanCreateAndReadBranch() + { + var branchId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature-x")); + + var branch = await DataModel.GetLatest(branchId); + branch.Should().NotBeNull(); + branch!.Id.Should().Be(branchId); + branch.Name.Should().Be("feature-x"); + } + + [Fact] + public async Task DuplicateBranchNamesAreAllowed() + { + var firstId = Guid.NewGuid(); + var secondId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(firstId, "dup")); + await DataModel.AddChange(_localClientId, new CreateBranchChange(secondId, "dup")); + + var first = await DataModel.GetLatest(firstId); + var second = await DataModel.GetLatest(secondId); + first!.Name.Should().Be("dup"); + second!.Name.Should().Be("dup"); + first.Id.Should().NotBe(second.Id); + + var both = DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken).ToArray(); + both.Should().HaveCount(2); + } +} diff --git a/src/SIL.Harmony.Tests/SIL.Harmony.Tests.csproj b/src/SIL.Harmony.Tests/SIL.Harmony.Tests.csproj index f542263..3af010d 100644 --- a/src/SIL.Harmony.Tests/SIL.Harmony.Tests.csproj +++ b/src/SIL.Harmony.Tests/SIL.Harmony.Tests.csproj @@ -34,6 +34,7 @@ + From 93e8b1490f032896a5ec32c63c7766674c19ad08 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 11:31:32 +0700 Subject: [PATCH 04/18] Clean solution entry for SIL.Harmony.Refs. Keep Any CPU only and list Refs alongside the other projects without the x64/x86 platform churn from dotnet sln add. Co-authored-by: Cursor --- harmony.sln | 75 ++++------------------------------------------------- 1 file changed, 5 insertions(+), 70 deletions(-) diff --git a/harmony.sln b/harmony.sln index 62a2cac..129cce0 100644 --- a/harmony.sln +++ b/harmony.sln @@ -15,6 +15,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SIL.Harmony.Tests", "src\SI EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SIL.Harmony.Linq2db", "src\SIL.Harmony.Linq2db\SIL.Harmony.Linq2db.csproj", "{8EB807F6-C548-4016-856E-2ACCA5603036}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SIL.Harmony.Refs", "src\SIL.Harmony.Refs\SIL.Harmony.Refs.csproj", "{C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}" +EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{321635F9-11D9-4B45-A2F8-F3B70E9A4196}" ProjectSection(SolutionItems) = preProject src\Directory.Build.props = src\Directory.Build.props @@ -22,109 +24,42 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution src\.editorconfig = src\.editorconfig EndProjectSection EndProject -Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{827E0CD3-B72D-47B6-A68D-7590B98EB39B}" -EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SIL.Harmony.Refs", "src\SIL.Harmony.Refs\SIL.Harmony.Refs.csproj", "{C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}" -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU - Debug|x64 = Debug|x64 - Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU - Release|x64 = Release|x64 - Release|x86 = Release|x86 + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|Any CPU.Build.0 = Debug|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x64.ActiveCfg = Debug|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x64.Build.0 = Debug|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x86.ActiveCfg = Debug|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Debug|x86.Build.0 = Debug|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|Any CPU.ActiveCfg = Release|Any CPU {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|Any CPU.Build.0 = Release|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x64.ActiveCfg = Release|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x64.Build.0 = Release|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x86.ActiveCfg = Release|Any CPU - {9AD0CB91-4C08-4CA8-B65A-9B9217F316B2}.Release|x86.Build.0 = Release|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|Any CPU.Build.0 = Debug|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x64.ActiveCfg = Debug|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x64.Build.0 = Debug|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x86.ActiveCfg = Debug|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Debug|x86.Build.0 = Debug|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|Any CPU.ActiveCfg = Release|Any CPU {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|Any CPU.Build.0 = Release|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x64.ActiveCfg = Release|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x64.Build.0 = Release|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x86.ActiveCfg = Release|Any CPU - {C4F67DF7-7DA3-4BA6-93E1-13C12AC1B027}.Release|x86.Build.0 = Release|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|Any CPU.Build.0 = Debug|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x64.ActiveCfg = Debug|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x64.Build.0 = Debug|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x86.ActiveCfg = Debug|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Debug|x86.Build.0 = Debug|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|Any CPU.ActiveCfg = Release|Any CPU {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|Any CPU.Build.0 = Release|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x64.ActiveCfg = Release|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x64.Build.0 = Release|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x86.ActiveCfg = Release|Any CPU - {AA69420C-9569-4374-8434-6804AA7EC9AE}.Release|x86.Build.0 = Release|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|Any CPU.Build.0 = Debug|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x64.ActiveCfg = Debug|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x64.Build.0 = Debug|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x86.ActiveCfg = Debug|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Debug|x86.Build.0 = Debug|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|Any CPU.ActiveCfg = Release|Any CPU {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|Any CPU.Build.0 = Release|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x64.ActiveCfg = Release|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x64.Build.0 = Release|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x86.ActiveCfg = Release|Any CPU - {BAB394E4-AC1F-4E9B-BA18-88FA70341C1F}.Release|x86.Build.0 = Release|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|Any CPU.Build.0 = Debug|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x64.ActiveCfg = Debug|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x64.Build.0 = Debug|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x86.ActiveCfg = Debug|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Debug|x86.Build.0 = Debug|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|Any CPU.ActiveCfg = Release|Any CPU {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|Any CPU.Build.0 = Release|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x64.ActiveCfg = Release|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x64.Build.0 = Release|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x86.ActiveCfg = Release|Any CPU - {58A1DABF-1662-4E1E-A3B7-9F3AC7FC678F}.Release|x86.Build.0 = Release|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|Any CPU.Build.0 = Debug|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x64.ActiveCfg = Debug|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x64.Build.0 = Debug|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x86.ActiveCfg = Debug|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Debug|x86.Build.0 = Debug|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|Any CPU.ActiveCfg = Release|Any CPU {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|Any CPU.Build.0 = Release|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x64.ActiveCfg = Release|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x64.Build.0 = Release|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x86.ActiveCfg = Release|Any CPU - {8EB807F6-C548-4016-856E-2ACCA5603036}.Release|x86.Build.0 = Release|Any CPU {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|Any CPU.Build.0 = Debug|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x64.ActiveCfg = Debug|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x64.Build.0 = Debug|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x86.ActiveCfg = Debug|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Debug|x86.Build.0 = Debug|Any CPU {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|Any CPU.ActiveCfg = Release|Any CPU {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|Any CPU.Build.0 = Release|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x64.ActiveCfg = Release|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x64.Build.0 = Release|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x86.ActiveCfg = Release|Any CPU - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744}.Release|x86.Build.0 = Release|Any CPU - EndGlobalSection - GlobalSection(SolutionProperties) = preSolution - HideSolutionNode = FALSE - EndGlobalSection - GlobalSection(NestedProjects) = preSolution - {C1BAB3FF-7B5B-4847-B5DF-7B87BB0DA744} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} EndGlobalSection EndGlobal From 23319c2f2df7330c965362461c67312a2354300a Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 11:42:02 +0700 Subject: [PATCH 05/18] Add branch-scoped authoring with main-line query isolation. RefsDataModel stamps immutable harmony.branchId from local checkout (overridable), and AddHarmonyRefs filters those commits out of main materialization until merge or a branch checkout view lands. Co-authored-by: Cursor --- src/SIL.Harmony.Refs/BranchAssignment.cs | 32 +++++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 13 ++- .../MainLineOnlyMaterializationFilter.cs | 13 +++ src/SIL.Harmony.Refs/RefCheckout.cs | 15 ++++ src/SIL.Harmony.Refs/RefMetadata.cs | 29 +++++++ src/SIL.Harmony.Refs/RefsDataModel.cs | 58 +++++++++++++ src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj | 2 +- .../ScopedAuthoringMainIsolationTests.cs | 83 +++++++++++++++++++ 8 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 src/SIL.Harmony.Refs/BranchAssignment.cs create mode 100644 src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs create mode 100644 src/SIL.Harmony.Refs/RefCheckout.cs create mode 100644 src/SIL.Harmony.Refs/RefMetadata.cs create mode 100644 src/SIL.Harmony.Refs/RefsDataModel.cs create mode 100644 src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs diff --git a/src/SIL.Harmony.Refs/BranchAssignment.cs b/src/SIL.Harmony.Refs/BranchAssignment.cs new file mode 100644 index 0000000..468ed05 --- /dev/null +++ b/src/SIL.Harmony.Refs/BranchAssignment.cs @@ -0,0 +1,32 @@ +namespace SIL.Harmony.Refs; + +/// +/// Where a newly authored commit should be assigned. +/// +public readonly record struct BranchAssignment +{ + private BranchAssignment(BranchAssignmentKind kind, Guid? branchId) + { + Kind = kind; + BranchId = branchId; + } + + public BranchAssignmentKind Kind { get; } + public Guid? BranchId { get; } + + /// Use the current local checkout (default). + public static BranchAssignment FromCheckout { get; } = new(BranchAssignmentKind.FromCheckout, null); + + /// Force main line (no branch metadata). + public static BranchAssignment Main { get; } = new(BranchAssignmentKind.Main, null); + + /// Force a specific branch. + public static BranchAssignment ToBranch(Guid branchId) => new(BranchAssignmentKind.Branch, branchId); +} + +public enum BranchAssignmentKind +{ + FromCheckout, + Main, + Branch +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index 7111b1d..5f701dc 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -1,17 +1,28 @@ using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; +using Microsoft.Extensions.DependencyInjection; namespace SIL.Harmony.Refs; public static class HarmonyRefsKernel { /// - /// Registers branch (and later tag) ref entities and change types on an existing Harmony . + /// Registers branch ref entities, change types, and main-line-only materialization on an existing Harmony . /// public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) { config.ObjectTypeListBuilder.DefaultAdapter().Add(); config.ChangeTypeListBuilder.Add(); + config.CommitMaterializationFilter = MainLineOnlyMaterializationFilter.Instance; return config; } + + /// + /// Registers for DI. Call alongside . + /// + public static IServiceCollection AddHarmonyRefsDataModel(this IServiceCollection services) + { + services.AddScoped(); + return services; + } } diff --git a/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs b/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs new file mode 100644 index 0000000..21884d3 --- /dev/null +++ b/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs @@ -0,0 +1,13 @@ +namespace SIL.Harmony.Refs; + +/// +/// Materializes only main-line commits (no ). +/// Branch-scoped commits remain stored and synced; they become queryable after merge (later) or on a branch checkout view (ticket 11). +/// +public sealed class MainLineOnlyMaterializationFilter : ICommitMaterializationFilter +{ + public static MainLineOnlyMaterializationFilter Instance { get; } = new(); + + public bool Include(Commit commit) => + RefMetadata.GetBranchId(commit.Metadata) is null; +} diff --git a/src/SIL.Harmony.Refs/RefCheckout.cs b/src/SIL.Harmony.Refs/RefCheckout.cs new file mode 100644 index 0000000..6348e52 --- /dev/null +++ b/src/SIL.Harmony.Refs/RefCheckout.cs @@ -0,0 +1,15 @@ +namespace SIL.Harmony.Refs; + +/// +/// Local checkout selection. Not synced; only affects authoring defaults and (later) materialization views. +/// +public abstract record RefCheckout +{ + public static RefCheckout Main { get; } = new MainCheckout(); + + public static RefCheckout ForBranch(Guid branchId) => new BranchCheckout(branchId); +} + +public sealed record MainCheckout : RefCheckout; + +public sealed record BranchCheckout(Guid BranchId) : RefCheckout; diff --git a/src/SIL.Harmony.Refs/RefMetadata.cs b/src/SIL.Harmony.Refs/RefMetadata.cs new file mode 100644 index 0000000..b0e086f --- /dev/null +++ b/src/SIL.Harmony.Refs/RefMetadata.cs @@ -0,0 +1,29 @@ +namespace SIL.Harmony.Refs; + +/// +/// Well-known keys for the refs layer. +/// +public static class RefMetadata +{ + /// + /// Immutable branch assignment for a commit. Absent or empty means main line. + /// + public const string BranchIdKey = "harmony.branchId"; + + public static Guid? GetBranchId(CommitMetadata metadata) + { + var value = metadata[BranchIdKey]; + return Guid.TryParse(value, out var id) ? id : null; + } + + public static void SetBranchId(CommitMetadata metadata, Guid? branchId) + { + if (branchId is null) + { + metadata.ExtraMetadata.Remove(BranchIdKey); + return; + } + + metadata[BranchIdKey] = branchId.Value.ToString(); + } +} diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs new file mode 100644 index 0000000..5d1a4bd --- /dev/null +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -0,0 +1,58 @@ +using SIL.Harmony.Changes; + +namespace SIL.Harmony.Refs; + +/// +/// Thin authoring/checkout wrapper over . +/// Checkout is local and not synced. +/// +public class RefsDataModel(DataModel dataModel) +{ + public DataModel DataModel { get; } = dataModel; + + public RefCheckout Checkout { get; private set; } = RefCheckout.Main; + + public void CheckoutMain() => Checkout = RefCheckout.Main; + + public void CheckoutBranch(Guid branchId) => Checkout = RefCheckout.ForBranch(branchId); + + public Task AddChange( + Guid clientId, + IChange change, + BranchAssignment assignment = default, + CommitMetadata? commitMetadata = null) + { + if (assignment == default) assignment = BranchAssignment.FromCheckout; + return DataModel.AddChange(clientId, change, ApplyAssignment(commitMetadata, assignment)); + } + + public Task AddChanges( + Guid clientId, + IEnumerable changes, + BranchAssignment assignment = default, + CommitMetadata? commitMetadata = null) + { + if (assignment == default) assignment = BranchAssignment.FromCheckout; + return DataModel.AddChanges(clientId, changes, ApplyAssignment(commitMetadata, assignment)); + } + + private CommitMetadata ApplyAssignment(CommitMetadata? commitMetadata, BranchAssignment assignment) + { + var metadata = commitMetadata ?? new CommitMetadata(); + var branchId = ResolveBranchId(assignment); + RefMetadata.SetBranchId(metadata, branchId); + return metadata; + } + + private Guid? ResolveBranchId(BranchAssignment assignment) => assignment.Kind switch + { + BranchAssignmentKind.Main => null, + BranchAssignmentKind.Branch => assignment.BranchId, + BranchAssignmentKind.FromCheckout => Checkout switch + { + BranchCheckout branch => branch.BranchId, + _ => null + }, + _ => null + }; +} diff --git a/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj index 1bebbfd..61b5972 100644 --- a/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj +++ b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj @@ -3,7 +3,7 @@ SIL.Harmony.Refs true - Optional branches and tags layer for SIL.Harmony. + Optional branches layer for SIL.Harmony. diff --git a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs new file mode 100644 index 0000000..964fc6f --- /dev/null +++ b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs @@ -0,0 +1,83 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Sample.Models; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class ScopedAuthoringMainIsolationTests : DataModelTestBase +{ + private readonly RefsDataModel _refs; + + public ScopedAuthoringMainIsolationTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }) + { + _refs = _services.GetRequiredService(); + } + + [Fact] + public async Task BranchAuthoringIsHiddenOnMainCheckout() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + _refs.CheckoutBranch(branchId); + await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + + _refs.CheckoutMain(); + var word = await DataModel.GetLatest(wordId); + word.Should().BeNull(); + DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken) + .Should().BeEmpty(); + } + + [Fact] + public async Task BranchAuthoringSetsImmutableBranchMetadata() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + _refs.CheckoutBranch(branchId); + var commit = await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + + RefMetadata.GetBranchId(commit.Metadata).Should().Be(branchId); + + // later commits do not rewrite earlier assignment + await _refs.AddChange(_localClientId, SetWord(wordId, "again")); + var stored = await DbContext.Commits.AsNoTracking().SingleAsync(c => c.Id == commit.Id, TestContext.Current.CancellationToken); + RefMetadata.GetBranchId(stored.Metadata).Should().Be(branchId); + } + + [Fact] + public async Task AuthoringOverrideCanForceMainOrAnotherBranch() + { + var featureId = Guid.NewGuid(); + var otherId = Guid.NewGuid(); + var mainWordId = Guid.NewGuid(); + var otherWordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); + await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + + _refs.CheckoutBranch(featureId); + + var mainCommit = await _refs.AddChange( + _localClientId, + SetWord(mainWordId, "forced-main"), + BranchAssignment.Main); + RefMetadata.GetBranchId(mainCommit.Metadata).Should().BeNull(); + (await DataModel.GetLatest(mainWordId))!.Text.Should().Be("forced-main"); + + var otherCommit = await _refs.AddChange( + _localClientId, + SetWord(otherWordId, "other-branch"), + BranchAssignment.ToBranch(otherId)); + RefMetadata.GetBranchId(otherCommit.Metadata).Should().Be(otherId); + (await DataModel.GetLatest(otherWordId)).Should().BeNull(); + } +} From 82ad90a2a08fbb48237c7e418da65f849f07c417 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 12:09:44 +0700 Subject: [PATCH 06/18] Add checkout-aware branch views with rematerialization. Switching RefsDataModel between main and a branch updates the materialization filter and regenerates snapshots so queries show main plus that branch only. Co-authored-by: Cursor --- .../CheckoutMaterializationFilter.cs | 20 ++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 9 +- .../MainLineOnlyMaterializationFilter.cs | 3 +- src/SIL.Harmony.Refs/RefsDataModel.cs | 20 +++- .../BranchCheckoutViewTests.cs | 94 +++++++++++++++++++ .../ScopedAuthoringMainIsolationTests.cs | 8 +- src/SIL.Harmony/CrdtKernel.cs | 3 +- src/SIL.Harmony/DataModel.cs | 10 +- 8 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs create mode 100644 src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs new file mode 100644 index 0000000..9aeb68f --- /dev/null +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -0,0 +1,20 @@ +namespace SIL.Harmony.Refs; + +/// +/// Materializes commits visible for the current local checkout: +/// main line (no branch id), plus commits scoped to the checked-out branch when on a branch. +/// +public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter +{ + public RefCheckout Checkout { get; set; } = RefCheckout.Main; + + public bool Include(Commit commit) + { + var branchId = RefMetadata.GetBranchId(commit.Metadata); + return Checkout switch + { + BranchCheckout branch => branchId is null || branchId == branch.BranchId, + _ => branchId is null + }; + } +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index 5f701dc..2c81dc7 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -7,7 +7,8 @@ namespace SIL.Harmony.Refs; public static class HarmonyRefsKernel { /// - /// Registers branch ref entities, change types, and main-line-only materialization on an existing Harmony . + /// Registers branch ref entities and change types. Sets a main-line-only materialization filter + /// as a fallback when is not used. /// public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) { @@ -18,10 +19,14 @@ public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) } /// - /// Registers for DI. Call alongside . + /// Registers checkout-aware materialization and . + /// Prefer this for apps that switch between main and branch views. /// public static IServiceCollection AddHarmonyRefsDataModel(this IServiceCollection services) { + services.AddScoped(); + services.AddScoped(sp => + sp.GetRequiredService()); services.AddScoped(); return services; } diff --git a/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs b/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs index 21884d3..d67ec94 100644 --- a/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/MainLineOnlyMaterializationFilter.cs @@ -2,7 +2,8 @@ namespace SIL.Harmony.Refs; /// /// Materializes only main-line commits (no ). -/// Branch-scoped commits remain stored and synced; they become queryable after merge (later) or on a branch checkout view (ticket 11). +/// Used as the fallback when checkout-aware DI is not registered. +/// Prefer via AddHarmonyRefsDataModel for branch views. /// public sealed class MainLineOnlyMaterializationFilter : ICommitMaterializationFilter { diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 5d1a4bd..1bf71b0 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -4,17 +4,29 @@ namespace SIL.Harmony.Refs; /// /// Thin authoring/checkout wrapper over . -/// Checkout is local and not synced. +/// Checkout is local and not synced; changing it rematerializes snapshots for the new view. /// -public class RefsDataModel(DataModel dataModel) +public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter filter) { public DataModel DataModel { get; } = dataModel; public RefCheckout Checkout { get; private set; } = RefCheckout.Main; - public void CheckoutMain() => Checkout = RefCheckout.Main; + public async Task CheckoutMain() + { + if (Checkout is MainCheckout) return; + Checkout = RefCheckout.Main; + filter.Checkout = Checkout; + await DataModel.RegenerateSnapshots(); + } - public void CheckoutBranch(Guid branchId) => Checkout = RefCheckout.ForBranch(branchId); + public async Task CheckoutBranch(Guid branchId) + { + if (Checkout is BranchCheckout current && current.BranchId == branchId) return; + Checkout = RefCheckout.ForBranch(branchId); + filter.Checkout = Checkout; + await DataModel.RegenerateSnapshots(); + } public Task AddChange( Guid clientId, diff --git a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs new file mode 100644 index 0000000..e8486ea --- /dev/null +++ b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs @@ -0,0 +1,94 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Sample.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class BranchCheckoutViewTests : DataModelTestBase +{ + private readonly RefsDataModel _refs; + + public BranchCheckoutViewTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }) + { + _refs = _services.GetRequiredService(); + } + + [Fact] + public async Task BranchCheckoutShowsMainPlusBranchCommits() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.AddChange(_localClientId, SetWord(wordId, "main"), BranchAssignment.Main); + + await _refs.CheckoutBranch(branchId); + await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + + (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-branch"); + + await _refs.CheckoutMain(); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("main"); + } + + [Fact] + public async Task OtherBranchCommitsStayInvisible() + { + var featureId = Guid.NewGuid(); + var otherId = Guid.NewGuid(); + var featureWordId = Guid.NewGuid(); + var otherWordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); + await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + + await _refs.CheckoutBranch(featureId); + await _refs.AddChange(_localClientId, SetWord(featureWordId, "feature-word")); + + await _refs.CheckoutBranch(otherId); + await _refs.AddChange(_localClientId, SetWord(otherWordId, "other-word")); + + await _refs.CheckoutBranch(featureId); + (await DataModel.GetLatest(featureWordId))!.Text.Should().Be("feature-word"); + (await DataModel.GetLatest(otherWordId)).Should().BeNull(); + } + + [Fact] + public async Task BranchAndMainCommitsInterleaveByAuthorTime() + { + var branchId = Guid.NewGuid(); + var earlyId = Guid.NewGuid(); + var lateId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + // t1 main + await _refs.AddChange(_localClientId, SetWord(earlyId, "early-main"), BranchAssignment.Main); + await _refs.CheckoutBranch(branchId); + // t2 branch (between main commits chronologically once late main is written) + await _refs.AddChange(_localClientId, SetWord(Guid.NewGuid(), "mid-branch")); + await _refs.CheckoutMain(); + // t3 main + await _refs.AddChange(_localClientId, SetWord(lateId, "late-main"), BranchAssignment.Main); + + await _refs.CheckoutBranch(branchId); + var words = DataModel.QueryLatest() + .ToBlockingEnumerable(TestContext.Current.CancellationToken) + .Select(w => w.Text) + .OrderBy(t => t) + .ToArray(); + + // All three domain commits visible on branch; main-only would miss mid-branch + words.Should().BeEquivalentTo(["early-main", "late-main", "mid-branch"]); + + await _refs.CheckoutMain(); + var mainWords = DataModel.QueryLatest() + .ToBlockingEnumerable(TestContext.Current.CancellationToken) + .Select(w => w.Text) + .OrderBy(t => t) + .ToArray(); + mainWords.Should().BeEquivalentTo(["early-main", "late-main"]); + } +} diff --git a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs index 964fc6f..d9fd539 100644 --- a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs +++ b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs @@ -26,10 +26,10 @@ public async Task BranchAuthoringIsHiddenOnMainCheckout() var wordId = Guid.NewGuid(); await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); - _refs.CheckoutBranch(branchId); + await _refs.CheckoutBranch(branchId); await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); - _refs.CheckoutMain(); + await _refs.CheckoutMain(); var word = await DataModel.GetLatest(wordId); word.Should().BeNull(); DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken) @@ -43,7 +43,7 @@ public async Task BranchAuthoringSetsImmutableBranchMetadata() var wordId = Guid.NewGuid(); await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); - _refs.CheckoutBranch(branchId); + await _refs.CheckoutBranch(branchId); var commit = await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); RefMetadata.GetBranchId(commit.Metadata).Should().Be(branchId); @@ -64,7 +64,7 @@ public async Task AuthoringOverrideCanForceMainOrAnotherBranch() await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); - _refs.CheckoutBranch(featureId); + await _refs.CheckoutBranch(featureId); var mainCommit = await _refs.AddChange( _localClientId, diff --git a/src/SIL.Harmony/CrdtKernel.cs b/src/SIL.Harmony/CrdtKernel.cs index 2d1c95d..d622f91 100644 --- a/src/SIL.Harmony/CrdtKernel.cs +++ b/src/SIL.Harmony/CrdtKernel.cs @@ -40,7 +40,8 @@ public static IServiceCollection AddCrdtDataCore(this IServiceCollection service provider.GetRequiredService(), provider.GetRequiredService(), provider.GetRequiredService>(), - provider.GetRequiredService>() + provider.GetRequiredService>(), + provider.GetService() )); //must use factory method because ResourceService constructor is internal services.AddScoped(provider => new ResourceService( diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 5b36323..200d752 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -23,21 +23,27 @@ public class DataModel : ISyncable, IAsyncDisposable private readonly IHybridDateTimeProvider _timeProvider; private readonly IOptions _crdtConfig; private readonly ILogger _logger; + private readonly ICommitMaterializationFilter? _materializationFilter; //constructor must be internal because CrdtRepository is internal internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, JsonSerializerOptions serializerOptions, IHybridDateTimeProvider timeProvider, IOptions crdtConfig, - ILogger logger) + ILogger logger, + ICommitMaterializationFilter? materializationFilter = null) { _crdtRepositoryFactory = crdtRepositoryFactory; _serializerOptions = serializerOptions; _timeProvider = timeProvider; _crdtConfig = crdtConfig; _logger = logger; + _materializationFilter = materializationFilter; } + private ICommitMaterializationFilter MaterializationFilter => + _materializationFilter ?? _crdtConfig.Value.CommitMaterializationFilter; + /// /// add a change to the model, snapshots will be updated @@ -186,7 +192,7 @@ ValueTask ISyncable.ShouldSync() private async Task UpdateSnapshots(CrdtRepository repo, SortedSet commitsToApply) { if (commitsToApply.Count == 0) return; - var filter = _crdtConfig.Value.CommitMaterializationFilter; + var filter = MaterializationFilter; var commitsToMaterialize = commitsToApply .Where(filter.Include) .ToSortedSet(); From bfa3aaad9c8db7db367a2b25bec6197ea2a4154a Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 12:15:41 +0700 Subject: [PATCH 07/18] Use CheckoutMaterializationFilter as the sole checkout source. RefsDataModel now reads and updates filter.Checkout only, so authoring and materialization cannot diverge. Co-authored-by: Cursor --- src/SIL.Harmony.Refs/RefsDataModel.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 1bf71b0..4d06fe4 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -4,27 +4,26 @@ namespace SIL.Harmony.Refs; /// /// Thin authoring/checkout wrapper over . -/// Checkout is local and not synced; changing it rematerializes snapshots for the new view. +/// Checkout lives on (single source of truth); +/// changing it rematerializes snapshots for the new view. /// public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter filter) { public DataModel DataModel { get; } = dataModel; - public RefCheckout Checkout { get; private set; } = RefCheckout.Main; + public RefCheckout Checkout => filter.Checkout; public async Task CheckoutMain() { if (Checkout is MainCheckout) return; - Checkout = RefCheckout.Main; - filter.Checkout = Checkout; + filter.Checkout = RefCheckout.Main; await DataModel.RegenerateSnapshots(); } public async Task CheckoutBranch(Guid branchId) { if (Checkout is BranchCheckout current && current.BranchId == branchId) return; - Checkout = RefCheckout.ForBranch(branchId); - filter.Checkout = Checkout; + filter.Checkout = RefCheckout.ForBranch(branchId); await DataModel.RegenerateSnapshots(); } From bdbb0a7f6d0a2a3229d6423a9db17fa2af078e49 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 12:23:15 +0700 Subject: [PATCH 08/18] Add branch merge with interleaved rematerialization. MergeBranchChange incorporates a branch on main; the checkout filter expands the apply window from the earliest branch commit so snapshots replay in HybridDateTime order without rewriting commit metadata. Co-authored-by: Cursor --- .../Changes/MergeBranchChange.cs | 20 ++++ .../CheckoutMaterializationFilter.cs | 61 +++++++++- src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 1 + src/SIL.Harmony.Refs/RefsDataModel.cs | 11 ++ src/SIL.Harmony.Tests/MergeBranchTests.cs | 107 ++++++++++++++++++ src/SIL.Harmony/DataModel.cs | 3 + .../IMaterializationApplyWindow.cs | 12 ++ src/SIL.Harmony/SIL.Harmony.csproj | 1 + 8 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 src/SIL.Harmony.Refs/Changes/MergeBranchChange.cs create mode 100644 src/SIL.Harmony.Tests/MergeBranchTests.cs create mode 100644 src/SIL.Harmony/IMaterializationApplyWindow.cs diff --git a/src/SIL.Harmony.Refs/Changes/MergeBranchChange.cs b/src/SIL.Harmony.Refs/Changes/MergeBranchChange.cs new file mode 100644 index 0000000..cdb0f20 --- /dev/null +++ b/src/SIL.Harmony.Refs/Changes/MergeBranchChange.cs @@ -0,0 +1,20 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Entities; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs.Changes; + +/// +/// Main-line commit that incorporates a branch into main visibility and deletes the branch entity. +/// Does not rewrite branch commit payloads; materialization starts including those commits after this change. +/// +public class MergeBranchChange(Guid entityId) : EditChange(entityId), IPolyType +{ + public override ValueTask ApplyChange(Branch entity, IChangeContext context) + { + entity.DeletedAt = context.Commit.DateTime; + return ValueTask.CompletedTask; + } + + public static string TypeName => "merge:branch"; +} diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs index 9aeb68f..135392c 100644 --- a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -1,20 +1,73 @@ +using Microsoft.EntityFrameworkCore; +using SIL.Harmony.Db; +using SIL.Harmony.Refs.Changes; + namespace SIL.Harmony.Refs; /// /// Materializes commits visible for the current local checkout: -/// main line (no branch id), plus commits scoped to the checked-out branch when on a branch. +/// main line (no branch id), commits for incorporated (merged) branches on main, +/// plus commits scoped to the checked-out branch when on a branch. /// -public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter +public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter, IMaterializationApplyWindow { + private readonly HashSet _incorporatedBranchIds = []; + public RefCheckout Checkout { get; set; } = RefCheckout.Main; public bool Include(Commit commit) { var branchId = RefMetadata.GetBranchId(commit.Metadata); + if (branchId is null) return true; + return Checkout switch { - BranchCheckout branch => branchId is null || branchId == branch.BranchId, - _ => branchId is null + BranchCheckout branch => branchId == branch.BranchId || _incorporatedBranchIds.Contains(branchId.Value), + _ => _incorporatedBranchIds.Contains(branchId.Value) }; } + + async Task> IMaterializationApplyWindow.PrepareApplyWindowAsync( + CrdtRepository repo, + SortedSet commitsToApply) + { + var allCommits = await repo.CurrentCommits() + .Include(c => c.ChangeEntities) + .AsNoTracking() + .ToSortedSetAsync(); + + _incorporatedBranchIds.Clear(); + foreach (var commit in allCommits) + { + foreach (var merge in MergesIn(commit)) + _incorporatedBranchIds.Add(merge.EntityId); + } + + var mergedInWindow = commitsToApply + .SelectMany(MergesIn) + .Select(m => m.EntityId) + .ToHashSet(); + if (mergedInWindow.Count == 0) return commitsToApply; + + Commit? oldest = commitsToApply.MinBy(c => c.CompareKey); + foreach (var commit in allCommits) + { + var branchId = RefMetadata.GetBranchId(commit.Metadata); + if (branchId is Guid id && mergedInWindow.Contains(id) && + (oldest is null || commit.CompareKey.CompareTo(oldest.CompareKey) < 0)) + { + oldest = commit; + } + } + + if (oldest is null) return commitsToApply; + + var parent = await repo.FindPreviousCommit(oldest); + return (await repo.GetCommitsAfter(parent)).ToSortedSet(); + } + + private static IEnumerable MergesIn(Commit commit) => + commit.ChangeEntities + .Select(ce => ce.Change) + .OfType(); } diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index 2c81dc7..fa5402c 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -14,6 +14,7 @@ public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) { config.ObjectTypeListBuilder.DefaultAdapter().Add(); config.ChangeTypeListBuilder.Add(); + config.ChangeTypeListBuilder.Add(); config.CommitMaterializationFilter = MainLineOnlyMaterializationFilter.Instance; return config; } diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 4d06fe4..12f4b1c 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -1,4 +1,5 @@ using SIL.Harmony.Changes; +using SIL.Harmony.Refs.Changes; namespace SIL.Harmony.Refs; @@ -27,6 +28,16 @@ public async Task CheckoutBranch(Guid branchId) await DataModel.RegenerateSnapshots(); } + /// + /// Incorporates into main visibility and deletes the branch entity. + /// Authored as a main-line commit; materialization expands from the earliest branch commit. + /// + public async Task MergeBranch(Guid clientId, Guid branchId) + { + await CheckoutMain(); + return await AddChange(clientId, new MergeBranchChange(branchId), BranchAssignment.Main); + } + public Task AddChange( Guid clientId, IChange change, diff --git a/src/SIL.Harmony.Tests/MergeBranchTests.cs b/src/SIL.Harmony.Tests/MergeBranchTests.cs new file mode 100644 index 0000000..b212857 --- /dev/null +++ b/src/SIL.Harmony.Tests/MergeBranchTests.cs @@ -0,0 +1,107 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Refs.Entities; +using SIL.Harmony.Sample.Models; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class MergeBranchTests : DataModelTestBase +{ + private readonly RefsDataModel _refs; + + public MergeBranchTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }) + { + _refs = _services.GetRequiredService(); + } + + [Fact] + public async Task MergeInterleavesBranchCommitsIntoMainByAuthorTime() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + // main: 1, 2, 3 — branch: A, B, C interleaved by time on the same entity + await AddAt(_localClientId, NextDate(), SetWord(wordId, "1")); + await _refs.CheckoutBranch(branchId); + await AddAt(_localClientId, NextDate(), SetWord(wordId, "A"), BranchMeta(branchId)); + await _refs.CheckoutMain(); + await AddAt(_localClientId, NextDate(), SetWord(wordId, "2")); + await _refs.CheckoutBranch(branchId); + await AddAt(_localClientId, NextDate(), SetWord(wordId, "B"), BranchMeta(branchId)); + await _refs.CheckoutMain(); + await AddAt(_localClientId, NextDate(), SetWord(wordId, "3")); + await _refs.CheckoutBranch(branchId); + await AddAt(_localClientId, NextDate(), SetWord(wordId, "C"), BranchMeta(branchId)); + + await _refs.CheckoutMain(); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("3"); + + var merge = await _refs.MergeBranch(_localClientId, branchId); + RefMetadata.GetBranchId(merge.Metadata).Should().BeNull(); + merge.ChangeEntities.Should().ContainSingle(ce => ce.Change is MergeBranchChange); + + (await DataModel.GetLatest(wordId))!.Text.Should().Be("C"); + + var branchScoped = await DbContext.Commits.AsNoTracking() + .ToListAsync(TestContext.Current.CancellationToken); + branchScoped.Where(c => RefMetadata.GetBranchId(c.Metadata) == branchId) + .Should().HaveCount(3); + + var deleted = await DataModel.GetLatest(branchId); + deleted!.DeletedAt.Should().NotBeNull(); + DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken) + .Should().BeEmpty(); + } + + [Fact] + public async Task MergeExpandsReplayFromEarliestBranchCommit() + { + var branchId = Guid.NewGuid(); + var earlyId = Guid.NewGuid(); + var lateId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + await _refs.CheckoutBranch(branchId); + var earlyBranch = await AddAt(_localClientId, NextDate(), SetWord(earlyId, "early-branch"), + BranchMeta(branchId)); + await _refs.CheckoutMain(); + await AddAt(_localClientId, NextDate(), SetWord(lateId, "late-main")); + + (await DataModel.GetLatest(earlyId)).Should().BeNull(); + (await DataModel.GetLatest(lateId))!.Text.Should().Be("late-main"); + + await _refs.MergeBranch(_localClientId, branchId); + + (await DataModel.GetLatest(earlyId))!.Text.Should().Be("early-branch"); + (await DataModel.GetLatest(lateId))!.Text.Should().Be("late-main"); + + var storedEarly = await DbContext.Commits.AsNoTracking() + .SingleAsync(c => c.Id == earlyBranch.Id, TestContext.Current.CancellationToken); + RefMetadata.GetBranchId(storedEarly.Metadata).Should().Be(branchId); + } + + private static CommitMetadata BranchMeta(Guid branchId) + { + var metadata = new CommitMetadata(); + RefMetadata.SetBranchId(metadata, branchId); + return metadata; + } + + private async Task AddAt( + Guid clientId, + DateTimeOffset dateTime, + IChange change, + CommitMetadata? commitMetadata = null) + { + MockTimeProvider.SetNextDateTime(dateTime); + return await DataModel.AddChange(clientId, change, commitMetadata); + } +} diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index 200d752..d8f766d 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -193,6 +193,9 @@ private async Task UpdateSnapshots(CrdtRepository repo, SortedSet commit { if (commitsToApply.Count == 0) return; var filter = MaterializationFilter; + if (filter is IMaterializationApplyWindow applyWindow) + commitsToApply = await applyWindow.PrepareApplyWindowAsync(repo, commitsToApply); + var commitsToMaterialize = commitsToApply .Where(filter.Include) .ToSortedSet(); diff --git a/src/SIL.Harmony/IMaterializationApplyWindow.cs b/src/SIL.Harmony/IMaterializationApplyWindow.cs new file mode 100644 index 0000000..ab97ebb --- /dev/null +++ b/src/SIL.Harmony/IMaterializationApplyWindow.cs @@ -0,0 +1,12 @@ +using SIL.Harmony.Db; + +namespace SIL.Harmony; + +/// +/// Optional materialization filter hook to expand the commit apply window +/// (e.g. when a merge makes earlier branch-scoped commits newly visible). +/// +internal interface IMaterializationApplyWindow +{ + Task> PrepareApplyWindowAsync(CrdtRepository repo, SortedSet commitsToApply); +} diff --git a/src/SIL.Harmony/SIL.Harmony.csproj b/src/SIL.Harmony/SIL.Harmony.csproj index 094fe40..95fbd63 100644 --- a/src/SIL.Harmony/SIL.Harmony.csproj +++ b/src/SIL.Harmony/SIL.Harmony.csproj @@ -8,6 +8,7 @@ + From b7e3ff6c661e791d9936aaf9d132a3a0f7a5dc2f Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 13:11:27 +0700 Subject: [PATCH 09/18] Query incorporated branches by change type and slice the merge apply window. Issue 12 follow-up: read merge entity ids from ChangeEntities via JSON ->>, then expand rematerialization from the first ordered commit without a second commits query. Co-authored-by: Cursor --- .../CheckoutMaterializationFilter.cs | 33 ++++++------- src/SIL.Harmony/Db/CrdtRepository.cs | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs index 135392c..7e5d5cb 100644 --- a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -31,17 +31,8 @@ async Task> IMaterializationApplyWindow.PrepareApplyWindowAsyn CrdtRepository repo, SortedSet commitsToApply) { - var allCommits = await repo.CurrentCommits() - .Include(c => c.ChangeEntities) - .AsNoTracking() - .ToSortedSetAsync(); - _incorporatedBranchIds.Clear(); - foreach (var commit in allCommits) - { - foreach (var merge in MergesIn(commit)) - _incorporatedBranchIds.Add(merge.EntityId); - } + _incorporatedBranchIds.UnionWith(await repo.GetEntityIdsForChangeType()); var mergedInWindow = commitsToApply .SelectMany(MergesIn) @@ -49,21 +40,25 @@ async Task> IMaterializationApplyWindow.PrepareApplyWindowAsyn .ToHashSet(); if (mergedInWindow.Count == 0) return commitsToApply; - Commit? oldest = commitsToApply.MinBy(c => c.CompareKey); - foreach (var commit in allCommits) + // CurrentCommits is DefaultOrder (oldest first). First hit is the window start: + // earliest merged-branch commit, or the apply-window head if that is older. + var applyMinId = commitsToApply.Min!.Id; + var allCommits = await repo.CurrentCommits() + .Include(c => c.ChangeEntities) + .ToListAsync(); + + for (var i = 0; i < allCommits.Count; i++) { + var commit = allCommits[i]; var branchId = RefMetadata.GetBranchId(commit.Metadata); - if (branchId is Guid id && mergedInWindow.Contains(id) && - (oldest is null || commit.CompareKey.CompareTo(oldest.CompareKey) < 0)) + if (commit.Id == applyMinId || + (branchId is Guid id && mergedInWindow.Contains(id))) { - oldest = commit; + return allCommits.Skip(i).ToSortedSet(); } } - if (oldest is null) return commitsToApply; - - var parent = await repo.FindPreviousCommit(oldest); - return (await repo.GetCommitsAfter(parent)).ToSortedSet(); + return commitsToApply; } private static IEnumerable MergesIn(Commit commit) => diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 6312044..3ae14cc 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Options; using Nito.AsyncEx; using SIL.Harmony.Changes; +using SIL.Harmony.Entities; using SIL.Harmony.Resource; namespace SIL.Harmony.Db; @@ -247,6 +248,52 @@ public async Task GetCommitsAfter(Commit? commit) .ToArrayAsync(); } + /// + /// Distinct entity ids from ChangeEntities whose JSON change has the given polymorphic type name. + /// + public async Task GetEntityIdsForChangeType(string changeTypeName) + { + ArgumentException.ThrowIfNullOrWhiteSpace(changeTypeName); + // Discriminator key is "$type"; SQLite requires a quoted JSON path for the leading $. + var typePath = $"$.\"{CrdtConstants.ChangeDiscriminatorProperty}\""; + return await _dbContext.Database + .SqlQuery( + $""" + SELECT DISTINCT "EntityId" AS "Value" + FROM "ChangeEntities" + WHERE "Change" ->> {typePath} = {changeTypeName} + """) + .ToArrayAsync(); + } + + /// + /// Distinct entity ids from ChangeEntities for the given change type's . + /// + public Task GetEntityIdsForChangeType() where TChange : IPolyType => + GetEntityIdsForChangeType(TChange.TypeName); + + /// + /// Distinct entity ids from ChangeEntities for a change type that implements . + /// + public Task GetEntityIdsForChangeType(Type changeType) + { + ArgumentNullException.ThrowIfNull(changeType); + if (!typeof(IPolyType).IsAssignableFrom(changeType)) + { + throw new ArgumentException($"Change type {changeType} must implement {nameof(IPolyType)}.", nameof(changeType)); + } + + var typeName = changeType + .GetProperty(nameof(IPolyType.TypeName), BindingFlags.Public | BindingFlags.Static | BindingFlags.FlattenHierarchy) + ?.GetValue(null) as string; + if (string.IsNullOrWhiteSpace(typeName)) + { + throw new ArgumentException($"Change type {changeType} does not expose a static {nameof(IPolyType.TypeName)}.", nameof(changeType)); + } + + return GetEntityIdsForChangeType(typeName); + } + public async Task FindSnapshot(Guid id, bool tracking = false) { return await Snapshots From 996eb041783aafc475c14c55fd9f10708cb26328 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 13:30:48 +0700 Subject: [PATCH 10/18] Add tags with checkout, move, and roll-forward notifications. Issue 13: Tag entities point at commits; tag checkout rematerializes as-of the tip (including branch tips), rejects authoring by default, and rolls forward with CheckoutChanged on local move or RefsDataModel.SyncWith. Co-authored-by: Cursor --- .../Changes/CreateTagChange.cs | 23 ++++ src/SIL.Harmony.Refs/Changes/MoveTagChange.cs | 18 ++++ .../CheckoutMaterializationFilter.cs | 61 ++++++++++- src/SIL.Harmony.Refs/Entities/Tag.cs | 31 ++++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 6 +- src/SIL.Harmony.Refs/RefCheckout.cs | 15 ++- src/SIL.Harmony.Refs/RefsDataModel.cs | 100 +++++++++++++++++- src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj | 2 +- src/SIL.Harmony.Tests/CreateTagTests.cs | 46 ++++++++ src/SIL.Harmony.Tests/DataModelTestBase.cs | 4 + src/SIL.Harmony.Tests/TagCheckoutTests.cs | 95 +++++++++++++++++ .../TagSyncRollForwardTests.cs | 64 +++++++++++ src/SIL.Harmony/DataModel.cs | 6 ++ 13 files changed, 463 insertions(+), 8 deletions(-) create mode 100644 src/SIL.Harmony.Refs/Changes/CreateTagChange.cs create mode 100644 src/SIL.Harmony.Refs/Changes/MoveTagChange.cs create mode 100644 src/SIL.Harmony.Refs/Entities/Tag.cs create mode 100644 src/SIL.Harmony.Tests/CreateTagTests.cs create mode 100644 src/SIL.Harmony.Tests/TagCheckoutTests.cs create mode 100644 src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs diff --git a/src/SIL.Harmony.Refs/Changes/CreateTagChange.cs b/src/SIL.Harmony.Refs/Changes/CreateTagChange.cs new file mode 100644 index 0000000..479c752 --- /dev/null +++ b/src/SIL.Harmony.Refs/Changes/CreateTagChange.cs @@ -0,0 +1,23 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Entities; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs.Changes; + +public class CreateTagChange(Guid entityId, string name, Guid targetCommitId) : CreateChange(entityId), IPolyType +{ + public string Name { get; } = name; + public Guid TargetCommitId { get; } = targetCommitId; + + public override ValueTask NewEntity(Commit commit, IChangeContext context) + { + return ValueTask.FromResult(new Tag + { + Id = EntityId, + Name = Name, + TargetCommitId = TargetCommitId + }); + } + + public static string TypeName => "create:tag"; +} diff --git a/src/SIL.Harmony.Refs/Changes/MoveTagChange.cs b/src/SIL.Harmony.Refs/Changes/MoveTagChange.cs new file mode 100644 index 0000000..02a226b --- /dev/null +++ b/src/SIL.Harmony.Refs/Changes/MoveTagChange.cs @@ -0,0 +1,18 @@ +using SIL.Harmony.Changes; +using SIL.Harmony.Entities; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs.Changes; + +public class MoveTagChange(Guid entityId, Guid targetCommitId) : EditChange(entityId), IPolyType +{ + public Guid TargetCommitId { get; } = targetCommitId; + + public override ValueTask ApplyChange(Tag entity, IChangeContext context) + { + entity.TargetCommitId = TargetCommitId; + return ValueTask.CompletedTask; + } + + public static string TypeName => "move:tag"; +} diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs index 7e5d5cb..75b0d1a 100644 --- a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -7,22 +7,52 @@ namespace SIL.Harmony.Refs; /// /// Materializes commits visible for the current local checkout: /// main line (no branch id), commits for incorporated (merged) branches on main, -/// plus commits scoped to the checked-out branch when on a branch. +/// plus commits scoped to the checked-out branch when on a branch, +/// or commits at-or-before a tag tip with visibility evaluated at that tip. /// public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter, IMaterializationApplyWindow { private readonly HashSet _incorporatedBranchIds = []; + private (DateTimeOffset, long, Guid)? _asOfCompareKey; + private Guid? _tipBranchId; public RefCheckout Checkout { get; set; } = RefCheckout.Main; + public bool HasAsOfTip => _asOfCompareKey is not null; + + /// + /// When set (tag checkout), only commits at or before this compare key are included, + /// and merge incorporation is evaluated only through this tip. + /// + public void SetAsOfTip(Commit tip) + { + _asOfCompareKey = tip.CompareKey; + _tipBranchId = RefMetadata.GetBranchId(tip.Metadata); + } + + public void ClearAsOfTip() + { + _asOfCompareKey = null; + _tipBranchId = null; + } + public bool Include(Commit commit) { + // Keep the active tag entity materializable even when create/move commits are after the tip. + if (Checkout is TagCheckout tag && TouchesTag(commit, tag.TagId)) + return true; + + if (_asOfCompareKey is { } asOf && commit.CompareKey.CompareTo(asOf) > 0) + return false; + var branchId = RefMetadata.GetBranchId(commit.Metadata); if (branchId is null) return true; return Checkout switch { BranchCheckout branch => branchId == branch.BranchId || _incorporatedBranchIds.Contains(branchId.Value), + TagCheckout when _tipBranchId is Guid tipBranch => + branchId == tipBranch || _incorporatedBranchIds.Contains(branchId.Value), _ => _incorporatedBranchIds.Contains(branchId.Value) }; } @@ -31,8 +61,7 @@ async Task> IMaterializationApplyWindow.PrepareApplyWindowAsyn CrdtRepository repo, SortedSet commitsToApply) { - _incorporatedBranchIds.Clear(); - _incorporatedBranchIds.UnionWith(await repo.GetEntityIdsForChangeType()); + await RefreshIncorporatedBranchIds(repo); var mergedInWindow = commitsToApply .SelectMany(MergesIn) @@ -61,8 +90,34 @@ async Task> IMaterializationApplyWindow.PrepareApplyWindowAsyn return commitsToApply; } + private async Task RefreshIncorporatedBranchIds(CrdtRepository repo) + { + _incorporatedBranchIds.Clear(); + if (_asOfCompareKey is { } asOf) + { + // Only merges at or before the tip count for tag (as-of) visibility. + var commits = await repo.CurrentCommits() + .Include(c => c.ChangeEntities) + .ToListAsync(); + foreach (var commit in commits) + { + if (commit.CompareKey.CompareTo(asOf) > 0) break; + foreach (var merge in MergesIn(commit)) + _incorporatedBranchIds.Add(merge.EntityId); + } + } + else + { + _incorporatedBranchIds.UnionWith(await repo.GetEntityIdsForChangeType()); + } + } + private static IEnumerable MergesIn(Commit commit) => commit.ChangeEntities .Select(ce => ce.Change) .OfType(); + + private static bool TouchesTag(Commit commit, Guid tagId) => + commit.ChangeEntities.Any(ce => ce.EntityId == tagId && + (ce.Change is CreateTagChange or MoveTagChange)); } diff --git a/src/SIL.Harmony.Refs/Entities/Tag.cs b/src/SIL.Harmony.Refs/Entities/Tag.cs new file mode 100644 index 0000000..9ad2b45 --- /dev/null +++ b/src/SIL.Harmony.Refs/Entities/Tag.cs @@ -0,0 +1,31 @@ +using SIL.Harmony.Entities; + +namespace SIL.Harmony.Refs.Entities; + +/// +/// A named movable pointer to a commit. Display is not unique; identity is . +/// Polymorphic type name is harmony:tag to avoid colliding with app-domain tag types. +/// +public class Tag : IObjectBase +{ + static string IPolyType.TypeName => "harmony:tag"; + + public Guid Id { get; init; } + public required string Name { get; set; } + public Guid TargetCommitId { get; set; } + public DateTimeOffset? DeletedAt { get; set; } + + public Guid[] GetReferences() => []; + + public void RemoveReference(Guid id, CommitBase commit) + { + } + + public IObjectBase Copy() => new Tag + { + Id = Id, + Name = Name, + TargetCommitId = TargetCommitId, + DeletedAt = DeletedAt + }; +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index fa5402c..ce62b97 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -1,5 +1,6 @@ using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; namespace SIL.Harmony.Refs; @@ -7,14 +8,17 @@ namespace SIL.Harmony.Refs; public static class HarmonyRefsKernel { /// - /// Registers branch ref entities and change types. Sets a main-line-only materialization filter + /// Registers branch/tag ref entities and change types. Sets a main-line-only materialization filter /// as a fallback when is not used. /// public static CrdtConfig AddHarmonyRefs(this CrdtConfig config) { config.ObjectTypeListBuilder.DefaultAdapter().Add(); + config.ObjectTypeListBuilder.DefaultAdapter().Add(builder => builder.ToTable("HarmonyTags")); config.ChangeTypeListBuilder.Add(); config.ChangeTypeListBuilder.Add(); + config.ChangeTypeListBuilder.Add(); + config.ChangeTypeListBuilder.Add(); config.CommitMaterializationFilter = MainLineOnlyMaterializationFilter.Instance; return config; } diff --git a/src/SIL.Harmony.Refs/RefCheckout.cs b/src/SIL.Harmony.Refs/RefCheckout.cs index 6348e52..9e3a58f 100644 --- a/src/SIL.Harmony.Refs/RefCheckout.cs +++ b/src/SIL.Harmony.Refs/RefCheckout.cs @@ -1,15 +1,28 @@ namespace SIL.Harmony.Refs; /// -/// Local checkout selection. Not synced; only affects authoring defaults and (later) materialization views. +/// Local checkout selection. Not synced; affects authoring defaults and materialization views. /// public abstract record RefCheckout { public static RefCheckout Main { get; } = new MainCheckout(); public static RefCheckout ForBranch(Guid branchId) => new BranchCheckout(branchId); + + public static RefCheckout ForTag(Guid tagId) => new TagCheckout(tagId); } public sealed record MainCheckout : RefCheckout; public sealed record BranchCheckout(Guid BranchId) : RefCheckout; + +public sealed record TagCheckout(Guid TagId) : RefCheckout; + +/// +/// Raised when the active checkout's tip advances (e.g. tag move / roll-forward after sync). +/// +public sealed class RefCheckoutChangedEventArgs(RefCheckout checkout, Guid tipCommitId) : EventArgs +{ + public RefCheckout Checkout { get; } = checkout; + public Guid TipCommitId { get; } = tipCommitId; +} diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 12f4b1c..17a4369 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -1,5 +1,6 @@ using SIL.Harmony.Changes; using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Refs.Entities; namespace SIL.Harmony.Refs; @@ -14,20 +15,55 @@ public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter fi public RefCheckout Checkout => filter.Checkout; + /// + /// When true, authoring on a tag checkout writes to main. Default is false (rejected). + /// + public bool AllowAuthoringOnTagToMain { get; set; } + + /// + /// Fired when the active checkout tip advances (tag move / roll-forward). + /// + public event EventHandler? CheckoutChanged; + public async Task CheckoutMain() { - if (Checkout is MainCheckout) return; + if (Checkout is MainCheckout && !filter.HasAsOfTip) return; + filter.ClearAsOfTip(); filter.Checkout = RefCheckout.Main; await DataModel.RegenerateSnapshots(); } public async Task CheckoutBranch(Guid branchId) { - if (Checkout is BranchCheckout current && current.BranchId == branchId) return; + if (Checkout is BranchCheckout current && current.BranchId == branchId && !filter.HasAsOfTip) + return; + + filter.ClearAsOfTip(); filter.Checkout = RefCheckout.ForBranch(branchId); await DataModel.RegenerateSnapshots(); } + public async Task CheckoutTag(Guid tagId) + { + var tip = await ResolveTagTip(tagId); + filter.Checkout = RefCheckout.ForTag(tagId); + filter.SetAsOfTip(tip); + await DataModel.RegenerateSnapshots(); + } + + public Task CreateTag(Guid clientId, Guid tagId, string name, Guid targetCommitId) => + DataModel.AddChange(clientId, new CreateTagChange(tagId, name, targetCommitId), + ApplyAssignment(null, BranchAssignment.Main)); + + public async Task MoveTag(Guid clientId, Guid tagId, Guid targetCommitId) + { + var commit = await DataModel.AddChange(clientId, new MoveTagChange(tagId, targetCommitId), + ApplyAssignment(null, BranchAssignment.Main)); + if (Checkout is TagCheckout tag && tag.TagId == tagId) + await RollForwardActiveTag(tagId); + return commit; + } + /// /// Incorporates into main visibility and deletes the branch entity. /// Authored as a main-line commit; materialization expands from the earliest branch commit. @@ -38,12 +74,46 @@ public async Task MergeBranch(Guid clientId, Guid branchId) return await AddChange(clientId, new MergeBranchChange(branchId), BranchAssignment.Main); } + /// + /// Syncs via then rolls forward an active tag checkout if the tip moved. + /// + public async Task SyncWith(ISyncable remote) + { + Guid? tipBefore = null; + if (Checkout is TagCheckout active) + { + try { tipBefore = (await ResolveTagTip(active.TagId)).Id; } + catch (InvalidOperationException) { /* tag not projected yet */ } + } + + var results = await DataModel.SyncWith(remote); + + if (Checkout is TagCheckout checkedOut) + { + var tipAfter = await ResolveTagTip(checkedOut.TagId); + if (tipBefore != tipAfter.Id) + await RollForwardActiveTag(checkedOut.TagId); + } + + return results; + } + + /// + /// After sync (or external apply), refresh tag checkout if the tip moved. + /// + public async Task RefreshCheckoutAfterSync() + { + if (Checkout is not TagCheckout tag) return; + await RollForwardActiveTag(tag.TagId); + } + public Task AddChange( Guid clientId, IChange change, BranchAssignment assignment = default, CommitMetadata? commitMetadata = null) { + EnsureCanAuthor(); if (assignment == default) assignment = BranchAssignment.FromCheckout; return DataModel.AddChange(clientId, change, ApplyAssignment(commitMetadata, assignment)); } @@ -54,10 +124,35 @@ public Task AddChanges( BranchAssignment assignment = default, CommitMetadata? commitMetadata = null) { + EnsureCanAuthor(); if (assignment == default) assignment = BranchAssignment.FromCheckout; return DataModel.AddChanges(clientId, changes, ApplyAssignment(commitMetadata, assignment)); } + private void EnsureCanAuthor() + { + if (Checkout is not TagCheckout) return; + if (AllowAuthoringOnTagToMain) return; + throw new InvalidOperationException( + "Authoring is not allowed while checked out on a tag. Set AllowAuthoringOnTagToMain to write to main, or checkout main/branch first."); + } + + private async Task RollForwardActiveTag(Guid tagId) + { + var tip = await ResolveTagTip(tagId); + filter.Checkout = RefCheckout.ForTag(tagId); + filter.SetAsOfTip(tip); + await DataModel.RegenerateSnapshots(); + CheckoutChanged?.Invoke(this, new RefCheckoutChangedEventArgs(filter.Checkout, tip.Id)); + } + + private async Task ResolveTagTip(Guid tagId) + { + var tag = await DataModel.GetLatest(tagId) + ?? throw new InvalidOperationException($"Tag {tagId} was not found."); + return await DataModel.GetCommit(tag.TargetCommitId); + } + private CommitMetadata ApplyAssignment(CommitMetadata? commitMetadata, BranchAssignment assignment) { var metadata = commitMetadata ?? new CommitMetadata(); @@ -73,6 +168,7 @@ private CommitMetadata ApplyAssignment(CommitMetadata? commitMetadata, BranchAss BranchAssignmentKind.FromCheckout => Checkout switch { BranchCheckout branch => branch.BranchId, + TagCheckout when AllowAuthoringOnTagToMain => null, _ => null }, _ => null diff --git a/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj index 61b5972..1bebbfd 100644 --- a/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj +++ b/src/SIL.Harmony.Refs/SIL.Harmony.Refs.csproj @@ -3,7 +3,7 @@ SIL.Harmony.Refs true - Optional branches layer for SIL.Harmony. + Optional branches and tags layer for SIL.Harmony. diff --git a/src/SIL.Harmony.Tests/CreateTagTests.cs b/src/SIL.Harmony.Tests/CreateTagTests.cs new file mode 100644 index 0000000..2de32b9 --- /dev/null +++ b/src/SIL.Harmony.Tests/CreateTagTests.cs @@ -0,0 +1,46 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Refs.Entities; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class CreateTagTests : DataModelTestBase +{ + public CreateTagTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + }) + { + } + + [Fact] + public async Task CanCreateAndReadTag() + { + var wordId = Guid.NewGuid(); + var tip = await DataModel.AddChange(_localClientId, SetWord(wordId, "at-tip")); + var tagId = Guid.NewGuid(); + + await DataModel.AddChange(_localClientId, new CreateTagChange(tagId, "release", tip.Id)); + + var tag = await DataModel.GetLatest(tagId); + tag.Should().NotBeNull(); + tag!.Id.Should().Be(tagId); + tag.Name.Should().Be("release"); + tag.TargetCommitId.Should().Be(tip.Id); + } + + [Fact] + public async Task DuplicateTagNamesAreAllowed() + { + var tip = await DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "x")); + var firstId = Guid.NewGuid(); + var secondId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateTagChange(firstId, "dup", tip.Id)); + await DataModel.AddChange(_localClientId, new CreateTagChange(secondId, "dup", tip.Id)); + + var both = DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken).ToArray(); + both.Should().HaveCount(2); + both.Select(t => t.Name).Should().AllBe("dup"); + } +} diff --git a/src/SIL.Harmony.Tests/DataModelTestBase.cs b/src/SIL.Harmony.Tests/DataModelTestBase.cs index 80402eb..57b73aa 100644 --- a/src/SIL.Harmony.Tests/DataModelTestBase.cs +++ b/src/SIL.Harmony.Tests/DataModelTestBase.cs @@ -20,6 +20,10 @@ public class DataModelTestBase : IAsyncLifetime public readonly SampleDbContext DbContext; protected readonly MockTimeProvider MockTimeProvider = new(); + public Guid LocalClientId => _localClientId; + + public T GetRequiredService() where T : notnull => _services.GetRequiredService(); + public DataModelTestBase(bool saveToDisk = false, bool alwaysValidate = true, Action? configure = null, bool performanceTest = false) : this(saveToDisk ? new SqliteConnection("Data Source=test.db") diff --git a/src/SIL.Harmony.Tests/TagCheckoutTests.cs b/src/SIL.Harmony.Tests/TagCheckoutTests.cs new file mode 100644 index 0000000..7e0e9c3 --- /dev/null +++ b/src/SIL.Harmony.Tests/TagCheckoutTests.cs @@ -0,0 +1,95 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Sample.Models; +using Microsoft.Extensions.DependencyInjection; +using RefTag = SIL.Harmony.Refs.Entities.Tag; + +namespace SIL.Harmony.Tests; + +public class TagCheckoutTests : DataModelTestBase +{ + private readonly RefsDataModel _refs; + + public TagCheckoutTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }) + { + _refs = _services.GetRequiredService(); + } + + [Fact] + public async Task TagCheckoutShowsStateAsOfTip() + { + var wordId = Guid.NewGuid(); + var tip = await DataModel.AddChange(_localClientId, SetWord(wordId, "at-tag")); + var tagId = Guid.NewGuid(); + await _refs.CreateTag(_localClientId, tagId, "v1", tip.Id); + await DataModel.AddChange(_localClientId, SetWord(wordId, "after-tag")); + + (await DataModel.GetLatest(wordId))!.Text.Should().Be("after-tag"); + + await _refs.CheckoutTag(tagId); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("at-tag"); + + await _refs.CheckoutMain(); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("after-tag"); + } + + [Fact] + public async Task AuthoringOnTagCheckoutIsRejectedByDefault() + { + var tip = await DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "x")); + var tagId = Guid.NewGuid(); + await _refs.CreateTag(_localClientId, tagId, "v1", tip.Id); + await _refs.CheckoutTag(tagId); + + var act = () => _refs.AddChange(_localClientId, SetWord(Guid.NewGuid(), "nope")); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task MoveTagWhileCheckedOutRematerializesAndNotifies() + { + var wordId = Guid.NewGuid(); + var first = await DataModel.AddChange(_localClientId, SetWord(wordId, "first")); + var second = await DataModel.AddChange(_localClientId, SetWord(wordId, "second")); + var tagId = Guid.NewGuid(); + await _refs.CreateTag(_localClientId, tagId, "moving", first.Id); + + RefCheckoutChangedEventArgs? notified = null; + _refs.CheckoutChanged += (_, args) => notified = args; + + await _refs.CheckoutTag(tagId); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + await _refs.MoveTag(_localClientId, tagId, second.Id); + + (await DataModel.GetLatest(wordId))!.Text.Should().Be("second"); + (await DataModel.GetLatest(tagId))!.TargetCommitId.Should().Be(second.Id); + notified.Should().NotBeNull(); + notified!.Checkout.Should().BeOfType().Which.TagId.Should().Be(tagId); + notified.TipCommitId.Should().Be(second.Id); + } + + [Fact] + public async Task TagOnBranchTipShowsBranchViewAsOfTip() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.AddChange(_localClientId, SetWord(wordId, "main"), BranchAssignment.Main); + + await _refs.CheckoutBranch(branchId); + var branchTip = await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + + var tagId = Guid.NewGuid(); + await _refs.CheckoutMain(); + await _refs.CreateTag(_localClientId, tagId, "wip", branchTip.Id); + await _refs.AddChange(_localClientId, SetWord(wordId, "later-main"), BranchAssignment.Main); + + await _refs.CheckoutTag(tagId); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-branch"); + } +} diff --git a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs new file mode 100644 index 0000000..0f5450c --- /dev/null +++ b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs @@ -0,0 +1,64 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Sample.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +public class TagSyncRollForwardTests : IAsyncLifetime +{ + private readonly DataModelTestBase _client1; + private readonly DataModelTestBase _client2; + private readonly RefsDataModel _refs1; + private readonly RefsDataModel _refs2; + + public TagSyncRollForwardTests() + { + static void Configure(IServiceCollection services) + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + } + + _client1 = new DataModelTestBase(configure: Configure); + _client2 = new DataModelTestBase(configure: Configure); + _refs1 = _client1.GetRequiredService(); + _refs2 = _client2.GetRequiredService(); + } + + public async ValueTask InitializeAsync() + { + await _client1.InitializeAsync(); + await _client2.InitializeAsync(); + } + + public async ValueTask DisposeAsync() + { + await _client1.DisposeAsync(); + await _client2.DisposeAsync(); + } + + [Fact] + public async Task SyncMoveTagRollsForwardActiveCheckout() + { + var wordId = Guid.NewGuid(); + var first = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "first")); + var second = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "second")); + var tagId = Guid.NewGuid(); + await _refs1.CreateTag(_client1.LocalClientId, tagId, "release", first.Id); + + await _refs1.SyncWith(_client2.DataModel); + + await _refs2.CheckoutTag(tagId); + (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + RefCheckoutChangedEventArgs? notified = null; + _refs2.CheckoutChanged += (_, args) => notified = args; + + await _refs1.MoveTag(_client1.LocalClientId, tagId, second.Id); + await _refs2.SyncWith(_client1.DataModel); + + (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); + notified.Should().NotBeNull(); + notified!.TipCommitId.Should().Be(second.Id); + } +} diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index d8f766d..b4d4b99 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -245,6 +245,12 @@ private async Task ValidateCommits(CrdtRepository repo) } } + public async Task GetCommit(Guid commitId) + { + await using var repo = await _crdtRepositoryFactory.CreateRepository(); + return await repo.CurrentCommits().AsNoTracking().SingleAsync(c => c.Id == commitId); + } + public async Task RegenerateSnapshots() { await using var repo = await _crdtRepositoryFactory.CreateRepository(); From c8b3f985758386c4b1b6d9307e49e96046765950 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 14:01:56 +0700 Subject: [PATCH 11/18] Scope change-type entity ids to the tip and simplify tag checkout config Scope GetEntityIdsForChangeType to the repository's ignoreChangesAfter tip by joining ChangeEntities to Commits with the same at-or-before-tip bound ScopedDbContext applies (one query, null-tip guard keeps unscoped behaviour). With that, CheckoutMaterializationFilter drops its per-commit as-of loop and _tipBranchId: it holds a single _asOfTip commit and derives incorporated branches via GetScopedRepository(tip).GetEntityIdsForChangeType. Move AllowAuthoringOnTagToMain onto CrdtConfig; RefsDataModel now reads it from options. Drop the CheckoutChanged event and RefCheckoutChangedEventArgs. Co-Authored-By: Claude Opus 4.8 --- .../CheckoutMaterializationFilter.cs | 44 +++++-------------- src/SIL.Harmony.Refs/RefCheckout.cs | 9 ---- src/SIL.Harmony.Refs/RefsDataModel.cs | 12 ++--- src/SIL.Harmony.Tests/RepositoryTests.cs | 42 ++++++++++++++++++ src/SIL.Harmony.Tests/TagCheckoutTests.cs | 38 +++++++++++++--- .../TagSyncRollForwardTests.cs | 5 --- src/SIL.Harmony/CrdtConfig.cs | 5 +++ src/SIL.Harmony/Db/CrdtRepository.cs | 20 +++++++-- 8 files changed, 110 insertions(+), 65 deletions(-) diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs index 75b0d1a..ea85166 100644 --- a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -13,28 +13,19 @@ namespace SIL.Harmony.Refs; public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter, IMaterializationApplyWindow { private readonly HashSet _incorporatedBranchIds = []; - private (DateTimeOffset, long, Guid)? _asOfCompareKey; - private Guid? _tipBranchId; + private Commit? _asOfTip; public RefCheckout Checkout { get; set; } = RefCheckout.Main; - public bool HasAsOfTip => _asOfCompareKey is not null; + public bool HasAsOfTip => _asOfTip is not null; /// - /// When set (tag checkout), only commits at or before this compare key are included, + /// When set (tag checkout), only commits at or before this tip are included, /// and merge incorporation is evaluated only through this tip. /// - public void SetAsOfTip(Commit tip) - { - _asOfCompareKey = tip.CompareKey; - _tipBranchId = RefMetadata.GetBranchId(tip.Metadata); - } + public void SetAsOfTip(Commit tip) => _asOfTip = tip; - public void ClearAsOfTip() - { - _asOfCompareKey = null; - _tipBranchId = null; - } + public void ClearAsOfTip() => _asOfTip = null; public bool Include(Commit commit) { @@ -42,7 +33,7 @@ public bool Include(Commit commit) if (Checkout is TagCheckout tag && TouchesTag(commit, tag.TagId)) return true; - if (_asOfCompareKey is { } asOf && commit.CompareKey.CompareTo(asOf) > 0) + if (_asOfTip is { } tip && commit.CompareKey.CompareTo(tip.CompareKey) > 0) return false; var branchId = RefMetadata.GetBranchId(commit.Metadata); @@ -51,7 +42,7 @@ public bool Include(Commit commit) return Checkout switch { BranchCheckout branch => branchId == branch.BranchId || _incorporatedBranchIds.Contains(branchId.Value), - TagCheckout when _tipBranchId is Guid tipBranch => + TagCheckout when _asOfTip is { } asOfTip && RefMetadata.GetBranchId(asOfTip.Metadata) is Guid tipBranch => branchId == tipBranch || _incorporatedBranchIds.Contains(branchId.Value), _ => _incorporatedBranchIds.Contains(branchId.Value) }; @@ -93,23 +84,10 @@ async Task> IMaterializationApplyWindow.PrepareApplyWindowAsyn private async Task RefreshIncorporatedBranchIds(CrdtRepository repo) { _incorporatedBranchIds.Clear(); - if (_asOfCompareKey is { } asOf) - { - // Only merges at or before the tip count for tag (as-of) visibility. - var commits = await repo.CurrentCommits() - .Include(c => c.ChangeEntities) - .ToListAsync(); - foreach (var commit in commits) - { - if (commit.CompareKey.CompareTo(asOf) > 0) break; - foreach (var merge in MergesIn(commit)) - _incorporatedBranchIds.Add(merge.EntityId); - } - } - else - { - _incorporatedBranchIds.UnionWith(await repo.GetEntityIdsForChangeType()); - } + // Scoping to the tip bounds GetEntityIdsForChangeType to merges at or before it (see issue 15), + // so a single query serves both the tag (as-of) and unscoped cases. + if (_asOfTip is { } tip) repo = repo.GetScopedRepository(tip); + _incorporatedBranchIds.UnionWith(await repo.GetEntityIdsForChangeType()); } private static IEnumerable MergesIn(Commit commit) => diff --git a/src/SIL.Harmony.Refs/RefCheckout.cs b/src/SIL.Harmony.Refs/RefCheckout.cs index 9e3a58f..9b1a93b 100644 --- a/src/SIL.Harmony.Refs/RefCheckout.cs +++ b/src/SIL.Harmony.Refs/RefCheckout.cs @@ -17,12 +17,3 @@ public sealed record MainCheckout : RefCheckout; public sealed record BranchCheckout(Guid BranchId) : RefCheckout; public sealed record TagCheckout(Guid TagId) : RefCheckout; - -/// -/// Raised when the active checkout's tip advances (e.g. tag move / roll-forward after sync). -/// -public sealed class RefCheckoutChangedEventArgs(RefCheckout checkout, Guid tipCommitId) : EventArgs -{ - public RefCheckout Checkout { get; } = checkout; - public Guid TipCommitId { get; } = tipCommitId; -} diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 17a4369..70a53a0 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Options; using SIL.Harmony.Changes; using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; @@ -9,7 +10,7 @@ namespace SIL.Harmony.Refs; /// Checkout lives on (single source of truth); /// changing it rematerializes snapshots for the new view. /// -public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter filter) +public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter filter, IOptions config) { public DataModel DataModel { get; } = dataModel; @@ -17,13 +18,9 @@ public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter fi /// /// When true, authoring on a tag checkout writes to main. Default is false (rejected). + /// Configured via . /// - public bool AllowAuthoringOnTagToMain { get; set; } - - /// - /// Fired when the active checkout tip advances (tag move / roll-forward). - /// - public event EventHandler? CheckoutChanged; + public bool AllowAuthoringOnTagToMain => config.Value.AllowAuthoringOnTagToMain; public async Task CheckoutMain() { @@ -143,7 +140,6 @@ private async Task RollForwardActiveTag(Guid tagId) filter.Checkout = RefCheckout.ForTag(tagId); filter.SetAsOfTip(tip); await DataModel.RegenerateSnapshots(); - CheckoutChanged?.Invoke(this, new RefCheckoutChangedEventArgs(filter.Checkout, tip.Id)); } private async Task ResolveTagTip(Guid tagId) diff --git a/src/SIL.Harmony.Tests/RepositoryTests.cs b/src/SIL.Harmony.Tests/RepositoryTests.cs index 3a697ea..4d50832 100644 --- a/src/SIL.Harmony.Tests/RepositoryTests.cs +++ b/src/SIL.Harmony.Tests/RepositoryTests.cs @@ -242,6 +242,48 @@ await _repository.AddSnapshots([ commit.Id.Should().Be(commitIds[1], $"commit order: [{string.Join(", ", commitIds)}]"); } + [Fact] + public async Task GetEntityIdsForChangeType_ReturnsAllMatchingChangesWhenUnscoped() + { + var commit1 = Commit(Guid.NewGuid(), Time(1, 0)); + var commit2 = Commit(Guid.NewGuid(), Time(2, 0)); + var commit3 = Commit(Guid.NewGuid(), Time(3, 0)); + await _repository.AddCommits([commit1, commit2, commit3]); + + var ids = await _repository.GetEntityIdsForChangeType(); + + ids.Should().BeEquivalentTo(new[] + { + commit1.ChangeEntities[0].EntityId, + commit2.ChangeEntities[0].EntityId, + commit3.ChangeEntities[0].EntityId, + }); + } + + [Fact] + public async Task GetEntityIdsForChangeType_ScopedRepoIgnoresChangesAfterTip() + { + var commit1 = Commit(Guid.NewGuid(), Time(1, 0)); + var commit2 = Commit(Guid.NewGuid(), Time(2, 0)); + var commit3 = Commit(Guid.NewGuid(), Time(3, 0)); + await _repository.AddCommits([commit1, commit2, commit3]); + + var scopedIds = await _repository.GetScopedRepository(commit2) + .GetEntityIdsForChangeType(); + + // commit2 is the tip: at-or-before is included (inclusive), the later commit3 change is not. + scopedIds.Should().BeEquivalentTo(new[] + { + commit1.ChangeEntities[0].EntityId, + commit2.ChangeEntities[0].EntityId, + }); + scopedIds.Should().NotContain(commit3.ChangeEntities[0].EntityId); + + // the unscoped repo still sees the post-tip change. + var allIds = await _repository.GetEntityIdsForChangeType(); + allIds.Should().Contain(commit3.ChangeEntities[0].EntityId); + } + [Fact] public async Task DeleteStaleSnapshots_Works() { diff --git a/src/SIL.Harmony.Tests/TagCheckoutTests.cs b/src/SIL.Harmony.Tests/TagCheckoutTests.cs index 7e0e9c3..a45450e 100644 --- a/src/SIL.Harmony.Tests/TagCheckoutTests.cs +++ b/src/SIL.Harmony.Tests/TagCheckoutTests.cs @@ -50,7 +50,7 @@ public async Task AuthoringOnTagCheckoutIsRejectedByDefault() } [Fact] - public async Task MoveTagWhileCheckedOutRematerializesAndNotifies() + public async Task MoveTagWhileCheckedOutRematerializes() { var wordId = Guid.NewGuid(); var first = await DataModel.AddChange(_localClientId, SetWord(wordId, "first")); @@ -58,9 +58,6 @@ public async Task MoveTagWhileCheckedOutRematerializesAndNotifies() var tagId = Guid.NewGuid(); await _refs.CreateTag(_localClientId, tagId, "moving", first.Id); - RefCheckoutChangedEventArgs? notified = null; - _refs.CheckoutChanged += (_, args) => notified = args; - await _refs.CheckoutTag(tagId); (await DataModel.GetLatest(wordId))!.Text.Should().Be("first"); @@ -68,9 +65,36 @@ public async Task MoveTagWhileCheckedOutRematerializesAndNotifies() (await DataModel.GetLatest(wordId))!.Text.Should().Be("second"); (await DataModel.GetLatest(tagId))!.TargetCommitId.Should().Be(second.Id); - notified.Should().NotBeNull(); - notified!.Checkout.Should().BeOfType().Which.TagId.Should().Be(tagId); - notified.TipCommitId.Should().Be(second.Id); + _refs.Checkout.Should().BeOfType().Which.TagId.Should().Be(tagId); + } + + [Fact] + public async Task AuthoringOnTagCheckoutWritesToMainWhenConfigured() + { + // Fresh model with AllowAuthoringOnTagToMain enabled via CrdtConfig. + await using var withConfig = new DataModelTestBase(configure: services => + { + services.Configure(config => + { + config.AddHarmonyRefs(); + config.AllowAuthoringOnTagToMain = true; + }); + services.AddHarmonyRefsDataModel(); + }); + var refs = withConfig.GetRequiredService(); + refs.AllowAuthoringOnTagToMain.Should().BeTrue(); + + var tip = await withConfig.DataModel.AddChange(withConfig.LocalClientId, withConfig.SetWord(Guid.NewGuid(), "at-tag")); + var tagId = Guid.NewGuid(); + await refs.CreateTag(withConfig.LocalClientId, tagId, "v1", tip.Id); + await refs.CheckoutTag(tagId); + + var newWordId = Guid.NewGuid(); + await refs.AddChange(withConfig.LocalClientId, withConfig.SetWord(newWordId, "authored-to-main")); + + // The change was authored to main (no branch id), so it is visible on the main checkout. + await refs.CheckoutMain(); + (await withConfig.DataModel.GetLatest(newWordId))!.Text.Should().Be("authored-to-main"); } [Fact] diff --git a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs index 0f5450c..8a69c32 100644 --- a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs +++ b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs @@ -51,14 +51,9 @@ public async Task SyncMoveTagRollsForwardActiveCheckout() await _refs2.CheckoutTag(tagId); (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); - RefCheckoutChangedEventArgs? notified = null; - _refs2.CheckoutChanged += (_, args) => notified = args; - await _refs1.MoveTag(_client1.LocalClientId, tagId, second.Id); await _refs2.SyncWith(_client1.DataModel); (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); - notified.Should().NotBeNull(); - notified!.TipCommitId.Should().Be(second.Id); } } diff --git a/src/SIL.Harmony/CrdtConfig.cs b/src/SIL.Harmony/CrdtConfig.cs index 851ac98..dc7ae2d 100644 --- a/src/SIL.Harmony/CrdtConfig.cs +++ b/src/SIL.Harmony/CrdtConfig.cs @@ -27,6 +27,11 @@ public class CrdtConfig /// public ICommitMaterializationFilter CommitMaterializationFilter { get; set; } = IncludeAllCommitsFilter.Instance; + /// + /// When true, authoring while checked out on a tag writes to main instead of being rejected. + /// Only relevant when using SIL.Harmony.Refs (RefsDataModel). Default false. + /// + public bool AllowAuthoringOnTagToMain { get; set; } public ChangeTypeListBuilder ChangeTypeListBuilder { get; } = new(); public IEnumerable ChangeTypes => ChangeTypeListBuilder.Types.Select(t => t.DerivedType); public ObjectTypeListBuilder ObjectTypeListBuilder { get; } = new(); diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 3ae14cc..2ebba7c 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -52,12 +52,14 @@ internal class CrdtRepository : IDisposable, IAsyncDisposable private readonly ICrdtDbContext _dbContext; private readonly IOptions _crdtConfig; private readonly ILogger _logger; + private readonly Commit? _ignoreChangesAfter; public CrdtRepository(ICrdtDbContext dbContext, IOptions crdtConfig, ILogger logger, Commit? ignoreChangesAfter = null) { _crdtConfig = crdtConfig; + _ignoreChangesAfter = ignoreChangesAfter; _dbContext = ignoreChangesAfter is not null ? new ScopedDbContext(dbContext, ignoreChangesAfter) : dbContext; _logger = logger; //we can't use the scoped db context is it prevents access to the DbSet for the Snapshots, @@ -256,12 +258,24 @@ public async Task GetEntityIdsForChangeType(string changeTypeName) ArgumentException.ThrowIfNullOrWhiteSpace(changeTypeName); // Discriminator key is "$type"; SQLite requires a quoted JSON path for the leading $. var typePath = $"$.\"{CrdtConstants.ChangeDiscriminatorProperty}\""; + // When scoped (as-of / tag tip), only count changes on commits at or before the tip, matching the + // before/at-tip ordering ScopedDbContext applies to Commits/Snapshots (WhereBefore inclusive). + // A null tip disables the bound, so this one query serves both scoped and unscoped repos. + var ignoreAfterDate = _ignoreChangesAfter?.HybridDateTime.DateTime.UtcDateTime; + var ignoreAfterCounter = _ignoreChangesAfter?.HybridDateTime.Counter; + var ignoreAfterCommitId = _ignoreChangesAfter?.Id; return await _dbContext.Database .SqlQuery( $""" - SELECT DISTINCT "EntityId" AS "Value" - FROM "ChangeEntities" - WHERE "Change" ->> {typePath} = {changeTypeName} + SELECT DISTINCT "ce"."EntityId" AS "Value" + FROM "ChangeEntities" AS "ce" + INNER JOIN "Commits" AS "c" ON "ce"."CommitId" = "c"."Id" + WHERE "ce"."Change" ->> {typePath} = {changeTypeName} + AND ({ignoreAfterDate} IS NULL + OR "c"."DateTime" < {ignoreAfterDate} + OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" < {ignoreAfterCounter}) + OR ("c"."DateTime" = {ignoreAfterDate} AND "c"."Counter" = {ignoreAfterCounter} AND "c"."Id" < {ignoreAfterCommitId}) + OR "c"."Id" = {ignoreAfterCommitId}) """) .ToArrayAsync(); } From 466940ae33c0a61337543fd22d14ab04e3cbffb1 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 14:59:46 +0700 Subject: [PATCH 12/18] Add commit-author interceptor for transparent branch assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a core, refs-agnostic ICommitInterceptor invoked once per locally-authored commit at DataModel.NewCommit (the single author choke point; sync-applied commits bypass it). DataModel resolves it optionally from DI, mirroring ICommitMaterializationFilter. Refs registers CheckoutCommitInterceptor, which stamps the current checkout's branch id or rejects tag authoring — so clients can author via DataModel.AddChange directly without the RefsDataModel wrapper. A public RefMetadata.SetAssignment helper + harmony.branchAssigned marker carries an explicit per-call override that the interceptor honours; RefsDataModel's own assignment path now sets the marker so its commits skip re-derivation. Implements ticket 04 of refs-transparent-authoring. Co-Authored-By: Claude Opus 4.8 --- .../CheckoutCommitInterceptor.cs | 31 ++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 1 + src/SIL.Harmony.Refs/RefMetadata.cs | 40 +++++ src/SIL.Harmony.Refs/RefsDataModel.cs | 5 +- .../TransparentAuthoringTests.cs | 161 ++++++++++++++++++ src/SIL.Harmony/CrdtKernel.cs | 3 +- src/SIL.Harmony/DataModel.cs | 10 +- src/SIL.Harmony/ICommitInterceptor.cs | 11 ++ 8 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs create mode 100644 src/SIL.Harmony.Tests/TransparentAuthoringTests.cs create mode 100644 src/SIL.Harmony/ICommitInterceptor.cs diff --git a/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs b/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs new file mode 100644 index 0000000..1813365 --- /dev/null +++ b/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs @@ -0,0 +1,31 @@ +using Microsoft.Extensions.Options; + +namespace SIL.Harmony.Refs; + +/// +/// Applies the current checkout's branch assignment to locally-authored commits so that +/// clients can author through directly without routing +/// through . Commits that already carry an explicit assignment +/// (see ) are left untouched. +/// +public sealed class CheckoutCommitInterceptor(CheckoutMaterializationFilter filter, IOptions config) + : ICommitInterceptor +{ + public void OnCommitAuthored(Commit commit) + { + // Explicit per-call override or a ref-lifecycle assignment already decided this commit. + if (RefMetadata.IsAssigned(commit.Metadata)) return; + + switch (filter.Checkout) + { + case BranchCheckout branch: + RefMetadata.SetBranchId(commit.Metadata, branch.BranchId); + break; + case TagCheckout when !config.Value.AllowAuthoringOnTagToMain: + throw new InvalidOperationException( + "Authoring is not allowed while checked out on a tag. Set AllowAuthoringOnTagToMain to write to main, " + + "author with an explicit BranchAssignment, or checkout main/branch first."); + // MainCheckout, or a tag checkout with AllowAuthoringOnTagToMain: author to main (no branch id). + } + } +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index ce62b97..2cf6ee0 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -32,6 +32,7 @@ public static IServiceCollection AddHarmonyRefsDataModel(this IServiceCollection services.AddScoped(); services.AddScoped(sp => sp.GetRequiredService()); + services.AddScoped(); services.AddScoped(); return services; } diff --git a/src/SIL.Harmony.Refs/RefMetadata.cs b/src/SIL.Harmony.Refs/RefMetadata.cs index b0e086f..7afde45 100644 --- a/src/SIL.Harmony.Refs/RefMetadata.cs +++ b/src/SIL.Harmony.Refs/RefMetadata.cs @@ -10,6 +10,14 @@ public static class RefMetadata /// public const string BranchIdKey = "harmony.branchId"; + /// + /// Marks that a commit's branch assignment was set deliberately (including to main). + /// Distinguishes an explicit assignment from an unassigned commit — both of which + /// have no when the assignment is main — so the authoring + /// interceptor knows to leave the commit alone rather than derive from the checkout. + /// + public const string BranchAssignedKey = "harmony.branchAssigned"; + public static Guid? GetBranchId(CommitMetadata metadata) { var value = metadata[BranchIdKey]; @@ -26,4 +34,36 @@ public static void SetBranchId(CommitMetadata metadata, Guid? branchId) metadata[BranchIdKey] = branchId.Value.ToString(); } + + /// + /// True when the commit carries a deliberate branch assignment (see ). + /// + public static bool IsAssigned(CommitMetadata metadata) => metadata[BranchAssignedKey] is not null; + + /// + /// Records an explicit per-call branch assignment on so it takes + /// precedence over the current checkout when authoring through . + /// and mark the commit; + /// leaves it unmarked so the checkout is used. + /// Returns the same metadata for fluent use. + /// + public static CommitMetadata SetAssignment(CommitMetadata metadata, BranchAssignment assignment) + { + switch (assignment.Kind) + { + case BranchAssignmentKind.Main: + SetBranchId(metadata, null); + metadata[BranchAssignedKey] = bool.TrueString; + break; + case BranchAssignmentKind.Branch: + SetBranchId(metadata, assignment.BranchId); + metadata[BranchAssignedKey] = bool.TrueString; + break; + case BranchAssignmentKind.FromCheckout: + // Leave unmarked: the interceptor derives the branch id from the checkout. + break; + } + + return metadata; + } } diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 70a53a0..f2f8ac8 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -152,8 +152,11 @@ private async Task ResolveTagTip(Guid tagId) private CommitMetadata ApplyAssignment(CommitMetadata? commitMetadata, BranchAssignment assignment) { var metadata = commitMetadata ?? new CommitMetadata(); + // Resolve to a concrete assignment and mark it, so the commit interceptor treats it as + // already-decided (and does not re-derive or reject it, e.g. ref-lifecycle writes to main + // while a tag is checked out). var branchId = ResolveBranchId(assignment); - RefMetadata.SetBranchId(metadata, branchId); + RefMetadata.SetAssignment(metadata, branchId is null ? BranchAssignment.Main : BranchAssignment.ToBranch(branchId.Value)); return metadata; } diff --git a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs new file mode 100644 index 0000000..418b74e --- /dev/null +++ b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs @@ -0,0 +1,161 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Sample.Models; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +/// +/// Ticket 04: authoring through directly (no RefsDataModel +/// wrapper) transparently applies the current checkout's branch assignment, and a +/// per-call override via takes precedence. +/// +public class TransparentAuthoringTests : DataModelTestBase +{ + private readonly RefsDataModel _refs; + + public TransparentAuthoringTests() : base(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }) + { + _refs = _services.GetRequiredService(); + } + + [Fact] + public async Task DirectAuthoringOnBranchCheckoutStampsBranchAndIsHiddenOnMain() + { + var branchId = Guid.NewGuid(); + var wordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + + await _refs.CheckoutBranch(branchId); + var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); + + RefMetadata.GetBranchId(commit.Metadata).Should().Be(branchId); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-branch"); + + await _refs.CheckoutMain(); + (await DataModel.GetLatest(wordId)).Should().BeNull(); + } + + [Fact] + public async Task DirectAuthoringOnMainCheckoutAuthorsToMain() + { + var wordId = Guid.NewGuid(); + var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-main")); + + RefMetadata.GetBranchId(commit.Metadata).Should().BeNull(); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-main"); + } + + [Fact] + public async Task DirectAuthoringOnTagCheckoutThrowsByDefault() + { + var tip = await DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "at-tag")); + var tagId = Guid.NewGuid(); + await _refs.CreateTag(_localClientId, tagId, "v1", tip.Id); + await _refs.CheckoutTag(tagId); + + var act = () => DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "nope")); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task DirectAuthoringOnTagCheckoutWritesToMainWhenConfigured() + { + await using var withConfig = new DataModelTestBase(configure: services => + { + services.Configure(config => + { + config.AddHarmonyRefs(); + config.AllowAuthoringOnTagToMain = true; + }); + services.AddHarmonyRefsDataModel(); + }); + var refs = withConfig.GetRequiredService(); + + var tip = await withConfig.DataModel.AddChange(withConfig.LocalClientId, withConfig.SetWord(Guid.NewGuid(), "at-tag")); + var tagId = Guid.NewGuid(); + await refs.CreateTag(withConfig.LocalClientId, tagId, "v1", tip.Id); + await refs.CheckoutTag(tagId); + + var newWordId = Guid.NewGuid(); + var commit = await withConfig.DataModel.AddChange(withConfig.LocalClientId, withConfig.SetWord(newWordId, "authored-to-main")); + RefMetadata.GetBranchId(commit.Metadata).Should().BeNull(); + + await refs.CheckoutMain(); + (await withConfig.DataModel.GetLatest(newWordId))!.Text.Should().Be("authored-to-main"); + } + + [Fact] + public async Task PerCallOverrideForcesMainOrAnotherBranchRegardlessOfCheckout() + { + var featureId = Guid.NewGuid(); + var otherId = Guid.NewGuid(); + var mainWordId = Guid.NewGuid(); + var otherWordId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); + await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + + await _refs.CheckoutBranch(featureId); + + var mainCommit = await DataModel.AddChange( + _localClientId, + SetWord(mainWordId, "forced-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + RefMetadata.GetBranchId(mainCommit.Metadata).Should().BeNull(); + (await DataModel.GetLatest(mainWordId))!.Text.Should().Be("forced-main"); + + var otherCommit = await DataModel.AddChange( + _localClientId, + SetWord(otherWordId, "other-branch"), + RefMetadata.SetAssignment(new(), BranchAssignment.ToBranch(otherId))); + RefMetadata.GetBranchId(otherCommit.Metadata).Should().Be(otherId); + (await DataModel.GetLatest(otherWordId)).Should().BeNull(); + } + + [Fact] + public async Task PerCallOverrideToMainOnTagCheckoutDoesNotThrow() + { + await using var withConfig = new DataModelTestBase(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }); + var refs = withConfig.GetRequiredService(); + + var tip = await withConfig.DataModel.AddChange(withConfig.LocalClientId, withConfig.SetWord(Guid.NewGuid(), "at-tag")); + var tagId = Guid.NewGuid(); + await refs.CreateTag(withConfig.LocalClientId, tagId, "v1", tip.Id); + await refs.CheckoutTag(tagId); + + var wordId = Guid.NewGuid(); + // Explicit main assignment bypasses the tag-authoring rejection even though + // AllowAuthoringOnTagToMain is false, because the caller opted in per-call. + var commit = await withConfig.DataModel.AddChange( + withConfig.LocalClientId, + withConfig.SetWord(wordId, "explicit-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + RefMetadata.GetBranchId(commit.Metadata).Should().BeNull(); + } +} + +/// +/// Regression: with refs not registered, authoring through +/// behaves exactly as before — no interceptor, no branch metadata. +/// +public class CoreOnlyAuthoringTests : DataModelTestBase +{ + [Fact] + public async Task AuthoringWithoutRefsAppliesNoBranchMetadata() + { + var wordId = Guid.NewGuid(); + var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "plain")); + + commit.Metadata.ExtraMetadata.Should().BeEmpty(); + (await DataModel.GetLatest(wordId))!.Text.Should().Be("plain"); + } +} diff --git a/src/SIL.Harmony/CrdtKernel.cs b/src/SIL.Harmony/CrdtKernel.cs index d622f91..7594439 100644 --- a/src/SIL.Harmony/CrdtKernel.cs +++ b/src/SIL.Harmony/CrdtKernel.cs @@ -41,7 +41,8 @@ public static IServiceCollection AddCrdtDataCore(this IServiceCollection service provider.GetRequiredService(), provider.GetRequiredService>(), provider.GetRequiredService>(), - provider.GetService() + provider.GetService(), + provider.GetService() )); //must use factory method because ResourceService constructor is internal services.AddScoped(provider => new ResourceService( diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index b4d4b99..f119ace 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -24,6 +24,7 @@ public class DataModel : ISyncable, IAsyncDisposable private readonly IOptions _crdtConfig; private readonly ILogger _logger; private readonly ICommitMaterializationFilter? _materializationFilter; + private readonly ICommitInterceptor? _commitInterceptor; //constructor must be internal because CrdtRepository is internal internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, @@ -31,7 +32,8 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, IHybridDateTimeProvider timeProvider, IOptions crdtConfig, ILogger logger, - ICommitMaterializationFilter? materializationFilter = null) + ICommitMaterializationFilter? materializationFilter = null, + ICommitInterceptor? commitInterceptor = null) { _crdtRepositoryFactory = crdtRepositoryFactory; _serializerOptions = serializerOptions; @@ -39,6 +41,7 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, _crdtConfig = crdtConfig; _logger = logger; _materializationFilter = materializationFilter; + _commitInterceptor = commitInterceptor; } private ICommitMaterializationFilter MaterializationFilter => @@ -104,6 +107,11 @@ private Commit NewCommit(Guid clientId, CommitMetadata? commitMetadata, IEnumera Metadata = commitMetadata ?? new() }; commit.ChangeEntities.AddRange(changes.Select((c, i) => ToChangeEntity(c, i, commit.Id))); + // Single author choke point: let an optional interceptor (e.g. refs branch assignment) + // stamp metadata or reject authoring before the commit is persisted. Metadata is not + // part of the commit hash, so this is safe to do here. Sync-applied commits never pass + // through NewCommit, so their assignment is left untouched. + _commitInterceptor?.OnCommitAuthored(commit); return commit; } diff --git a/src/SIL.Harmony/ICommitInterceptor.cs b/src/SIL.Harmony/ICommitInterceptor.cs new file mode 100644 index 0000000..d2e3298 --- /dev/null +++ b/src/SIL.Harmony/ICommitInterceptor.cs @@ -0,0 +1,11 @@ +namespace SIL.Harmony; + +/// +/// Optional hook invoked once per locally-authored commit, before it is persisted. +/// Implementations may mutate and may throw to reject authoring. +/// It is not invoked for commits received via sync — those keep the assignment their author gave them. +/// +public interface ICommitInterceptor +{ + void OnCommitAuthored(Commit commit); +} From 03a4904a49f3e10b54f56b22697530986c6fa083 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 15:10:33 +0700 Subject: [PATCH 13/18] Make branch-assignment marker transient and support multiple interceptors The harmony.branchAssigned marker only needs to signal the authoring interceptor at author time; it should not pollute stored/synced metadata. The interceptor now consumes it (RefMetadata.ConsumeAssignment reads and removes it) before the commit is persisted, and the marker key is private. DataModel now resolves IEnumerable (GetServices) and invokes each in registration order, so multiple interceptors compose. Adds tests ExplicitAssignmentMarkerIsNotPersisted and AllRegisteredInterceptorsAreInvoked. Co-Authored-By: Claude Opus 4.8 --- .../CheckoutCommitInterceptor.cs | 5 +- src/SIL.Harmony.Refs/RefMetadata.cs | 19 ++++--- .../TransparentAuthoringTests.cs | 50 +++++++++++++++++++ src/SIL.Harmony/CrdtKernel.cs | 2 +- src/SIL.Harmony/DataModel.cs | 17 ++++--- 5 files changed, 75 insertions(+), 18 deletions(-) diff --git a/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs b/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs index 1813365..56f8564 100644 --- a/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs +++ b/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs @@ -6,7 +6,7 @@ namespace SIL.Harmony.Refs; /// Applies the current checkout's branch assignment to locally-authored commits so that /// clients can author through directly without routing /// through . Commits that already carry an explicit assignment -/// (see ) are left untouched. +/// are left untouched. /// public sealed class CheckoutCommitInterceptor(CheckoutMaterializationFilter filter, IOptions config) : ICommitInterceptor @@ -14,7 +14,8 @@ public sealed class CheckoutCommitInterceptor(CheckoutMaterializationFilter filt public void OnCommitAuthored(Commit commit) { // Explicit per-call override or a ref-lifecycle assignment already decided this commit. - if (RefMetadata.IsAssigned(commit.Metadata)) return; + // ConsumeAssignment also clears the transient marker so it is never persisted or synced. + if (RefMetadata.ConsumeAssignment(commit.Metadata)) return; switch (filter.Checkout) { diff --git a/src/SIL.Harmony.Refs/RefMetadata.cs b/src/SIL.Harmony.Refs/RefMetadata.cs index 7afde45..5a3346b 100644 --- a/src/SIL.Harmony.Refs/RefMetadata.cs +++ b/src/SIL.Harmony.Refs/RefMetadata.cs @@ -11,12 +11,13 @@ public static class RefMetadata public const string BranchIdKey = "harmony.branchId"; /// - /// Marks that a commit's branch assignment was set deliberately (including to main). - /// Distinguishes an explicit assignment from an unassigned commit — both of which - /// have no when the assignment is main — so the authoring - /// interceptor knows to leave the commit alone rather than derive from the checkout. + /// Transient authoring-time marker: distinguishes an explicit assignment (including to main, + /// which has no ) from an unassigned commit, so the authoring + /// interceptor knows to leave the commit alone rather than derive from the checkout. It is + /// consumed (removed) by the interceptor via before the commit + /// is persisted, so it never lands in stored or synced metadata. /// - public const string BranchAssignedKey = "harmony.branchAssigned"; + private const string BranchAssignedKey = "harmony.branchAssigned"; public static Guid? GetBranchId(CommitMetadata metadata) { @@ -36,15 +37,19 @@ public static void SetBranchId(CommitMetadata metadata, Guid? branchId) } /// - /// True when the commit carries a deliberate branch assignment (see ). + /// Reads and clears the transient assignment marker, returning whether the commit carried a + /// deliberate assignment. Called once by the authoring interceptor so the marker is consumed + /// before persistence and never synced. /// - public static bool IsAssigned(CommitMetadata metadata) => metadata[BranchAssignedKey] is not null; + internal static bool ConsumeAssignment(CommitMetadata metadata) => + metadata.ExtraMetadata.Remove(BranchAssignedKey); /// /// Records an explicit per-call branch assignment on so it takes /// precedence over the current checkout when authoring through . /// and mark the commit; /// leaves it unmarked so the checkout is used. + /// The marker is transient — the authoring interceptor consumes it, so it never persists. /// Returns the same metadata for fluent use. /// public static CommitMetadata SetAssignment(CommitMetadata metadata, BranchAssignment assignment) diff --git a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs index 418b74e..f46b05b 100644 --- a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs +++ b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs @@ -117,6 +117,56 @@ public async Task PerCallOverrideForcesMainOrAnotherBranchRegardlessOfCheckout() (await DataModel.GetLatest(otherWordId)).Should().BeNull(); } + [Fact] + public async Task AllRegisteredInterceptorsAreInvoked() + { + await using var tb = new DataModelTestBase(configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + services.AddScoped(); + services.AddScoped(sp => sp.GetRequiredService()); + }); + var recording = tb.GetRequiredService(); + var refs = tb.GetRequiredService(); + + var branchId = Guid.NewGuid(); + await tb.DataModel.AddChange(tb.LocalClientId, new CreateBranchChange(branchId, "feature")); + await refs.CheckoutBranch(branchId); + var commit = await tb.DataModel.AddChange(tb.LocalClientId, tb.SetWord(Guid.NewGuid(), "x")); + + // The extra interceptor ran alongside the checkout interceptor... + recording.Count.Should().BeGreaterThan(0); + // ...and the checkout interceptor still applied the branch assignment. + RefMetadata.GetBranchId(commit.Metadata).Should().Be(branchId); + } + + [Fact] + public async Task ExplicitAssignmentMarkerIsNotPersisted() + { + var featureId = Guid.NewGuid(); + await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); + await _refs.CheckoutBranch(featureId); + + var wordId = Guid.NewGuid(); + var commit = await DataModel.AddChange( + _localClientId, + SetWord(wordId, "forced-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + + // Explicit main leaves no branch id and no transient marker behind. + var stored = await DbContext.Commits.AsNoTracking() + .SingleAsync(c => c.Id == commit.Id, TestContext.Current.CancellationToken); + RefMetadata.GetBranchId(stored.Metadata).Should().BeNull(); + stored.Metadata.ExtraMetadata.Should().BeEmpty(); + } + + private sealed class RecordingInterceptor : ICommitInterceptor + { + public int Count { get; private set; } + public void OnCommitAuthored(Commit commit) => Count++; + } + [Fact] public async Task PerCallOverrideToMainOnTagCheckoutDoesNotThrow() { diff --git a/src/SIL.Harmony/CrdtKernel.cs b/src/SIL.Harmony/CrdtKernel.cs index 7594439..6c8391b 100644 --- a/src/SIL.Harmony/CrdtKernel.cs +++ b/src/SIL.Harmony/CrdtKernel.cs @@ -42,7 +42,7 @@ public static IServiceCollection AddCrdtDataCore(this IServiceCollection service provider.GetRequiredService>(), provider.GetRequiredService>(), provider.GetService(), - provider.GetService() + provider.GetServices() )); //must use factory method because ResourceService constructor is internal services.AddScoped(provider => new ResourceService( diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index f119ace..da9eefb 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -24,7 +24,7 @@ public class DataModel : ISyncable, IAsyncDisposable private readonly IOptions _crdtConfig; private readonly ILogger _logger; private readonly ICommitMaterializationFilter? _materializationFilter; - private readonly ICommitInterceptor? _commitInterceptor; + private readonly IReadOnlyList _commitInterceptors; //constructor must be internal because CrdtRepository is internal internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, @@ -33,7 +33,7 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, IOptions crdtConfig, ILogger logger, ICommitMaterializationFilter? materializationFilter = null, - ICommitInterceptor? commitInterceptor = null) + IEnumerable? commitInterceptors = null) { _crdtRepositoryFactory = crdtRepositoryFactory; _serializerOptions = serializerOptions; @@ -41,7 +41,7 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, _crdtConfig = crdtConfig; _logger = logger; _materializationFilter = materializationFilter; - _commitInterceptor = commitInterceptor; + _commitInterceptors = commitInterceptors?.ToArray() ?? []; } private ICommitMaterializationFilter MaterializationFilter => @@ -107,11 +107,12 @@ private Commit NewCommit(Guid clientId, CommitMetadata? commitMetadata, IEnumera Metadata = commitMetadata ?? new() }; commit.ChangeEntities.AddRange(changes.Select((c, i) => ToChangeEntity(c, i, commit.Id))); - // Single author choke point: let an optional interceptor (e.g. refs branch assignment) - // stamp metadata or reject authoring before the commit is persisted. Metadata is not - // part of the commit hash, so this is safe to do here. Sync-applied commits never pass - // through NewCommit, so their assignment is left untouched. - _commitInterceptor?.OnCommitAuthored(commit); + // Single author choke point: let any registered interceptors (e.g. refs branch assignment) + // stamp metadata or reject authoring before the commit is persisted. Metadata is not part + // of the commit hash, so this is safe to do here. Sync-applied commits never pass through + // NewCommit, so their assignment is left untouched. + foreach (var interceptor in _commitInterceptors) + interceptor.OnCommitAuthored(commit); return commit; } From a551514b0f42bb5c817e22765c80018b6b94b813 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 15:29:33 +0700 Subject: [PATCH 14/18] Add post-apply listener for transparent tag roll-forward MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core defines a refs-agnostic ICommitAppliedListener, resolved lazily by DataModel and fired after the apply transaction commits (and its lock is released) in Add, AddManyChanges, and AddRangeFromSync — all genuine applies, never in RegenerateSnapshots, so a listener that rematerializes cannot re-enter. Notifying after the lock is released also avoids a deadlock on persistent databases, whose apply lock is shared and non-reentrant, when the listener's roll-forward opens its own repository. The ticket-04 refs interceptor becomes CheckoutRefsHandler, now implementing both extension points: as the interceptor it stamps the checkout's branch assignment; as the listener it rolls an active tag checkout forward when the tip moved. It resolves DataModel lazily via IServiceProvider to break the roll-forward -> DataModel cycle. Co-Authored-By: Claude Opus 4.8 --- .../CheckoutCommitInterceptor.cs | 32 --- .../CheckoutMaterializationFilter.cs | 6 + src/SIL.Harmony.Refs/CheckoutRefsHandler.cs | 66 ++++++ src/SIL.Harmony.Refs/HarmonyRefsKernel.cs | 6 +- .../TagRollForwardListenerTests.cs | 190 ++++++++++++++++++ src/SIL.Harmony/CrdtKernel.cs | 3 +- src/SIL.Harmony/DataModel.cs | 110 ++++++---- src/SIL.Harmony/ICommitAppliedListener.cs | 13 ++ 8 files changed, 356 insertions(+), 70 deletions(-) delete mode 100644 src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs create mode 100644 src/SIL.Harmony.Refs/CheckoutRefsHandler.cs create mode 100644 src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs create mode 100644 src/SIL.Harmony/ICommitAppliedListener.cs diff --git a/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs b/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs deleted file mode 100644 index 56f8564..0000000 --- a/src/SIL.Harmony.Refs/CheckoutCommitInterceptor.cs +++ /dev/null @@ -1,32 +0,0 @@ -using Microsoft.Extensions.Options; - -namespace SIL.Harmony.Refs; - -/// -/// Applies the current checkout's branch assignment to locally-authored commits so that -/// clients can author through directly without routing -/// through . Commits that already carry an explicit assignment -/// are left untouched. -/// -public sealed class CheckoutCommitInterceptor(CheckoutMaterializationFilter filter, IOptions config) - : ICommitInterceptor -{ - public void OnCommitAuthored(Commit commit) - { - // Explicit per-call override or a ref-lifecycle assignment already decided this commit. - // ConsumeAssignment also clears the transient marker so it is never persisted or synced. - if (RefMetadata.ConsumeAssignment(commit.Metadata)) return; - - switch (filter.Checkout) - { - case BranchCheckout branch: - RefMetadata.SetBranchId(commit.Metadata, branch.BranchId); - break; - case TagCheckout when !config.Value.AllowAuthoringOnTagToMain: - throw new InvalidOperationException( - "Authoring is not allowed while checked out on a tag. Set AllowAuthoringOnTagToMain to write to main, " - + "author with an explicit BranchAssignment, or checkout main/branch first."); - // MainCheckout, or a tag checkout with AllowAuthoringOnTagToMain: author to main (no branch id). - } - } -} diff --git a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs index ea85166..a2dac44 100644 --- a/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs +++ b/src/SIL.Harmony.Refs/CheckoutMaterializationFilter.cs @@ -19,6 +19,12 @@ public sealed class CheckoutMaterializationFilter : ICommitMaterializationFilter public bool HasAsOfTip => _asOfTip is not null; + /// + /// The commit the current tag checkout is pinned to, or null when not pinned to a tip. + /// Lets the roll-forward listener skip rematerialization when the tag tip has not moved. + /// + public Guid? AsOfTipId => _asOfTip?.Id; + /// /// When set (tag checkout), only commits at or before this tip are included, /// and merge incorporation is evaluated only through this tip. diff --git a/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs b/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs new file mode 100644 index 0000000..640402c --- /dev/null +++ b/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs @@ -0,0 +1,66 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs; + +/// +/// The refs bridge into 's generic extension points, so clients can author +/// and sync through directly and still get branch/tag behaviour without the +/// wrapper. +/// +/// As an it stamps the current checkout's branch assignment +/// on locally-authored commits (or rejects authoring on a tag). +/// As an it rolls an active tag checkout forward after +/// any apply — local or synced — when the tag's tip moved. +/// +/// is resolved lazily (only when rolling forward) because the roll-forward +/// depends on it, yet it is already constructed by the time an apply completes; requiring it in the +/// constructor would form a cycle, since this same instance is also the constructor-injected interceptor. +/// +public sealed class CheckoutRefsHandler( + CheckoutMaterializationFilter filter, + IOptions config, + IServiceProvider serviceProvider) + : ICommitInterceptor, ICommitAppliedListener +{ + public void OnCommitAuthored(Commit commit) + { + // Explicit per-call override or a ref-lifecycle assignment already decided this commit. + // ConsumeAssignment also clears the transient marker so it is never persisted or synced. + if (RefMetadata.ConsumeAssignment(commit.Metadata)) return; + + switch (filter.Checkout) + { + case BranchCheckout branch: + RefMetadata.SetBranchId(commit.Metadata, branch.BranchId); + break; + case TagCheckout when !config.Value.AllowAuthoringOnTagToMain: + throw new InvalidOperationException( + "Authoring is not allowed while checked out on a tag. Set AllowAuthoringOnTagToMain to write to main, " + + "author with an explicit BranchAssignment, or checkout main/branch first."); + // MainCheckout, or a tag checkout with AllowAuthoringOnTagToMain: author to main (no branch id). + } + } + + public async Task OnCommitsAppliedAsync(IReadOnlyCollection commits) + { + // Roll-forward only concerns tag checkouts; a main/branch view never moves on apply. + if (filter.Checkout is not TagCheckout tag) return; + + var dataModel = serviceProvider.GetRequiredService(); + var tip = await ResolveTagTip(dataModel, tag.TagId); + // Skip rematerialization unless the tag's tip actually moved. + if (filter.AsOfTipId == tip.Id) return; + + filter.SetAsOfTip(tip); + await dataModel.RegenerateSnapshots(); + } + + private static async Task ResolveTagTip(DataModel dataModel, Guid tagId) + { + var tag = await dataModel.GetLatest(tagId) + ?? throw new InvalidOperationException($"Tag {tagId} was not found."); + return await dataModel.GetCommit(tag.TargetCommitId); + } +} diff --git a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs index 2cf6ee0..96ecc89 100644 --- a/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs +++ b/src/SIL.Harmony.Refs/HarmonyRefsKernel.cs @@ -32,7 +32,11 @@ public static IServiceCollection AddHarmonyRefsDataModel(this IServiceCollection services.AddScoped(); services.AddScoped(sp => sp.GetRequiredService()); - services.AddScoped(); + // One handler instance serves both extension points: the authoring interceptor and the + // post-apply roll-forward listener. + services.AddScoped(); + services.AddScoped(sp => sp.GetRequiredService()); + services.AddScoped(sp => sp.GetRequiredService()); services.AddScoped(); return services; } diff --git a/src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs b/src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs new file mode 100644 index 0000000..84ff005 --- /dev/null +++ b/src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs @@ -0,0 +1,190 @@ +using SIL.Harmony.Refs; +using SIL.Harmony.Refs.Changes; +using SIL.Harmony.Sample.Models; +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.DependencyInjection; + +namespace SIL.Harmony.Tests; + +/// +/// Ticket 05: with refs registered, an active tag checkout rolls forward automatically after any +/// apply — through for a synced tag move and through +/// for a local one — driven by the post-apply listener rather than +/// a wrapper call. Roll-forward is skipped when the tip did not move. +/// +public class TagRollForwardListenerTests : IAsyncLifetime +{ + private readonly DataModelTestBase _client1; + private readonly DataModelTestBase _client2; + private readonly RefsDataModel _refs1; + private readonly RefsDataModel _refs2; + + public TagRollForwardListenerTests() + { + static void Configure(IServiceCollection services) + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + } + + _client1 = new DataModelTestBase(configure: Configure); + _client2 = new DataModelTestBase(configure: Configure); + _refs1 = _client1.GetRequiredService(); + _refs2 = _client2.GetRequiredService(); + } + + public async ValueTask InitializeAsync() + { + await _client1.InitializeAsync(); + await _client2.InitializeAsync(); + } + + public async ValueTask DisposeAsync() + { + await _client1.DisposeAsync(); + await _client2.DisposeAsync(); + } + + [Fact] + public async Task SyncedTagMoveRollsForwardActiveCheckout_ViaDataModelSyncWith() + { + var wordId = Guid.NewGuid(); + var first = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "first")); + var second = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "second")); + var tagId = Guid.NewGuid(); + await _refs1.CreateTag(_client1.LocalClientId, tagId, "release", first.Id); + + await _client1.DataModel.SyncWith(_client2.DataModel); + + await _refs2.CheckoutTag(tagId); + (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + // Move the tag on client1, then sync through the plain DataModel API — the listener, not a + // RefsDataModel.SyncWith wrapper, must roll client2's active tag view forward. + await _refs1.MoveTag(_client1.LocalClientId, tagId, second.Id); + await _client2.DataModel.SyncWith(_client1.DataModel); + + (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); + } + + [Fact] + public async Task LocalTagMoveRollsForwardActiveCheckout_ViaListener() + { + var wordId = Guid.NewGuid(); + var first = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "first")); + var second = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "second")); + var tagId = Guid.NewGuid(); + await _refs1.CreateTag(_client1.LocalClientId, tagId, "release", first.Id); + + await _refs1.CheckoutTag(tagId); + (await _client1.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + // Author the move straight through DataModel (explicit main assignment, so it is allowed on a + // tag checkout and lands on main). Only the post-apply listener can roll the view forward here. + await _client1.DataModel.AddChange( + _client1.LocalClientId, + new MoveTagChange(tagId, second.Id), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + + (await _client1.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); + } + + [Fact(Timeout = 30_000)] + public async Task LocalTagMoveRollsForwardOnPersistentDatabase() + { + // A file-backed database uses a stable, shared apply lock (unlike :memory:, which normalizes + // to a fresh lock per repository). The lock is not reentrant, so if the post-apply + // notification fired while the apply lock was held, the listener's roll-forward would deadlock + // trying to take that same lock. This exercises that path; a regression would hang and trip + // the timeout. + var dbPath = Path.Combine(Path.GetTempPath(), $"harmony-rollforward-{Guid.NewGuid():N}.db"); + var connection = new SqliteConnection($"Data Source={dbPath}"); + var client = new DataModelTestBase(connection, configure: services => + { + services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); + }); + try + { + var refs = client.GetRequiredService(); + + var wordId = Guid.NewGuid(); + var first = await client.DataModel.AddChange(client.LocalClientId, client.SetWord(wordId, "first")); + var second = await client.DataModel.AddChange(client.LocalClientId, client.SetWord(wordId, "second")); + var tagId = Guid.NewGuid(); + await refs.CreateTag(client.LocalClientId, tagId, "release", first.Id); + + await refs.CheckoutTag(tagId); + (await client.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + await client.DataModel.AddChange( + client.LocalClientId, + new MoveTagChange(tagId, second.Id), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + + (await client.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); + } + finally + { + await client.DisposeAsync(); + SqliteConnection.ClearAllPools(); + if (File.Exists(dbPath)) File.Delete(dbPath); + } + } + + [Fact] + public async Task NoRollForwardWhenActiveTagTipDidNotMove() + { + var wordId = Guid.NewGuid(); + var laterId = Guid.NewGuid(); + var first = await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "first")); + var tagId = Guid.NewGuid(); + await _refs1.CreateTag(_client1.LocalClientId, tagId, "release", first.Id); + + await _refs1.CheckoutTag(tagId); + (await _client1.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + + // A main-line commit that does not touch the tag: the tip is unchanged, so the view stays + // pinned at the tag tip and the new commit remains invisible in the tag view. + await _client1.DataModel.AddChange( + _client1.LocalClientId, + _client1.SetWord(laterId, "after-tip"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + + (await _client1.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); + (await _client1.DataModel.GetLatest(laterId)).Should().BeNull(); + } +} + +/// +/// Regression: with refs not registered, 's apply paths (local author + sync) +/// behave exactly as before — no listener runs. +/// +public class CoreOnlyApplyTests : IAsyncLifetime +{ + private readonly DataModelTestBase _client1 = new(); + private readonly DataModelTestBase _client2 = new(); + + public async ValueTask InitializeAsync() + { + await _client1.InitializeAsync(); + await _client2.InitializeAsync(); + } + + public async ValueTask DisposeAsync() + { + await _client1.DisposeAsync(); + await _client2.DisposeAsync(); + } + + [Fact] + public async Task AuthorAndSyncBehaveAsBeforeWithoutRefs() + { + var wordId = Guid.NewGuid(); + await _client1.DataModel.AddChange(_client1.LocalClientId, _client1.SetWord(wordId, "hello")); + + await _client1.DataModel.SyncWith(_client2.DataModel); + + (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("hello"); + } +} diff --git a/src/SIL.Harmony/CrdtKernel.cs b/src/SIL.Harmony/CrdtKernel.cs index 6c8391b..d777aa2 100644 --- a/src/SIL.Harmony/CrdtKernel.cs +++ b/src/SIL.Harmony/CrdtKernel.cs @@ -42,7 +42,8 @@ public static IServiceCollection AddCrdtDataCore(this IServiceCollection service provider.GetRequiredService>(), provider.GetRequiredService>(), provider.GetService(), - provider.GetServices() + provider.GetServices(), + () => provider.GetServices() )); //must use factory method because ResourceService constructor is internal services.AddScoped(provider => new ResourceService( diff --git a/src/SIL.Harmony/DataModel.cs b/src/SIL.Harmony/DataModel.cs index da9eefb..5b93edb 100644 --- a/src/SIL.Harmony/DataModel.cs +++ b/src/SIL.Harmony/DataModel.cs @@ -25,6 +25,7 @@ public class DataModel : ISyncable, IAsyncDisposable private readonly ILogger _logger; private readonly ICommitMaterializationFilter? _materializationFilter; private readonly IReadOnlyList _commitInterceptors; + private readonly Func>? _commitAppliedListenersFactory; //constructor must be internal because CrdtRepository is internal internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, @@ -33,7 +34,8 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, IOptions crdtConfig, ILogger logger, ICommitMaterializationFilter? materializationFilter = null, - IEnumerable? commitInterceptors = null) + IEnumerable? commitInterceptors = null, + Func>? commitAppliedListenersFactory = null) { _crdtRepositoryFactory = crdtRepositoryFactory; _serializerOptions = serializerOptions; @@ -42,6 +44,10 @@ internal DataModel(CrdtRepositoryFactory crdtRepositoryFactory, _logger = logger; _materializationFilter = materializationFilter; _commitInterceptors = commitInterceptors?.ToArray() ?? []; + // Listeners are resolved lazily (at apply time) rather than in the constructor: a listener's + // roll-forward depends transitively on this DataModel, so eager resolution would form a + // constructor cycle. The interceptor above has no such dependency and is resolved eagerly. + _commitAppliedListenersFactory = commitAppliedListenersFactory; } private ICommitMaterializationFilter MaterializationFilter => @@ -71,20 +77,27 @@ public async Task AddManyChanges(Guid clientId, Func commitMetadata, int changesPerCommitMax = 100) { - await using var repo = await _crdtRepositoryFactory.CreateRepository(); - var commits = changes - .Chunk(changesPerCommitMax) - .Select(chunk => NewCommit(clientId, commitMetadata(), chunk)) - .ToArray(); - if (commits is []) return; - using var locked = await repo.Lock(); - repo.ClearChangeTracker(); + Commit[] commits; + await using (var repo = await _crdtRepositoryFactory.CreateRepository()) + { + commits = changes + .Chunk(changesPerCommitMax) + .Select(chunk => NewCommit(clientId, commitMetadata(), chunk)) + .ToArray(); + if (commits is []) return; + using var locked = await repo.Lock(); + repo.ClearChangeTracker(); - await using var transaction = await repo.BeginTransactionAsync(); - var updatedCommits = await repo.AddCommits(commits); - await UpdateSnapshots(repo, updatedCommits); - await ValidateCommits(repo); - await transaction.CommitAsync(); + await using var transaction = await repo.BeginTransactionAsync(); + var updatedCommits = await repo.AddCommits(commits); + await UpdateSnapshots(repo, updatedCommits); + await ValidateCommits(repo); + await transaction.CommitAsync(); + } + // Notify after the repo (and its lock) is released: a listener's roll-forward opens its own + // repository, and the apply lock is not reentrant, so notifying while holding it would deadlock + // on a persistent database. + await NotifyCommitsApplied(commits); } /// @@ -118,19 +131,38 @@ private Commit NewCommit(Guid clientId, CommitMetadata? commitMetadata, IEnumera private async Task Add(Commit commit) { - await using var repo = await _crdtRepositoryFactory.CreateRepository(); - using var locked = await repo.Lock(); - if (await repo.HasCommit(commit.Id)) return; - repo.ClearChangeTracker(); + await using (var repo = await _crdtRepositoryFactory.CreateRepository()) + { + using var locked = await repo.Lock(); + if (await repo.HasCommit(commit.Id)) return; + repo.ClearChangeTracker(); - await using var transaction = repo.IsInTransaction ? null : await repo.BeginTransactionAsync(); - var updatedCommits = await repo.AddCommit(commit); - await UpdateSnapshots(repo, updatedCommits); + await using var transaction = repo.IsInTransaction ? null : await repo.BeginTransactionAsync(); + var updatedCommits = await repo.AddCommit(commit); + await UpdateSnapshots(repo, updatedCommits); - if (AlwaysValidate) await ValidateCommits(repo); + if (AlwaysValidate) await ValidateCommits(repo); + // Nested apply: the caller owns the transaction and the post-apply notification, so a + // listener never observes uncommitted state. Bail before notifying. + if (transaction is null) return; + await transaction.CommitAsync(); + } + // See NotifyCommitsApplied / AddManyChanges: fire only after the lock is released. + await NotifyCommitsApplied([commit]); + } - if (transaction is not null) await transaction.CommitAsync(); + /// + /// Fires registered s after an apply transaction commits and + /// the repository lock is released. Only invoked from the genuine apply entrypoints (author + + /// sync) — never from — so a listener that rematerializes + /// cannot re-enter. + /// + private async Task NotifyCommitsApplied(IReadOnlyCollection commits) + { + if (_commitAppliedListenersFactory is null || commits.Count == 0) return; + foreach (var listener in _commitAppliedListenersFactory()) + await listener.OnCommitsAppliedAsync(commits); } public ValueTask DisposeAsync() @@ -151,19 +183,25 @@ async Task ISyncable.AddRangeFromSync(IEnumerable commits) commits = commits.ToArray(); try { - await using var repo = await _crdtRepositoryFactory.CreateRepository(); - using var locked = await repo.Lock(); - repo.ClearChangeTracker(); - _timeProvider.TakeLatestTime(commits.Select(c => c.HybridDateTime)); - var (oldestChange, newCommits) = await repo.FilterExistingCommits(commits.ToArray()); - //no changes added - if (oldestChange is null || newCommits is []) return; - - await using var transaction = await repo.BeginTransactionAsync(); - var updatedCommits = await repo.AddCommits(newCommits); - await UpdateSnapshots(repo, updatedCommits); - await ValidateCommits(repo); - await transaction.CommitAsync(); + Commit[] newCommits; + await using (var repo = await _crdtRepositoryFactory.CreateRepository()) + { + using var locked = await repo.Lock(); + repo.ClearChangeTracker(); + _timeProvider.TakeLatestTime(commits.Select(c => c.HybridDateTime)); + Commit? oldestChange; + (oldestChange, newCommits) = await repo.FilterExistingCommits(commits.ToArray()); + //no changes added + if (oldestChange is null || newCommits is []) return; + + await using var transaction = await repo.BeginTransactionAsync(); + var updatedCommits = await repo.AddCommits(newCommits); + await UpdateSnapshots(repo, updatedCommits); + await ValidateCommits(repo); + await transaction.CommitAsync(); + } + // Fire after the lock is released so a listener's roll-forward can take it (see NotifyCommitsApplied). + await NotifyCommitsApplied(newCommits); } catch (DbUpdateException e) { diff --git a/src/SIL.Harmony/ICommitAppliedListener.cs b/src/SIL.Harmony/ICommitAppliedListener.cs new file mode 100644 index 0000000..6ef8db4 --- /dev/null +++ b/src/SIL.Harmony/ICommitAppliedListener.cs @@ -0,0 +1,13 @@ +namespace SIL.Harmony; + +/// +/// Optional hook invoked after commits are applied — for both local authoring and sync — once the +/// apply transaction has committed. The method is asynchronous so implementations can do database +/// work (e.g. rolling a tag checkout forward). It is not invoked from +/// or scoped read paths; this entrypoint boundary is what prevents a listener that rematerializes +/// from re-entering. +/// +public interface ICommitAppliedListener +{ + Task OnCommitsAppliedAsync(IReadOnlyCollection commits); +} From a115e6034e26ac0c346a290c843ef6129c6670ab Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 15:50:16 +0700 Subject: [PATCH 15/18] Slim RefsDataModel to checkout and ref lifecycle only. Authoring and sync now go through DataModel directly; the interceptor and post-apply listener own assignment stamping and tag roll-forward. Co-authored-by: Cursor --- src/SIL.Harmony.Refs/RefsDataModel.cs | 118 ++---------------- .../BranchCheckoutViewTests.cs | 17 +-- .../ScopedAuthoringMainIsolationTests.cs | 14 +-- src/SIL.Harmony.Tests/TagCheckoutTests.cs | 12 +- .../TagSyncRollForwardTests.cs | 4 +- 5 files changed, 36 insertions(+), 129 deletions(-) diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index f2f8ac8..13bd29d 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -1,13 +1,14 @@ using Microsoft.Extensions.Options; -using SIL.Harmony.Changes; using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; namespace SIL.Harmony.Refs; /// -/// Thin authoring/checkout wrapper over . -/// Checkout lives on (single source of truth); +/// Checkout switching and ref lifecycle over . +/// Authoring and sync go through directly; the refs handler stamps +/// branch assignment on commit and rolls tag checkouts forward after apply. +/// Checkout state lives on (single source of truth); /// changing it rematerializes snapshots for the new view. /// public class RefsDataModel(DataModel dataModel, CheckoutMaterializationFilter filter, IOptions config) @@ -50,16 +51,11 @@ public async Task CheckoutTag(Guid tagId) public Task CreateTag(Guid clientId, Guid tagId, string name, Guid targetCommitId) => DataModel.AddChange(clientId, new CreateTagChange(tagId, name, targetCommitId), - ApplyAssignment(null, BranchAssignment.Main)); + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); - public async Task MoveTag(Guid clientId, Guid tagId, Guid targetCommitId) - { - var commit = await DataModel.AddChange(clientId, new MoveTagChange(tagId, targetCommitId), - ApplyAssignment(null, BranchAssignment.Main)); - if (Checkout is TagCheckout tag && tag.TagId == tagId) - await RollForwardActiveTag(tagId); - return commit; - } + public Task MoveTag(Guid clientId, Guid tagId, Guid targetCommitId) => + DataModel.AddChange(clientId, new MoveTagChange(tagId, targetCommitId), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); /// /// Incorporates into main visibility and deletes the branch entity. @@ -68,78 +64,8 @@ public async Task MoveTag(Guid clientId, Guid tagId, Guid targetCommitId public async Task MergeBranch(Guid clientId, Guid branchId) { await CheckoutMain(); - return await AddChange(clientId, new MergeBranchChange(branchId), BranchAssignment.Main); - } - - /// - /// Syncs via then rolls forward an active tag checkout if the tip moved. - /// - public async Task SyncWith(ISyncable remote) - { - Guid? tipBefore = null; - if (Checkout is TagCheckout active) - { - try { tipBefore = (await ResolveTagTip(active.TagId)).Id; } - catch (InvalidOperationException) { /* tag not projected yet */ } - } - - var results = await DataModel.SyncWith(remote); - - if (Checkout is TagCheckout checkedOut) - { - var tipAfter = await ResolveTagTip(checkedOut.TagId); - if (tipBefore != tipAfter.Id) - await RollForwardActiveTag(checkedOut.TagId); - } - - return results; - } - - /// - /// After sync (or external apply), refresh tag checkout if the tip moved. - /// - public async Task RefreshCheckoutAfterSync() - { - if (Checkout is not TagCheckout tag) return; - await RollForwardActiveTag(tag.TagId); - } - - public Task AddChange( - Guid clientId, - IChange change, - BranchAssignment assignment = default, - CommitMetadata? commitMetadata = null) - { - EnsureCanAuthor(); - if (assignment == default) assignment = BranchAssignment.FromCheckout; - return DataModel.AddChange(clientId, change, ApplyAssignment(commitMetadata, assignment)); - } - - public Task AddChanges( - Guid clientId, - IEnumerable changes, - BranchAssignment assignment = default, - CommitMetadata? commitMetadata = null) - { - EnsureCanAuthor(); - if (assignment == default) assignment = BranchAssignment.FromCheckout; - return DataModel.AddChanges(clientId, changes, ApplyAssignment(commitMetadata, assignment)); - } - - private void EnsureCanAuthor() - { - if (Checkout is not TagCheckout) return; - if (AllowAuthoringOnTagToMain) return; - throw new InvalidOperationException( - "Authoring is not allowed while checked out on a tag. Set AllowAuthoringOnTagToMain to write to main, or checkout main/branch first."); - } - - private async Task RollForwardActiveTag(Guid tagId) - { - var tip = await ResolveTagTip(tagId); - filter.Checkout = RefCheckout.ForTag(tagId); - filter.SetAsOfTip(tip); - await DataModel.RegenerateSnapshots(); + return await DataModel.AddChange(clientId, new MergeBranchChange(branchId), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); } private async Task ResolveTagTip(Guid tagId) @@ -148,28 +74,4 @@ private async Task ResolveTagTip(Guid tagId) ?? throw new InvalidOperationException($"Tag {tagId} was not found."); return await DataModel.GetCommit(tag.TargetCommitId); } - - private CommitMetadata ApplyAssignment(CommitMetadata? commitMetadata, BranchAssignment assignment) - { - var metadata = commitMetadata ?? new CommitMetadata(); - // Resolve to a concrete assignment and mark it, so the commit interceptor treats it as - // already-decided (and does not re-derive or reject it, e.g. ref-lifecycle writes to main - // while a tag is checked out). - var branchId = ResolveBranchId(assignment); - RefMetadata.SetAssignment(metadata, branchId is null ? BranchAssignment.Main : BranchAssignment.ToBranch(branchId.Value)); - return metadata; - } - - private Guid? ResolveBranchId(BranchAssignment assignment) => assignment.Kind switch - { - BranchAssignmentKind.Main => null, - BranchAssignmentKind.Branch => assignment.BranchId, - BranchAssignmentKind.FromCheckout => Checkout switch - { - BranchCheckout branch => branch.BranchId, - TagCheckout when AllowAuthoringOnTagToMain => null, - _ => null - }, - _ => null - }; } diff --git a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs index e8486ea..a01610c 100644 --- a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs +++ b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs @@ -24,10 +24,11 @@ public async Task BranchCheckoutShowsMainPlusBranchCommits() var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); - await _refs.AddChange(_localClientId, SetWord(wordId, "main"), BranchAssignment.Main); + await DataModel.AddChange(_localClientId, SetWord(wordId, "main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); await _refs.CheckoutBranch(branchId); - await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-branch"); @@ -46,10 +47,10 @@ public async Task OtherBranchCommitsStayInvisible() await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); await _refs.CheckoutBranch(featureId); - await _refs.AddChange(_localClientId, SetWord(featureWordId, "feature-word")); + await DataModel.AddChange(_localClientId, SetWord(featureWordId, "feature-word")); await _refs.CheckoutBranch(otherId); - await _refs.AddChange(_localClientId, SetWord(otherWordId, "other-word")); + await DataModel.AddChange(_localClientId, SetWord(otherWordId, "other-word")); await _refs.CheckoutBranch(featureId); (await DataModel.GetLatest(featureWordId))!.Text.Should().Be("feature-word"); @@ -65,13 +66,15 @@ public async Task BranchAndMainCommitsInterleaveByAuthorTime() await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); // t1 main - await _refs.AddChange(_localClientId, SetWord(earlyId, "early-main"), BranchAssignment.Main); + await DataModel.AddChange(_localClientId, SetWord(earlyId, "early-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); await _refs.CheckoutBranch(branchId); // t2 branch (between main commits chronologically once late main is written) - await _refs.AddChange(_localClientId, SetWord(Guid.NewGuid(), "mid-branch")); + await DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "mid-branch")); await _refs.CheckoutMain(); // t3 main - await _refs.AddChange(_localClientId, SetWord(lateId, "late-main"), BranchAssignment.Main); + await DataModel.AddChange(_localClientId, SetWord(lateId, "late-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); await _refs.CheckoutBranch(branchId); var words = DataModel.QueryLatest() diff --git a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs index d9fd539..034409a 100644 --- a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs +++ b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs @@ -27,7 +27,7 @@ public async Task BranchAuthoringIsHiddenOnMainCheckout() await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); await _refs.CheckoutBranch(branchId); - await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); await _refs.CheckoutMain(); var word = await DataModel.GetLatest(wordId); @@ -44,12 +44,12 @@ public async Task BranchAuthoringSetsImmutableBranchMetadata() await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); await _refs.CheckoutBranch(branchId); - var commit = await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); RefMetadata.GetBranchId(commit.Metadata).Should().Be(branchId); // later commits do not rewrite earlier assignment - await _refs.AddChange(_localClientId, SetWord(wordId, "again")); + await DataModel.AddChange(_localClientId, SetWord(wordId, "again")); var stored = await DbContext.Commits.AsNoTracking().SingleAsync(c => c.Id == commit.Id, TestContext.Current.CancellationToken); RefMetadata.GetBranchId(stored.Metadata).Should().Be(branchId); } @@ -66,17 +66,17 @@ public async Task AuthoringOverrideCanForceMainOrAnotherBranch() await _refs.CheckoutBranch(featureId); - var mainCommit = await _refs.AddChange( + var mainCommit = await DataModel.AddChange( _localClientId, SetWord(mainWordId, "forced-main"), - BranchAssignment.Main); + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); RefMetadata.GetBranchId(mainCommit.Metadata).Should().BeNull(); (await DataModel.GetLatest(mainWordId))!.Text.Should().Be("forced-main"); - var otherCommit = await _refs.AddChange( + var otherCommit = await DataModel.AddChange( _localClientId, SetWord(otherWordId, "other-branch"), - BranchAssignment.ToBranch(otherId)); + RefMetadata.SetAssignment(new(), BranchAssignment.ToBranch(otherId))); RefMetadata.GetBranchId(otherCommit.Metadata).Should().Be(otherId); (await DataModel.GetLatest(otherWordId)).Should().BeNull(); } diff --git a/src/SIL.Harmony.Tests/TagCheckoutTests.cs b/src/SIL.Harmony.Tests/TagCheckoutTests.cs index a45450e..ec7c693 100644 --- a/src/SIL.Harmony.Tests/TagCheckoutTests.cs +++ b/src/SIL.Harmony.Tests/TagCheckoutTests.cs @@ -45,7 +45,7 @@ public async Task AuthoringOnTagCheckoutIsRejectedByDefault() await _refs.CreateTag(_localClientId, tagId, "v1", tip.Id); await _refs.CheckoutTag(tagId); - var act = () => _refs.AddChange(_localClientId, SetWord(Guid.NewGuid(), "nope")); + var act = () => DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "nope")); await act.Should().ThrowAsync(); } @@ -90,7 +90,7 @@ public async Task AuthoringOnTagCheckoutWritesToMainWhenConfigured() await refs.CheckoutTag(tagId); var newWordId = Guid.NewGuid(); - await refs.AddChange(withConfig.LocalClientId, withConfig.SetWord(newWordId, "authored-to-main")); + await withConfig.DataModel.AddChange(withConfig.LocalClientId, withConfig.SetWord(newWordId, "authored-to-main")); // The change was authored to main (no branch id), so it is visible on the main checkout. await refs.CheckoutMain(); @@ -103,15 +103,17 @@ public async Task TagOnBranchTipShowsBranchViewAsOfTip() var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); - await _refs.AddChange(_localClientId, SetWord(wordId, "main"), BranchAssignment.Main); + await DataModel.AddChange(_localClientId, SetWord(wordId, "main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); await _refs.CheckoutBranch(branchId); - var branchTip = await _refs.AddChange(_localClientId, SetWord(wordId, "on-branch")); + var branchTip = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); var tagId = Guid.NewGuid(); await _refs.CheckoutMain(); await _refs.CreateTag(_localClientId, tagId, "wip", branchTip.Id); - await _refs.AddChange(_localClientId, SetWord(wordId, "later-main"), BranchAssignment.Main); + await DataModel.AddChange(_localClientId, SetWord(wordId, "later-main"), + RefMetadata.SetAssignment(new(), BranchAssignment.Main)); await _refs.CheckoutTag(tagId); (await DataModel.GetLatest(wordId))!.Text.Should().Be("on-branch"); diff --git a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs index 8a69c32..e9d328d 100644 --- a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs +++ b/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs @@ -46,13 +46,13 @@ public async Task SyncMoveTagRollsForwardActiveCheckout() var tagId = Guid.NewGuid(); await _refs1.CreateTag(_client1.LocalClientId, tagId, "release", first.Id); - await _refs1.SyncWith(_client2.DataModel); + await _client1.DataModel.SyncWith(_client2.DataModel); await _refs2.CheckoutTag(tagId); (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("first"); await _refs1.MoveTag(_client1.LocalClientId, tagId, second.Id); - await _refs2.SyncWith(_client1.DataModel); + await _client2.DataModel.SyncWith(_client1.DataModel); (await _client2.DataModel.GetLatest(wordId))!.Text.Should().Be("second"); } From 85ff0c52c5738f77e9185e518a527aa9c0b6a75f Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 16:00:34 +0700 Subject: [PATCH 16/18] Add CreateBranch and List*; dedupe tag tip resolution Co-authored-by: Cursor --- src/SIL.Harmony.Refs/CheckoutRefsHandler.cs | 10 +--------- src/SIL.Harmony.Refs/RefsDataModel.cs | 16 ++++++++-------- src/SIL.Harmony.Refs/TagTipResolver.cs | 14 ++++++++++++++ src/SIL.Harmony.Tests/CreateBranchTests.cs | 14 ++++++++++---- src/SIL.Harmony.Tests/CreateTagTests.cs | 14 ++++++++++---- 5 files changed, 43 insertions(+), 25 deletions(-) create mode 100644 src/SIL.Harmony.Refs/TagTipResolver.cs diff --git a/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs b/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs index 640402c..e1ef19f 100644 --- a/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs +++ b/src/SIL.Harmony.Refs/CheckoutRefsHandler.cs @@ -1,6 +1,5 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using SIL.Harmony.Refs.Entities; namespace SIL.Harmony.Refs; @@ -49,18 +48,11 @@ public async Task OnCommitsAppliedAsync(IReadOnlyCollection commits) if (filter.Checkout is not TagCheckout tag) return; var dataModel = serviceProvider.GetRequiredService(); - var tip = await ResolveTagTip(dataModel, tag.TagId); + var tip = await TagTipResolver.ResolveTagTip(dataModel, tag.TagId); // Skip rematerialization unless the tag's tip actually moved. if (filter.AsOfTipId == tip.Id) return; filter.SetAsOfTip(tip); await dataModel.RegenerateSnapshots(); } - - private static async Task ResolveTagTip(DataModel dataModel, Guid tagId) - { - var tag = await dataModel.GetLatest(tagId) - ?? throw new InvalidOperationException($"Tag {tagId} was not found."); - return await dataModel.GetCommit(tag.TargetCommitId); - } } diff --git a/src/SIL.Harmony.Refs/RefsDataModel.cs b/src/SIL.Harmony.Refs/RefsDataModel.cs index 13bd29d..5fe2978 100644 --- a/src/SIL.Harmony.Refs/RefsDataModel.cs +++ b/src/SIL.Harmony.Refs/RefsDataModel.cs @@ -43,12 +43,15 @@ public async Task CheckoutBranch(Guid branchId) public async Task CheckoutTag(Guid tagId) { - var tip = await ResolveTagTip(tagId); + var tip = await TagTipResolver.ResolveTagTip(DataModel, tagId); filter.Checkout = RefCheckout.ForTag(tagId); filter.SetAsOfTip(tip); await DataModel.RegenerateSnapshots(); } + public Task CreateBranch(Guid clientId, Guid branchId, string name) => + DataModel.AddChange(clientId, new CreateBranchChange(branchId, name), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + public Task CreateTag(Guid clientId, Guid tagId, string name, Guid targetCommitId) => DataModel.AddChange(clientId, new CreateTagChange(tagId, name, targetCommitId), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); @@ -57,6 +60,10 @@ public Task MoveTag(Guid clientId, Guid tagId, Guid targetCommitId) => DataModel.AddChange(clientId, new MoveTagChange(tagId, targetCommitId), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); + public IAsyncEnumerable ListBranches() => DataModel.QueryLatest(); + + public IAsyncEnumerable ListTags() => DataModel.QueryLatest(); + /// /// Incorporates into main visibility and deletes the branch entity. /// Authored as a main-line commit; materialization expands from the earliest branch commit. @@ -67,11 +74,4 @@ public async Task MergeBranch(Guid clientId, Guid branchId) return await DataModel.AddChange(clientId, new MergeBranchChange(branchId), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); } - - private async Task ResolveTagTip(Guid tagId) - { - var tag = await DataModel.GetLatest(tagId) - ?? throw new InvalidOperationException($"Tag {tagId} was not found."); - return await DataModel.GetCommit(tag.TargetCommitId); - } } diff --git a/src/SIL.Harmony.Refs/TagTipResolver.cs b/src/SIL.Harmony.Refs/TagTipResolver.cs new file mode 100644 index 0000000..ed6a307 --- /dev/null +++ b/src/SIL.Harmony.Refs/TagTipResolver.cs @@ -0,0 +1,14 @@ +using SIL.Harmony.Refs.Entities; + +namespace SIL.Harmony.Refs; + +internal static class TagTipResolver +{ + public static async Task ResolveTagTip(DataModel dataModel, Guid tagId) + { + var tag = await dataModel.GetLatest(tagId) + ?? throw new InvalidOperationException($"Tag {tagId} was not found."); + return await dataModel.GetCommit(tag.TargetCommitId); + } +} + diff --git a/src/SIL.Harmony.Tests/CreateBranchTests.cs b/src/SIL.Harmony.Tests/CreateBranchTests.cs index 87def89..bc4bc0b 100644 --- a/src/SIL.Harmony.Tests/CreateBranchTests.cs +++ b/src/SIL.Harmony.Tests/CreateBranchTests.cs @@ -7,18 +7,22 @@ namespace SIL.Harmony.Tests; public class CreateBranchTests : DataModelTestBase { + private readonly RefsDataModel _refs; + public CreateBranchTests() : base(configure: services => { services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); }) { + _refs = _services.GetRequiredService(); } [Fact] public async Task CanCreateAndReadBranch() { var branchId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature-x")); + await _refs.CreateBranch(_localClientId, branchId, "feature-x"); var branch = await DataModel.GetLatest(branchId); branch.Should().NotBeNull(); @@ -31,8 +35,8 @@ public async Task DuplicateBranchNamesAreAllowed() { var firstId = Guid.NewGuid(); var secondId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(firstId, "dup")); - await DataModel.AddChange(_localClientId, new CreateBranchChange(secondId, "dup")); + await _refs.CreateBranch(_localClientId, firstId, "dup"); + await _refs.CreateBranch(_localClientId, secondId, "dup"); var first = await DataModel.GetLatest(firstId); var second = await DataModel.GetLatest(secondId); @@ -40,7 +44,9 @@ public async Task DuplicateBranchNamesAreAllowed() second!.Name.Should().Be("dup"); first.Id.Should().NotBe(second.Id); - var both = DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken).ToArray(); + var both = _refs.ListBranches() + .ToBlockingEnumerable(TestContext.Current.CancellationToken) + .ToArray(); both.Should().HaveCount(2); } } diff --git a/src/SIL.Harmony.Tests/CreateTagTests.cs b/src/SIL.Harmony.Tests/CreateTagTests.cs index 2de32b9..a21aa42 100644 --- a/src/SIL.Harmony.Tests/CreateTagTests.cs +++ b/src/SIL.Harmony.Tests/CreateTagTests.cs @@ -7,11 +7,15 @@ namespace SIL.Harmony.Tests; public class CreateTagTests : DataModelTestBase { + private readonly RefsDataModel _refs; + public CreateTagTests() : base(configure: services => { services.Configure(config => config.AddHarmonyRefs()); + services.AddHarmonyRefsDataModel(); }) { + _refs = _services.GetRequiredService(); } [Fact] @@ -21,7 +25,7 @@ public async Task CanCreateAndReadTag() var tip = await DataModel.AddChange(_localClientId, SetWord(wordId, "at-tip")); var tagId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateTagChange(tagId, "release", tip.Id)); + await _refs.CreateTag(_localClientId, tagId, "release", tip.Id); var tag = await DataModel.GetLatest(tagId); tag.Should().NotBeNull(); @@ -36,10 +40,12 @@ public async Task DuplicateTagNamesAreAllowed() var tip = await DataModel.AddChange(_localClientId, SetWord(Guid.NewGuid(), "x")); var firstId = Guid.NewGuid(); var secondId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateTagChange(firstId, "dup", tip.Id)); - await DataModel.AddChange(_localClientId, new CreateTagChange(secondId, "dup", tip.Id)); + await _refs.CreateTag(_localClientId, firstId, "dup", tip.Id); + await _refs.CreateTag(_localClientId, secondId, "dup", tip.Id); - var both = DataModel.QueryLatest().ToBlockingEnumerable(TestContext.Current.CancellationToken).ToArray(); + var both = _refs.ListTags() + .ToBlockingEnumerable(TestContext.Current.CancellationToken) + .ToArray(); both.Should().HaveCount(2); both.Select(t => t.Name).Should().AllBe("dup"); } From 8a57cfdc3c221d97d37403e2509bb026787fa2d8 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 16:07:05 +0700 Subject: [PATCH 17/18] Update refs tests to create branches via RefsDataModel. Use the new CreateBranch helper throughout refs-oriented coverage so tests exercise the public refs lifecycle surface instead of constructing branch changes directly. Co-authored-by: Cursor --- src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs | 9 ++++----- src/SIL.Harmony.Tests/MergeBranchTests.cs | 4 ++-- .../ScopedAuthoringMainIsolationTests.cs | 9 ++++----- src/SIL.Harmony.Tests/TagCheckoutTests.cs | 3 +-- src/SIL.Harmony.Tests/TransparentAuthoringTests.cs | 11 +++++------ 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs index a01610c..4005721 100644 --- a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs +++ b/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs @@ -1,5 +1,4 @@ using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Sample.Models; using Microsoft.Extensions.DependencyInjection; @@ -23,7 +22,7 @@ public async Task BranchCheckoutShowsMainPlusBranchCommits() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await DataModel.AddChange(_localClientId, SetWord(wordId, "main"), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); @@ -43,8 +42,8 @@ public async Task OtherBranchCommitsStayInvisible() var otherId = Guid.NewGuid(); var featureWordId = Guid.NewGuid(); var otherWordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); - await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + await _refs.CreateBranch(_localClientId, featureId, "feature"); + await _refs.CreateBranch(_localClientId, otherId, "other"); await _refs.CheckoutBranch(featureId); await DataModel.AddChange(_localClientId, SetWord(featureWordId, "feature-word")); @@ -63,7 +62,7 @@ public async Task BranchAndMainCommitsInterleaveByAuthorTime() var branchId = Guid.NewGuid(); var earlyId = Guid.NewGuid(); var lateId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); // t1 main await DataModel.AddChange(_localClientId, SetWord(earlyId, "early-main"), diff --git a/src/SIL.Harmony.Tests/MergeBranchTests.cs b/src/SIL.Harmony.Tests/MergeBranchTests.cs index b212857..dc65e5a 100644 --- a/src/SIL.Harmony.Tests/MergeBranchTests.cs +++ b/src/SIL.Harmony.Tests/MergeBranchTests.cs @@ -26,7 +26,7 @@ public async Task MergeInterleavesBranchCommitsIntoMainByAuthorTime() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); // main: 1, 2, 3 — branch: A, B, C interleaved by time on the same entity await AddAt(_localClientId, NextDate(), SetWord(wordId, "1")); @@ -67,7 +67,7 @@ public async Task MergeExpandsReplayFromEarliestBranchCommit() var branchId = Guid.NewGuid(); var earlyId = Guid.NewGuid(); var lateId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await _refs.CheckoutBranch(branchId); var earlyBranch = await AddAt(_localClientId, NextDate(), SetWord(earlyId, "early-branch"), diff --git a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs index 034409a..d433846 100644 --- a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs +++ b/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs @@ -1,5 +1,4 @@ using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Sample.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -24,7 +23,7 @@ public async Task BranchAuthoringIsHiddenOnMainCheckout() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await _refs.CheckoutBranch(branchId); await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); @@ -41,7 +40,7 @@ public async Task BranchAuthoringSetsImmutableBranchMetadata() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await _refs.CheckoutBranch(branchId); var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); @@ -61,8 +60,8 @@ public async Task AuthoringOverrideCanForceMainOrAnotherBranch() var otherId = Guid.NewGuid(); var mainWordId = Guid.NewGuid(); var otherWordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); - await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + await _refs.CreateBranch(_localClientId, featureId, "feature"); + await _refs.CreateBranch(_localClientId, otherId, "other"); await _refs.CheckoutBranch(featureId); diff --git a/src/SIL.Harmony.Tests/TagCheckoutTests.cs b/src/SIL.Harmony.Tests/TagCheckoutTests.cs index ec7c693..2f38c14 100644 --- a/src/SIL.Harmony.Tests/TagCheckoutTests.cs +++ b/src/SIL.Harmony.Tests/TagCheckoutTests.cs @@ -1,5 +1,4 @@ using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Sample.Models; using Microsoft.Extensions.DependencyInjection; using RefTag = SIL.Harmony.Refs.Entities.Tag; @@ -102,7 +101,7 @@ public async Task TagOnBranchTipShowsBranchViewAsOfTip() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await DataModel.AddChange(_localClientId, SetWord(wordId, "main"), RefMetadata.SetAssignment(new(), BranchAssignment.Main)); diff --git a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs index f46b05b..ffb1267 100644 --- a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs +++ b/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs @@ -1,5 +1,4 @@ using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Sample.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -29,7 +28,7 @@ public async Task DirectAuthoringOnBranchCheckoutStampsBranchAndIsHiddenOnMain() { var branchId = Guid.NewGuid(); var wordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(branchId, "feature")); + await _refs.CreateBranch(_localClientId, branchId, "feature"); await _refs.CheckoutBranch(branchId); var commit = await DataModel.AddChange(_localClientId, SetWord(wordId, "on-branch")); @@ -97,8 +96,8 @@ public async Task PerCallOverrideForcesMainOrAnotherBranchRegardlessOfCheckout() var otherId = Guid.NewGuid(); var mainWordId = Guid.NewGuid(); var otherWordId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); - await DataModel.AddChange(_localClientId, new CreateBranchChange(otherId, "other")); + await _refs.CreateBranch(_localClientId, featureId, "feature"); + await _refs.CreateBranch(_localClientId, otherId, "other"); await _refs.CheckoutBranch(featureId); @@ -131,7 +130,7 @@ public async Task AllRegisteredInterceptorsAreInvoked() var refs = tb.GetRequiredService(); var branchId = Guid.NewGuid(); - await tb.DataModel.AddChange(tb.LocalClientId, new CreateBranchChange(branchId, "feature")); + await refs.CreateBranch(tb.LocalClientId, branchId, "feature"); await refs.CheckoutBranch(branchId); var commit = await tb.DataModel.AddChange(tb.LocalClientId, tb.SetWord(Guid.NewGuid(), "x")); @@ -145,7 +144,7 @@ public async Task AllRegisteredInterceptorsAreInvoked() public async Task ExplicitAssignmentMarkerIsNotPersisted() { var featureId = Guid.NewGuid(); - await DataModel.AddChange(_localClientId, new CreateBranchChange(featureId, "feature")); + await _refs.CreateBranch(_localClientId, featureId, "feature"); await _refs.CheckoutBranch(featureId); var wordId = Guid.NewGuid(); From 0a5329d1bf6fc3274f4283057135e269d6515e38 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 16 Jul 2026 16:11:30 +0700 Subject: [PATCH 18/18] Reorganize refs tests into dedicated folder Group refs-specific coverage under SIL.Harmony.Tests/Refs so the refs surface and behavior tests are easier to browse without mixing them into the core test root. Co-authored-by: Cursor --- src/SIL.Harmony.Tests/{ => Refs}/BranchCheckoutViewTests.cs | 4 ++-- src/SIL.Harmony.Tests/{ => Refs}/CreateBranchTests.cs | 5 ++--- src/SIL.Harmony.Tests/{ => Refs}/CreateTagTests.cs | 5 ++--- src/SIL.Harmony.Tests/{ => Refs}/MergeBranchTests.cs | 6 +++--- .../{ => Refs}/ScopedAuthoringMainIsolationTests.cs | 6 +++--- src/SIL.Harmony.Tests/{ => Refs}/TagCheckoutTests.cs | 4 ++-- .../{ => Refs}/TagRollForwardListenerTests.cs | 6 +++--- src/SIL.Harmony.Tests/{ => Refs}/TagSyncRollForwardTests.cs | 4 ++-- .../{ => Refs}/TransparentAuthoringTests.cs | 6 +++--- 9 files changed, 22 insertions(+), 24 deletions(-) rename src/SIL.Harmony.Tests/{ => Refs}/BranchCheckoutViewTests.cs (99%) rename src/SIL.Harmony.Tests/{ => Refs}/CreateBranchTests.cs (96%) rename src/SIL.Harmony.Tests/{ => Refs}/CreateTagTests.cs (96%) rename src/SIL.Harmony.Tests/{ => Refs}/MergeBranchTests.cs (99%) rename src/SIL.Harmony.Tests/{ => Refs}/ScopedAuthoringMainIsolationTests.cs (98%) rename src/SIL.Harmony.Tests/{ => Refs}/TagCheckoutTests.cs (99%) rename src/SIL.Harmony.Tests/{ => Refs}/TagRollForwardListenerTests.cs (99%) rename src/SIL.Harmony.Tests/{ => Refs}/TagSyncRollForwardTests.cs (98%) rename src/SIL.Harmony.Tests/{ => Refs}/TransparentAuthoringTests.cs (99%) diff --git a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs b/src/SIL.Harmony.Tests/Refs/BranchCheckoutViewTests.cs similarity index 99% rename from src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs rename to src/SIL.Harmony.Tests/Refs/BranchCheckoutViewTests.cs index 4005721..70c1565 100644 --- a/src/SIL.Harmony.Tests/BranchCheckoutViewTests.cs +++ b/src/SIL.Harmony.Tests/Refs/BranchCheckoutViewTests.cs @@ -1,8 +1,8 @@ +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; using SIL.Harmony.Sample.Models; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class BranchCheckoutViewTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/CreateBranchTests.cs b/src/SIL.Harmony.Tests/Refs/CreateBranchTests.cs similarity index 96% rename from src/SIL.Harmony.Tests/CreateBranchTests.cs rename to src/SIL.Harmony.Tests/Refs/CreateBranchTests.cs index bc4bc0b..c5b7fd2 100644 --- a/src/SIL.Harmony.Tests/CreateBranchTests.cs +++ b/src/SIL.Harmony.Tests/Refs/CreateBranchTests.cs @@ -1,9 +1,8 @@ +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class CreateBranchTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/CreateTagTests.cs b/src/SIL.Harmony.Tests/Refs/CreateTagTests.cs similarity index 96% rename from src/SIL.Harmony.Tests/CreateTagTests.cs rename to src/SIL.Harmony.Tests/Refs/CreateTagTests.cs index a21aa42..5cac3d6 100644 --- a/src/SIL.Harmony.Tests/CreateTagTests.cs +++ b/src/SIL.Harmony.Tests/Refs/CreateTagTests.cs @@ -1,9 +1,8 @@ +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; -using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class CreateTagTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/MergeBranchTests.cs b/src/SIL.Harmony.Tests/Refs/MergeBranchTests.cs similarity index 99% rename from src/SIL.Harmony.Tests/MergeBranchTests.cs rename to src/SIL.Harmony.Tests/Refs/MergeBranchTests.cs index dc65e5a..2a23d08 100644 --- a/src/SIL.Harmony.Tests/MergeBranchTests.cs +++ b/src/SIL.Harmony.Tests/Refs/MergeBranchTests.cs @@ -1,12 +1,12 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Changes; using SIL.Harmony.Refs; using SIL.Harmony.Refs.Changes; using SIL.Harmony.Refs.Entities; using SIL.Harmony.Sample.Models; -using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class MergeBranchTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs b/src/SIL.Harmony.Tests/Refs/ScopedAuthoringMainIsolationTests.cs similarity index 98% rename from src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs rename to src/SIL.Harmony.Tests/Refs/ScopedAuthoringMainIsolationTests.cs index d433846..27654e4 100644 --- a/src/SIL.Harmony.Tests/ScopedAuthoringMainIsolationTests.cs +++ b/src/SIL.Harmony.Tests/Refs/ScopedAuthoringMainIsolationTests.cs @@ -1,9 +1,9 @@ -using SIL.Harmony.Refs; -using SIL.Harmony.Sample.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using SIL.Harmony.Refs; +using SIL.Harmony.Sample.Models; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class ScopedAuthoringMainIsolationTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/TagCheckoutTests.cs b/src/SIL.Harmony.Tests/Refs/TagCheckoutTests.cs similarity index 99% rename from src/SIL.Harmony.Tests/TagCheckoutTests.cs rename to src/SIL.Harmony.Tests/Refs/TagCheckoutTests.cs index 2f38c14..e9f4273 100644 --- a/src/SIL.Harmony.Tests/TagCheckoutTests.cs +++ b/src/SIL.Harmony.Tests/Refs/TagCheckoutTests.cs @@ -1,9 +1,9 @@ +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; using SIL.Harmony.Sample.Models; -using Microsoft.Extensions.DependencyInjection; using RefTag = SIL.Harmony.Refs.Entities.Tag; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class TagCheckoutTests : DataModelTestBase { diff --git a/src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs b/src/SIL.Harmony.Tests/Refs/TagRollForwardListenerTests.cs similarity index 99% rename from src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs rename to src/SIL.Harmony.Tests/Refs/TagRollForwardListenerTests.cs index 84ff005..39d49e0 100644 --- a/src/SIL.Harmony.Tests/TagRollForwardListenerTests.cs +++ b/src/SIL.Harmony.Tests/Refs/TagRollForwardListenerTests.cs @@ -1,10 +1,10 @@ +using Microsoft.Data.Sqlite; +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; using SIL.Harmony.Refs.Changes; using SIL.Harmony.Sample.Models; -using Microsoft.Data.Sqlite; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; /// /// Ticket 05: with refs registered, an active tag checkout rolls forward automatically after any diff --git a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs b/src/SIL.Harmony.Tests/Refs/TagSyncRollForwardTests.cs similarity index 98% rename from src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs rename to src/SIL.Harmony.Tests/Refs/TagSyncRollForwardTests.cs index e9d328d..bcf7a62 100644 --- a/src/SIL.Harmony.Tests/TagSyncRollForwardTests.cs +++ b/src/SIL.Harmony.Tests/Refs/TagSyncRollForwardTests.cs @@ -1,8 +1,8 @@ +using Microsoft.Extensions.DependencyInjection; using SIL.Harmony.Refs; using SIL.Harmony.Sample.Models; -using Microsoft.Extensions.DependencyInjection; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; public class TagSyncRollForwardTests : IAsyncLifetime { diff --git a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs b/src/SIL.Harmony.Tests/Refs/TransparentAuthoringTests.cs similarity index 99% rename from src/SIL.Harmony.Tests/TransparentAuthoringTests.cs rename to src/SIL.Harmony.Tests/Refs/TransparentAuthoringTests.cs index ffb1267..a515264 100644 --- a/src/SIL.Harmony.Tests/TransparentAuthoringTests.cs +++ b/src/SIL.Harmony.Tests/Refs/TransparentAuthoringTests.cs @@ -1,9 +1,9 @@ -using SIL.Harmony.Refs; -using SIL.Harmony.Sample.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; +using SIL.Harmony.Refs; +using SIL.Harmony.Sample.Models; -namespace SIL.Harmony.Tests; +namespace SIL.Harmony.Tests.Refs; /// /// Ticket 04: authoring through directly (no RefsDataModel