Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change | Separate tests for NetFx and NetCore - NetFx-Only Connection String Properties #2466

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Change | Separate tests for NetFx and NetCore - NetFx-Only Connection String Properties #2466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ceeacb
Adding TransparentNetworkIpResolution to list of unsupported on platf…
benrr101 Apr 17, 2024
f643c1d
Merge branch 'dotnet:main' into russellben/netfx-connection-string-pr…
benrr101 Apr 22, 2024
8078560
Remove DeprecatedSynonymCount since referencing the unsupported array…
benrr101 Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs
View file
Open in desktop
Expand Up @@ -1037,7 +1037,6 @@ internal static class DbConnectionStringKeywords
internal const string OmitOracleConnectionName = "Omit Oracle Connection Name";

// SqlClient
internal const string TransparentNetworkIPResolution = "Transparent Network IP Resolution";
internal const string Certificate = "Certificate";
#endif
// SqlClient
Expand Down Expand Up @@ -1073,6 +1072,7 @@ internal static class DbConnectionStringKeywords
internal const string IPAddressPreference = "IP Address Preference";
internal const string ServerSPN = "Server SPN";
internal const string FailoverPartnerSPN = "Failover Partner SPN";
internal const string TransparentNetworkIPResolution = "Transparent Network IP Resolution";

// common keywords (OleDb, OracleClient, SqlClient)
internal const string DataSource = "Data Source";
Expand All @@ -1092,10 +1092,9 @@ internal static class DbConnectionStringKeywords

internal static class DbConnectionStringSynonyms
{
#if NETFRAMEWORK
//internal const string TransparentNetworkIPResolution = TRANSPARENTNETWORKIPRESOLUTION;
internal const string TRANSPARENTNETWORKIPRESOLUTION = "transparentnetworkipresolution";
#endif

//internal const string ApplicationName = APP;
internal const string APP = "app";

Expand Down
3 changes: 1 addition & 2 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
View file
Open in desktop
Expand Up @@ -233,7 +233,6 @@ internal static class TRANSACTIONBINDING
internal const int SynonymCount = 33;
#else
internal const int SynonymCount = 30;
internal const int DeprecatedSynonymCount = 2;
#endif // NETFRAMEWORK

private static Dictionary<string, string> s_sqlClientSynonyms;
Expand Down Expand Up @@ -837,7 +836,7 @@ private static bool CompareHostName(ref string host, string name, bool fixup)

int count = SqlConnectionStringBuilder.KeywordsCount + SynonymCount;
#if !NETFRAMEWORK
count += SqlConnectionStringBuilder.DeprecatedKeywordsCount + DeprecatedSynonymCount;
count += SqlConnectionStringBuilder.DeprecatedKeywordsCount;
#endif
synonyms = new Dictionary<string, string>(count, StringComparer.OrdinalIgnoreCase)
{
Expand Down
11 changes: 6 additions & 5 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs
View file
Open in desktop
Expand Up @@ -139,7 +139,7 @@ private enum Keywords
private string _certificate = DbConnectionStringDefaults.Certificate;
#endif
#else
internal const int DeprecatedKeywordsCount = 3;
internal const int DeprecatedKeywordsCount = 5;
#endif
#endregion //Fields

Expand Down Expand Up @@ -363,7 +363,6 @@ private object GetAt(Keywords index)
return MinPoolSize;
case Keywords.MultiSubnetFailover:
return MultiSubnetFailover;
// case Keywords.NamedConnection: return NamedConnection;
case Keywords.PacketSize:
return PacketSize;
case Keywords.Password:
Expand Down Expand Up @@ -912,17 +911,19 @@ public override StandardValuesCollection GetStandardValues(ITypeDescriptorContex
}
}
#else
private static readonly string[] s_notSupportedKeywords = new string[DeprecatedKeywordsCount] {
private static readonly string[] s_notSupportedKeywords = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array initialization is always the same size as the number of elements, so its unnecessary to specify it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are eliminating the DeprecatedKeywordsCount reference here, you should just get rid of DeprecatedKeywordsCount entirely and just reference the size of this array in the one other place it's used. Otherwise, DeprecatedKeywordsCount needs to be changed from 3 to 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@David-Engel Ok, I've updated the PR, chaning DeprecatedKeywordsCount to 5 and removing DeprecatedSynonymsCount from SqlConnectionString. As far as I can tell, I can't access s_notSupportedKeywords array from SqlConnectionString, so I think it makes more sense to just change the number.

As far as I can tell, the only reason for calculating the count is to determine the optimum size of the dictionary. net8 added support for a FrozenDictionary that has its size fixed at initialization, but it looks like we can't utilize that today. That'd be a nice option here...

Please let me know if this isn't what you had in mind.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't considering DeprecatedSynonymCount, just DeprecatedKeywordsCount. Just make sure the count is correct in the debug assert at line 927/928 of SqlConnectionString.cs after your changes. I'm pretty sure it's there to ensure we are initializing the Dictionary to the correct size to maximize perf. An incorrect count won't show up at runtime or in tests.
Debug.Assert(synonyms.Count == count, "incorrect initial ParseSynonyms size");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. It sounds like we're on the same page, then.

DbConnectionStringKeywords.ConnectionReset,
DbConnectionStringKeywords.ContextConnection,
DbConnectionStringKeywords.TransactionBinding,
DbConnectionStringKeywords.TransparentNetworkIPResolution,
DbConnectionStringSynonyms.TRANSPARENTNETWORKIPRESOLUTION,
};

private static readonly string[] s_notSupportedNetworkLibraryKeywords = new string[] {
private static readonly string[] s_notSupportedNetworkLibraryKeywords = {
DbConnectionStringKeywords.NetworkLibrary,

DbConnectionStringSynonyms.NET,
DbConnectionStringSynonyms.NETWORK
DbConnectionStringSynonyms.NETWORK,
};
#endif
#endregion //Private Methods
Expand Down
33 changes: 24 additions & 9 deletions src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionStringBuilderTest.cs
View file
Open in desktop
Expand Up @@ -107,19 +107,34 @@ public void ConnectionStringTests(string connectionString)
ExecuteConnectionStringTests(connectionString);
}

public static readonly IEnumerable<object[]> ConnectionStringTestsNetFx_TestCases = new[]
{
new object[] { "Connection Reset = false" },
new object[] { "Context Connection = false" },
new object[] { "Network Library = dbmssocn" },
new object[] { "Network = dbnmpntw" },
new object[] { "Net = dbmsrpcn" },
new object[] { "TransparentNetworkIPResolution = false" },
new object[] { "Transparent Network IP Resolution = true" },
};

[Theory]
[InlineData("Connection Reset = false")]
[InlineData("Context Connection = false")]
[InlineData("Network Library = dbmssocn")]
[InlineData("Network = dbnmpntw")]
[InlineData("Net = dbmsrpcn")]
[InlineData("TransparentNetworkIPResolution = false")]
[InlineData("Transparent Network IP Resolution = true")]
[SkipOnTargetFramework(~TargetFrameworkMonikers.NetFramework)]
public void ConnectionStringTestsNetFx(string connectionString)
[MemberData(nameof(ConnectionStringTestsNetFx_TestCases))]
#if NETFRAMEWORK
public void ConnectionStringTestsNetFx_OnNetFx_Success(string connectionString)
{
ExecuteConnectionStringTests(connectionString);
}
#else
public void ConnectionStringTestsNetFx_OnNetCore_Throws(string connectionString)
{
// Act
Action action = () => _ = new SqlConnectionStringBuilder(connectionString);

// Assert
Assert.Throws<NotSupportedException>(action);
}
#endif

[Fact]
public void SetInvalidApplicationIntent_Throws()
Expand Down