Skip to content

remove more duplicate lookups#4089

Merged
mdaigle merged 2 commits into
dotnet:mainfrom
SimonCropp:remove-duplicate-lookups-in-EnclaveDelegate.Crypto.cs
Apr 17, 2026
Merged

remove more duplicate lookups#4089
mdaigle merged 2 commits into
dotnet:mainfrom
SimonCropp:remove-duplicate-lookups-in-EnclaveDelegate.Crypto.cs

Conversation

@SimonCropp
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings March 26, 2026 00:44
@SimonCropp SimonCropp requested a review from a team as a code owner March 26, 2026 00:44
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Mar 26, 2026
Copy link
Copy Markdown
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.

Pull request overview

This PR refactors enclave provider creation in EnclaveDelegate.Crypto.cs to avoid redundant ConcurrentDictionary lookups when initializing providers for different attestation protocols.

Changes:

  • Removes duplicate dictionary lookups by assigning the newly created provider directly to sqlColumnEncryptionEnclaveProvider.
  • Stores the provider instance into s_enclaveProviders without re-reading it back.

Replace ContainsKey + indexer patterns with TryGetParsetableValue (for connection options) and TryGetValue (for _appDomainKeyHash) to avoid double dictionary lookups, remove variable shadowing, and make intent clearer. Existing logic and assertions are preserved.
@SimonCropp SimonCropp changed the title remove duplicate lookups in EnclaveDelegate.Crypto.cs remove more duplicate lookups Mar 26, 2026
@SimonCropp
Copy link
Copy Markdown
Contributor Author

sorry. i dont know how i missed these on the previous pass

@mdaigle mdaigle added this to the 7.1.0-preview1 milestone Apr 1, 2026
@mdaigle mdaigle moved this from To triage to In review in SqlClient Board Apr 1, 2026
@mdaigle
Copy link
Copy Markdown
Contributor

mdaigle commented Apr 16, 2026

/azp run

@mdaigle mdaigle enabled auto-merge (squash) April 16, 2026 23:04
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@mdaigle mdaigle merged commit ea0b777 into dotnet:main Apr 17, 2026
299 of 301 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in SqlClient Board Apr 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.19%. Comparing base (60d4b92) to head (84884b4).
⚠️ Report is 42 commits behind head on main.

Files with missing lines Patch % Lines
...Microsoft/Data/SqlClient/EnclaveDelegate.Crypto.cs 0.00% 6 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (60d4b92) and HEAD (84884b4). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (60d4b92) HEAD (84884b4)
CI-SqlClient 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4089      +/-   ##
==========================================
- Coverage   73.22%   64.19%   -9.03%     
==========================================
  Files         280      271       -9     
  Lines       43000    65722   +22722     
==========================================
+ Hits        31486    42190   +10704     
- Misses      11514    23532   +12018     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 64.19% <14.28%> (?)

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.

@SimonCropp SimonCropp deleted the remove-duplicate-lookups-in-EnclaveDelegate.Crypto.cs branch April 17, 2026 10:01
paulmedynski pushed a commit that referenced this pull request May 7, 2026
* remove duplicate lookups in EnclaveDelegate.Crypto.cs

* Use TryGet methods to avoid double lookups

Replace ContainsKey + indexer patterns with TryGetParsetableValue (for connection options) and TryGetValue (for _appDomainKeyHash) to avoid double dictionary lookups, remove variable shadowing, and make intent clearer. Existing logic and assertions are preserved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants