Skip to content

Remove @Valid even for enum types defined in components #18451 #18453

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#required}}@NotNull {{/required}}{{^isPrimitiveType}}{{^isDate}}{{^isDateTime}}{{^isString}}{{^isFile}}@Valid {{/isFile}}{{/isString}}{{/isDateTime}}{{/isDate}}{{/isPrimitiveType}}{{>beanValidationCore}}
{{#required}}@NotNull {{/required}}{{#isContainer}}{{^isPrimitiveType}}{{^isDate}}{{^isDateTime}}{{^isString}}{{^isFile}}{{^isEnumOrRef}}@Valid {{/isEnumOrRef}}{{/isFile}}{{/isString}}{{/isDateTime}}{{/isDate}}{{/isPrimitiveType}}{{/isContainer}}{{>beanValidationCore}}
Copy link
Member Author

Choose a reason for hiding this comment

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

{{#isContainer}}{{^isPrimitiveType}}{{^isDate}}{{^isDateTime}}{{^isString}}{{^isFile}}{{^isEnumOrRef}}@Valid {{/isEnumOrRef}}{{/isFile}}{{/isString}}{{/isDateTime}}{{/isDate}}{{/isPrimitiveType}}{{/isContainer}}

let's try to simplify the logic here

can you tell me what type(s) need @Valid ?

Copy link
Contributor

@CREKD127 CREKD127 Apr 24, 2024

Choose a reason for hiding this comment

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

The types I want to remove @Valid from are:

MyEnumClass: A class of type Enum that is not inline (as I mentioned in the issue, @Valid is not applied in the case of inline definitions. This behavior is preferred).
List<MyEnumClass>: A list of Enums.

Copy link
Member Author

Choose a reason for hiding this comment

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

what about other way around - what needs the @Valid annotation?

Copy link
Contributor

@CREKD127 CREKD127 Apr 24, 2024

Choose a reason for hiding this comment

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

This action is an additional measure for the main issue at #14435. In my environment, the performance of the Hibernate Validator related to Enum values has significantly decreased. Therefore, I would like to implement measures only for Enum values in this case. At the moment, I have no other requests, such as wanting to add @Valid to other items.

Copy link
Member Author

Choose a reason for hiding this comment

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

I simply wanted to simplify the logic here a bit to make it easier to maintain moving forward. (not saying your approach/change is right or wrong)

Copy link
Contributor

@CREKD127 CREKD127 Apr 25, 2024

Choose a reason for hiding this comment

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

I don't think @Valid is needed for PrimitiveType's (its wrapper types) List and Enum's List.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any progress on this one? @wing328

Copy link
Contributor

@CREKD127 CREKD127 May 3, 2024

Choose a reason for hiding this comment

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

I would like to see this implemented, what do you think?

I don't think @Valid is needed for PrimitiveType's (its wrapper types) List and Enum's List.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

when you've time this week, can you please PM me via Slack to further discuss this?

https://join.slack.com/t/openapi-generator/shared_invite/zt-12jxxd7p2-XUeQM~4pzsU9x~eGLQqX2g

Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,7 @@ Here are some companies/projects (alphabetical order) using OpenAPI Generator in
- 2023-12-10 - [UnityでOpenAPI Generatorを使う](https://www.youtube.com/watch?v=CbNwKVV5LRM) by [Soup Tori](https://www.youtube.com/@souptori8417)
- 2024-03-04 - [Generating TypeScript Types with OpenAPI for REST API Consumption](https://www.pullrequest.com/blog/generating-typescript-types-with-openapi-for-rest-api-consumption/) by [PullRequest](https://www.pullrequest.com/)
- 2024-03-07 - [Fully typed Web Apps with OpenAPI (Part 1)](https://medium.com/@gfox1984/fully-typed-web-apps-with-openapi-part-1-595d55766670) by [Guillaume Renard](https://medium.com/@gfox1984)
- 2024-03-08 - [Laravel OpenAPIによる "辛くない" スキーマ駆動開発](https://fortee.jp/phperkaigi-2024/proposal/9e2e6c38-d078-4efa-99b4-83ebf9033b34) by [KentarouTakeda](https://twitter.com/KentarouTakeda)

## [6 - About Us](#table-of-contents)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public AnotherFakeApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Client.class)
})
public Response call123testSpecialTags(@ApiParam(value = "to test uuid example value" ,required=true)@HeaderParam("uuid_test") UUID uuidTest,@ApiParam(value = "client model", required = true) @NotNull @Valid Client body,@Context SecurityContext securityContext)
public Response call123testSpecialTags(@ApiParam(value = "to test uuid example value" ,required=true)@HeaderParam("uuid_test") UUID uuidTest,@ApiParam(value = "client model", required = true) @NotNull Client body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.call123testSpecialTags(uuidTest, body, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public FakeApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Void.class)
})
public Response createXmlItem(@ApiParam(value = "XmlItem Body", required = true) @NotNull @Valid XmlItem xmlItem,@Context SecurityContext securityContext)
public Response createXmlItem(@ApiParam(value = "XmlItem Body", required = true) @NotNull XmlItem xmlItem,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.createXmlItem(xmlItem, securityContext);
}
Expand All @@ -96,7 +96,7 @@ public Response fakeOuterBooleanSerialize(@ApiParam(value = "Input boolean as po
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "Output composite", response = OuterComposite.class)
})
public Response fakeOuterCompositeSerialize(@ApiParam(value = "Input composite as post body") @Valid OuterComposite body,@Context SecurityContext securityContext)
public Response fakeOuterCompositeSerialize(@ApiParam(value = "Input composite as post body") OuterComposite body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.fakeOuterCompositeSerialize(body, securityContext);
}
Expand Down Expand Up @@ -132,7 +132,7 @@ public Response fakeOuterStringSerialize(@ApiParam(value = "Input string as post
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "Success", response = Void.class)
})
public Response testBodyWithFileSchema(@ApiParam(value = "", required = true) @NotNull @Valid FileSchemaTestClass body,@Context SecurityContext securityContext)
public Response testBodyWithFileSchema(@ApiParam(value = "", required = true) @NotNull FileSchemaTestClass body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.testBodyWithFileSchema(body, securityContext);
}
Expand All @@ -144,7 +144,7 @@ public Response testBodyWithFileSchema(@ApiParam(value = "", required = true) @N
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "Success", response = Void.class)
})
public Response testBodyWithQueryParams(@ApiParam(value = "", required = true) @QueryParam("query") @NotNull String query,@ApiParam(value = "", required = true) @NotNull @Valid User body,@Context SecurityContext securityContext)
public Response testBodyWithQueryParams(@ApiParam(value = "", required = true) @QueryParam("query") @NotNull String query,@ApiParam(value = "", required = true) @NotNull User body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.testBodyWithQueryParams(query, body, securityContext);
}
Expand All @@ -156,7 +156,7 @@ public Response testBodyWithQueryParams(@ApiParam(value = "", required = true) @
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Client.class)
})
public Response testClientModel(@ApiParam(value = "client model", required = true) @NotNull @Valid Client body,@Context SecurityContext securityContext)
public Response testClientModel(@ApiParam(value = "client model", required = true) @NotNull Client body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.testClientModel(body, securityContext);
}
Expand Down Expand Up @@ -185,7 +185,7 @@ public Response testEndpointParameters(@ApiParam(value = "None", required=true)
@io.swagger.annotations.ApiResponse(code = 400, message = "Invalid request", response = Void.class),
@io.swagger.annotations.ApiResponse(code = 404, message = "Not found", response = Void.class)
})
public Response testEnumParameters(@ApiParam(value = "Header parameter enum test (string array)" , allowableValues=">, $")@HeaderParam("enum_header_string_array") List<String> enumHeaderStringArray,@ApiParam(value = "Header parameter enum test (string)" , allowableValues="_abc, -efg, (xyz)", defaultValue="-efg")@HeaderParam("enum_header_string") String enumHeaderString,@ApiParam(value = "Query parameter enum test (string array)") @QueryParam("enum_query_string_array") @Valid List<String> enumQueryStringArray,@ApiParam(value = "Query parameter enum test (string)", allowableValues="_abc, -efg, (xyz)", defaultValue = "-efg") @DefaultValue("-efg") @QueryParam("enum_query_string") String enumQueryString,@ApiParam(value = "Query parameter enum test (double)", allowableValues="1, -2") @QueryParam("enum_query_integer") Integer enumQueryInteger,@ApiParam(value = "Query parameter enum test (double)", allowableValues="1.1, -1.2") @QueryParam("enum_query_double") Double enumQueryDouble,@ApiParam(value = "Form parameter enum test (string array)", allowableValues=">, $", defaultValue="$") @DefaultValue("$") @FormParam("enum_form_string_array") List<String> enumFormStringArray,@ApiParam(value = "Form parameter enum test (string)", allowableValues="_abc, -efg, (xyz)", defaultValue="-efg") @DefaultValue("-efg") @FormParam("enum_form_string") String enumFormString,@Context SecurityContext securityContext)
public Response testEnumParameters(@ApiParam(value = "Header parameter enum test (string array)" , allowableValues=">, $")@HeaderParam("enum_header_string_array") List<String> enumHeaderStringArray,@ApiParam(value = "Header parameter enum test (string)" , allowableValues="_abc, -efg, (xyz)", defaultValue="-efg")@HeaderParam("enum_header_string") String enumHeaderString,@ApiParam(value = "Query parameter enum test (string array)") @QueryParam("enum_query_string_array") List<String> enumQueryStringArray,@ApiParam(value = "Query parameter enum test (string)", allowableValues="_abc, -efg, (xyz)", defaultValue = "-efg") @DefaultValue("-efg") @QueryParam("enum_query_string") String enumQueryString,@ApiParam(value = "Query parameter enum test (double)", allowableValues="1, -2") @QueryParam("enum_query_integer") Integer enumQueryInteger,@ApiParam(value = "Query parameter enum test (double)", allowableValues="1.1, -1.2") @QueryParam("enum_query_double") Double enumQueryDouble,@ApiParam(value = "Form parameter enum test (string array)", allowableValues=">, $", defaultValue="$") @DefaultValue("$") @FormParam("enum_form_string_array") List<String> enumFormStringArray,@ApiParam(value = "Form parameter enum test (string)", allowableValues="_abc, -efg, (xyz)", defaultValue="-efg") @DefaultValue("-efg") @FormParam("enum_form_string") String enumFormString,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.testEnumParameters(enumHeaderStringArray, enumHeaderString, enumQueryStringArray, enumQueryString, enumQueryInteger, enumQueryDouble, enumFormStringArray, enumFormString, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public FakeClassnameTestApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Client.class)
})
public Response testClassname(@ApiParam(value = "client model", required = true) @NotNull @Valid Client body,@Context SecurityContext securityContext)
public Response testClassname(@ApiParam(value = "client model", required = true) @NotNull Client body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.testClassname(body, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public PetApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Void.class),
@io.swagger.annotations.ApiResponse(code = 405, message = "Invalid input", response = Void.class)
})
public Response addPet(@ApiParam(value = "Pet object that needs to be added to the store", required = true) @NotNull @Valid Pet body,@Context SecurityContext securityContext)
public Response addPet(@ApiParam(value = "Pet object that needs to be added to the store", required = true) @NotNull Pet body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.addPet(body, securityContext);
}
Expand Down Expand Up @@ -107,7 +107,7 @@ public Response deletePet(@ApiParam(value = "Pet id to delete", required = true)
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Pet.class, responseContainer = "List"),
@io.swagger.annotations.ApiResponse(code = 400, message = "Invalid status value", response = Void.class)
})
public Response findPetsByStatus(@ApiParam(value = "Status values that need to be considered for filter", required = true) @QueryParam("status") @NotNull @Valid List<String> status,@Context SecurityContext securityContext)
public Response findPetsByStatus(@ApiParam(value = "Status values that need to be considered for filter", required = true) @QueryParam("status") @NotNull List<String> status,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.findPetsByStatus(status, securityContext);
}
Expand Down Expand Up @@ -161,7 +161,7 @@ public Response getPetById(@ApiParam(value = "ID of pet to return", required = t
@io.swagger.annotations.ApiResponse(code = 404, message = "Pet not found", response = Void.class),
@io.swagger.annotations.ApiResponse(code = 405, message = "Validation exception", response = Void.class)
})
public Response updatePet(@ApiParam(value = "Pet object that needs to be added to the store", required = true) @NotNull @Valid Pet body,@Context SecurityContext securityContext)
public Response updatePet(@ApiParam(value = "Pet object that needs to be added to the store", required = true) @NotNull Pet body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.updatePet(body, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public Response getOrderById(@ApiParam(value = "ID of pet that needs to be fetch
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Order.class),
@io.swagger.annotations.ApiResponse(code = 400, message = "Invalid Order", response = Void.class)
})
public Response placeOrder(@ApiParam(value = "order placed for purchasing the pet", required = true) @NotNull @Valid Order body,@Context SecurityContext securityContext)
public Response placeOrder(@ApiParam(value = "order placed for purchasing the pet", required = true) @NotNull Order body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.placeOrder(body, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public UserApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Void.class)
})
public Response createUser(@ApiParam(value = "Created user object", required = true) @NotNull @Valid User body,@Context SecurityContext securityContext)
public Response createUser(@ApiParam(value = "Created user object", required = true) @NotNull User body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.createUser(body, securityContext);
}
Expand Down Expand Up @@ -152,7 +152,7 @@ public Response logoutUser(@Context SecurityContext securityContext)
@io.swagger.annotations.ApiResponse(code = 400, message = "Invalid user supplied", response = Void.class),
@io.swagger.annotations.ApiResponse(code = 404, message = "User not found", response = Void.class)
})
public Response updateUser(@ApiParam(value = "name that need to be deleted", required = true) @PathParam("username") @NotNull String username,@ApiParam(value = "Updated user object", required = true) @NotNull @Valid User body,@Context SecurityContext securityContext)
public Response updateUser(@ApiParam(value = "name that need to be deleted", required = true) @PathParam("username") @NotNull String username,@ApiParam(value = "Updated user object", required = true) @NotNull User body,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.updateUser(username, body, securityContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ public EnumTest outerEnum(OuterEnum outerEnum) {
**/
@JsonProperty(value = "outerEnum")
@ApiModelProperty(value = "")
@Valid

public OuterEnum getOuterEnum() {
return outerEnum;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public FileSchemaTestClass _file(ModelFile _file) {
**/
@JsonProperty(value = "file")
@ApiModelProperty(value = "")
@Valid

public ModelFile getFile() {
return _file;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public FormatTest number(BigDecimal number) {
**/
@JsonProperty(value = "number")
@ApiModelProperty(required = true, value = "")
@NotNull @Valid @DecimalMin("32.1") @DecimalMax("543.2")
@NotNull @DecimalMin("32.1") @DecimalMax("543.2")
public BigDecimal getNumber() {
return number;
}
Expand Down Expand Up @@ -387,7 +387,7 @@ public FormatTest bigDecimal(BigDecimal bigDecimal) {
**/
@JsonProperty(value = "BigDecimal")
@ApiModelProperty(value = "")
@Valid

public BigDecimal getBigDecimal() {
return bigDecimal;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public NumberOnly justNumber(BigDecimal justNumber) {
**/
@JsonProperty(value = "JustNumber")
@ApiModelProperty(value = "")
@Valid

public BigDecimal getJustNumber() {
return justNumber;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public OuterComposite myNumber(BigDecimal myNumber) {
**/
@JsonProperty(value = "my_number")
@ApiModelProperty(value = "")
@Valid

public BigDecimal getMyNumber() {
return myNumber;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public Pet category(Category category) {
**/
@JsonProperty(value = "category")
@ApiModelProperty(value = "")
@Valid

public Category getCategory() {
return category;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public TypeHolderDefault numberItem(BigDecimal numberItem) {
**/
@JsonProperty(value = "number_item")
@ApiModelProperty(required = true, value = "")
@NotNull @Valid
@NotNull
public BigDecimal getNumberItem() {
return numberItem;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public TypeHolderExample numberItem(BigDecimal numberItem) {
**/
@JsonProperty(value = "number_item")
@ApiModelProperty(example = "1.234", required = true, value = "")
@NotNull @Valid
@NotNull
public BigDecimal getNumberItem() {
return numberItem;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public XmlItem attributeNumber(BigDecimal attributeNumber) {
**/
@JsonProperty(value = "attribute_number")
@ApiModelProperty(example = "1.234", value = "")
@Valid

public BigDecimal getAttributeNumber() {
return attributeNumber;
}
Expand Down Expand Up @@ -318,7 +318,7 @@ public XmlItem nameNumber(BigDecimal nameNumber) {
**/
@JsonProperty(value = "name_number")
@ApiModelProperty(example = "1.234", value = "")
@Valid

public BigDecimal getNameNumber() {
return nameNumber;
}
Expand Down Expand Up @@ -454,7 +454,7 @@ public XmlItem prefixNumber(BigDecimal prefixNumber) {
**/
@JsonProperty(value = "prefix_number")
@ApiModelProperty(example = "1.234", value = "")
@Valid

public BigDecimal getPrefixNumber() {
return prefixNumber;
}
Expand Down Expand Up @@ -590,7 +590,7 @@ public XmlItem namespaceNumber(BigDecimal namespaceNumber) {
**/
@JsonProperty(value = "namespace_number")
@ApiModelProperty(example = "1.234", value = "")
@Valid

public BigDecimal getNamespaceNumber() {
return namespaceNumber;
}
Expand Down Expand Up @@ -726,7 +726,7 @@ public XmlItem prefixNsNumber(BigDecimal prefixNsNumber) {
**/
@JsonProperty(value = "prefix_ns_number")
@ApiModelProperty(example = "1.234", value = "")
@Valid

public BigDecimal getPrefixNsNumber() {
return prefixNsNumber;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public AnotherFakeApi(@Context ServletConfig servletContext) {
@io.swagger.annotations.ApiResponses(value = {
@io.swagger.annotations.ApiResponse(code = 200, message = "successful operation", response = Client.class)
})
public Response call123testSpecialTags(@ApiParam(value = "client model", required = true) @NotNull @Valid Client client,@Context SecurityContext securityContext)
public Response call123testSpecialTags(@ApiParam(value = "client model", required = true) @NotNull Client client,@Context SecurityContext securityContext)
throws NotFoundException {
return delegate.call123testSpecialTags(client, securityContext);
}
Expand Down
Loading
Loading