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

ICollection<T> support #33

Open
tomasz-soltysik opened this issue Jan 9, 2024 · 2 comments
Open

ICollection<T> support #33

tomasz-soltysik opened this issue Jan 9, 2024 · 2 comments
Assignees
Labels
question Further information is requested

Comments

@tomasz-soltysik
Copy link

Current implementation checks if parameter type is class. This check fails if we use ICollection<T> as parameter type.
ASP.NET is able to deserialize to it, it is also possible to create and register IValidator<ICollection<T>>.

What is the reason for not allowing such interfaces to be auto-validated?

In PR with refactor of IsCustomType (#32) it is proposed to change to even more strict, by disallowing anything that implements IEnumerable, which I would consider breaking change and would like to understand the reason for it.

@mvdgun mvdgun added the question Further information is requested label Jan 9, 2024
@mvdgun mvdgun self-assigned this Jan 9, 2024
@mvdgun
Copy link
Member

mvdgun commented Jan 9, 2024

Hi @tomasz-soltysik, thanks for expressing your concerns! I haven't really gotten around to reviewing the PR that you mentioned. If the merging of said PR would result in a breaking change it will not be something that will be released in the v1.x versions. If a breaking change would slip through the cracks it is something I would revert right away.

Regarding the validation of ICollection and variants; great point, I guess if FluentValidation supports validation for it we should support it as well. If anything I am of the opinion that the IsCustomType check should be more relaxed instead of more strict. I am also reluctant to relaxing the IsCustomType check in the v1.x versions since you also could argue that that would also be considered a breaking change :).

@Tihomirov-Vasiliy
Copy link

@mvdgun @tomasz-soltysik I added another commit to remove the check for IEnumerable

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

No branches or pull requests

3 participants