-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Automatically add required if a field is @notNull or @NotBlank #2817
Comments
@bnasslahsen This appears to have broken the fix for #2787 as it has returned to only considering the mandatory/optional status of a child field, and not whether the parent object is mandatory/optional. #2787 intended to prevent a field being shown as mandatory if its parent field was not mandatory in a |
It is expected. I have reviewed your PR, to only consider this behaviour for annotations on the child fields levels, and not on the parent Object level. There is no defined Java rules for that. |
I'm not sure what you mean by this as it isn't a Java requirement, but a combinatorial logic problem. It may not be stated in a specification but is an implication of the process being followed, namely that in a collapsed structure, something being non-mandatory immediately implies that all child elements are non-mandatory, even if they are directly specified as mandatory. If it helps, consider the same scenario for 'hidden' fields: a child field cannot be made visible in a collapsed structure if the parent field isn't visible. We have the convenience of being able to short-circuit any further processing on hidden fields but would need to continue processing in the case of the mandatory/non-mandatory status, but would need to end up with a similar outcome. I think this issue is actually pointing out the same problem as I'd raised in the |
The problem is in your combinatorial logic. From the Java point of view, there is no relation between the method parameters annotations and the exploded parameters definitions. |
I have reverted all the changes from your PR, from what i see, not all the use cases are well descibed. We need to have a formal expected test cases, something similar to the following:
Can you formalize / review the expected use cases first ? Otherwise, we leave the simple way, which is only consider required on nested fields only when required is defined with |
I'm not sure how I've created the table below based on the following models that cover all the cases I think you've specified: public record MultiFieldParameterObject(
@Valid @Schema(requiredMode = RequiredMode.REQUIRED) @NotNull MultiFieldNestedParameterObject requiredNotNullParameterObject,
@Valid @Schema(requiredMode = RequiredMode.REQUIRED) MultiFieldNestedParameterObject requiredNoValidationParameterObject,
@Valid @Schema(requiredMode = RequiredMode.NOT_REQUIRED) @NotNull MultiFieldNestedParameterObject notRequiredNotNullParameterObject,
@Valid @Schema(requiredMode = RequiredMode.NOT_REQUIRED) MultiFieldNestedParameterObject notRequiredNoValidationParameterObject,
@Valid @NotNull MultiFieldNestedParameterObject noSchemaNotNullParameterObject,
@Valid MultiFieldNestedParameterObject noSchemaNoValidationParameterObject) {
}
public record MultiFieldNestedParameterObject (
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) @NotNull String requiredNotNullField,
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) String requiredNoValidationField,
@Schema(requiredMode = RequiredMode.NOT_REQUIRED) @NotNull String notRequiredNotNullField,
@Schema(requiredMode = RequiredMode.NOT_REQUIRED) String notRequiredNoValidationField,
@NotNull String noSchemaNotNullField,
String noSchemaNoValidationField) {
}
|
Fixes springdoc#2817" This reverts commit 39ef9f4.
To make sure i share the same understanding.
This is the revised version. I have highlighted some results in bold, as they should be considered as priority if they are explicitly defined by the user on the field level
|
Yes, I'd agree the line you've called-out matches the initial report. I'd called it My assumptions when drawing up the table:
I notice that your updated table uses the |
…doc#2817 When creating the flattened parameter definitions for an item annotated with @ParameterObject, the @Schema and @Property annotations on any fields prior to the final child-node of each branch of the parameter tree are not taken into consideration. This results in the field name in the code being used even where annotations may override that, prevents the fields being hidden where one of the parent fields is marked as hidden but the child field isn't hidden, and marks a field as mandatory even where the field would only be mandatory if the parent object had been declared through a sibling of the target field being set. To overcome this, whilst the flattened parameter map is being built, each field is now being inspected for a @parameter annotation or - where the @parameter annotation isn't found - a @Schema annotation. Where a custom name is included in either of those annotations then the overridden field name is used in the flattened definition. If either annotation declares the field as hidden then the field and all child fields are removed from the resulting definition, and a field is no longer set as mandatory unless the parent field was also declared as mandatory, or resolved as non-null and was set in an automatic state, or the child field is specifically set as required in the Schema or Parameter annotation.
I've raised #2832 which is basically just re-applying #2788 but with an additional line to handle the forced |
Thanks @mc1arke, I am currently a little sick. |
…doc#2817 When creating the flattened parameter definitions for an item annotated with @ParameterObject, the @Schema and @Property annotations on any fields prior to the final child-node of each branch of the parameter tree are not taken into consideration. This results in the field name in the code being used even where annotations may override that, prevents the fields being hidden where one of the parent fields is marked as hidden but the child field isn't hidden, and marks a field as mandatory even where the field would only be mandatory if the parent object had been declared through a sibling of the target field being set. To overcome this, whilst the flattened parameter map is being built, each field is now being inspected for a @parameter annotation or - where the @parameter annotation isn't found - a @Schema annotation. Where a custom name is included in either of those annotations then the overridden field name is used in the flattened definition. If either annotation declares the field as hidden then the field and all child fields are removed from the resulting definition, and a field is no longer set as mandatory unless the parent field was also declared as mandatory, or resolved as non-null and was set in an automatic state, or the child field is specifically set as required in the Schema or Parameter annotation.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [nu.ndw.nls.locationdataissuesapi:client-feign](https://dev.azure.com/ndwnu/NLS/_git/nls-location-data-issues) ([source](https://dev.azure.com/ndwnu/NLS/_git/nls-location-data-issues)) | compile | patch | `1.0.17` -> `1.0.18` | | [org.springdoc:springdoc-openapi-starter-webmvc-ui](https://springdoc.org/) ([source](https://github.com/springdoc/springdoc-openapi)) | compile | minor | `2.7.0` -> `2.8.0` | --- ### Release Notes <details> <summary>springdoc/springdoc-openapi (org.springdoc:springdoc-openapi-starter-webmvc-ui)</summary> ### [`v2.8.0`](https://github.com/springdoc/springdoc-openapi/blob/HEAD/CHANGELOG.md#280---2025-01-03) [Compare Source](springdoc/springdoc-openapi@v2.7.0...v2.8.0) ##### Added - [#​2790](springdoc/springdoc-openapi#2790) - Moving to OpenAPI 3.1 as the default implementation for springdoc-openapi - [#​2817](springdoc/springdoc-openapi#2817) - Obey annotations when flattening ParameterObject fields - [#​2826](springdoc/springdoc-openapi#2826) - Make it possible to mark parameters with [@​RequestParam](https://github.com/RequestParam) annotation to be sent in form instead of query. - [#​2822](springdoc/springdoc-openapi#2822) - Support returning null in ParameterCustomizer - [#​2830](springdoc/springdoc-openapi#2830) - Add support for deprecated fields. - [#​2780](springdoc/springdoc-openapi#2780) - Add Security Schema by AutoConfigure ##### Changed - Upgrade spring-boot to 3.4.1 - Upgrade spring-cloud-function to 4.2.0 - Upgrade swagger-core to 2.2.27 ##### Fixed - [#​2804](springdoc/springdoc-openapi#2804) - Stable release 2.7.0 depends on Spring Cloud Milestone 4.2.0-M1 - [#​2828](springdoc/springdoc-openapi#2828) - Required a bean of type 'org.springframework.data.rest.webmvc.mapping.Associations' that could not be found. - [#​2823](springdoc/springdoc-openapi#2823) - Capturing pattern in identical paths only renders the path element of one method - [#​2817](springdoc/springdoc-openapi#2817) - Automatically add required if a field is [@​notNull](https://github.com/notNull) or [@​NotBlank](https://github.com/NotBlank). - [#​2814](springdoc/springdoc-openapi#2814) - An unresolvable circular reference with management.endpoint.gateway.enabled=true. - [#​2798](springdoc/springdoc-openapi#2798) - Object schema generated for Unit Kotlin type. - [#​2797](springdoc/springdoc-openapi#2797) - Removing operationId via customizer does not w...
I have a project with Swagger 3, I noticed a different behavior compared to Swagger 2 and I think it's a bug because with a structure of a certain type, I can make the call to the API, but Swagger doesn't allow me to do it.
I have this API:
As you can see,
ClassOne
has an object with the@Valid
annotation and I am not obliged to pass it (because maybe I want to pass other attributes that I have not added in this example). However, the@Valid
annotation ensures that if I pass any attribute that is inside that class, then it must respect the rules that I indicate inside (@NotNull etc.). The problem is that on the swagger it says required, even if I do not indicate required and therefore I cannot make the call to the API. This is therefore a problem as I said before, because I may want to send other attributes ofClassOne
.This problem occurs only in the case of
@PraameterObject
, while if I use@RequestBody
, it is not mandatory.In my opinion swagger should not automatically add requird if an attribute has the annotation
@NotNull
or@NotBlank
, it should refer to what the user indicates with the annotation @Schemaattached is a zip with an example.
demo.zip
The text was updated successfully, but these errors were encountered: