Improve Table Name Validation

ClosedXML.Tests/Excel/Tables/TableNameValidationTests.cs

@@ -0,0 +1,142 @@
+using System;
+using System.Data;
+using System.Linq;
+using ClosedXML.Excel;
+using NUnit.Framework;
+
+namespace ClosedXML.Tests.Excel.Tables
+{
+ [TestFixture]
+ public class TableNameValidationTests
+ {
+ [Test]
+ public void TestTableNameValidatorRules()
+ {
+ string message;
+ using (var wb = new XLWorkbook())
+ {
+ var ws = wb.AddWorksheet(0);
+ //Table names cannot be empty
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook(string.Empty, ws, out message));
+ Assert.AreEqual("The table name '' is invalid", message);
+
+ //Table names cannot be Whitespace
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook(" ", ws, out message));
+ Assert.AreEqual("The table name ' ' is invalid", message);
+
+ //Table names cannot be Null
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook(null, ws, out message));
+ Assert.AreEqual("The table name '' is invalid", message);
+
+ //Table names cannot start with number
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook("1Table", ws, out message));
+ Assert.AreEqual("The table name '1Table' does not begin with a letter, an underscore or a backslash.",
+ message);
+
+ //Strings cannot be longer then 255 charters
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook(
+ new string(Enumerable.Repeat('a', 256).ToArray()), ws, out message));
+ Assert.AreEqual("The table name is more than 255 characters", message);
+
+ //Table names cannot contain spaces
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook("Spaces in name", ws, out message));
+ Assert.AreEqual("Table names cannot contain spaces", message);
+
+ //Table names cannot be a cell address
+ Assert.False(TableNameValidator.IsValidTableNameInWorkbook("R1C2", ws, out message));
+ Assert.AreEqual("Table name cannot be a valid Cell Address 'R1C2'.", message);
+ }
+ }
+
+ [Test]
+ public void AssertCreatingTableWithSpaceInNameThrowsException()
+ {
+ using (var wb = new XLWorkbook())
+ {
+ var ws1 = wb.AddWorksheet();
+ var t1 = ws1.FirstCell().InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ Assert.AreEqual("Table1", t1.Name);
+ Assert.Throws<ArgumentException>(() => t1.Name = "Table name with spaces");
+ }
+ }
+
+ [Test]
+ public void AssertSettingExistingTableToSameNameDoesNotThrowException()
+ {
+ using (var wb = new XLWorkbook())
+ {
+ var ws1 = wb.AddWorksheet();
+ var t1 = ws1.FirstCell().InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ Assert.AreEqual("Table1", t1.Name);
+ Assert.DoesNotThrow(() => t1.Name = "TABLE1");
+ }
+ }
+
+ [Test]
+ public void AssertInsertingTableWithInvalidTableNamesThrowsException()
+ {
+ var dt = new DataTable("sheet1");
+ dt.Columns.Add("Patient", typeof(string));
+ dt.Rows.Add("David");
+
+ using (var wb = new XLWorkbook())
+ {
+ var ws = wb.AddWorksheet("Sheet1");
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "May2019"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "A1"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "R1C2"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "r3c2"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "R2C33333"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "RC"));
+ Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "RC"));
+ }
+ }
+
+ [Test]
+ public void TestTableMustBeUniqueAcrossTheWorksheet()
+ {
+ using (var wb = new XLWorkbook())
+ {
+ var ws1 = wb.AddWorksheet();
+ var t1 = ws1.FirstCell().InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ var t2 = ws1.Cell("G1").InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ Assert.AreEqual("Table1", t1.Name);
+ Assert.AreEqual("Table2", t2.Name);
+ var ex = Assert.Throws<ArgumentException>(() => t2.Name = "TABLE1");
+ Assert.AreEqual("There is already a table named 'TABLE1'", ex?.Message);
+ }
+ }
+
+ [Test]
+ public void TestTableNameIsUniqueAcrossDefinedNames()
+ {
+ using (var wb = new XLWorkbook())
+ {
+ var ws1 = wb.AddWorksheet();
+ var ws2 = wb.AddWorksheet();
+
+ //Create workbook scoped defined name
+ wb.NamedRanges.Add("WorkbookScopedDefinedName", "Sheet1!A1:A10");
+ ws2.NamedRanges.Add("WorksheetScopedDefinedName", "Sheet2!A1:A10");
+
+
+ var t1 = ws1.FirstCell().InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ var t2 = ws2.FirstCell().InsertTable(Enumerable.Range(1, 10).Select(i => new { Number = i }));
+ Assert.AreEqual("Table1", t1.Name);
+ Assert.AreEqual("Table2", t2.Name);
+
+ var ex = Assert.Throws<ArgumentException>(() => t1.Name = "WorkbookScopedDefinedName");
+ if (ex != null)
+ Assert.AreEqual(
+ "Table name must be unique across all named ranges 'WorkbookScopedDefinedName'.",
+ ex.Message);
+
+ ex = Assert.Throws<ArgumentException>(() => t2.Name = "WorksheetScopedDefinedName");
+ if (ex != null)
+ Assert.AreEqual(
+ "Table name must be unique across all named ranges 'WorksheetScopedDefinedName'.",
+ ex.Message);
+ }
+ }
+ }
+}

ClosedXML.Tests/Excel/Tables/TablesTests.cs

@@ -547,25 +547,6 @@ public void CanDeleteTable()
  }
  }
 
- [Test]
- public void TableNameCannotBeValidCellName()
- {
- var dt = new DataTable("sheet1");
- dt.Columns.Add("Patient", typeof(string));
- dt.Rows.Add("David");
-
- using (var wb = new XLWorkbook())
- {
- IXLWorksheet ws = wb.AddWorksheet("Sheet1");
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "May2019"));
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "A1"));
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "R1C2"));
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "r3c2"));
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "R2C33333"));
- Assert.Throws<InvalidOperationException>(() => ws.Cell(1, 1).InsertTable(dt, "RC"));
- }
- }
-
  [Test]
  public void CanDeleteTableField()
  {
@@ -972,7 +953,7 @@ public void CopyDetachedTableDifferentWorksheets()
  ws1.Cell("A2").Value = "Value 1";
  ws1.Cell("B2").Value = 123.45;
  ws1.Cell("C2").Value = new DateTime(2018, 5, 10);
- var original = ws1.Range("A1:C2").AsTable("Detached table");
+ var original = ws1.Range("A1:C2").AsTable("Detached_table");
  var ws2 = wb.Worksheets.Add("Sheet2");
 
  var copy = original.CopyTo(ws2);
@@ -1002,7 +983,7 @@ public void CopyTableDifferentWorksheets()
  ws1.Cell("A2").Value = "Value 1";
  ws1.Cell("B2").Value = 123.45;
  ws1.Cell("C2").Value = new DateTime(2018, 5, 10);
- var original = ws1.Range("A1:C2").AsTable("Attached table");
+ var original = ws1.Range("A1:C2").AsTable("Attached_table");
  ws1.Tables.Add(original);
  var ws2 = wb.Worksheets.Add("Sheet2");
 
@@ -1038,7 +1019,7 @@ public void NewTableHasNullRelId()
  ws.Cell("A2").Value = "Value 1";
  ws.Cell("B2").Value = 123.45;
  ws.Cell("C2").Value = new DateTime(2018, 5, 10);
- var original = ws.Range("A1:C2").CreateTable("Attached table");
+ var original = ws.Range("A1:C2").CreateTable("Attached_table");
 
  Assert.AreEqual(1, ws.Tables.Count());
  Assert.IsNull((original as XLTable).RelId);
@@ -1082,7 +1063,7 @@ public void CopyTableWithoutData()
  ws1.Cell("A2").Value = "Value 1";
  ws1.Cell("B2").Value = 123.45;
  ws1.Cell("C2").Value = new DateTime(2018, 5, 10);
- var original = ws1.Range("A1:C2").AsTable("Attached table");
+ var original = ws1.Range("A1:C2").AsTable("Attached_table");
  ws1.Tables.Add(original);
  var ws2 = wb.Worksheets.Add("Sheet2") as XLWorksheet;
 

ClosedXML.Tests/Excel/Worksheets/XLWorksheetTests.cs

@@ -622,7 +622,7 @@ public void CopyWorksheetPreservesTables()
  ws1.Cell("B3").Value = 50;
  ws1.Cell("A4").Value = "Ivan Ivanov";
  ws1.Cell("B4").Value = 40;
- var table1 = ws1.Range("A2:B4").CreateTable("Test table 1");
+ var table1 = ws1.Range("A2:B4").CreateTable("Test_table_1");
  table1
  .SetShowAutoFilter(true)
  .SetShowTotalsRow(true)

ClosedXML/Excel/Tables/TableNameValidator.cs

@@ -0,0 +1,80 @@
+using System;
+using System.Linq;
+
+namespace ClosedXML.Excel
+{
+ /*
+ * A string representing the name of the table. This is the name that shall be used in formula references,
+ * and displayed in the UI to the spreadsheet user. This name shall not have any spaces in it,
+ * and it must be unique amongst all other displayNames and definedNames in the workbook.
+ * The character lengths and restrictions are the same as for definedNames.
+ * See SpreadsheetML Reference - Workbook definedNames section for details
+ * The possible values for this attribute are defined by the ST_Xstring simple type (§3.18.96).
+ */
+
+ internal static class TableNameValidator
+ {
+ /// <summary>
+ /// Validates if a suggested TableName is valid in the context of a specific workbook
+ /// </summary>
+ /// <param name="tableName">Proposed Table Name</param>
+ /// <param name="worksheet">The worksheet the table will live on</param>
+ /// <param name="message">Message if validation fails</param>
+ /// <returns>True if the proposed table name is valid in the context of the workbook</returns>
+ public static bool IsValidTableNameInWorkbook(string tableName, IXLWorksheet worksheet, out string message)
+ {
+ message = "";
+
+ //TODO: Technically a table name should be unique across the entire workbook not just a sheet.
+ //For now I am just consolidating the table name validation logic here. If we wanted to match Excel
+ //We are going to have to refactor logic around copying tables and worksheets between workbooks
+ //Currently this isn't a problem because on save we call GetTableName in XL_Workbook_Save which will ensure
+ //There are no conflicts in table names when we write the files, but will mean names are changed when you
+ //Reopen the file.
+ var existingSheetNames = worksheet.Tables.Select(t => t.Name);
+ // var existingSheetNames = GetTableNamesAcrossWorkbook(worksheet.Workbook);
+
+ //Validate common name rules, as well as check for existing conflicts
+ if (!XLHelper.ValidateName("table", tableName, String.Empty, existingSheetNames, out message))
+ {
+ return false;
+ }
+
+ //Table names cannot contain spaces
+ if (tableName.Contains(" "))
+ {
+ message = "Table names cannot contain spaces";
+ return false;
+ }
+
+ //Validate TableName is not a Cell Address
+ if (XLHelper.IsValidA1Address(tableName) || XLHelper.IsValidRCAddress(tableName))
+ {
+ message = $"Table name cannot be a valid Cell Address '{tableName}'.";
+ return false;
+ }
+
+
+ //A Table name must be unique across all defined names regardless of if it scoped to workbook or sheet
+ if (IsTableNameIsUniqueAcrossNamedRanges(tableName, worksheet.Workbook))
+ {
+ message = $"Table name must be unique across all named ranges '{tableName}'.";
+ return false;
+ }
+
+ return true;
+ }
+
+ private static bool IsTableNameIsUniqueAcrossNamedRanges(string tableName, IXLWorkbook workbook)
+ {
+ //Check both workbook and worksheet scoped named ranges
+ return workbook.NamedRanges.Contains(tableName) ||
+ workbook.Worksheets.Any(ws => ws.NamedRanges.Contains(tableName));
+ }
+
+ // private static IList<string> GetTableNamesAcrossWorkbook(IXLWorkbook workbook)
+ // {
+ // return workbook.Worksheets.SelectMany(ws => ws.Tables.Select(t => t.Name)).ToList();
+ // }
+ }
+}

ClosedXML/Excel/Tables/XLTable.cs

@@ -51,7 +51,7 @@ public Dictionary<String, IXLTableField> FieldNames
  _lastRangeAddress = RangeAddress;
 
  RescanFieldNames();
- 
+
  return _fieldNames;
  }
  }
@@ -199,8 +199,12 @@ public String Name
  // Validation rules for table names
  var oldname = _name ?? string.Empty;
 
- if (!XLHelper.ValidateName("table", value, oldname, Worksheet.Tables.Select(t => t.Name), out String message))
- throw new ArgumentException(message, nameof(value));
+
+ var casingOnlyChange = IsNameChangeACasingOnlyChange(oldname, value);
+
+ if (!casingOnlyChange && !TableNameValidator.IsValidTableNameInWorkbook(value, Worksheet, out string message)){
+ throw new ArgumentException(message);
+ }
 
  _name = value;
 
@@ -217,6 +221,12 @@ public String Name
  }
  }
 
+ private bool IsNameChangeACasingOnlyChange(string oldname, string newName)
+ {
+ return !String.IsNullOrWhiteSpace(oldname) &&
+ String.Equals(oldname, newName, StringComparison.OrdinalIgnoreCase);
+ }
+
  public Boolean ShowTotalsRow
  {
  get { return _showTotalsRow; }