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

Support runtime property attributes via TypeDescriptor #53

Merged

Conversation

warappa
Copy link
Contributor

@warappa warappa commented Oct 11, 2023

This adds support for property attributes configured via TypeDescriptor.

The code for PropertyOverridingTypeDescriptor is taken from StackOverflow as stated in the comment.

This should fix #52.

Copy link
Owner

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

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

Thanks very much for this. A few small requests.

tests/MiniValidation.UnitTests/TypeDescriptorUtils.cs Outdated Show resolved Hide resolved
@warappa
Copy link
Contributor Author

warappa commented Oct 13, 2023

BTW: Is the dependency on version 5.0.0 of System.ComponentModel.Annotation for .NET Standard 2.0 OK, or should it be a lower version?

@DamianEdwards
Copy link
Owner

BTW: Is the dependency on version 5.0.0 of System.ComponentModel.Annotation for .NET Standard 2.0 OK, or should it be a lower version?

This is fine. It's the latest version and supports netstandard2.0.

Feel free to bump the version in src/Directory.Build.props to 0.9.0 too.

@DamianEdwards DamianEdwards self-requested a review October 13, 2023 14:03
Copy link
Owner

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

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

Feel free to bump version to 0.9.0.

@warappa
Copy link
Contributor Author

warappa commented Oct 13, 2023

Please don't yet merge, I think I found a double-report of validation errors for normal attributes.

@warappa
Copy link
Contributor Author

warappa commented Oct 13, 2023

TypeDescriptor seems to also report the normal attributes.
The weird thing is, that with a static Required attribute there is no double-report. But with a MaxLength attribute it was reported twice.

Anyway, with the previous commit the static attributes are now only considered once.

@DamianEdwards
Copy link
Owner

Required has special treatment in Validator to allow for early exit if it fails so my guess is that's what caused the behavior you saw.

@DamianEdwards
Copy link
Owner

@warappa is this ready to go then?

@warappa
Copy link
Contributor Author

warappa commented Oct 15, 2023

@DamianEdwards Yes, it's ready

@DamianEdwards DamianEdwards merged commit ce56e98 into DamianEdwards:main Oct 15, 2023
1 check passed
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.

Support for TypeDescriptor
2 participants