Fix Comparable implementation for ConnectionDetails (#1739)

microsoft · Nov 1, 2022 · 36965f6 · 36965f6
1 parent 2f2f65c
commit 36965f6
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 32 deletions.
diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionDetails.cs
@@ -22,7 +22,7 @@ public ConnectionDetails() : base()
         /// <summary>
         /// Gets or sets the connection password
         /// </summary>
-        public string Password 
+        public string Password
         {
             get
             {
@@ -155,7 +155,7 @@ public string? Encrypt
             get
             {
                 string? value = GetOptionValue<string?>("encrypt");
-                if(string.IsNullOrEmpty(value))
+                if (string.IsNullOrEmpty(value))
                 {
                     // Accept boolean values for backwards compatibility.
                     value = GetOptionValue<bool?>("encrypt")?.ToString();
@@ -503,7 +503,7 @@ public int? Port
             {
                 SetOptionValue("port", value);
             }
-        }        
+        }
 
         /// <summary>
         /// Gets or sets a string value that indicates the type system the application expects.
@@ -540,7 +540,7 @@ public string ConnectionString
         /// <summary>
         /// Gets or sets the group ID
         /// </summary>
-        public string GroupId 
+        public string GroupId
         {
             get
             {
@@ -555,7 +555,7 @@ public string GroupId
         /// <summary>
         /// Gets or sets the database display name
         /// </summary>
-        public string DatabaseDisplayName 
+        public string DatabaseDisplayName
         {
             get
             {
@@ -566,7 +566,7 @@ public string DatabaseDisplayName
                 SetOptionValue("databaseDisplayName", value);
             }
         }
-        
+
         public string AzureAccountToken
         {
             get
@@ -591,32 +591,47 @@ public int? ExpiresOn
             }
         }
 
+        /// <summary>
+        /// Compares all SQL Server Connection properties to be able to identify differences in current instance and provided instance appropriately.
+        /// </summary>
+        /// <param name="other">Instance to compare with.</param>
+        /// <returns>True if comparison yeilds no differences, otherwise false.</returns>
         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
@@ -261,5 +261,25 @@ public void SettingConnectiomTimeoutToLongWhichCannotBeConvertedToIntShouldNotCr
             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.");
+        }
     }
 }