From a8f9219f098841e03900b83de02012d04c0ab604 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Tue, 3 Jan 2023 21:42:32 -0800 Subject: [PATCH] Introduce support for "Command Timeout" connection property (#1765) --- .../ConnectionProviderOptionsHelper.cs | 10 ++++ .../Connection/ConnectionService.cs | 10 +++- .../Connection/Contracts/ConnectionDetails.cs | 17 +++++++ .../Contracts/ConnectionDetailsExtensions.cs | 1 + .../Connection/ConnectionDetailsTests.cs | 49 ++++++++++++++++++- .../Connection/ConnectionServiceTests.cs | 7 ++- 6 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionProviderOptionsHelper.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionProviderOptionsHelper.cs index a1c4a043..38181fa0 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionProviderOptionsHelper.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionProviderOptionsHelper.cs @@ -101,6 +101,16 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection GroupName = "Initialization" }, new ConnectionOption + { + Name = "commandTimeout", + DisplayName = "Command timeout", + Description = + "The length of time (in seconds) to wait for a command to complete on the server before terminating the attempt and generating an error", + ValueType = ConnectionOption.ValueTypeNumber, + DefaultValue = "30", + GroupName = "Initialization" + }, + new ConnectionOption { Name = "currentLanguage", DisplayName = "Current language", diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs index b0fa53b0..56ea1de9 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs @@ -1403,6 +1403,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection { connectionBuilder.ConnectTimeout = connectionDetails.ConnectTimeout.Value; } + if (connectionDetails.CommandTimeout.HasValue) + { + connectionBuilder.CommandTimeout = connectionDetails.CommandTimeout.Value; + } if (connectionDetails.ConnectRetryCount.HasValue) { connectionBuilder.ConnectRetryCount = connectionDetails.ConnectRetryCount.Value; @@ -1554,6 +1558,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection ConnectRetryCount = builder.ConnectRetryCount, ConnectRetryInterval = builder.ConnectRetryInterval, ConnectTimeout = builder.ConnectTimeout, + CommandTimeout = builder.CommandTimeout, CurrentLanguage = builder.CurrentLanguage, DatabaseName = builder.InitialCatalog, ColumnEncryptionSetting = builder.ColumnEncryptionSetting.ToString(), @@ -1731,11 +1736,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection { // capture original values int? originalTimeout = connInfo.ConnectionDetails.ConnectTimeout; + int? originalCommandTimeout = connInfo.ConnectionDetails.CommandTimeout; bool? originalPersistSecurityInfo = connInfo.ConnectionDetails.PersistSecurityInfo; bool? originalPooling = connInfo.ConnectionDetails.Pooling; - // increase the connection timeout to at least 30 seconds and and build connection string + // increase the connection and command timeout to at least 30 seconds and and build connection string connInfo.ConnectionDetails.ConnectTimeout = Math.Max(30, originalTimeout ?? 0); + connInfo.ConnectionDetails.CommandTimeout = Math.Max(30, originalCommandTimeout ?? 0); // enable PersistSecurityInfo to handle issues in SMO where the connection context is lost in reconnections connInfo.ConnectionDetails.PersistSecurityInfo = true; // turn off connection pool to avoid hold locks on server resources after calling SqlConnection Close method @@ -1747,6 +1754,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection // restore original values connInfo.ConnectionDetails.ConnectTimeout = originalTimeout; + connInfo.ConnectionDetails.CommandTimeout = originalCommandTimeout; connInfo.ConnectionDetails.PersistSecurityInfo = originalPersistSecurityInfo; connInfo.ConnectionDetails.Pooling = originalPooling; diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs index 6634b4e8..8e31bb68 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs @@ -233,6 +233,22 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts } } + /// + /// Gets or sets the length of time (in seconds) to wait for a command to complete on the server before terminating the attempt and generating an error. + /// + public int? CommandTimeout + { + get + { + return GetOptionValue("commandTimeout"); + } + + set + { + SetOptionValue("commandTimeout", value); + } + } + /// /// The number of reconnections attempted after identifying that there was an idle connection failure. /// @@ -608,6 +624,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts && ConnectRetryCount == other.ConnectRetryCount && ConnectRetryInterval == other.ConnectRetryInterval && ConnectTimeout == other.ConnectTimeout + && CommandTimeout == other.CommandTimeout && string.Equals(CurrentLanguage, other.CurrentLanguage, System.StringComparison.InvariantCultureIgnoreCase) && string.Equals(DatabaseDisplayName, other.DatabaseDisplayName, System.StringComparison.InvariantCultureIgnoreCase) && string.Equals(DatabaseName, other.DatabaseName, System.StringComparison.InvariantCultureIgnoreCase) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetailsExtensions.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetailsExtensions.cs index 0fd7ae83..e5353a6b 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetailsExtensions.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetailsExtensions.cs @@ -30,6 +30,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts HostNameInCertificate = details.HostNameInCertificate, PersistSecurityInfo = details.PersistSecurityInfo, ConnectTimeout = details.ConnectTimeout, + CommandTimeout = details.CommandTimeout, ConnectRetryCount = details.ConnectRetryCount, ConnectRetryInterval = details.ConnectRetryInterval, ApplicationName = details.ApplicationName, diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs index 900973c7..a14042b2 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs @@ -41,6 +41,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.AreEqual(details.ConnectRetryInterval, expectedForInt); Assert.AreEqual(details.ConnectRetryCount, expectedForInt); Assert.AreEqual(details.ConnectTimeout, expectedForInt); + Assert.AreEqual(details.CommandTimeout, expectedForInt); Assert.AreEqual(details.LoadBalanceTimeout, expectedForInt); Assert.AreEqual(details.MaxPoolSize, expectedForInt); Assert.AreEqual(details.MinPoolSize, expectedForInt); @@ -82,6 +83,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection details.ConnectRetryInterval = expectedForInt + index++; details.ConnectRetryCount = expectedForInt + index++; details.ConnectTimeout = expectedForInt + index++; + details.CommandTimeout = expectedForInt + index++; details.LoadBalanceTimeout = expectedForInt + index++; details.MaxPoolSize = expectedForInt + index++; details.MinPoolSize = expectedForInt + index++; @@ -115,6 +117,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.AreEqual(details.ConnectRetryInterval, expectedForInt + index++); Assert.AreEqual(details.ConnectRetryCount, expectedForInt + index++); Assert.AreEqual(details.ConnectTimeout, expectedForInt + index++); + Assert.AreEqual(details.CommandTimeout, expectedForInt + index++); Assert.AreEqual(details.LoadBalanceTimeout, expectedForInt + index++); Assert.AreEqual(details.MaxPoolSize, expectedForInt + index++); Assert.AreEqual(details.MinPoolSize, expectedForInt + index++); @@ -157,6 +160,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection details.ConnectRetryInterval = expectedForInt + index++; details.ConnectRetryCount = expectedForInt + index++; details.ConnectTimeout = expectedForInt + index++; + details.CommandTimeout = expectedForInt + index++; details.LoadBalanceTimeout = expectedForInt + index++; details.MaxPoolSize = expectedForInt + index++; details.MinPoolSize = expectedForInt + index++; @@ -199,7 +203,6 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection } } - [Test] public void SettingConnectiomTimeoutToLongShouldStillReturnInt() { @@ -229,6 +232,35 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.AreEqual(details.ConnectTimeout, expectedValue); } + [Test] + public void SettingCommandTimeoutToLongShouldStillReturnInt() + { + ConnectionDetails details = new ConnectionDetails(); + + long timeout = 30; + int? expectedValue = 30; + details.Options["commandTimeout"] = timeout; + + Assert.AreEqual(details.CommandTimeout, expectedValue); + } + + [Test] + public void CommandTimeoutShouldReturnNullIfNotSet() + { + ConnectionDetails details = new ConnectionDetails(); + int? expectedValue = null; + Assert.AreEqual(details.CommandTimeout, expectedValue); + } + + [Test] + public void CommandTimeoutShouldReturnNullIfSetToNull() + { + ConnectionDetails details = new ConnectionDetails(); + details.Options["commandTimeout"] = null; + int? expectedValue = null; + Assert.AreEqual(details.CommandTimeout, expectedValue); + } + [Test] public void SettingEncrypShouldReturnExpectedEncryptOption() { @@ -262,6 +294,21 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.That(details.Encrypt, Is.EqualTo("Mandatory"), "Encrypt should be mandatory."); } + [Test] + public void SettingCommandTimeoutToLongWhichCannotBeConvertedToIntShouldNotCrash() + { + ConnectionDetails details = new ConnectionDetails(); + + long timeout = long.MaxValue; + int? expectedValue = null; + string? expectedEncryptValue = "Strict"; + details.Options["commandTimeout"] = timeout; + details.Options["encrypt"] = expectedEncryptValue; + + Assert.That(details.CommandTimeout, Is.EqualTo(expectedValue), "Command Timeout not as expected"); + Assert.That(details.Encrypt, Is.EqualTo("Strict"), "Encrypt should be strict."); + } + [Test] public void ConnectionSettingsComparableShouldConsiderAdvancedOptions() { diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionServiceTests.cs index b3a3a119..e02b56cc 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionServiceTests.cs @@ -538,6 +538,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection new object[] {"PersistSecurityInfo", true, "Persist Security Info"}, new object[] {"PersistSecurityInfo", false, "Persist Security Info"}, new object[] {"ConnectTimeout", 15, "Connect Timeout"}, + new object[] {"CommandTimeout", 30, "Command Timeout"}, new object[] {"ConnectRetryCount", 1, "Connect Retry Count"}, new object[] {"ConnectRetryInterval", 10, "Connect Retry Interval"}, new object[] {"ApplicationName", "vscode-mssql", "Application Name"}, @@ -1687,7 +1688,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection // If we make a connection to a live database ConnectionService service = ConnectionService.Instance; - var connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Persist Security Info=False;User ID={your_username};Password={your_password};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;HostNameInCertificate={servername}"; + var connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Persist Security Info=False;User ID={your_username};Password={your_password};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;Command Timeout=30;HostNameInCertificate={servername}"; var details = service.ParseConnectionString(connectionString); Assert.That(details.ServerName, Is.EqualTo("tcp:{servername},1433"), "Unexpected server name"); @@ -1700,6 +1701,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.That(details.TrustServerCertificate, Is.False, "Unexpected database name value"); Assert.That(details.HostNameInCertificate, Is.EqualTo("{servername}"), "Unexpected Host Name in Certificate value"); Assert.That(details.ConnectTimeout, Is.EqualTo(30), "Unexpected Connect Timeout value"); + Assert.That(details.CommandTimeout, Is.EqualTo(30), "Unexpected CommandTimeout Timeout value"); } /// @@ -1711,7 +1713,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection // If we make a connection to a live database ConnectionService service = ConnectionService.Instance; - var connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Persist Security Info=False;User ID={your_username};Password={your_password};MultipleActiveResultSets=False;Encrypt=Strict;TrustServerCertificate=False;Connection Timeout=30;HostNameInCertificate={servername}"; + var connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Persist Security Info=False;User ID={your_username};Password={your_password};MultipleActiveResultSets=False;Encrypt=Strict;TrustServerCertificate=False;Connection Timeout=30;Command Timeout=30;HostNameInCertificate={servername}"; var details = service.ParseConnectionString(connectionString); Assert.That(details.ServerName, Is.EqualTo("tcp:{servername},1433"), "Unexpected server name"); @@ -1724,6 +1726,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.That(details.TrustServerCertificate, Is.False, "Unexpected database name value"); Assert.That(details.HostNameInCertificate, Is.EqualTo("{servername}"), "Unexpected Host Name in Certificate value"); Assert.That(details.ConnectTimeout, Is.EqualTo(30), "Unexpected Connect Timeout value"); + Assert.That(details.CommandTimeout, Is.EqualTo(30), "Unexpected Command Timeout value"); } [Test]