Conversation
cheenamalhotra
added
the
Hotfix Candidate 🚑
There was a problem hiding this comment.
Pull request overview
This PR removes the EnableUserAgent AppContext switch and makes the UserAgent TDS feature extension always included when feature extensions are requested, aligning the driver behavior with always-on UserAgent support.
Changes:
- Remove
LocalAppContextSwitches.EnableUserAgentand related test helper plumbing. - Always write the UserAgent feature extension request in
TdsParserwhenrequestedFeaturesincludesUserAgent. - Update simulated server unit tests to stop toggling the removed switch.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs | Removes per-test switch toggling for UserAgent feature extension. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs | Drops the default-value assertion for the removed switch. |
| src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs | Removes reflection-based support for capturing/restoring the removed switch. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs | Removes the AppContext gate so UserAgent request is written whenever requested. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs | Deletes the EnableUserAgent switch definition, cache field, and accessor. |
| [Fact] | ||
| public void TestConnWithoutUserAgentFeatureExtension() | ||
| { | ||
| // Disable the client-side UserAgent field entirely | ||
| using LocalAppContextSwitchesHelper switchesHelper = new(); | ||
| switchesHelper.EnableUserAgent = false; | ||
|
|
||
| using var server = new TdsServer(); | ||
| server.Start(); | ||
|
|
There was a problem hiding this comment.
TestConnWithoutUserAgentFeatureExtension now runs with UserAgent implicitly requested (see SqlConnectionInternal sets requestedFeatures |= ...UserAgent), so this test's purpose/expectations are no longer valid and it will fail once the client always sends the UserAgent feature extension. Consider removing this test or rewriting it to validate behavior when the server does not ACK/support UserAgent (connection still succeeds, and/or correct ordering/payload).
cheenamalhotra
added
Hotfix 7.0.1
|
Is this info avaliable on the Azure SQL side and if so, how? |
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from In review
to In progress
in SqlClient Board
Removes app context switch no longer needed - feature is to be always-enabled.