Fill in missing row and cell references in templates (#863) (#1015)

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.

Author: gdols

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<XElement> 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.");
 

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<string, object>
+        {
+            ["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<string, object?>)row).Values.ToArray()).ToList();
+        var actualRows = _excelImporter.Query(actual.FilePath).Select(row => ((IDictionary<string, object?>)row).Values.ToArray()).ToList();
+        Assert.Equal("A1:C7", SheetHelper.GetFirstSheetDimensionRefValue(actual.FilePath));
+        Assert.Equal(expectedRows, actualRows);
+        Assert.Equal("Keaton", actualRows[6][1]);
+    }
 }

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<string, object>
+        {
+            ["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<object?[]> ReadValues(string path) =>
+        _excelImporter.Query(path).Select(row => ((IDictionary<string, object?>)row).Values.ToArray()).ToList();
 }

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);
+        }
+    }
 }