feat(core): Identifier in NanoTDF ResourceLocator borrowed from Protocol #106
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduced an IdentifierType enum to categorize different identifier lengths. Updated ResourceLocator constructors to handle identifiers, including parsing and setting appropriate identifier types. Modified ByteBuffer constructor to extract identifier information from the protocol byte.
Replaced string literals for protocol matching with constants and removed unnecessary identifier length variables, streamlining the class. Adjusted identifier handling to directly utilize the array length for type determination, enhancing readability and performance.
Corrected the byte manipulation for protocol and identifier nibbles in ResourceLocator. Removed commented-out identifier types in NanoTDFType and added a new method to retrieve the identifier in ResourceLocator. Added an assertion to ensure the identifier is not null during ResourceLocator creation in NanoTDF.
Enhanced `ResourceLocator` to handle various identifier lengths and added JUnit tests to verify correctness. Updated the `writeIntoBuffer` method to accurately write the identifier based on its type, and added detailed class documentation for better understanding.
pflynn-virtru
changed the title
Added final modifiers to method parameters in ResourceLocator class. Introduced a new method setIdentifier and refactored identifier length handling with system array copy. Modified NanoTDFType to include identifier length constants, and updated tests to reflect these changes.
mkleene
reviewed
Aug 14, 2024
Contributor
mkleene
left a comment
mkleene
left a comment
There was a problem hiding this comment.
Looks good except for the one comment!
| this.identifierType = NanoTDFType.IdentifierType.NONE; | ||
| this.identifier = new byte[0]; | ||
| } else { | ||
| byte[] identifierBytes = identifier.getBytes(); |
Contributor
There was a problem hiding this comment.
I think we should set the charset here. If we don't it uses the platform default
Member
|
merged as #112 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for identifiers in ResourceLocator
Introduced an IdentifierType enum to categorize different identifier lengths. Updated ResourceLocator constructors to handle identifiers, including parsing and setting appropriate identifier types. Modified ByteBuffer constructor to extract identifier information from the protocol byte.
Issue: opentdf/platform#1203
Specification: opentdf/spec#40
ADR: opentdf/platform#900