Skip to content

Commit

Permalink
Don't serialize comments to JSON when calling LfMergeBridge (#338)
Browse files Browse the repository at this point in the history
* Get ready to use new LfMergeBridge API

Many parts of the LfMerge model have been transferred over to
LfMergeBridge, so that the new API can reference the same data types
that LfMerge does.

* Register new model class location with Mongo

The Mongo class registration needs to be told about the new namespace
that some of our model classes live in, so that the auto-registration
can do its job and they can be serialized to Mongo correctly.

* Call LfMergeBridge without JSON serialization

Now that the LfMergeBridge code is working, we can switch to calling it
without serializing comments as JSON. This will save quite a bit of
memory for large projects.

* Remove no-longer-needed LfMergeBridge option

We no longer need the `serializedCommentsFromLfMerge` option now that we
pass the comments in as objects.

* Use new LfMergeBridge API to write to ChorusNotes

Similarly to what we did with the GetChorusNotes handler, the other
direction (WriteToChorusNotes) can also skip serializing JSON.

* Use LfMergeBridge NuGet package

LfMergeBridge build in GitHub Actions now uploads nuget packages, so we
can reference the latest LfMergeBridge package to use the new API.

* Update LibChorus to go along with LfMergeBridge update

The new LfMergeBridge package needs a higher version of LibChorus than
we were referencing before.

* Register new model class location for test double

In addition to registering the Mongo serialization options for the real
MongoConnection, we also need to do the same thing for the test double
of the MongoConnection class, so that it can test real data correctly.
  • Loading branch information
rmunn authored Jul 9, 2024
1 parent 4f1e85f commit f518d8d
Show file tree
Hide file tree
Showing 31 changed files with 73 additions and 326 deletions.
2 changes: 1 addition & 1 deletion src/LfMerge.Core.Tests/Lcm/LcmTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using LfMerge.Core.Actions;
using LfMerge.Core.DataConverters;
using LfMerge.Core.FieldWorks;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.MongoConnector;
using LfMerge.Core.Reporting;
using LfMerge.Core.Settings;
Expand Down
1 change: 1 addition & 0 deletions src/LfMerge.Core.Tests/Lcm/RoundTripTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using LfMerge.Core.DataConverters;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using MongoDB.Bson;
using NUnit.Framework;
using SIL.LCModel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using LfMerge.Core.DataConverters;
using LfMerge.Core.LanguageForge.Config;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using NUnit.Framework;
using SIL.LCModel;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using LfMerge.Core.Actions.Infrastructure;
using LfMerge.Core.DataConverters;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using MongoDB.Bson;
using NUnit.Framework;
using System;
Expand Down
3 changes: 2 additions & 1 deletion src/LfMerge.Core.Tests/TestDoubles.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using System.Linq.Expressions;
using Bugsnag.Payload;
using LfMergeBridge.LfMergeModel;
using IniParser.Model;
using LfMerge.Core.Actions;
using LfMerge.Core.Actions.Infrastructure;
Expand Down Expand Up @@ -107,7 +108,7 @@ public static void Initialize()
ConventionRegistry.Register(
"My Custom Conventions",
pack,
t => t.FullName.StartsWith("LfMerge."));
t => t.FullName.StartsWith("LfMerge.") || t.FullName.StartsWith("LfMergeBridge.LfMergeModel"));

// Register class mappings before opening first connection
new MongoRegistrarForLfConfig().RegisterClassMappings();
Expand Down
51 changes: 23 additions & 28 deletions src/LfMerge.Core/DataConverters/ConvertLcmToMongoComments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
using LfMerge.Core.Logging;
using LfMerge.Core.MongoConnector;
using LfMerge.Core.LanguageForge.Config;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Reporting;
using Newtonsoft.Json;
using SIL.LCModel;
Expand Down Expand Up @@ -43,17 +44,13 @@ public List<CommentConversionError<ILexEntry>> RunConversion()
var exceptions = new List<CommentConversionError<ILexEntry>>();
var fixedComments = new List<LfComment>(_conn.GetComments(_project));
string allCommentsJson = JsonConvert.SerializeObject(fixedComments);
// _logger.Debug("Doing Lcm->Mongo direction. The json for ALL comments from Mongo would be: {0}", allCommentsJson);
// _logger.Debug("Doing Lcm->Mongo direction. About to call LfMergeBridge with that JSON...");
string bridgeOutput;
if (CallLfMergeBridge(allCommentsJson, out bridgeOutput))
GetChorusNotesResponse response = CallLfMergeBridge(fixedComments, out bridgeOutput);
if (response != null)
{
string newCommentsStr = ConvertMongoToLcmComments.GetPrefixedStringFromLfMergeBridgeOutput(bridgeOutput, "New comments not yet in LF: ");
string newRepliesStr = ConvertMongoToLcmComments.GetPrefixedStringFromLfMergeBridgeOutput(bridgeOutput, "New replies on comments already in LF: ");
string newStatusChangesStr = ConvertMongoToLcmComments.GetPrefixedStringFromLfMergeBridgeOutput(bridgeOutput, "New status changes on comments already in LF: ");
List<LfComment> comments = JsonConvert.DeserializeObject<List<LfComment>>(newCommentsStr);
List<Tuple<string, List<LfCommentReply>>> replies = JsonConvert.DeserializeObject<List<Tuple<string, List<LfCommentReply>>>>(newRepliesStr);
List<KeyValuePair<string, Tuple<string, string>>> statusChanges = JsonConvert.DeserializeObject<List<KeyValuePair<string, Tuple<string, string>>>>(newStatusChangesStr);
List<LfComment> comments = response.LfComments;
List<Tuple<string, List<LfCommentReply>>> replies = response.LfReplies;
List<KeyValuePair<string, Tuple<string, string>>> statusChanges = response.LfStatusChanges;

var entryErrorsByGuid = _entryConversionErrors.EntryErrors.ToDictionary(s => s.EntryGuid());
foreach (LfComment comment in comments)
Expand Down Expand Up @@ -290,28 +287,26 @@ public ILexEntry OwningEntry(Guid guidOfUnknownLcmObject)
}
}

private bool CallLfMergeBridge(string bridgeInput, out string bridgeOutput)
private GetChorusNotesResponse CallLfMergeBridge(List<LfComment> comments, out string bridgeOutput)
{
// Call into LF Bridge to do the work.
bridgeOutput = string.Empty;
using (var tmpFile = new SIL.IO.TempFile(bridgeInput))
var bridgeInput = new LfMergeBridge.GetChorusNotesInput { LfComments = comments };
var options = new Dictionary<string, string>
{
var options = new Dictionary<string, string>
{
{"-p", _project.FwDataPath},
{"serializedCommentsFromLfMerge", tmpFile.Path},
};
if (!LfMergeBridge.LfMergeBridge.Execute("Language_Forge_Get_Chorus_Notes", _progress,
options, out bridgeOutput))
{
_logger.Error("Got an error from Language_Forge_Get_Chorus_Notes: {0}", bridgeOutput);
return false;
}
else
{
// _logger.Debug("Got the JSON from Language_Forge_Get_Chorus_Notes: {0}", bridgeOutput);
return true;
}
{"-p", _project.FwDataPath},
};
LfMergeBridge.LfMergeBridge.ExtraInputData.Add(options, bridgeInput);
if (!LfMergeBridge.LfMergeBridge.Execute("Language_Forge_Get_Chorus_Notes", _progress,
options, out bridgeOutput))
{
_logger.Error("Got an error from Language_Forge_Get_Chorus_Notes: {0}", bridgeOutput);
return null;
}
else
{
var success = LfMergeBridge.LfMergeBridge.ExtraOutputData.TryGetValue(options, out var outputObject);
return outputObject as GetChorusNotesResponse;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using LfMerge.Core.FieldWorks;
using LfMerge.Core.LanguageForge.Config;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Logging;
using LfMerge.Core.MongoConnector;
using LfMerge.Core.Reporting;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Collections.Generic;
using System.Linq;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Logging;
using SIL.LCModel;
using SIL.LCModel.Core.KernelInterfaces;
Expand Down
87 changes: 24 additions & 63 deletions src/LfMerge.Core/DataConverters/ConvertMongoToLcmComments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
using System;
using System.Collections.Generic;
using System.Linq;
using LfMerge.Core.Actions.Infrastructure;
using LfMerge.Core.Logging;
using LfMerge.Core.MongoConnector;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Reporting;
using MongoDB.Bson;
using Newtonsoft.Json;
using SIL.Progress;
using LfMergeBridge;

namespace LfMerge.Core.DataConverters
{
Expand Down Expand Up @@ -84,16 +84,15 @@ public List<CommentConversionError<LfLexEntry>> RunConversion(Dictionary<MongoDB
}
}
var skippedCommentGuids = new HashSet<string>(exceptions.Select(s => s.CommentGuid().ToString()));
string unskippedCommentsJson = JsonConvert.SerializeObject(commentsWithIds.Where(s => !skippedCommentGuids.Contains(s.Key)));
var unskippedComments = commentsWithIds.Where(s => !skippedCommentGuids.Contains(s.Key)).ToList();
string bridgeOutput;
CallLfMergeBridge(unskippedCommentsJson, out bridgeOutput);
// LfMergeBridge returns two lists of IDs (comment IDs or reply IDs) that need to have their GUIDs updated in Mongo.
string commentGuidMappingsStr = GetPrefixedStringFromLfMergeBridgeOutput(bridgeOutput, "New comment ID->Guid mappings: ");
string replyGuidMappingsStr = GetPrefixedStringFromLfMergeBridgeOutput(bridgeOutput, "New reply ID->Guid mappings: ");
Dictionary<string, Guid> commentIdToGuidMappings = ParseGuidMappings(commentGuidMappingsStr);
Dictionary<string, Guid> uniqIdToGuidMappings = ParseGuidMappings(replyGuidMappingsStr);
_conn.SetCommentGuids(_project, commentIdToGuidMappings);
_conn.SetCommentReplyGuids(_project, uniqIdToGuidMappings);
var response = CallLfMergeBridge(unskippedComments, out bridgeOutput);
if (response != null)
{
// LfMergeBridge returns two lists of IDs (comment IDs or reply IDs) that need to have their GUIDs updated in Mongo.
_conn.SetCommentGuids(_project, response.CommentIdsThatNeedGuids);
_conn.SetCommentReplyGuids(_project, response.ReplyIdsThatNeedGuids);
}
return exceptions;
}

Expand All @@ -102,72 +101,34 @@ private LfLexEntry GetLexEntry(ObjectId idOfEntry)
return _conn.GetRecords<LfLexEntry>(_project, MagicStrings.LfCollectionNameForLexicon, entry => entry.Id == idOfEntry).First();
}

private bool CallLfMergeBridge(string bridgeInput, out string bridgeOutput)
private WriteToChorusNotesResponse CallLfMergeBridge(List<KeyValuePair<string, LfComment>> lfComments, out string bridgeOutput)
{
bridgeOutput = string.Empty;
using (var tmpFile = new SIL.IO.TempFile(bridgeInput))
var options = new Dictionary<string, string>
{
var options = new Dictionary<string, string>
{
{"-p", _project.FwDataPath},
{"serializedCommentsFromLfMerge", tmpFile.Path}
};
try {
{"-p", _project.FwDataPath},
};
try {
var bridgeInput = new WriteToChorusNotesInput { LfComments = lfComments };
LfMergeBridge.LfMergeBridge.ExtraInputData.Add(options, bridgeInput);
if (!LfMergeBridge.LfMergeBridge.Execute("Language_Forge_Write_To_Chorus_Notes", _progress,
options, out bridgeOutput))
{
_logger.Error("Got an error from Language_Forge_Write_To_Chorus_Notes: {0}", bridgeOutput);
return false;
return null;
}
else
{
// _logger.Debug("Good output from Language_Forge_Write_To_Chorus_Notes: {0}", bridgeOutput);
return true;
}
}
catch (NullReferenceException)
{
_logger.Debug("Got an exception. Before rethrowing it, here is what LfMergeBridge sent:");
_logger.Debug("{0}", bridgeOutput);
throw;
var success = LfMergeBridge.LfMergeBridge.ExtraOutputData.TryGetValue(options, out var outputObject);
return outputObject as WriteToChorusNotesResponse;
}
}
}

public static string GetPrefixedStringFromLfMergeBridgeOutput(string lfMergeBridgeOutput, string prefix)
{
if (string.IsNullOrEmpty(prefix) || string.IsNullOrEmpty(lfMergeBridgeOutput))
{
return string.Empty;
}
string result = LfMergeBridgeServices.GetLineContaining(lfMergeBridgeOutput, prefix);
if (result.StartsWith(prefix))
catch (NullReferenceException)
{
return result.Substring(prefix.Length);
}
else
{
return string.Empty; // If the "prefix" wasn't actually a prefix, this wasn't the string we wanted.
}
}

public Dictionary<string, Guid> ParseGuidMappings(string input)
{
string[] parts = input.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
var result = new Dictionary<string, Guid>();
foreach (string part in parts)
{
string[] kv = part.Split(new char[] { '=' }, StringSplitOptions.RemoveEmptyEntries);
if (kv.Length == 2)
{
Guid parsed;
if (Guid.TryParse(kv[1], out parsed))
{
result[kv[0]] = parsed;
}
}
_logger.Debug("Got an exception. Before rethrowing it, here is what LfMergeBridge sent:");
_logger.Debug("{0}", bridgeOutput);
throw;
}
return result;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using LfMerge.Core.DataConverters.CanonicalSources;
using LfMerge.Core.FieldWorks;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Logging;
using LfMerge.Core.MongoConnector;
using LfMerge.Core.Reporting;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using LfMerge.Core.DataConverters.CanonicalSources;
using LfMerge.Core.LanguageForge.Model;
using LfMergeBridge.LfMergeModel;
using LfMerge.Core.Logging;
using SIL.LCModel;

Expand Down
15 changes: 0 additions & 15 deletions src/LfMerge.Core/LanguageForge/Model/IHasNullableGuid.cs

This file was deleted.

16 changes: 0 additions & 16 deletions src/LfMerge.Core/LanguageForge/Model/LfAuthorInfo.cs

This file was deleted.

43 changes: 0 additions & 43 deletions src/LfMerge.Core/LanguageForge/Model/LfComment.cs

This file was deleted.

31 changes: 0 additions & 31 deletions src/LfMerge.Core/LanguageForge/Model/LfCommentRegarding.cs

This file was deleted.

Loading

0 comments on commit f518d8d

Please sign in to comment.