chore: return false when peerId is blank #1113
Conversation
sofastack-cla
bot
added
question
WalkthroughThe primary change involves enhancing the Changes
Sequence Diagram(s)No sequence diagrams are necessary for these changes as they pertain primarily to input validation and related tests. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- jraft-core/src/main/java/com/alipay/sofa/jraft/entity/PeerId.java (1 hunks)
Additional comments not posted (1)
jraft-core/src/main/java/com/alipay/sofa/jraft/entity/PeerId.java (1)
195-195: Enhanced Validation in PeerId.parse MethodThe addition of
StringUtils.isBlank(s)alongsideStringUtils.isEmpty(s)enhances the input validation by ensuring that strings that are only whitespace are also considered invalid. This change increases the robustness of the method and prevents potential errors in scenarios where the input string is expected to contain meaningful data but does not. Good job on improving the input validation logic.
|
cc @fengjiachun @killme2008 PATL, thanks |
|
Hi, could you add a unit test for this? @caicancai |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java (1 hunks)
Files skipped from review due to trivial changes (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java
done |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- jraft-core/src/test/java/com/alipay/sofa/jraft/entity/PeerIdTest.java
caicancai
changed the title
|
@caicancai Thank you! |
Motivation:
When peerId satisfies StringUtils.isBlank(s) == true, no further judgment is required
Modification:
Describe the idea and modifications you've done.
Result:
Fixes #.
If there is no issue then describe the changes introduced by this PR.
Summary by CodeRabbit
Bug Fixes
PeerIdparsing to handle empty, blank, or whitespace strings correctly.Tests
PeerIdparsing of null, empty, and whitespace strings.