From c7a2cca7b435949352de290627bad53e889d6cd0 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 22 Oct 2023 22:40:40 -0700 Subject: [PATCH 1/8] Add a test that demonstrates the issue --- .../Automation/CommandLineIntegrationTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index bf8e8bb26c..bb1180cdfb 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -282,6 +282,40 @@ await _automationHelper.RunCommand( AssertRecoveryCleanedUp(); } + [Fact] + public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() + { + var path = $"{FolderPath}/test.pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); + Assert.False(File.Exists($"{FolderPath}/test.4.pdf")); + Assert.False(File.Exists($"{FolderPath}/test.5.pdf")); + Assert.False(File.Exists($"{FolderPath}/test.6.pdf")); + AssertRecoveryCleanedUp(); + } + [Fact] public async Task MultipleImages() { From fb1a110c7668f361dc1874414fcfd0b644d250ce Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Sun, 22 Oct 2023 22:44:57 -0700 Subject: [PATCH 2/8] Utilize the force overwrite option during placeholder substitution --- NAPS2.Lib/Automation/AutomatedScanning.cs | 2 +- NAPS2.Lib/Pdf/SavePdfOperation.cs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/NAPS2.Lib/Automation/AutomatedScanning.cs b/NAPS2.Lib/Automation/AutomatedScanning.cs index f9f7c389fe..38dff62859 100644 --- a/NAPS2.Lib/Automation/AutomatedScanning.cs +++ b/NAPS2.Lib/Automation/AutomatedScanning.cs @@ -585,7 +585,7 @@ private async Task DoExportToPdf(string path, bool email) } }; int digits = (int) Math.Floor(Math.Log10(_scanList.Count)) + 1; - string actualPath = _placeholders.Substitute(path, true, scanIndex++, _scanList.Count > 1 ? digits : 0); + string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, scanIndex++, _scanList.Count > 1 ? digits : 0); op.Start(actualPath, _placeholders, fileContents, _config.Get(c => c.PdfSettings), _ocrParams); if (!await op.Success) { diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index e149b1c3d8..1101503301 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -72,8 +72,9 @@ public bool Start(string fileName, Placeholders placeholders, ICollection Date: Sun, 22 Oct 2023 23:36:37 -0700 Subject: [PATCH 3/8] Add the NoOverwrite test --- .../Automation/CommandLineIntegrationTests.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index bb1180cdfb..5bac758ae1 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -282,6 +282,38 @@ await _automationHelper.RunCommand( AssertRecoveryCleanedUp(); } + [Fact] + public async Task ExistingFile_NoOverwrite_SplitWithNoPlaceholder() + { + var path = $"{FolderPath}/test.pdf"; + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + await _automationHelper.RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); + Assert.True(File.Exists($"{FolderPath}/test.4.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.5.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.6.pdf")); + AssertRecoveryCleanedUp(); + } + [Fact] public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() { From 4d65b4bcd7adad57e0bccb676bb08f50364944f8 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Mon, 23 Oct 2023 18:09:04 -0700 Subject: [PATCH 4/8] Remove confirm overwrite from PDF save --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 1101503301..776f4eb93a 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -70,11 +70,10 @@ public bool Start(string fileName, Placeholders placeholders, ICollection Date: Tue, 24 Oct 2023 00:42:49 -0700 Subject: [PATCH 5/8] Add WithPlaceholder tests --- .../Automation/CommandLineIntegrationTests.cs | 102 +++++++++++++++--- 1 file changed, 90 insertions(+), 12 deletions(-) diff --git a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs index 5bac758ae1..bf6ef1e794 100644 --- a/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs +++ b/NAPS2.Lib.Tests/Automation/CommandLineIntegrationTests.cs @@ -283,9 +283,9 @@ await _automationHelper.RunCommand( } [Fact] - public async Task ExistingFile_NoOverwrite_SplitWithNoPlaceholder() + public async Task ExistingFile_NoOverwrite_SplitWithPlaceholder() { - var path = $"{FolderPath}/test.pdf"; + var path = $"{FolderPath}/test$(n).pdf"; await _automationHelper.RunCommand( new AutomatedScanningOptions { @@ -295,39 +295,104 @@ await _automationHelper.RunCommand( Verbose = true }, new[] { Image1, Image2, Image3 }); + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + PdfAsserts.AssertImages($"{FolderPath}/test1.pdf", Image1); + PdfAsserts.AssertImages($"{FolderPath}/test2.pdf", Image2); + PdfAsserts.AssertImages($"{FolderPath}/test3.pdf", Image3); + Assert.True(File.Exists($"{FolderPath}/test4.pdf")); + Assert.True(File.Exists($"{FolderPath}/test5.pdf")); + Assert.True(File.Exists($"{FolderPath}/test6.pdf")); + AssertRecoveryCleanedUp(); + } + + [Fact] + public async Task ExistingFile_ForceOverwrite_SplitWithPlaceholder() + { + var path = $"{FolderPath}/test$(n).pdf"; await _automationHelper.RunCommand( new AutomatedScanningOptions { OutputPath = path, + ForceOverwrite = true, ProfileName = string.Empty, Split = true, Verbose = true }, new[] { Image1, Image2, Image3 }); - - PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); - PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); - PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); - Assert.True(File.Exists($"{FolderPath}/test.4.pdf")); - Assert.True(File.Exists($"{FolderPath}/test.5.pdf")); - Assert.True(File.Exists($"{FolderPath}/test.6.pdf")); + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + PdfAsserts.AssertImages($"{FolderPath}/test1.pdf", Image1); + PdfAsserts.AssertImages($"{FolderPath}/test2.pdf", Image2); + PdfAsserts.AssertImages($"{FolderPath}/test3.pdf", Image3); + Assert.False(File.Exists($"{FolderPath}/test4.pdf")); + Assert.False(File.Exists($"{FolderPath}/test5.pdf")); + Assert.False(File.Exists($"{FolderPath}/test6.pdf")); AssertRecoveryCleanedUp(); } [Fact] - public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() + public async Task ExistingFile_NoOverwrite_SplitWithNoPlaceholder() { var path = $"{FolderPath}/test.pdf"; await _automationHelper.RunCommand( new AutomatedScanningOptions { OutputPath = path, - ForceOverwrite = true, ProfileName = string.Empty, Split = true, Verbose = true }, new[] { Image1, Image2, Image3 }); + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); + PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); + Assert.True(File.Exists($"{FolderPath}/test.4.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.5.pdf")); + Assert.True(File.Exists($"{FolderPath}/test.6.pdf")); + AssertRecoveryCleanedUp(); + } + + [Fact] + public async Task ExistingFile_ForceOverwrite_SplitWithNoPlaceholder() + { + var path = $"{FolderPath}/test.pdf"; await _automationHelper.RunCommand( new AutomatedScanningOptions { @@ -338,7 +403,20 @@ await _automationHelper.RunCommand( Verbose = true }, new[] { Image1, Image2, Image3 }); - + await _automationHelper.WithContainer(container => + { + var profileManager = container.Resolve(); + profileManager.Profiles.Remove(profileManager.Profiles.Where(p => p.IsDefault).First()); + }).RunCommand( + new AutomatedScanningOptions + { + OutputPath = path, + ForceOverwrite = true, + ProfileName = string.Empty, + Split = true, + Verbose = true + }, + new[] { Image1, Image2, Image3 }); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.1.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.2.pdf"); PdfAsserts.AssertPageCount(1, $"{FolderPath}/test.3.pdf"); From 29ab522b38a06acf474954878f2f99dd589852b9 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:49:50 -0700 Subject: [PATCH 6/8] Add incrementPlaceholderIfExists substitution parameter -Used to determine whether to increment the placeholder number. --- NAPS2.Lib/Automation/AutomatedScanning.cs | 3 ++- NAPS2.Sdk/ImportExport/Placeholders.cs | 11 ++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/NAPS2.Lib/Automation/AutomatedScanning.cs b/NAPS2.Lib/Automation/AutomatedScanning.cs index 38dff62859..d008044bd9 100644 --- a/NAPS2.Lib/Automation/AutomatedScanning.cs +++ b/NAPS2.Lib/Automation/AutomatedScanning.cs @@ -585,7 +585,8 @@ private async Task DoExportToPdf(string path, bool email) } }; int digits = (int) Math.Floor(Math.Log10(_scanList.Count)) + 1; - string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, scanIndex++, _scanList.Count > 1 ? digits : 0); + string actualPath = _placeholders.Substitute(path, !_options.ForceOverwrite, + scanIndex++, _scanList.Count > 1 ? digits : 0, !_options.ForceOverwrite); op.Start(actualPath, _placeholders, fileContents, _config.Get(c => c.PdfSettings), _ocrParams); if (!await op.Success) { diff --git a/NAPS2.Sdk/ImportExport/Placeholders.cs b/NAPS2.Sdk/ImportExport/Placeholders.cs index b0f82bf2a0..be54902435 100644 --- a/NAPS2.Sdk/ImportExport/Placeholders.cs +++ b/NAPS2.Sdk/ImportExport/Placeholders.cs @@ -46,21 +46,22 @@ internal abstract class Placeholders /// Whether to use an auto-incrementing file number to make the file name unique. /// The file number will be at least one bigger than this value. /// The minimum number of digits in the file number. Only has an effect if the path does not contain a numeric placeholder like $(n) or $(nnn). + /// Whether to increment the placeholder number to make the file name unique. /// The file path with substitutions. [return: NotNullIfNotNull("filePath")] public abstract string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0); + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true); public class StubPlaceholders : Placeholders { public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) => filePath; + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) => filePath; } public class EnvironmentPlaceholders : Placeholders { public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) { if (filePath == null) return null; return Environment.ExpandEnvironmentVariables(filePath); @@ -99,7 +100,7 @@ public DefaultPlaceholders(DateTime? dateTimeOverride = null) [return: NotNullIfNotNull("filePath")] public override string? Substitute(string? filePath, bool incrementIfExists = true, int numberSkip = 0, - int autoNumberDigits = 0) + int autoNumberDigits = 0, bool incrementPlaceholderIfExists = true) { if (filePath == null) { @@ -116,7 +117,7 @@ public DefaultPlaceholders(DateTime? dateTimeOverride = null) if (match.Success) { result = NumberPlaceholderPattern.Replace(result, ""); - result = SubstituteNumber(result, match.Index, match.Length - 3, numberSkip, true); + result = SubstituteNumber(result, match.Index, match.Length - 3, numberSkip, incrementPlaceholderIfExists); } else if (autoNumberDigits > 0) { From 5344dc519e5a6e889ce04c713a16186ee979a1d1 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Tue, 24 Oct 2023 00:55:50 -0700 Subject: [PATCH 7/8] Include the original todo message --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 776f4eb93a..7989ce8b42 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -74,6 +74,7 @@ public bool Start(string fileName, Placeholders placeholders, ICollection Date: Tue, 24 Oct 2023 00:57:29 -0700 Subject: [PATCH 8/8] Fix original spacing --- NAPS2.Lib/Pdf/SavePdfOperation.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/NAPS2.Lib/Pdf/SavePdfOperation.cs b/NAPS2.Lib/Pdf/SavePdfOperation.cs index 7989ce8b42..89d15aac5c 100644 --- a/NAPS2.Lib/Pdf/SavePdfOperation.cs +++ b/NAPS2.Lib/Pdf/SavePdfOperation.cs @@ -70,7 +70,6 @@ public bool Start(string fileName, Placeholders placeholders, ICollection