Add validators for strings #57
Conversation
Wicpar
left a comment
Wicpar
left a comment
There was a problem hiding this comment.
It seems OK to me except for the added model, as it won't work because the model generated for string is an instance of SchemaModelLitteral so it will always fail at if (model is SchemaModel.SchemaModelString && types.contains(type))
To test it the best is to launch the test server from the tests
|
I'm not quite sure how to edit your code while preserving your contribution history... |
bargergo
changed the title
bargergo
changed the title
|
Please let me know if more changes are needed, or something is missing. |
|
Hi @bargergo , Sorry for the delay, |
|
No problem, it can wait until next Wednesday. I have some more ideas:
Can I add them to this PR? |
|
Yes those changes would be great :) |
bargergo
force-pushed
the
string-length-validators
branch
from
2948bbc to
a3c2171
Compare
| class B(val i: @Min(0) @Max(2) Int) : Base() | ||
| class B(@Min(0) @Max(2) val i: Int) : Base() | ||
|
|
||
| @WithExample | ||
| class C(val l: @Clamp(0, 10) Long) : Base() { | ||
| class C( @Clamp(0, 10) val l: Long) : Base() { |
There was a problem hiding this comment.
Were these annotations used on purpose between the name and the type of the constructor parameter? I modified this, because it caused problems. This way the annotation instance didn't have an errorMessage property, only a value property.
There was a problem hiding this comment.
Yes, at the beginning the annotations were only supported on types due to some issue, i believe it is fixed now and didn't really change that to the more natural syntax.
There was a problem hiding this comment.
Ok, it was new for me. I didn't see any annotations on types before.
It seems like the annotation parameters with default values are ignored if I use the annotation on the type.
It causes this error on start:
kotlin.reflect.jvm.internal.KotlinReflectionInternalError: Method is not supported: public abstract java.lang.String com.papsign.ktor.openapigen.annotations.type.string.length.Length.errorMessage() (args: [])
In the getConstraint method of the LengthProcessor class I can see with the debugger, that there are values for min and max only, the errorMessage is missing.
|
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 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. |
|
It would not be the first time this library gets the kotlin team to work on their bugs. |
|
Ok, this option didn't occur to me, but yes, this can be a good solution. :) |
2 participants
String length can be validated using annotations (Length, MaxLength or MinLength)