From 5ba819698974204938f50e49ec6f4ecd4aed7f9a Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Thu, 1 Oct 2026 12:18:21 +0700 Subject: [PATCH 1/2] Fix incorrect handling of fresh clone If a project's Mercurial repo has been deleted from webwork and LfMerge clones a fresh copy of the LexBox repo, the fresh copy may be at a different Mercurial revision than the one that LfMerge last synced from. If that happens, then our modifiedDate handling, which only checks if the FLEx modified date is *different* from the Mongo copy, can incorrectly flag a copy modified in FLEx as having been modified in LF, and therefore overwrite FLEx's *newer* data with LF's *older* data. This has already happened to one project, and I had to roll back the Mercurial revision that LfMerge made, because it had replaced several hundred FLEx entries (modified in 2026) with stale LF entries last modified in 2024. The code in this commit was created by Claude Opus 5.5 after examining the state of that particular project; I (Robin Munn) will be making a few changes in later commits, mostly to reduce the verbosity of the comments that Claude always includes. But the logic is correct, and should prevent this problem from happening again. --- .../Lcm/TransferMongoToLcmActionTests.cs | 108 +++++++++++++++++- src/LfMerge.Core.Tests/TestDoubles.cs | 17 ++- .../ConvertMongoToLcmLexicon.cs | 84 +++++++++++++- 3 files changed, 206 insertions(+), 3 deletions(-) diff --git a/src/LfMerge.Core.Tests/Lcm/TransferMongoToLcmActionTests.cs b/src/LfMerge.Core.Tests/Lcm/TransferMongoToLcmActionTests.cs index c6aee22c..0013eca5 100644 --- a/src/LfMerge.Core.Tests/Lcm/TransferMongoToLcmActionTests.cs +++ b/src/LfMerge.Core.Tests/Lcm/TransferMongoToLcmActionTests.cs @@ -1,7 +1,8 @@ -// Copyright (c) 2016-2018 SIL International +// Copyright (c) 2016-2018 SIL International // This software is licensed under the MIT license (http://opensource.org/licenses/MIT) using LfMerge.Core.Actions.Infrastructure; using LfMerge.Core.DataConverters; +using LfMerge.Core.FieldWorks; using LfMerge.Core.LanguageForge.Model; using LfMergeBridge.LfMergeModel; using MongoDB.Bson; @@ -12,6 +13,7 @@ using System.Linq; using SIL.LCModel; using SIL.LCModel.Core.KernelInterfaces; +using SIL.LCModel.Infrastructure; namespace LfMerge.Core.Tests.Lcm { @@ -639,6 +641,110 @@ public void Action_RunTwiceWithTheSameEntryDeletedEachTime_ShouldCountJustOneDel Is.EqualTo("Language Forge S/R")); } + // The tests below are what a fresh clone looks like: LfMerge's working copy already holds work + // FLEx users did after LF's last sync, which LF's copy in Mongo knows nothing about. + + private static void EditInFieldWorks(FwProject project, ILexEntry entry, string newLexeme) + { + UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("undo", "redo", project.Cache.ActionHandlerAccessor, () => + { + entry.LexemeFormOA.Form.SetVernacularDefaultWritingSystem(newLexeme); + entry.DateModified = DateTime.Now; + }); + } + + [Test] + public void Action_EntryEditedOnlyInFieldWorksSinceLastSync_KeepsFieldWorksVersion() + { + // Setup + var lfProj = _lfProj; + SutLcmToMongo.Run(lfProj); + FwProject fwProject = lfProj.FieldWorksProject; + ILexEntry editedInFlex = LcmTestHelper.GetEntry(fwProject, Guid.Parse(TestEntryGuidStr)); + EditInFieldWorks(fwProject, editedInFlex, "edited in FLEx after LF's last sync"); + + string vernacularWS = fwProject.Cache.LanguageProject.DefaultVernacularWritingSystem.Id; + LfLexEntry editedInLf = _conn.GetLfLexEntryByGuid(lfProj, Guid.Parse(KenEntryGuidStr)); + editedInLf.Lexeme = LfMultiText.FromSingleStringMapping(vernacularWS, "edited in LF after the last sync"); + editedInLf.AuthorInfo.ModifiedDate = DateTime.UtcNow; + _conn.UpdateMockLfLexEntry(editedInLf); + + // Exercise + SutMongoToLcm.Run(lfProj); + + // Verify + Assert.That(editedInFlex.LexemeFormOA.Form.VernacularDefaultWritingSystem.Text, + Is.EqualTo("edited in FLEx after LF's last sync")); + ILexEntry ken = LcmTestHelper.GetEntry(fwProject, Guid.Parse(KenEntryGuidStr)); + Assert.That(ken.LexemeFormOA.Form.VernacularDefaultWritingSystem.Text, + Is.EqualTo("edited in LF after the last sync")); + Assert.That(_counts.Added, Is.EqualTo(0)); + Assert.That(_counts.Modified, Is.EqualTo(1)); + Assert.That(_counts.Deleted, Is.EqualTo(0)); + } + + [Test] + public void Action_EntryDeletedInFieldWorksSinceLastSync_IsNotRecreated() + { + // Setup + var lfProj = _lfProj; + SutLcmToMongo.Run(lfProj); + FwProject fwProject = lfProj.FieldWorksProject; + Guid kenGuid = Guid.Parse(KenEntryGuidStr); + LcmTestHelper.DeleteEntry(fwProject, kenGuid); + + // Exercise + SutMongoToLcm.Run(lfProj); + + // Verify + ILexEntry ken; + Assert.That(fwProject.ServiceLocator.GetInstance().TryGetObject(kenGuid, out ken), Is.False); + Assert.That(_counts.Added, Is.EqualTo(0)); + Assert.That(_counts.Modified, Is.EqualTo(0)); + Assert.That(_counts.Deleted, Is.EqualTo(0)); + } + + [Test] + public void Action_LfDeletionOlderThanLastSync_DoesNotDeleteTheFieldWorksEntry() + { + // Setup: an entry LF deleted before the last sync, which FieldWorks has since restored + var lfProj = _lfProj; + SutLcmToMongo.Run(lfProj); + Guid entryGuid = Guid.Parse(TestEntryGuidStr); + LfLexEntry entry = _conn.GetLfLexEntryByGuid(lfProj, entryGuid); + entry.IsDeleted = true; + entry.DateModified = _conn.GetLastSyncedDate(lfProj).Value.AddDays(-1); + _conn.UpdateRecord(lfProj, entry); + + // Exercise + SutMongoToLcm.Run(lfProj); + + // Verify + ILexEntry stillThere; + Assert.That(lfProj.FieldWorksProject.ServiceLocator.GetInstance().TryGetObject(entryGuid, out stillThere), Is.True); + Assert.That(_counts.Deleted, Is.EqualTo(0)); + } + + [Test] + public void Action_NeverSyncedProjectWhoseEntriesLfMergeWrote_KeepsFieldWorksVersion() + { + // Setup: an initial clone whose transfer to Mongo never got as far as recording the sync + var lfProj = _lfProj; + SutLcmToMongo.Run(lfProj); + _conn.SetLastSyncedDate(lfProj, MagicValues.UnixEpoch); + FwProject fwProject = lfProj.FieldWorksProject; + ILexEntry editedInFlex = LcmTestHelper.GetEntry(fwProject, Guid.Parse(TestEntryGuidStr)); + EditInFieldWorks(fwProject, editedInFlex, "edited in FLEx after the initial clone"); + + // Exercise + SutMongoToLcm.Run(lfProj); + + // Verify + Assert.That(editedInFlex.LexemeFormOA.Form.VernacularDefaultWritingSystem.Text, + Is.EqualTo("edited in FLEx after the initial clone")); + Assert.That(_counts.Modified, Is.EqualTo(0)); + } + // TODO: Move custom field tests to their own test class, and move these helper functions with them public void AddCustomFieldToSense(LfSense sense, string fieldName, BsonValue fieldValue) { diff --git a/src/LfMerge.Core.Tests/TestDoubles.cs b/src/LfMerge.Core.Tests/TestDoubles.cs index c22e5bfb..0c771ade 100644 --- a/src/LfMerge.Core.Tests/TestDoubles.cs +++ b/src/LfMerge.Core.Tests/TestDoubles.cs @@ -188,10 +188,22 @@ public void UpdateMockLfLexEntry(BsonDocument mockData) UpdateMockLfLexEntry(data); } + // The user SampleData's entries were last modified by + private static readonly ObjectId LfUserRef = ObjectId.Parse("561b666c0f87096a35c3cf2d"); + + /// + /// Store an entry the way an LF user's edit does: LF's MapperModel::write stamps DateModified, + /// and LexEntryCommands::updateEntry records who made the edit. LfMerge's own writes go + /// through UpdateRecord, which does neither. + /// public void UpdateMockLfLexEntry(LfLexEntry mockData) { Guid guid = mockData.Guid ?? Guid.Empty; - _storedLfLexEntries[guid] = DeepCopy(mockData); + var stored = DeepCopy(mockData); + stored.DateModified = DateTime.UtcNow; + if (stored.AuthorInfo != null && stored.AuthorInfo.ModifiedByUserRef == null) + stored.AuthorInfo.ModifiedByUserRef = LfUserRef; + _storedLfLexEntries[guid] = stored; } public void UpdateMockOptionList(BsonDocument mockData) @@ -290,6 +302,9 @@ public bool RemoveRecord(ILfProject project, Guid guid) public bool SetLastSyncedDate(ILfProject project, DateTime? newSyncedDate) { + // Mongo keeps dates to the millisecond, as DeepCopy does for the entries' dates + if (newSyncedDate != null) + newSyncedDate = newSyncedDate.Value.AddTicks(-(newSyncedDate.Value.Ticks % TimeSpan.TicksPerMillisecond)); _storedLastSyncDate[project.ProjectCode] = newSyncedDate; // Also update on the fake project record, since EnsureCloneAction looks at the project record to check its initial-clone logic if (_projectRecordFactory != null) diff --git a/src/LfMerge.Core/DataConverters/ConvertMongoToLcmLexicon.cs b/src/LfMerge.Core/DataConverters/ConvertMongoToLcmLexicon.cs index 2371695a..70d172fb 100644 --- a/src/LfMerge.Core/DataConverters/ConvertMongoToLcmLexicon.cs +++ b/src/LfMerge.Core/DataConverters/ConvertMongoToLcmLexicon.cs @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2018 SIL International +// Copyright (c) 2016-2018 SIL International // This software is licensed under the MIT license (http://opensource.org/licenses/MIT) using System; using System.Collections.Generic; @@ -40,6 +40,11 @@ public class ConvertMongoToLcmLexicon private int _wsEn; private ConvertMongoToLcmCustomField _convertCustomField; + // Entries where LF's copy differs from FieldWorks but no LF user has touched it since the + // last sync, so FieldWorks holds the newer version (see EditedInLfSinceLastSync) + private int _changedOnlyInFieldWorks; + private int _absentOnlyFromFieldWorks; + // Shorter names to use in this class since MagicStrings.LfOptionListCodeForGrammaticalInfo // (etc.) are real mouthfuls private const string GrammarListCode = MagicStrings.LfOptionListCodeForGrammaticalInfo; @@ -143,6 +148,8 @@ public ConversionError RunConversion() _convertCustomField = new ConvertMongoToLcmCustomField(Cache, ServiceLocator, Logger, _wsEn); IEnumerable lexicon = GetLexicon(LfProject); + _changedOnlyInFieldWorks = 0; + _absentOnlyFromFieldWorks = 0; UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("undo", "redo", Cache.ActionHandlerAccessor, () => { #if false // Once we allow LanguageForge to create optionlist items with "canonical" values (parts of speech, semantic domains, etc.), uncomment this block @@ -164,6 +171,16 @@ public ConversionError RunConversion() } } }); + if (_changedOnlyInFieldWorks > 0 || _absentOnlyFromFieldWorks > 0) + { + // Normally zero: LfMerge's working copy is what LF last synced with, so the two copies + // can only differ where LF users have edited since. Anything else means the working copy + // has moved on without LF, as a fresh clone of a project FLEx has kept working on has. + Logger.Warning("MongoToLcm: {0} entries differ from FieldWorks and {1} are missing from it, though no LF user has " + + "edited them since the last sync ({2}); kept FieldWorks's version of all of them", + _changedOnlyInFieldWorks, _absentOnlyFromFieldWorks, + LastSyncedDate == null ? "never synced" : LastSyncedDate.Value.ToString("u")); + } // Comment conversion gets run AFTER lexicon conversion, so that any comments on new entries are handled correctly in FW var commCvtr = new ConvertMongoToLcmComments(Connection, LfProject, exceptions, Logger, Progress); var commErrors = commCvtr.RunConversion(entryObjectIdToGuidMappings); @@ -494,9 +511,66 @@ private IMoForm CreateOwnedLexemeForm(ILexEntry owner, string morphologyType) return result; } + /// + /// The last sync's date, or null if the project has never been synced. + /// + private DateTime? LastSyncedDate + { + get + { + DateTime? lastSynced = ProjectRecord.LastSyncedDate; + return lastSynced == null || lastSynced.Value <= MagicValues.UnixEpoch ? null : lastSynced; + } + } + + /// + /// Whether an LF user has edited, created or deleted this entry since the last sync, or ever, + /// if the project has never been synced. + /// + /// Mongo holds FieldWorks as it was at the last sync plus what LF users have done since, so + /// those are the only entries LF has anything to say about. Comparing LF's copy with + /// FieldWorks's instead is only right while LfMerge's working copy is still exactly what it + /// last synced: on a fresh clone of a project FLEx has kept working on, every entry FLEx + /// changed since then would look changed in LF, and LF's older copy would be written over it. + /// + /// LF stamps an entry's DateModified on every edit and deletion (MapperModel::write), and + /// LfMerge's own writes to Mongo all come before it records LastSyncedDate. Both dates come + /// from the server's clock, so a FLEx user's clock being wrong cannot affect this. Mongo keeps + /// them to the millisecond, and a tie counts as edited: that falls back to comparing LF's copy + /// with FieldWorks's, which is what LfMerge has always done. + /// + /// A project that has never been synced has no date to go by, though its entries can still be + /// LfMerge's: the initial clone's transfer to Mongo does not always get as far as recording a + /// sync. LfMerge clears the user references on every entry it writes, and LF sets one on every + /// edit and creation; a deletion sets none, but deleting what FieldWorks lacks is harmless. + /// + private bool EditedInLfSinceLastSync(LfLexEntry lfEntry) + { + DateTime? lastSynced = LastSyncedDate; + if (lastSynced == null) + return lfEntry.IsDeleted || (lfEntry.AuthorInfo != null && + (lfEntry.AuthorInfo.ModifiedByUserRef != null || lfEntry.AuthorInfo.CreatedByUserRef != null)); + return lfEntry.DateModified >= lastSynced.Value; + } + private void LfLexEntryToLcmLexEntry(LfLexEntry lfEntry) { Guid guid = lfEntry.Guid ?? Guid.Empty; + if (!EditedInLfSinceLastSync(lfEntry)) + { + // Nothing for FieldWorks here. Where it differs, it is the newer one, and the transfer + // back to Mongo after Send/Receive will bring LF up to date with it. + ILexEntry existing; + if (lfEntry.IsDeleted) + { + // An LF deletion that an earlier sync has already sent + } + else if (!GetInstance().TryGetObject(guid, out existing)) + _absentOnlyFromFieldWorks++; + else if (lfEntry.AuthorInfo == null || lfEntry.AuthorInfo.ModifiedDate.ToLocalTime() != existing.DateModified) + _changedOnlyInFieldWorks++; + return; + } bool createdEntry = false; bool wantCreation = !lfEntry.IsDeleted; ILexEntry LcmEntry = GetOrCreateEntryByGuid(guid, wantCreation, out createdEntry); @@ -541,6 +615,14 @@ private void LfLexEntryToLcmLexEntry(LfLexEntry lfEntry) return; } } + if (!createdEntry && LastSyncedDate != null && LcmEntry.DateModified.ToUniversalTime() > LastSyncedDate.Value) + { + // Only possible when the working copy has moved on without LF (or a FLEx user's clock + // is ahead). LF's edit still wins, as it always has; this records what it replaced. + Logger.Warning("MongoToLcm: entry {0} ({1}) was edited in FieldWorks on {2:u} as well as in LF since the last sync; " + + "LF's version replaces FieldWorks's", guid, ConvertUtilities.EntryNameForDebugging(lfEntry), + LcmEntry.DateModified.ToUniversalTime()); + } // Fields in order by lfEntry property, except for Senses and CustomFields, which are handled at the end SetMultiStringFrom(LcmEntry.CitationForm, lfEntry.CitationForm); From 4f382fdd36fa1719cc8d5d283c79ad43fa7657ad Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Thu, 1 Oct 2026 13:15:11 +0700 Subject: [PATCH 2/2] Test double needs to refresh LastSyncedDate Tests now depend on LastSyncedDate being correct, since the Mongo->LCM logic relies on it now in order to detect cases where FW work went on while LF didn't know about it. So the test double needs to refresh its LastSyncedDate value the same way that the real object does, in order for the unit tests to be correctly simulating the real thing. --- src/LfMerge.Core.Tests/TestDoubles.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/LfMerge.Core.Tests/TestDoubles.cs b/src/LfMerge.Core.Tests/TestDoubles.cs index 0c771ade..e8667743 100644 --- a/src/LfMerge.Core.Tests/TestDoubles.cs +++ b/src/LfMerge.Core.Tests/TestDoubles.cs @@ -381,6 +381,7 @@ public override MongoProjectRecord Create(ILfProject project) MongoProjectRecord record; if (_projectRecords.TryGetValue(project.ProjectCode, out record)) { + RefreshLastSyncedDate(record, project); return record; } else @@ -411,9 +412,19 @@ public override MongoProjectRecord Create(ILfProject project) Config = sampleConfig }; _projectRecords.Add(project.ProjectCode, record); + RefreshLastSyncedDate(record, project); return record; } } + + /// The real factory reads the record from Mongo every time, so it always sees the last sync's + /// date. Each instance of this double keeps its own records, and the connection double only + /// updates the most recently created instance's, so take the date from the connection instead. + private void RefreshLastSyncedDate(MongoProjectRecord record, ILfProject project) + { + var testDouble = Connection as MongoConnectionDouble; + if (testDouble != null) record.LastSyncedDate = testDouble.GetLastSyncedDate(project); + } } class LanguageDepotProjectDouble: ILanguageDepotProject