Follow-up from the test-suite review (batches merged as #92–#96). This is the most intricate merge logic in the engine and currently has no coverage.
Gap
SnapshotWorker.MarkDeleted: when an entity is deleted, references to it are removed from referencing snapshots (RemoveReference), and if that removal itself deletes another entity the method recurses (deletedByRemoveRef → recursive MarkDeleted). The recursion guard comment (L142–146) implies past loop bugs.
|
private async ValueTask MarkDeleted(Guid deletedEntityId, ChangeContext context) |
|
{ |
|
// Including deleted shouldn't be necessary, because change objects are responsible for not adding references to deleted entities. |
|
// But maybe it's a good fallback. |
|
var toRemoveRefFrom = await GetSnapshotsReferencing(deletedEntityId, true) |
|
.ToArrayAsync(); |
|
|
|
var commit = context.Commit; |
|
foreach (var snapshot in toRemoveRefFrom) |
|
{ |
|
var updatedEntry = snapshot.Entity.Copy(); |
|
var wasDeleted = updatedEntry.DeletedAt.HasValue; |
|
|
|
updatedEntry.RemoveReference(deletedEntityId, commit); |
|
var deletedByRemoveRef = !wasDeleted && updatedEntry.DeletedAt.HasValue; |
|
|
|
await GenerateSnapshotForEntity(updatedEntry, snapshot, context); |
|
|
|
//we need to do this after we add the snapshot above otherwise we might get stuck in a loop of deletions |
|
if (deletedByRemoveRef) |
|
{ |
|
await MarkDeleted(updatedEntry.Id, context); |
|
} |
|
} |
|
} |
No test:
- deletes a referenced entity (e.g. a
Tag referenced by a Word/WordTag) and asserts the reference is scrubbed from the referrer;
- exercises the cascade where removing a reference deletes the referrer, driving the recursion.
Suggested tests
DeletingReferencedEntityRemovesReferenceFromReferrers
DeletingEntityCascadesWhenRemovalDeletesReferrer
Why it matters
Reference scrubbing and cascade deletion are core CRDT-merge behaviors; a regression here silently corrupts entity relationships. The recursion guard is untested against the loop it exists to prevent.
Follow-up from the test-suite review (batches merged as #92–#96). This is the most intricate merge logic in the engine and currently has no coverage.
Gap
SnapshotWorker.MarkDeleted: when an entity is deleted, references to it are removed from referencing snapshots (RemoveReference), and if that removal itself deletes another entity the method recurses (deletedByRemoveRef→ recursiveMarkDeleted). The recursion guard comment (L142–146) implies past loop bugs.harmony/src/SIL.Harmony/SnapshotWorker.cs
Lines 124 to 148 in 03f609f
No test:
Tagreferenced by aWord/WordTag) and asserts the reference is scrubbed from the referrer;Suggested tests
DeletingReferencedEntityRemovesReferenceFromReferrersDeletingEntityCascadesWhenRemovalDeletesReferrerWhy it matters
Reference scrubbing and cascade deletion are core CRDT-merge behaviors; a regression here silently corrupts entity relationships. The recursion guard is untested against the loop it exists to prevent.