From 24f77ce40d1439e77fa766dbaf63a9b45861305b Mon Sep 17 00:00:00 2001 From: John Lambert Date: Mon, 12 Oct 2026 07:22:28 -0400 Subject: [PATCH 1/2] Test that a memory-only copy of a partly activated cache keeps HVOs unique Copying a cache whose objects are only partly activated currently carries the source cache's HVOs onto dormant objects in the target, where the target's own HVO counter hands the same numbers out again. This test fails until the copy gives each object an HVO from the target cache. Co-Authored-By: Claude Opus 5.5 --- .../Impl/PersistingLayerTests.BEPPortTests.cs | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/SIL.LCModel.Tests/Infrastructure/Impl/PersistingLayerTests.BEPPortTests.cs b/tests/SIL.LCModel.Tests/Infrastructure/Impl/PersistingLayerTests.BEPPortTests.cs index df224a49..55b02b4b 100644 --- a/tests/SIL.LCModel.Tests/Infrastructure/Impl/PersistingLayerTests.BEPPortTests.cs +++ b/tests/SIL.LCModel.Tests/Infrastructure/Impl/PersistingLayerTests.BEPPortTests.cs @@ -250,6 +250,77 @@ public void PortAllBEPsTestsUsingAnUnopenedSource( } sourceGuids.Clear(); } + + /// + /// A copy taken from a source whose objects are only partly activated must still give every + /// object in the target an HVO of its own, including objects created in the target afterwards. + /// + [Test] + public void PortFromPartlyActivatedSourceGivesMemoryOnlyTargetUniqueHvos() + { + var sourceProjectId = new TestProjectId(BackendProviderType.kXMLWithMemoryOnlyWsMgr, + GenerateBackendStartupParameters(false, BackendProviderType.kXMLWithMemoryOnlyWsMgr).ProjectId.Path); + using (var newCache = LcmCache.CreateCacheWithNewBlankLangProj(sourceProjectId, "en", "fr", "en", new DummyLcmUI(), + m_lcmDirectories, new LcmSettings())) + { + newCache.ServiceLocator.GetInstance().Save(); + } + + using (var sourceCache = LcmCache.CreateCacheFromExistingData(sourceProjectId, "en", new DummyLcmUI(), + m_lcmDirectories, new LcmSettings(), new DummyProgressDlg())) + { + var dormant = IdentityMapOf(sourceCache).AllObjectsOrSurrogates().Where(item => !item.HasObject).ToList(); + Assert.That(dormant.Count, Is.GreaterThan(10), "The source should start mostly dormant."); + // Activate every other dormant object so the copy sees both activated and dormant sources. + for (var i = 0; i < dormant.Count; i += 2) + Assert.That(dormant[i].Object, Is.Not.Null); + + var targetProjectId = new TestProjectId(BackendProviderType.kMemoryOnly, null); + using (var targetCache = LcmCache.CreateCacheCopy(targetProjectId, "en", new DummyLcmUI(), + m_lcmDirectories, new LcmSettings(), sourceCache)) + { + AssertHvosAreUnique(targetCache, "after the copy"); + + var entryFactory = targetCache.ServiceLocator.GetInstance(); + NonUndoableUnitOfWorkHelper.Do(targetCache.ActionHandlerAccessor, () => + { + for (var i = 0; i < dormant.Count; i++) + entryFactory.Create(); + }); + AssertHvosAreUnique(targetCache, "after creating objects in the target"); + + var repository = targetCache.ServiceLocator.GetInstance(); + foreach (var pair in HvosOf(targetCache).ToList()) + Assert.That(repository.GetObject(pair.Value).Guid, Is.EqualTo(pair.Key), "Wrong object for HVO " + pair.Value); + } + } + } + + private static IdentityMap IdentityMapOf(LcmCache cache) + { + return ((IServiceLocatorInternal)cache.ServiceLocator).IdentityMap; + } + + /// + /// The HVO of every object in the cache that has one, read without activating any dormant object. + /// + private static IEnumerable> HvosOf(LcmCache cache) + { + foreach (var item in IdentityMapOf(cache).AllObjectsOrSurrogates()) + { + if (item.HasObject) + yield return new KeyValuePair(item.Id.Guid, item.Object.Hvo); + else if (item.Id is CmObjectIdWithHvo idWithHvo) + yield return new KeyValuePair(item.Id.Guid, idWithHvo.Hvo); + } + } + + private static void AssertHvosAreUnique(LcmCache cache, string when) + { + var shared = HvosOf(cache).GroupBy(pair => pair.Value).Where(group => group.Count() > 1).ToList(); + Assert.That(shared.Count, Is.Zero, "HVOs shared by more than one object " + when + ": " + + string.Join(", ", shared.Take(5).Select(group => group.Key + " (" + group.Count() + " objects)"))); + } #endregion } } From fac53625d05b5533ab38b35c066c9b5c6cb574f6 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Mon, 12 Oct 2026 07:26:00 -0400 Subject: [PATCH 2/2] Give ported objects an HVO from the target cache When a cache is copied with InitializeFromSource, dormant source objects kept the HVO their CmObjectIdWithHvo had in the source cache. The target never reserved those numbers, so its own HVO counter handed them out again to activated or newly created objects, and GetObject(hvo) could return the wrong object. The cross-cache surrogate constructor now always takes a new id from the target identity map, as already happened for activated sources. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/SIL.LCModel/Infrastructure/Impl/CmObjectSurrogate.cs | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a172e5d..c74513a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- [SIL.LCModel] A cache copied with `InitializeFromSource`/`CreateCacheCopy` gives every ported object an HVO from the target cache, so objects activated or created in the copy no longer share an HVO with a dormant ported object and `GetObject(hvo)` no longer returns the wrong object - [SIL.LCModel] BaseStyleInfo now loads default font features (ktptFontVariations) from style rules, fixing font features set as style defaults being dropped on reload (LT-22351) - [SIL.LCModel] BaseStyleInfo no longer clobbers ws-specific font overrides inherited from a based-on style with the default font info's merely-inherited values; only explicitly-set default properties are propagated to ws overrides (LT-22351) - [SIL.LCModel] BulletInfo now decodes all properties of an encoded bullet font info instead of stopping at the first string property, fixing bullet font features (ktptFontVariations) being lost when a bullet font name was also set (LT-22351) diff --git a/src/SIL.LCModel/Infrastructure/Impl/CmObjectSurrogate.cs b/src/SIL.LCModel/Infrastructure/Impl/CmObjectSurrogate.cs index 8d1fefb6..dedcea53 100644 --- a/src/SIL.LCModel/Infrastructure/Impl/CmObjectSurrogate.cs +++ b/src/SIL.LCModel/Infrastructure/Impl/CmObjectSurrogate.cs @@ -185,9 +185,8 @@ internal CmObjectSurrogate(LcmCache cache, ICmObjectSurrogate sourceSurrogate) RawXmlBytes = surr.RawXmlBytes; else Xml = sourceSurrogate.XML; - ICmObjectId objId = surr.Id; - m_guid = objId is CmObjectIdWithHvo ? objId - : ((IServiceLocatorInternal)cache.ServiceLocator).IdentityMap.CreateObjectIdWithHvo(objId.Guid); + // An HVO from the source cache means nothing in this one, so always take one from this cache. + m_guid = ((IServiceLocatorInternal)cache.ServiceLocator).IdentityMap.CreateObjectIdWithHvo(surr.Id.Guid); SetClassName(surr.m_classname); }