Skip to content

Commit 543c920

Browse files
bnasslahsenbnasslahsen
committed
code review
2 parents de8a7ec + 197f7b4 commit 543c920

File tree

6 files changed

+381
-18
lines changed

6 files changed

+381
-18
lines changed

Diff for: ‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/DelegatingMethodParameter.java

+19-4
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ public class DelegatingMethodParameter extends MethodParameter {
9393
*/
9494
private final Annotation[] methodAnnotations;
9595

96+
/**
97+
* The annotations to mask from the list of annotations on this method parameter.
98+
*/
99+
private final Annotation[] maskedAnnotations;
100+
96101
/**
97102
* The Is not required.
98103
*/
@@ -105,17 +110,19 @@ public class DelegatingMethodParameter extends MethodParameter {
105110
* @param parameterName the parameter name
106111
* @param additionalParameterAnnotations the additional parameter annotations
107112
* @param methodAnnotations the method annotations
113+
* @param maskedAnnotations any annotations that should not be included in the final list of annotations
108114
* @param isParameterObject the is parameter object
109115
* @param isNotRequired the is required
110116
*/
111-
DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, boolean isParameterObject, boolean isNotRequired) {
117+
DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, Annotation[] maskedAnnotations, boolean isParameterObject, boolean isNotRequired) {
112118
super(delegate);
113119
this.delegate = delegate;
114120
this.additionalParameterAnnotations = additionalParameterAnnotations;
115121
this.parameterName = parameterName;
116122
this.isParameterObject = isParameterObject;
117123
this.isNotRequired = isNotRequired;
118-
this.methodAnnotations =methodAnnotations;
124+
this.methodAnnotations = methodAnnotations;
125+
this.maskedAnnotations = maskedAnnotations;
119126
}
120127

121128
/**
@@ -146,7 +153,7 @@ public static MethodParameter[] customize(String[] pNames, MethodParameter[] par
146153
}
147154
else {
148155
String name = pNames != null ? pNames[i] : p.getParameterName();
149-
explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, false, false));
156+
explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, null, false, false));
150157
}
151158
}
152159
return explodedParameters.toArray(new MethodParameter[0]);
@@ -179,7 +186,15 @@ public static MethodParameter changeContainingClass(MethodParameter methodParame
179186
@NonNull
180187
public Annotation[] getParameterAnnotations() {
181188
Annotation[] methodAnnotations = ArrayUtils.addAll(delegate.getParameterAnnotations(), this.methodAnnotations);
182-
return ArrayUtils.addAll(methodAnnotations, additionalParameterAnnotations);
189+
methodAnnotations = ArrayUtils.addAll(methodAnnotations, additionalParameterAnnotations);
190+
if (maskedAnnotations == null) {
191+
return methodAnnotations;
192+
} else {
193+
List<Annotation> maskedAnnotationList = List.of(maskedAnnotations);
194+
return Arrays.stream(methodAnnotations)
195+
.filter(annotation -> !maskedAnnotationList.contains(annotation))
196+
.toArray(Annotation[]::new);
197+
}
183198
}
184199

185200
@Override

Diff for: ‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/extractor/MethodParameterPojoExtractor.java

+103-12
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,13 @@
5050
import java.util.concurrent.atomic.AtomicInteger;
5151
import java.util.concurrent.atomic.AtomicLong;
5252
import java.util.function.Predicate;
53+
import java.util.stream.Collectors;
5354
import java.util.stream.Stream;
5455

5556
import io.swagger.v3.core.util.PrimitiveType;
5657
import io.swagger.v3.oas.annotations.Parameter;
58+
import io.swagger.v3.oas.annotations.media.Schema;
59+
import org.springdoc.core.service.AbstractRequestService;
5760

5861
import org.springframework.core.GenericTypeResolver;
5962
import org.springframework.core.MethodParameter;
@@ -63,7 +66,7 @@
6366
/**
6467
* The type Method parameter pojo extractor.
6568
*
66-
* @author bnasslahsen
69+
* @author bnasslahsen, michael.clarke
6770
*/
6871
public class MethodParameterPojoExtractor {
6972

@@ -113,20 +116,21 @@ private MethodParameterPojoExtractor() {
113116
* @return the stream
114117
*/
115118
static Stream<MethodParameter> extractFrom(Class<?> clazz) {
116-
return extractFrom(clazz, "");
119+
return extractFrom(clazz, "", true);
117120
}
118121

119122
/**
120123
* Extract from stream.
121124
*
122125
* @param clazz the clazz
123126
* @param fieldNamePrefix the field name prefix
127+
* @param parentRequired whether the field that hold the class currently being inspected was required or optional
124128
* @return the stream
125129
*/
126-
private static Stream<MethodParameter> extractFrom(Class<?> clazz, String fieldNamePrefix) {
130+
private static Stream<MethodParameter> extractFrom(Class<?> clazz, String fieldNamePrefix, boolean parentRequired) {
127131
return allFieldsOf(clazz).stream()
128132
.filter(field -> !field.getType().equals(clazz))
129-
.flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix))
133+
.flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix, parentRequired))
130134
.filter(Objects::nonNull);
131135
}
132136

@@ -136,20 +140,95 @@ private static Stream<MethodParameter> extractFrom(Class<?> clazz, String fieldN
136140
* @param paramClass the param class
137141
* @param field the field
138142
* @param fieldNamePrefix the field name prefix
143+
* @param parentRequired whether the field that holds the class currently being examined was required or optional
139144
* @return the stream
140145
*/
141-
private static Stream<MethodParameter> fromGetterOfField(Class<?> paramClass, Field field, String fieldNamePrefix) {
146+
private static Stream<MethodParameter> fromGetterOfField(Class<?> paramClass, Field field, String fieldNamePrefix, boolean parentRequired) {
142147
Class<?> type = extractType(paramClass, field);
143148

144149
if (Objects.isNull(type))
145150
return Stream.empty();
146151

147152
if (isSimpleType(type))
148-
return fromSimpleClass(paramClass, field, fieldNamePrefix);
153+
return fromSimpleClass(paramClass, field, fieldNamePrefix, parentRequired);
149154
else {
150-
String prefix = fieldNamePrefix + field.getName() + DOT;
151-
return extractFrom(type, prefix);
155+
Parameter parameter = field.getAnnotation(Parameter.class);
156+
Schema schema = field.getAnnotation(Schema.class);
157+
boolean visible = resolveVisible(parameter, schema);
158+
if (!visible) {
159+
return Stream.empty();
160+
}
161+
String prefix = fieldNamePrefix + resolveName(parameter, schema).orElse(field.getName()) + DOT;
162+
boolean notNullAnnotationsPresent = AbstractRequestService.hasNotNullAnnotation(Arrays.stream(field.getDeclaredAnnotations())
163+
.map(Annotation::annotationType)
164+
.map(Class::getSimpleName)
165+
.collect(Collectors.toSet()));
166+
return extractFrom(type, prefix, parentRequired && resolveRequired(schema, parameter, !notNullAnnotationsPresent));
167+
}
168+
}
169+
170+
private static Optional<String> resolveName(Parameter parameter, Schema schema) {
171+
if (parameter != null) {
172+
return resolveNameFromParameter(parameter);
173+
}
174+
if (schema != null) {
175+
return resolveNameFromSchema(schema);
176+
}
177+
return Optional.empty();
178+
}
179+
180+
private static Optional<String> resolveNameFromParameter(Parameter parameter) {
181+
if (parameter.name().isEmpty()) {
182+
return Optional.empty();
183+
}
184+
return Optional.of(parameter.name());
185+
}
186+
187+
private static Optional<String> resolveNameFromSchema(Schema schema) {
188+
if (schema.name().isEmpty()) {
189+
return Optional.empty();
190+
}
191+
return Optional.of(schema.name());
192+
}
193+
194+
private static boolean resolveVisible(Parameter parameter, Schema schema) {
195+
if (parameter != null) {
196+
return !parameter.hidden();
152197
}
198+
if (schema != null) {
199+
return !schema.hidden();
200+
}
201+
return true;
202+
}
203+
204+
private static boolean resolveRequired(Schema schema, Parameter parameter, boolean nullable) {
205+
if (parameter != null) {
206+
return resolveRequiredFromParameter(parameter, nullable);
207+
}
208+
if (schema != null) {
209+
return resolveRequiredFromSchema(schema, nullable);
210+
}
211+
return !nullable;
212+
}
213+
214+
private static boolean resolveRequiredFromParameter(Parameter parameter, boolean nullable) {
215+
if (parameter.required()) {
216+
return true;
217+
}
218+
return !nullable;
219+
}
220+
221+
private static boolean resolveRequiredFromSchema(Schema schema, boolean nullable) {
222+
if (schema.required()) {
223+
return true;
224+
}
225+
else if (schema.requiredMode() == Schema.RequiredMode.REQUIRED) {
226+
return true;
227+
}
228+
else if (schema.requiredMode() == Schema.RequiredMode.NOT_REQUIRED) {
229+
return false;
230+
}
231+
return !nullable;
153232
}
154233

155234
/**
@@ -181,18 +260,30 @@ private static Class<?> extractType(Class<?> paramClass, Field field) {
181260
* @param fieldNamePrefix the field name prefix
182261
* @return the stream
183262
*/
184-
private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Field field, String fieldNamePrefix) {
263+
private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Field field, String fieldNamePrefix, boolean isParentRequired) {
185264
Annotation[] fieldAnnotations = field.getDeclaredAnnotations();
186265
try {
187266
Parameter parameter = field.getAnnotation(Parameter.class);
188-
boolean isNotRequired = parameter == null || !parameter.required();
267+
Schema schema = field.getAnnotation(Schema.class);
268+
boolean visible = resolveVisible(parameter, schema);
269+
if (!visible) {
270+
return Stream.empty();
271+
}
272+
273+
boolean isNotRequired = !(isParentRequired && resolveRequired(schema, parameter, !AbstractRequestService.hasNotNullAnnotation(Arrays.stream(fieldAnnotations)
274+
.map(Annotation::annotationType)
275+
.map(Class::getSimpleName)
276+
.collect(Collectors.toSet()))));
277+
Annotation[] notNullFieldAnnotations = Arrays.stream(fieldAnnotations)
278+
.filter(annotation -> AbstractRequestService.hasNotNullAnnotation(List.of(annotation.annotationType().getSimpleName())))
279+
.toArray(Annotation[]::new);
189280
if (paramClass.getSuperclass() != null && paramClass.isRecord()) {
190281
return Stream.of(paramClass.getRecordComponents())
191282
.filter(d -> d.getName().equals(field.getName()))
192283
.map(RecordComponent::getAccessor)
193284
.map(method -> new MethodParameter(method, -1))
194285
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
195-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired));
286+
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired));
196287

197288
}
198289
else
@@ -202,7 +293,7 @@ private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Fiel
202293
.filter(Objects::nonNull)
203294
.map(method -> new MethodParameter(method, -1))
204295
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
205-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired));
296+
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired));
206297
}
207298
catch (IntrospectionException e) {
208299
return Stream.of();

Diff for: ‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/service/AbstractRequestService.java

+15-2
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,9 @@ public Parameter buildParam(ParameterInfo parameterInfo, Components components,
570570
if (parameter.getRequired() == null)
571571
parameter.setRequired(parameterInfo.isRequired());
572572

573+
if (Boolean.TRUE.equals(parameter.getRequired()) && parameterInfo.getMethodParameter() instanceof DelegatingMethodParameter delegatingMethodParameter && delegatingMethodParameter.isNotRequired())
574+
parameter.setRequired(false);
575+
573576
if (containsDeprecatedAnnotation(parameterInfo.getMethodParameter().getParameterAnnotations()))
574577
parameter.setDeprecated(true);
575578

@@ -602,7 +605,7 @@ public void applyBeanValidatorAnnotations(final Parameter parameter, final List<
602605
Map<String, Annotation> annos = new HashMap<>();
603606
if (annotations != null)
604607
annotations.forEach(annotation -> annos.put(annotation.annotationType().getSimpleName(), annotation));
605-
boolean annotationExists = Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annos::containsKey);
608+
boolean annotationExists = hasNotNullAnnotation(annos.keySet());
606609
if (annotationExists)
607610
parameter.setRequired(true);
608611
Schema<?> schema = parameter.getSchema();
@@ -629,7 +632,7 @@ public void applyBeanValidatorAnnotations(final RequestBody requestBody, final L
629632
.filter(annotation -> io.swagger.v3.oas.annotations.parameters.RequestBody.class.equals(annotation.annotationType()))
630633
.anyMatch(annotation -> ((io.swagger.v3.oas.annotations.parameters.RequestBody) annotation).required());
631634
}
632-
boolean validationExists = Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annos::containsKey);
635+
boolean validationExists = hasNotNullAnnotation(annos.keySet());
633636

634637
if (validationExists || (!isOptional && (springRequestBodyRequired || swaggerRequestBodyRequired)))
635638
requestBody.setRequired(true);
@@ -831,4 +834,14 @@ else if (requestBody.content().length > 0)
831834
}
832835
return false;
833836
}
837+
838+
/**
839+
* Check if the parameter has any of the annotations that make it non-optional
840+
*
841+
* @param annotationSimpleNames the annotation simple class named, e.g. NotNull
842+
* @return whether any of the known NotNull annotations are present
843+
*/
844+
public static boolean hasNotNullAnnotation(Collection<String> annotationSimpleNames) {
845+
return Arrays.stream(ANNOTATIONS_FOR_REQUIRED).anyMatch(annotationSimpleNames::contains);
846+
}
834847
}

Diff for: ‎springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app234/ParameterController.java

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
*
3+
* *
4+
* * *
5+
* * * *
6+
* * * * *
7+
* * * * * * Copyright 2019-2024 the original author or authors.
8+
* * * * * *
9+
* * * * * * Licensed under the Apache License, Version 2.0 (the "License");
10+
* * * * * * you may not use this file except in compliance with the License.
11+
* * * * * * You may obtain a copy of the License at
12+
* * * * * *
13+
* * * * * * https://www.apache.org/licenses/LICENSE-2.0
14+
* * * * * *
15+
* * * * * * Unless required by applicable law or agreed to in writing, software
16+
* * * * * * distributed under the License is distributed on an "AS IS" BASIS,
17+
* * * * * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
18+
* * * * * * See the License for the specific language governing permissions and
19+
* * * * * * limitations under the License.
20+
* * * * *
21+
* * * *
22+
* * *
23+
* *
24+
*
25+
*/
26+
package test.org.springdoc.api.v30.app234;
27+
28+
import io.swagger.v3.oas.annotations.Parameter;
29+
import io.swagger.v3.oas.annotations.media.Schema;
30+
import jakarta.validation.constraints.NotNull;
31+
import org.springdoc.core.annotations.ParameterObject;
32+
33+
import org.springframework.web.bind.annotation.GetMapping;
34+
import org.springframework.web.bind.annotation.RestController;
35+
36+
@RestController
37+
public class ParameterController {
38+
39+
@GetMapping("/hidden-parent")
40+
public void nestedParameterObjectWithHiddenParentField(@ParameterObject ParameterObjectWithHiddenField parameters) {
41+
42+
}
43+
44+
public record ParameterObjectWithHiddenField(
45+
@Schema(hidden = true) NestedParameterObject schemaHiddenNestedParameterObject,
46+
@Parameter(hidden = true) NestedParameterObject parameterHiddenNestedParameterObject,
47+
NestedParameterObject visibleNestedParameterObject
48+
) {
49+
50+
}
51+
52+
public record NestedParameterObject(
53+
String parameterField) {
54+
}
55+
56+
@GetMapping("/renamed-parent")
57+
public void nestedParameterObjectWithRenamedParentField(@ParameterObject ParameterObjectWithRenamedField parameters) {
58+
59+
}
60+
61+
public record ParameterObjectWithRenamedField(
62+
@Schema(name = "schemaRenamed") NestedParameterObject schemaRenamedNestedParameterObject,
63+
@Parameter(name = "parameterRenamed") NestedParameterObject parameterRenamedNestedParameterObject,
64+
NestedParameterObject originalNameNestedParameterObject
65+
) {
66+
67+
}
68+
69+
@GetMapping("/optional-parent")
70+
public void nestedParameterObjectWithOptionalParentField(@ParameterObject ParameterObjectWithOptionalField parameters) {
71+
72+
}
73+
74+
public record ParameterObjectWithOptionalField(
75+
@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED) NestedRequiredParameterObject schemaNotRequiredNestedParameterObject,
76+
@Parameter NestedRequiredParameterObject parameterNotRequiredNestedParameterObject,
77+
@Parameter(required = true) NestedRequiredParameterObject requiredNestedParameterObject
78+
) {
79+
80+
}
81+
82+
public record NestedRequiredParameterObject(
83+
@Schema(requiredMode = Schema.RequiredMode.REQUIRED) @NotNull String requiredParameterField) {
84+
}
85+
86+
}

0 commit comments

Comments
 (0)