[ClientEncryption] Remove JsonProcessor from EncryptionOptions #5454
Conversation
MartinSarkany
requested review from
FabianMeiswinkel,
Pilchie,
kirankumarkolli and
sboshra
as code owners
| /// <summary> | ||
| /// API for JSON processing | ||
| /// </summary> | ||
| public enum JsonProcessor |
There was a problem hiding this comment.
Do we need to keep this public?
There was a problem hiding this comment.
Depends on how we want users to set the processor in RequestOptions. RequestOptions.Properties can store it as enum or string. Do you suggest we only keep string? Or we add helper methods that set it without actually exposing the enum?
There was a problem hiding this comment.
At this point I would say we might as well keep it as a string. The API will likely only remain for "finite" amount of time and since we will not be adding the convenience extension as per Kiran's request I do not see much value in this enum being public.
There was a problem hiding this comment.
I made it internal. Please review the changes.
| }, | ||
| Properties = new Dictionary<string, object> | ||
| { | ||
| { "encryption-json-processor", JsonProcessor.Stream } |
There was a problem hiding this comment.
JsonProcessor is an internal type and not accessible for external usage.
Please update test exactly the way external CX use.
There was a problem hiding this comment.
I updated tests to use string instead of enum.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
4 participants
Pull Request Template
#4678
Description
This removes
JsonProcessorfromEncryptionOptions.EncryptionOptionsare used inRequestOptionsthat already containJsonProcessorinRequestOptions.Propertiesdictionary.EncryptionOptions.JsonProcessoris redundant and cannot be used inRequestOptionssubclasses that do not containEncryptionOptions.Type of change