wy471x commented on PR #6348: URL: https://github.com/apache/shenyu/pull/6348#issuecomment-4971594120
> Thanks for the PR. The main route registration path looks like the right direction, but I think we should add one regression test before merging. > > The new logic expands class-level paths in `SpringMvcClientEventListener#handle()` and also changes API doc generation to iterate `buildApiSuperPaths()`. However, the current unit tests only assert `buildApiSuperPaths()` and `buildApiDocSextet()`, while the integration test verifies gateway routing only. > > Please add a publisher-level/unit test that captures the emitted `MetaDataRegisterDTO` and `ApiDocRegisterDTO` for a controller with multiple class-level paths, and assert the exact count and paths, for example both: > > * `/mvc/multipath/v1/greet` > * `/mvc/multipath/v2/greet` > > This is important because the registration contract is now "one Java method can produce multiple registered paths", while `metaDataMap` is still keyed by `Method` and API doc generation depends on that registration flow. Even if `ApiExt` is path-independent today, this behavior should be locked by tests. > > Also, please consider avoiding the suffix-based fallback heuristic in `buildApiPath()` (`superPath.endsWith(formatPath(p))`) or at least cover it with tests. It is hard to reason about method-vs-class annotation provenance from string shape alone. I have made the corresponding fixes and added relevant test cases for coverage. Please review them, thank you. -- 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]
