Skip to content

Commit 87497dc

Browse files
committed
- Addressed review comments:
- Added XML docs throughout. - Improved unit test names. - Removed <RootNamespace> from project files. - Removed unnecessary sample files from the Abstractions project.
1 parent 48e5eb7 commit 87497dc

14 files changed

+264
-167
lines changed

src/Microsoft.Data.SqlClient.Extensions/Abstractions/doc/Sample.xml

Lines changed: 0 additions & 18 deletions
This file was deleted.

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@
2323
<!-- Build Config -->
2424
<PropertyGroup>
2525
<AssemblyName>Microsoft.Data.SqlClient.Extensions.Abstractions</AssemblyName>
26-
<RootNamespace>$(AssemblyName)</RootNamespace>
26+
27+
<!--
28+
Clear the root namespace since our directory structure doesn't match our
29+
namespace definitions. Otherwise, if omitted it would default to the
30+
AssemblyName.
31+
-->
32+
<RootNamespace></RootNamespace>
2733

2834
<!--
2935
We pin the AssemblyVersion of our package to the default major version

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Sample.cs

Lines changed: 0 additions & 20 deletions
This file was deleted.

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ namespace Microsoft.Data.SqlClient;
77
/// <include file='../doc/SqlAuthenticationProviderException.xml' path='docs/members[@name="SqlAuthenticationProviderException"]/SqlAuthenticationProviderException/*'/>
88
public abstract class SqlAuthenticationProviderException : Exception
99
{
10+
/// <summary>
11+
/// The string value used when the failure code is not known.
12+
/// </summary>
13+
private const string Unknown = "Unknown";
14+
1015
/// <include file='../doc/SqlAuthenticationProviderException.xml' path='docs/members[@name="SqlAuthenticationProviderException"]/ctor1/*'/>
1116
protected SqlAuthenticationProviderException(
1217
string message,
@@ -54,6 +59,4 @@ public override string ToString()
5459
$" Method={Method} FailureCode={FailureCode}" +
5560
$" ShouldRetry={ShouldRetry} RetryPeriod={RetryPeriod}";
5661
}
57-
58-
private const string Unknown = "Unknown";
5962
}

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderInternal.cs

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,26 @@ namespace Microsoft.Data.SqlClient;
99
/// <include file='../doc/SqlAuthenticationProvider.xml' path='docs/members[@name="SqlAuthenticationProvider"]/SqlAuthenticationProvider/*'/>
1010
public abstract partial class SqlAuthenticationProvider
1111
{
12-
// This class implements the static GetProvider and SetProvider methods by
13-
// using reflection to call into the Microsoft.Data.SqlClient package's
14-
// SqlAuthenticationProviderManager class, if that assembly is present.
12+
/// <summary>
13+
/// This class implements the static GetProvider and SetProvider methods by
14+
/// using reflection to call into the Microsoft.Data.SqlClient package's
15+
/// SqlAuthenticationProviderManager class, if that assembly is present.
16+
/// </summary>
1517
private static class Internal
1618
{
17-
// Handles to the reflected get/set methods.
19+
/// <summary>
20+
/// Our handle to the reflected GetProvider() method.
21+
/// </summary>
1822
private static MethodInfo? _getProvider = null;
23+
24+
/// <summary>
25+
/// Our handle to the reflected SetProvider() method.
26+
/// </summary>
1927
private static MethodInfo? _setProvider = null;
2028

21-
// Static construction performs the reflection lookups.
29+
/// <summary>
30+
/// Static construction performs the reflection lookups.
31+
/// </summary>
2232
static Internal()
2333
{
2434
// If the MDS package is present, load its
@@ -86,6 +96,7 @@ ex is BadImageFormatException ||
8696
ex is FileLoadException ||
8797
ex is FileNotFoundException)
8898
{
99+
// TODO: Logging
89100
// SqlClientEventSource.Log.TryTraceEvent(
90101
// nameof(SqlAuthenticationProviderManager) +
91102
// $": Azure extension assembly={assemblyName} not found or " +
@@ -95,12 +106,17 @@ ex is FileLoadException ||
95106
// Any other exceptions are fatal.
96107
}
97108

98-
// Call the reflected GetProvider method.
99-
//
100-
// Returns null if reflection failed or any exceptions occur.
101-
// Otherwise, returns as the reflected method does.
102-
//
103-
public static SqlAuthenticationProvider? GetProvider(
109+
/// <summary>
110+
/// Call the reflected GetProvider method.
111+
/// </summary>
112+
/// <param name="authenticationMethod">
113+
/// The authentication method whose provider to get.
114+
/// </param>
115+
/// <returns>
116+
/// Returns null if reflection failed or any exceptions occur.
117+
/// Otherwise, returns as the reflected method does.
118+
/// </returns>
119+
internal static SqlAuthenticationProvider? GetProvider(
104120
SqlAuthenticationMethod authenticationMethod)
105121
{
106122
if (_getProvider is null)
@@ -124,13 +140,21 @@ ex is NotSupportedException ||
124140
}
125141
}
126142

127-
128-
// Call the reflected SetProvider method.
129-
//
130-
// Returns false if reflection failed, invocation fails, or any
131-
// exceptions occur. Otherwise, returns as the reflected method does.
132-
//
133-
public static bool SetProvider(
143+
/// <summary>
144+
/// Call the reflected SetProvider method.
145+
/// </summary>
146+
/// <param name="authenticationMethod">
147+
/// The authentication method whose provider to set.
148+
/// </param>
149+
/// <param name="provider">
150+
/// The provider to set.
151+
/// </param>
152+
/// <returns>
153+
/// Returns false if reflection failed, invocation fails, or any
154+
/// exceptions occur. Otherwise, returns as the reflected method
155+
/// does.
156+
/// </returns>
157+
internal static bool SetProvider(
134158
SqlAuthenticationMethod authenticationMethod,
135159
SqlAuthenticationProvider provider)
136160
{

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationToken.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,15 @@ public SqlAuthenticationToken(
2727
ExpiresOn = expiresOn;
2828
}
2929

30+
/// <summary>
31+
/// The exception thrown by the SqlAuthenticationToken constructor.
32+
/// </summary>
3033
internal sealed class TokenException : SqlAuthenticationProviderException
3134
{
35+
/// <summary>
36+
/// Construct with the exception message.
37+
/// </summary>
38+
/// <param name="message">The exception message.</param>
3239
internal TokenException(string message)
3340
: base(message)
3441
{

src/Microsoft.Data.SqlClient.Extensions/Abstractions/test/SampleTest.cs

Lines changed: 0 additions & 19 deletions
This file was deleted.

src/Microsoft.Data.SqlClient.Extensions/Abstractions/test/SqlAuthenticationMethodTest.cs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ namespace Microsoft.Data.SqlClient.Extensions.Abstractions.Test;
66

77
public class SqlAuthenticationMethodTest
88
{
9-
// Verify the number of expected enum members.
9+
#region Tests
10+
11+
/// <summary>
12+
/// Verify the number of expected enum members.
13+
/// </summary>
1014
[Fact]
11-
public void Member_Count()
15+
public void Confirm_Expected_Member_Count()
1216
{
1317
#if NET
1418
Assert.Equal(11, Enum.GetNames<SqlAuthenticationMethod>().Length);
@@ -17,9 +21,11 @@ public void Member_Count()
1721
#endif
1822
}
1923

20-
// Verify each of the enum member numeric values.
24+
/// <summary>
25+
/// Verify each of the enum member numeric values.
26+
/// </summary>
2127
[Fact]
22-
public void Member_Values()
28+
public void Confirm_Expected_Member_Values()
2329
{
2430
Assert.Equal(0, (int)SqlAuthenticationMethod.NotSpecified);
2531
Assert.Equal(1, (int)SqlAuthenticationMethod.SqlPassword);
@@ -35,4 +41,6 @@ public void Member_Values()
3541
Assert.Equal(9, (int)SqlAuthenticationMethod.ActiveDirectoryDefault);
3642
Assert.Equal(10, (int)SqlAuthenticationMethod.ActiveDirectoryWorkloadIdentity);
3743
}
44+
45+
#endregion
3846
}

src/Microsoft.Data.SqlClient.Extensions/Abstractions/test/SqlAuthenticationParametersTest.cs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ namespace Microsoft.Data.SqlClient.Extensions.Abstractions.Test;
66

77
public class SqlAuthenticationParametersTest
88
{
9-
// Verify that the properties are set correctly when nullable arguments are
10-
// null.
9+
#region Tests
10+
11+
/// <summary>
12+
/// Verify that the properties are set correctly when nullable arguments are
13+
/// null.
14+
/// </summary>
1115
[Fact]
12-
public void Constructor_WithNulls()
16+
public void Constructor_ValidArguments_WithNulls()
1317
{
1418
var method = SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow;
1519
var server = "server";
@@ -43,10 +47,12 @@ public void Constructor_WithNulls()
4347
Assert.Equal(timeout, parameters.ConnectionTimeout);
4448
}
4549

46-
// Verify that the properties are set correctly when nullable arguments
47-
// are non-null.
50+
/// <summary>
51+
/// Verify that the properties are set correctly when nullable arguments are
52+
/// non-null.
53+
/// </summary>
4854
[Fact]
49-
public void Constructor_WithoutNulls()
55+
public void Constructor_ValidArguments_WithoutNulls()
5056
{
5157
var method = SqlAuthenticationMethod.ActiveDirectoryIntegrated;
5258
var server = "server";
@@ -79,4 +85,6 @@ public void Constructor_WithoutNulls()
7985
Assert.Equal(id, parameters.ConnectionId);
8086
Assert.Equal(timeout, parameters.ConnectionTimeout);
8187
}
88+
89+
#endregion
8290
}

0 commit comments

Comments
 (0)