Copilot commented on code in PR #7851: URL: https://github.com/apache/incubator-seata/pull/7851#discussion_r2612690385
########## namingserver/src/main/java/org/apache/seata/namingserver/entity/vo/v2/NamespaceVO.java: ########## @@ -0,0 +1,34 @@ +/* + * 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.entity.vo.v2; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + Review Comment: Missing documentation for the new VO class. The NamespaceVO class in the v2 package should have Javadoc documentation explaining its purpose and how it differs from the original NamespaceVO class. This is important for API consumers to understand the data structure they're receiving. ```suggestion /** * Value Object representing namespace information for the v2 API. * <p> * This class provides a mapping between cluster names and their associated vgroup lists. * It is used in the v2 version of the API and may differ from the original {@code NamespaceVO} * (in the v1 or root package) in its structure or the semantics of its fields. * <p> * <b>Differences from the original NamespaceVO:</b> * <ul> * <li>Located in the {@code org.apache.seata.namingserver.entity.vo.v2} package, indicating it is for the v2 API.</li> * <li>Contains a {@code clusterVgroups} map, which maps cluster names to lists of vgroup names.</li> * <li>May have a different structure or additional fields compared to the original version.</li> * </ul> * <p> * API consumers should refer to this class when interacting with the v2 endpoints to understand * the data structure being returned. */ ``` ########## console/src/main/resources/static/console-fe/src/utils/requestV2.ts: ########## @@ -0,0 +1,92 @@ +/* + * 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. + */ +import React from 'react'; +import axios, { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios'; +import { Message } from '@alicloud/console-components'; +import { get } from 'lodash'; +import { GlobalStateModel } from '@/reducers'; +import { AUTHORIZATION_HEADER } from '@/contants'; + +const API_GENERAL_ERROR_MESSAGE: string = 'Request error, please try again later!'; + +const codeMessage = { + 200: '服务器成功返回请求的数据。', + 201: '新建或修改数据成功。', + 202: '一个请求已经进入后台排队（异步任务）。', + 204: '删除数据成功。', + 400: '发出的请求有错误，服务器没有进行新建或修改数据的操作。', + 401: '用户没有权限（令牌、用户名、密码错误）。', + 403: '用户得到授权，但是访问是被禁止的。', + 404: '发出的请求针对的是不存在的记录，服务器没有进行操作。', + 406: '请求的格式不可得。', + 410: '请求的资源被永久删除，且不会再得到的。', + 422: '当创建一个对象时，发生一个验证错误。', + 500: '服务器发生错误，请检查服务器。', + 502: '网关错误。', + 503: '服务不可用，服务器暂时过载或维护。', + 504: '网关超时。', + '-1000': '项目名称已存在, 请使用其他名称', +}; + +const requestV2 = () => { + const instance: AxiosInstance = axios.create({ + baseURL: 'api/v2', + method: 'get', + }); + + instance.interceptors.request.use((config: AxiosRequestConfig) => { + let authHeader: string | null = localStorage.getItem(AUTHORIZATION_HEADER); + // add jwt header + config.headers[AUTHORIZATION_HEADER] = authHeader; + + return config; + }) + + instance.interceptors.response.use( + (response: AxiosResponse): Promise<any> => { + const code = get(response, 'data.code'); + if (response.status === 200 && code === '200') { + return Promise.resolve(get(response, 'data')); + } else { + const errorText = + (codeMessage as any)[code] || + get(response, 'data.message') || + get(response, 'data.errorMsg') || + response.statusText; + Message.error(errorText || `请求错误 ${code}: ${get(response, 'config.url', '')}`); Review Comment: The error message is partially in Chinese. For consistency with the rest of the codebase and better international usability, error messages should be in English or properly internationalized. ########## namingserver/src/test/java/org/apache/seata/namingserver/NamingControllerV2Test.java: ########## @@ -0,0 +1,83 @@ +/* + * 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; + +import org.apache.seata.common.metadata.Node; +import org.apache.seata.common.metadata.namingserver.NamingServerNode; +import org.apache.seata.common.result.SingleResult; +import org.apache.seata.namingserver.controller.NamingControllerV2; +import org.apache.seata.namingserver.entity.vo.v2.NamespaceVO; +import org.apache.seata.namingserver.manager.NamingManager; +import org.junit.jupiter.api.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.junit4.SpringRunner; + +import java.util.Map; +import java.util.UUID; + +import static org.apache.seata.common.NamingServerConstants.CONSTANT_GROUP; +import static org.junit.jupiter.api.Assertions.*; + +@RunWith(SpringRunner.class) +@SpringBootTest Review Comment: The test class is using @RunWith(SpringRunner.class) which is from JUnit 4, but it's also using @Test from JUnit 5 (org.junit.jupiter.api.Test). This mixing of JUnit 4 and JUnit 5 annotations is inconsistent and could lead to unexpected behavior. Either use @ExtendWith(SpringExtension.class) from JUnit 5 or use org.junit.Test from JUnit 4 for consistency. ########## namingserver/src/main/java/org/apache/seata/namingserver/controller/NamingControllerV2.java: ########## @@ -0,0 +1,44 @@ +/* + * 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.controller; + +import org.apache.seata.common.result.SingleResult; +import org.apache.seata.namingserver.entity.vo.v2.NamespaceVO; +import org.apache.seata.namingserver.manager.NamingManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import javax.annotation.Resource; +import java.util.Map; + +@RestController +@RequestMapping(value = {"/naming/v2", "/api/v2/naming"}) +public class NamingControllerV2 { + + private static final Logger LOGGER = LoggerFactory.getLogger(NamingControllerV2.class); + + @Resource + private NamingManager namingManager; + Review Comment: Missing documentation for the new public controller method. The namespaces method in NamingControllerV2 should have Javadoc documentation explaining its purpose, the API endpoint it exposes, and what data structure it returns. This is particularly important for REST API endpoints that external consumers will use. ```suggestion /** * Retrieves all namespaces. * <p> * API Endpoint: GET /naming/v2/namespace or /api/v2/naming/namespace * </p> * * @return a {@link SingleResult} containing a map where the key is the namespace name and the value is a {@link NamespaceVO} object */ ``` ########## namingserver/src/main/java/org/apache/seata/namingserver/manager/NamingManager.java: ########## @@ -495,6 +495,64 @@ public SingleResult<Map<String, NamespaceVO>> namespace() { namespaceVO.getClusters().add(clusterName); namespaceVO.getVgroups().add(vGroup); }))); + + return SingleResult.success(namespaceVOs); + } + Review Comment: Missing documentation for the new public method. The namespaceV2 method should have Javadoc documentation explaining its purpose, return value, and how it differs from the existing namespace() method. This is especially important for public API methods to help other developers understand the API version differences. ```suggestion /** * Returns a map of namespace names to their corresponding v2 {@code NamespaceVO} representations. * <p> * This method provides a new version (v2) of the namespace information, including detailed mappings * of clusters and vgroups for each namespace. The returned structure is designed to support enhanced * querying and display of namespace relationships, such as which clusters and vgroups belong to each namespace, * and the mapping between clusters and their vgroups. * </p> * <p> * <b>Difference from {@link #namespace()}:</b> Unlike the {@code namespace()} method, which returns a map of * namespaces to a basic {@code NamespaceVO} (with lists of clusters and vgroups), this v2 method returns a map * to a v2 {@code NamespaceVO} that contains more granular information, including a mapping of clusters to their * associated vgroups within each namespace. This allows clients to better understand the relationships between * namespaces, clusters, and vgroups. * </p> * * @return a {@code SingleResult} containing a map where the key is the namespace name and the value is a v2 * {@code NamespaceVO} object with detailed cluster and vgroup information. */ ``` ########## console/src/main/resources/static/console-fe/src/service/globalLockInfo.ts: ########## @@ -33,7 +34,7 @@ export type GlobalLockParam = { }; export async function fetchNamespace():Promise<any> { - const result = await request.get('/naming/namespace', { + const result = await requestV2.get('/namespace', { Review Comment: The API endpoint path is inconsistent with the backend controller mapping. The backend NamingControllerV2 is mapped to "/naming/v2/namespace" or "/api/v2/naming/namespace", but this code is calling "/namespace" which would resolve to "api/v2/namespace". This path doesn't match the backend mapping and the request will fail with a 404 error. ```suggestion const result = await requestV2.get('/naming/v2/namespace', { ``` ########## namingserver/src/main/java/org/apache/seata/namingserver/manager/NamingManager.java: ########## @@ -495,6 +495,64 @@ public SingleResult<Map<String, NamespaceVO>> namespace() { namespaceVO.getClusters().add(clusterName); namespaceVO.getVgroups().add(vGroup); }))); + + return SingleResult.success(namespaceVOs); + } + + public SingleResult<Map<String, org.apache.seata.namingserver.entity.vo.v2.NamespaceVO>> namespaceV2() { + Map<String, Set<String>> clustersMap = new HashMap<>(); + Map<String, Set<String>> vgroupsMap = new HashMap<>(); + Map<String, Map<String, Set<String>>> clusterVgroupsMap = new HashMap<>(); // namespace -> cluster -> vgroups + + // Collect all namespaces + Set<String> allNamespaces = new HashSet<>(); + allNamespaces.addAll(namespaceClusterDataMap.keySet()); + vGroupMap.asMap().values().forEach(namespaceMap -> allNamespaces.addAll(namespaceMap.keySet())); + + // Initialize maps for all namespaces + allNamespaces.forEach(namespace -> { + clustersMap.computeIfAbsent(namespace, k -> new HashSet<>()); + vgroupsMap.computeIfAbsent(namespace, k -> new HashSet<>()); + clusterVgroupsMap.computeIfAbsent(namespace, k -> new HashMap<>()); + }); + + // Collect all clusters from namespaceClusterDataMap + namespaceClusterDataMap.forEach((namespace, clusterDataMap) -> { + Set<String> clusters = clustersMap.get(namespace); + clusters.addAll(clusterDataMap.keySet()); + }); + + // Collect vgroups and build cluster to vgroups mapping + Map<String /* VGroup */, ConcurrentMap<String /* namespace */, NamespaceBO>> currentVGroupMap = + new HashMap<>(vGroupMap.asMap()); + currentVGroupMap.forEach((vGroup, namespaceMap) -> namespaceMap.forEach((namespace, namespaceBO) -> { + Set<String> vgroups = vgroupsMap.get(namespace); + vgroups.add(vGroup); + + // Build cluster to vgroups + Map<String, Set<String>> clusterVg = clusterVgroupsMap.get(namespace); + namespaceBO.getClusterMap().forEach((clusterName, clusterBO) -> { + Set<String> vgSet = clusterVg.computeIfAbsent(clusterName, k -> new HashSet<>()); + vgSet.add(vGroup); + }); + })); + + // Build NamespaceVOv2 Review Comment: Typo in comment. "NamespaceVOv2" should be "NamespaceVO v2" (with space) for consistency with the class name org.apache.seata.namingserver.entity.vo.v2.NamespaceVO. ```suggestion // Build NamespaceVO v2 ``` ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -949,6 +1016,12 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> <Icon type="search" />{searchButtonLabel} </Form.Submit> </FormItem> + {/* {create vgroup button} */} Review Comment: The comment syntax is incorrect for JSX/TSX. This should use standard single-line comment syntax or be removed entirely. HTML-style comments are not valid in JSX and this may cause parsing issues. ########## console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx: ########## @@ -187,15 +192,33 @@ class GlobalLockInfo extends React.Component<GlobalProps, GlobalLockInfoState> { searchFilterOnChange = (key:string, val:string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + clusters, + vgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val, cluster: firstCluster}), }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -416,11 +425,30 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> searchFilterOnChange = (key: string, val: string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + clusters, + vgroups, + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val, cluster: firstCluster}), }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -416,11 +425,30 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> searchFilterOnChange = (key: string, val: string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + clusters, + vgroups, + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val, cluster: firstCluster}), }); + } else if (key === 'cluster') { + const currentNamespace = this.state.globalSessionParam.namespace; + if (currentNamespace) { + const namespaceData = this.state.namespaceOptions.get(currentNamespace); + const clusterVgroups = namespaceData ? namespaceData.clusterVgroups : {}; + const selectedVgroups = clusterVgroups[val] || []; + this.setState({ + vgroups: selectedVgroups, + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + }); + } else { + this.setState({ + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx: ########## @@ -187,15 +192,33 @@ class GlobalLockInfo extends React.Component<GlobalProps, GlobalLockInfoState> { searchFilterOnChange = (key:string, val:string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + clusters, + vgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val, cluster: firstCluster}), }); + } else if (key === 'cluster') { + const currentNamespace = this.state.globalLockParam.namespace; + if (currentNamespace) { + const namespaceData = this.state.namespaceOptions.get(currentNamespace); + const clusterVgroups = namespaceData ? namespaceData.clusterVgroups : {}; + const selectedVgroups = clusterVgroups[val] || []; + this.setState({ + vgroups: selectedVgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + }); + } else { + this.setState({ + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + }); + } } else { this.setState({ - globalLockParam: Object.assign(this.state.globalLockParam, - {[key]: val}), + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## server/pom.xml: ########## @@ -301,7 +301,6 @@ <dependency> <groupId>com.squareup.okhttp3</groupId> <artifactId>okhttp</artifactId> Review Comment: The okhttp dependency scope has been changed from test to compile without any explanation or visible usage in the server module's main code. This change increases the production dependency footprint. Please verify that okhttp is actually needed in the main server code, or if this change was made in error and should remain as a test dependency. ```suggestion <artifactId>okhttp</artifactId> <scope>test</scope> ``` ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -839,6 +867,41 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> }); } + showCreateVGroupDialog = () => { + this.setState({ + createVGroupDialogVisible: true, + vGroupName: '', + }); + } + + closeCreateVGroupDialog = () => { + this.setState({ + createVGroupDialogVisible: false, + vGroupName: '', + }); + } + + handleCreateVGroup = () => { + const { locale = {} } = this.props; + const { createVGroupErrorMessage, createVGroupSuccessMessage, createVGroupFailMessage } = locale; + const { namespace, cluster } = this.state.globalSessionParam; + const { vGroupName } = this.state; + if (!namespace || !cluster || !vGroupName.trim()) { + Message.error(createVGroupErrorMessage); + return; + } + addGroup(namespace, cluster, vGroupName.trim()).then(() => { + Message.success(createVGroupSuccessMessage); + this.closeCreateVGroupDialog(); + // Delay 5 seconds before reloading namespaces to get the latest vgroup list + setTimeout(() => { + this.loadNamespaces(); + }, 5000); Review Comment: The hardcoded 5-second delay is a code smell. This arbitrary delay for reloading namespaces creates a poor user experience and doesn't guarantee the backend has finished processing. Consider implementing a proper callback mechanism, polling with a reasonable timeout, or returning an updated state from the API. ########## console/src/main/resources/static/console-fe/src/utils/requestV2.ts: ########## @@ -0,0 +1,92 @@ +/* + * 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. + */ +import React from 'react'; +import axios, { AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios'; +import { Message } from '@alicloud/console-components'; +import { get } from 'lodash'; +import { GlobalStateModel } from '@/reducers'; Review Comment: The GlobalStateModel import is unused in this file. This import should be removed as it serves no purpose in this utility module. ```suggestion ``` ########## console/src/main/resources/static/console-fe/src/utils/requestV2.ts: ########## @@ -0,0 +1,92 @@ +/* + * 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. + */ +import React from 'react'; Review Comment: The React import is unused in this file. This import should be removed as it serves no purpose in this utility module. ```suggestion ``` ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -839,6 +867,41 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> }); } + showCreateVGroupDialog = () => { + this.setState({ + createVGroupDialogVisible: true, + vGroupName: '', + }); + } + + closeCreateVGroupDialog = () => { + this.setState({ + createVGroupDialogVisible: false, + vGroupName: '', + }); + } + + handleCreateVGroup = () => { + const { locale = {} } = this.props; + const { createVGroupErrorMessage, createVGroupSuccessMessage, createVGroupFailMessage } = locale; + const { namespace, cluster } = this.state.globalSessionParam; + const { vGroupName } = this.state; + if (!namespace || !cluster || !vGroupName.trim()) { + Message.error(createVGroupErrorMessage); Review Comment: The API call is missing input validation. The vGroupName should be validated beyond just trimming whitespace, such as checking for invalid characters, maximum length, or other business rules to prevent potential issues with VGroup names. ```suggestion // Helper function to validate vGroupName validateVGroupName = (name: string): string | null => { const trimmed = name.trim(); // Example rules: 1-32 chars, alphanumeric, underscores, hyphens only if (!trimmed) { return 'VGroup name cannot be empty.'; } if (trimmed.length > 32) { return 'VGroup name must be at most 32 characters.'; } if (!/^[a-zA-Z0-9_-]+$/.test(trimmed)) { return 'VGroup name can only contain letters, numbers, underscores, and hyphens.'; } return null; } handleCreateVGroup = () => { const { locale = {} } = this.props; const { createVGroupErrorMessage, createVGroupSuccessMessage, createVGroupFailMessage } = locale; const { namespace, cluster } = this.state.globalSessionParam; const { vGroupName } = this.state; const validationError = this.validateVGroupName(vGroupName); if (!namespace || !cluster || validationError) { Message.error(validationError || createVGroupErrorMessage); ``` ########## namingserver/src/main/java/org/apache/seata/namingserver/manager/NamingManager.java: ########## @@ -495,6 +495,64 @@ public SingleResult<Map<String, NamespaceVO>> namespace() { namespaceVO.getClusters().add(clusterName); namespaceVO.getVgroups().add(vGroup); }))); + + return SingleResult.success(namespaceVOs); + } + + public SingleResult<Map<String, org.apache.seata.namingserver.entity.vo.v2.NamespaceVO>> namespaceV2() { + Map<String, Set<String>> clustersMap = new HashMap<>(); + Map<String, Set<String>> vgroupsMap = new HashMap<>(); + Map<String, Map<String, Set<String>>> clusterVgroupsMap = new HashMap<>(); // namespace -> cluster -> vgroups + + // Collect all namespaces + Set<String> allNamespaces = new HashSet<>(); + allNamespaces.addAll(namespaceClusterDataMap.keySet()); + vGroupMap.asMap().values().forEach(namespaceMap -> allNamespaces.addAll(namespaceMap.keySet())); + + // Initialize maps for all namespaces + allNamespaces.forEach(namespace -> { + clustersMap.computeIfAbsent(namespace, k -> new HashSet<>()); + vgroupsMap.computeIfAbsent(namespace, k -> new HashSet<>()); + clusterVgroupsMap.computeIfAbsent(namespace, k -> new HashMap<>()); + }); + + // Collect all clusters from namespaceClusterDataMap + namespaceClusterDataMap.forEach((namespace, clusterDataMap) -> { + Set<String> clusters = clustersMap.get(namespace); + clusters.addAll(clusterDataMap.keySet()); + }); + + // Collect vgroups and build cluster to vgroups mapping + Map<String /* VGroup */, ConcurrentMap<String /* namespace */, NamespaceBO>> currentVGroupMap = + new HashMap<>(vGroupMap.asMap()); + currentVGroupMap.forEach((vGroup, namespaceMap) -> namespaceMap.forEach((namespace, namespaceBO) -> { + Set<String> vgroups = vgroupsMap.get(namespace); + vgroups.add(vGroup); + + // Build cluster to vgroups + Map<String, Set<String>> clusterVg = clusterVgroupsMap.get(namespace); + namespaceBO.getClusterMap().forEach((clusterName, clusterBO) -> { + Set<String> vgSet = clusterVg.computeIfAbsent(clusterName, k -> new HashSet<>()); + vgSet.add(vGroup); + }); + })); + + // Build NamespaceVOv2 + Map<String, org.apache.seata.namingserver.entity.vo.v2.NamespaceVO> namespaceVOs = new HashMap<>(); + clustersMap.forEach((namespace, clusters) -> { + org.apache.seata.namingserver.entity.vo.v2.NamespaceVO namespaceVO = + new org.apache.seata.namingserver.entity.vo.v2.NamespaceVO(); + Map<String, List<String>> clusterVgList = new HashMap<>(); + Map<String, Set<String>> clusterVgSet = clusterVgroupsMap.get(namespace); + clusters.forEach(cluster -> { + Set<String> vgSet = clusterVgSet.get(cluster); + clusterVgList.put(cluster, vgSet != null ? new ArrayList<>(vgSet) : new ArrayList<>()); + }); + namespaceVO.setClusterVgroups(clusterVgList); + + namespaceVOs.put(namespace, namespaceVO); + }); + return SingleResult.success(namespaceVOs); } Review Comment: Code duplication with the namespace() method. The namespaceV2() method has significant overlap with the existing namespace() method in terms of data collection logic. Consider refactoring common logic into helper methods to reduce duplication and improve maintainability. ########## namingserver/src/main/java/org/apache/seata/namingserver/entity/vo/v2/NamespaceVO.java: ########## @@ -0,0 +1,34 @@ +/* + * 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.entity.vo.v2; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class NamespaceVO { + + Map<String, List<String>> clusterVgroups = new HashMap<>(); Review Comment: The field visibility should be private. Java best practices recommend that class fields should be private with public getter/setter methods to maintain encapsulation. ```suggestion private Map<String, List<String>> clusterVgroups = new HashMap<>(); ``` ########## console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx: ########## @@ -40,15 +40,15 @@ import moment from 'moment'; import './index.scss'; import {get} from "lodash"; import {enUsKey, getCurrentLanguage} from "@/reducers/locale"; -import {fetchNamespace} from "@/service/transactionInfo"; +import {fetchNamespaceV2} from "@/service/transactionInfo"; Review Comment: Naming inconsistency between implementations. In GlobalLockInfo, the method is calling fetchNamespaceV2 from the transactionInfo service, but in TransactionInfo component the same import uses fetchNamespace. For better maintainability and clarity, these should use consistent function names or the GlobalLockInfo should have its own properly named service function. ########## console/src/main/resources/static/console-fe/src/pages/TransactionInfo/TransactionInfo.tsx: ########## @@ -416,11 +425,30 @@ class TransactionInfo extends React.Component<GlobalProps, TransactionInfoState> searchFilterOnChange = (key: string, val: string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + clusters, + vgroups, + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val, cluster: firstCluster}), }); + } else if (key === 'cluster') { + const currentNamespace = this.state.globalSessionParam.namespace; + if (currentNamespace) { + const namespaceData = this.state.namespaceOptions.get(currentNamespace); + const clusterVgroups = namespaceData ? namespaceData.clusterVgroups : {}; + const selectedVgroups = clusterVgroups[val] || []; + this.setState({ + vgroups: selectedVgroups, + globalSessionParam: Object.assign(this.state.globalSessionParam, {[key]: val}), + }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx: ########## @@ -187,15 +192,33 @@ class GlobalLockInfo extends React.Component<GlobalProps, GlobalLockInfoState> { searchFilterOnChange = (key:string, val:string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + clusters, + vgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val, cluster: firstCluster}), }); + } else if (key === 'cluster') { + const currentNamespace = this.state.globalLockParam.namespace; + if (currentNamespace) { + const namespaceData = this.state.namespaceOptions.get(currentNamespace); + const clusterVgroups = namespaceData ? namespaceData.clusterVgroups : {}; + const selectedVgroups = clusterVgroups[val] || []; + this.setState({ + vgroups: selectedVgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + }); Review Comment: Component state update uses [potentially inconsistent value](1). ########## console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx: ########## @@ -187,15 +192,33 @@ class GlobalLockInfo extends React.Component<GlobalProps, GlobalLockInfoState> { searchFilterOnChange = (key:string, val:string) => { if (key === 'namespace') { const selectedNamespace = this.state.namespaceOptions.get(val); + const clusters = selectedNamespace ? selectedNamespace.clusters : []; + const firstCluster = clusters.length > 0 ? clusters[0] : undefined; + const clusterVgroups = selectedNamespace ? selectedNamespace.clusterVgroups : {}; + const vgroups = firstCluster ? clusterVgroups[firstCluster] || [] : []; this.setState({ - clusters: selectedNamespace ? selectedNamespace.clusters : [], - vgroups: selectedNamespace ? selectedNamespace.vgroups : [], - globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + clusters, + vgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val, cluster: firstCluster}), }); + } else if (key === 'cluster') { + const currentNamespace = this.state.globalLockParam.namespace; + if (currentNamespace) { + const namespaceData = this.state.namespaceOptions.get(currentNamespace); + const clusterVgroups = namespaceData ? namespaceData.clusterVgroups : {}; + const selectedVgroups = clusterVgroups[val] || []; + this.setState({ + vgroups: selectedVgroups, + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + }); + } else { + this.setState({ + globalLockParam: Object.assign(this.state.globalLockParam, {[key]: val}), + }); Review Comment: Component state update uses [potentially inconsistent value](1). -- 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]
