-
Notifications
You must be signed in to change notification settings - Fork 13.2k
[issue #13777] Fix improper exception handling and some other basic coding issues #13782
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
Conversation
- 修改"un registered"为"unregistered",规范日志信息表述 - 修改错误的变量定义
- NamespaceOperationService将检查命名空间是否存在方法抽离出来,存在返回true,不存在返回false,使代码更加清晰。 - 修正一些因此修改产生变化的代码逻辑
- 优化了 NamespaceOperationService 类中的namespaceExists方法异常打印内容 - 改进了 NamespaceValidationRequestFilter 中的命名空间存在性检查逻辑,将原先逻辑删除,在filter方法里直接调用namespaceOperationService.namespaceExists方法即可
| @Service | ||
| public class NamespaceOperationService { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please dont change the indent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, the code indentation has been reverted to its original state.
- 调整代码缩进和空格,与原来保持一致
- 调整代码缩进和空格,与原来保持一致
|
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
|
|
- 重构 namespaceExists 方法,try-catch包裹默认namespace判断逻辑 - 将原先在NamespaceValidationRequestFilterTest类中的在校验命名空间时报错测试,改到NamespaceOperationServiceTest,因为现在异常处理和NamespaceValidationRequest类无关,和NamespaceOperationService类有关 - 在改到NamespaceOperationServiceTest类中增加异常处理测试用例,验证数据库查询失败时的返回结果 - 在NamespaceOperationServiceTest类中添加默认命名空间和非默认命名空间存在性验证的测试用例
|
The Maven test error in GitHub Actions has been resolved. The main issue was that the code logic for verifying the existence of namespaces has changed, and the original test method could no longer meet the testing requirements. |
|
@iggzq unit test can't pass. Please fix. |
|
|
- 删除了 checkNamespaceIdExistWithException 测试方法。因为判断命名空间是否存在的逻辑已经发生改变,namespaceInnerHandler.checkNamespaceIdExist此方法无需再捕获NacosApiException异常来设置true或false,所以此测试方法可以直接删除
Now it should be completely resolved:
|
|
PR merged, @iggzq Would you mind provider an dingtalk number to invite you join Nacos contributor group. |
ok,my dingtalk number is gaozongqi1024 |
|
|
What is the purpose of the change
FIX #13777: Fix improper exception handling and other minor issues
Brief changelog
NacosBootstrapclass. The correct name should be SPRING_JMX_ENABLED.GrpcRequestAcceptorclass. The incorrect message "un registered" has been corrected to "unregistered".NamespaceOperationServiceclass into two separate methods: namespaceExists and validateNamespaceNotExists. This resolves the issue of improper exception catching in other classes caused by misuse of the original method.NamespaceValidationRequestFilterclass to adapt to the new logic for checking namespace existence.NamespaceInnerHandlerclass to align with the new namespace existence checking logic.NamespaceInnerHandlerTestandNamespaceValidationRequestFilterTestto reflect the updated namespace existence checking logic.