Skip to content

Conversation

@paulmedynski
Copy link
Contributor

Port of PR #3149 from main to release/6.0:

  • Added ClientInterface class that knows how to build a suitable TDS Login ClientInterfaceName value.
  • Added tests for ClientInterface.
  • Removed unnecessary SQL_PROVIDER_NAME constant.

Spec is here: https://microsoft.sharepoint-df.com/:w:/t/sqldevx/EZYUHqdnoXhKoQv0kxcM1QUB2V_uTCWN9GHfrDQY2tojmA?e=tf4xco

Additional changes:

  • Upgraded a few test dependencies to avoid vulnerabilities.

- Moved logic to build client interface name into its own ClientInterface class.
- Added tests for ClientInterface.
- Reverted changes to Application Name.
- Removed unnecessary SQP_PROVIDER_NAME constant.
- Fixed vulnerable transitive dependencies.
- Removed duplicate System.Text.Json version.
@paulmedynski paulmedynski requested review from a team and Copilot March 5, 2025 13:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR ports changes from main to release/6.0 to enhance the TDS Login ClientInterfaceName field behavior, update related tests, and remove obsolete constants. Key changes include:

  • Introducing the ClientInterface class and a new test (ConnectionOpen_ClientInterfaceName) to validate its behavior.
  • Replacing TdsEnums.SQL_PROVIDER_NAME with Common.DbConnectionStringDefaults.ApplicationName in error reporting and source properties.
  • Removing the SQL_PROVIDER_NAME constant from TdsEnums and updating TdsParser to use ClientInterface.Name.

Reviewed Changes

File Description
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs Added test to verify the ClientInterfaceName in the LOGIN7 packet
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlException.cs Updated the Source property to use the new application name
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlError.cs Updated error source from SQL_PROVIDER_NAME to ApplicationName
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs Removed the obsolete SQL_PROVIDER_NAME constant
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs Updated clientInterfaceName assignment to use ClientInterface.Name
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/GenericTDSServer.cs Added a delegate for processing and capturing validated LOGIN7 requests

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs:125

  • Consider adding a validation to ensure that the length of ClientInterface.Name does not exceed TdsEnums.MAXLEN_CLIENTINTERFACE, similar to the previous debug assert.
string clientInterfaceName = ClientInterface.Name;

@codecov
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 89.47368% with 8 lines in your changes missing coverage. Please review.

Project coverage is 72.64%. Comparing base (cfb007d) to head (2297191).
Report is 3 commits behind head on release/6.0.

Files with missing lines Patch % Lines
...nt/src/Microsoft/Data/SqlClient/ClientInterface.cs 89.04% 8 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           release/6.0    #3200      +/-   ##
===============================================
- Coverage        72.78%   72.64%   -0.15%     
===============================================
  Files              285      286       +1     
  Lines            59162    59235      +73     
===============================================
- Hits             43064    43034      -30     
- Misses           16098    16201     +103     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore 75.46% <89.47%> (-0.08%) ⬇️
netfx 71.05% <86.48%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cheenamalhotra cheenamalhotra added this to the 6.0.2 milestone Mar 5, 2025
@cheenamalhotra cheenamalhotra merged commit f3ff3d3 into release/6.0 Mar 5, 2025
252 checks passed
@cheenamalhotra cheenamalhotra deleted the dev/paul/release/6.0 branch March 5, 2025 23:16
cheenamalhotra added a commit that referenced this pull request Mar 12, 2025
cheenamalhotra added a commit that referenced this pull request Mar 13, 2025
@cheenamalhotra cheenamalhotra removed this from the 6.0.2 milestone Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants