Targets ClosedXML#1951 - Improves Table Name Validation when setting …

…a table name.

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())
+            {
+                //Table names cannot be empty
+                Assert.False(TableNameValidator.IsValidTableNameInWorkbook(string.Empty, wb, out message));
+                Assert.AreEqual("The table name '' is invalid", message);
+
+                //Table names cannot be Whitespace
+                Assert.False(TableNameValidator.IsValidTableNameInWorkbook("   ", wb, out message));
+                Assert.AreEqual("The table name '   ' is invalid", message);
+
+                //Table names cannot be Null
+                Assert.False(TableNameValidator.IsValidTableNameInWorkbook(null, wb, out message));
+                Assert.AreEqual("The table name '' is invalid", message);
+
+                //Table names cannot start with number
+                Assert.False(TableNameValidator.IsValidTableNameInWorkbook("1Table", wb, 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()), wb, 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", wb, out message));
+                Assert.AreEqual("Table names cannot contain spaces", message);
+
+                //Table names cannot be a cell address
+                Assert.False(TableNameValidator.IsValidTableNameInWorkbook("R1C2", wb, 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 TestTableMustBeUniqueAcrossTheWorkbook()
+        {
+            using (var wb = new XLWorkbook())
+            {
+                var ws1 = wb.AddWorksheet();
+                var ws2 = wb.AddWorksheet();
+                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>(() => t2.Name = "TABLE1");
+                Assert.AreEqual("There is already a table named 'TABLE1' (Parameter 'value')", 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'. (Parameter 'value')",
+                        ex.Message);
+
+                ex = Assert.Throws<ArgumentException>(() => t2.Name = "WorksheetScopedDefinedName");
+                if (ex != null)
+                    Assert.AreEqual(
+                        "Table name must be unique across all named ranges 'WorksheetScopedDefinedName'. (Parameter 'value')",
+                        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()
         {

ClosedXML/Excel/Tables/TableNameValidator.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Collections.Generic;
+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="workbook"></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, IXLWorkbook workbook, out string message)
+        {
+            message = "";
+
+            var existingSheetNames = GetTableNamesAcrossWorkbook(workbook);
+
+            //Validate common name rules, as well as check for existing conflicts
+            if (!XLHelper.ValidateName("table", tableName, String.Empty, existingSheetNames, out message))
+            {
+                return false;
+            }
+
+            //Perform table specific names validation
+            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, 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));
+        }
+
+        /// <summary>
+        /// Get all tables names in the workbook. Table names MUST be unique across the whole workbook, not just the sheet
+        /// </summary>
+        /// <param name="workbook">workbook context</param>
+        /// <returns>String collection representing all the table names in the workbook</returns>
+        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 override XLRangeType RangeType
                 _lastRangeAddress = RangeAddress;
 
                 RescanFieldNames();
-                
+
                 return _fieldNames;
             }
         }
@@ -199,16 +199,20 @@ 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))
+
+                var casingOnlyChange = IsNameChangeACasingOnlyChange(oldname, value);
+
+                if (!casingOnlyChange && !TableNameValidator.IsValidTableNameInWorkbook(value, Worksheet.Workbook, out string message)){
                     throw new ArgumentException(message, nameof(value));
+                }
 
                 _name = value;
 
                 // Some totals row formula depend on the table name. Update them.
                 if (_fieldNames?.Any() ?? false)
                     this.Fields.ForEach(f => (f as XLTableField).UpdateTableFieldTotalsRowFormula());
 
-                if (!String.IsNullOrWhiteSpace(oldname) && !String.Equals(oldname, _name, StringComparison.OrdinalIgnoreCase))
+                if (!casingOnlyChange)
                 {
                     Worksheet.Tables.Add(this);
                     if (Worksheet.Tables.Contains(oldname))
@@ -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; }