-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes encryption handler #13985
Fixes encryption handler #13985
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
/ok-to-test sha=d6d45cf4d44822d8a2a5533048381136e6cf2a5f |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2358435555. |
UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2358435555. Click to view performance test results
|
1 similar comment
UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2358435555. Click to view performance test results
|
@@ -207,7 +208,8 @@ List<CandidateField> findCandidateFieldsForType(@NonNull Object source) { | |||
|
|||
} | |||
|
|||
synchronized boolean convertEncryption(Object source, Function<String, String> transformer) { | |||
// Return value of convertEncryption does not accurately tell you whether there are encrypted properties or not but can be treated as good estimate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey could you please help me understand in what cases the estimation here would be off @sidhantgoel ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there is collection_unknown and map_unknown in fields it will return true while it may not be true.
1 question, what is need of making this function synchronized?
@nidhi-nair
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's so that the cache we're using does not keep recalculating the values during load time. It basically waits for when that source class type has a type defined and then uses that. We should may be look at improving the conditions for when the recalculation happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for synchronization or inaccurate estimate?
f85a522
to
70c9391
Compare
70c9391
to
c979e64
Compare
/ok-to-test sha=54b82d050212c2f862ece30be5e95b9a075cba24 |
Recording notes from convo @sidhantgoel Revert includes:
|
/ok-to-test sha=c979e6442e059f29f44fe530f93520a06fe9ba70 |
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2370946893. |
Description
Fixes bug in EncryptionHandler that does not work with collection any other than List. This makes EncryptionHandler work with any collection.
Fixes # (issue)
Type of change
How Has This Been Tested?
Checklist: