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); } 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 } }