Skip to content

fix validate violations is swallowed by ValidationException for issue#10415#10434

Closed
zhouxinlei828 wants to merge 2 commits intoapache:3.0from
zhouxinlei828:3.0-fix-validation
Closed

fix validate violations is swallowed by ValidationException for issue#10415#10434
zhouxinlei828 wants to merge 2 commits intoapache:3.0from
zhouxinlei828:3.0-fix-validation

Conversation

@zhouxinlei828
Copy link

@zhouxinlei828 zhouxinlei828 commented Aug 10, 2022

What is the purpose of the change

fix validate violations is swallowed by ValidationException

Brief changelog

Verifying this change

Checklist

  • Make sure there is a GitHub_issue field 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.
  • 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.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • 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 sample in dubbo samples project.
  • Add some description to dubbo-website project if you are requesting to add a feature.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

@zhouxinlei828
Copy link
Author

zhouxinlei828 commented Aug 10, 2022

fix issue#10415

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022

Codecov Report

Merging #10434 (55e77e1) into 3.0 (1bc5e62) will decrease coverage by 0.03%.
The diff coverage is 60.00%.

@@             Coverage Diff              @@
##                3.0   #10434      +/-   ##
============================================
- Coverage     65.68%   65.65%   -0.04%     
- Complexity      296      318      +22     
============================================
  Files          1234     1234              
  Lines         53913    53913              
  Branches       8155     8156       +1     
============================================
- Hits          35415    35395      -20     
- Misses        14645    14670      +25     
+ Partials       3853     3848       -5     
Impacted Files Coverage Δ
.../validation/support/jvalidation/JValidatorNew.java 0.00% <0.00%> (ø)
...bbo/validation/support/jvalidation/JValidator.java 70.62% <100.00%> (-1.25%) ⬇️
.../apache/dubbo/rpc/filter/ProfilerServerFilter.java 62.85% <0.00%> (-20.00%) ⬇️
...dubbo/remoting/exchange/support/DefaultFuture.java 87.93% <0.00%> (-4.32%) ⬇️
.../apache/dubbo/rpc/protocol/injvm/InjvmInvoker.java 63.56% <0.00%> (-3.88%) ⬇️
...org/apache/dubbo/rpc/protocol/AbstractInvoker.java 66.97% <0.00%> (-3.67%) ⬇️
...he/dubbo/remoting/transport/netty/NettyServer.java 70.17% <0.00%> (-3.51%) ⬇️
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java 88.09% <0.00%> (-2.39%) ⬇️
...exchange/support/header/HeaderExchangeHandler.java 61.06% <0.00%> (-1.77%) ⬇️
... and 8 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

}
} catch (ValidationException e) {
// only use exception's message to avoid potential serialization issue
throw new ValidationException(e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

javax.validation.ConstraintViolationException might have potential serialization issue. Should be converted.

@zhouxinlei828 zhouxinlei828 requested a review from AlbumenJ August 11, 2022 05:36
@liufeiyu1002
Copy link
Contributor

ConstraintViolationImpl 不同版本字段不一样, 新版本新增了字段,如果单纯这么包装的话 意义不大,只是拿到了你认为的有效信息, 最起码新的结果要实现 ConstraintViolation 接口

@chickenlj
Copy link
Contributor

Check conclustion in #10415

@chickenlj chickenlj closed this Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants