From 54b9df46c6c54250035ee21112c8fd294135c8f8 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Thu, 1 Aug 2024 22:26:25 -0400 Subject: [PATCH 01/15] Switch to new Machine classes for usfm updating --- .../Services/IScriptureDataFileService.cs | 1 + .../Services/ScriptureDataFileService.cs | 6 + .../Services/ZipParatextProjectTextUpdater.cs | 28 +++ .../Services/PretranslationService.cs | 160 ++++++++---------- .../Services/PretranslationServiceTests.cs | 53 ++++-- 5 files changed, 145 insertions(+), 103 deletions(-) create mode 100644 src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index 92fe96d9..bd632e84 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -3,5 +3,6 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); + public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); Task ReadParatextProjectBookAsync(string filename, string book); } diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 1a5a9753..7a580984 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -12,6 +12,12 @@ public ParatextProjectSettings GetParatextProjectSettings(string filename) return ParseProjectSettings(container); } + public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename) + { + using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); + return new ZipParatextProjectTextUpdater(container); + } + public async Task ReadParatextProjectBookAsync(string filename, string book) { using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs new file mode 100644 index 00000000..1a905d6c --- /dev/null +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -0,0 +1,28 @@ +namespace Serval.Shared.Services; + +public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase +{ + public ZipParatextProjectTextUpdater(IZipContainer container) + : base(new ZipParatextProjectSettingsParser(container)) + { + _projectContainer = container; + } + + public ZipParatextProjectTextUpdater(IZipContainer container, ParatextProjectSettings settings) + : base(settings) + { + _projectContainer = container; + } + + private readonly IZipContainer _projectContainer; + + protected override bool Exists(string fileName) + { + return _projectContainer.EntryExists(fileName); + } + + protected override Stream Open(string fileName) + { + return _projectContainer.OpenEntry(fileName); + } +} diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index 8f167ce9..fdf32c50 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -71,107 +71,87 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) // Update the target book if it exists if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Target) { - string? usfm = await _scriptureDataFileService.ReadParatextProjectBookAsync(targetFile.Filename, textId); - if (usfm is not null) + // the pretranslations are generated from the source book and inserted into the target book + // use relaxed references since the USFM structure may not be the same + pretranslations = pretranslations.Select(p => + ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) + ); + Shared.Services.ZipParatextProjectTextUpdater updater = + _scriptureDataFileService.GetZipParatextProjectTextUpdater(targetFile.Filename); + string usfm = ""; + switch (textOrigin) { - // the pretranslations are generated from the source book and inserted into the target book - // use relaxed references since the USFM structure may not be the same - pretranslations = pretranslations.Select(p => - ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) - ); - switch (textOrigin) - { - case PretranslationUsfmTextOrigin.PreferExisting: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: true - ); - case PretranslationUsfmTextOrigin.PreferPretranslated: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); - case PretranslationUsfmTextOrigin.OnlyExisting: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations: [], // don't put any pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); - case PretranslationUsfmTextOrigin.OnlyPretranslated: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: false - ); - } + case PretranslationUsfmTextOrigin.PreferExisting: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: true + ); + break; + case PretranslationUsfmTextOrigin.PreferPretranslated: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ); + break; + case PretranslationUsfmTextOrigin.OnlyExisting: + usfm = updater.UpdateUsfm( + textId, + [], // don't put any pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ); + break; + case PretranslationUsfmTextOrigin.OnlyPretranslated: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: false + ); + break; } + // In order to support PretranslationUsfmTemplate.Auto + if (usfm != "") + return usfm; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) { + Shared.Services.ZipParatextProjectTextUpdater updater = + _scriptureDataFileService.GetZipParatextProjectTextUpdater(sourceFile.Filename); + // Copy and update the source book if it exists - string? usfm = await _scriptureDataFileService.ReadParatextProjectBookAsync(sourceFile.Filename, textId); - if (usfm is not null) + switch (textOrigin) { - switch (textOrigin) - { - case PretranslationUsfmTextOrigin.PreferExisting: - case PretranslationUsfmTextOrigin.PreferPretranslated: - case PretranslationUsfmTextOrigin.OnlyPretranslated: - return UpdateUsfm( - sourceSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); - case PretranslationUsfmTextOrigin.OnlyExisting: - return UpdateUsfm( - sourceSettings, - usfm, - pretranslations: [], // don't pass the pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); - } + case PretranslationUsfmTextOrigin.PreferExisting: + case PretranslationUsfmTextOrigin.PreferPretranslated: + case PretranslationUsfmTextOrigin.OnlyPretranslated: + return updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ); + case PretranslationUsfmTextOrigin.OnlyExisting: + return updater.UpdateUsfm( + textId, + [], // don't pass the pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ); } } return ""; } - - private static string UpdateUsfm( - ParatextProjectSettings settings, - string usfm, - IEnumerable<(IReadOnlyList, string)> pretranslations, - string? fullName = null, - bool stripAllText = false, - bool preferExistingText = true - ) - { - var updater = new UsfmTextUpdater( - pretranslations.ToArray(), - fullName is null ? null : $"- {fullName}", - stripAllText, - preferExistingText: preferExistingText - ); - UsfmParser.Parse(usfm, updater, settings.Stylesheet, settings.Versification); - return updater.GetUsfm(settings.Stylesheet); - } } diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index 966d1023..aa987072 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -346,6 +346,25 @@ public TestEnvironment() ScriptureDataFileService .ReadParatextProjectBookAsync("file2.zip", "MAT") .Returns(Task.FromResult(null)); + var zipSubstituteSource = Substitute.For(); + var zipSubstituteTarget = Substitute.For(); + zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(StringToStream(SourceUsfm)); + zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(StringToStream("")); + zipSubstituteSource.EntryExists(Arg.Any()).Returns(false); + zipSubstituteTarget.EntryExists(Arg.Any()).Returns(false); + zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); + zipSubstituteTarget.EntryExists("MATTRG.SFM").Returns(true); + TargetZipContainer = zipSubstituteTarget; + var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( + zipSubstituteSource, + CreateProjectSettings("SRC") + ); + var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( + zipSubstituteTarget, + CreateProjectSettings("TRG") + ); + ScriptureDataFileService.GetZipParatextProjectTextUpdater("file1.zip").Returns(textUpdaterSource); + ScriptureDataFileService.GetZipParatextProjectTextUpdater("file2.zip").Returns(textUpdaterTarget); Service = new PretranslationService(Pretranslations, Engines, ScriptureDataFileService); } @@ -353,29 +372,37 @@ public TestEnvironment() public MemoryRepository Pretranslations { get; } public MemoryRepository Engines { get; } public IScriptureDataFileService ScriptureDataFileService { get; } + public IZipContainer TargetZipContainer { get; } + + private static Stream StringToStream(string s) + { + var stream = new MemoryStream(); + var streamWriter = new StreamWriter(stream); + streamWriter.Write(s); + streamWriter.Flush(); + stream.Seek(0, SeekOrigin.Begin); + return stream; + } public async Task GetUsfmAsync( PretranslationUsfmTextOrigin textOrigin, PretranslationUsfmTemplate template ) { - return ( - await Service.GetUsfmAsync( - engineId: "engine1", - modelRevision: 1, - corpusId: "corpus1", - textId: "MAT", - textOrigin: textOrigin, - template: template - ) - ).Replace("\r\n", "\n"); + string usfm = await Service.GetUsfmAsync( + engineId: "engine1", + modelRevision: 1, + corpusId: "corpus1", + textId: "MAT", + textOrigin: textOrigin, + template: template + ); + return usfm.Replace("\r\n", "\n"); } public void AddMatthewToTarget() { - ScriptureDataFileService - .ReadParatextProjectBookAsync("file2.zip", "MAT") - .Returns(Task.FromResult(TargetUsfm)); + TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(StringToStream(TargetUsfm)); } private static ParatextProjectSettings CreateProjectSettings(string name) From 9f41969e65aab04f1cea745f8bbbdcfce3b1b3eb Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Fri, 2 Aug 2024 07:56:31 -0400 Subject: [PATCH 02/15] Remove unnecessary method; add test to scripture data file service --- .../Services/IScriptureDataFileService.cs | 1 - .../Services/ScriptureDataFileService.cs | 11 ---------- .../Services/ScriptureDataFileServiceTests.cs | 21 +++++------------- .../Services/PretranslationServiceTests.cs | 22 +++---------------- 4 files changed, 9 insertions(+), 46 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index bd632e84..be36b66d 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -4,5 +4,4 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); - Task ReadParatextProjectBookAsync(string filename, string book); } diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 7a580984..9f633a02 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -18,17 +18,6 @@ public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string fil return new ZipParatextProjectTextUpdater(container); } - public async Task ReadParatextProjectBookAsync(string filename, string book) - { - using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); - ParatextProjectSettings settings = ParseProjectSettings(container); - string entryName = settings.GetBookFileName(book); - if (!container.EntryExists(entryName)) - return null; - using StreamReader reader = new(container.OpenEntry(entryName)); - return await reader.ReadToEndAsync(); - } - private string GetFilePath(string filename) { return Path.Combine(_dataFileOptions.CurrentValue.FilesDirectory, filename); diff --git a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs index 83f55aa7..091fae66 100644 --- a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs +++ b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs @@ -12,16 +12,15 @@ public void GetParatextProjectSettings() } [Test] - public async Task ReadParatextProjectBookAsync_Exists() + public void GetZipParatextProjectTextUpdater() { TestEnvironment env = new(); - string? usfm = await env.Service.ReadParatextProjectBookAsync("file1.zip", "MAT"); - Assert.That(usfm, Is.Not.Null); + ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); Assert.That( - usfm.Replace("\r\n", "\n"), + updater.UpdateUsfm("MAT", [], preferExistingText: true), Is.EqualTo( - @"\id MAT - PROJ -\h Matthew + $@"\id MAT - PROJ +\h {Canon.BookIdToEnglishName("MAT")} \c 1 \p \v 1 Chapter one, verse one. @@ -30,19 +29,11 @@ public async Task ReadParatextProjectBookAsync_Exists() \p \v 1 Chapter two, verse one. \v 2 Chapter two, verse two. -".Replace("\r\n", "\n") +".Replace("\n", "\r\n") ) ); } - [Test] - public async Task ReadParatextProjectBookAsync_DoesNotExist() - { - TestEnvironment env = new(); - string? usfm = await env.Service.ReadParatextProjectBookAsync("file1.zip", "MRK"); - Assert.That(usfm, Is.Null); - } - private class TestEnvironment { public TestEnvironment() diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index aa987072..3b328b25 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -340,16 +340,10 @@ public TestEnvironment() ScriptureDataFileService = Substitute.For(); ScriptureDataFileService.GetParatextProjectSettings("file1.zip").Returns(CreateProjectSettings("SRC")); ScriptureDataFileService.GetParatextProjectSettings("file2.zip").Returns(CreateProjectSettings("TRG")); - ScriptureDataFileService - .ReadParatextProjectBookAsync("file1.zip", "MAT") - .Returns(Task.FromResult(SourceUsfm)); - ScriptureDataFileService - .ReadParatextProjectBookAsync("file2.zip", "MAT") - .Returns(Task.FromResult(null)); var zipSubstituteSource = Substitute.For(); var zipSubstituteTarget = Substitute.For(); - zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(StringToStream(SourceUsfm)); - zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(StringToStream("")); + zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(SourceUsfm))); + zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(""))); zipSubstituteSource.EntryExists(Arg.Any()).Returns(false); zipSubstituteTarget.EntryExists(Arg.Any()).Returns(false); zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); @@ -374,16 +368,6 @@ public TestEnvironment() public IScriptureDataFileService ScriptureDataFileService { get; } public IZipContainer TargetZipContainer { get; } - private static Stream StringToStream(string s) - { - var stream = new MemoryStream(); - var streamWriter = new StreamWriter(stream); - streamWriter.Write(s); - streamWriter.Flush(); - stream.Seek(0, SeekOrigin.Begin); - return stream; - } - public async Task GetUsfmAsync( PretranslationUsfmTextOrigin textOrigin, PretranslationUsfmTemplate template @@ -402,7 +386,7 @@ PretranslationUsfmTemplate template public void AddMatthewToTarget() { - TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(StringToStream(TargetUsfm)); + TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(TargetUsfm))); } private static ParatextProjectSettings CreateProjectSettings(string name) From 788536130115272e622ed0f4a567d32ac27016fc Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Mon, 5 Aug 2024 13:58:45 -0400 Subject: [PATCH 03/15] Review feedback --- .../src/Serval.Shared/Services/IScriptureDataFileService.cs | 2 +- .../src/Serval.Translation/Services/PretranslationService.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index be36b66d..08424a55 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -3,5 +3,5 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); - public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); + ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); } diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index fdf32c50..cec34a3c 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -119,7 +119,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) break; } // In order to support PretranslationUsfmTemplate.Auto - if (usfm != "") + if (usfm != null) return usfm; } From 176c8efce4081ab45ba418cfb0daca85c317d9e2 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Mon, 5 Aug 2024 16:43:57 -0400 Subject: [PATCH 04/15] Make zip text updater disposable to avoid disposing zipcontainer; use isnullorempty --- .../Serval.Shared/Services/ScriptureDataFileService.cs | 2 +- .../Services/ZipParatextProjectTextUpdater.cs | 8 +++++++- .../Serval.Translation/Services/PretranslationService.cs | 6 +++--- .../Services/ScriptureDataFileServiceTests.cs | 2 +- .../Services/PretranslationServiceTests.cs | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 9f633a02..e38b8c08 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -14,7 +14,7 @@ public ParatextProjectSettings GetParatextProjectSettings(string filename) public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename) { - using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); + IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); return new ZipParatextProjectTextUpdater(container); } diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs index 1a905d6c..d8ebd7d1 100644 --- a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -1,6 +1,6 @@ namespace Serval.Shared.Services; -public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase +public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase, IDisposable { public ZipParatextProjectTextUpdater(IZipContainer container) : base(new ZipParatextProjectSettingsParser(container)) @@ -25,4 +25,10 @@ protected override Stream Open(string fileName) { return _projectContainer.OpenEntry(fileName); } + + public void Dispose() + { + _projectContainer.Dispose(); + GC.SuppressFinalize(this); + } } diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index cec34a3c..811fa87b 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -76,7 +76,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) pretranslations = pretranslations.Select(p => ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) ); - Shared.Services.ZipParatextProjectTextUpdater updater = + using Shared.Services.ZipParatextProjectTextUpdater updater = _scriptureDataFileService.GetZipParatextProjectTextUpdater(targetFile.Filename); string usfm = ""; switch (textOrigin) @@ -119,13 +119,13 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) break; } // In order to support PretranslationUsfmTemplate.Auto - if (usfm != null) + if (!string.IsNullOrEmpty(usfm)) return usfm; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) { - Shared.Services.ZipParatextProjectTextUpdater updater = + using Shared.Services.ZipParatextProjectTextUpdater updater = _scriptureDataFileService.GetZipParatextProjectTextUpdater(sourceFile.Filename); // Copy and update the source book if it exists diff --git a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs index 091fae66..be7481ec 100644 --- a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs +++ b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs @@ -15,7 +15,7 @@ public void GetParatextProjectSettings() public void GetZipParatextProjectTextUpdater() { TestEnvironment env = new(); - ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); + using ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); Assert.That( updater.UpdateUsfm("MAT", [], preferExistingText: true), Is.EqualTo( diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index 3b328b25..b2f08824 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -349,11 +349,11 @@ public TestEnvironment() zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); zipSubstituteTarget.EntryExists("MATTRG.SFM").Returns(true); TargetZipContainer = zipSubstituteTarget; - var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( + using var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( zipSubstituteSource, CreateProjectSettings("SRC") ); - var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( + using var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( zipSubstituteTarget, CreateProjectSettings("TRG") ); From 6f3d89b382a8567d460bb42ea65d5de2c4490b00 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Tue, 6 Aug 2024 12:47:16 -0400 Subject: [PATCH 05/15] Use disposable pattern --- .../Services/ZipParatextProjectTextUpdater.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs index d8ebd7d1..b7fb4d5d 100644 --- a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -2,6 +2,8 @@ namespace Serval.Shared.Services; public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase, IDisposable { + private bool _disposed; + public ZipParatextProjectTextUpdater(IZipContainer container) : base(new ZipParatextProjectSettingsParser(container)) { @@ -28,7 +30,19 @@ protected override Stream Open(string fileName) public void Dispose() { - _projectContainer.Dispose(); + Dispose(true); GC.SuppressFinalize(this); } + + protected virtual void Dispose(bool disposing) + { + if (!_disposed) + { + if (disposing) + { + _projectContainer.Dispose(); + } + _disposed = true; + } + } } From 693ff0bac70a9696da3b31361bb479009521fa8f Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Thu, 1 Aug 2024 22:26:25 -0400 Subject: [PATCH 06/15] Switch to new Machine classes for usfm updating --- .../Services/IScriptureDataFileService.cs | 1 + .../Services/ScriptureDataFileService.cs | 6 + .../Services/ZipParatextProjectTextUpdater.cs | 28 +++ .../Services/PretranslationService.cs | 160 ++++++++---------- .../Services/PretranslationServiceTests.cs | 53 ++++-- 5 files changed, 145 insertions(+), 103 deletions(-) create mode 100644 src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index 92fe96d9..bd632e84 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -3,5 +3,6 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); + public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); Task ReadParatextProjectBookAsync(string filename, string book); } diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 1a5a9753..7a580984 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -12,6 +12,12 @@ public ParatextProjectSettings GetParatextProjectSettings(string filename) return ParseProjectSettings(container); } + public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename) + { + using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); + return new ZipParatextProjectTextUpdater(container); + } + public async Task ReadParatextProjectBookAsync(string filename, string book) { using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs new file mode 100644 index 00000000..1a905d6c --- /dev/null +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -0,0 +1,28 @@ +namespace Serval.Shared.Services; + +public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase +{ + public ZipParatextProjectTextUpdater(IZipContainer container) + : base(new ZipParatextProjectSettingsParser(container)) + { + _projectContainer = container; + } + + public ZipParatextProjectTextUpdater(IZipContainer container, ParatextProjectSettings settings) + : base(settings) + { + _projectContainer = container; + } + + private readonly IZipContainer _projectContainer; + + protected override bool Exists(string fileName) + { + return _projectContainer.EntryExists(fileName); + } + + protected override Stream Open(string fileName) + { + return _projectContainer.OpenEntry(fileName); + } +} diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index 8f167ce9..fdf32c50 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -71,107 +71,87 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) // Update the target book if it exists if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Target) { - string? usfm = await _scriptureDataFileService.ReadParatextProjectBookAsync(targetFile.Filename, textId); - if (usfm is not null) + // the pretranslations are generated from the source book and inserted into the target book + // use relaxed references since the USFM structure may not be the same + pretranslations = pretranslations.Select(p => + ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) + ); + Shared.Services.ZipParatextProjectTextUpdater updater = + _scriptureDataFileService.GetZipParatextProjectTextUpdater(targetFile.Filename); + string usfm = ""; + switch (textOrigin) { - // the pretranslations are generated from the source book and inserted into the target book - // use relaxed references since the USFM structure may not be the same - pretranslations = pretranslations.Select(p => - ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) - ); - switch (textOrigin) - { - case PretranslationUsfmTextOrigin.PreferExisting: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: true - ); - case PretranslationUsfmTextOrigin.PreferPretranslated: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); - case PretranslationUsfmTextOrigin.OnlyExisting: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations: [], // don't put any pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); - case PretranslationUsfmTextOrigin.OnlyPretranslated: - return UpdateUsfm( - targetSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: false - ); - } + case PretranslationUsfmTextOrigin.PreferExisting: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: true + ); + break; + case PretranslationUsfmTextOrigin.PreferPretranslated: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ); + break; + case PretranslationUsfmTextOrigin.OnlyExisting: + usfm = updater.UpdateUsfm( + textId, + [], // don't put any pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ); + break; + case PretranslationUsfmTextOrigin.OnlyPretranslated: + usfm = updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: false + ); + break; } + // In order to support PretranslationUsfmTemplate.Auto + if (usfm != "") + return usfm; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) { + Shared.Services.ZipParatextProjectTextUpdater updater = + _scriptureDataFileService.GetZipParatextProjectTextUpdater(sourceFile.Filename); + // Copy and update the source book if it exists - string? usfm = await _scriptureDataFileService.ReadParatextProjectBookAsync(sourceFile.Filename, textId); - if (usfm is not null) + switch (textOrigin) { - switch (textOrigin) - { - case PretranslationUsfmTextOrigin.PreferExisting: - case PretranslationUsfmTextOrigin.PreferPretranslated: - case PretranslationUsfmTextOrigin.OnlyPretranslated: - return UpdateUsfm( - sourceSettings, - usfm, - pretranslations, - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); - case PretranslationUsfmTextOrigin.OnlyExisting: - return UpdateUsfm( - sourceSettings, - usfm, - pretranslations: [], // don't pass the pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); - } + case PretranslationUsfmTextOrigin.PreferExisting: + case PretranslationUsfmTextOrigin.PreferPretranslated: + case PretranslationUsfmTextOrigin.OnlyPretranslated: + return updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ); + case PretranslationUsfmTextOrigin.OnlyExisting: + return updater.UpdateUsfm( + textId, + [], // don't pass the pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ); } } return ""; } - - private static string UpdateUsfm( - ParatextProjectSettings settings, - string usfm, - IEnumerable<(IReadOnlyList, string)> pretranslations, - string? fullName = null, - bool stripAllText = false, - bool preferExistingText = true - ) - { - var updater = new UsfmTextUpdater( - pretranslations.ToArray(), - fullName is null ? null : $"- {fullName}", - stripAllText, - preferExistingText: preferExistingText - ); - UsfmParser.Parse(usfm, updater, settings.Stylesheet, settings.Versification); - return updater.GetUsfm(settings.Stylesheet); - } } diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index 966d1023..aa987072 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -346,6 +346,25 @@ public TestEnvironment() ScriptureDataFileService .ReadParatextProjectBookAsync("file2.zip", "MAT") .Returns(Task.FromResult(null)); + var zipSubstituteSource = Substitute.For(); + var zipSubstituteTarget = Substitute.For(); + zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(StringToStream(SourceUsfm)); + zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(StringToStream("")); + zipSubstituteSource.EntryExists(Arg.Any()).Returns(false); + zipSubstituteTarget.EntryExists(Arg.Any()).Returns(false); + zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); + zipSubstituteTarget.EntryExists("MATTRG.SFM").Returns(true); + TargetZipContainer = zipSubstituteTarget; + var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( + zipSubstituteSource, + CreateProjectSettings("SRC") + ); + var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( + zipSubstituteTarget, + CreateProjectSettings("TRG") + ); + ScriptureDataFileService.GetZipParatextProjectTextUpdater("file1.zip").Returns(textUpdaterSource); + ScriptureDataFileService.GetZipParatextProjectTextUpdater("file2.zip").Returns(textUpdaterTarget); Service = new PretranslationService(Pretranslations, Engines, ScriptureDataFileService); } @@ -353,29 +372,37 @@ public TestEnvironment() public MemoryRepository Pretranslations { get; } public MemoryRepository Engines { get; } public IScriptureDataFileService ScriptureDataFileService { get; } + public IZipContainer TargetZipContainer { get; } + + private static Stream StringToStream(string s) + { + var stream = new MemoryStream(); + var streamWriter = new StreamWriter(stream); + streamWriter.Write(s); + streamWriter.Flush(); + stream.Seek(0, SeekOrigin.Begin); + return stream; + } public async Task GetUsfmAsync( PretranslationUsfmTextOrigin textOrigin, PretranslationUsfmTemplate template ) { - return ( - await Service.GetUsfmAsync( - engineId: "engine1", - modelRevision: 1, - corpusId: "corpus1", - textId: "MAT", - textOrigin: textOrigin, - template: template - ) - ).Replace("\r\n", "\n"); + string usfm = await Service.GetUsfmAsync( + engineId: "engine1", + modelRevision: 1, + corpusId: "corpus1", + textId: "MAT", + textOrigin: textOrigin, + template: template + ); + return usfm.Replace("\r\n", "\n"); } public void AddMatthewToTarget() { - ScriptureDataFileService - .ReadParatextProjectBookAsync("file2.zip", "MAT") - .Returns(Task.FromResult(TargetUsfm)); + TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(StringToStream(TargetUsfm)); } private static ParatextProjectSettings CreateProjectSettings(string name) From 114912b8cf48fc5dffc611d3d9b48af886a8275f Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Fri, 2 Aug 2024 07:56:31 -0400 Subject: [PATCH 07/15] Remove unnecessary method; add test to scripture data file service --- .../Services/IScriptureDataFileService.cs | 1 - .../Services/ScriptureDataFileService.cs | 11 ---------- .../Services/ScriptureDataFileServiceTests.cs | 21 +++++------------- .../Services/PretranslationServiceTests.cs | 22 +++---------------- 4 files changed, 9 insertions(+), 46 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index bd632e84..be36b66d 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -4,5 +4,4 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); - Task ReadParatextProjectBookAsync(string filename, string book); } diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 7a580984..9f633a02 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -18,17 +18,6 @@ public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string fil return new ZipParatextProjectTextUpdater(container); } - public async Task ReadParatextProjectBookAsync(string filename, string book) - { - using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); - ParatextProjectSettings settings = ParseProjectSettings(container); - string entryName = settings.GetBookFileName(book); - if (!container.EntryExists(entryName)) - return null; - using StreamReader reader = new(container.OpenEntry(entryName)); - return await reader.ReadToEndAsync(); - } - private string GetFilePath(string filename) { return Path.Combine(_dataFileOptions.CurrentValue.FilesDirectory, filename); diff --git a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs index 83f55aa7..091fae66 100644 --- a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs +++ b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs @@ -12,16 +12,15 @@ public void GetParatextProjectSettings() } [Test] - public async Task ReadParatextProjectBookAsync_Exists() + public void GetZipParatextProjectTextUpdater() { TestEnvironment env = new(); - string? usfm = await env.Service.ReadParatextProjectBookAsync("file1.zip", "MAT"); - Assert.That(usfm, Is.Not.Null); + ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); Assert.That( - usfm.Replace("\r\n", "\n"), + updater.UpdateUsfm("MAT", [], preferExistingText: true), Is.EqualTo( - @"\id MAT - PROJ -\h Matthew + $@"\id MAT - PROJ +\h {Canon.BookIdToEnglishName("MAT")} \c 1 \p \v 1 Chapter one, verse one. @@ -30,19 +29,11 @@ public async Task ReadParatextProjectBookAsync_Exists() \p \v 1 Chapter two, verse one. \v 2 Chapter two, verse two. -".Replace("\r\n", "\n") +".Replace("\n", "\r\n") ) ); } - [Test] - public async Task ReadParatextProjectBookAsync_DoesNotExist() - { - TestEnvironment env = new(); - string? usfm = await env.Service.ReadParatextProjectBookAsync("file1.zip", "MRK"); - Assert.That(usfm, Is.Null); - } - private class TestEnvironment { public TestEnvironment() diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index aa987072..3b328b25 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -340,16 +340,10 @@ public TestEnvironment() ScriptureDataFileService = Substitute.For(); ScriptureDataFileService.GetParatextProjectSettings("file1.zip").Returns(CreateProjectSettings("SRC")); ScriptureDataFileService.GetParatextProjectSettings("file2.zip").Returns(CreateProjectSettings("TRG")); - ScriptureDataFileService - .ReadParatextProjectBookAsync("file1.zip", "MAT") - .Returns(Task.FromResult(SourceUsfm)); - ScriptureDataFileService - .ReadParatextProjectBookAsync("file2.zip", "MAT") - .Returns(Task.FromResult(null)); var zipSubstituteSource = Substitute.For(); var zipSubstituteTarget = Substitute.For(); - zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(StringToStream(SourceUsfm)); - zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(StringToStream("")); + zipSubstituteSource.OpenEntry("MATSRC.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(SourceUsfm))); + zipSubstituteTarget.OpenEntry("MATTRG.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(""))); zipSubstituteSource.EntryExists(Arg.Any()).Returns(false); zipSubstituteTarget.EntryExists(Arg.Any()).Returns(false); zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); @@ -374,16 +368,6 @@ public TestEnvironment() public IScriptureDataFileService ScriptureDataFileService { get; } public IZipContainer TargetZipContainer { get; } - private static Stream StringToStream(string s) - { - var stream = new MemoryStream(); - var streamWriter = new StreamWriter(stream); - streamWriter.Write(s); - streamWriter.Flush(); - stream.Seek(0, SeekOrigin.Begin); - return stream; - } - public async Task GetUsfmAsync( PretranslationUsfmTextOrigin textOrigin, PretranslationUsfmTemplate template @@ -402,7 +386,7 @@ PretranslationUsfmTemplate template public void AddMatthewToTarget() { - TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(StringToStream(TargetUsfm)); + TargetZipContainer.OpenEntry("MATTRG.SFM").Returns(new MemoryStream(Encoding.UTF8.GetBytes(TargetUsfm))); } private static ParatextProjectSettings CreateProjectSettings(string name) From ca37ef97fd048e9a5e0c1c92fd9344e5c4c78e02 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Mon, 5 Aug 2024 13:58:45 -0400 Subject: [PATCH 08/15] Review feedback --- .../src/Serval.Shared/Services/IScriptureDataFileService.cs | 2 +- .../src/Serval.Translation/Services/PretranslationService.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs index be36b66d..08424a55 100644 --- a/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/IScriptureDataFileService.cs @@ -3,5 +3,5 @@ public interface IScriptureDataFileService { ParatextProjectSettings GetParatextProjectSettings(string filename); - public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); + ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename); } diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index fdf32c50..cec34a3c 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -119,7 +119,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) break; } // In order to support PretranslationUsfmTemplate.Auto - if (usfm != "") + if (usfm != null) return usfm; } From 99ef2e10bcef1b5a03a8f10054aeb405bcc18537 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Mon, 5 Aug 2024 16:43:57 -0400 Subject: [PATCH 09/15] Make zip text updater disposable to avoid disposing zipcontainer; use isnullorempty --- .../Serval.Shared/Services/ScriptureDataFileService.cs | 2 +- .../Services/ZipParatextProjectTextUpdater.cs | 8 +++++++- .../Serval.Translation/Services/PretranslationService.cs | 6 +++--- .../Services/ScriptureDataFileServiceTests.cs | 2 +- .../Services/PretranslationServiceTests.cs | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs index 9f633a02..e38b8c08 100644 --- a/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs +++ b/src/Serval/src/Serval.Shared/Services/ScriptureDataFileService.cs @@ -14,7 +14,7 @@ public ParatextProjectSettings GetParatextProjectSettings(string filename) public ZipParatextProjectTextUpdater GetZipParatextProjectTextUpdater(string filename) { - using IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); + IZipContainer container = _fileSystem.OpenZipFile(GetFilePath(filename)); return new ZipParatextProjectTextUpdater(container); } diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs index 1a905d6c..d8ebd7d1 100644 --- a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -1,6 +1,6 @@ namespace Serval.Shared.Services; -public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase +public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase, IDisposable { public ZipParatextProjectTextUpdater(IZipContainer container) : base(new ZipParatextProjectSettingsParser(container)) @@ -25,4 +25,10 @@ protected override Stream Open(string fileName) { return _projectContainer.OpenEntry(fileName); } + + public void Dispose() + { + _projectContainer.Dispose(); + GC.SuppressFinalize(this); + } } diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index cec34a3c..811fa87b 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -76,7 +76,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) pretranslations = pretranslations.Select(p => ((IReadOnlyList)p.Refs.Select(r => r.ToRelaxed()).ToArray(), p.Translation) ); - Shared.Services.ZipParatextProjectTextUpdater updater = + using Shared.Services.ZipParatextProjectTextUpdater updater = _scriptureDataFileService.GetZipParatextProjectTextUpdater(targetFile.Filename); string usfm = ""; switch (textOrigin) @@ -119,13 +119,13 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) break; } // In order to support PretranslationUsfmTemplate.Auto - if (usfm != null) + if (!string.IsNullOrEmpty(usfm)) return usfm; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) { - Shared.Services.ZipParatextProjectTextUpdater updater = + using Shared.Services.ZipParatextProjectTextUpdater updater = _scriptureDataFileService.GetZipParatextProjectTextUpdater(sourceFile.Filename); // Copy and update the source book if it exists diff --git a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs index 091fae66..be7481ec 100644 --- a/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs +++ b/src/Serval/test/Serval.Shared.Tests/Services/ScriptureDataFileServiceTests.cs @@ -15,7 +15,7 @@ public void GetParatextProjectSettings() public void GetZipParatextProjectTextUpdater() { TestEnvironment env = new(); - ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); + using ZipParatextProjectTextUpdater updater = env.Service.GetZipParatextProjectTextUpdater("file1.zip"); Assert.That( updater.UpdateUsfm("MAT", [], preferExistingText: true), Is.EqualTo( diff --git a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs index 3b328b25..b2f08824 100644 --- a/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs +++ b/src/Serval/test/Serval.Translation.Tests/Services/PretranslationServiceTests.cs @@ -349,11 +349,11 @@ public TestEnvironment() zipSubstituteSource.EntryExists("MATSRC.SFM").Returns(true); zipSubstituteTarget.EntryExists("MATTRG.SFM").Returns(true); TargetZipContainer = zipSubstituteTarget; - var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( + using var textUpdaterSource = new Shared.Services.ZipParatextProjectTextUpdater( zipSubstituteSource, CreateProjectSettings("SRC") ); - var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( + using var textUpdaterTarget = new Shared.Services.ZipParatextProjectTextUpdater( zipSubstituteTarget, CreateProjectSettings("TRG") ); From 32bd91ec796e4bbec529a8c17f2918d03c89153b Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Tue, 6 Aug 2024 12:47:16 -0400 Subject: [PATCH 10/15] Use disposable pattern --- .../Services/ZipParatextProjectTextUpdater.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs index d8ebd7d1..b7fb4d5d 100644 --- a/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs +++ b/src/Serval/src/Serval.Shared/Services/ZipParatextProjectTextUpdater.cs @@ -2,6 +2,8 @@ namespace Serval.Shared.Services; public class ZipParatextProjectTextUpdater : ParatextProjectTextUpdaterBase, IDisposable { + private bool _disposed; + public ZipParatextProjectTextUpdater(IZipContainer container) : base(new ZipParatextProjectSettingsParser(container)) { @@ -28,7 +30,19 @@ protected override Stream Open(string fileName) public void Dispose() { - _projectContainer.Dispose(); + Dispose(true); GC.SuppressFinalize(this); } + + protected virtual void Dispose(bool disposing) + { + if (!_disposed) + { + if (disposing) + { + _projectContainer.Dispose(); + } + _disposed = true; + } + } } From 00cd43a5786e9fa6d505779247da2f0cbce32229 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Tue, 6 Aug 2024 18:33:57 -0400 Subject: [PATCH 11/15] Upgrade to machine library version --- .../src/Serval.Machine.Shared/Serval.Machine.Shared.csproj | 6 +++--- src/Serval/src/Serval.Shared/Serval.Shared.csproj | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj b/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj index bdd410f8..08281b26 100644 --- a/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj +++ b/src/Machine/src/Serval.Machine.Shared/Serval.Machine.Shared.csproj @@ -37,9 +37,9 @@ - - - + + + diff --git a/src/Serval/src/Serval.Shared/Serval.Shared.csproj b/src/Serval/src/Serval.Shared/Serval.Shared.csproj index 30b7cc35..0270f7df 100644 --- a/src/Serval/src/Serval.Shared/Serval.Shared.csproj +++ b/src/Serval/src/Serval.Shared/Serval.Shared.csproj @@ -19,7 +19,7 @@ - + From cf3274a301b1b0da8e8cabd09de00b42920201b5 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Wed, 7 Aug 2024 14:27:32 -0400 Subject: [PATCH 12/15] Fix broken test --- .../src/Serval.Translation/Services/PretranslationService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index 811fa87b..e381e759 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -120,7 +120,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) } // In order to support PretranslationUsfmTemplate.Auto if (!string.IsNullOrEmpty(usfm)) - return usfm; + return ""; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) From 095e1cc5f4429cbb9e362224325d893e4e39e2e5 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Wed, 7 Aug 2024 18:03:02 -0400 Subject: [PATCH 13/15] Fix tests --- .../src/Serval.Translation/Services/PretranslationService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index e381e759..811fa87b 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -120,7 +120,7 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) } // In order to support PretranslationUsfmTemplate.Auto if (!string.IsNullOrEmpty(usfm)) - return ""; + return usfm; } if (template is PretranslationUsfmTemplate.Auto or PretranslationUsfmTemplate.Source) From 83f24719d09d0e11cf00201de238a67d0961dde6 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Wed, 7 Aug 2024 18:12:56 -0400 Subject: [PATCH 14/15] Fix tests --- .../Services/PretranslationService.cs | 84 ++++++++++--------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs index 811fa87b..48e89b91 100644 --- a/src/Serval/src/Serval.Translation/Services/PretranslationService.cs +++ b/src/Serval/src/Serval.Translation/Services/PretranslationService.cs @@ -82,40 +82,44 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) switch (textOrigin) { case PretranslationUsfmTextOrigin.PreferExisting: - usfm = updater.UpdateUsfm( - textId, - pretranslations.ToList(), - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: true - ); + usfm = + updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: true + ) ?? ""; break; case PretranslationUsfmTextOrigin.PreferPretranslated: - usfm = updater.UpdateUsfm( - textId, - pretranslations.ToList(), - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); + usfm = + updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ) ?? ""; break; case PretranslationUsfmTextOrigin.OnlyExisting: - usfm = updater.UpdateUsfm( - textId, - [], // don't put any pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: false, - preferExistingText: false - ); + usfm = + updater.UpdateUsfm( + textId, + [], // don't put any pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: false, + preferExistingText: false + ) ?? ""; break; case PretranslationUsfmTextOrigin.OnlyPretranslated: - usfm = updater.UpdateUsfm( - textId, - pretranslations.ToList(), - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: false - ); + usfm = + updater.UpdateUsfm( + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: false + ) ?? ""; break; } // In order to support PretranslationUsfmTemplate.Auto @@ -135,20 +139,20 @@ await GetAllAsync(engineId, modelRevision, corpusId, textId, cancellationToken) case PretranslationUsfmTextOrigin.PreferPretranslated: case PretranslationUsfmTextOrigin.OnlyPretranslated: return updater.UpdateUsfm( - textId, - pretranslations.ToList(), - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); + textId, + pretranslations.ToList(), + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ) ?? ""; case PretranslationUsfmTextOrigin.OnlyExisting: return updater.UpdateUsfm( - textId, - [], // don't pass the pretranslations, we only want the existing text. - fullName: targetSettings.FullName, - stripAllText: true, - preferExistingText: true - ); + textId, + [], // don't pass the pretranslations, we only want the existing text. + fullName: targetSettings.FullName, + stripAllText: true, + preferExistingText: true + ) ?? ""; } } From 95e51119a85702a65b5085bd5c12381360907a7d Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Wed, 7 Aug 2024 18:12:57 -0400 Subject: [PATCH 15/15] Fix tests --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index 10918d17..8a599c18 100644 --- a/.gitignore +++ b/.gitignore @@ -45,3 +45,7 @@ artifacts .db *.zip +*__pycache__* +*engine_data* +*src_test* +*trg_test* \ No newline at end of file