Encryption predicate #71
Conversation
|
first issue is see is that this can't work with the EncryptionTransformer if i'm not mistaking. That does not have access to the entity so it cant call the predicate function. Does someone see way around that? |
src/entity.ts
Outdated
| @@ -14,6 +14,9 @@ export function encrypt<T extends ObjectLiteral>(entity: any): any { | |||
| let { propertyName, mode, target } = columnMetadata; | |||
| let options: ExtendedColumnOptions = columnMetadata.options; | |||
| let encrypt = options.encrypt; | |||
| if (encrypt?.encryptionPredicate && !encrypt?.encryptionPredicate(entity)) { | |||
There was a problem hiding this comment.
can you merge this with let encrypt = options.encrypt above or with the condition below?
src/entity.ts
Outdated
| @@ -42,6 +45,9 @@ export function decrypt<T extends ObjectLiteral>(entity: any): any { | |||
| let { propertyName, mode, target } = columnMetadata; | |||
| let options: ExtendedColumnOptions = columnMetadata.options; | |||
| let encrypt = options.encrypt; | |||
| if (encrypt?.encryptionPredicate && !encrypt?.encryptionPredicate(entity)) { | |||
There was a problem hiding this comment.
can you merge this with let encrypt = options.encrypt above or with the condition below?
| @@ -5,4 +5,5 @@ export interface EncryptionOptions { | |||
| iv?: string; //// For testing mainly. | |||
| authTagLength?: number; | |||
| looseMatching?: boolean; | |||
| encryptionPredicate?: (entity: any) => boolean; | |||
There was a problem hiding this comment.
maybe change to shouldEncrypt and let it be a function or boolean value?
There was a problem hiding this comment.
I (respectfully) think we should not do that...
Because the term predicate implies a callable on a condition. shouldEncrypt with dual a usage (bool and a callback) as you propose would be less clear and complicates the if checks in the entity class. Lastly the whole intent is it should resolve to a boolean based on some condition. The condition should not be a hardcode false as that simply turn off encryption on column as a whole, and then why would a user use the encrypt at in the first place.
Unless you disagree with this of course I'm happy to make the change.
|
@generalpiston hello. Did you/do you have time to do an other review maybe this week? |
|
Good stuff! I should be able to run a release this weekend. |
#70
I think it is a simple as this... Let me know if see see any issues...