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

IdCardValidator 不支持null值返回true #479

Closed
xyfy opened this issue Mar 3, 2023 · 8 comments · Fixed by #485, masastack/MASA.Docs#132 or #497
Closed

IdCardValidator 不支持null值返回true #479

xyfy opened this issue Mar 3, 2023 · 8 comments · Fixed by #485, masastack/MASA.Docs#132 or #497
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@xyfy
Copy link
Contributor

xyfy commented Mar 3, 2023

@zhenlei520

没有找到一种可以标记可选的办法,理论上应该是Idcard在null或者空的情况下通过校验,靠notempty/notnull来控制是否可空,但是目前IdCardValidator没有

Originally posted by @xyfy in masastack/MASA.Auth#730 (comment)

see the EmailValidator of fluentvalidation
it will return true when value is null

@doddgu
Copy link
Contributor

doddgu commented Mar 5, 2023

我认为可以增加 AllowNull,AllowNullOrEmpty 的属性来控制validation内部判断逻辑。理论上来说验证格式就要符合格式要求,明显null和empty不符合格式要求,返回失败是正确的。但如果明确指定可以为null或empty,那一样返回验证成功只是可选项,而不是默认逻辑。

@xyfy
Copy link
Contributor Author

xyfy commented Mar 6, 2023

我认为可以增加 AllowNull,AllowNullOrEmpty 的属性来控制validation内部判断逻辑。理论上来说验证格式就要符合格式要求,明显null和empty不符合格式要求,返回失败是正确的。但如果明确指定可以为null或empty,那一样返回验证成功只是可选项,而不是默认逻辑。

[fluentvalidation]内置了NotNull NotEmpty 这两个校验器,就默认情况它的内置校验器都只校验非null值,可能因为null值没有规则校验意义吧,除非加上这两个之一才会校验非空。 按照Webapi正常而言,如果一个字段没有值,会被是赋予null,如果要逆转这个规则,可以通过这个属性

@zhenlei520
Copy link
Contributor

zhenlei520 commented Mar 6, 2023

按照我们的理解,当需要验证身份证号时,它不应该为null或者empty,但考虑与Fluentvalidation的验证规则冲突,会增加额外的学习成本,这里改为:

当value为null时,验证通过,但当value为empty时,它需要进行验证而不是验证通过,这与Fluentvalidation的规则一致

@doddgu @xyfy

@doddgu
Copy link
Contributor

doddgu commented Mar 6, 2023

可以,就这样吧。

@zhenlei520
Copy link
Contributor

@xyfy 你那边方便改一下吗?除了IdCardValidator之外,还有PhoneValidator也有同样的问题

@xyfy
Copy link
Contributor Author

xyfy commented Mar 6, 2023

@xyfy xyfy self-assigned this Mar 6, 2023
@xyfy xyfy added the enhancement New feature or request label Mar 6, 2023
@xyfy
Copy link
Contributor Author

xyfy commented Mar 6, 2023

但是如果这里改了的话,其它已经调用的地方,就得加上notnull来保证校验逻辑的一致性,这个算BreakChange? @zhenlei520

@zhenlei520
Copy link
Contributor

但是如果这里改了的话,其它已经调用的地方,就得加上notnull来保证校验逻辑的一致性,这个算BreakChange? @zhenlei520

是的,需要在发版描述以及1.0.0 文档的升级指南中写清楚

@xyfy xyfy added this to the 1.0.0 milestone Mar 6, 2023
xyfy added a commit that referenced this issue Mar 7, 2023
1. add `SetValidatorWithCondition` and `WhenIdCard` and `WhenPhone`  extension method for optional validator.
2. IdCardValidator and PhoneValidator will pass when given an NULL value
3. adujst unit test case
4. change PasswordValidator base class to `MasaRegularExpressionValidator`

Closes #479

BREAKING CHANGE:

IdCardValidator and PhoneValidator will pass when given an NULL value , consistent with the logic of the official built-in validator
xyfy added a commit that referenced this issue Mar 8, 2023
FluentValidation  忽略Null值
内置正则表达式校验空字符串
提供WhenNotEmpty方法

close: #479
xyfy added a commit that referenced this issue Mar 8, 2023
FluentValidation  忽略Null值
内置正则表达式校验空字符串
提供WhenNotEmpty方法

close: #479
@xyfy xyfy linked a pull request Mar 8, 2023 that will close this issue
zhenlei520 pushed a commit that referenced this issue Mar 8, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* feat: Utils: FluentValidation 校验逻辑修正  #479

FluentValidation  忽略Null值
内置正则表达式校验空字符串
提供WhenNotEmpty方法

close: #479

* fix nullable alert

* remove useless code

* reflactor: reflactor code

* fix: set the nullable param
@xyfy xyfy linked a pull request Mar 15, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment