Skip to content

Commit

Permalink
Obey annotations when flattening ParameterObject fields. Fixes #2787
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mc1arke committed Nov 24, 2024
1 parent e8de90e commit 197f7b4
Show file tree
Hide file tree
Showing 6 changed files with 369 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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<Annotation> maskedAnnotationList = List.of(maskedAnnotations);
return Arrays.stream(methodAnnotations)
.filter(annotation -> !maskedAnnotationList.contains(annotation))
.toArray(Annotation[]::new);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -63,7 +66,7 @@
/**
* The type Method parameter pojo extractor.
*
* @author bnasslahsen
* @author bnasslahsen, michael.clarke
*/
public class MethodParameterPojoExtractor {

Expand Down Expand Up @@ -113,20 +116,21 @@ private MethodParameterPojoExtractor() {
* @return the stream
*/
static Stream<MethodParameter> extractFrom(Class<?> clazz) {
return extractFrom(clazz, "");
return extractFrom(clazz, "", true);
}

/**
* Extract from stream.
*
* @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<MethodParameter> extractFrom(Class<?> clazz, String fieldNamePrefix) {
private static Stream<MethodParameter> 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);
}

Expand All @@ -136,20 +140,95 @@ private static Stream<MethodParameter> 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<MethodParameter> fromGetterOfField(Class<?> paramClass, Field field, String fieldNamePrefix) {
private static Stream<MethodParameter> 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<String> resolveName(Parameter parameter, Schema schema) {
if (parameter != null) {
return resolveNameFromParameter(parameter);
}
if (schema != null) {
return resolveNameFromSchema(schema);
}
return Optional.empty();
}

private static Optional<String> resolveNameFromParameter(Parameter parameter) {
if (parameter.name().isEmpty()) {
return Optional.empty();
}
return Optional.of(parameter.name());
}

private static Optional<String> 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;
}

/**
Expand Down Expand Up @@ -181,18 +260,30 @@ private static Class<?> extractType(Class<?> paramClass, Field field) {
* @param fieldNamePrefix the field name prefix
* @return the stream
*/
private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Field field, String fieldNamePrefix) {
private static Stream<MethodParameter> 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
Expand All @@ -202,7 +293,7 @@ private static Stream<MethodParameter> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -602,7 +605,7 @@ public void applyBeanValidatorAnnotations(final Parameter parameter, final List<
Map<String, Annotation> 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();
Expand All @@ -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);
Expand Down Expand Up @@ -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<String> annotationSimpleNames) {
return Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annotationSimpleNames::contains);
}
}
Original file line number Diff line number Diff line change
@@ -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) {
}

}
Loading

0 comments on commit 197f7b4

Please sign in to comment.