papsign / Ktor-OpenAPI-Generator

Ktor OpenAPI/Swagger 3 Generator
Apache License 2.0
241 stars 42 forks source link

Add validators for strings #57

Closed bargergo closed 4 years ago

bargergo commented 4 years ago

String length can be validated using annotations (Length, MaxLength or MinLength)

Wicpar commented 4 years ago

I'm not quite sure how to edit your code while preserving your contribution history...

bargergo commented 4 years ago

Please let me know if more changes are needed, or something is missing.

Wicpar commented 4 years ago

Hi @bargergo , Sorry for the delay, I went a bit over my head on a freelance mission and am on the brink of falling behind schedule, it may take until next Wednesday to get any spare time, is it essential to you that the changes are added to the main repo before then ?

bargergo commented 4 years ago

No problem, it can wait until next Wednesday.

I have some more ideas:

Can I add them to this PR?

Wicpar commented 4 years ago

Yes those changes would be great :)

bargergo commented 4 years ago

Thanks for the merge, but I think there is one issue that should be solved somehow.

The annotation parameters with default values (errorMessage) are ignored when the annotation is used on a type and no value is specified. This leads to a runtime error when starting the application, because these parameters are missing from the annotation, when it is processed.

Solution 1: Remove type from annotation targets in validation annotations, so only properties can annotated Solution 2: The user of the annotations should always specify all parameters, when annotating type

I think the first solution would be better, because users tend to forget such rules and I don't know if there is a reason to keep the option to annotate types.

Wicpar commented 4 years ago

It would not be the first time this library gets the kotlin team to work on their bugs. Option 3: create an issue: https://youtrack.jetbrains.com/issue/KT-39369

bargergo commented 4 years ago

Ok, this option didn't occur to me, but yes, this can be a good solution. :)