Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #12017] Add console backend unit tests and fix bugs #12622

Open
wants to merge 19 commits into
base: summer-ospp#12017
Choose a base branch
from

Conversation

RickonZhang0929
Copy link

What is the purpose of the change

For 12017

Add console backend unit tests and fix bugs

Brief changelog

  • Add unit tests for all sections

  • Add getAllSubClientConfigByIp method in config section

  • Fix bugs in AuthFilter and RemoteRequestAuthFilter

  • Fix bugs in ServiceProxy

Verifying this change

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

* Add service handling module

* Add instance handling module
* Add user handling module

* Add role handling module

* Add permission handling module
* Fix the error by adding ApiType
…oller

* Refactor the old version of the console's controller
* Updated ControllerV3 for auth section
* Add ApiType annotation

* Transfer the updateCluster method

* Fix Compilation Errors
* Add unit tests for all sections
* Add getAllSubClientConfigByIp method in config section

* Fix bugs in AuthFilter and RemoteRequestAuthFilter

* Fix bugs in ServiceProxy
Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. ApiType别漏了, 都检查一下
  2. Service的API,也全都检查一下, 逻辑上应该是namespaceId,groupName,serviceName 3个要素, 有的API抄的老v1 API,只有namespaceId和serviceName,groupName包含在serviceName中,这个是要修改的,不能保留下来。
  3. HTTP已经规定的状态吗,不需要重新定义,直接用spring的对应常量即可。

/**
* Conflict error.
*/
CONFLICT(409, "Conflict"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP的通用状态码不需要重新定义。

@@ -64,6 +66,11 @@ public Result<Map<String, String>> serverState() {
@GetMapping("/announcement")
public Result<String> getAnnouncement(
@RequestParam(required = false, name = "language", defaultValue = "zh-CN") String language) {
// Validate the language parameter
List<String> supportedLanguages = Arrays.asList("zh-CN", "en-US");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个可以做成常量。

* Get subscribe information from client side.
*/
@GetMapping("/listener/ip")
@Secured(resource = Constants.LISTENER_CONTROLLER_PATH, action = ActionTypes.READ, signType = SignType.CONFIG)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiType又漏了?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

全都再检查一下,好像其他也有漏的。

@@ -227,6 +229,7 @@ public Object getServiceList(@RequestParam(required = false) boolean withInstanc
@GetMapping()
public Object getServiceDetail(@RequestParam(defaultValue = Constants.DEFAULT_NAMESPACE_ID) String namespaceId,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

groupName呢?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants