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 #11343] Simplified validation logic #11345

Merged
merged 15 commits into from
Nov 15, 2023
Merged

Conversation

985492783
Copy link
Contributor

Simplified validation logic and fix bug.
优化点

  1. Manger和多个filter过度设计。
  2. 通过拼接url+method+module,设计繁琐,且是有问题的。
  3. 通过Extractor中注册url来定位需要参数校验的请求,不合理。

修改点

  1. 定义@ParamChecker.Checker注解。将控制权转给controller,类似@Secured注解定义资源类型。
  2. 去除Manger和多个filter过度设计,用一个filter代替。
  3. filter通过ControllerMethodsCache和request定位类和方法,从方法中找Checker注解,找不到再去类中找,找不到不需要校验参数,找到了根据具体的Checker属性取出对应的Extractor并做后续处理。

Optimisation points

  1. Manger and multiple filters are overdesigned.
  2. by splicing url+method+module, the design is cumbersome and is problematic.
  3. By registering url in Extractor to locate the request that needs parameter checking, it is unreasonable.

Modification points

  1. Define @ParamChecker.Checker annotation. Transfer control to controller, similar to @Secured annotation to define resource type.
  2. remove Manger and multiple filter overdesign, replace with one filter.
  3. filter through the ControllerMethodsCache and request to locate the class and method, from the method to find Checker annotations, can not find and then go to the class to find , can not find the parameters do not need to check , found according to the specific Checker attributes to take out the corresponding Extractor and do the subsequent processing.

@985492783
Copy link
Contributor Author

link #11343

@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2023

Codecov Report

Merging #11345 (65a9889) into develop (0d46af0) will decrease coverage by 0.05%.
Report is 1 commits behind head on develop.
The diff coverage is 29.62%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #11345      +/-   ##
=============================================
- Coverage      60.50%   60.46%   -0.05%     
+ Complexity      5836     5826      -10     
=============================================
  Files            883      885       +2     
  Lines          26827    26738      -89     
  Branches        2770     2763       -7     
=============================================
- Hits           16231    16166      -65     
+ Misses          9481     9460      -21     
+ Partials        1115     1112       -3     
Files Coverage Δ
...aba/nacos/console/controller/HealthController.java 100.00% <ø> (ø)
.../nacos/console/controller/NamespaceController.java 92.30% <ø> (ø)
...acos/console/controller/ServerStateController.java 37.50% <ø> (ø)
...acos/console/controller/v2/HealthControllerV2.java 100.00% <ø> (ø)
...s/console/controller/v2/NamespaceControllerV2.java 72.72% <ø> (ø)
...e/paramcheck/ConsoleDefaultHttpParamExtractor.java 92.30% <ø> (+72.30%) ⬆️
...os/core/paramcheck/AbstractHttpParamExtractor.java 100.00% <100.00%> (+60.00%) ⬆️
...cos/core/paramcheck/AbstractRpcParamExtractor.java 100.00% <100.00%> (ø)
...check/impl/BatchInstanceRequestParamExtractor.java 4.76% <ø> (-47.42%) ⬇️
...k/impl/ConfigBatchListenRequestParamExtractor.java 7.14% <ø> (-36.61%) ⬇️
... and 32 more

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d46af0...65a9889. Read the comment docs.

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.

一个问题

注解里的参数为checker,但是指定的是参数提取用的extractor, 是不是改下名字。

@985492783
Copy link
Contributor Author

一个问题

注解里的参数为checker,但是指定的是参数提取用的extractor, 是不是改下名字。

done

@985492783 985492783 requested a review from KomachiSion November 9, 2023 09:52
@KomachiSion KomachiSion merged commit a935fa1 into alibaba:develop Nov 15, 2023
6 checks passed
@KomachiSion KomachiSion added the kind/enhancement Category issues or prs related to enhancement. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants