From 36965f63554e8d895a76a6580490b0e7a09335ce Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Mon, 31 Oct 2022 18:32:40 -0700 Subject: [PATCH] Fix Comparable implementation for ConnectionDetails (#1739) --- .../Connection/Contracts/ConnectionDetails.cs | 79 +++++++++++-------- .../Connection/ConnectionDetailsTests.cs | 20 +++++ 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs index 56972809..6634b4e8 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs @@ -22,7 +22,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts /// /// Gets or sets the connection password /// - public string Password + public string Password { get { @@ -155,7 +155,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts get { string? value = GetOptionValue("encrypt"); - if(string.IsNullOrEmpty(value)) + if (string.IsNullOrEmpty(value)) { // Accept boolean values for backwards compatibility. value = GetOptionValue("encrypt")?.ToString(); @@ -503,7 +503,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts { SetOptionValue("port", value); } - } + } /// /// Gets or sets a string value that indicates the type system the application expects. @@ -540,7 +540,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts /// /// Gets or sets the group ID /// - public string GroupId + public string GroupId { get { @@ -555,7 +555,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts /// /// Gets or sets the database display name /// - public string DatabaseDisplayName + public string DatabaseDisplayName { get { @@ -566,7 +566,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts SetOptionValue("databaseDisplayName", value); } } - + public string AzureAccountToken { get @@ -591,32 +591,47 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts } } + /// + /// Compares all SQL Server Connection properties to be able to identify differences in current instance and provided instance appropriately. + /// + /// Instance to compare with. + /// True if comparison yeilds no differences, otherwise false. public bool IsComparableTo(ConnectionDetails other) - { - if (other == null) - { - return false; - } - - if (ServerName != other.ServerName - || AuthenticationType != other.AuthenticationType - || UserName != other.UserName - || AzureAccountToken != other.AzureAccountToken) - { - return false; - } - - // For database name, only compare if neither is empty. This is important - // Since it allows for handling of connections to the default database, but is - // not a 100% accurate heuristic. - if (!string.IsNullOrEmpty(DatabaseName) - && !string.IsNullOrEmpty(other.DatabaseName) - && DatabaseName != other.DatabaseName) - { - return false; - } - - return true; - } + => other != null + && string.Equals(ApplicationIntent, other.ApplicationIntent, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(ApplicationName, other.ApplicationName, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(AttachDbFilename, other.AttachDbFilename, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(AuthenticationType, other.AuthenticationType, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(AzureAccountToken, other.AzureAccountToken, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(ColumnEncryptionSetting, other.ColumnEncryptionSetting, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(ConnectionString, other.ConnectionString, System.StringComparison.InvariantCultureIgnoreCase) + && ConnectRetryCount == other.ConnectRetryCount + && ConnectRetryInterval == other.ConnectRetryInterval + && ConnectTimeout == other.ConnectTimeout + && string.Equals(CurrentLanguage, other.CurrentLanguage, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(DatabaseDisplayName, other.DatabaseDisplayName, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(DatabaseName, other.DatabaseName, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(EnclaveAttestationProtocol, other.EnclaveAttestationProtocol, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(EnclaveAttestationUrl, other.EnclaveAttestationUrl, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(Encrypt, other.Encrypt, System.StringComparison.InvariantCultureIgnoreCase) + && ExpiresOn == other.ExpiresOn + && string.Equals(FailoverPartner, other.FailoverPartner, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(HostNameInCertificate, other.HostNameInCertificate, System.StringComparison.InvariantCultureIgnoreCase) + && LoadBalanceTimeout == other.LoadBalanceTimeout + && MaxPoolSize == other.MaxPoolSize + && MinPoolSize == other.MinPoolSize + && MultipleActiveResultSets == other.MultipleActiveResultSets + && MultiSubnetFailover == other.MultiSubnetFailover + && PacketSize == other.PacketSize + && string.Equals(Password, other.Password, System.StringComparison.InvariantCultureIgnoreCase) + && PersistSecurityInfo == other.PersistSecurityInfo + && Pooling == other.Pooling + && Port == other.Port + && Replication == other.Replication + && string.Equals(ServerName, other.ServerName, System.StringComparison.InvariantCultureIgnoreCase) + && TrustServerCertificate == other.TrustServerCertificate + && string.Equals(TypeSystemVersion, other.TypeSystemVersion, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(UserName, other.UserName, System.StringComparison.InvariantCultureIgnoreCase) + && string.Equals(WorkstationId, other.WorkstationId, System.StringComparison.InvariantCultureIgnoreCase); } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs index 0387d7f8..900973c7 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionDetailsTests.cs @@ -261,5 +261,25 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Connection Assert.That(details.ConnectTimeout, Is.EqualTo(expectedValue), "Connect Timeout not as expected"); Assert.That(details.Encrypt, Is.EqualTo("Mandatory"), "Encrypt should be mandatory."); } + + [Test] + public void ConnectionSettingsComparableShouldConsiderAdvancedOptions() + { + ConnectionDetails details1 = new ConnectionDetails() + { + ServerName = "Server1", + DatabaseName = "Database", + AuthenticationType = "Integrated", + Encrypt = "Mandatory", + TrustServerCertificate = true + }; + + ConnectionDetails details2 = details1.Clone(); + details2.ConnectTimeout = 60; + Assert.That(details1.IsComparableTo(details2), Is.False, "Different Connection Settings must not be comparable."); + + details1 = details2.Clone(); + Assert.That(details1.IsComparableTo(details2), Is.True, "Same Connection Settings must be comparable."); + } } }