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/Services/BatchEngine.cs b/BatchExcel/Services/BatchEngine.cs index d2224b6..bc3fe6a 100644 --- a/BatchExcel/Services/BatchEngine.cs +++ b/BatchExcel/Services/BatchEngine.cs @@ -65,6 +65,12 @@ public async Task RunAsync(string batcherFilePath, int workerCount, bool saveRun if (!File.Exists(calculationFullPath)) throw new FileNotFoundException($"Calculation spreadsheet not found: {calculationFullPath}"); + // 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}"); Directory.CreateDirectory(outFolder); diff --git a/BatchExcel/Services/CalculationHeaderWriter.cs b/BatchExcel/Services/CalculationHeaderWriter.cs index 81493c0..bae12c6 100644 --- a/BatchExcel/Services/CalculationHeaderWriter.cs +++ b/BatchExcel/Services/CalculationHeaderWriter.cs @@ -29,7 +29,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. @@ -69,33 +69,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/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/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/ViewModels/MainViewModel.cs b/BatchExcel/ViewModels/MainViewModel.cs index b52bdb0..1a76002 100644 --- a/BatchExcel/ViewModels/MainViewModel.cs +++ b/BatchExcel/ViewModels/MainViewModel.cs @@ -278,6 +278,16 @@ private async Task RunBatch() if (killedPath > 0) AppendLog($"\nCleaned up {killedPath} zombie Excel process(es)."); } + 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 cd0f5b8..8c75d40 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 **72 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 - `ExcelProcessTracker` — PID tracking, kill-only-running semantics, `SafeQuitExcel` kill-fallback (via mocked `IProcessInterop`)