Repository navigation
refactor: centralize Camel URI syntax analysis - #3576
Conversation
📝 WalkthroughWalkthroughCamelUriHelper centralizes URI syntax analysis in ChangesCamel URI syntax handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/utils/camel-uri-helper.test.ts (1)
452-507: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression case for overlapping delimiters.
Add a syntax such as
foo:path/segment//tailand assert that the delimiter regex matches//before/, with keys['path', 'segment', '', 'tail']. This test would prevent the parser regression described inanalyzeSyntax.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/utils/camel-uri-helper.test.ts` around lines 452 - 507, Add an analyzeSyntax regression case for foo:path/segment//tail, asserting delimiters recognize the overlapping // token before the single / token and produce keys ['path', 'segment', '', 'tail']; keep the test alongside the existing canonical syntax cases in the analyzeSyntax suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/utils/camel-uri-helper.ts`:
- Around line 218-222: Normalize delimiter matches in the split-regex
construction near URI separator parsing by escaping each delimiter and ordering
them longest-first before joining them, so overlapping separators such as "/"
and "//" match the longer token first. Add mixed-delimiter coverage in
packages/ui/src/utils/camel-uri-helper.test.ts within the existing delimiter
parsing tests, verifying both source and test behavior for syntaxes like
example:x/y//z:w:a.
---
Nitpick comments:
In `@packages/ui/src/utils/camel-uri-helper.test.ts`:
- Around line 452-507: Add an analyzeSyntax regression case for
foo:path/segment//tail, asserting delimiters recognize the overlapping // token
before the single / token and produce keys ['path', 'segment', '', 'tail']; keep
the test alongside the existing canonical syntax cases in the analyzeSyntax
suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9ab7560-b1a3-4aff-99a6-5114e5315eca
📒 Files selected for processing (2)
packages/ui/src/utils/camel-uri-helper.test.tspackages/ui/src/utils/camel-uri-helper.ts
|
Hello! Thanks for checking this issue. Could you also fix the sonarqube issue, please? |
|
No problem - I'll take care of the SonarQube issue. Thanks for pointing it out! |
c1566fe to
7c55439
Compare
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/utils/camel-uri-helper.ts (1)
227-228: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the generated delimiter regex.
Line 227 preserves every separator occurrence, so templates with repeated separators create a regex with repeated alternatives whose size grows with the syntax length. Prefer a bounded matcher or deduplicated delimiter set while preserving the existing discovery order and behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/utils/camel-uri-helper.ts` around lines 227 - 228, Deduplicate the delimiter alternatives before constructing delimiterRegex so repeated separators in syntax cannot make the generated matcher grow with every occurrence. Preserve the first-seen discovery order and existing syntax.split behavior, including the null-delimiter path.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ui/src/utils/camel-uri-helper.ts`:
- Around line 227-228: Deduplicate the delimiter alternatives before
constructing delimiterRegex so repeated separators in syntax cannot make the
generated matcher grow with every occurrence. Preserve the first-seen discovery
order and existing syntax.split behavior, including the null-delimiter path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e8cf11c7-ec38-43f8-96ad-85249c902cf4
📒 Files selected for processing (2)
packages/ui/src/utils/camel-uri-helper.test.tspackages/ui/src/utils/camel-uri-helper.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/utils/camel-uri-helper.test.ts
PVinaches
left a comment
PVinaches
left a comment
There was a problem hiding this comment.
LGTM. Thanks for working on this issue!
Description
Centralize Camel URI syntax-template analysis in one private
analyzeSyntaxhelper. Path serialization and parsing now obtain their syntax-template metadata from the same helper, while keeping runtime URI handling separate.The maintainer-requested follow-up replaces the global-regex
String.match()call with iterativeRegExp.exec()matching to address Sonar S6594 without dropping repeated or mixed URI delimiters.Closes #3558
Type of Change
How Has This Been Tested?
yarn workspace @kaoto/kaoto test src/utils/camel-uri-helper.test.ts(103 passed)yarn workspace @kaoto/kaoto test --maxWorkers=8(480 test files passed, 1 skipped; 6947 tests passed, 5 skipped, 2 todo)yarn workspace @kaoto/kaoto lintyarn workspace @kaoto/kaoto lint:styleyarn workspace @kaoto/kaoto buildChecklist
AI assistance
AI assistance: I used an AI coding assistant to help implement and review this refactor. I reviewed the changes and test results and take responsibility for the contribution.
Summary by CodeRabbit
Bug Fixes
Tests