Skip to content

fix: fix api path tree building #660

Open
fuyuwei01 wants to merge 4 commits intopolarismesh:mainfrom
fuyuwei01:fix-path-building
Open

fix: fix api path tree building #660
fuyuwei01 wants to merge 4 commits intopolarismesh:mainfrom
fuyuwei01:fix-path-building

Conversation

@fuyuwei01
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.85%. Comparing base (9b8cfec) to head (d628c92).

Files with missing lines Patch % Lines
...n/java/com/tencent/polaris/api/utils/TrieUtil.java 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #660      +/-   ##
============================================
+ Coverage     19.79%   19.85%   +0.05%     
- Complexity      946      950       +4     
============================================
  Files           375      375              
  Lines         15598    15607       +9     
  Branches       1998     2001       +3     
============================================
+ Hits           3088     3099      +11     
  Misses        12125    12125              
+ Partials        385      383       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

// 跳过第一个为空的str
TrieNode<String> node = root;
// 一些场景下(例如dubbo的接口名是com.tencent.polaris.serviceName),apiPath 不以"/"开头和分割,则直接构造node
if (apiPaths.length == 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果path是/,那么apiPaths是什么样的?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apiPaths.length为0,仅返回根节点

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里会有个情况是,/path会和path生成同样的树结构。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要补充多点单测用例,来保证正确。

// 跳过第一个为空的str
TrieNode<String> node = root;
// 一些场景下(例如dubbo的接口名是com.tencent.polaris.serviceName),apiPath 不以"/"开头和分割,则直接构造node
if (apiPaths.length == 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要补充多点单测用例,来保证正确。

@SkyeBeFreeman
Copy link
Member

Code Review

严重 Bug

1. checkSimpleApi 中使用了 getOrCreateSubNode 而非 getSubNode

if (apiPathInfo.contains("#")) {
    node = node.getOrCreateSubNode(apiPathInfo);  // 错误,应该是 getSubNode

查找操作不应创建节点。getOrCreateSubNode 永远不返回 null,导致对不存在路径的查找也会继续执行 checkApiNodeInfo。更严重的是,在并发场景下 checkSimpleApi 对 Trie 进行了写操作,属于非预期副作用。目前凑巧没有误报 true(因为新节点的 nodeInfo 为 null,两个分支均为 false),但逻辑是错误的。

2. checkSimpleApi 使用 apiPathInfo 而 build 使用 path

// build 中
if (path.contains("#")) {
    node = node.getOrCreateSubNode(path);  // 用 path(已去除 -METHOD 后缀)

// check 中
if (apiPathInfo.contains("#")) {
    node = node.getOrCreateSubNode(apiPathInfo);  // 用原始字符串

当 gRPC 路径含有 - 时(如 com.Service#method-foo),build 存的是去后缀的 path,check 查的是原始 apiPathInfo,两者不一致导致匹配失败。


设计问题

3. gRPC 路径仍然走 HTTP method 解析流程

gRPC 路径不含 -METHOD 后缀,但每次都要先执行 lastIndexOf("-") 的解析逻辑,再才进入 # 判断分支。建议将路径类型判断提到最前面,分两个独立分支处理,逻辑更清晰。


代码质量

4. Checkstyle 格式问题(CI 会报错)

if(apiPathInfo.contains("#")){  // if 后缺空格,{ 前缺空格

5. 多余的连续空行

删除 String[] apiPaths = path.split("/"); 后留下了两个连续空行,违反项目"最多 1 个空行"规范。


测试遗漏

6. 缺少在空 Trie / 不含该路径的 Trie 中查找 gRPC 路径的测试

所有测试中 root 都是基于对应路径 build 出来的。因为 bug #1getOrCreateSubNode),checkSimpleApi(emptyRoot, "com.Service#method") 实际上会在 Trie 中创建节点,该 bug 因此未被测试暴露。

7. 缺少混合 Trie 的测试

没有"同一个 Trie 同时包含 HTTP 路径和 gRPC 路径"的场景测试。


总结

严重程度 问题
checkSimpleApigetOrCreateSubNode 而非 getSubNode,check 操作修改了 Trie 结构
build 用 path,check 用 apiPathInfo,含 - 的 gRPC 路径会匹配失败
gRPC 路径未提前分流,仍走无效的 HTTP method 解析
Checkstyle 格式问题(if( 缺空格,连续空行)
缺少空 Trie + gRPC 路径查找、混合 Trie 的测试用例

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants