RockteMQ-AI commented on PR #535:
URL: https://github.com/apache/rocketmq-connect/pull/535#issuecomment-5561319053
Thanks for this contribution! The feature looks useful for dynamic URL
routing.
**However, there's a bug in the placeholder detection logic:**
```java
if (!PATTERN.matcher(url).matches()) {
return url;
}
```
`String.matches()` requires the **entire string** to match the regex
pattern. For a URL like `http://example.com/{id}/path`, this will return
`false` because the entire URL doesn't match `\{(\w+)\}`. As a result,
placeholders in real-world URLs will never be replaced.
**Fix:** Use `find()` instead of `matches()`:
```java
if (!PATTERN.matcher(url).find()) {
return url;
}
```
`find()` scans the string for any occurrence of the pattern, which is the
intended behavior here.
**Suggested test cases:**
- `http://example.com/{id}` → should replace `{id}`
- `http://example.com/{id}/details` → should replace `{id}` (currently
broken with `matches()`)
- `http://example.com/path` → no placeholders, return as-is
Once this is fixed, the change looks good to merge.
---
*Automated review by github-manager-bot*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
