From 0851c7a896cd12a9b34c86ad93ec927d6fb89a74 Mon Sep 17 00:00:00 2001 From: Guille Dols Date: Tue, 22 Sep 2026 12:24:20 +0200 Subject: [PATCH] Fill in missing row and cell references in templates (#863) The "r" attribute on rows and cells is optional (ECMA-376 18.3.1.73 and 18.3.1.4): without it, a row follows the previous row and a cell follows the previous cell, which is how OpenXmlReader already reads them. The template code keys everything on "r", so a template written that way crashed in UpdateDimensionAndGetRowsInfo with an ArgumentNullException. FillMissingReferences fills in the missing ones when the template sheet is loaded, in both the create and update modes, so FillTemplate and MergeSameCells both work. References that are already there are left as they are, so a malformed one still gets the existing NotSupportedException. --- .../Templates/OpenXmlTemplate.Impl.cs | 36 ++++++++++++++- .../Issues/MiniExcelGithubIssuesAsyncTests.cs | 26 +++++++++++ .../Issues/MiniExcelGithubIssuesTests.cs | 45 +++++++++++++++++++ .../Utils/SheetHelper.cs | 21 +++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs index e3933f21..46f565e7 100644 --- a/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs +++ b/src/MiniExcel.OpenXml/Templates/OpenXmlTemplate.Impl.cs @@ -49,6 +49,7 @@ private async Task GenerateSheetByUpdateModeAsync(ZipArchiveEntry sheetZipEntry, var sheetData = worksheet?.Element(SpreadsheetNs + "sheetData"); var newSheetData = new XElement(sheetData); var rows = newSheetData.Elements(SpreadsheetNs + "row"); + FillMissingReferences(rows); InjectSharedStrings(sharedStrings, rows); GetMergeCells(worksheet); @@ -83,6 +84,7 @@ private async Task GenerateSheetByCreateModeAsync(ZipArchiveEntry templateSheetZ var newSheetData = new XElement(sheetData); var rows = newSheetData.Elements(SpreadsheetNs + "row"); + FillMissingReferences(rows); InjectSharedStrings(sharedStrings, rows); GetMergeCells(worksheet); @@ -97,6 +99,39 @@ private async Task GenerateSheetByCreateModeAsync(ZipArchiveEntry templateSheetZ await WriteSheetXmlAsync(writer, worksheet, sheetData, mergeCells, cancellationToken).ConfigureAwait(false); } + // "r" is optional on rows and cells (ECMA-376 18.3.1.73, 18.3.1.4); without it they follow the previous one, + // which is also how OpenXmlReader reads them. The rest of the template code keys on "r", so fill it in first. + private static void FillMissingReferences(IEnumerable rows) + { + var rowIndex = 0; + foreach (var row in rows) + { + if (row.Attribute("r") is not { } rowReference) + { + rowIndex++; + row.SetAttributeValue("r", rowIndex.ToString()); + } + else if (int.TryParse(rowReference.Value, out var explicitRowIndex)) + { + rowIndex = explicitRowIndex; + } + + var columnIndex = 0; + foreach (var cell in row.Elements(SpreadsheetNs + "c")) + { + if (cell.Attribute("r") is not { } cellReference) + { + columnIndex++; + cell.SetAttributeValue("r", CellReferenceConverter.GetCellFromCoordinates(columnIndex, rowIndex)); + } + else if (CellReferenceConverter.TryParseCellReference(cellReference.Value, out var explicitColumnIndex, out _)) + { + columnIndex = explicitColumnIndex; + } + } + } + } + private void GetMergeCells(XElement worksheet) { if (worksheet.Element(SpreadsheetNs + "mergeCells") is not { } mergeCells) @@ -334,7 +369,6 @@ var s when s.StartsWith("@header") => SpecialCellType.Header, } } - //TODO: Fix parsing for documents that don't have the "r" attribute on rows if (row.Attribute("r")?.Value is not { } rVal || !int.TryParse(rVal, out var originRowIndex)) throw new NotSupportedException("The format of the chosen template is not currently supported."); diff --git a/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesAsyncTests.cs b/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesAsyncTests.cs index 75d26f9e..bf85ff61 100644 --- a/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesAsyncTests.cs +++ b/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesAsyncTests.cs @@ -1313,4 +1313,30 @@ public async Task TestIssue980() // deterministic file generation var bytes2 = await File.ReadAllBytesAsync(path2.FilePath); Assert.True(bytes1.SequenceEqual(bytes2)); } + + [Fact] + public async Task TestIssue863() // template rows and cells without the optional "r" reference + { + var original = PathHelper.GetFile("xlsx/TestTemplateComplex.xlsx"); + using var withoutReferences = AutoDeletingPath.Create(); + SheetHelper.CopyWithoutCellReferences(original, withoutReferences.FilePath); + + var value = new Dictionary + { + ["title"] = "FooCompany", + ["managers"] = new[] { new { name = "Jack", department = "HR" }, new { name = "Loan", department = "IT" } }, + ["employees"] = new[] { new { name = "Wade", department = "HR" }, new { name = "Keaton", department = "IT" } } + }; + + using var expected = AutoDeletingPath.Create(); + using var actual = AutoDeletingPath.Create(); + await _excelTemplater.FillTemplateAsync(expected.FilePath, original, value); + await _excelTemplater.FillTemplateAsync(actual.FilePath, withoutReferences.FilePath, value); + + var expectedRows = _excelImporter.Query(expected.FilePath).Select(row => ((IDictionary)row).Values.ToArray()).ToList(); + var actualRows = _excelImporter.Query(actual.FilePath).Select(row => ((IDictionary)row).Values.ToArray()).ToList(); + Assert.Equal("A1:C7", SheetHelper.GetFirstSheetDimensionRefValue(actual.FilePath)); + Assert.Equal(expectedRows, actualRows); + Assert.Equal("Keaton", actualRows[6][1]); + } } diff --git a/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesTests.cs b/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesTests.cs index bc95e98a..56b4d853 100644 --- a/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesTests.cs +++ b/tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesTests.cs @@ -3029,4 +3029,49 @@ public void TestIssue980() // deterministic file generation var bytes2 = File.ReadAllBytes(path2.FilePath); Assert.True(bytes1.SequenceEqual(bytes2)); } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void TestIssue863(bool keepRowReferences) // template rows and cells without the optional "r" reference + { + var original = PathHelper.GetFile("xlsx/TestTemplateComplex.xlsx"); + using var withoutReferences = AutoDeletingPath.Create(); + SheetHelper.CopyWithoutCellReferences(original, withoutReferences.FilePath, keepRowReferences); + + var value = new Dictionary + { + ["title"] = "FooCompany", + ["managers"] = new[] { new { name = "Jack", department = "HR" }, new { name = "Loan", department = "IT" } }, + ["employees"] = new[] { new { name = "Wade", department = "HR" }, new { name = "Keaton", department = "IT" } } + }; + + using var expected = AutoDeletingPath.Create(); + using var actual = AutoDeletingPath.Create(); + _excelTemplater.FillTemplate(expected.FilePath, original, value); + _excelTemplater.FillTemplate(actual.FilePath, withoutReferences.FilePath, value); + + var actualRows = ReadValues(actual.FilePath); + Assert.Equal("A1:C7", SheetHelper.GetFirstSheetDimensionRefValue(actual.FilePath)); + Assert.Equal(ReadValues(expected.FilePath), actualRows); + Assert.Equal("Keaton", actualRows[6][1]); + } + + [Fact] + public void TestIssue863_MergeSameCells() + { + using var withoutReferences = AutoDeletingPath.Create(); + SheetHelper.CopyWithoutCellReferences(PathHelper.GetFile("xlsx/TestMergeWithTag.xlsx"), withoutReferences.FilePath); + using var merged = AutoDeletingPath.Create(); + + _excelTemplater.MergeSameCells(merged.FilePath, withoutReferences.FilePath); + + var mergedCells = SheetHelper.GetFirstSheetMergedCells(merged.FilePath); + Assert.Equal("A2:A4", mergedCells[0]); + Assert.Equal("C3:C4", mergedCells[1]); + Assert.Equal("A7:A8", mergedCells[2]); + } + + private List ReadValues(string path) => + _excelImporter.Query(path).Select(row => ((IDictionary)row).Values.ToArray()).ToList(); } diff --git a/tests/MiniExcel.OpenXml.Tests/Utils/SheetHelper.cs b/tests/MiniExcel.OpenXml.Tests/Utils/SheetHelper.cs index 056bd99c..26cc2961 100644 --- a/tests/MiniExcel.OpenXml.Tests/Utils/SheetHelper.cs +++ b/tests/MiniExcel.OpenXml.Tests/Utils/SheetHelper.cs @@ -133,4 +133,25 @@ internal static MemoryStream CreateTestWorkbookStream() stream.Position = 0; return stream; } + + // Copy of the workbook without "r" on its cells, and on its rows unless keepRowReferences (ECMA-376 allows both) + internal static void CopyWithoutCellReferences(string source, string target, bool keepRowReferences = false) + { + File.Copy(source, target, overwrite: true); + using var zip = ZipFile.Open(target, ZipArchiveMode.Update); + foreach (var entry in zip.Entries.Where(e => e.FullName.StartsWith("xl/worksheets/sheet")).ToList()) + { + XDocument doc; + using (var stream = entry.Open()) + doc = XDocument.Load(stream); + + foreach (var element in doc.Descendants().Where(e => e.Name.LocalName == "c" || (e.Name.LocalName == "row" && !keepRowReferences))) + element.Attribute("r")?.Remove(); + + var name = entry.FullName; + entry.Delete(); + using var output = zip.CreateEntry(name).Open(); + doc.Save(output); + } + } }