feat(sdk): add nanotdf plaintext policy #259
Conversation
sujankota
commented
May 21, 2025
sujankota
commented
- Updated NanoTDF and Config to support EMBEDDED_POLICY_PLAIN_TEXT and EMBEDDED_POLICY_ENCRYPTED policy types
- Enabled encryption and decryption workflows for both policy configurations
- Ensured compatibility and configurability via Config.withPolicyType
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively introduces support for EMBEDDED_POLICY_PLAIN_TEXT and EMBEDDED_POLICY_ENCRYPTED policy types in NanoTDF, enhancing its flexibility. The changes are well-structured, and the inclusion of new tests for these policy types is commendable.
I've identified a couple of areas for improvement primarily related to code clarity, minor efficiency, and error reporting, which are detailed in the comments below. Addressing these would further strengthen the codebase.
Summary of Findings
- Code Optimization in Policy Handling: In
NanoTDF.java(lines 120-132), thepolicyObjectAsStr.getBytes()call is duplicated, andAesGcmis initialized unconditionally even when not needed for plaintext policies. Refactoring this can improve efficiency and readability. - Error Message Clarity: The error message for unsupported policy types in
NanoTDF.java(line 172) is generic. Making it more specific by including the problematic policy type would improve debuggability.
Merge Readiness
The pull request is well on its way to being mergeable. The core functionality is implemented and tested. However, I recommend addressing the medium severity comments regarding code optimization and error message clarity to enhance the overall quality and maintainability of the code. Once these suggestions are considered, the PR should be in excellent shape. As an AI assistant, I am not authorized to approve pull requests; please ensure further review and approval by authorized team members before merging.
|
🤖 I have created a release *beep* *boop* --- <details><summary>0.8.0</summary> ## [0.8.0](v0.7.9...v0.8.0) (2025-05-27) ### ⚠ BREAKING CHANGES * **sdk:** better control which classes we expose ([#254](#254)) * **sdk:** expose method to build an http client ([#256](#256)) * **sdk:** switch to `connect-rpc` for GRPC ([#244](#244)) * **sdk:** DSPX-1037 update sdk api ([#249](#249)) ### Features * Add initial Dependency Review configuration ([#252](#252)) ([ba11d49](ba11d49)) * **sdk:** add nanotdf plaintext policy ([#259](#259)) ([be9cd94](be9cd94)) * **sdk:** better control which classes we expose ([#254](#254)) ([e00eeaf](e00eeaf)) * **sdk:** DSPX-1037 update sdk api ([#249](#249)) ([f2e1307](f2e1307)) * **sdk:** expose method to build an http client ([#256](#256)) ([2cac97c](2cac97c)) * **sdk:** switch to `connect-rpc` for GRPC ([#244](#244)) ([ff36a1d](ff36a1d)) ### Bug Fixes * **sdk:** add coverage ([#258](#258)) ([e6ad1bc](e6ad1bc)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
