From f034430b0cefcee068201d6af071ebed86b8695e Mon Sep 17 00:00:00 2001 From: Eli Lindner Date: Mon, 1 Jun 2026 14:08:56 +1000 Subject: [PATCH 1/5] test: add TemplateValidator test scaffold and expected behaviors --- BatchExcel.Tests/TemplateValidatorTests.cs | 122 +++++++++++++++++++++ BatchExcel/Services/TemplateValidator.cs | 16 +++ 2 files changed, 138 insertions(+) create mode 100644 BatchExcel.Tests/TemplateValidatorTests.cs create mode 100644 BatchExcel/Services/TemplateValidator.cs diff --git a/BatchExcel.Tests/TemplateValidatorTests.cs b/BatchExcel.Tests/TemplateValidatorTests.cs new file mode 100644 index 0000000..0af8b62 --- /dev/null +++ b/BatchExcel.Tests/TemplateValidatorTests.cs @@ -0,0 +1,122 @@ +using BatchExcel.Models; +using BatchExcel.Services; +using ClosedXML.Excel; + +namespace BatchExcel.Tests; + +public class TemplateValidatorTests : IDisposable +{ + private readonly string _tempPath; + + public TemplateValidatorTests() + { + _tempPath = Path.Combine(Path.GetTempPath(), $"test_template_{Guid.NewGuid()}.xlsx"); + } + + public void Dispose() + { + if (File.Exists(_tempPath)) + File.Delete(_tempPath); + } + + private void CreateTestTemplate(string[] sheets, Dictionary? namedRanges = null) + { + using var workbook = new XLWorkbook(); + foreach (var sheetName in sheets) + { + workbook.Worksheets.Add(sheetName); + } + + if (namedRanges != null) + { + foreach (var nr in namedRanges) + { + // Simple named range to a cell in the first sheet + workbook.DefinedNames.Add(nr.Key, $"{sheets[0]}!$A$1"); + } + } + + workbook.SaveAs(_tempPath); + } + + [Fact] + public void Validate_ValidTemplate_DoesNotThrow() + { + // Arrange + CreateTestTemplate(new[] { "Sheet1", "Main" }, new Dictionary { ["JobNumber"] = "Sheet1!$A$1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "A1", 0) }, + OutputFields = { new FieldDefinition("Main", "B2", 0) }, + HeaderInputs = { ["JobNumber"] = "123" } + }; + + // Act & Assert + TemplateValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_MissingSheet_ThrowsValidationException() + { + // Arrange + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("MissingSheet", "A1", 0) } + }; + + // Act & Assert + var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); + Assert.Contains("Missing sheet 'MissingSheet'", ex.Message); + } + + [Fact] + public void Validate_MissingNamedRange_ThrowsValidationException() + { + // Arrange + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + HeaderInputs = { ["MissingRange"] = "val" } + }; + + // Act & Assert + var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); + Assert.Contains("Missing named range 'MissingRange'", ex.Message); + } + + [Fact] + public void Validate_FileInUse_CanStillRead() + { + // Arrange + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "A1", 0) } + }; + + // Open the file exclusively to simulate it being open in Excel + using var fs = new FileStream(_tempPath, FileMode.Open, FileAccess.Read, FileShare.None); + + // Act & Assert + // This should NOT throw IOException if we use FileShare.ReadWrite in the validator + TemplateValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_AggregatesMultipleErrors() + { + // Arrange + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("MissingSheet", "A1", 0) }, + HeaderInputs = { ["MissingRange"] = "val" } + }; + + // Act & Assert + var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); + Assert.Contains("Missing sheet 'MissingSheet'", ex.Message); + Assert.Contains("Missing named range 'MissingRange'", ex.Message); + } +} diff --git a/BatchExcel/Services/TemplateValidator.cs b/BatchExcel/Services/TemplateValidator.cs new file mode 100644 index 0000000..2c850ea --- /dev/null +++ b/BatchExcel/Services/TemplateValidator.cs @@ -0,0 +1,16 @@ +using BatchExcel.Models; + +namespace BatchExcel.Services; + +public class ValidationException : Exception +{ + public ValidationException(string message) : base(message) { } +} + +public static class TemplateValidator +{ + public static void Validate(string calculationPath, BatchConfig config) + { + // Stub for compilation + } +} From 9a72c89691daa73e7a7c78d3cfc53edaa82ca9b9 Mon Sep 17 00:00:00 2001 From: Eli Lindner Date: Mon, 1 Jun 2026 14:09:31 +1000 Subject: [PATCH 2/5] feat: implement OpenXML TemplateValidator service --- BatchExcel/Services/TemplateValidator.cs | 62 +++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/BatchExcel/Services/TemplateValidator.cs b/BatchExcel/Services/TemplateValidator.cs index 2c850ea..ac23117 100644 --- a/BatchExcel/Services/TemplateValidator.cs +++ b/BatchExcel/Services/TemplateValidator.cs @@ -1,4 +1,7 @@ +using System.IO; using BatchExcel.Models; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Spreadsheet; namespace BatchExcel.Services; @@ -9,8 +12,65 @@ public ValidationException(string message) : base(message) { } public static class TemplateValidator { + /// + /// Validates that all sheets and named ranges referenced in the configuration exist + /// in the calculation template. Opens the file with non-locking shared access to + /// avoid exceptions if the file is open in Excel. + /// public static void Validate(string calculationPath, BatchConfig config) { - // Stub for compilation + var errors = new List(); + + // Open with FileShare.ReadWrite to avoid locking issues if open in Excel + using var fs = new FileStream(calculationPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); + using var document = SpreadsheetDocument.Open(fs, isEditable: false); + + var workbookPart = document.WorkbookPart ?? throw new InvalidOperationException("Workbook part not found."); + + // Extract sheet names + var sheets = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var s in workbookPart.Workbook.Descendants()) + { + if (s.Name?.Value != null) + sheets.Add(s.Name.Value); + } + + // Extract defined names (named ranges) + var definedNames = new HashSet(StringComparer.OrdinalIgnoreCase); + if (workbookPart.Workbook.DefinedNames != null) + { + foreach (var dn in workbookPart.Workbook.DefinedNames.Elements()) + { + if (dn.Name?.Value != null) + definedNames.Add(dn.Name.Value); + } + } + + // Validate Input Fields + foreach (var field in config.InputFields) + { + if (!sheets.Contains(field.Sheet)) + errors.Add($"Missing sheet '{field.Sheet}' (referenced by input field '{field.Range}')"); + } + + // Validate Output Fields + foreach (var field in config.OutputFields) + { + if (!sheets.Contains(field.Sheet)) + errors.Add($"Missing sheet '{field.Sheet}' (referenced by output field '{field.Range}')"); + } + + // Validate Header Inputs (Named Ranges) + foreach (var rangeName in config.HeaderInputs.Keys) + { + // "Date" is a special case written automatically, but it still needs a defined name + if (!definedNames.Contains(rangeName)) + errors.Add($"Missing named range '{rangeName}' (referenced by project header)"); + } + + if (errors.Count > 0) + { + throw new ValidationException("Template validation failed:\n- " + string.Join("\n- ", errors)); + } } } From 86f73ca91d4e9f1035ec21578cda67d080c4851b Mon Sep 17 00:00:00 2001 From: Eli Lindner Date: Mon, 1 Jun 2026 14:09:45 +1000 Subject: [PATCH 3/5] feat: integrate TemplateValidator into BatchEngine workflow --- BatchExcel/Services/BatchEngine.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/BatchExcel/Services/BatchEngine.cs b/BatchExcel/Services/BatchEngine.cs index 55761ae..96144eb 100644 --- a/BatchExcel/Services/BatchEngine.cs +++ b/BatchExcel/Services/BatchEngine.cs @@ -65,6 +65,9 @@ public async Task RunAsync(string batcherFilePath, int workerCount, bool saveRun if (!File.Exists(calculationFullPath)) throw new FileNotFoundException($"Calculation spreadsheet not found: {calculationFullPath}"); + // Validate template exists and contains all required sheets/ranges (Dry Run) + TemplateValidator.Validate(calculationFullPath, config); + // Step 3: Create output folder var outFolder = Path.Combine(batcherDir, $"batch_run_{timestamp}"); Directory.CreateDirectory(outFolder); From 17d223e138768dff4e8d06321874c3e1ae61e0cb Mon Sep 17 00:00:00 2001 From: Eli Lindner Date: Mon, 1 Jun 2026 14:36:04 +1000 Subject: [PATCH 4/5] fix: refine dry-run validation logic and fix cross-platform test stability --- BatchExcel.Tests/TemplateValidatorTests.cs | 41 +++++++++++++++---- .../Services/CalculationHeaderWriter.cs | 29 +------------ BatchExcel/Services/OpenXmlHelpers.cs | 32 +++++++++++++++ BatchExcel/Services/TemplateValidator.cs | 27 ++++-------- 4 files changed, 73 insertions(+), 56 deletions(-) diff --git a/BatchExcel.Tests/TemplateValidatorTests.cs b/BatchExcel.Tests/TemplateValidatorTests.cs index 0af8b62..2c63ea2 100644 --- a/BatchExcel.Tests/TemplateValidatorTests.cs +++ b/BatchExcel.Tests/TemplateValidatorTests.cs @@ -71,7 +71,7 @@ public void Validate_MissingSheet_ThrowsValidationException() } [Fact] - public void Validate_MissingNamedRange_ThrowsValidationException() + public void Validate_MissingNamedRange_IsSilentlyIgnored() { // Arrange CreateTestTemplate(new[] { "Sheet1" }); @@ -81,8 +81,31 @@ public void Validate_MissingNamedRange_ThrowsValidationException() }; // Act & Assert - var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); - Assert.Contains("Missing named range 'MissingRange'", ex.Message); + // Should not throw, matching CalculationHeaderWriter behavior + TemplateValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_UnsupportedNamedRangeFormula_IsIgnored() + { + // Arrange + // Create a template with a named range that uses a formula (OFFSET) which our parser ignores + using (var workbook = new XLWorkbook()) + { + workbook.Worksheets.Add("Sheet1"); + workbook.DefinedNames.Add("ComplexRange", "OFFSET(Sheet1!$A$1,0,0,1,1)"); + workbook.SaveAs(_tempPath); + } + + var config = new BatchConfig + { + HeaderInputs = { ["ComplexRange"] = "val" } + }; + + // Act & Assert + // Should not throw, but should also not be considered "writeable" if we were checking writeability. + // For now, we just ensure it doesn't cause a false error or a crash. + TemplateValidator.Validate(_tempPath, config); } [Fact] @@ -95,8 +118,8 @@ public void Validate_FileInUse_CanStillRead() InputFields = { new FieldDefinition("Sheet1", "A1", 0) } }; - // Open the file exclusively to simulate it being open in Excel - using var fs = new FileStream(_tempPath, FileMode.Open, FileAccess.Read, FileShare.None); + // Open the file to simulate it being open in Excel (allowing shared reading) + using var fs = new FileStream(_tempPath, FileMode.Open, FileAccess.Read, FileShare.Read); // Act & Assert // This should NOT throw IOException if we use FileShare.ReadWrite in the validator @@ -110,13 +133,13 @@ public void Validate_AggregatesMultipleErrors() CreateTestTemplate(new[] { "Sheet1" }); var config = new BatchConfig { - InputFields = { new FieldDefinition("MissingSheet", "A1", 0) }, - HeaderInputs = { ["MissingRange"] = "val" } + InputFields = { new FieldDefinition("MissingSheet1", "A1", 0) }, + OutputFields = { new FieldDefinition("MissingSheet2", "B2", 0) } }; // Act & Assert var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); - Assert.Contains("Missing sheet 'MissingSheet'", ex.Message); - Assert.Contains("Missing named range 'MissingRange'", ex.Message); + Assert.Contains("Missing sheet 'MissingSheet1'", ex.Message); + Assert.Contains("Missing sheet 'MissingSheet2'", ex.Message); } } diff --git a/BatchExcel/Services/CalculationHeaderWriter.cs b/BatchExcel/Services/CalculationHeaderWriter.cs index dcdb18b..94d8c76 100644 --- a/BatchExcel/Services/CalculationHeaderWriter.cs +++ b/BatchExcel/Services/CalculationHeaderWriter.cs @@ -27,7 +27,7 @@ public static void Write( var workbookPart = document.WorkbookPart ?? throw new InvalidOperationException("Workbook part not found."); - var definedNames = BuildDefinedNamesLookup(workbookPart); + var definedNames = OpenXmlHelpers.BuildDefinedNamesLookup(workbookPart); var sheetLookup = BuildWorksheetLookup(workbookPart); // Cache one SheetWriter per worksheet so multiple writes to the same sheet share an index. @@ -67,33 +67,6 @@ SheetWriter GetWriter(WorksheetPart wsPart) } } - /// - /// Builds a lookup of defined name → (sheet name, cell reference). - /// Parses references like "SheetName!$B$5" into their components. - /// - private static Dictionary BuildDefinedNamesLookup(WorkbookPart workbookPart) - { - var definedNames = new Dictionary(StringComparer.OrdinalIgnoreCase); - - if (workbookPart.Workbook.DefinedNames == null) - return definedNames; - - foreach (var dn in workbookPart.Workbook.DefinedNames.Elements()) - { - string? name = dn.Name; - var reference = dn.Text; - if (name == null) continue; - - var parts = reference.Split('!'); - if (parts.Length != 2) continue; - var sName = parts[0].Trim('\''); - var cRef = parts[1].Replace("$", ""); - definedNames[name] = (sName, cRef); - } - - return definedNames; - } - /// /// Builds a lookup of sheet name → WorksheetPart, skipping chart sheets and other non-worksheet parts. /// diff --git a/BatchExcel/Services/OpenXmlHelpers.cs b/BatchExcel/Services/OpenXmlHelpers.cs index 32c5fce..37e2a5a 100644 --- a/BatchExcel/Services/OpenXmlHelpers.cs +++ b/BatchExcel/Services/OpenXmlHelpers.cs @@ -1,4 +1,7 @@ using System.Globalization; +using System.Collections.Generic; +using System.Linq; +using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Spreadsheet; namespace BatchExcel.Services; @@ -8,6 +11,35 @@ namespace BatchExcel.Services; /// internal static class OpenXmlHelpers { + /// + /// Builds a lookup of defined name → (sheet name, cell reference). + /// Parses references like "SheetName!$B$5" into their components. + /// Returns ONLY valid, writable cell references; offsets and formulas are ignored. + /// + public static Dictionary BuildDefinedNamesLookup(WorkbookPart workbookPart) + { + var definedNames = new Dictionary(StringComparer.OrdinalIgnoreCase); + + if (workbookPart.Workbook.DefinedNames == null) + return definedNames; + + foreach (var dn in workbookPart.Workbook.DefinedNames.Elements()) + { + string? name = dn.Name; + var reference = dn.Text; + if (name == null) continue; + + // Strict parsing: expects "Sheet!$A$1" + var parts = reference.Split('!'); + if (parts.Length != 2) continue; + var sName = parts[0].Trim('\''); + var cRef = parts[1].Replace("$", ""); + definedNames[name] = (sName, cRef); + } + + return definedNames; + } + /// /// Converts 1-based row and column indices to an Excel cell reference (e.g., row=1, col=2 → "B1"). /// diff --git a/BatchExcel/Services/TemplateValidator.cs b/BatchExcel/Services/TemplateValidator.cs index ac23117..907d03e 100644 --- a/BatchExcel/Services/TemplateValidator.cs +++ b/BatchExcel/Services/TemplateValidator.cs @@ -35,25 +35,14 @@ public static void Validate(string calculationPath, BatchConfig config) sheets.Add(s.Name.Value); } - // Extract defined names (named ranges) - var definedNames = new HashSet(StringComparer.OrdinalIgnoreCase); - if (workbookPart.Workbook.DefinedNames != null) - { - foreach (var dn in workbookPart.Workbook.DefinedNames.Elements()) - { - if (dn.Name?.Value != null) - definedNames.Add(dn.Name.Value); - } - } - - // Validate Input Fields + // Validate Input Fields (Sheets must exist) foreach (var field in config.InputFields) { if (!sheets.Contains(field.Sheet)) errors.Add($"Missing sheet '{field.Sheet}' (referenced by input field '{field.Range}')"); } - // Validate Output Fields + // Validate Output Fields (Sheets must exist) foreach (var field in config.OutputFields) { if (!sheets.Contains(field.Sheet)) @@ -61,12 +50,12 @@ public static void Validate(string calculationPath, BatchConfig config) } // Validate Header Inputs (Named Ranges) - foreach (var rangeName in config.HeaderInputs.Keys) - { - // "Date" is a special case written automatically, but it still needs a defined name - if (!definedNames.Contains(rangeName)) - errors.Add($"Missing named range '{rangeName}' (referenced by project header)"); - } + // Note: We use the centralized parser to ensure we only validate what the engine can actually write to. + // We do NOT throw if a header is missing from the template, as the writer ignores them silently. + var definedNames = OpenXmlHelpers.BuildDefinedNamesLookup(workbookPart); + + // Optional: We could log a warning if a header is missing, but for now we just + // ensure the validator doesn't block the run. if (errors.Count > 0) { From e43c0f85b931306840b0f1e903a8aa0e0e46735e Mon Sep 17 00:00:00 2001 From: Mitchell tesch Date: Mon, 8 Jun 2026 21:43:18 +1000 Subject: [PATCH 5/5] Replace `TemplateValidator` with a redesigned `CalculationValidator` for improved template validation. Includes updated validation logic, error aggregation, test coverage, and integration into `BatchEngine`. Updated README with validation details. --- BatchExcel.Tests/CalculationValidatorTests.cs | 217 ++++++++++++++++++ BatchExcel.Tests/TemplateValidatorTests.cs | 145 ------------ BatchExcel/Services/BatchEngine.cs | 7 +- BatchExcel/Services/CalculationValidator.cs | 139 +++++++++++ BatchExcel/Services/TemplateValidator.cs | 65 ------ BatchExcel/ViewModels/MainViewModel.cs | 10 + README.md | 21 +- 7 files changed, 383 insertions(+), 221 deletions(-) create mode 100644 BatchExcel.Tests/CalculationValidatorTests.cs delete mode 100644 BatchExcel.Tests/TemplateValidatorTests.cs create mode 100644 BatchExcel/Services/CalculationValidator.cs delete mode 100644 BatchExcel/Services/TemplateValidator.cs diff --git a/BatchExcel.Tests/CalculationValidatorTests.cs b/BatchExcel.Tests/CalculationValidatorTests.cs new file mode 100644 index 0000000..82c36d7 --- /dev/null +++ b/BatchExcel.Tests/CalculationValidatorTests.cs @@ -0,0 +1,217 @@ +using BatchExcel.Models; +using BatchExcel.Services; +using ClosedXML.Excel; + +namespace BatchExcel.Tests; + +public class CalculationValidatorTests : IDisposable +{ + private readonly string _tempPath; + + public CalculationValidatorTests() + { + _tempPath = Path.Combine(Path.GetTempPath(), $"test_template_{Guid.NewGuid()}.xlsx"); + } + + public void Dispose() + { + if (File.Exists(_tempPath)) + File.Delete(_tempPath); + GC.SuppressFinalize(this); + } + + private void CreateTestTemplate(string[] sheets, Dictionary? namedRanges = null) + { + using var workbook = new XLWorkbook(); + foreach (var sheetName in sheets) + workbook.Worksheets.Add(sheetName); + + if (namedRanges != null) + { + foreach (var nr in namedRanges) + workbook.DefinedNames.Add(nr.Key, nr.Value); + } + + workbook.SaveAs(_tempPath); + } + + [Fact] + public void Validate_ValidTemplate_DoesNotThrow() + { + CreateTestTemplate( + new[] { "Sheet1", "Main" }, + new Dictionary { ["JobNumber"] = "Sheet1!$A$1" }); + + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "A1", 0) }, + OutputFields = { new FieldDefinition("Main", "B2", 0) }, + HeaderInputs = { ["JobNumber"] = "123" } + }; + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_EmptyConfig_DoesNotThrow() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig(); + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_MissingSheet_ThrowsValidationException() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("MissingSheet", "A1", 0) } + }; + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("Missing sheet 'MissingSheet'", ex.Message); + Assert.Contains("input field", ex.Message); + } + + [Fact] + public void Validate_MissingSheetForOutputField_ThrowsValidationException() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + OutputFields = { new FieldDefinition("Nope", "B2", 0) } + }; + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("Missing sheet 'Nope'", ex.Message); + Assert.Contains("output field", ex.Message); + } + + [Fact] + public void Validate_HeaderInputs_AreNotEnforced() + { + // Header inputs are intentionally not validated — the writer ignores missing ones. + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + HeaderInputs = { ["MissingRange"] = "val" } + }; + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_A1Range_IsAccepted() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = + { + new FieldDefinition("Sheet1", "A1", 0), + new FieldDefinition("Sheet1", "$B$5", 1), + new FieldDefinition("Sheet1", "AA100:AB200", 2), + new FieldDefinition("Sheet1", "C10:D20", 3), + } + }; + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_NamedRangeAsFieldRange_IsAccepted() + { + CreateTestTemplate( + new[] { "Sheet1" }, + new Dictionary { ["MyInput"] = "Sheet1!$A$1" }); + + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "MyInput", 0) } + }; + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_UnknownNonA1Range_ThrowsValidationException() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "NotARangeOrName", 0) } + }; + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("'NotARangeOrName'", ex.Message); + Assert.Contains("not a valid A1 reference", ex.Message); + } + + [Fact] + public void Validate_EmptyRange_ThrowsValidationException() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "", 0) } + }; + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("Empty range", ex.Message); + } + + [Fact] + public void Validate_FileInUse_CanStillRead() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = { new FieldDefinition("Sheet1", "A1", 0) } + }; + + // Simulate file being open in Excel — exclusive write but shared read. + using var fs = new FileStream(_tempPath, FileMode.Open, FileAccess.Read, FileShare.Read); + + CalculationValidator.Validate(_tempPath, config); + } + + [Fact] + public void Validate_AggregatesMultipleErrors() + { + CreateTestTemplate(new[] { "Sheet1" }); + var config = new BatchConfig + { + InputFields = + { + new FieldDefinition("MissingSheet1", "A1", 0), + new FieldDefinition("Sheet1", "BogusRange", 1), + }, + OutputFields = { new FieldDefinition("MissingSheet2", "B2", 0) } + }; + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("Missing sheet 'MissingSheet1'", ex.Message); + Assert.Contains("Missing sheet 'MissingSheet2'", ex.Message); + Assert.Contains("'BogusRange'", ex.Message); + } + + [Fact] + public void Validate_CorruptFile_ThrowsValidationException() + { + // Write garbage bytes — not a valid .xlsx package. + File.WriteAllBytes(_tempPath, new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04 }); + var config = new BatchConfig(); + + var ex = Assert.Throws( + () => CalculationValidator.Validate(_tempPath, config)); + Assert.Contains("not a valid .xlsx file", ex.Message); + } +} + diff --git a/BatchExcel.Tests/TemplateValidatorTests.cs b/BatchExcel.Tests/TemplateValidatorTests.cs deleted file mode 100644 index 2c63ea2..0000000 --- a/BatchExcel.Tests/TemplateValidatorTests.cs +++ /dev/null @@ -1,145 +0,0 @@ -using BatchExcel.Models; -using BatchExcel.Services; -using ClosedXML.Excel; - -namespace BatchExcel.Tests; - -public class TemplateValidatorTests : IDisposable -{ - private readonly string _tempPath; - - public TemplateValidatorTests() - { - _tempPath = Path.Combine(Path.GetTempPath(), $"test_template_{Guid.NewGuid()}.xlsx"); - } - - public void Dispose() - { - if (File.Exists(_tempPath)) - File.Delete(_tempPath); - } - - private void CreateTestTemplate(string[] sheets, Dictionary? namedRanges = null) - { - using var workbook = new XLWorkbook(); - foreach (var sheetName in sheets) - { - workbook.Worksheets.Add(sheetName); - } - - if (namedRanges != null) - { - foreach (var nr in namedRanges) - { - // Simple named range to a cell in the first sheet - workbook.DefinedNames.Add(nr.Key, $"{sheets[0]}!$A$1"); - } - } - - workbook.SaveAs(_tempPath); - } - - [Fact] - public void Validate_ValidTemplate_DoesNotThrow() - { - // Arrange - CreateTestTemplate(new[] { "Sheet1", "Main" }, new Dictionary { ["JobNumber"] = "Sheet1!$A$1" }); - var config = new BatchConfig - { - InputFields = { new FieldDefinition("Sheet1", "A1", 0) }, - OutputFields = { new FieldDefinition("Main", "B2", 0) }, - HeaderInputs = { ["JobNumber"] = "123" } - }; - - // Act & Assert - TemplateValidator.Validate(_tempPath, config); - } - - [Fact] - public void Validate_MissingSheet_ThrowsValidationException() - { - // Arrange - CreateTestTemplate(new[] { "Sheet1" }); - var config = new BatchConfig - { - InputFields = { new FieldDefinition("MissingSheet", "A1", 0) } - }; - - // Act & Assert - var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); - Assert.Contains("Missing sheet 'MissingSheet'", ex.Message); - } - - [Fact] - public void Validate_MissingNamedRange_IsSilentlyIgnored() - { - // Arrange - CreateTestTemplate(new[] { "Sheet1" }); - var config = new BatchConfig - { - HeaderInputs = { ["MissingRange"] = "val" } - }; - - // Act & Assert - // Should not throw, matching CalculationHeaderWriter behavior - TemplateValidator.Validate(_tempPath, config); - } - - [Fact] - public void Validate_UnsupportedNamedRangeFormula_IsIgnored() - { - // Arrange - // Create a template with a named range that uses a formula (OFFSET) which our parser ignores - using (var workbook = new XLWorkbook()) - { - workbook.Worksheets.Add("Sheet1"); - workbook.DefinedNames.Add("ComplexRange", "OFFSET(Sheet1!$A$1,0,0,1,1)"); - workbook.SaveAs(_tempPath); - } - - var config = new BatchConfig - { - HeaderInputs = { ["ComplexRange"] = "val" } - }; - - // Act & Assert - // Should not throw, but should also not be considered "writeable" if we were checking writeability. - // For now, we just ensure it doesn't cause a false error or a crash. - TemplateValidator.Validate(_tempPath, config); - } - - [Fact] - public void Validate_FileInUse_CanStillRead() - { - // Arrange - CreateTestTemplate(new[] { "Sheet1" }); - var config = new BatchConfig - { - InputFields = { new FieldDefinition("Sheet1", "A1", 0) } - }; - - // Open the file to simulate it being open in Excel (allowing shared reading) - using var fs = new FileStream(_tempPath, FileMode.Open, FileAccess.Read, FileShare.Read); - - // Act & Assert - // This should NOT throw IOException if we use FileShare.ReadWrite in the validator - TemplateValidator.Validate(_tempPath, config); - } - - [Fact] - public void Validate_AggregatesMultipleErrors() - { - // Arrange - CreateTestTemplate(new[] { "Sheet1" }); - var config = new BatchConfig - { - InputFields = { new FieldDefinition("MissingSheet1", "A1", 0) }, - OutputFields = { new FieldDefinition("MissingSheet2", "B2", 0) } - }; - - // Act & Assert - var ex = Assert.Throws(() => TemplateValidator.Validate(_tempPath, config)); - Assert.Contains("Missing sheet 'MissingSheet1'", ex.Message); - Assert.Contains("Missing sheet 'MissingSheet2'", ex.Message); - } -} diff --git a/BatchExcel/Services/BatchEngine.cs b/BatchExcel/Services/BatchEngine.cs index 96144eb..b0d4429 100644 --- a/BatchExcel/Services/BatchEngine.cs +++ b/BatchExcel/Services/BatchEngine.cs @@ -65,8 +65,11 @@ public async Task RunAsync(string batcherFilePath, int workerCount, bool saveRun if (!File.Exists(calculationFullPath)) throw new FileNotFoundException($"Calculation spreadsheet not found: {calculationFullPath}"); - // Validate template exists and contains all required sheets/ranges (Dry Run) - TemplateValidator.Validate(calculationFullPath, config); + // Step 2: Dry-run validation — fail fast before spinning up Excel workers if any + // input/output field references a missing sheet or unresolvable range. + Log("\nValidating calculation spreadsheet... "); + CalculationValidator.Validate(calculationFullPath, config); + Log("done."); // Step 3: Create output folder var outFolder = Path.Combine(batcherDir, $"batch_run_{timestamp}"); diff --git a/BatchExcel/Services/CalculationValidator.cs b/BatchExcel/Services/CalculationValidator.cs new file mode 100644 index 0000000..20d0b6d --- /dev/null +++ b/BatchExcel/Services/CalculationValidator.cs @@ -0,0 +1,139 @@ +using System.IO; +using System.Text.RegularExpressions; +using BatchExcel.Models; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Spreadsheet; + +namespace BatchExcel.Services; + +/// +/// Thrown when the calculation workbook fails dry-run validation (missing sheets, +/// unresolved ranges/named ranges, corrupt file, etc.). Carries a user-friendly message +/// that the UI surfaces directly without a stack trace. +/// +public class ValidationException : Exception +{ + public ValidationException(string message) : base(message) { } + public ValidationException(string message, Exception innerException) : base(message, innerException) { } +} + +/// +/// Dry-run validator for the calculation workbook. Checks that every sheet and range +/// referenced by / +/// can be resolved before the engine spins up Excel workers. +/// Header inputs (named ranges) are intentionally NOT enforced — the writer ignores +/// missing ones, matching behavior. +/// +public static class CalculationValidator +{ + // Matches A1-style references and ranges: A1, $A$1, A1:B10, $A$1:$B$10, AA100:AB200, etc. + // Case-insensitive; allows optional absolute-reference dollar signs on either side. + private static readonly Regex A1RangePattern = new( + @"^\$?[A-Za-z]{1,3}\$?[0-9]+(:\$?[A-Za-z]{1,3}\$?[0-9]+)?$", + RegexOptions.Compiled); + + /// + /// Validates the calculation template. Opens the file with shared read access so + /// it works even while the user has the workbook open in Excel. + /// Throws with an aggregated error list on failure. + /// + public static void Validate(string calculationPath, BatchConfig config) + { + FileStream fs; + try + { + // FileShare.ReadWrite so we don't conflict with Excel if the user has it open. + fs = new FileStream(calculationPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); + } + catch (IOException ex) + { + throw new ValidationException( + $"Could not open calculation template '{calculationPath}': {ex.Message}", ex); + } + + try + { + SpreadsheetDocument document; + try + { + document = SpreadsheetDocument.Open(fs, isEditable: false); + } + catch (Exception ex) when (ex is not ValidationException) + { + throw new ValidationException( + $"Calculation template '{calculationPath}' is not a valid .xlsx file: {ex.Message}", ex); + } + + using (document) + { + var workbookPart = document.WorkbookPart + ?? throw new ValidationException( + $"Calculation template '{calculationPath}' has no workbook part."); + + var sheets = BuildSheetNameSet(workbookPart); + var definedNames = OpenXmlHelpers.BuildDefinedNamesLookup(workbookPart); + + var errors = new List(); + ValidateFields(config.InputFields, "input", sheets, definedNames, errors); + ValidateFields(config.OutputFields, "output", sheets, definedNames, errors); + + if (errors.Count > 0) + { + throw new ValidationException( + "Calculation template validation failed:\n - " + string.Join("\n - ", errors)); + } + } + } + finally + { + fs.Dispose(); + } + } + + private static HashSet BuildSheetNameSet(WorkbookPart workbookPart) + { + var sheets = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var s in workbookPart.Workbook.Descendants()) + { + if (s.Name?.Value is { } name) + sheets.Add(name); + } + return sheets; + } + + private static void ValidateFields( + List fields, + string fieldKind, + HashSet sheets, + Dictionary definedNames, + List errors) + { + foreach (var field in fields) + { + // Sheet must exist + if (!sheets.Contains(field.Sheet)) + { + errors.Add($"Missing sheet '{field.Sheet}' (referenced by {fieldKind} field range '{field.Range}')"); + continue; // skip range check — without the sheet, range info is moot + } + + // Range must be either an A1-style reference or a known defined name. + if (string.IsNullOrWhiteSpace(field.Range)) + { + errors.Add($"Empty range on sheet '{field.Sheet}' for {fieldKind} field"); + continue; + } + + if (A1RangePattern.IsMatch(field.Range)) + continue; + + if (definedNames.ContainsKey(field.Range)) + continue; + + errors.Add( + $"Range '{field.Range}' on sheet '{field.Sheet}' for {fieldKind} field is not a valid A1 " + + "reference and is not a defined name in the calculation template"); + } + } +} + diff --git a/BatchExcel/Services/TemplateValidator.cs b/BatchExcel/Services/TemplateValidator.cs deleted file mode 100644 index 907d03e..0000000 --- a/BatchExcel/Services/TemplateValidator.cs +++ /dev/null @@ -1,65 +0,0 @@ -using System.IO; -using BatchExcel.Models; -using DocumentFormat.OpenXml.Packaging; -using DocumentFormat.OpenXml.Spreadsheet; - -namespace BatchExcel.Services; - -public class ValidationException : Exception -{ - public ValidationException(string message) : base(message) { } -} - -public static class TemplateValidator -{ - /// - /// Validates that all sheets and named ranges referenced in the configuration exist - /// in the calculation template. Opens the file with non-locking shared access to - /// avoid exceptions if the file is open in Excel. - /// - public static void Validate(string calculationPath, BatchConfig config) - { - var errors = new List(); - - // Open with FileShare.ReadWrite to avoid locking issues if open in Excel - using var fs = new FileStream(calculationPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - using var document = SpreadsheetDocument.Open(fs, isEditable: false); - - var workbookPart = document.WorkbookPart ?? throw new InvalidOperationException("Workbook part not found."); - - // Extract sheet names - var sheets = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var s in workbookPart.Workbook.Descendants()) - { - if (s.Name?.Value != null) - sheets.Add(s.Name.Value); - } - - // Validate Input Fields (Sheets must exist) - foreach (var field in config.InputFields) - { - if (!sheets.Contains(field.Sheet)) - errors.Add($"Missing sheet '{field.Sheet}' (referenced by input field '{field.Range}')"); - } - - // Validate Output Fields (Sheets must exist) - foreach (var field in config.OutputFields) - { - if (!sheets.Contains(field.Sheet)) - errors.Add($"Missing sheet '{field.Sheet}' (referenced by output field '{field.Range}')"); - } - - // Validate Header Inputs (Named Ranges) - // Note: We use the centralized parser to ensure we only validate what the engine can actually write to. - // We do NOT throw if a header is missing from the template, as the writer ignores them silently. - var definedNames = OpenXmlHelpers.BuildDefinedNamesLookup(workbookPart); - - // Optional: We could log a warning if a header is missing, but for now we just - // ensure the validator doesn't block the run. - - if (errors.Count > 0) - { - throw new ValidationException("Template validation failed:\n- " + string.Join("\n- ", errors)); - } - } -} diff --git a/BatchExcel/ViewModels/MainViewModel.cs b/BatchExcel/ViewModels/MainViewModel.cs index f7a3f2a..a05748f 100644 --- a/BatchExcel/ViewModels/MainViewModel.cs +++ b/BatchExcel/ViewModels/MainViewModel.cs @@ -243,6 +243,16 @@ private async Task RunBatch() ProgressText = "Complete"; ProgressPercent = 100; } + catch (ValidationException vex) + { + // Configuration mistake (missing sheet, bad range, corrupt template, etc.). + // Show a clean user-facing message — no stack trace clutter. + AppendLog($"\n*** Validation failed ***\n{vex.Message}"); + ProgressText = "Validation failed"; + FlushLog(); + MessageBox.Show(vex.Message, "Calculation Template Validation Failed", + MessageBoxButton.OK, MessageBoxImage.Warning); + } catch (Exception ex) { AppendLog($"\n*** ERROR: {ex.Message} ***"); diff --git a/README.md b/README.md index c0d745c..3411b53 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,7 @@ BatchExcel/ │ ├── ExcelProcessTracker.cs # PID tracking + safe quit + zombie cleanup │ ├── BatcherReader.cs # Read config (ClosedXML), write results (OpenXML) │ ├── CalculationHeaderWriter.cs # Write date/job/etc. headers via OpenXML defined names +│ ├── CalculationValidator.cs # Dry-run validation of calculation template (sheets + ranges) │ ├── OpenXmlHelpers.cs # SheetWriter (indexed bulk writes) + cell helpers │ ├── CsvResultWriter.cs # CSV output with injection protection │ ├── PdfExporter.cs # Multi-sheet PDF export via Excel COM @@ -156,15 +157,16 @@ BatchExcel/ ``` 1. Read config BatcherReader.ReadConfig ClosedXML, direct file (~50 ms) -2. Copy Calculation one .xlsx per worker File.Copy × N -3. Write headers CalculationHeaderWriter.Write OpenXML on each copy (~20 ms) -4. Launch workers RunOnStaThread STA + IOleMessageFilter, staggered -5. Process runs ExcelWorker.ProcessRunQueue Cached COM refs → Calculate() → read +2. Validate template CalculationValidator.Validate OpenXML, fail-fast dry run (~10 ms) +3. Copy Calculation one .xlsx per worker File.Copy × N +4. Write headers CalculationHeaderWriter.Write OpenXML on each copy (~20 ms) +5. Launch workers RunOnStaThread STA + IOleMessageFilter, staggered +6. Process runs ExcelWorker.ProcessRunQueue Cached COM refs → Calculate() → read + per-run Save & PDF (optional) -6. Release & quit ExcelWorker finally FinalReleaseComObject + SafeQuitExcel -7. Write results BatcherReader.WriteResults Single OpenXML pass → File.Copy ×2 -8. Write CSV CsvResultWriter.Write StreamWriter, injection-safe escaping -9. Cleanup Delete worker copies KillAllTracked on exit / on errors +7. Release & quit ExcelWorker finally FinalReleaseComObject + SafeQuitExcel +8. Write results BatcherReader.WriteResults Single OpenXML pass → File.Copy ×2 +9. Write CSV CsvResultWriter.Write StreamWriter, injection-safe escaping +10. Cleanup Delete worker copies KillAllTracked on exit / on errors ``` ### Performance design @@ -187,12 +189,13 @@ BatchExcel/ ## Testing -The `BatchExcel.Tests` project (xUnit) currently includes **69 tests** covering: +The `BatchExcel.Tests` project (xUnit) currently includes **81 tests** covering: - `BatcherReader` — round-trips against generated `.xlsx` fixtures (no Excel required) - `OpenXmlHelpers` / `SheetWriter` — cell reference math, indexed bulk writes, typed-value coverage (`double` / `int` / `long` / `float` / `decimal` / `bool` / `DateTime`) - `CsvResultWriter` — escaping, status distinction, formula-injection neutralisation, invariant-culture numbers - `BatchConfig` — macro parsing, included-run count +- `CalculationValidator` — dry-run validation: sheet existence, A1 and named-range resolution, error aggregation, corrupt-file handling - `FileNameSanitizer` — invalid character handling - `UserSettings` — load / save round-trip