Skip to content

Commit 8c07306

Browse files
bnasslahsenbnasslahsen
committed
code review
1 parent 34a468a commit 8c07306

File tree

10 files changed

+240
-51
lines changed

10 files changed

+240
-51
lines changed

‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/data/DataRestRequestService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ private void addParameters(OpenAPI openAPI, RequestMethod requestMethod, MethodA
227227
MethodParameter methodParameter, ParameterInfo parameterInfo, Parameter parameter) {
228228
List<Annotation> parameterAnnotations = Arrays.asList(getParameterAnnotations(methodParameter));
229229
if (requestBuilder.isValidParameter(parameter,methodAttributes)) {
230-
requestBuilder.applyBeanValidatorAnnotations(parameter, parameterAnnotations);
230+
requestBuilder.applyBeanValidatorAnnotations(parameter, parameterAnnotations, parameterInfo.isParameterObject());
231231
operation.addParametersItem(parameter);
232232
}
233233
else if (!RequestMethod.GET.equals(requestMethod)) {

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

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,6 @@ 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-
10196
/**
10297
* The Is not required.
10398
*/
@@ -110,19 +105,17 @@ public class DelegatingMethodParameter extends MethodParameter {
110105
* @param parameterName the parameter name
111106
* @param additionalParameterAnnotations the additional parameter annotations
112107
* @param methodAnnotations the method annotations
113-
* @param maskedAnnotations any annotations that should not be included in the final list of annotations
114108
* @param isParameterObject the is parameter object
115109
* @param isNotRequired the is required
116110
*/
117-
DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, Annotation[] maskedAnnotations, boolean isParameterObject, boolean isNotRequired) {
111+
DelegatingMethodParameter(MethodParameter delegate, String parameterName, Annotation[] additionalParameterAnnotations, Annotation[] methodAnnotations, boolean isParameterObject, boolean isNotRequired) {
118112
super(delegate);
119113
this.delegate = delegate;
120114
this.additionalParameterAnnotations = additionalParameterAnnotations;
121115
this.parameterName = parameterName;
122116
this.isParameterObject = isParameterObject;
123117
this.isNotRequired = isNotRequired;
124-
this.methodAnnotations = methodAnnotations;
125-
this.maskedAnnotations = maskedAnnotations;
118+
this.methodAnnotations =methodAnnotations;
126119
}
127120

128121
/**
@@ -153,7 +146,7 @@ public static MethodParameter[] customize(String[] pNames, MethodParameter[] par
153146
}
154147
else {
155148
String name = pNames != null ? pNames[i] : p.getParameterName();
156-
explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, null, false, false));
149+
explodedParameters.add(new DelegatingMethodParameter(p, name, null, null, false, false));
157150
}
158151
}
159152
return explodedParameters.toArray(new MethodParameter[0]);
@@ -186,15 +179,7 @@ public static MethodParameter changeContainingClass(MethodParameter methodParame
186179
@NonNull
187180
public Annotation[] getParameterAnnotations() {
188181
Annotation[] methodAnnotations = ArrayUtils.addAll(delegate.getParameterAnnotations(), this.methodAnnotations);
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-
}
182+
return ArrayUtils.addAll(methodAnnotations, additionalParameterAnnotations);
198183
}
199184

200185
@Override

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

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.time.LocalTime;
3939
import java.util.ArrayList;
4040
import java.util.Arrays;
41+
import java.util.Collection;
4142
import java.util.HashSet;
4243
import java.util.List;
4344
import java.util.Map;
@@ -56,11 +57,11 @@
5657
import io.swagger.v3.core.util.PrimitiveType;
5758
import io.swagger.v3.oas.annotations.Parameter;
5859
import io.swagger.v3.oas.annotations.media.Schema;
59-
import org.springdoc.core.service.AbstractRequestService;
6060

6161
import org.springframework.core.GenericTypeResolver;
6262
import org.springframework.core.MethodParameter;
6363

64+
import static org.springdoc.core.service.AbstractRequestService.hasNotNullAnnotation;
6465
import static org.springdoc.core.utils.Constants.DOT;
6566

6667
/**
@@ -159,11 +160,8 @@ private static Stream<MethodParameter> fromGetterOfField(Class<?> paramClass, Fi
159160
return Stream.empty();
160161
}
161162
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));
163+
boolean isNullable = isNullable(field.getDeclaredAnnotations());
164+
return extractFrom(type, prefix, parentRequired && resolveRequired(schema, parameter, isNullable));
167165
}
168166
}
169167

@@ -265,28 +263,15 @@ private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Fiel
265263
try {
266264
Parameter parameter = field.getAnnotation(Parameter.class);
267265
Schema schema = field.getAnnotation(Schema.class);
268-
boolean visible = resolveVisible(parameter, schema);
269-
if (!visible) {
270-
return Stream.empty();
271-
}
272-
273-
boolean forcedRequired = resolveRequired(schema, parameter, true);
274-
275-
boolean isNotRequired = !((isParentRequired || forcedRequired) && resolveRequired(schema, parameter, !AbstractRequestService.hasNotNullAnnotation(Arrays.stream(fieldAnnotations)
276-
.map(Annotation::annotationType)
277-
.map(Class::getSimpleName)
278-
.collect(Collectors.toSet()))));
279-
Annotation[] notNullFieldAnnotations = Arrays.stream(fieldAnnotations)
280-
.filter(annotation -> AbstractRequestService.hasNotNullAnnotation(List.of(annotation.annotationType().getSimpleName())))
281-
.toArray(Annotation[]::new);
266+
boolean isNullable = isNullable(fieldAnnotations);
267+
boolean isNotRequired = !(isParentRequired && resolveRequired(schema, parameter, isNullable));
282268
if (paramClass.getSuperclass() != null && paramClass.isRecord()) {
283269
return Stream.of(paramClass.getRecordComponents())
284270
.filter(d -> d.getName().equals(field.getName()))
285271
.map(RecordComponent::getAccessor)
286272
.map(method -> new MethodParameter(method, -1))
287273
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
288-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired));
289-
274+
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired));
290275
}
291276
else
292277
return Stream.of(Introspector.getBeanInfo(paramClass).getPropertyDescriptors())
@@ -295,7 +280,7 @@ private static Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Fiel
295280
.filter(Objects::nonNull)
296281
.map(method -> new MethodParameter(method, -1))
297282
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
298-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), notNullFieldAnnotations, true, isNotRequired));
283+
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, isNotRequired));
299284
}
300285
catch (IntrospectionException e) {
301286
return Stream.of();
@@ -366,4 +351,17 @@ public static void removeSimpleTypes(Class<?>... classes) {
366351
SIMPLE_TYPES.removeAll(Arrays.asList(classes));
367352
}
368353

354+
/**
355+
* Is nullable boolean.
356+
*
357+
* @param fieldAnnotations the field annotations
358+
* @return the boolean
359+
*/
360+
private static boolean isNullable(Annotation[] fieldAnnotations) {
361+
Collection<String> annotationSimpleNames = Arrays.stream(fieldAnnotations)
362+
.map(Annotation::annotationType)
363+
.map(Class::getSimpleName)
364+
.collect(Collectors.toSet());
365+
return !hasNotNullAnnotation(annotationSimpleNames);
366+
}
369367
}

‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/models/ParameterInfo.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.apache.commons.lang3.StringUtils;
3232
import org.slf4j.Logger;
3333
import org.slf4j.LoggerFactory;
34+
import org.springdoc.core.extractor.DelegatingMethodParameter;
3435
import org.springdoc.core.service.GenericParameterService;
3536

3637
import org.springframework.core.MethodParameter;
@@ -337,4 +338,14 @@ public ParameterId getParameterId() {
337338
public void setParameterId(ParameterId parameterId) {
338339
this.parameterId = parameterId;
339340
}
341+
342+
/**
343+
* Is parameter object boolean.
344+
*
345+
* @return the boolean
346+
*/
347+
public boolean isParameterObject() {
348+
return methodParameter instanceof DelegatingMethodParameter delegatingMethodParameter
349+
&& delegatingMethodParameter.isParameterObject();
350+
}
340351
}

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ public Operation build(HandlerMethod handlerMethod, RequestMethod requestMethod,
343343
parameter.setDescription(paramJavadocDescription);
344344
}
345345
}
346-
applyBeanValidatorAnnotations(parameter, parameterAnnotations);
346+
applyBeanValidatorAnnotations(parameter, parameterAnnotations, parameterInfo.isParameterObject());
347347
}
348348
else if (!RequestMethod.GET.equals(requestMethod) || OpenApiVersion.OPENAPI_3_1.getVersion().equals(openAPI.getOpenapi())) {
349349
if (operation.getRequestBody() != null)
@@ -584,9 +584,6 @@ public Parameter buildParam(ParameterInfo parameterInfo, Components components,
584584
if (parameter.getRequired() == null)
585585
parameter.setRequired(parameterInfo.isRequired());
586586

587-
if (Boolean.TRUE.equals(parameter.getRequired()) && parameterInfo.getMethodParameter() instanceof DelegatingMethodParameter delegatingMethodParameter && delegatingMethodParameter.isNotRequired())
588-
parameter.setRequired(false);
589-
590587
if (containsDeprecatedAnnotation(parameterInfo.getMethodParameter().getParameterAnnotations()))
591588
parameter.setDeprecated(true);
592589

@@ -612,15 +609,16 @@ public Parameter buildParam(ParameterInfo parameterInfo, Components components,
612609
/**
613610
* Apply bean validator annotations.
614611
*
615-
* @param parameter the parameter
616-
* @param annotations the annotations
612+
* @param parameter the parameter
613+
* @param annotations the annotations
614+
* @param isParameterObject the is parameter object
617615
*/
618-
public void applyBeanValidatorAnnotations(final Parameter parameter, final List<Annotation> annotations) {
616+
public void applyBeanValidatorAnnotations(final Parameter parameter, final List<Annotation> annotations, final boolean isParameterObject) {
619617
Map<String, Annotation> annos = new HashMap<>();
620618
if (annotations != null)
621619
annotations.forEach(annotation -> annos.put(annotation.annotationType().getSimpleName(), annotation));
622620
boolean annotationExists = hasNotNullAnnotation(annos.keySet());
623-
if (annotationExists)
621+
if (annotationExists && !isParameterObject)
624622
parameter.setRequired(true);
625623
Schema<?> schema = parameter.getSchema();
626624
applyValidationsToSchema(annos, schema);

‎springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app236/ClassOne.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package test.org.springdoc.api.v30.app236;
2+
3+
4+
import jakarta.validation.Valid;
5+
6+
public class ClassOne {
7+
8+
@Valid
9+
private ClassTwo description;
10+
11+
public @Valid ClassTwo getDescription() {
12+
return description;
13+
}
14+
15+
public void setDescription(@Valid ClassTwo description) {
16+
this.description = description;
17+
}
18+
}

‎springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app236/ClassTwo.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.org.springdoc.api.v30.app236;
2+
3+
4+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
5+
import jakarta.validation.constraints.NotNull;
6+
7+
@JsonIgnoreProperties(ignoreUnknown = true)
8+
public class ClassTwo {
9+
10+
@NotNull
11+
private String name;
12+
13+
public @NotNull String getName() {
14+
return name;
15+
}
16+
17+
public void setName(@NotNull String name) {
18+
this.name = name;
19+
}
20+
}

‎springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app236/HelloController.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package test.org.springdoc.api.v30.app236;
2+
3+
4+
import jakarta.validation.Valid;
5+
import org.springdoc.core.annotations.ParameterObject;
6+
import org.springframework.http.MediaType;
7+
import org.springframework.http.ResponseEntity;
8+
import org.springframework.web.bind.annotation.PostMapping;
9+
import org.springframework.web.bind.annotation.RequestBody;
10+
import org.springframework.web.bind.annotation.RestController;
11+
12+
@RestController
13+
public class HelloController {
14+
15+
@PostMapping(path = "/testOne", consumes = MediaType.APPLICATION_JSON_VALUE)
16+
ResponseEntity<String> testOne(@ParameterObject @Valid ClassOne test) {
17+
return ResponseEntity.ok("ok");
18+
}
19+
20+
@PostMapping(path = "/testTwo", consumes = MediaType.APPLICATION_JSON_VALUE)
21+
ResponseEntity<String> testTwo(@RequestBody ClassOne test) {
22+
return ResponseEntity.ok("ok");
23+
}
24+
}

‎springdoc-openapi-starter-webmvc-api/src/test/java/test/org/springdoc/api/v30/app236/SpringDocApp236Test.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
27+
package test.org.springdoc.api.v30.app236;
28+
29+
import test.org.springdoc.api.v30.AbstractSpringDocV30Test;
30+
31+
import org.springframework.boot.autoconfigure.SpringBootApplication;
32+
33+
public class SpringDocApp236Test extends AbstractSpringDocV30Test {
34+
35+
@SpringBootApplication
36+
static class SpringDocTestApp {}
37+
}

0 commit comments

Comments
 (0)