Copilot commented on code in PR #7813:
URL: https://github.com/apache/shenyu/pull/7813#discussion_r4306643891
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/init/SpringMvcClientEventListener.java:
##########
@@ -238,71 +238,113 @@ protected void handleMethod(final Object bean, final
Class<?> clazz,
// the result of ReflectionUtils#getUniqueDeclaredMethods contains
method such as hashCode, wait, toSting
// add Objects.nonNull(requestMapping) to make sure not register wrong
method
if (Objects.nonNull(methodShenyuClient) &&
Objects.nonNull(requestMapping)) {
- List<String> namespaceIds = super.getNamespace();
- for (String namespaceId : namespaceIds) {
- final String apiPath = hasMethodAnnotation
- ? buildApiPath(method, superPath, methodShenyuClient)
- : buildApiPathFromRequestMapping(method, superPath);
- final MetaDataRegisterDTO metaData = buildMetaDataDTO(bean,
methodShenyuClient,
- apiPath, clazz, method, namespaceId);
- getPublisher().publishEvent(metaData);
- getMetaDataMap().put(method, metaData);
+ // register every method-level alias (native and ShenYu) under the
current class super path
+ final List<String> apiPaths = (hasMethodAnnotation
+ ? buildApiPaths(method, superPath, methodShenyuClient)
+ : buildApiPathsFromRequestMapping(method, superPath))
+ .stream().distinct().collect(Collectors.toList());
+ for (String namespaceId : super.getNamespace()) {
+ for (String apiPath : apiPaths) {
+ final MetaDataRegisterDTO metaData =
buildMetaDataDTO(bean, methodShenyuClient,
+ apiPath, clazz, method, namespaceId);
+ getPublisher().publishEvent(metaData);
+ getMetaDataMap().put(method, metaData);
+ }
}
}
}
@Override
protected String buildApiPath(final Method method, final String superPath,
@NonNull final ShenyuSpringMvcClient
methodShenyuClient) {
- String contextPath = getContextPath();
- final String[] annotationPaths = methodShenyuClient.path();
- final String annotationPath = ArrayUtils.isNotEmpty(annotationPaths) ?
annotationPaths[0] : "";
- if (StringUtils.isNotBlank(annotationPath)) {
- return pathJoin(contextPath, superPath, annotationPath);
+ return buildApiPaths(method, superPath, methodShenyuClient).get(0);
+ }
+
+ /**
+ * Builds every effective api path alias for the method under the given
super path, honouring an
+ * explicit {@link ShenyuSpringMvcClient} override first and falling back
to the native mapping array.
+ *
+ * @param method the method
+ * @param superPath the class-level super path
+ * @param methodShenyuClient the effective ShenYu annotation
+ * @return all effective api paths, never empty
+ */
+ protected List<String> buildApiPaths(final Method method, final String
superPath,
+ @NonNull final ShenyuSpringMvcClient
methodShenyuClient) {
+ final String contextPath = getContextPath();
+ final List<String> annotationPaths =
distinctNonBlank(methodShenyuClient.path());
+ if (!annotationPaths.isEmpty()) {
+ return joinAll(contextPath, superPath, annotationPaths);
}
- final String path = getPathByMethod(method);
- if (StringUtils.isNotBlank(path)) {
- return pathJoin(contextPath, superPath, path);
+ final List<String> nativePaths = getPathListByMethod(method);
+ if (!nativePaths.isEmpty()) {
+ return joinAll(contextPath, superPath, nativePaths);
}
- return pathJoin(contextPath, superPath);
+ return Collections.singletonList(pathJoin(contextPath, superPath));
}
String buildApiPathFromRequestMapping(final Method method, final String
superPath) {
- String contextPath = getContextPath();
- final String path = getPathByMethod(method);
- if (StringUtils.isNotBlank(path)) {
- return pathJoin(contextPath, superPath, path);
+ return buildApiPathsFromRequestMapping(method, superPath).get(0);
+ }
+
+ /**
+ * Builds every effective api path alias for the method from its native
mapping array under the given super path.
+ *
+ * @param method the method
+ * @param superPath the class-level super path
+ * @return all effective api paths, never empty
+ */
+ protected List<String> buildApiPathsFromRequestMapping(final Method
method, final String superPath) {
+ final String contextPath = getContextPath();
+ final List<String> nativePaths = getPathListByMethod(method);
+ if (!nativePaths.isEmpty()) {
+ return joinAll(contextPath, superPath, nativePaths);
}
- return pathJoin(contextPath, superPath);
+ return Collections.singletonList(pathJoin(contextPath, superPath));
+ }
+
+ private List<String> joinAll(final String contextPath, final String
superPath, final List<String> paths) {
+ return paths.stream()
+ .map(path -> pathJoin(contextPath, superPath, path))
+ .collect(Collectors.toList());
}
private String formatPath(final String path) {
return path.replaceAll("/+", "/").replaceFirst("/$", "");
}
- private String getPathByMethod(@NonNull final Method method) {
+ private List<String> getPathListByMethod(@NonNull final Method method) {
for (Class<? extends Annotation> mapping : mappingAnnotation) {
- final String pathByAnnotation =
getPathByAnnotation(AnnotatedElementUtils.findMergedAnnotation(method,
mapping));
- if (StringUtils.isNotBlank(pathByAnnotation)) {
- return pathByAnnotation;
+ final List<String> paths =
getPathListByAnnotation(AnnotatedElementUtils.findMergedAnnotation(method,
mapping));
+ if (!paths.isEmpty()) {
+ return paths;
}
}
- return null;
+ return Collections.emptyList();
}
- private String getPathByAnnotation(@Nullable final Annotation annotation) {
+ private List<String> getPathListByAnnotation(@Nullable final Annotation
annotation) {
if (Objects.isNull(annotation)) {
- return null;
+ return Collections.emptyList();
}
final Object value = AnnotationUtils.getValue(annotation, "value");
if (value instanceof String && StringUtils.isNotBlank((String) value))
{
- return (String) value;
+ return Collections.singletonList((String) value);
+ }
+ if (value instanceof String[]) {
+ return distinctNonBlank((String[]) value);
Review Comment:
For @GetMapping({"", "/detail"}) under /orders, removing the empty entry
leaves only /orders/detail and drops the previously registered /orders
endpoint. Preserve empty entries when the merged annotation is RequestMapping,
while retaining distinctNonBlank for ShenYu annotations. Also update joinAll to
use pathJoin(contextPath, superPath) for an empty method alias; passing a third
empty segment adds a trailing slash and does not preserve the previous path.
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/register/SpringMvcApiMetaRegister.java:
##########
@@ -122,47 +151,110 @@ protected List<MetaDataRegisterDTO> parse(final
ApiBean.ApiDefinition apiDefinit
ShenyuSpringMvcClient methodAnnotation =
apiDefinition.getAnnotation(ShenyuSpringMvcClient.class);
- String methodPath = Objects.isNull(methodAnnotation) ?
StringUtils.EMPTY : methodAnnotation.path()[0];
-
- if (StringUtils.isEmpty(methodPath)) {
- methodPath = apiDefinition.getMethodPath();
- }
-
ShenyuSpringMvcClient classAnnotation = apiDefinition.getApiBean()
.getAnnotation(ShenyuSpringMvcClient.class);
- String beanPath = Objects.isNull(classAnnotation) ||
StringUtils.isBlank(classAnnotation.path()[0])
- ? apiDefinition.getBeanPath() : classAnnotation.path()[0];
-
- String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ List<String> beanPaths = resolveBeanPaths(apiDefinition,
classAnnotation);
+ List<String> methodPaths = resolveMethodPaths(apiDefinition,
methodAnnotation);
String pathDesc = Objects.isNull(methodAnnotation) ?
classAnnotation.desc() : methodAnnotation.desc();
boolean enabled = (Objects.isNull(classAnnotation) ||
classAnnotation.enabled())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.enabled());
- String ruleName = Objects.isNull(methodAnnotation) ||
StringUtils.isEmpty(methodAnnotation.ruleName())
- ? apiPath : methodAnnotation.ruleName();
+ String explicitRuleName = Objects.isNull(methodAnnotation) ? null :
methodAnnotation.ruleName();
boolean registerMetaData = (Objects.isNull(classAnnotation) ||
classAnnotation.registerMetaData())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.registerMetaData());
- return clientRegisterConfig.getNamespace().stream().map(namespaceId ->
MetaDataRegisterDTO.builder()
- .namespaceId(namespaceId)
- .contextPath(clientRegisterConfig.getContextPath())
- .addPrefixed(addPrefixed)
- .appName(appName)
- .host(host)
- .port(port)
- .serviceName(apiDefinition.getBeanClass().getName())
- .methodName(apiDefinition.getApiMethodName())
- .path(apiPath)
- .pathDesc(pathDesc)
- .parameterTypes(apiDefinition.getParameterTypes())
- .rpcType(RpcTypeEnum.HTTP.getName())
- .enabled(enabled)
- .ruleName(ruleName)
- .registerMetaData(registerMetaData)
- .build()).collect(Collectors.toList());
+ List<MetaDataRegisterDTO> result = new ArrayList<>();
+ for (String namespaceId : clientRegisterConfig.getNamespace()) {
+ Set<String> seen = new LinkedHashSet<>();
+ for (String beanPath : beanPaths) {
+ for (String methodPath : methodPaths) {
+ String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ if (!seen.add(apiPath)) {
+ continue;
+ }
+ String ruleName = StringUtils.isEmpty(explicitRuleName) ?
apiPath : explicitRuleName;
Review Comment:
With an explicit ruleName, every expanded alias receives the same rule
identity. Admin's RuleServiceImpl.registerDefault (lines 154–156) skips an
existing selector/name pair, so aliases under that selector get metadata but
only the first gets a routing rule. This affects method aliases here, wildcard
aliases through buildPreMetaData, and the new method-alias loop in
SpringMvcClientEventListener. Preserve the primary rule name and give
additional aliases distinct identities, or update registration to combine their
URI conditions into one rule that matches any alias.
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/register/SpringMvcApiMetaRegister.java:
##########
@@ -122,47 +151,110 @@ protected List<MetaDataRegisterDTO> parse(final
ApiBean.ApiDefinition apiDefinit
ShenyuSpringMvcClient methodAnnotation =
apiDefinition.getAnnotation(ShenyuSpringMvcClient.class);
- String methodPath = Objects.isNull(methodAnnotation) ?
StringUtils.EMPTY : methodAnnotation.path()[0];
-
- if (StringUtils.isEmpty(methodPath)) {
- methodPath = apiDefinition.getMethodPath();
- }
-
ShenyuSpringMvcClient classAnnotation = apiDefinition.getApiBean()
.getAnnotation(ShenyuSpringMvcClient.class);
- String beanPath = Objects.isNull(classAnnotation) ||
StringUtils.isBlank(classAnnotation.path()[0])
- ? apiDefinition.getBeanPath() : classAnnotation.path()[0];
-
- String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ List<String> beanPaths = resolveBeanPaths(apiDefinition,
classAnnotation);
+ List<String> methodPaths = resolveMethodPaths(apiDefinition,
methodAnnotation);
String pathDesc = Objects.isNull(methodAnnotation) ?
classAnnotation.desc() : methodAnnotation.desc();
boolean enabled = (Objects.isNull(classAnnotation) ||
classAnnotation.enabled())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.enabled());
- String ruleName = Objects.isNull(methodAnnotation) ||
StringUtils.isEmpty(methodAnnotation.ruleName())
- ? apiPath : methodAnnotation.ruleName();
+ String explicitRuleName = Objects.isNull(methodAnnotation) ? null :
methodAnnotation.ruleName();
boolean registerMetaData = (Objects.isNull(classAnnotation) ||
classAnnotation.registerMetaData())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.registerMetaData());
- return clientRegisterConfig.getNamespace().stream().map(namespaceId ->
MetaDataRegisterDTO.builder()
- .namespaceId(namespaceId)
- .contextPath(clientRegisterConfig.getContextPath())
- .addPrefixed(addPrefixed)
- .appName(appName)
- .host(host)
- .port(port)
- .serviceName(apiDefinition.getBeanClass().getName())
- .methodName(apiDefinition.getApiMethodName())
- .path(apiPath)
- .pathDesc(pathDesc)
- .parameterTypes(apiDefinition.getParameterTypes())
- .rpcType(RpcTypeEnum.HTTP.getName())
- .enabled(enabled)
- .ruleName(ruleName)
- .registerMetaData(registerMetaData)
- .build()).collect(Collectors.toList());
+ List<MetaDataRegisterDTO> result = new ArrayList<>();
+ for (String namespaceId : clientRegisterConfig.getNamespace()) {
+ Set<String> seen = new LinkedHashSet<>();
+ for (String beanPath : beanPaths) {
+ for (String methodPath : methodPaths) {
+ String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ if (!seen.add(apiPath)) {
+ continue;
+ }
+ String ruleName = StringUtils.isEmpty(explicitRuleName) ?
apiPath : explicitRuleName;
+ result.add(MetaDataRegisterDTO.builder()
+ .namespaceId(namespaceId)
+ .contextPath(clientRegisterConfig.getContextPath())
+ .addPrefixed(addPrefixed)
+ .appName(appName)
+ .host(host)
+ .port(port)
+
.serviceName(apiDefinition.getBeanClass().getName())
+ .methodName(apiDefinition.getApiMethodName())
+ .path(apiPath)
+ .pathDesc(pathDesc)
+ .parameterTypes(apiDefinition.getParameterTypes())
+ .rpcType(RpcTypeEnum.HTTP.getName())
+ .enabled(enabled)
+ .ruleName(ruleName)
+ .registerMetaData(registerMetaData)
+ .build());
+ }
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Resolves every effective class-level path alias, honouring an explicit
{@link ShenyuSpringMvcClient}
+ * override first and falling back to the native {@link RequestMapping}
array so that no alias is truncated.
+ */
+ private List<String> resolveBeanPaths(final ApiBean.ApiDefinition
apiDefinition,
+ final ShenyuSpringMvcClient
classAnnotation) {
+ if (Objects.nonNull(classAnnotation)) {
+ List<String> paths = distinctNonBlank(classAnnotation.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ RequestMapping requestMapping =
apiDefinition.getApiBean().getAnnotation(RequestMapping.class);
+ if (Objects.nonNull(requestMapping)) {
+ List<String> paths = distinctNonBlank(requestMapping.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ return
Collections.singletonList(StringUtils.defaultString(apiDefinition.getBeanPath()));
+ }
+
+ /**
+ * Resolves every effective method-level path alias, honouring an explicit
{@link ShenyuSpringMvcClient}
+ * override first and falling back to the native {@link RequestMapping}
array so that no alias is truncated.
+ */
+ private List<String> resolveMethodPaths(final ApiBean.ApiDefinition
apiDefinition,
+ final ShenyuSpringMvcClient
methodAnnotation) {
+ if (Objects.nonNull(methodAnnotation)) {
+ List<String> paths = distinctNonBlank(methodAnnotation.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ RequestMapping requestMapping =
apiDefinition.getAnnotation(RequestMapping.class);
+ if (Objects.nonNull(requestMapping)) {
+ List<String> paths = distinctNonBlank(requestMapping.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ return
Collections.singletonList(StringUtils.defaultString(apiDefinition.getMethodPath()));
+ }
+
+ private boolean isWildcard(final ShenyuSpringMvcClient annotation) {
+ return ArrayUtils.isNotEmpty(annotation.path()) &&
annotation.path()[0].endsWith("/**");
Review Comment:
The first alias determines wildcard handling for the entire bean. With class
paths {"/a/**", "/b"} and method /foo, preMatch short-circuits method
registration and emits /b instead of /b/foo. Reversing the aliases instead
produces /a/**/foo. Classify aliases individually: register wildcard aliases
directly and expand ordinary aliases through the methods, as
SpringMvcClientEventListener.handle already does. Changing this predicate to
anyMatch alone would still skip ordinary aliases' methods.
##########
shenyu-client/shenyu-client-http/shenyu-client-springmvc/src/main/java/org/apache/shenyu/client/springmvc/register/SpringMvcApiMetaRegister.java:
##########
@@ -122,47 +151,110 @@ protected List<MetaDataRegisterDTO> parse(final
ApiBean.ApiDefinition apiDefinit
ShenyuSpringMvcClient methodAnnotation =
apiDefinition.getAnnotation(ShenyuSpringMvcClient.class);
- String methodPath = Objects.isNull(methodAnnotation) ?
StringUtils.EMPTY : methodAnnotation.path()[0];
-
- if (StringUtils.isEmpty(methodPath)) {
- methodPath = apiDefinition.getMethodPath();
- }
-
ShenyuSpringMvcClient classAnnotation = apiDefinition.getApiBean()
.getAnnotation(ShenyuSpringMvcClient.class);
- String beanPath = Objects.isNull(classAnnotation) ||
StringUtils.isBlank(classAnnotation.path()[0])
- ? apiDefinition.getBeanPath() : classAnnotation.path()[0];
-
- String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ List<String> beanPaths = resolveBeanPaths(apiDefinition,
classAnnotation);
+ List<String> methodPaths = resolveMethodPaths(apiDefinition,
methodAnnotation);
String pathDesc = Objects.isNull(methodAnnotation) ?
classAnnotation.desc() : methodAnnotation.desc();
boolean enabled = (Objects.isNull(classAnnotation) ||
classAnnotation.enabled())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.enabled());
- String ruleName = Objects.isNull(methodAnnotation) ||
StringUtils.isEmpty(methodAnnotation.ruleName())
- ? apiPath : methodAnnotation.ruleName();
+ String explicitRuleName = Objects.isNull(methodAnnotation) ? null :
methodAnnotation.ruleName();
boolean registerMetaData = (Objects.isNull(classAnnotation) ||
classAnnotation.registerMetaData())
&& (Objects.isNull(methodAnnotation) ||
methodAnnotation.registerMetaData());
- return clientRegisterConfig.getNamespace().stream().map(namespaceId ->
MetaDataRegisterDTO.builder()
- .namespaceId(namespaceId)
- .contextPath(clientRegisterConfig.getContextPath())
- .addPrefixed(addPrefixed)
- .appName(appName)
- .host(host)
- .port(port)
- .serviceName(apiDefinition.getBeanClass().getName())
- .methodName(apiDefinition.getApiMethodName())
- .path(apiPath)
- .pathDesc(pathDesc)
- .parameterTypes(apiDefinition.getParameterTypes())
- .rpcType(RpcTypeEnum.HTTP.getName())
- .enabled(enabled)
- .ruleName(ruleName)
- .registerMetaData(registerMetaData)
- .build()).collect(Collectors.toList());
+ List<MetaDataRegisterDTO> result = new ArrayList<>();
+ for (String namespaceId : clientRegisterConfig.getNamespace()) {
+ Set<String> seen = new LinkedHashSet<>();
+ for (String beanPath : beanPaths) {
+ for (String methodPath : methodPaths) {
+ String apiPath =
PathUtils.pathJoin(clientRegisterConfig.getContextPath(), beanPath, methodPath);
+ if (!seen.add(apiPath)) {
+ continue;
+ }
+ String ruleName = StringUtils.isEmpty(explicitRuleName) ?
apiPath : explicitRuleName;
+ result.add(MetaDataRegisterDTO.builder()
+ .namespaceId(namespaceId)
+ .contextPath(clientRegisterConfig.getContextPath())
+ .addPrefixed(addPrefixed)
+ .appName(appName)
+ .host(host)
+ .port(port)
+
.serviceName(apiDefinition.getBeanClass().getName())
+ .methodName(apiDefinition.getApiMethodName())
+ .path(apiPath)
+ .pathDesc(pathDesc)
+ .parameterTypes(apiDefinition.getParameterTypes())
+ .rpcType(RpcTypeEnum.HTTP.getName())
+ .enabled(enabled)
+ .ruleName(ruleName)
+ .registerMetaData(registerMetaData)
+ .build());
+ }
+ }
+ }
+ return result;
+ }
+
+ /**
+ * Resolves every effective class-level path alias, honouring an explicit
{@link ShenyuSpringMvcClient}
+ * override first and falling back to the native {@link RequestMapping}
array so that no alias is truncated.
+ */
+ private List<String> resolveBeanPaths(final ApiBean.ApiDefinition
apiDefinition,
+ final ShenyuSpringMvcClient
classAnnotation) {
+ if (Objects.nonNull(classAnnotation)) {
+ List<String> paths = distinctNonBlank(classAnnotation.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ RequestMapping requestMapping =
apiDefinition.getApiBean().getAnnotation(RequestMapping.class);
+ if (Objects.nonNull(requestMapping)) {
+ List<String> paths = distinctNonBlank(requestMapping.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ return
Collections.singletonList(StringUtils.defaultString(apiDefinition.getBeanPath()));
+ }
+
+ /**
+ * Resolves every effective method-level path alias, honouring an explicit
{@link ShenyuSpringMvcClient}
+ * override first and falling back to the native {@link RequestMapping}
array so that no alias is truncated.
+ */
+ private List<String> resolveMethodPaths(final ApiBean.ApiDefinition
apiDefinition,
+ final ShenyuSpringMvcClient
methodAnnotation) {
+ if (Objects.nonNull(methodAnnotation)) {
+ List<String> paths = distinctNonBlank(methodAnnotation.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ RequestMapping requestMapping =
apiDefinition.getAnnotation(RequestMapping.class);
+ if (Objects.nonNull(requestMapping)) {
+ List<String> paths = distinctNonBlank(requestMapping.path());
+ if (!paths.isEmpty()) {
+ return paths;
+ }
+ }
+ return
Collections.singletonList(StringUtils.defaultString(apiDefinition.getMethodPath()));
+ }
+
+ private boolean isWildcard(final ShenyuSpringMvcClient annotation) {
+ return ArrayUtils.isNotEmpty(annotation.path()) &&
annotation.path()[0].endsWith("/**");
+ }
+
+ private List<String> distinctNonBlank(final String[] paths) {
+ if (ArrayUtils.isEmpty(paths)) {
+ return Collections.emptyList();
+ }
+ return Arrays.stream(paths)
+ .filter(StringUtils::isNotBlank)
Review Comment:
Empty strings are valid native root aliases. For a class with
@RequestMapping({"", "/v1"}) and a /foo method, this filter registers only
/app/v1/foo and drops the previously registered /app/foo. In both native-path
resolvers, use
Arrays.stream(requestMapping.path()).distinct().collect(Collectors.toList())
instead of distinctNonBlank. Keep the default-empty ShenYu annotation's
fallback behavior separate.
--
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]
