fix: Set max of 101 characters for main table name, to leave enough s…

…pace for suffixes (#109)
CluedIn-io · Jan 18, 2024 · 69352e0 · 69352e0
1 parent 11e6946
commit 69352e0
Show file tree

Hide file tree

Showing 9 changed files with 120 additions and 10 deletions.
diff --git a/global.json b/global.json
@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "7.0.103",
+    "version": "8.0.101",
     "rollForward": "latestFeature"
   },
   "msbuild-sdks": {

diff --git a/src/Connector.SqlServer/Connector/SqlServerConnector.cs b/src/Connector.SqlServer/Connector/SqlServerConnector.cs
@@ -44,7 +44,7 @@ public override async Task<string> GetValidContainerName(ExecutionContext execut
             await using var connectionAndTransaction = await _client.BeginTransaction(config.Authentication);
             var transaction = connectionAndTransaction.Transaction;
 
-            var cleanName = containerName.ToSanitizedSqlName();
+            var cleanName = containerName.ToSanitizedMainTableName();
 
             if (!await CheckTableExists(executionContext, connectorProviderDefinitionId, transaction, cleanName))
             {
@@ -57,7 +57,7 @@ public override async Task<string> GetValidContainerName(ExecutionContext execut
             do
             {
                 count++;
-                newName = $"{cleanName}{count}";
+                newName = $"{cleanName}{count}".ToSanitizedMainTableName();
             } while (await CheckTableExists(executionContext, connectorProviderDefinitionId, transaction, newName));
 
             return newName;

diff --git a/src/Connector.SqlServer/Utils/CreateTableCommandUtility.cs b/src/Connector.SqlServer/Utils/CreateTableCommandUtility.cs
@@ -88,8 +88,9 @@ private static void AddPrimaryConstraint(StringBuilder builder, SqlTableName tab
 
             var keysJoinedWithUnderscore = string.Join("_", primaryKeys.Select(key => key.Name.ToSanitizedSqlName()));
             var keysJoinedWithComma = string.Join(", ", primaryKeys.Select(key => $"[{key.Name.ToSanitizedSqlName()}]"));
+            var primaryKeyName = $"PK_{tableName.LocalName}_{keysJoinedWithUnderscore}".ToSanitizedSqlName();
 
-            builder.AppendLine($"CONSTRAINT [PK_{tableName.LocalName}_{keysJoinedWithUnderscore}] PRIMARY KEY NONCLUSTERED ({keysJoinedWithComma})");
+            builder.AppendLine($"CONSTRAINT [{primaryKeyName}] PRIMARY KEY NONCLUSTERED ({keysJoinedWithComma})");
         }
 
         private static void AddIndexes(StringBuilder builder, SqlTableName tableName, IEnumerable<ColumnDefinition> indexKeys)
@@ -104,7 +105,8 @@ private static void AddIndex(StringBuilder builder, SqlTableName tableName, Colu
         {
             var clusteredString = "NONCLUSTERED";
             var sanitizedName = indexKey.Name.ToSanitizedSqlName();
-            builder.AppendLine($"CREATE {clusteredString} INDEX [IX_{tableName.LocalName}_{sanitizedName}] ON {tableName.FullyQualifiedName} ([{sanitizedName}])");
+            var sanitizedIndexName = $"IX_{tableName.LocalName}_{sanitizedName}".ToSanitizedSqlName();
+            builder.AppendLine($"CREATE {clusteredString} INDEX [{sanitizedIndexName}] ON {tableName.FullyQualifiedName} ([{sanitizedName}])");
         }
     }
 }
diff --git a/src/Connector.SqlServer/Utils/SqlName.cs b/src/Connector.SqlServer/Utils/SqlName.cs
@@ -25,6 +25,20 @@ public static SqlName FromUnsafe(string value)
             return new SqlName(sanitizedValue);
         }
 
+        public static SqlName FromUnsafeMainTable(string value)
+        {
+            if (string.IsNullOrEmpty(value))
+                throw new ArgumentException("Value cannot be null or empty.", nameof(value));
+
+            var sanitizedValue = value.ToSanitizedMainTableName();
+            if (string.IsNullOrEmpty(sanitizedValue))
+            {
+                throw new ArgumentException("Name cannot be empty after being sanitized", nameof(value));
+            }
+
+            return new SqlName(sanitizedValue);
+        }
+
         public static SqlName FromSanitized(string value)
         {
             if (string.IsNullOrEmpty(value)) throw new ArgumentException("Value cannot be null or empty.", nameof(value));

diff --git a/src/Connector.SqlServer/Utils/StringExtensions.cs b/src/Connector.SqlServer/Utils/StringExtensions.cs
@@ -9,6 +9,26 @@ internal static class StringExtensions
         /// Sanitize potentially unsafe value to be a valid SQL name
         /// </summary>
         public static string ToSanitizedSqlName(this string value)
+        {
+            // 127 instead of 128, since we need space for parameter declaration '@'
+            return ToSanitizedSqlNameInternal(value, maxLength: 127);
+        }
+
+        public static string ToSanitizedMainTableName(this string value)
+        {
+            // 101 instead of 128, since we need to leave space for three things:
+            // - Parameter declaration '@'
+            // - Other table names are created using main table name,
+            //   and a suffix, the longest of which is "OutgoingEdgeProperties",
+            //   which has a length of 22 characters.
+            // - Custom types are created, using tables names, and the suffix of "Type",
+            //   which has a length of 4 characters
+            //
+            // In summary: 128 - (1 + 22 + 4) = 101
+            return ToSanitizedSqlNameInternal(value, 101);
+        }
+
+        private static string ToSanitizedSqlNameInternal(string value, int maxLength)
         {
             var sanitizedSqlName = Regex.Replace(value, @"[^_A-Za-z0-9]+", string.Empty);
 
@@ -22,11 +42,11 @@ public static string ToSanitizedSqlName(this string value)
                 sanitizedSqlName = $"_{sanitizedSqlName}";
             }
 
-            if (sanitizedSqlName.Length > 127) // 127 instead of 128, since we need space for parameter declaration '@'
+            if (sanitizedSqlName.Length > maxLength)
             {
                 var hashValue = Math.Abs(GetStableHashCode(value));
                 var postFix = $"_{hashValue:x8}";
-                sanitizedSqlName = $"{sanitizedSqlName.Remove(127 - postFix.Length)}{postFix}";
+                sanitizedSqlName = $"{sanitizedSqlName.Remove(maxLength - postFix.Length)}{postFix}";
             }
 
             return sanitizedSqlName;

diff --git a/src/Connector.SqlServer/Utils/TableNameUtility.cs b/src/Connector.SqlServer/Utils/TableNameUtility.cs
@@ -15,7 +15,7 @@ internal static class TableNameUtility
         [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
         public static SqlTableName GetMainTableName(string name, SqlName schema)
         {
-            return SqlName.FromUnsafe(name).ToTableName(schema);
+            return SqlName.FromUnsafeMainTable(name).ToTableName(schema);
         }
 
         public static SqlTableName GetCodeTableName(IReadOnlyStreamModel model, SqlName schema)

diff --git a/test/unit/Connector.SqlServer.Test/Utils/CreateCustomTypeCommandUtilityTests.cs b/test/unit/Connector.SqlServer.Test/Utils/CreateCustomTypeCommandUtilityTests.cs
@@ -1,4 +1,5 @@
-using CluedIn.Connector.SqlServer.Unit.Tests.Customizations;
+using AutoFixture.Xunit2;
+using CluedIn.Connector.SqlServer.Unit.Tests.Customizations;
 using CluedIn.Connector.SqlServer.Utils;
 using CluedIn.Core.Connectors;
 using CluedIn.Core.Streams.Models;
@@ -10,6 +11,41 @@ namespace CluedIn.Connector.SqlServer.Unit.Tests.Utils;
 
 public class CreateCustomTypeCommandUtilityTests
 {
+    [Theory]
+    [InlineAutoData("TestTableName", "TestTableNameCodesType", "TestTableNameOutgoingEdgesType", "TestTableNameOutgoingEdgePropertiesType", "TestTableNameIncomingEdgesType", "TestTableNameIncomingEdgePropertiesType")]
+    [InlineAutoData(
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeCodesType",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeOutgoingEdgesType",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeOutgoingEdgePropertiesType",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeIncomingEdgesType",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeIncomingEdgePropertiesType")]
+    internal void CustomTypeNamesShouldBeExpectedNames(
+            string containerName,
+            string expectedCodeCustomTableTypeName, string expectedOutgoingEdgeCustomTableTypeName, string expectedOutgoingEdgePropertiesCustomTableTypeName, string expectedIncomingEdgeCustomTableTypeName, string expectedIncomingEdgePropertiesCustomTableTypeName)
+    {
+        // arrange
+        var schema = new SqlName();
+        var streamModel = Substitute.For<IReadOnlyStreamModel>();
+        var createContainerModel = Substitute.For<IReadOnlyCreateContainerModelV2>();
+        streamModel.ContainerName.Returns(containerName);
+        createContainerModel.Name.Returns(containerName);
+
+        // act
+        var codeCustomTableTypeName = CreateCustomTypeCommandUtility.GetCodeTableCustomTypeName(streamModel, schema);
+        var outgoingEdgeCustomTableTypeName = CreateCustomTypeCommandUtility.GetEdgeTableCustomTypeName(streamModel, EdgeDirection.Outgoing, schema);
+        var outgoingEdgePropertiesCustomTableTypeName = CreateCustomTypeCommandUtility.GetEdgePropertiesTableCustomTypeName(streamModel, EdgeDirection.Outgoing, schema);
+        var incomingEdgeCustomTableTypeName = CreateCustomTypeCommandUtility.GetEdgeTableCustomTypeName(streamModel, EdgeDirection.Incoming, schema);
+        var incomingEdgePropertiesCustomTableTypeName = CreateCustomTypeCommandUtility.GetEdgePropertiesTableCustomTypeName(streamModel, EdgeDirection.Incoming, schema);
+
+        // assert
+        codeCustomTableTypeName.LocalName.Value.Should().BeEquivalentTo(expectedCodeCustomTableTypeName);
+        outgoingEdgeCustomTableTypeName.LocalName.Value.Should().BeEquivalentTo(expectedOutgoingEdgeCustomTableTypeName);
+        outgoingEdgePropertiesCustomTableTypeName.LocalName.Value.Should().BeEquivalentTo(expectedOutgoingEdgePropertiesCustomTableTypeName);
+        incomingEdgeCustomTableTypeName.LocalName.Value.Should().BeEquivalentTo(expectedIncomingEdgeCustomTableTypeName);
+        incomingEdgePropertiesCustomTableTypeName.LocalName.Value.Should().BeEquivalentTo(expectedIncomingEdgePropertiesCustomTableTypeName);
+    }
+
     [Theory, AutoNData]
     internal void AllOverloadsShouldGiveSameCodeTableCustomTypeName(IReadOnlyStreamModel streamModel, IReadOnlyCreateContainerModelV2 createContainerModel)
     {

diff --git a/test/unit/Connector.SqlServer.Test/Utils/StringExtensionsTests.cs b/test/unit/Connector.SqlServer.Test/Utils/StringExtensionsTests.cs
@@ -16,6 +16,7 @@ public class StringExtensionsTests
     [InlineAutoData("1", "_1")]
     [InlineAutoData("123", "_123")]
     [InlineAutoData("1Table", "_1Table")]
+    [InlineAutoData("PK_TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", "PK_TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_421fbb16")] // Expected result should not be changed, since we need to ensure that hash is stable
     [InlineAutoData("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa_1760cfe8")] // Expected result should not be changed, since we need to ensure that hash is stable
     public void ToSanitizedTableName_ShouldYieldName(string input, string expectedOutput)
     {

diff --git a/test/unit/Connector.SqlServer.Test/Utils/TableNameUtilityTests.cs b/test/unit/Connector.SqlServer.Test/Utils/TableNameUtilityTests.cs
@@ -1,4 +1,5 @@
-using CluedIn.Connector.SqlServer.Unit.Tests.Customizations;
+using AutoFixture.Xunit2;
+using CluedIn.Connector.SqlServer.Unit.Tests.Customizations;
 using CluedIn.Connector.SqlServer.Utils;
 using CluedIn.Core.Connectors;
 using CluedIn.Core.Streams.Models;
@@ -10,6 +11,42 @@ namespace CluedIn.Connector.SqlServer.Unit.Tests.Utils
 {
     public class TableNameUtilityTests
     {
+        [Theory]
+        [InlineAutoData("TestTableName", "TestTableName", "TestTableNameCodes", "TestTableNameOutgoingEdges", "TestTableNameOutgoingEdgeProperties", "TestTableNameIncomingEdges", "TestTableNameIncomingEdgeProperties")]
+        [InlineAutoData(
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbe",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeCodes",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeOutgoingEdges",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeOutgoingEdgeProperties",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeIncomingEdges",
+            "TestTableName127CharactersAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA_3b2a4bbeIncomingEdgeProperties")]
+        internal void TableNamesShouldBeExpectedNames(
+            string containerName,
+            string expectedMainTableName, string expectedCodeTableName, string expectedOutgoingEdgeTableName, string expectedOutgoingEdgePropertiesTableName, string expectedIncomingEdgeTableName, string expectedIncomingEdgePropertiesTableName)
+        {
+            // arrange
+            var schema = new SqlName();
+            var streamModel = Substitute.For<IReadOnlyStreamModel>();
+            streamModel.ContainerName.Returns(containerName);
+
+            // act
+            var mainTableName                   = TableNameUtility.GetMainTableName(streamModel, schema);
+            var codeTableName                   = TableNameUtility.GetCodeTableName(streamModel, schema);
+            var outgoingEdgeTableName           = TableNameUtility.GetEdgesTableName(streamModel, EdgeDirection.Outgoing, schema);
+            var outgoingEdgePropertiesTableName = TableNameUtility.GetEdgePropertiesTableName(streamModel, EdgeDirection.Outgoing, schema);
+            var incomingEdgeTableName           = TableNameUtility.GetEdgesTableName(streamModel, EdgeDirection.Incoming, schema);
+            var incomingEdgePropertiesTableName = TableNameUtility.GetEdgePropertiesTableName(streamModel, EdgeDirection.Incoming, schema);
+
+            // assert
+            mainTableName.LocalName.Value.Should().BeEquivalentTo(expectedMainTableName);
+            codeTableName.LocalName.Value.Should().BeEquivalentTo(expectedCodeTableName);
+            outgoingEdgeTableName.LocalName.Value.Should().BeEquivalentTo(expectedOutgoingEdgeTableName);
+            outgoingEdgePropertiesTableName.LocalName.Value.Should().BeEquivalentTo(expectedOutgoingEdgePropertiesTableName);
+            incomingEdgeTableName.LocalName.Value.Should().BeEquivalentTo(expectedIncomingEdgeTableName);
+            incomingEdgePropertiesTableName.LocalName.Value.Should().BeEquivalentTo(expectedIncomingEdgePropertiesTableName);
+        }
+
         [Theory, AutoNData]
         internal void AllOverloadsShouldGiveSameMainTableName(IReadOnlyStreamModel streamModel, IReadOnlyCreateContainerModelV2 createContainerModel)
         {