-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add RHTNode removal to converter for consistency #201
Conversation
WalkthroughThe recent updates introduce a new boolean field Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant Client
participant Converter
participant Rht
participant NodeAttr
Client->>Converter: Call toCrdtTreeNode()
Converter->>Rht: Convert attributes using toRht()
Rht->>NodeAttr: Set attributes with is_removed field
NodeAttr-->>Rht: Return updated attributes
Rht-->>Converter: Return converted attributes
Converter-->>Client: Return CRDT Tree Node
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- yorkie/proto/yorkie/v1/resources.proto (1 hunks)
- yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt (5 hunks)
- yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt (2 hunks)
- yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt (2 hunks)
- yorkie/src/test/kotlin/dev/yorkie/document/crdt/RhtTest.kt (1 hunks)
Additional comments not posted (10)
yorkie/src/main/kotlin/dev/yorkie/document/crdt/Rht.kt (2)
49-60
: The implementation ofsetInternal
correctly handles the addition and conditional increment of removed elements. Good use of direct map access for performance.
100-101
: ThedeepCopy
method effectively utilizessetInternal
to ensure a complete and accurate copy of theRht
instance, including the state of removal flags.yorkie/src/test/kotlin/dev/yorkie/document/crdt/RhtTest.kt (1)
234-242
: The testshould deepcopy correctly
effectively verifies the deep copy functionality by checking both the JSON output and the size of theRht
instances. It covers scenarios with both active and removed nodes.yorkie/proto/yorkie/v1/resources.proto (1)
229-229
: The addition of theis_removed
field to theNodeAttr
message is a crucial update for handling removal states consistently in serialized forms.yorkie/src/test/kotlin/dev/yorkie/api/ConverterTest.kt (1)
Line range hint
424-450
: The addition of thetree()
method inJsonObject
and its usage in tests effectively demonstrate the manipulation of tree structures, ensuring the correct application of styles and their removal. The tests are well-structured to validate the functionality.yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.kt (5)
263-271
: The conversion functionIterable<RhtNode>.toPBRht()
is implemented correctly and efficiently.
273-279
: The conversion functionMap<String, NodeAttr>.toRht()
is implemented correctly and efficiently.
242-242
: The conversion functionPBTreeNode.toCrdtTreeNode()
is implemented correctly and efficiently.
352-352
: The conversion functionCrdtTreeNode.toPBTreeNodes()
is implemented correctly and efficiently.
11-11
: Ensure that the newly added imports are utilized effectively in the file.Also applies to: 48-48
Verification successful
The newly added imports
NodeAttr
andRhtNode
are utilized effectively in the file.
NodeAttr
is used in the functionstoPBRht
andtoRht
.RhtNode
is used in the functiontoPBRht
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of newly added imports `NodeAttr` and `RhtNode`. # Test: Search for the usage of `NodeAttr` and `RhtNode`. Expect: At least one occurrence of each. rg --type kotlin 'NodeAttr|RhtNode' yorkie/src/main/kotlin/dev/yorkie/api/ElementConverter.ktLength of output: 479
What this PR does / why we need it?
Any background context you want to provide?
What are the relevant tickets?
Fixes #
Checklist
Summary by CodeRabbit
New Features
is_removed
to enhance node attribute management.Refactor
Rht
class with a new method for setting key-value pairs.Tests