From 197f7b438cbfbd1f7e9cd5fca31808c387adc8d7 Mon Sep 17 00:00:00 2001 From: Michael Clarke Date: Sun, 24 Nov 2024 15:16:22 +0000 Subject: [PATCH] Obey annotations when flattening ParameterObject fields. Fixes #2787 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. To ensure that the post-processing steps don't re-apply `required` properties on parameters that have purposefully had them removed, the delegate now tracks any annotations what should not be shown as being on the parameter, and excludes them from the annotation list during subsequent calls. --- .../extractor/DelegatingMethodParameter.java | 23 +++- .../MethodParameterPojoExtractor.java | 115 +++++++++++++++-- .../core/service/AbstractRequestService.java | 17 ++- .../api/v30/app232/ParameterController.java | 86 +++++++++++++ .../api/v30/app232/SpringDocApp232Test.java | 29 +++++ .../test/resources/results/3.0.1/app232.json | 117 ++++++++++++++++++ 6 files changed, 369 insertions(+), 18 deletions(-) create mode 100644 springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/ParameterController.java create mode 100644 springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/SpringDocApp232Test.java create mode 100644 springdoc-openapi-starter-webmvc-api/src/test/resources/results/3.0.1/app232.json diff --git a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/DelegatingMethodParameter.java b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/DelegatingMethodParameter.java index e704d51a4..a04958218 100644 --- a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/DelegatingMethodParameter.java +++ b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/DelegatingMethodParameter.java @@ -93,6 +93,11 @@ public class DelegatingMethodParameter extends MethodParameter { */ private final Annotation[] methodAnnotations; + /** + * The annotations to mask from the list of annotations on this method parameter. + */ + private final Annotation[] maskedAnnotations; + /** * The Is not required. */ @@ -105,17 +110,19 @@ public class DelegatingMethodParameter extends MethodParameter { * @param parameterName the parameter name * @param additionalParameterAnnotations the additional parameter annotations * @param methodAnnotations the method annotations + * @param maskedAnnotations any annotations that should not be included in the final list of annotations * @param isParameterObject the is parameter object * @param isNotRequired the is required */ - DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, boolean isParameterObject, boolean isNotRequired) { + DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, Annotation[] maskedAnnotations, boolean isParameterObject, boolean isNotRequired) { super(delegate); this.delegate = delegate; this.additionalParameterAnnotations = additionalParameterAnnotations; this.parameterName = parameterName; this.isParameterObject = isParameterObject; this.isNotRequired = isNotRequired; - this.methodAnnotations =methodAnnotations; + this.methodAnnotations = methodAnnotations; + this.maskedAnnotations = maskedAnnotations; } /** @@ -146,7 +153,7 @@ public static MethodParameter[] customize(String[] pNames, MethodParameter[] par } else { String name = pNames != null ? pNames[i] : p.getParameterName(); - explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, false, false)); + explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, null, false, false)); } } return explodedParameters.toArray(new MethodParameter[0]); @@ -179,7 +186,15 @@ public static MethodParameter changeContainingClass(MethodParameter methodParame @NonNull public Annotation[] getParameterAnnotations() { Annotation[] methodAnnotations = ArrayUtils.addAll(delegate.getParameterAnnotations(), this.methodAnnotations); - return ArrayUtils.addAll(methodAnnotations, additionalParameterAnnotations); + methodAnnotations = ArrayUtils.addAll(methodAnnotations, additionalParameterAnnotations); + if (maskedAnnotations == null) { + return methodAnnotations; + } else { + List maskedAnnotationList = List.of(maskedAnnotations); + return Arrays.stream(methodAnnotations) + .filter(annotation -> !maskedAnnotationList.contains(annotation)) + .toArray(Annotation[]::new); + } } @Override diff --git a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/MethodParameterPojoExtractor.java b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/MethodParameterPojoExtractor.java index 837c79b29..c0390e883 100644 --- a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/MethodParameterPojoExtractor.java +++ b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/MethodParameterPojoExtractor.java @@ -50,10 +50,13 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Predicate; +import java.util.stream.Collectors; import java.util.stream.Stream; import io.swagger.v3.core.util.PrimitiveType; import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.media.Schema; +import org.springdoc.core.service.AbstractRequestService; import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; @@ -63,7 +66,7 @@ /** * The type Method parameter pojo extractor. * - * @author bnasslahsen + * @author bnasslahsen, michael.clarke */ public class MethodParameterPojoExtractor { @@ -113,7 +116,7 @@ private MethodParameterPojoExtractor() { * @return the stream */ static Stream extractFrom(Class clazz) { - return extractFrom(clazz, ""); + return extractFrom(clazz, "", true); } /** @@ -121,12 +124,13 @@ static Stream extractFrom(Class clazz) { * * @param clazz the clazz * @param fieldNamePrefix the field name prefix + * @param parentRequired whether the field that hold the class currently being inspected was required or optional * @return the stream */ - private static Stream extractFrom(Class clazz, String fieldNamePrefix) { + private static Stream extractFrom(Class clazz, String fieldNamePrefix, boolean parentRequired) { return allFieldsOf(clazz).stream() .filter(field -> !field.getType().equals(clazz)) - .flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix)) + .flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix, parentRequired)) .filter(Objects::nonNull); } @@ -136,20 +140,95 @@ private static Stream extractFrom(Class clazz, String fieldN * @param paramClass the param class * @param field the field * @param fieldNamePrefix the field name prefix + * @param parentRequired whether the field that holds the class currently being examined was required or optional * @return the stream */ - private static Stream fromGetterOfField(Class paramClass, Field field, String fieldNamePrefix) { + private static Stream fromGetterOfField(Class paramClass, Field field, String fieldNamePrefix, boolean parentRequired) { Class type = extractType(paramClass, field); if (Objects.isNull(type)) return Stream.empty(); if (isSimpleType(type)) - return fromSimpleClass(paramClass, field, fieldNamePrefix); + return fromSimpleClass(paramClass, field, fieldNamePrefix, parentRequired); else { - String prefix = fieldNamePrefix + field.getName() + DOT; - return extractFrom(type, prefix); + Parameter parameter = field.getAnnotation(Parameter.class); + Schema schema = field.getAnnotation(Schema.class); + boolean visible = resolveVisible(parameter, schema); + if (!visible) { + return Stream.empty(); + } + String prefix = fieldNamePrefix + resolveName(parameter, schema).orElse(field.getName()) + DOT; + boolean notNullAnnotationsPresent = AbstractRequestService.hasNotNullAnnotation(Arrays.stream(field.getDeclaredAnnotations()) + .map(Annotation::annotationType) + .map(Class::getSimpleName) + .collect(Collectors.toSet())); + return extractFrom(type, prefix, parentRequired && resolveRequired(schema, parameter, !notNullAnnotationsPresent)); + } + } + + private static Optional resolveName(Parameter parameter, Schema schema) { + if (parameter != null) { + return resolveNameFromParameter(parameter); + } + if (schema != null) { + return resolveNameFromSchema(schema); + } + return Optional.empty(); + } + + private static Optional resolveNameFromParameter(Parameter parameter) { + if (parameter.name().isEmpty()) { + return Optional.empty(); + } + return Optional.of(parameter.name()); + } + + private static Optional resolveNameFromSchema(Schema schema) { + if (schema.name().isEmpty()) { + return Optional.empty(); + } + return Optional.of(schema.name()); + } + + private static boolean resolveVisible(Parameter parameter, Schema schema) { + if (parameter != null) { + return !parameter.hidden(); } + if (schema != null) { + return !schema.hidden(); + } + return true; + } + + private static boolean resolveRequired(Schema schema, Parameter parameter, boolean nullable) { + if (parameter != null) { + return resolveRequiredFromParameter(parameter, nullable); + } + if (schema != null) { + return resolveRequiredFromSchema(schema, nullable); + } + return !nullable; + } + + private static boolean resolveRequiredFromParameter(Parameter parameter, boolean nullable) { + if (parameter.required()) { + return true; + } + return !nullable; + } + + private static boolean resolveRequiredFromSchema(Schema schema, boolean nullable) { + if (schema.required()) { + return true; + } + else if (schema.requiredMode() == Schema.RequiredMode.REQUIRED) { + return true; + } + else if (schema.requiredMode() == Schema.RequiredMode.NOT_REQUIRED) { + return false; + } + return !nullable; } /** @@ -181,18 +260,30 @@ private static Class extractType(Class paramClass, Field field) { * @param fieldNamePrefix the field name prefix * @return the stream */ - private static Stream fromSimpleClass(Class paramClass, Field field, String fieldNamePrefix) { + private static Stream fromSimpleClass(Class paramClass, Field field, String fieldNamePrefix, boolean isParentRequired) { Annotation[] fieldAnnotations = field.getDeclaredAnnotations(); try { Parameter parameter = field.getAnnotation(Parameter.class); - boolean isNotRequired = parameter == null || !parameter.required(); + Schema schema = field.getAnnotation(Schema.class); + boolean visible = resolveVisible(parameter, schema); + if (!visible) { + return Stream.empty(); + } + + boolean isNotRequired = !(isParentRequired && resolveRequired(schema, parameter, !AbstractRequestService.hasNotNullAnnotation(Arrays.stream(fieldAnnotations) + .map(Annotation::annotationType) + .map(Class::getSimpleName) + .collect(Collectors.toSet())))); + Annotation[] notNullFieldAnnotations = Arrays.stream(fieldAnnotations) + .filter(annotation -> AbstractRequestService.hasNotNullAnnotation(List.of(annotation.annotationType().getSimpleName()))) + .toArray(Annotation[]::new); if (paramClass.getSuperclass() != null && paramClass.isRecord()) { return Stream.of(paramClass.getRecordComponents()) .filter(d -> d.getName().equals(field.getName())) .map(RecordComponent::getAccessor) .map(method -> new MethodParameter(method, -1)) .map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass)) - .map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired)); + .map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired)); } else @@ -202,7 +293,7 @@ private static Stream fromSimpleClass(Class paramClass, Fiel .filter(Objects::nonNull) .map(method -> new MethodParameter(method, -1)) .map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass)) - .map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired)); + .map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired)); } catch (IntrospectionException e) { return Stream.of(); diff --git a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/service/AbstractRequestService.java b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/service/AbstractRequestService.java index a2a7d9b45..5f704b77c 100644 --- a/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/service/AbstractRequestService.java +++ b/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/service/AbstractRequestService.java @@ -570,6 +570,9 @@ public Parameter buildParam(ParameterInfo parameterInfo, Components components, if (parameter.getRequired() == null) parameter.setRequired(parameterInfo.isRequired()); + if (Boolean.TRUE.equals(parameter.getRequired()) && parameterInfo.getMethodParameter() instanceof DelegatingMethodParameter delegatingMethodParameter && delegatingMethodParameter.isNotRequired()) + parameter.setRequired(false); + if (containsDeprecatedAnnotation(parameterInfo.getMethodParameter().getParameterAnnotations())) parameter.setDeprecated(true); @@ -602,7 +605,7 @@ public void applyBeanValidatorAnnotations(final Parameter parameter, final List< Map annos = new HashMap<>(); if (annotations != null) annotations.forEach(annotation -> annos.put(annotation.annotationType().getSimpleName(), annotation)); - boolean annotationExists = Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annos::containsKey); + boolean annotationExists = hasNotNullAnnotation(annos.keySet()); if (annotationExists) parameter.setRequired(true); Schema schema = parameter.getSchema(); @@ -629,7 +632,7 @@ public void applyBeanValidatorAnnotations(final RequestBody requestBody, final L .filter(annotation -> io.swagger.v3.oas.annotations.parameters.RequestBody.class.equals(annotation.annotationType())) .anyMatch(annotation -> ((io.swagger.v3.oas.annotations.parameters.RequestBody) annotation).required()); } - boolean validationExists = Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annos::containsKey); + boolean validationExists = hasNotNullAnnotation(annos.keySet()); if (validationExists || (!isOptional && (springRequestBodyRequired || swaggerRequestBodyRequired))) requestBody.setRequired(true); @@ -831,4 +834,14 @@ else if (requestBody.content().length > 0) } return false; } + + /** + * Check if the parameter has any of the annotations that make it non-optional + * + * @param annotationSimpleNames the annotation simple class named, e.g. NotNull + * @return whether any of the known NotNull annotations are present + */ + public static boolean hasNotNullAnnotation(Collection annotationSimpleNames) { + return Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annotationSimpleNames::contains); + } } diff --git a/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/ParameterController.java b/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/ParameterController.java new file mode 100644 index 000000000..2f6e14588 --- /dev/null +++ b/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/ParameterController.java @@ -0,0 +1,86 @@ +/* + * + * * + * * * + * * * * + * * * * * + * * * * * * Copyright 2019-2024 the original author or authors. + * * * * * * + * * * * * * Licensed under the Apache License, Version 2.0 (the "License"); + * * * * * * you may not use this file except in compliance with the License. + * * * * * * You may obtain a copy of the License at + * * * * * * + * * * * * * https://www.apache.org/licenses/LICENSE-2.0 + * * * * * * + * * * * * * Unless required by applicable law or agreed to in writing, software + * * * * * * distributed under the License is distributed on an "AS IS" BASIS, + * * * * * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * * * * * See the License for the specific language governing permissions and + * * * * * * limitations under the License. + * * * * * + * * * * + * * * + * * + * + */ +package test.org.springdoc.api.v30.app232; + +import io.swagger.v3.oas.annotations.Parameter; +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; +import org.springdoc.core.annotations.ParameterObject; + +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class ParameterController { + + @GetMapping("/hidden-parent") + public void nestedParameterObjectWithHiddenParentField(@ParameterObject ParameterObjectWithHiddenField parameters) { + + } + + public record ParameterObjectWithHiddenField( + @Schema(hidden = true) NestedParameterObject schemaHiddenNestedParameterObject, + @Parameter(hidden = true) NestedParameterObject parameterHiddenNestedParameterObject, + NestedParameterObject visibleNestedParameterObject + ) { + + } + + public record NestedParameterObject( + String parameterField) { + } + + @GetMapping("/renamed-parent") + public void nestedParameterObjectWithRenamedParentField(@ParameterObject ParameterObjectWithRenamedField parameters) { + + } + + public record ParameterObjectWithRenamedField( + @Schema(name = "schemaRenamed") NestedParameterObject schemaRenamedNestedParameterObject, + @Parameter(name = "parameterRenamed") NestedParameterObject parameterRenamedNestedParameterObject, + NestedParameterObject originalNameNestedParameterObject + ) { + + } + + @GetMapping("/optional-parent") + public void nestedParameterObjectWithOptionalParentField(@ParameterObject ParameterObjectWithOptionalField parameters) { + + } + + public record ParameterObjectWithOptionalField( + @Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED) NestedRequiredParameterObject schemaNotRequiredNestedParameterObject, + @Parameter NestedRequiredParameterObject parameterNotRequiredNestedParameterObject, + @Parameter(required = true) NestedRequiredParameterObject requiredNestedParameterObject + ) { + + } + + public record NestedRequiredParameterObject( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) @NotNull String requiredParameterField) { + } + +} diff --git a/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/SpringDocApp232Test.java b/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/SpringDocApp232Test.java new file mode 100644 index 000000000..73d5d7109 --- /dev/null +++ b/springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app232/SpringDocApp232Test.java @@ -0,0 +1,29 @@ +/* + * + * * Copyright 2019-2024 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * https://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ +package test.org.springdoc.api.v30.app232; + +import test.org.springdoc.api.v30.AbstractSpringDocV30Test; + +import org.springframework.boot.autoconfigure.SpringBootApplication; + +class SpringDocApp232Test extends AbstractSpringDocV30Test { + + @SpringBootApplication + static class SpringDocTestApp {} + +} diff --git a/springdoc-openapi-starter-webmvc-api/src/test/resources/results/3.0.1/app232.json b/springdoc-openapi-starter-webmvc-api/src/test/resources/results/3.0.1/app232.json new file mode 100644 index 000000000..be5d0d307 --- /dev/null +++ b/springdoc-openapi-starter-webmvc-api/src/test/resources/results/3.0.1/app232.json @@ -0,0 +1,117 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "OpenAPI definition", + "version": "v0" + }, + "servers": [ + { + "url": "http://localhost", + "description": "Generated server url" + } + ], + "paths": { + "/renamed-parent": { + "get": { + "tags": [ + "parameter-controller" + ], + "operationId": "nestedParameterObjectWithRenamedParentField", + "parameters": [ + { + "name": "schemaRenamed.parameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "parameterRenamed.parameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "originalNameNestedParameterObject.parameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK" + } + } + } + }, + "/optional-parent": { + "get": { + "tags": [ + "parameter-controller" + ], + "operationId": "nestedParameterObjectWithOptionalParentField", + "parameters": [ + { + "name": "schemaNotRequiredNestedParameterObject.requiredParameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "parameterNotRequiredNestedParameterObject.requiredParameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "requiredNestedParameterObject.requiredParameterField", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK" + } + } + } + }, + "/hidden-parent": { + "get": { + "tags": [ + "parameter-controller" + ], + "operationId": "nestedParameterObjectWithHiddenParentField", + "parameters": [ + { + "name": "visibleNestedParameterObject.parameterField", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK" + } + } + } + } + }, + "components": {} +}