Copilot commented on code in PR #6348:
URL: https://github.com/apache/shenyu/pull/6348#discussion_r3370618887
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/init/SpringMvcClientEventListener.java:
##########
@@ -212,8 +250,13 @@ protected void handleMethod(final Object bean, final
Class<?> clazz,
protected String buildApiPath(final Method method, final String superPath,
@NonNull final ShenyuSpringMvcClient
methodShenyuClient) {
String contextPath = getContextPath();
- if (StringUtils.isNotBlank(methodShenyuClient.path()[0])) {
- return pathJoin(contextPath, superPath,
methodShenyuClient.path()[0]);
+ // Skip if any annotation path is already captured in superPath (class
annotation used as method fallback)
+ final String annotationPath = methodShenyuClient.path()[0];
+ boolean alreadyInSuperPath = Arrays.stream(methodShenyuClient.path())
+ .filter(StringUtils::isNotBlank)
+ .anyMatch(p -> superPath.endsWith(formatPath(p)));
Review Comment:
`methodShenyuClient.path()[0]` will throw `ArrayIndexOutOfBoundsException`
if a user configures `@ShenyuSpringMvcClient(path = {})` (an empty array is
valid Java annotation syntax). Guard against empty arrays before accessing
index 0.
##########
shenyu-integrated-test/shenyu-integrated-test-http/src/test/java/org/apache/shenyu/integrated/test/http/SpringMvcMappingPathControllerTest.java:
##########
@@ -17,30 +17,75 @@
package org.apache.shenyu.integrated.test.http;
-import org.junit.jupiter.api.Test;
-import org.apache.shenyu.integratedtest.common.helper.HttpHelper;
import org.apache.shenyu.integratedtest.common.AbstractTest;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.apache.shenyu.integratedtest.common.helper.HttpHelper;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
import java.io.IOException;
+import java.util.concurrent.TimeUnit;
-public class SpringMvcMappingPathControllerTest extends AbstractTest {
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class SpringMvcMappingPathControllerTest extends AbstractTest {
+
+ private static final String MULTI_PATH_SUFFIX = "I'm Shenyu-Gateway
System. Welcome!";
+
+ @BeforeAll
+ static void waitForMultiPathRoutes() throws InterruptedException {
+ // Multi-path routes are registered asynchronously; poll until
available
+ for (int i = 0; i < 30; i++) {
+ try {
+ String res =
HttpHelper.INSTANCE.postGateway("/http/multipath/v1/greet", String.class);
+ if (("hello from multipath! " +
MULTI_PATH_SUFFIX).equals(res)) {
+ return;
+ }
+ } catch (IOException e) {
+ // route not ready yet, keep waiting
+ }
+ Thread.sleep(TimeUnit.SECONDS.toMillis(2));
+ }
+ }
Review Comment:
`waitForMultiPathRoutes` polls but never fails if the route is still
unavailable after the timeout. This can add ~60s of extra delay before the real
test failures and makes the root cause less clear. After the loop, explicitly
fail fast so the suite stops with a clear error when multi-path registration
never completes.
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/init/SpringMvcClientEventListener.java:
##########
@@ -167,20 +168,57 @@ protected String getClientName() {
return RpcTypeEnum.HTTP.getName();
}
+ @Override
+ protected void handle(final String beanName, final Object bean) {
+ Class<?> clazz = getCorrectedClass(bean);
+ final ShenyuSpringMvcClient beanShenyuClient =
AnnotatedElementUtils.findMergedAnnotation(clazz, getAnnotationType());
+ final List<String> superPaths = buildApiSuperPaths(clazz,
beanShenyuClient);
+ final Method[] methods =
ReflectionUtils.getUniqueDeclaredMethods(clazz);
+ for (String superPath : superPaths) {
+ if (Objects.nonNull(beanShenyuClient) && superPath.contains("*")) {
+ handleClass(clazz, bean, beanShenyuClient, superPath);
+ continue;
+ }
+ for (Method method : methods) {
+ handleMethod(bean, clazz, beanShenyuClient, method, superPath);
+ }
+ }
+ }
+
@Override
protected String buildApiSuperPath(final Class<?> clazz, @Nullable final
ShenyuSpringMvcClient beanShenyuClient) {
- final String servletPath =
StringUtils.defaultString(this.env.getProperty("spring.mvc.servlet.path"), "");
- final String servletContextPath =
StringUtils.defaultString(this.env.getProperty("server.servlet.context-path"),
"");
- final String rootPath = String.format("/%s/%s/", servletContextPath,
servletPath);
- if (Objects.nonNull(beanShenyuClient) &&
StringUtils.isNotBlank(beanShenyuClient.path()[0])) {
- return formatPath(String.format("%s/%s", rootPath,
beanShenyuClient.path()[0]));
+ List<String> paths = buildApiSuperPaths(clazz, beanShenyuClient);
+ return paths.isEmpty() ? formatPath(buildRootPath()) : paths.get(0);
+ }
Review Comment:
Multi-path metadata registration is implemented by overriding `handle(...)`
to iterate `buildApiSuperPaths(...)`, but API doc generation in
`AbstractContextRefreshedEventListener` still calls `buildApiSuperPath(...)`
(singular) when building `ApiDocRegisterDTO` (see
`shenyu-client-core/.../AbstractContextRefreshedEventListener.java:197-214`).
With multi-path controllers like `SpringMvcMultiPathController`, only the first
super-path will be used for generated API docs, so the second prefix will be
missing from admin docs.
--
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]
