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

#1857 Use @ApiModelProperty required value if present instead of @NotNull #2606

Closed
wants to merge 2 commits into from
Closed

Conversation

m-radzikowski
Copy link

As stated in #1857, in this situation:

@ApiModelProperty(required = false, value = "Required only for Ipsum group")
@NotNull(groups = Ipsum.class)
private int lorem;

field lorem is marked as required, as it has @javax.validation.constraints.NotNull annotation. This change modifies behavior so if @ApiModelProperty is present, it will always be used it's required value. Swagger annotations should have precedence over other annotations.

Only drawback is that in existing code this:

@ApiModelProperty(value = "Smth")
@NotNull
private int lorem;

currently is marked as required (because of @NotNull), and after change it will be marked as not-required (as required=false is default value in @ApiModelProperty).

Every other code will act as before, so this:

@NotNull
private int lorem;

will be marked as required, since there is no Swagger annotation that would override it.

This change can be done for 2.0 version as well, so if it will be accepted I can make another PR for 2.0. Or maybe this change should go only to 2.0, as it somehow changes previous behavior. Please let me know what do you think.

@frantuma frantuma changed the base branch from master to 1.5 April 30, 2018 15:30
@frantuma
Copy link
Member

frantuma commented May 7, 2018

Thanks with your effort with this one! I don't think we can go this way actually, because of the issue you mention in your comment about required defaulting to false which wouldn't allow to use the annotation "freely". A solution similar to what done for readOnly in #2675 for 1.5 and #2695 for 2.0 should be applicable here as well, adding an annotation field named e.g. necessity (or better naming..) with related enum with values like AUTO, REQUIRED, NOT_REQUIRED, with related support in reader/resolver code, and deprecating required.

Please let me know if you can help, and/or any comment.

@MariusKl
Copy link

MariusKl commented May 20, 2020

This is still an issue with v2. I face the same issue when working around the lack of swagger supporting regex with (?i)


@Pattern(regexp = "^(?i)(new|closed)$")
@Schema(description ="In fact the field allows the pattern ^(?i)(new|closed)$ but this causes an invalid swagger document", 
pattern = "^(new|closed|NEW|CLOSED)$")
String state;

The pattern regex is used when generating the swagger documentation with the maven-swagger-plugin

"state" : {
            "pattern" : "^(?i)(new|closed)$",
            "type" : "string",

Would be great to favor the own annotations over external ones.

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.

3 participants