Task 41737: Merge feat/azure-split to main: Step 3 - Tie everything together

paulmedynski · paulmedynski · commit 9aa4f090a854 · 2026-01-23T10:51:40.000-04:00
- Re-activate the Azure tests.
- Address PR comments.
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderInternal.cs b/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderInternal.cs
@@ -48,7 +48,8 @@ static Internal()
                     return;
                 }
 
-                // TODO(ADO-39845): Verify the assembly is signed by us?
+                // TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39845):
+                // Verify the assembly is signed by us?
 
                 // Look for the manager class.
                 const string className = "Microsoft.Data.SqlClient.SqlAuthenticationProviderManager";
@@ -65,7 +66,7 @@ static Internal()
                 _getProvider = manager.GetMethod(
                     "GetProvider",
                     BindingFlags.NonPublic | BindingFlags.Static);
-                
+
                 if (_getProvider is null)
                 {
                     Log($"MDS GetProvider() method not found; " +
@@ -75,7 +76,7 @@ static Internal()
                 _setProvider = manager.GetMethod(
                     "SetProvider",
                     BindingFlags.NonPublic | BindingFlags.Static);
-                
+
                 if (_setProvider is null)
                 {
                     Log($"MDS SetProvider() method not found; " +
@@ -164,7 +165,7 @@ internal static bool SetProvider(
                 if (!result.HasValue)
                 {
                     Log($"SetProvider() invocation returned null; " +
-                        "translating to false"); 
+                        "translating to false");
                     return false;
                 }
 
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/AADAuthenticationTests.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/AADAuthenticationTests.cs
@@ -4,10 +4,6 @@
 
 namespace Microsoft.Data.SqlClient.Extensions.Azure.Test;
 
-// TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):
-// Enable this file once the MDS Azure files have been removed.
-#if false
-
 // These tests were moved from MDS FunctionalTests AADAuthenticationTests.cs.
 public class AADAuthenticationTests
 {
@@ -35,5 +31,3 @@ public void CustomActiveDirectoryProviderTest_AppClientId_DeviceFlowCallback()
         Assert.Same(authProvider, SqlAuthenticationProvider.GetProvider(SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow));
     }
 }
-
-#endif
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/AADConnectionTest.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/AADConnectionTest.cs
@@ -2,10 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):
-// Enable this file once the MDS Azure files have been removed.
-#if false
-
 using System.Text.RegularExpressions;
 
 namespace Microsoft.Data.SqlClient.Extensions.Azure.Test;
@@ -392,5 +388,3 @@ public static string RetrieveValueFromConnStr(string connStr, string[] keywords)
 
     #endregion
 }
-
-#endif
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/Azure.Test.csproj b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/Azure.Test.csproj
@@ -44,19 +44,14 @@
     <ProjectReference Include="$(RepoRoot)/src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Microsoft.Data.SqlClient.TestUtilities.csproj" />
   </ItemGroup>
 
-  <!--
-    TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):
-    Uncomment the below items once the Azure files are removed from MDS.
-  -->
-
   <!-- Conditional Project References -->
-  <!-- <ItemGroup Condition="'$(ReferenceType)' == 'Project'">
+  <ItemGroup Condition="'$(ReferenceType)' == 'Project'">
     <ProjectReference Include="$(RepoRoot)/src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj" />
-  </ItemGroup> -->
+  </ItemGroup>
 
   <!-- Conditional Package References -->
-  <!-- <ItemGroup Condition="'$(ReferenceType)' == 'Package'">
+  <ItemGroup Condition="'$(ReferenceType)' == 'Package'">
     <PackageReference Include="Microsoft.Data.SqlClient" />
-  </ItemGroup> -->
+  </ItemGroup>
 
 </Project>
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs
@@ -63,10 +63,8 @@ internal static class Config
     internal static bool HasUserManagedIdentityClientId() => !UserManagedIdentityClientId.Empty();
     internal static bool HasWorkloadIdentityFederationServiceConnectionId() => !WorkloadIdentityFederationServiceConnectionId.Empty();
 
-    // TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):
-    // Uncomment this once the MDS Azure files have been removed.
-    // internal static bool IsAzureSqlServer() =>
-    //     Utils.IsAzureSqlServer(new SqlConnectionStringBuilder(TcpConnectionString).DataSource);
+    internal static bool IsAzureSqlServer() =>
+        Utils.IsAzureSqlServer(new SqlConnectionStringBuilder(TcpConnectionString).DataSource);
 
     internal static bool OnAdoPool() => AdoPool;
     internal static bool OnLinux() => RuntimeInformation.IsOSPlatform(OSPlatform.Linux);
diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/DefaultAuthProviderTests.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/DefaultAuthProviderTests.cs
@@ -2,10 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):
-// Enable this file once the MDS Azure files have been removed.
-#if false
-
 namespace Microsoft.Data.SqlClient.Extensions.Azure.Test;
 
 public class DefaultAuthProviderTests
@@ -64,5 +60,3 @@ public void AuthProviderInstalled()
         }
     }
 }
-
-#endif
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs
@@ -1674,7 +1674,7 @@ internal void OnFeatureExtAck(int featureId, byte[] data)
                             $"SqlInternalConnectionTds.OnFeatureExtAck | ERR | " +
                             $"Object ID {ObjectID}, " +
                             $"Unknown token for JSONSUPPORT");
-                        
+
                         throw SQL.ParsingError(ParsingErrorState.CorruptedTdsStream);
                     }
 
@@ -2691,7 +2691,7 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
             const int defaultRetryPeriod = 100;
 
             // Number of attempts we are willing to perform.
-            const int maxAttempts = 1;
+            const int maxAttempts = 2;
 
             // Username to use in error messages.
             string? username = null;
@@ -2704,7 +2704,7 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
 
             // We will perform retries if the provider indicates an error that
             // is retryable.
-            for (int attempt = 0; attempt <= maxAttempts; ++attempt)
+            for (int attempt = 1; attempt <= maxAttempts; ++attempt)
             {
                 try
                 {
@@ -2809,7 +2809,7 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
                                     authParamsBuilder.WithPassword(ConnectionOptions.Password);
                                 }
                                 SqlAuthenticationParameters parameters = authParamsBuilder;
-                                CancellationTokenSource cts = new();
+                                using CancellationTokenSource cts = new();
                                 // Use Connection timeout value to cancel token acquire request after certain period of time.(int)
                                 if (_timeout.MillisecondsRemaining < Int32.MaxValue)
                                 {
@@ -2845,12 +2845,12 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
                         throw SQL.ActiveDirectoryTokenRetrievingTimeout(Enum.GetName(typeof(SqlAuthenticationMethod), ConnectionOptions.Authentication), ex.FailureCode, ex);
                     }
 
-                    // We use a doubling backoff if the provider didn't provide
+                    // We use a linear backoff if the provider didn't provide
                     // a retry period.
                     int retryPeriod =
                       ex.RetryPeriod > 0
                       ? ex.RetryPeriod
-                      : defaultRetryPeriod * (2 ^ attempt);
+                      : defaultRetryPeriod * attempt;
 
                     SqlClientEventSource.Log.TryAdvancedTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken|ADV> {0}, Attempt: {1}, sleeping {2}[Milliseconds]", ObjectID, attempt, retryPeriod);
                     SqlClientEventSource.Log.TryAdvancedTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken|ADV> {0}, Attempt: {1}, remaining {2}[Milliseconds]", ObjectID, attempt, _timeout.MillisecondsRemaining);
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAuthenticationProviderManager.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAuthenticationProviderManager.cs
@@ -66,7 +66,8 @@ static SqlAuthenticationProviderManager()
                     return;
                 }
 
-                // TODO(ADO-39845): Verify the assembly is signed by us?
+                // TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39845):
+                // Verify the assembly is signed by us?
 
                 SqlClientEventSource.Log.TryTraceEvent(
                     nameof(SqlAuthenticationProviderManager) +
@@ -262,18 +263,15 @@ internal static bool SetProvider(SqlAuthenticationMethod authenticationMethod, S
                 throw SQL.UnsupportedAuthenticationByProvider(authenticationMethod.ToString(), provider.GetType().Name);
             }
             var methodName = "SetProvider";
-            if (Instance._authenticationsWithAppSpecifiedProvider.Count > 0)
+            foreach (SqlAuthenticationMethod candidateMethod in Instance._authenticationsWithAppSpecifiedProvider)
             {
-                foreach (SqlAuthenticationMethod candidateMethod in Instance._authenticationsWithAppSpecifiedProvider)
+                if (candidateMethod == authenticationMethod)
                 {
-                    if (candidateMethod == authenticationMethod)
-                    {
-                        Instance._sqlAuthLogger.LogError(nameof(SqlAuthenticationProviderManager), methodName, $"Failed to add provider {GetProviderType(provider)} because a user-defined provider with type {GetProviderType(Instance._providers[authenticationMethod])} already existed for authentication {authenticationMethod}.");
+                    Instance._sqlAuthLogger.LogError(nameof(SqlAuthenticationProviderManager), methodName, $"Failed to add provider {GetProviderType(provider)} because a user-defined provider with type {GetProviderType(Instance._providers[authenticationMethod])} already existed for authentication {authenticationMethod}.");
 
-                        // The app has already specified a Provider for this
-                        // authentication method, so we won't override it.
-                        return false;
-                    }
+                    // The app has already specified a Provider for this
+                    // authentication method, so we won't override it.
+                    return false;
                 }
             }
             Instance._providers.AddOrUpdate(

Original file line number	Diff line number	Diff line change
`@@ -4,10 +4,6 @@`
`4`	`4`
`5`	`5`	`namespace Microsoft.Data.SqlClient.Extensions.Azure.Test;`
`6`	`6`
`7`		`-// TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):`
`8`		`-// Enable this file once the MDS Azure files have been removed.`
`9`		`-#if false`
`10`		`-`
`11`	`7`	`// These tests were moved from MDS FunctionalTests AADAuthenticationTests.cs.`
`12`	`8`	`public class AADAuthenticationTests`
`13`	`9`	`{`
`@@ -35,5 +31,3 @@ public void CustomActiveDirectoryProviderTest_AppClientId_DeviceFlowCallback()`
`35`	`31`	`Assert.Same(authProvider, SqlAuthenticationProvider.GetProvider(SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow));`
`36`	`32`	`}`
`37`	`33`	`}`
`38`		`-`
`39`		`-#endif`
Original file line number	Diff line number	Diff line change
`@@ -2,10 +2,6 @@`
`2`	`2`	`// The .NET Foundation licenses this file to you under the MIT license.`
`3`	`3`	`// See the LICENSE file in the project root for more information.`
`4`	`4`
`5`		`-// TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39233):`
`6`		`-// Enable this file once the MDS Azure files have been removed.`
`7`		`-#if false`
`8`		`-`
`9`	`5`	`namespace Microsoft.Data.SqlClient.Extensions.Azure.Test;`
`10`	`6`
`11`	`7`	`public class DefaultAuthProviderTests`
`@@ -64,5 +60,3 @@ public void AuthProviderInstalled()`
`64`	`60`	`}`
`65`	`61`	`}`
`66`	`62`	`}`
`67`		`-`
`68`		`-#endif`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,8 @@ static SqlAuthenticationProviderManager()`
`66`	`66`	`return;`
`67`	`67`	`}`
`68`	`68`
`69`		`- // TODO(ADO-39845): Verify the assembly is signed by us?`
	`69`	`+ // TODO(https://sqlclientdrivers.visualstudio.com/ADO.Net/_workitems/edit/39845):`
	`70`	`+ // Verify the assembly is signed by us?`
`70`	`71`
`71`	`72`	`SqlClientEventSource.Log.TryTraceEvent(`
`72`	`73`	`nameof(SqlAuthenticationProviderManager) +`
`@@ -262,18 +263,15 @@ internal static bool SetProvider(SqlAuthenticationMethod authenticationMethod, S`
`262`	`263`	`throw SQL.UnsupportedAuthenticationByProvider(authenticationMethod.ToString(), provider.GetType().Name);`
`263`	`264`	`}`
`264`	`265`	`var methodName = "SetProvider";`
`265`		`- if (Instance._authenticationsWithAppSpecifiedProvider.Count > 0)`
	`266`	`+ foreach (SqlAuthenticationMethod candidateMethod in Instance._authenticationsWithAppSpecifiedProvider)`
`266`	`267`	`{`
`267`		`- foreach (SqlAuthenticationMethod candidateMethod in Instance._authenticationsWithAppSpecifiedProvider)`
	`268`	`+ if (candidateMethod == authenticationMethod)`
`268`	`269`	`{`
`269`		`- if (candidateMethod == authenticationMethod)`
`270`		`- {`
`271`		`- Instance._sqlAuthLogger.LogError(nameof(SqlAuthenticationProviderManager), methodName, $"Failed to add provider {GetProviderType(provider)} because a user-defined provider with type {GetProviderType(Instance._providers[authenticationMethod])} already existed for authentication {authenticationMethod}.");`
	`270`	`+ Instance._sqlAuthLogger.LogError(nameof(SqlAuthenticationProviderManager), methodName, $"Failed to add provider {GetProviderType(provider)} because a user-defined provider with type {GetProviderType(Instance._providers[authenticationMethod])} already existed for authentication {authenticationMethod}.");`
`272`	`271`
`273`		`- // The app has already specified a Provider for this`
`274`		`- // authentication method, so we won't override it.`
`275`		`- return false;`
`276`		`- }`
	`272`	`+ // The app has already specified a Provider for this`
	`273`	`+ // authentication method, so we won't override it.`
	`274`	`+ return false;`
`277`	`275`	`}`
`278`	`276`	`}`
`279`	`277`	`Instance._providers.AddOrUpdate(`