feature: migrate the console to the naming server #7157

funky-eyes · 2025-02-11T07:14:11Z

I have registered the PR changes.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

…and TransactionInfo pages

…fo pages

YongGoose · 2025-02-11T07:21:51Z

@funky-eyes

If you're not planning to merge it right away, would it be okay if I take a look at the code and leave some comments?
Personally, I’m very interested in Spring. 🙂

funky-eyes · 2025-02-11T07:25:26Z

@funky-eyes

If you're not planning to merge it right away, would it be okay if I take a look at the code and leave some comments? Personally, I’m very interested in Spring. 🙂

Of course

namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java

YongGoose

Since I’m not familiar with the overall code of the namingserver module, my comment might not be helpful.

I’d appreciate it if you take it lightly.

Great work! 👍

YongGoose · 2025-02-11T07:39:49Z

namingserver/src/main/java/org/apache/seata/namingserver/controller/NamingController.java

        Result<String> addGroupResult = namingManager.changeGroup(namespace, vGroup, clusterName, unitName);
        if (!addGroupResult.isSuccess()) {
            return addGroupResult;
        }
        return new Result<>("200", "change vGroup " + vGroup + "to cluster " + clusterName + " successfully!");
    }

+    @GetMapping("/namespace")
+    public SingleResult<Map<String, NamespaceVO>> namespaces() {


DTO can be a good option.

DTO can be a good option.

I think DTO is a transfer object and VO is a presentation object, so I used VO. Maybe my understanding is incorrect, please point it out.

My intention was not to replace NamespaceVO with a DTO but rather to replace Map<String, NamespaceVO> with a DTO.

As you mentioned, DTO is meant for transferring data between layers, so I thought it would be appropriate.

I think this website could be helpful for our discussion.

https://stackoverflow.com/questions/1612334/difference-between-dto-vo-pojo-javabeans

Since the controller layer does not perform any actions and the VO is the data that needs to be displayed on the front end, I believe it is sufficient to use Map<String, NamespaceVO> in the response. If we return a DTO to the controller side, the controller would need to convert it to VO, which I think is redundant.

If we return a DTO to the controller side, the controller would need to convert it to VO, which I think is redundant.

Thank you for your opinion.

If the controller layer simply returns what the service layer provides, the DTO might seem unnecessary. However, when transmitting complex objects, I believe using a DTO is appropriate.

Discussions about DTOs and VOs never seem to have a definitive answer. 😅

This time, I think it would be better to follow your suggestion and use a VO to avoid redundancy.

namingserver/src/main/java/org/apache/seata/namingserver/config/WebConfig.java

namingserver/src/main/java/org/apache/seata/namingserver/controller/NamingController.java

YongGoose · 2025-02-11T07:51:12Z

namingserver/src/main/java/org/apache/seata/namingserver/controller/NamingController.java

@@ -113,14 +115,18 @@ public Result<String> changeGroup(@RequestParam String namespace,
                                      @RequestParam String clusterName,
                                      @RequestParam String unitName,
                                      @RequestParam String vGroup) {
-
        Result<String> addGroupResult = namingManager.changeGroup(namespace, vGroup, clusterName, unitName);
        if (!addGroupResult.isSuccess()) {
            return addGroupResult;
        }
        return new Result<>("200", "change vGroup " + vGroup + "to cluster " + clusterName + " successfully!");


You can use the constants from Result.

incubator-seata/common/src/main/java/org/apache/seata/common/result/Result.java

Line 27 in 2055817

public static final String SUCCESS_CODE = "200";

Or how about creating a success API like SingleResult?

Yes, but it is not within the scope of responsibility of the pr.

Yes, but it is not within the scope of responsibility of the pr.

I’ll take care of this part.

namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java

codecov · 2025-02-11T08:09:09Z

Codecov Report

Attention: Patch coverage is 21.87500% with 100 lines in your changes missing coverage. Please review.

Project coverage is 52.09%. Comparing base (679a2e0) to head (a871bf0).
Report is 2 commits behind head on 2.x.

Files with missing lines	Patch %	Lines
...ata/namingserver/filter/ConsoleRemotingFilter.java	10.52%	51 Missing ⚠️
...ache/seata/namingserver/manager/NamingManager.java	0.00%	21 Missing ⚠️
...ingserver/filter/CachedBodyHttpServletRequest.java	0.00%	14 Missing ⚠️
...ache/seata/namingserver/entity/vo/NamespaceVO.java	0.00%	9 Missing ⚠️
...seata/server/console/param/GlobalSessionParam.java	25.00%	3 Missing ⚠️
...eata/namingserver/controller/NamingController.java	50.00%	1 Missing ⚠️
...onsole/impl/file/GlobalSessionFileServiceImpl.java	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##                2.x    #7157      +/-   ##
============================================
- Coverage     52.20%   52.09%   -0.11%     
- Complexity     6817     6856      +39     
============================================
  Files          1154     1169      +15     
  Lines         41116    41443     +327     
  Branches       4820     4847      +27     
============================================
+ Hits          21464    21591     +127     
- Misses        17611    17797     +186     
- Partials       2041     2055      +14

Files with missing lines	Coverage Δ
...he/seata/namingserver/NamingserverApplication.java	`33.33% <ø> (ø)`
...rg/apache/seata/namingserver/config/WebConfig.java	`100.00% <100.00%> (ø)`
...eata/namingserver/controller/NamingController.java	`31.81% <50.00%> (-7.08%)`	⬇️
...onsole/impl/file/GlobalSessionFileServiceImpl.java	`77.77% <0.00%> (-4.58%)`	⬇️
...seata/server/console/param/GlobalSessionParam.java	`85.00% <25.00%> (-9.12%)`	⬇️
...ache/seata/namingserver/entity/vo/NamespaceVO.java	`0.00% <0.00%> (ø)`
...ingserver/filter/CachedBodyHttpServletRequest.java	`0.00% <0.00%> (ø)`
...ache/seata/namingserver/manager/NamingManager.java	`50.20% <0.00%> (-5.76%)`	⬇️
...ata/namingserver/filter/ConsoleRemotingFilter.java	`10.52% <10.52%> (ø)`

... and 11 files with indirect coverage changes

YongGoose · 2025-02-11T08:11:57Z

@funky-eyes

The Result object seemed to not effectively utilize the type received from the generic.
What do you think about this?

If you agree with my opinion, I will create an issue after resolving my current PR.

funky-eyes · 2025-02-11T08:17:07Z

@funky-eyes

The Result object seemed to not effectively utilize the type received from the generic. What do you think about this?

If you agree with my opinion, I will create an issue after resolving my current PR.

I agree with you

xjlgod · 2025-02-14T12:14:29Z

console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx

-      if (xid !== undefined) {
+      const { xid,vgroup ,namespace,cluster} = query;
+      if (xid !== undefined && vgroup !== undefined) {
+        console.log('query:', query);


Is this line used to debug?

Yes, I should delete it

xjlgod · 2025-02-14T12:23:42Z

namingserver/src/main/java/org/apache/seata/namingserver/config/WebConfig.java

+    }
+
+    @Bean
+    public FilterRegistrationBean<Filter> consoleRemotingFilter(NamingManager namingManager,


what is the use of consoleRemotingFilter

It is used to route console requests to the corresponding seata server

console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx

…ockInfo/GlobalLockInfo.tsx

xjlgod · 2025-02-15T12:34:21Z

namingserver/src/main/java/org/apache/seata/namingserver/contants/NamingConstant.java

+
+    String CONSOLE_PATTERN = "^/api/.*/console/.*";
+
+    int DEFAULT_REQUEST_TIMEOUT = 5000;


Should this config can be set by user?

Should this config can be set by user?

Theoretically, it's not necessary. I believe 5 seconds is already quite long, because the naming server and seata-server are on the same internal network, and since it's a paginated query, there isn't much data.

xjlgod

LGTM

funky-eyes added 6 commits February 8, 2025 15:44

update

b9e4c09

add console remoting filter and web configuration

29ab964

feat: add namespace, cluster, and vgroup selection to GlobalLockInfo …

f15af28

…and TransactionInfo pages

add vgroup support to GlobalSession and TransactionInfo components

750d11f

set default namespace and cluster in GlobalLockInfo and TransactionIn…

e1ca7f6

…fo pages

migrate console to naming server and update configuration

7331e88

funky-eyes added this to the 2.4.0 milestone Feb 11, 2025

funky-eyes added type: feature Category issues or prs related to feature request. module/server server module module/console module/namingserver labels Feb 11, 2025

fix ut

b0f1ce6

github-advanced-security bot found potential problems Feb 11, 2025

View reviewed changes

namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java Fixed Show fixed Hide fixed

funky-eyes added 3 commits February 11, 2025 15:52

fix ut

f620eb2

fix ut

a2136c0

fix ut

df42d2d

YongGoose approved these changes Feb 11, 2025

View reviewed changes

funky-eyes added 2 commits February 11, 2025 16:28

refactor naming server configuration and console filter

7d0c2e1

Merge branch '2.x' into console_namingserver

0bdeec0

YongGoose mentioned this pull request Feb 11, 2025

Refactoring: Refactoring the Result Class #7158

Open

funky-eyes requested review from slievrly and xjlgod February 11, 2025 10:06

Merge branch '2.x' into console_namingserver

b2e53a2

xjlgod reviewed Feb 14, 2025

View reviewed changes

funky-eyes commented Feb 15, 2025

View reviewed changes

console/src/main/resources/static/console-fe/src/pages/GlobalLockInfo/GlobalLockInfo.tsx Outdated Show resolved Hide resolved

Update console/src/main/resources/static/console-fe/src/pages/GlobalL…

05e1c7c

…ockInfo/GlobalLockInfo.tsx

xjlgod reviewed Feb 15, 2025

View reviewed changes

Merge branch '2.x' into console_namingserver

3d7fb20

slievrly self-assigned this Feb 18, 2025

Merge branch '2.x' into console_namingserver

aeb5be5

xjlgod approved these changes Feb 19, 2025

View reviewed changes

funky-eyes added 3 commits February 19, 2025 16:23

Merge branch '2.x' into console_namingserver

d89845f

Merge branch '2.x' into console_namingserver

2019ddb

Merge branch '2.x' into console_namingserver

a871bf0

funky-eyes merged commit d667466 into apache:2.x Feb 24, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: migrate the console to the naming server #7157

feature: migrate the console to the naming server #7157

funky-eyes commented Feb 11, 2025

YongGoose commented Feb 11, 2025 •

edited

Loading

funky-eyes commented Feb 11, 2025

YongGoose left a comment

YongGoose Feb 11, 2025

funky-eyes Feb 11, 2025

YongGoose Feb 11, 2025 •

edited

Loading

funky-eyes Feb 11, 2025 •

edited

Loading

YongGoose Feb 11, 2025

YongGoose Feb 11, 2025

funky-eyes Feb 11, 2025

YongGoose Feb 11, 2025

codecov bot commented Feb 11, 2025 •

edited

Loading

YongGoose commented Feb 11, 2025

funky-eyes commented Feb 11, 2025

xjlgod Feb 14, 2025

funky-eyes Feb 15, 2025

xjlgod Feb 14, 2025

funky-eyes Feb 15, 2025

xjlgod Feb 15, 2025

funky-eyes Feb 15, 2025

xjlgod left a comment


		String CONSOLE_PATTERN = "^/api/./console/.";

		int DEFAULT_REQUEST_TIMEOUT = 5000;

feature: migrate the console to the naming server #7157

feature: migrate the console to the naming server #7157

Conversation

funky-eyes commented Feb 11, 2025

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

YongGoose commented Feb 11, 2025 • edited Loading

funky-eyes commented Feb 11, 2025

YongGoose left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YongGoose Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

funky-eyes Feb 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 11, 2025 • edited Loading

Codecov Report

YongGoose commented Feb 11, 2025

funky-eyes commented Feb 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xjlgod left a comment

Choose a reason for hiding this comment

YongGoose commented Feb 11, 2025 •

edited

Loading

YongGoose Feb 11, 2025 •

edited

Loading

funky-eyes Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading