diff --git a/src/MiniExcel.OpenXml/MiniExcel.OpenXml.csproj b/src/MiniExcel.OpenXml/MiniExcel.OpenXml.csproj index f0b6a087..5a10e735 100644 --- a/src/MiniExcel.OpenXml/MiniExcel.OpenXml.csproj +++ b/src/MiniExcel.OpenXml/MiniExcel.OpenXml.csproj @@ -18,8 +18,4 @@ - - - - diff --git a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs index 09009ffe..e3933f21 100644 --- a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs +++ b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs @@ -937,7 +937,16 @@ private void ProcessFormulas(StringBuilder rowXml, int rowIndex) str.AddBeforeSelf(fNode); str.Remove(); - var celRef = CellReferenceConverter.GetCellFromCoordinates(index, rowIndex); + // the cell no longer holds an inline string; keeping t="inlineStr" without the tag is invalid + cell.Attribute("t")?.Remove(); + + // take the column from the cell's own reference — the running index is wrong for + // sparse rows (cells without content are not emitted, so position != column) + var rAttr = cell.Attribute("r")?.Value; + var celRef = string.IsNullOrEmpty(rAttr) + ? CellReferenceConverter.GetCellFromCoordinates(index, rowIndex) + : rAttr; + _calcChainCellRefs.Add(celRef); } } @@ -964,11 +973,11 @@ private void ProcessFormulas(StringBuilder rowXml, int rowIndex) private static StringBuilder CleanXml(StringBuilder xml, string? prefix = null) { var sb = xml - .Replace("xmlns:x14ac=\"http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac\"", "") - .Replace("xmlns=\"http://schemas.openxmlformats.org/spreadsheetml/2006/main\"", ""); + .Replace($"xmlns:x14ac={Schemas.SpreadsheetmlXmlX14Ac}", "") + .Replace($"xmlns={Schemas.SpreadsheetmlXmlMain}", ""); return !string.IsNullOrEmpty(prefix) - ? sb.Replace($"xmlns:{prefix}=\"http://schemas.openxmlformats.org/spreadsheetml/2006/main\"", "") + ? sb.Replace($"xmlns:{prefix}={Schemas.SpreadsheetmlXmlMain}", "") : sb; } @@ -982,7 +991,7 @@ private static void InjectSharedStrings(IDictionary sharedStrings, var t = cell.Attribute("t"); var v = cell.Element(SpreadsheetNs + "v"); - if (v?.Value is null || t?.Value != "s") + if (v?.Value is null || t?.Value != ExcelDataTypes.SharedString) continue; //needs to check if sharedstring exists or not @@ -995,7 +1004,7 @@ private static void InjectSharedStrings(IDictionary sharedStrings, var tNode = new XElement(SpreadsheetNs + "t", shared); var isNode = new XElement(SpreadsheetNs + "is", tNode); cell.Add(isNode); - cell.SetAttributeValue("t", "inlineStr"); + cell.SetAttributeValue("t", ExcelDataTypes.InlineString); } } } @@ -1003,13 +1012,13 @@ private static void InjectSharedStrings(IDictionary sharedStrings, private static void SetCellType(XElement cell, string type) { // Force inlineStr for strings - if (type == "str") - type = "inlineStr"; + if (type == ExcelDataTypes.CalculatedString) + type = ExcelDataTypes.InlineString; - if (type == "inlineStr") + if (type == ExcelDataTypes.InlineString) { // Ensure ... - cell.SetAttributeValue("t", "inlineStr"); + cell.SetAttributeValue("t", ExcelDataTypes.InlineString); if (cell.Element(SpreadsheetNs + "v") is { } v) { @@ -1020,7 +1029,7 @@ private static void SetCellType(XElement cell, string type) var isNode = new XElement(SpreadsheetNs + "is", tNode); cell.Add(isNode); - cell.SetAttributeValue("t", "inlineStr"); + cell.SetAttributeValue("t", ExcelDataTypes.InlineString); } else if (cell.Element(SpreadsheetNs + "is") is null) { @@ -1036,8 +1045,8 @@ private static void SetCellType(XElement cell, string type) // Ensure ... // For numbers/booleans, we remove 't' attribute to let it be default (number) // or we could set it to 'n' explicitly, but removing is safer for general number types - if (type == "b") - cell.SetAttributeValue("t", "b"); + if (type == ExcelDataTypes.Boolean) + cell.SetAttributeValue("t", ExcelDataTypes.Boolean); else cell.Attribute("t")?.Remove(); diff --git a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.ValueExtractorHook.cs b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.ValueExtractorHook.cs index 175b935b..8e3fbac8 100644 --- a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.ValueExtractorHook.cs +++ b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.ValueExtractorHook.cs @@ -169,7 +169,7 @@ static void TraverseAndFlatten( /// Adds worksheets to the workbook and register them int workbook.xml and workbook.xml.rels /// [CreateSyncVersion] - private static async Task BatchAddSheetsToWorkbookAsync(ZipArchive outputZip, ZipArchive templateArchive, List<(int Index, string Name)> sheetInfos, CancellationToken cancellationToken) + private static async Task BatchAddSheetsToWorkbookAsync(ZipArchive outputZip, ZipArchive templateArchive, List<(int Index, string Name)> sheetInfos, bool removeCalcChainFromRels, CancellationToken cancellationToken) { // Load the workbook and its relationships from the template var relDoc = await LoadXmlAsync(templateArchive, ExcelFileNames.WorkbookRels, cancellationToken).ConfigureAwait(false); @@ -188,25 +188,34 @@ private static async Task BatchAddSheetsToWorkbookAsync(ZipArchive outputZip, Zi } // 2. Clean up all relationship records pointing to worksheets in workbook.xml.rels - var relsRoot = relDoc.Root; - if (relsRoot != null) + if (relDoc.Root is { } relsRoot) { - // Only delete relationships of Type 'worksheet', preserving core relationships like sharedStrings/styles/theme + // Remove the calcChain relationship if the contents have been invalidated upstream + if (removeCalcChainFromRels) + { + var calcChainRecord = relsRoot.Elements().FirstOrDefault(x => + x.Attribute("Target")?.Value + .EndsWith("calcChain.xml", StringComparison.OrdinalIgnoreCase) is true + ); + calcChainRecord?.Remove(); + } + + // Delete relationships of Type 'worksheet', preserving core relationships like sharedStrings/styles/theme var worksheetRels = relsRoot.Elements(PackageRelNs + "Relationship") .Where(r => r.Attribute("Type")?.Value == Schemas.SpreadsheetmlXmlWorksheetRelationship); // Remove the filtered worksheet relationships foreach (var rel in worksheetRels) rel.Remove(); - } - // Batch add new relationship records for each generated sheet - foreach (var sheet in sheetInfos) - { - relDoc.Root!.Add(new XElement(PackageRelNs + "Relationship", - new XAttribute("Id", $"rIdSheet{sheet.Index}"), - new XAttribute("Type", Schemas.SpreadsheetmlXmlWorksheetRelationship), - new XAttribute("Target", $"worksheets/sheet{sheet.Index}.xml"))); + // Batch add new relationship records for each generated sheet + foreach (var sheet in sheetInfos) + { + relsRoot.Add(new XElement(PackageRelNs + "Relationship", + new XAttribute("Id", $"rIdSheet{sheet.Index}"), + new XAttribute("Type", Schemas.SpreadsheetmlXmlWorksheetRelationship), + new XAttribute("Target", $"worksheets/sheet{sheet.Index}.xml"))); + } } // Batch add new sheet definitions to the workbook @@ -245,10 +254,11 @@ private async Task> GetSheetNameMapAsync(ZipArchive a if (rel.Attribute("Id")?.Value is { } rid) { var target = rel.Attribute("Target")?.Value; - if (string.IsNullOrEmpty(rid) || string.IsNullOrEmpty(target)) continue; + if (string.IsNullOrEmpty(rid) || string.IsNullOrEmpty(target)) + continue; // Construct the full internal path (ensure forward slashes for consistency) - var fullSheetPath = Path.Combine("xl", target).Replace("\\", "/"); + var fullSheetPath = Path.Combine("xl", target).Replace("\\", "/").TrimStart('/'); ridToSheetPath[rid] = fullSheetPath; } } diff --git a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs index 60888c6d..be31791c 100644 --- a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs +++ b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.cs @@ -87,6 +87,7 @@ public async Task SaveAsByTemplateAsync(Stream templateStream, object value, Can var entryName = entry.FullName.TrimStart('/'); if (entryName.StartsWith(ExcelFileNames.WorksheetBase, StringComparison.OrdinalIgnoreCase) || entryName.Equals(ExcelFileNames.CalcChain, StringComparison.OrdinalIgnoreCase) || + entryName.Equals(ExcelFileNames.ContentTypes, StringComparison.OrdinalIgnoreCase) || entryName.Equals(ExcelFileNames.Workbook, StringComparison.OrdinalIgnoreCase) || entryName.Equals(ExcelFileNames.WorkbookRels, StringComparison.OrdinalIgnoreCase)) { @@ -124,7 +125,7 @@ await originalEntryStream.CopyToAsync(newEntryStream foreach (var templateSheet in templateSheets) { - // XRowInfos musy be cleared for every sheet or it'll cause duplicates: https://user-images.githubusercontent.com/12729184/115003101-0fcab700-9ed8-11eb-9151-ca4d7b86d59e.png + // XRowInfos must be cleared for every sheet or it'll cause duplicates: https://user-images.githubusercontent.com/12729184/115003101-0fcab700-9ed8-11eb-9151-ca4d7b86d59e.png _xRowInfos.Clear(); _xMergeCellInfos.Clear(); _newXMergeCellInfos.Clear(); @@ -155,16 +156,20 @@ await originalEntryStream.CopyToAsync(newEntryStream } } - // batch add sheet - await BatchAddSheetsToWorkbookAsync(outputFileArchive.ZipFile, originalArchive, allSheetInfos, cancellationToken).ConfigureAwait(false); - - // create mode we need to not create first then create here - var calcChain = outputFileArchive.EntryCollection.FirstOrDefault(e + // The template's own calcChain cannot be reused: row insertion shifts formula cells and its + // entries would point at the old addresses. It is regenerated from the rendered formulas — + // and when none were rendered, dropped entirely, because a calcChain with no entries is + // schema-invalid and Excel rejects the whole package either way. + // Excel rebuilds the chain on open, so dropping it is always safe. + var calcChain = outputFileArchive.EntryCollection.FirstOrDefault(e => e.FullName.TrimStart('/').Equals(ExcelFileNames.CalcChain, StringComparison.OrdinalIgnoreCase)); - if (calcChain is not null) + var contentTypesDoc = await LoadXmlAsync(originalArchive, ExcelFileNames.ContentTypes, cancellationToken).ConfigureAwait(false); + var isValidCalcChain = calcChain is not null && _calcChainContent.Length > 0; + + if (isValidCalcChain) { - var calcChainEntry = outputFileArchive.ZipFile.CreateEntry(calcChain.FullName); + var calcChainEntry = outputFileArchive.ZipFile.CreateEntry(calcChain!.FullName); var calcChainStream = await calcChainEntry.OpenAsync(cancellationToken).ConfigureAwait(false); await using var disposableChainEntryStream = calcChainStream.ConfigureAwait(false); @@ -172,27 +177,19 @@ await originalEntryStream.CopyToAsync(newEntryStream } else { - foreach (var entry in originalArchive.Entries) - { - if (entry.FullName.TrimStart('/').Equals(ExcelFileNames.CalcChain, StringComparison.OrdinalIgnoreCase)) - { - var newEntry = outputFileArchive.ZipFile.CreateEntry(entry.FullName); - - // Copy the content of the original entry to the new entry - var originalEntryStream = await entry.OpenAsync(cancellationToken).ConfigureAwait(false); - await using var disposableEntryStream = originalEntryStream.ConfigureAwait(false); - - var newEntryStream = await newEntry.OpenAsync(cancellationToken).ConfigureAwait(false); - await using var disposableNewEntryStream = newEntryStream.ConfigureAwait(false); - - await originalEntryStream.CopyToAsync(newEntryStream -#if NET - , cancellationToken -#endif - ).ConfigureAwait(false); - } - } + var elements = contentTypesDoc.Root?.Elements(); + var calcChainRecord = elements?.FirstOrDefault(x => + x.Attribute("PartName")?.Value.TrimStart('/') + .Equals(ExcelFileNames.CalcChain, StringComparison.OrdinalIgnoreCase) is true + ); + calcChainRecord?.Remove(); } + + // saving the (possibly edited) [Content_Types].xml entry + await SaveXmlToZipAsync(outputFileArchive.ZipFile, ExcelFileNames.ContentTypes, contentTypesDoc, cancellationToken).ConfigureAwait(false); + + // editing the workbook and its rels to reflect the new worksheets' metadata + await BatchAddSheetsToWorkbookAsync(outputFileArchive.ZipFile, originalArchive, allSheetInfos, !isValidCalcChain, cancellationToken).ConfigureAwait(false); #if NET10_0_OR_GREATER await outputFileArchive.ZipFile.DisposeAsync().ConfigureAwait(false); diff --git a/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainAsyncTests.cs b/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainAsyncTests.cs new file mode 100644 index 00000000..823845e1 --- /dev/null +++ b/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainAsyncTests.cs @@ -0,0 +1,107 @@ +using System.Xml.Linq; +using ClosedXML.Excel; +using MiniExcelLib.OpenXml.Constants; +using MiniExcelLib.Tests.Common.Utils; + +namespace MiniExcelLib.OpenXml.Tests.Templates; + +public class CalcChainAsyncTests +{ + private readonly OpenXmlTemplater _templater = MiniExcel.Templaters.GetOpenXmlTemplater(); + + [Fact] + public async Task TemplateWithStaticFormula_DoesNotWriteStaleOrEmptyCalcChain() + { + // A template with a static Excel formula (below an IEnumerable row) carries a calcChain + // pointing at the formula's pre-render address. After rows are inserted the address is + // stale — the rendered package must not contain a stale or empty calcChain. + using var template = AutoDeletingPath.Create(); + using (var wb = new XLWorkbook()) + { + var ws = wb.AddWorksheet("Sheet1"); + ws.Cell("A1").Value = "{{title}}"; + ws.Cell("A3").Value = "{{items.Name}}"; + ws.Cell("B3").Value = "{{items.Qty}}"; + ws.Cell("B5").FormulaA1 = "SUM(B3:B4)"; + wb.SaveAs(template.FilePath); + } + + using var path = AutoDeletingPath.Create(); + Dictionary data = new() + { + ["title"] = "FooCompany", + ["items"] = new[] + { + new { Name = "A", Qty = 1 }, + new { Name = "B", Qty = 2 }, + } + }; + await _templater.FillTemplateAsync(path.ToString(), template.FilePath, data); + + using var zip = ZipFile.OpenRead(path.ToString()); + var calcChain = zip.GetEntry("xl/calcChain.xml"); + if (calcChain != null) + { + using var reader = new StreamReader(calcChain.Open()); + var content = await reader.ReadToEndAsync(); + Assert.Contains(" data = new() + { + ["title"] = "FooCompany", + ["items"] = new[] + { + new { Name = "A", Qty = 1 }, + new { Name = "B", Qty = 2 }, + } + }; + await _templater.FillTemplateAsync(path.ToString(), template.FilePath, data); + + using var zip = ZipFile.OpenRead(path.ToString()); + + // the formula cell: (two items shift row 7 to 8) with a namespaced child and no inlineStr type + XDocument doc; + await using (var sheet = zip.GetEntry("xl/worksheets/sheet1.xml")!.Open()) + { + doc = await XDocument.LoadAsync(sheet, System.Xml.Linq.LoadOptions.None, CancellationToken.None); + } + + var ns = (XNamespace)Schemas.SpreadsheetmlXmlMain; + var formulaCell = doc.Descendants(ns + "c").FirstOrDefault(x => x.Attribute("r")?.Value == "D8"); + Assert.NotNull(formulaCell); + Assert.Equal("SUM(B5:B6)", formulaCell.Element(ns + "f")?.Value); + Assert.NotEqual("inlineStr", formulaCell.Attribute("t")?.Value); + + // the regenerated calcChain points at the formula's real address, not the one derived + // from the cell's position in the row (which would be column B here) + using var chainReader = new StreamReader(zip.GetEntry("xl/calcChain.xml")!.Open()); + var chain = await chainReader.ReadToEndAsync(); + Assert.Contains(@"r=""D8""", chain); + Assert.DoesNotContain(@"r=""B8""", chain); + } +} diff --git a/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainTests.cs b/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainTests.cs new file mode 100644 index 00000000..5264392a --- /dev/null +++ b/tests/MiniExcel.OpenXml.Tests/Templates/CalcChainTests.cs @@ -0,0 +1,114 @@ +using System.Xml.Linq; +using ClosedXML.Excel; +using MiniExcelLib.OpenXml.Constants; +using MiniExcelLib.Tests.Common.Utils; + +namespace MiniExcelLib.OpenXml.Tests.Templates; + +/// +/// Regression tests for calcChain.xml handling and '$='-formula cell serialization in template +/// rendering. A template containing any formula carries a calcChain part whose entries point at +/// cell addresses; row insertion shifts formula cells, so the chain must be regenerated from the +/// rendered output — never left stale, and never written empty (a calcChain with zero <c> +/// entries is schema-invalid and Excel refuses to open the whole file). +/// +public class CalcChainTests +{ + private readonly OpenXmlTemplater _templater = MiniExcel.Templaters.GetOpenXmlTemplater(); + + [Fact] + public void TemplateWithStaticFormula_DoesNotWriteStaleOrEmptyCalcChain() + { + // A template with a static Excel formula (below an IEnumerable row) carries a calcChain + // pointing at the formula's pre-render address. After rows are inserted the address is + // stale — the rendered package must not contain a stale or empty calcChain. + using var template = AutoDeletingPath.Create(); + using (var wb = new XLWorkbook()) + { + var ws = wb.AddWorksheet("Sheet1"); + ws.Cell("A1").Value = "{{title}}"; + ws.Cell("A3").Value = "{{items.Name}}"; + ws.Cell("B3").Value = "{{items.Qty}}"; + ws.Cell("B5").FormulaA1 = "SUM(B3:B4)"; + wb.SaveAs(template.FilePath); + } + + using var path = AutoDeletingPath.Create(); + Dictionary data = new() + { + ["title"] = "FooCompany", + ["items"] = new[] + { + new { Name = "A", Qty = 1 }, + new { Name = "B", Qty = 2 }, + } + }; + _templater.FillTemplate(path.ToString(), template.FilePath, data); + + using var zip = ZipFile.OpenRead(path.ToString()); + var calcChain = zip.GetEntry("xl/calcChain.xml"); + if (calcChain != null) + { + using var reader = new StreamReader(calcChain.Open()); + var content = reader.ReadToEnd(); + Assert.Contains(" data = new() + { + ["title"] = "FooCompany", + ["items"] = new[] + { + new { Name = "A", Qty = 1 }, + new { Name = "B", Qty = 2 }, + } + }; + _templater.FillTemplate(path.ToString(), template.FilePath, data); + + using var zip = ZipFile.OpenRead(path.ToString()); + + // the formula cell: (two items shift row 7 to 8) with a namespaced child and no inlineStr type + XDocument doc; + using (var sheet = zip.GetEntry("xl/worksheets/sheet1.xml")!.Open()) + { + doc = XDocument.Load(sheet); + } + + var ns = (XNamespace)Schemas.SpreadsheetmlXmlMain; + var formulaCell = doc.Descendants(ns + "c").FirstOrDefault(x => x.Attribute("r")?.Value == "D8"); + Assert.NotNull(formulaCell); + Assert.Equal("SUM(B5:B6)", formulaCell.Element(ns + "f")?.Value); + Assert.NotEqual("inlineStr", formulaCell.Attribute("t")?.Value); + + // the regenerated calcChain points at the formula's real address, not the one derived + // from the cell's position in the row (which would be column B here) + using var chainReader = new StreamReader(zip.GetEntry("xl/calcChain.xml")!.Open()); + var chain = chainReader.ReadToEnd(); + Assert.Contains(@"r=""D8""", chain); + Assert.DoesNotContain(@"r=""B8""", chain); + } +}