diff --git a/src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs b/src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs index 1d6993d4..ded254d6 100644 --- a/src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs +++ b/src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs @@ -4048,23 +4048,24 @@ protected override void SetDefaultValuesAfterInit() } /// - /// Gives an object an opportunity to do any class-specific side-effect work when it has - /// been cloned with DomainServices.CopyObject. In this case, the creation of a MoAffixProcess - /// adds default initial values that are not wanted in the cloned copy, so PostClone() - /// removes them. + /// Removes initialization defaults from this affix process's clone. /// + /// The map from source object identifiers to their clones. public override void PostClone(Dictionary copyMap) { - foreach (var cmObject in copyMap.Values) - { - var clonedProcess = cmObject as IMoAffixProcess; - if (clonedProcess == null) - return; - if (clonedProcess.InputOS.Count > 1) - clonedProcess.InputOS.RemoveAt(0); - if (clonedProcess.OutputOS.Count > 1) - clonedProcess.OutputOS.RemoveAt(0); - } + // The map can contain sibling clones; this source's identifier selects its own clone. + if (!copyMap.TryGetValue(Hvo, out var clone) || !(clone is IMoAffixProcess clonedProcess)) + return; + + // Factory-created clones contain leading defaults in addition to the source content. + var surplusInputs = clonedProcess.InputOS.Count - InputOS.Count; + for (var i = 0; i < surplusInputs; i++) + clonedProcess.InputOS.RemoveAt(0); + + // Removing a default input can also remove its referenced default output. + var surplusOutputs = clonedProcess.OutputOS.Count - OutputOS.Count; + for (var i = 0; i < surplusOutputs; i++) + clonedProcess.OutputOS.RemoveAt(0); } /// /// Gets all of the feature constraints in this rule. diff --git a/tests/SIL.LCModel.Tests/DomainImpl/AffixProcessCloneRoundTripTests.cs b/tests/SIL.LCModel.Tests/DomainImpl/AffixProcessCloneRoundTripTests.cs new file mode 100644 index 00000000..44892b44 --- /dev/null +++ b/tests/SIL.LCModel.Tests/DomainImpl/AffixProcessCloneRoundTripTests.cs @@ -0,0 +1,126 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using System.IO; +using NUnit.Framework; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Infrastructure; +using SIL.TestUtilities; + +namespace SIL.LCModel.DomainImpl +{ + /// + /// Verifies affix-process clones using a file-backed cache. + /// + [TestFixture] + public class AffixProcessCloneRoundTripTests + { + private TemporaryFolder m_projectsFolder; + private ILcmDirectories m_lcmDirectories; + + /// + [SetUp] + public void TestSetup() + { + m_projectsFolder = new TemporaryFolder("AffixProcessCloneRoundTrip" + Guid.NewGuid().ToString("N")); + m_lcmDirectories = new TestLcmDirectories(m_projectsFolder.Path); + } + + /// + [TearDown] + public void TestTeardown() + { + m_projectsFolder.Dispose(); + } + + /// + /// Verifies a moved affix-process clone retains its rule after saving and reloading. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_SurvivesSaveAndReload() + { + var projectName = "AffixProcessCloneRoundTrip" + new Random().Next(1000000); + var path = Path.Combine(m_projectsFolder.Path, LcmFileHelper.GetXmlDataFileName(projectName)); + var projectId = new TestProjectId(BackendProviderType.kXMLWithMemoryOnlyWsMgr, path); + + Guid newEntryGuid; + int expectedInputCount = 0; + int expectedOutputCount = 0; + + using (var cache = LcmCache.CreateCacheWithNewBlankLangProj(projectId, "en", "fr", "en", + new DummyLcmUI(), m_lcmDirectories, new LcmSettings())) + { + ILexEntry entry = null; + ILexSense senseToMove = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", cache.ActionHandlerAccessor, () => + { + var ws = cache.DefaultVernWs; + var entryFactory = cache.ServiceLocator.GetInstance(); + var senseFactory = cache.ServiceLocator.GetInstance(); + + entry = entryFactory.Create(); + var process = cache.ServiceLocator.GetInstance().Create(); + entry.LexemeFormOA = process; + process.Form.set_String(ws, TsStringUtils.MakeString("ed", ws)); + process.MorphTypeRA = cache.ServiceLocator.GetInstance() + .GetObject(MoMorphTypeTags.kguidMorphSuffix); + + // Distinct types make ordering and reference errors observable. + process.InputOS.Clear(); + process.OutputOS.Clear(); + var ctxt = cache.ServiceLocator.GetInstance().Create(); + process.InputOS.Add(ctxt); + var var1 = cache.ServiceLocator.GetInstance().Create(); + process.InputOS.Add(var1); + var copy = cache.ServiceLocator.GetInstance().Create(); + process.OutputOS.Add(copy); + copy.ContentRA = ctxt; + var modify = cache.ServiceLocator.GetInstance().Create(); + process.OutputOS.Add(modify); + modify.ContentRA = var1; + + expectedInputCount = process.InputOS.Count; + expectedOutputCount = process.OutputOS.Count; + + var sense1 = senseFactory.Create(); + entry.SensesOS.Add(sense1); + senseToMove = senseFactory.Create(); + entry.SensesOS.Add(senseToMove); + }); + + entry.MoveSenseToCopy(senseToMove); + newEntryGuid = senseToMove.Entry.Guid; + + cache.ServiceLocator.GetInstance().Save(); + } + + using (var reloaded = LcmCache.CreateCacheFromExistingData(projectId, "en", new DummyLcmUI(), + m_lcmDirectories, new LcmSettings(), new DummyProgressDlg())) + { + var newEntry = (ILexEntry)reloaded.ServiceLocator.GetObject(newEntryGuid); + var clonedProcess = newEntry.LexemeFormOA as IMoAffixProcess; + Assert.That(clonedProcess, Is.Not.Null, "reloaded clone should still be an affix process"); + + Assert.That(clonedProcess.InputOS.Count, Is.EqualTo(expectedInputCount), + "after save/reload, the clone's InputOS should match what was created, with no leaked " + + "default and no real content lost"); + Assert.That(clonedProcess.OutputOS.Count, Is.EqualTo(expectedOutputCount), + "after save/reload, the clone's OutputOS should match what was created, with no leaked " + + "default and no real content lost"); + Assert.That(clonedProcess.InputOS[0].ClassID, Is.EqualTo(PhSimpleContextNCTags.kClassId), + "first input after reload should be the real natural-class context, not a leaked default PhVariable"); + Assert.That(clonedProcess.InputOS[1].ClassID, Is.EqualTo(PhVariableTags.kClassId)); + Assert.That(clonedProcess.OutputOS[0].ClassID, Is.EqualTo(MoCopyFromInputTags.kClassId)); + Assert.That(clonedProcess.OutputOS[1].ClassID, Is.EqualTo(MoModifyFromInputTags.kClassId)); + var copy = (IMoCopyFromInput)clonedProcess.OutputOS[0]; + var modify = (IMoModifyFromInput)clonedProcess.OutputOS[1]; + Assert.That(copy.ContentRA, Is.SameAs(clonedProcess.InputOS[0]), + "reloaded copy mapping should reference the cloned natural-class input"); + Assert.That(modify.ContentRA, Is.SameAs(clonedProcess.InputOS[1]), + "reloaded modify mapping should reference the cloned variable input"); + } + } + } +} diff --git a/tests/SIL.LCModel.Tests/DomainImpl/LexEntryTests.cs b/tests/SIL.LCModel.Tests/DomainImpl/LexEntryTests.cs index 18df7b9a..9b13cb43 100644 --- a/tests/SIL.LCModel.Tests/DomainImpl/LexEntryTests.cs +++ b/tests/SIL.LCModel.Tests/DomainImpl/LexEntryTests.cs @@ -476,6 +476,306 @@ private ILexEntry MakeAffixProcessEntry(string form, Guid morphType) return entry; } + private IMoAffixProcess AddAffixProcessAllomorph(ILexEntry entry, string form) + { + var ws = Cache.DefaultVernWs; + var process = Cache.ServiceLocator.GetInstance().Create(); + entry.AlternateFormsOS.Add(process); + process.Form.set_String(ws, TsStringUtils.MakeString(form, ws)); + process.MorphTypeRA = Cache.ServiceLocator.GetInstance().GetObject(MoMorphTypeTags.kguidMorphSuffix); + return process; + } + + private IMoStemAllomorph AddStemAllomorph(ILexEntry entry, string form) + { + var ws = Cache.DefaultVernWs; + var stem = Cache.ServiceLocator.GetInstance().Create(); + entry.AlternateFormsOS.Add(stem); + stem.Form.set_String(ws, TsStringUtils.MakeString(form, ws)); + stem.MorphTypeRA = Cache.ServiceLocator.GetInstance().GetObject(MoMorphTypeTags.kguidMorphStem); + return stem; + } + + /// + /// Replaces generated defaults with distinguishable rule content. + /// + private void MakeNonTrivialRuleContent(IMoAffixProcess process) + { + process.InputOS.Clear(); + process.OutputOS.Clear(); + + var ctxt = Cache.ServiceLocator.GetInstance().Create(); + process.InputOS.Add(ctxt); + var var1 = Cache.ServiceLocator.GetInstance().Create(); + process.InputOS.Add(var1); + + var copy = Cache.ServiceLocator.GetInstance().Create(); + process.OutputOS.Add(copy); + copy.ContentRA = ctxt; + var modify = Cache.ServiceLocator.GetInstance().Create(); + process.OutputOS.Add(modify); + modify.ContentRA = var1; + } + + /// + /// Verifies that a rule clone preserves item order and mapping references. + /// + private void AssertNonTrivialRuleClonedCorrectly(IMoAffixProcess clone) + { + Assert.That(clone.InputOS.Count, Is.EqualTo(2), + "clone should have exactly the two real inputs: no leaked default, no lost real content"); + Assert.That(clone.InputOS[0].ClassID, Is.EqualTo(PhSimpleContextNCTags.kClassId), + "position 0 must be the real natural-class context -- not a leaked default PhVariable, " + + "and not some other real item shifted into this slot by removing the wrong element"); + Assert.That(clone.InputOS[1].ClassID, Is.EqualTo(PhVariableTags.kClassId), + "position 1 must be the real variable"); + + Assert.That(clone.OutputOS.Count, Is.EqualTo(2), + "clone should have exactly the two real outputs: no leaked default, no lost real content"); + Assert.That(clone.OutputOS[0].ClassID, Is.EqualTo(MoCopyFromInputTags.kClassId), + "position 0 must be the real copy-mapping"); + Assert.That(clone.OutputOS[1].ClassID, Is.EqualTo(MoModifyFromInputTags.kClassId), + "position 1 must be the real modify-mapping"); + + var copy = (IMoCopyFromInput)clone.OutputOS[0]; + var modify = (IMoModifyFromInput)clone.OutputOS[1]; + Assert.That(copy.ContentRA, Is.EqualTo(clone.InputOS[0]), + "the copy-mapping's ContentRA must be exactly this clone's own natural-class input object, " + + "identified by reference, not merely 'some' member of InputOS"); + Assert.That(modify.ContentRA, Is.EqualTo(clone.InputOS[1]), + "the modify-mapping's ContentRA must be exactly this clone's own variable input object, " + + "identified by reference, not merely 'some' member of InputOS"); + } + + /// + /// Verifies moving a sense preserves a non-trivial lexeme-form affix process. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_PreservesNonTrivialRule_SingleAllomorph() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceProcess = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeAffixProcessEntry("ed", MoMorphTypeTags.kguidMorphSuffix); + sourceProcess = (IMoAffixProcess)entry.LexemeFormOA; + MakeNonTrivialRuleContent(sourceProcess); + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + }); + + entry.MoveSenseToCopy(senseToMove); + + var newEntry = senseToMove.Entry; + Assert.That(newEntry, Is.Not.EqualTo(entry)); + var clonedProcess = newEntry.LexemeFormOA as IMoAffixProcess; + Assert.That(clonedProcess, Is.Not.Null, "clone should still be an affix process"); + + AssertNonTrivialRuleClonedCorrectly(clonedProcess); + } + + /// + /// Verifies moving a sense preserves an affix process preceded by a stem allomorph. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_StemAllomorphBeforeProcess_PreservesRealContent() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceProcess = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeEntry(); + AddStemAllomorph(entry, "stemA"); + sourceProcess = AddAffixProcessAllomorph(entry, "ed"); + MakeNonTrivialRuleContent(sourceProcess); + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + }); + + entry.MoveSenseToCopy(senseToMove); + + var newEntry = senseToMove.Entry; + Assert.That(newEntry.AlternateFormsOS.Count, Is.EqualTo(2), "both allomorphs should have been cloned"); + var clonedProcess = newEntry.AlternateFormsOS[1] as IMoAffixProcess; + Assert.That(clonedProcess, Is.Not.Null, "second allomorph clone should still be an affix process"); + + AssertNonTrivialRuleClonedCorrectly(clonedProcess); + } + + /// + /// Verifies moving a sense preserves two affix-process allomorphs cloned together. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_TwoProcessAllomorphs_NeitherLosesRealContent() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceA = null; + IMoAffixProcess sourceB = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeEntry(); + sourceA = AddAffixProcessAllomorph(entry, "edA"); + MakeNonTrivialRuleContent(sourceA); + sourceB = AddAffixProcessAllomorph(entry, "edB"); + MakeNonTrivialRuleContent(sourceB); + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + }); + + entry.MoveSenseToCopy(senseToMove); + + var newEntry = senseToMove.Entry; + Assert.That(newEntry.AlternateFormsOS.Count, Is.EqualTo(2), "both allomorphs should have been cloned"); + var clonedA = newEntry.AlternateFormsOS[0] as IMoAffixProcess; + var clonedB = newEntry.AlternateFormsOS[1] as IMoAffixProcess; + Assert.That(clonedA, Is.Not.Null); + Assert.That(clonedB, Is.Not.Null); + + AssertNonTrivialRuleClonedCorrectly(clonedA); + AssertNonTrivialRuleClonedCorrectly(clonedB); + } + + /// + /// Verifies cloned rule mappings refer to inputs owned by the same clone. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_ContentRAPointsIntoOwnClonesInputOS() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceA = null; + IMoAffixProcess sourceB = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeEntry(); + sourceA = AddAffixProcessAllomorph(entry, "edA"); + MakeNonTrivialRuleContent(sourceA); + sourceB = AddAffixProcessAllomorph(entry, "edB"); + MakeNonTrivialRuleContent(sourceB); + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + }); + + entry.MoveSenseToCopy(senseToMove); + + var newEntry = senseToMove.Entry; + var clonedA = (IMoAffixProcess)newEntry.AlternateFormsOS[0]; + var clonedB = (IMoAffixProcess)newEntry.AlternateFormsOS[1]; + + AssertNonTrivialRuleClonedCorrectly(clonedA); + AssertNonTrivialRuleClonedCorrectly(clonedB); + + foreach (var clone in new[] { clonedA, clonedB }) + { + foreach (var mapping in clone.OutputOS) + { + IPhContextOrVar content = null; + if (mapping is IMoCopyFromInput cfi) + content = cfi.ContentRA; + else if (mapping is IMoModifyFromInput mfi) + content = mfi.ContentRA; + if (content == null) + continue; + + Assert.That(clone.InputOS.Contains(content), Is.True, + "a rule-mapping's ContentRA must point into its OWN clone's InputOS"); + Assert.That(sourceA.InputOS.Contains(content), Is.False, + "a cloned rule-mapping's ContentRA must never point into the source's InputOS"); + Assert.That(sourceB.InputOS.Contains(content), Is.False, + "a cloned rule-mapping's ContentRA must never point into the source's InputOS"); + } + } + } + + /// + /// Verifies a moved sense preserves the affix process used by its morph bundle. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_ViaMorphBundleFailoverPath_PreservesNonTrivialRule() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceProcess = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeEntry(); + sourceProcess = Cache.ServiceLocator.GetInstance().Create(); + entry.LexemeFormOA = sourceProcess; + // A blank form exercises creation of a matching allomorph for the morph bundle. + sourceProcess.MorphTypeRA = Cache.ServiceLocator.GetInstance() + .GetObject(MoMorphTypeTags.kguidMorphSuffix); + MakeNonTrivialRuleContent(sourceProcess); + + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + + var wf = MakeWordform("ted"); + MakeAnalysis(wf, senseToMove); + }); + + entry.MoveSenseToCopy(senseToMove); + + try + { + var newEntry = senseToMove.Entry; + Assert.That(newEntry, Is.Not.EqualTo(entry)); + + var mb = (IWfiMorphBundle)senseToMove.ReferringObjects.First(o => o is IWfiMorphBundle); + var failoverClone = mb.MorphRA as IMoAffixProcess; + Assert.That(failoverClone, Is.Not.Null, + "the blank-Form failover should have created a second clone of the affix process"); + Assert.That(newEntry.AlternateFormsOS.Contains(failoverClone), Is.True, + "the failover clone should be a real allomorph on the new entry"); + Assert.That(failoverClone, Is.Not.EqualTo(newEntry.LexemeFormOA), + "the failover path creates a SECOND, independent clone, distinct from the one made " + + "by the normal LexemeFormOA clone path"); + + AssertNonTrivialRuleClonedCorrectly(failoverClone); + } + finally + { + // Commit because undoing the morph-bundle references throws from + // LcmAtomicRefPropertyChanged.Undo during teardown. + Cache.ActionHandlerAccessor.Commit(); + } + } + + /// + /// Verifies moving a sense preserves an affix process with empty rule content. + /// + [Test] + public void MoveSenseToCopy_AffixProcessClone_ZeroRealContent_NoLeakedDefault() + { + ILexEntry entry = null; + ILexSense senseToMove = null; + IMoAffixProcess sourceProcess = null; + UndoableUnitOfWorkHelper.Do("doit", "undoit", Cache.ActionHandlerAccessor, () => + { + entry = MakeAffixProcessEntry("ed", MoMorphTypeTags.kguidMorphSuffix); + sourceProcess = (IMoAffixProcess)entry.LexemeFormOA; + sourceProcess.InputOS.Clear(); + sourceProcess.OutputOS.Clear(); + MakeSense(entry, "stay"); + senseToMove = MakeSense(entry, "move"); + }); + + Assert.That(sourceProcess.InputOS.Count, Is.EqualTo(0)); + Assert.That(sourceProcess.OutputOS.Count, Is.EqualTo(0)); + + entry.MoveSenseToCopy(senseToMove); + + var newEntry = senseToMove.Entry; + var clonedProcess = newEntry.LexemeFormOA as IMoAffixProcess; + Assert.That(clonedProcess, Is.Not.Null); + Assert.That(clonedProcess.InputOS.Count, Is.EqualTo(0), + "a source with zero real inputs should clone to zero inputs, not one leaked default"); + Assert.That(clonedProcess.OutputOS.Count, Is.EqualTo(0), + "a source with zero real outputs should clone to zero outputs, not one leaked default"); + } + /// /// Test PrimaryEntryRoots and the closely related NonTrivialEntryRoots. ///