Copilot commented on code in PR #7893: URL: https://github.com/apache/incubator-seata/pull/7893#discussion_r2646464886
########## console/src/main/java/org/apache/seata/mcp/service/impl/ModifyConfirmServiceImpl.java: ########## @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.service.impl; + +import io.jsonwebtoken.Jwts; +import io.jsonwebtoken.SignatureAlgorithm; +import io.jsonwebtoken.io.Decoders; +import org.apache.seata.console.utils.JwtTokenUtils; +import org.apache.seata.mcp.service.ModifyConfirmService; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Service; + +import javax.crypto.spec.SecretKeySpec; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; + +@Service +public class ModifyConfirmServiceImpl implements ModifyConfirmService { + + private final JwtTokenUtils jwtTokenUtils; + + private static final long ModifyTokenValidityInMilliseconds = 60_000; Review Comment: The variable name 'ModifyTokenValidityInMilliseconds' does not follow Java naming conventions. Constants should be in UPPER_SNAKE_CASE. Consider renaming to 'MODIFY_TOKEN_VALIDITY_IN_MILLISECONDS'. ########## console/src/main/java/org/apache/seata/mcp/service/impl/ConsoleRemoteServiceImpl.java: ########## @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.service.impl; + +import org.apache.seata.common.NamingServerLocalMarker; +import org.apache.seata.common.exception.AuthenticationFailedException; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.console.config.WebSecurityConfig; +import org.apache.seata.console.utils.JwtTokenUtils; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +@ConditionalOnMissingBean(NamingServerLocalMarker.class) +@Service +public class ConsoleRemoteServiceImpl implements ConsoleApiService { + + private final JwtTokenUtils jwtTokenUtils; + + private final RestTemplate restTemplate; + + public ConsoleRemoteServiceImpl(JwtTokenUtils jwtTokenUtils, RestTemplate restTemplate) { + this.jwtTokenUtils = jwtTokenUtils; + this.restTemplate = restTemplate; + } + + private final String NAMING_SPACE_URL = "http://127.0.0.1:%s"; Review Comment: The hardcoded URL "http://127.0.0.1:%s" limits the service to localhost only. This prevents the console from connecting to remote naming servers. Consider making this URL configurable through application properties to support different deployment scenarios. ```suggestion @Value("${seata.console.naming-space-url:http://127.0.0.1:%s}") private String NAMING_SPACE_URL; ``` ########## console/src/main/java/org/apache/seata/mcp/entity/vo/GlobalSessionVO.java: ########## @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.vo; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import org.apache.seata.mcp.core.config.TimestampToStringDeserializer; + +import java.util.Set; + +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; + +/** + * GlobalSessionVO + */ +@JsonAutoDetect(getterVisibility = NONE, fieldVisibility = ANY) +public class GlobalSessionVO extends org.apache.seata.server.console.entity.vo.GlobalSessionVO { Review Comment: GlobalSessionVO has the same name as its supertype [org.apache.seata.server.console.entity.vo.GlobalSessionVO](1). ########## namingserver/src/main/java/org/apache/seata/namingserver/service/ConsoleLocalServiceImpl.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.namingserver.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.metadata.ClusterRole; +import org.apache.seata.common.metadata.Node; +import org.apache.seata.common.metadata.namingserver.NamingServerNode; +import org.apache.seata.common.util.CollectionUtils; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.impl.ConsoleRemoteServiceImpl; +import org.apache.seata.namingserver.manager.NamingManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.context.annotation.Primary; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ThreadLocalRandom; + +import static org.apache.seata.common.Constants.RAFT_GROUP_HEADER; + +@ConditionalOnBean(ConsoleRemoteServiceImpl.class) +@Primary +@Service +public class ConsoleLocalServiceImpl implements ConsoleApiService { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final NamingManager namingManager; + + private final RestTemplate restTemplate; + + private final ObjectMapper objectMapper; + + public ConsoleLocalServiceImpl(NamingManager namingManager, RestTemplate restTemplate, ObjectMapper objectMapper) { + this.namingManager = namingManager; + this.restTemplate = restTemplate; + this.objectMapper = objectMapper; + } + + public String getResult( + NameSpaceDetail nameSpaceDetail, + HttpMethod httpMethod, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + String namespace = nameSpaceDetail.getNamespace(); + String cluster = nameSpaceDetail.getCluster(); + String vgroup = nameSpaceDetail.getvGroup(); + if (StringUtils.isNotBlank(namespace) && (StringUtils.isNotBlank(cluster) || StringUtils.isNotBlank(vgroup))) { + List<NamingServerNode> list = null; + if (StringUtils.isNotBlank(vgroup)) { + list = namingManager.getInstancesByVgroupAndNamespace( + namespace, vgroup, StringUtils.equalsIgnoreCase(httpMethod.name(), HttpMethod.GET.name())); + } else if (StringUtils.isNotBlank(cluster)) { + list = namingManager.getInstances(namespace, cluster); + } + if (CollectionUtils.isNotEmpty(list)) { + // Randomly select a node from the list + NamingServerNode node = list.get(ThreadLocalRandom.current().nextInt(list.size())); + Node.Endpoint controlEndpoint = node.getControl(); + if (controlEndpoint != null) { + // Construct the target URL + String baseUrl = "http://" + controlEndpoint.getHost() + ":" + controlEndpoint.getPort(); + Map<String, Object> queryParamsMap = objectToQueryParamMap(queryParams); + String targetUrl = buildUrl(baseUrl, path, pathParams, queryParamsMap); + if (node.getRole() == ClusterRole.LEADER) { + headers.add(RAFT_GROUP_HEADER, node.getUnit()); + } + HttpEntity<String> entity = new HttpEntity<>(headers); + String responseBody = null; + try { + ResponseEntity<String> response = + restTemplate.exchange(targetUrl, httpMethod, entity, String.class); + + responseBody = response.getBody(); + + if (!response.getStatusCode().is2xxSuccessful()) { + logger.warn("MCP request returned non-success status: {}", response.getStatusCode()); + } + return responseBody; + } catch (RestClientException e) { + logger.error("MCP {} Call TC Failed: {}", httpMethod.name(), e.getMessage()); + return responseBody; + } + } + } + throw new IllegalArgumentException("Couldn't find target node url"); + } + throw new IllegalArgumentException("Invalid NameSpace Detail"); + } + + @Override + public String getCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.GET, path, queryParams, pathParams, headers); + } + + @Override + public String deleteCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.DELETE, path, queryParams, pathParams, headers); + } + + @Override + public String putCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.PUT, path, queryParams, pathParams, headers); + } + + @Override + public String getCallNameSpace( + String path, Object queryParams, Map<String, String> pathParams, HttpHeaders headers) { + String namespace; + try { + namespace = objectMapper.writeValueAsString(namingManager.namespace()); + } catch (JsonProcessingException e) { + logger.error("Get NameSpace failed: {}", e.getMessage()); + throw new RuntimeException(e); + } + return namespace; + } + + private Map<String, Object> objectToQueryParamMap(Object obj) { + if (obj == null) { + return Collections.emptyMap(); + } + if (obj instanceof Map) { + Map<?, ?> map = (Map<?, ?>) obj; + Map<String, Object> result = new HashMap<>(map.size()); + map.forEach((k, v) -> { + if (k != null && v != null) { + result.put(k.toString(), v); + } + }); + return result; + } + + Map<String, Object> paramMap = new HashMap<>(); + Class<?> clazz = obj.getClass(); + while (clazz != null && clazz != Object.class) { + for (Field field : clazz.getDeclaredFields()) { + try { + field.setAccessible(true); + Object value = field.get(obj); + if (value != null) { + paramMap.putIfAbsent(field.getName(), value); + } + } catch (IllegalAccessException e) { + logger.warn("Failed to access field {}: {}", field.getName(), e.getMessage()); + } + } + clazz = clazz.getSuperclass(); + } + return paramMap; Review Comment: Using reflection to access fields via 'field.setAccessible(true)' can cause security issues and may fail with Java's module system. Consider using proper getter methods or a library like Jackson's ObjectMapper to convert objects to Maps, which respects encapsulation and works with the module system. ########## console/src/main/java/org/apache/seata/mcp/tools/ModifyConfirmTools.java: ########## @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.tools; + +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.service.ModifyConfirmService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springaicommunity.mcp.annotation.McpToolParam; +import org.springframework.stereotype.Service; + +import java.util.Map; + +@Service +public class ModifyConfirmTools { + + private static final Logger LOGGER = LoggerFactory.getLogger(ModifyConfirmTools.class); + + private final ModifyConfirmService modifyConfirmService; + + public ModifyConfirmTools(ModifyConfirmService modifyConfirmService) { + this.modifyConfirmService = modifyConfirmService; + } + + @McpTool( + description = "Before modifying (update or delete) a transaction or lock, the user MUST manually confirm." + + "You are NOT allowed to fabricate or auto-confirm on behalf of the user.") + public Map<String, String> confirmAndGetKey( + @McpToolParam( + description = + "The confirmation string provided by the USER (not generated by the LLM).The content must repeat the modification action clearly.") + String userInputStr) { + if (StringUtils.isBlank(userInputStr)) { + throw new IllegalArgumentException("User confirmation string is required."); + } + if (!userInputStr.contains("确认") && !userInputStr.contains("confirm")) { + throw new IllegalArgumentException( + "Confirmation string must explicitly contain '确认' or 'confirm' and repeat the modification content. This must come from the user."); Review Comment: The validation message contains both Chinese characters ("确认") and English ("confirm"). For better internationalization support, consider extracting these validation messages into resource bundles or configuration, and use a consistent language approach. ########## console/src/main/java/org/apache/seata/mcp/core/utils/DateUtils.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.core.utils; + +import java.time.DateTimeException; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; +import java.util.regex.Pattern; + +public class DateUtils { + + public static final Long ONE_DAY_TIMESTAMP = 86400000L; + + private static final Pattern DATE_PATTERN = Pattern.compile("^\\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$"); + + public static boolean isValidDate(String dateStr) { + return DATE_PATTERN.matcher(dateStr).matches(); + } + + public static long convertToTimestampFromDate(String dateStr) { + if (!isValidDate(dateStr)) { + throw new DateTimeException("The time format does not match yyyy-mm-dd"); + } + LocalDate date = LocalDate.parse(dateStr); + ZonedDateTime zonedDateTime = date.atStartOfDay(ZoneId.systemDefault()); + return zonedDateTime.toInstant().toEpochMilli(); + } + + public static long convertToTimeStampFromDateTime(String dateTimeStr) { + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + + try { + LocalDateTime dateTime = LocalDateTime.parse(dateTimeStr, formatter); + return dateTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli(); + } catch (DateTimeParseException e) { + return -1; Review Comment: The method 'convertToTimeStampFromDateTime' returns -1 when parsing fails, which could be misinterpreted as a valid timestamp (1 millisecond before Unix epoch). Consider throwing an exception or returning an Optional<Long> to clearly indicate parsing failure, as -1 could lead to subtle bugs if not properly checked by callers. ```suggestion throw new DateTimeException("The time format does not match yyyy-MM-dd HH:mm:ss", e); ``` ########## console/src/main/java/org/apache/seata/mcp/tools/GlobalSessionTools.java: ########## @@ -0,0 +1,222 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.tools; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.result.PageResult; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.core.model.GlobalStatus; +import org.apache.seata.mcp.core.constant.RPCConstant; +import org.apache.seata.mcp.core.props.MCPProperties; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.core.utils.DateUtils; +import org.apache.seata.mcp.entity.dto.GlobalSessionParamDto; +import org.apache.seata.mcp.entity.param.GlobalAbnormalSessionParam; +import org.apache.seata.mcp.entity.param.GlobalSessionParam; +import org.apache.seata.mcp.entity.vo.GlobalSessionVO; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.ModifyConfirmService; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springaicommunity.mcp.annotation.McpToolParam; +import org.springframework.stereotype.Service; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@Service +public class GlobalSessionTools { + + private final ConsoleApiService mcpRPCService; + + private final MCPProperties mcpProperties; + + private final ObjectMapper objectMapper; + + private final ModifyConfirmService modifyConfirmService; + + private final List<Integer> exceptionStatus = new ArrayList<>(); + + public static int AbnormalSessionPageSize = 30; Review Comment: The variable name 'AbnormalSessionPageSize' does not follow Java naming conventions. Variables that are not compile-time constants should be in camelCase. Consider renaming to 'abnormalSessionPageSize' or making it a constant with 'ABNORMAL_SESSION_PAGE_SIZE'. ########## console/src/main/java/org/apache/seata/mcp/entity/dto/GlobalLockParamDto.java: ########## @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.dto; + +import org.springframework.ai.tool.annotation.ToolParam; + +import java.io.Serializable; + +public class GlobalLockParamDto implements Serializable { + + private static final long serialVersionUID = 615412528070131284L; + + @ToolParam(description = "Global transaction id", required = false) + private String xid; + + @ToolParam(description = "the table name", required = false) + private String tableName; + + @ToolParam(description = "the transaction id", required = false) + private String transactionId; + + @ToolParam(description = "the branch id", required = false) + private String branchId; + + private String pk; + + @ToolParam(description = "resourceId", required = false) + private String resourceId; + + @ToolParam(description = "page number") + private int pageNum; + + @ToolParam(description = "Page size") + private int pageSize; + + @ToolParam( + description = "Start time, The global lock create time is after this time (yyyy-MM-dd HH:mm:ss)", + required = false) + private String timeStart; + + @ToolParam( + description = "End time, The global lock create time is before this time (yyyy-MM-dd HH:mm:ss)", + required = false) + private String timeEnd; Review Comment: The annotation '@ToolParam' is used instead of '@McpToolParam' in this file, which is inconsistent with other parameter classes in the codebase. This inconsistency could lead to configuration issues or unexpected behavior. Update all '@ToolParam' annotations to '@McpToolParam' to maintain consistency. ########## console/src/main/java/org/apache/seata/mcp/core/utils/DateUtils.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.core.utils; + +import java.time.DateTimeException; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; +import java.util.regex.Pattern; + +public class DateUtils { + + public static final Long ONE_DAY_TIMESTAMP = 86400000L; + + private static final Pattern DATE_PATTERN = Pattern.compile("^\\d{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])$"); + + public static boolean isValidDate(String dateStr) { + return DATE_PATTERN.matcher(dateStr).matches(); + } + + public static long convertToTimestampFromDate(String dateStr) { + if (!isValidDate(dateStr)) { + throw new DateTimeException("The time format does not match yyyy-mm-dd"); + } + LocalDate date = LocalDate.parse(dateStr); + ZonedDateTime zonedDateTime = date.atStartOfDay(ZoneId.systemDefault()); + return zonedDateTime.toInstant().toEpochMilli(); + } + + public static long convertToTimeStampFromDateTime(String dateTimeStr) { + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + + try { + LocalDateTime dateTime = LocalDateTime.parse(dateTimeStr, formatter); + return dateTime.atZone(ZoneId.systemDefault()).toInstant().toEpochMilli(); + } catch (DateTimeParseException e) { + return -1; + } + } + + public static String convertToDateTimeFromTimestamp(Long timestamp) { + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); + LocalDateTime dateTime; + try { + dateTime = Instant.ofEpochMilli(timestamp) + .atZone(ZoneId.systemDefault()) + .toLocalDateTime(); + } catch (DateTimeException | ArithmeticException e) { + return "Parse Failed, please check that the timestamp is correct"; + } + return dateTime.format(formatter); + } + + public static boolean judgeExceedTimeDuration(Long startTime, Long endTime, Long maxDuration) { + if (endTime < startTime) throw new IllegalArgumentException("endTime must not be earlier than startTime"); + ; Review Comment: The empty semicolon after the throw statement should be removed. This is a redundant statement separator that serves no purpose. ```suggestion ``` ########## console/src/main/java/org/apache/seata/mcp/entity/vo/GlobalLockVO.java: ########## @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.vo; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import org.apache.seata.mcp.core.config.TimestampToStringDeserializer; + +/** + * GlobalLockVO + */ +public class GlobalLockVO { + + private String xid; + + private String transactionId; + + private String branchId; + + private String resourceId; + + private String tableName; + + private String pk; + + private String rowKey; + + /** + * the vgroup + */ + private String vgroup; + + @JsonDeserialize(using = TimestampToStringDeserializer.class) + private String gmtCreate; + + @JsonDeserialize(using = TimestampToStringDeserializer.class) + private String gmtModified; + + public String getXid() { + return xid; + } + + public void setXid(String xid) { + this.xid = xid; + } + + public String getTransactionId() { + return transactionId; + } + + public void setTransactionId(Long transactionId) { + this.transactionId = String.valueOf(transactionId); + } + + public String getBranchId() { + return branchId; + } + + public void setBranchId(Long branchId) { + this.branchId = String.valueOf(branchId); + } + + public String getResourceId() { + return resourceId; + } + + public void setResourceId(String resourceId) { + this.resourceId = resourceId; + } + + public String getTableName() { + return tableName; + } + + public void setTableName(String tableName) { + this.tableName = tableName; + } + + public String getPk() { + return pk; + } + + public void setPk(String pk) { + this.pk = pk; + } + + public String getRowKey() { + return rowKey; + } + + public void setRowKey(String rowKey) { + this.rowKey = rowKey; + } + + public String getGmtCreate() { + return gmtCreate; + } + + public void setGmtCreate(String gmtCreate) { + this.gmtCreate = gmtCreate; + } + + public String getGmtModified() { + return gmtModified; + } + + public void setGmtModified(String gmtModified) { + this.gmtModified = gmtModified; + } + + public void setTransactionId(String transactionId) { + this.transactionId = transactionId; + } + + public void setBranchId(String branchId) { + this.branchId = branchId; + } Review Comment: The GlobalLockVO class has duplicate setter methods for 'transactionId' and 'branchId'. Lines 64-66 and 124-126 define setTransactionId(Long) and setTransactionId(String), and lines 72-74 and 128-130 define setBranchId(Long) and setBranchId(String). This creates method overloading ambiguity. Consider removing the duplicate methods and keeping only one consistent setter for each field. ########## console/src/main/java/org/apache/seata/mcp/entity/param/GlobalLockParam.java: ########## @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.param; + +import org.apache.seata.common.util.PageUtil; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.utils.DateUtils; +import org.apache.seata.mcp.entity.dto.GlobalLockParamDto; +import org.springframework.beans.BeanUtils; + +/** + * Global lock param + */ +public class GlobalLockParam extends org.apache.seata.server.console.entity.param.GlobalLockParam { + + public static GlobalLockParam convertFromParamDto(GlobalLockParamDto paramDto) { + PageUtil.checkParam(paramDto.getPageNum(), paramDto.getPageSize()); + GlobalLockParam param = new GlobalLockParam(); Review Comment: GlobalLockParam has the same name as its supertype [org.apache.seata.server.console.entity.param.GlobalLockParam](1). ```suggestion public class McpGlobalLockParam extends org.apache.seata.server.console.entity.param.GlobalLockParam { public static McpGlobalLockParam convertFromParamDto(GlobalLockParamDto paramDto) { PageUtil.checkParam(paramDto.getPageNum(), paramDto.getPageSize()); McpGlobalLockParam param = new McpGlobalLockParam(); ``` ########## console/src/main/java/org/apache/seata/mcp/tools/GlobalSessionTools.java: ########## @@ -0,0 +1,222 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.tools; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.result.PageResult; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.core.model.GlobalStatus; +import org.apache.seata.mcp.core.constant.RPCConstant; +import org.apache.seata.mcp.core.props.MCPProperties; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.core.utils.DateUtils; +import org.apache.seata.mcp.entity.dto.GlobalSessionParamDto; +import org.apache.seata.mcp.entity.param.GlobalAbnormalSessionParam; +import org.apache.seata.mcp.entity.param.GlobalSessionParam; +import org.apache.seata.mcp.entity.vo.GlobalSessionVO; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.ModifyConfirmService; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springaicommunity.mcp.annotation.McpToolParam; +import org.springframework.stereotype.Service; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@Service +public class GlobalSessionTools { + + private final ConsoleApiService mcpRPCService; + + private final MCPProperties mcpProperties; + + private final ObjectMapper objectMapper; + + private final ModifyConfirmService modifyConfirmService; + + private final List<Integer> exceptionStatus = new ArrayList<>(); + + public static int AbnormalSessionPageSize = 30; + + public GlobalSessionTools( + ConsoleApiService mcpRPCService, + MCPProperties mcpProperties, + ObjectMapper objectMapper, + ModifyConfirmService modifyConfirmService) { + this.mcpRPCService = mcpRPCService; + this.mcpProperties = mcpProperties; + this.objectMapper = objectMapper; + this.modifyConfirmService = modifyConfirmService; + exceptionStatus.add(GlobalStatus.CommitFailed.getCode()); + exceptionStatus.add(GlobalStatus.TimeoutRollbackFailed.getCode()); + exceptionStatus.add(GlobalStatus.RollbackFailed.getCode()); + } + + @McpTool(description = "Query global transactions") + public PageResult<GlobalSessionVO> queryGlobalSession( + @McpToolParam(description = "Specify the namespace of the TC node") NameSpaceDetail nameSpaceDetail, + @McpToolParam(description = "Query parameter objects") GlobalSessionParamDto paramDto) { + GlobalSessionParam param = GlobalSessionParam.covertFromDtoParam(paramDto); + if (param.getTimeStart() != null) { + if (param.getTimeEnd() != null) { + if (DateUtils.judgeExceedTimeDuration( + param.getTimeStart(), param.getTimeEnd(), mcpProperties.getQueryDuration())) { + throw new IllegalArgumentException( + "The query time span is not allowed to exceed the max query duration : " + + DateUtils.convertToHourFromTimeStamp(mcpProperties.getQueryDuration()) + " hour"); Review Comment: The error message "The query time span is not allowed to exceed the max query duration : " + ... is constructed using string concatenation with multiple method calls. Consider using String.format() or a template with placeholders for better readability and maintainability. ########## namingserver/src/main/java/org/apache/seata/namingserver/service/ConsoleLocalServiceImpl.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.namingserver.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.metadata.ClusterRole; +import org.apache.seata.common.metadata.Node; +import org.apache.seata.common.metadata.namingserver.NamingServerNode; +import org.apache.seata.common.util.CollectionUtils; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.impl.ConsoleRemoteServiceImpl; +import org.apache.seata.namingserver.manager.NamingManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.context.annotation.Primary; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ThreadLocalRandom; + +import static org.apache.seata.common.Constants.RAFT_GROUP_HEADER; + +@ConditionalOnBean(ConsoleRemoteServiceImpl.class) +@Primary +@Service +public class ConsoleLocalServiceImpl implements ConsoleApiService { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final NamingManager namingManager; + + private final RestTemplate restTemplate; + + private final ObjectMapper objectMapper; + + public ConsoleLocalServiceImpl(NamingManager namingManager, RestTemplate restTemplate, ObjectMapper objectMapper) { + this.namingManager = namingManager; + this.restTemplate = restTemplate; + this.objectMapper = objectMapper; + } + + public String getResult( + NameSpaceDetail nameSpaceDetail, + HttpMethod httpMethod, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + String namespace = nameSpaceDetail.getNamespace(); + String cluster = nameSpaceDetail.getCluster(); + String vgroup = nameSpaceDetail.getvGroup(); + if (StringUtils.isNotBlank(namespace) && (StringUtils.isNotBlank(cluster) || StringUtils.isNotBlank(vgroup))) { + List<NamingServerNode> list = null; + if (StringUtils.isNotBlank(vgroup)) { + list = namingManager.getInstancesByVgroupAndNamespace( + namespace, vgroup, StringUtils.equalsIgnoreCase(httpMethod.name(), HttpMethod.GET.name())); + } else if (StringUtils.isNotBlank(cluster)) { + list = namingManager.getInstances(namespace, cluster); + } + if (CollectionUtils.isNotEmpty(list)) { + // Randomly select a node from the list + NamingServerNode node = list.get(ThreadLocalRandom.current().nextInt(list.size())); + Node.Endpoint controlEndpoint = node.getControl(); + if (controlEndpoint != null) { + // Construct the target URL + String baseUrl = "http://" + controlEndpoint.getHost() + ":" + controlEndpoint.getPort(); + Map<String, Object> queryParamsMap = objectToQueryParamMap(queryParams); + String targetUrl = buildUrl(baseUrl, path, pathParams, queryParamsMap); + if (node.getRole() == ClusterRole.LEADER) { + headers.add(RAFT_GROUP_HEADER, node.getUnit()); + } + HttpEntity<String> entity = new HttpEntity<>(headers); + String responseBody = null; + try { + ResponseEntity<String> response = + restTemplate.exchange(targetUrl, httpMethod, entity, String.class); + + responseBody = response.getBody(); + + if (!response.getStatusCode().is2xxSuccessful()) { + logger.warn("MCP request returned non-success status: {}", response.getStatusCode()); + } + return responseBody; + } catch (RestClientException e) { + logger.error("MCP {} Call TC Failed: {}", httpMethod.name(), e.getMessage()); + return responseBody; + } + } + } + throw new IllegalArgumentException("Couldn't find target node url"); + } + throw new IllegalArgumentException("Invalid NameSpace Detail"); + } + + @Override + public String getCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.GET, path, queryParams, pathParams, headers); + } + + @Override + public String deleteCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.DELETE, path, queryParams, pathParams, headers); + } + + @Override + public String putCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.PUT, path, queryParams, pathParams, headers); + } + + @Override + public String getCallNameSpace( + String path, Object queryParams, Map<String, String> pathParams, HttpHeaders headers) { + String namespace; + try { + namespace = objectMapper.writeValueAsString(namingManager.namespace()); + } catch (JsonProcessingException e) { + logger.error("Get NameSpace failed: {}", e.getMessage()); + throw new RuntimeException(e); + } + return namespace; + } + + private Map<String, Object> objectToQueryParamMap(Object obj) { + if (obj == null) { + return Collections.emptyMap(); + } + if (obj instanceof Map) { + Map<?, ?> map = (Map<?, ?>) obj; + Map<String, Object> result = new HashMap<>(map.size()); + map.forEach((k, v) -> { + if (k != null && v != null) { + result.put(k.toString(), v); + } + }); + return result; + } + + Map<String, Object> paramMap = new HashMap<>(); + Class<?> clazz = obj.getClass(); + while (clazz != null && clazz != Object.class) { + for (Field field : clazz.getDeclaredFields()) { + try { + field.setAccessible(true); + Object value = field.get(obj); + if (value != null) { + paramMap.putIfAbsent(field.getName(), value); + } + } catch (IllegalAccessException e) { + logger.warn("Failed to access field {}: {}", field.getName(), e.getMessage()); + } + } + clazz = clazz.getSuperclass(); + } + return paramMap; + } + + private String buildUrl( + String baseUrl, String path, Map<String, String> pathParams, Map<String, Object> queryParams) { + + UriComponentsBuilder builder = + UriComponentsBuilder.fromUriString(baseUrl).path(path); + + if (pathParams != null && !pathParams.isEmpty()) { + for (Map.Entry<String, String> entry : pathParams.entrySet()) { + builder.queryParam(entry.getKey(), entry.getValue()); + } + } + + if (queryParams != null && !queryParams.isEmpty()) { + for (Map.Entry<String, Object> entry : queryParams.entrySet()) { + if (entry.getValue() instanceof Iterable) { + for (Object value : (Iterable<?>) entry.getValue()) { + builder.queryParam(entry.getKey(), value); + } + } else if (entry.getValue() != null + && entry.getValue().getClass().isArray()) { + Object[] array = (Object[]) entry.getValue(); + for (Object value : array) { + builder.queryParam(entry.getKey(), value); + } + } else { + builder.queryParam(entry.getKey(), entry.getValue()); + } + } + } + + return builder.build().toUriString(); + } Review Comment: There is code duplication between ConsoleLocalServiceImpl and ConsoleRemoteServiceImpl. Both classes contain identical implementations of the 'objectToQueryParamMap' and 'buildUrl' methods (lines 167-232 and 223-288 respectively). Consider extracting these methods into a shared utility class to improve maintainability and reduce code duplication. ########## console/src/main/java/org/apache/seata/mcp/entity/param/GlobalSessionParam.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.param; + +import org.apache.seata.common.util.PageUtil; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.utils.DateUtils; +import org.apache.seata.mcp.entity.dto.GlobalSessionParamDto; + +/** + * Global session param + */ +public class GlobalSessionParam extends org.apache.seata.server.console.entity.param.GlobalSessionParam { Review Comment: GlobalSessionParam has the same name as its supertype [org.apache.seata.server.console.entity.param.GlobalSessionParam](1). ########## console/src/main/java/org/apache/seata/mcp/entity/vo/BranchSessionVO.java: ########## @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.entity.vo; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import org.apache.seata.mcp.core.config.TimestampToStringDeserializer; + +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; + +/** + * BranchSessionVO + */ +@JsonAutoDetect(getterVisibility = NONE, fieldVisibility = ANY) +public class BranchSessionVO extends org.apache.seata.server.console.entity.vo.BranchSessionVO { Review Comment: BranchSessionVO has the same name as its supertype [org.apache.seata.server.console.entity.vo.BranchSessionVO](1). ```suggestion * McpBranchSessionVO */ @JsonAutoDetect(getterVisibility = NONE, fieldVisibility = ANY) public class McpBranchSessionVO extends org.apache.seata.server.console.entity.vo.BranchSessionVO { ``` ########## console/src/main/java/org/apache/seata/mcp/tools/NameSpaceTools.java: ########## @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.mcp.tools; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.result.SingleResult; +import org.apache.seata.mcp.core.constant.RPCConstant; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springframework.stereotype.Service; + +import java.util.HashMap; +import java.util.Map; + +@Service +public class NameSpaceTools { + + private final ConsoleApiService mcpRPCService; + + private final ObjectMapper objectMapper; + + public NameSpaceTools(ConsoleApiService mcpRPCService, ObjectMapper objectMapper) { + this.mcpRPCService = mcpRPCService; + this.objectMapper = objectMapper; + } + + @McpTool(description = "Get the namespace and cluster or vgroup where all TC/Servers are located") + public SingleResult<?> getTCNameSpaces() { + String result = mcpRPCService.getCallNameSpace(RPCConstant.GET_NAMESPACE_PATH, null, null, null); + Map<String, Object> nameSpacesVo = new HashMap<>(); + try { + JsonNode root = objectMapper.readTree(result); + JsonNode dataNode = root.get("data"); + if (dataNode != null && !dataNode.isNull()) { + nameSpacesVo.put("namespaces", dataNode.toString()); + } + } catch (JsonProcessingException e) { + nameSpacesVo.put("failed", e.getMessage()); + return SingleResult.failure("get namespace failed:" + e.getMessage()); + } Review Comment: The error handling for JsonProcessingException is inconsistent. In the success path, the exception message is added to the result Map and a failure result is returned. However, in the non-null data node path, no exception handling is needed. Consider removing the redundant Map entry for 'failed' or restructuring the error handling for clarity. ########## namingserver/src/main/java/org/apache/seata/namingserver/service/ConsoleLocalServiceImpl.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.namingserver.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.metadata.ClusterRole; +import org.apache.seata.common.metadata.Node; +import org.apache.seata.common.metadata.namingserver.NamingServerNode; +import org.apache.seata.common.util.CollectionUtils; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.impl.ConsoleRemoteServiceImpl; +import org.apache.seata.namingserver.manager.NamingManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.context.annotation.Primary; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ThreadLocalRandom; + +import static org.apache.seata.common.Constants.RAFT_GROUP_HEADER; + +@ConditionalOnBean(ConsoleRemoteServiceImpl.class) +@Primary +@Service +public class ConsoleLocalServiceImpl implements ConsoleApiService { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final NamingManager namingManager; + + private final RestTemplate restTemplate; + + private final ObjectMapper objectMapper; + + public ConsoleLocalServiceImpl(NamingManager namingManager, RestTemplate restTemplate, ObjectMapper objectMapper) { + this.namingManager = namingManager; + this.restTemplate = restTemplate; + this.objectMapper = objectMapper; + } + + public String getResult( + NameSpaceDetail nameSpaceDetail, + HttpMethod httpMethod, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + String namespace = nameSpaceDetail.getNamespace(); + String cluster = nameSpaceDetail.getCluster(); + String vgroup = nameSpaceDetail.getvGroup(); + if (StringUtils.isNotBlank(namespace) && (StringUtils.isNotBlank(cluster) || StringUtils.isNotBlank(vgroup))) { + List<NamingServerNode> list = null; + if (StringUtils.isNotBlank(vgroup)) { + list = namingManager.getInstancesByVgroupAndNamespace( + namespace, vgroup, StringUtils.equalsIgnoreCase(httpMethod.name(), HttpMethod.GET.name())); + } else if (StringUtils.isNotBlank(cluster)) { + list = namingManager.getInstances(namespace, cluster); + } + if (CollectionUtils.isNotEmpty(list)) { + // Randomly select a node from the list + NamingServerNode node = list.get(ThreadLocalRandom.current().nextInt(list.size())); + Node.Endpoint controlEndpoint = node.getControl(); + if (controlEndpoint != null) { + // Construct the target URL + String baseUrl = "http://" + controlEndpoint.getHost() + ":" + controlEndpoint.getPort(); + Map<String, Object> queryParamsMap = objectToQueryParamMap(queryParams); + String targetUrl = buildUrl(baseUrl, path, pathParams, queryParamsMap); + if (node.getRole() == ClusterRole.LEADER) { + headers.add(RAFT_GROUP_HEADER, node.getUnit()); + } + HttpEntity<String> entity = new HttpEntity<>(headers); + String responseBody = null; + try { + ResponseEntity<String> response = + restTemplate.exchange(targetUrl, httpMethod, entity, String.class); + + responseBody = response.getBody(); + + if (!response.getStatusCode().is2xxSuccessful()) { + logger.warn("MCP request returned non-success status: {}", response.getStatusCode()); + } + return responseBody; + } catch (RestClientException e) { + logger.error("MCP {} Call TC Failed: {}", httpMethod.name(), e.getMessage()); + return responseBody; + } Review Comment: The exception handling returns 'responseBody' which is null at the point of the catch block. This means when a RestClientException occurs, the method will return null instead of a meaningful error response. Consider returning an error message or rethrowing the exception rather than returning null. ########## namingserver/src/main/java/org/apache/seata/namingserver/service/ConsoleLocalServiceImpl.java: ########## @@ -0,0 +1,233 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.namingserver.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.seata.common.metadata.ClusterRole; +import org.apache.seata.common.metadata.Node; +import org.apache.seata.common.metadata.namingserver.NamingServerNode; +import org.apache.seata.common.util.CollectionUtils; +import org.apache.seata.common.util.StringUtils; +import org.apache.seata.mcp.core.props.NameSpaceDetail; +import org.apache.seata.mcp.service.ConsoleApiService; +import org.apache.seata.mcp.service.impl.ConsoleRemoteServiceImpl; +import org.apache.seata.namingserver.manager.NamingManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.context.annotation.Primary; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Service; +import org.springframework.web.client.RestClientException; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ThreadLocalRandom; + +import static org.apache.seata.common.Constants.RAFT_GROUP_HEADER; + +@ConditionalOnBean(ConsoleRemoteServiceImpl.class) +@Primary +@Service +public class ConsoleLocalServiceImpl implements ConsoleApiService { + + private final Logger logger = LoggerFactory.getLogger(this.getClass()); + + private final NamingManager namingManager; + + private final RestTemplate restTemplate; + + private final ObjectMapper objectMapper; + + public ConsoleLocalServiceImpl(NamingManager namingManager, RestTemplate restTemplate, ObjectMapper objectMapper) { + this.namingManager = namingManager; + this.restTemplate = restTemplate; + this.objectMapper = objectMapper; + } + + public String getResult( + NameSpaceDetail nameSpaceDetail, + HttpMethod httpMethod, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + String namespace = nameSpaceDetail.getNamespace(); + String cluster = nameSpaceDetail.getCluster(); + String vgroup = nameSpaceDetail.getvGroup(); + if (StringUtils.isNotBlank(namespace) && (StringUtils.isNotBlank(cluster) || StringUtils.isNotBlank(vgroup))) { + List<NamingServerNode> list = null; + if (StringUtils.isNotBlank(vgroup)) { + list = namingManager.getInstancesByVgroupAndNamespace( + namespace, vgroup, StringUtils.equalsIgnoreCase(httpMethod.name(), HttpMethod.GET.name())); + } else if (StringUtils.isNotBlank(cluster)) { + list = namingManager.getInstances(namespace, cluster); + } + if (CollectionUtils.isNotEmpty(list)) { + // Randomly select a node from the list + NamingServerNode node = list.get(ThreadLocalRandom.current().nextInt(list.size())); + Node.Endpoint controlEndpoint = node.getControl(); + if (controlEndpoint != null) { + // Construct the target URL + String baseUrl = "http://" + controlEndpoint.getHost() + ":" + controlEndpoint.getPort(); + Map<String, Object> queryParamsMap = objectToQueryParamMap(queryParams); + String targetUrl = buildUrl(baseUrl, path, pathParams, queryParamsMap); + if (node.getRole() == ClusterRole.LEADER) { + headers.add(RAFT_GROUP_HEADER, node.getUnit()); + } + HttpEntity<String> entity = new HttpEntity<>(headers); + String responseBody = null; + try { + ResponseEntity<String> response = + restTemplate.exchange(targetUrl, httpMethod, entity, String.class); + + responseBody = response.getBody(); + + if (!response.getStatusCode().is2xxSuccessful()) { + logger.warn("MCP request returned non-success status: {}", response.getStatusCode()); + } + return responseBody; + } catch (RestClientException e) { + logger.error("MCP {} Call TC Failed: {}", httpMethod.name(), e.getMessage()); + return responseBody; + } + } + } + throw new IllegalArgumentException("Couldn't find target node url"); + } + throw new IllegalArgumentException("Invalid NameSpace Detail"); + } + + @Override + public String getCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.GET, path, queryParams, pathParams, headers); + } + + @Override + public String deleteCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.DELETE, path, queryParams, pathParams, headers); + } + + @Override + public String putCallTC( + NameSpaceDetail nameSpaceDetail, + String path, + Object queryParams, + Map<String, String> pathParams, + HttpHeaders headers) { + return getResult(nameSpaceDetail, HttpMethod.PUT, path, queryParams, pathParams, headers); + } + + @Override + public String getCallNameSpace( + String path, Object queryParams, Map<String, String> pathParams, HttpHeaders headers) { + String namespace; + try { + namespace = objectMapper.writeValueAsString(namingManager.namespace()); + } catch (JsonProcessingException e) { + logger.error("Get NameSpace failed: {}", e.getMessage()); + throw new RuntimeException(e); + } + return namespace; + } + + private Map<String, Object> objectToQueryParamMap(Object obj) { + if (obj == null) { + return Collections.emptyMap(); + } + if (obj instanceof Map) { + Map<?, ?> map = (Map<?, ?>) obj; + Map<String, Object> result = new HashMap<>(map.size()); + map.forEach((k, v) -> { + if (k != null && v != null) { + result.put(k.toString(), v); + } + }); + return result; + } + + Map<String, Object> paramMap = new HashMap<>(); + Class<?> clazz = obj.getClass(); + while (clazz != null && clazz != Object.class) { + for (Field field : clazz.getDeclaredFields()) { + try { + field.setAccessible(true); + Object value = field.get(obj); + if (value != null) { + paramMap.putIfAbsent(field.getName(), value); + } + } catch (IllegalAccessException e) { + logger.warn("Failed to access field {}: {}", field.getName(), e.getMessage()); + } + } + clazz = clazz.getSuperclass(); + } + return paramMap; + } + + private String buildUrl( + String baseUrl, String path, Map<String, String> pathParams, Map<String, Object> queryParams) { + + UriComponentsBuilder builder = + UriComponentsBuilder.fromUriString(baseUrl).path(path); + + if (pathParams != null && !pathParams.isEmpty()) { + for (Map.Entry<String, String> entry : pathParams.entrySet()) { + builder.queryParam(entry.getKey(), entry.getValue()); + } + } Review Comment: The method 'buildUrl' is using both 'pathParams' and 'queryParams' but adding both to query parameters. Based on the parameter name 'pathParams', it appears these should be path variables (URI template variables), not query parameters. If 'pathParams' are intended as query parameters, rename the parameter for clarity. If they should be path variables, use UriComponentsBuilder's 'uriVariables' method instead. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
