Introduce Async API counterparts for AE base class #3673
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds asynchronous counterparts to key store provider APIs in SqlColumnEncryptionKeyStoreProvider to enable async implementations for Always Encrypted key operations.
Key changes:
- Introduced async virtual methods (Decrypt/Encrypt column encryption key, Sign/Verify CMK metadata) that currently throw NotImplementedException.
- Added corresponding XML documentation entries for the new async methods.
- Minor wording adjustments in existing XML return documentation.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionKeyStoreProvider.cs | Adds async virtual method stubs for encryption key and metadata operations. |
| doc/snippets/Microsoft.Data.SqlClient/SqlColumnEncryptionKeyStoreProvider.xml | Documents newly added async methods and adjusts some existing return descriptions. |
...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionKeyStoreProvider.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionKeyStoreProvider.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionKeyStoreProvider.cs
Outdated
Show resolved
Hide resolved
...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlColumnEncryptionKeyStoreProvider.cs
Outdated
Show resolved
Hide resolved
doc/snippets/Microsoft.Data.SqlClient/SqlColumnEncryptionKeyStoreProvider.xml
Show resolved
Hide resolved
doc/snippets/Microsoft.Data.SqlClient/SqlColumnEncryptionKeyStoreProvider.xml
Outdated
Show resolved
Hide resolved
doc/snippets/Microsoft.Data.SqlClient/SqlColumnEncryptionKeyStoreProvider.xml
Outdated
Show resolved
Hide resolved
| public abstract byte[] DecryptColumnEncryptionKey(string masterKeyPath, string encryptionAlgorithm, byte[] encryptedColumnEncryptionKey); | ||
|
|
||
| /// <include file='../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlColumnEncryptionKeyStoreProvider.xml' path='docs/members[@name="SqlColumnEncryptionKeyStoreProvider"]/DecryptColumnEncryptionKeyAsync/*'/> | ||
| public virtual Task<byte[]> DecryptColumnEncryptionKeyAsync(string masterKeyPath, string encryptionAlgorithm, byte[] encryptedColumnEncryptionKey) |
There was a problem hiding this comment.
Cryptographic tasks will normally complete synchronously; Azure Key Vault is the exception here. What about returning ValueTask<byte[]>, to remove an allocation in the synchronous case?
There was a problem hiding this comment.
Since these 4 new async methods are all implemented by calling their synchronous counterparts, I would agree that the operations will definitely complete synchronously.
Do we plan to re-implement these in terms of some other underlying async methods later?
There was a problem hiding this comment.
Yes, these implementations will be re-implemented in all Key Vault providers, as issue #3672 covers. Both sync and async will be made truly sync and truly async flows - with external calls separated.
Also, in the codebase, we want to make sure we call sync v/s async APIs of AE that are separated based on caller.
This, here, is a fallback for any consumer of AE implementation without async implementation.
For #3672:
Introduces async API counterparts in base class 'SqlColumnEncryptionKeyStoreProvider'