Skip to content

Commit f44a815

Browse files
bnasslahsenbnasslahsen
committed
code review
1 parent 6b50ccf commit f44a815

6 files changed

Lines changed: 99 additions & 69 deletions

File tree

‎springdoc-openapi-starter-common/src/main/java/org/springdoc/api/AbstractOpenApiResource.java‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ public abstract class AbstractOpenApiResource extends SpecFilter {
224224
*/
225225
private final Pattern pathPattern = Pattern.compile("\\{(.*?)}");
226226

227+
/**
228+
* The resolved group config.
229+
*/
230+
private final GroupConfig resolvedGroupConfig;
231+
227232
/**
228233
* The Open api builder.
229234
*/
@@ -256,6 +261,10 @@ protected AbstractOpenApiResource(String groupName, ObjectFactory<OpenAPIService
256261
this.springDocProviders = springDocProviders;
257262
this.springDocCustomizers = springDocCustomizers;
258263
this.springDocConfigProperties = springDocConfigProperties;
264+
this.resolvedGroupConfig = springDocConfigProperties.getGroupConfigs().stream()
265+
.filter(groupConfig -> this.groupName.equals(groupConfig.getGroup()))
266+
.findAny()
267+
.orElse(null);
259268
if (springDocConfigProperties.isPreLoadingEnabled()) {
260269
if (CollectionUtils.isEmpty(springDocConfigProperties.getPreLoadingLocales())) {
261270
Executors.newSingleThreadExecutor().execute(this::getOpenApi);
@@ -324,7 +333,11 @@ public static boolean containsResponseBody(HandlerMethod handlerMethod) {
324333
* @return the boolean
325334
*/
326335
public static boolean isHiddenRestControllers(Class<?> rawClass) {
327-
return HIDDEN_REST_CONTROLLERS.stream().anyMatch(clazz -> ClassUtils.getUserClass(clazz).isAssignableFrom(rawClass));
336+
List<Class<?>> snapshot;
337+
synchronized (HIDDEN_REST_CONTROLLERS) {
338+
snapshot = new ArrayList<>(HIDDEN_REST_CONTROLLERS);
339+
}
340+
return snapshot.stream().anyMatch(clazz -> ClassUtils.getUserClass(clazz).isAssignableFrom(rawClass));
328341
}
329342

330343
/**
@@ -910,10 +923,8 @@ protected boolean isMethodToFilter(HandlerMethod handlerMethod) {
910923
*/
911924
protected boolean isConditionToMatch(String[] existingConditions, ConditionType conditionType) {
912925
List<String> conditionsToMatch = getConditionsToMatch(conditionType);
913-
if (CollectionUtils.isEmpty(conditionsToMatch)) {
914-
Optional<GroupConfig> optionalGroupConfig = springDocConfigProperties.getGroupConfigs().stream().filter(groupConfig -> this.groupName.equals(groupConfig.getGroup())).findAny();
915-
if (optionalGroupConfig.isPresent())
916-
conditionsToMatch = getConditionsToMatch(conditionType, optionalGroupConfig.get());
926+
if (CollectionUtils.isEmpty(conditionsToMatch) && resolvedGroupConfig != null) {
927+
conditionsToMatch = getConditionsToMatch(conditionType, resolvedGroupConfig);
917928
}
918929
return CollectionUtils.isEmpty(conditionsToMatch)
919930
|| (!ArrayUtils.isEmpty(existingConditions) && conditionsToMatch.size() == existingConditions.length && conditionsToMatch.containsAll(Arrays.asList(existingConditions)));
@@ -931,15 +942,11 @@ protected boolean isPackageToScan(Package aPackage) {
931942
final String packageName = aPackage.getName();
932943
List<String> packagesToScan = springDocConfigProperties.getPackagesToScan();
933944
List<String> packagesToExclude = springDocConfigProperties.getPackagesToExclude();
934-
if (CollectionUtils.isEmpty(packagesToScan)) {
935-
Optional<GroupConfig> optionalGroupConfig = springDocConfigProperties.getGroupConfigs().stream().filter(groupConfig -> this.groupName.equals(groupConfig.getGroup())).findAny();
936-
if (optionalGroupConfig.isPresent())
937-
packagesToScan = optionalGroupConfig.get().getPackagesToScan();
945+
if (CollectionUtils.isEmpty(packagesToScan) && resolvedGroupConfig != null) {
946+
packagesToScan = resolvedGroupConfig.getPackagesToScan();
938947
}
939-
if (CollectionUtils.isEmpty(packagesToExclude)) {
940-
Optional<GroupConfig> optionalGroupConfig = springDocConfigProperties.getGroupConfigs().stream().filter(groupConfig -> this.groupName.equals(groupConfig.getGroup())).findAny();
941-
if (optionalGroupConfig.isPresent())
942-
packagesToExclude = optionalGroupConfig.get().getPackagesToExclude();
948+
if (CollectionUtils.isEmpty(packagesToExclude) && resolvedGroupConfig != null) {
949+
packagesToExclude = resolvedGroupConfig.getPackagesToExclude();
943950
}
944951
boolean include = CollectionUtils.isEmpty(packagesToScan)
945952
|| packagesToScan.stream().anyMatch(pack -> packageName.equals(pack)
@@ -960,15 +967,11 @@ protected boolean isPackageToScan(Package aPackage) {
960967
protected boolean isPathToMatch(String operationPath) {
961968
List<String> pathsToMatch = springDocConfigProperties.getPathsToMatch();
962969
List<String> pathsToExclude = springDocConfigProperties.getPathsToExclude();
963-
if (CollectionUtils.isEmpty(pathsToMatch)) {
964-
Optional<GroupConfig> optionalGroupConfig = springDocConfigProperties.getGroupConfigs().stream().filter(groupConfig -> this.groupName.equals(groupConfig.getGroup())).findAny();
965-
if (optionalGroupConfig.isPresent())
966-
pathsToMatch = optionalGroupConfig.get().getPathsToMatch();
970+
if (CollectionUtils.isEmpty(pathsToMatch) && resolvedGroupConfig != null) {
971+
pathsToMatch = resolvedGroupConfig.getPathsToMatch();
967972
}
968-
if (CollectionUtils.isEmpty(pathsToExclude)) {
969-
Optional<GroupConfig> optionalGroupConfig = springDocConfigProperties.getGroupConfigs().stream().filter(groupConfig -> this.groupName.equals(groupConfig.getGroup())).findAny();
970-
if (optionalGroupConfig.isPresent())
971-
pathsToExclude = optionalGroupConfig.get().getPathsToExclude();
973+
if (CollectionUtils.isEmpty(pathsToExclude) && resolvedGroupConfig != null) {
974+
pathsToExclude = resolvedGroupConfig.getPathsToExclude();
972975
}
973976
boolean include = CollectionUtils.isEmpty(pathsToMatch) || pathsToMatch.stream().anyMatch(pattern -> antPathMatcher.match(pattern, operationPath));
974977
boolean exclude = !CollectionUtils.isEmpty(pathsToExclude) && pathsToExclude.stream().anyMatch(pattern -> antPathMatcher.match(pattern, operationPath));
@@ -997,7 +1000,11 @@ protected String decode(String requestURI) {
9971000
* @return the boolean
9981001
*/
9991002
protected boolean isAdditionalRestController(Class<?> rawClass) {
1000-
return ADDITIONAL_REST_CONTROLLERS.stream().anyMatch(clazz -> ClassUtils.getUserClass(clazz).isAssignableFrom(rawClass));
1003+
List<Class<?>> snapshot;
1004+
synchronized (ADDITIONAL_REST_CONTROLLERS) {
1005+
snapshot = new ArrayList<>(ADDITIONAL_REST_CONTROLLERS);
1006+
}
1007+
return snapshot.stream().anyMatch(clazz -> ClassUtils.getUserClass(clazz).isAssignableFrom(rawClass));
10011008
}
10021009

10031010
/**

‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/converters/PolymorphicModelConverter.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@
3030
import java.lang.reflect.Modifier;
3131
import java.util.ArrayList;
3232
import java.util.Collection;
33-
import java.util.Collections;
34-
import java.util.HashSet;
3533
import java.util.Iterator;
3634
import java.util.List;
3735
import java.util.Map;
3836
import java.util.Set;
37+
import java.util.concurrent.CopyOnWriteArraySet;
3938

4039
import com.fasterxml.jackson.annotation.JsonUnwrapped;
4140
import com.fasterxml.jackson.databind.JavaType;
@@ -65,12 +64,12 @@ public class PolymorphicModelConverter implements ModelConverter {
6564
/**
6665
* The constant PARENT_TYPES_TO_IGNORE.
6766
*/
68-
private static final Set<String> PARENT_TYPES_TO_IGNORE = Collections.synchronizedSet(new HashSet<>());
67+
private static final Set<String> PARENT_TYPES_TO_IGNORE = new CopyOnWriteArraySet<>();
6968

7069
/**
71-
* The constant PARENT_TYPES_TO_IGNORE.
70+
* The constant TYPES_TO_SKIP.
7271
*/
73-
private static final Set<String> TYPES_TO_SKIP = Collections.synchronizedSet(new HashSet<>());
72+
private static final Set<String> TYPES_TO_SKIP = new CopyOnWriteArraySet<>();
7473

7574
static {
7675
PARENT_TYPES_TO_IGNORE.add("JsonSchema");

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

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import java.time.ZoneOffset;
4747
import java.util.ArrayList;
4848
import java.util.Arrays;
49+
import java.util.Collections;
4950
import java.util.HashSet;
5051
import java.util.List;
5152
import java.util.Map;
@@ -59,6 +60,7 @@
5960
import java.util.concurrent.atomic.AtomicInteger;
6061
import java.util.concurrent.atomic.AtomicLong;
6162
import java.util.function.Predicate;
63+
import java.util.stream.Collectors;
6264
import java.util.stream.Stream;
6365

6466
import io.swagger.v3.core.util.PrimitiveType;
@@ -153,9 +155,18 @@ Stream<MethodParameter> extractFrom(Class<?> clazz) {
153155
* @return the stream
154156
*/
155157
private Stream<MethodParameter> extractFrom(Class<?> clazz, String fieldNamePrefix, boolean parentRequired) {
158+
Map<String, PropertyDescriptor> propertyDescriptorMap;
159+
try {
160+
propertyDescriptorMap = Arrays.stream(Introspector.getBeanInfo(clazz).getPropertyDescriptors())
161+
.collect(Collectors.toMap(PropertyDescriptor::getName, pd -> pd, (a, b) -> a));
162+
}
163+
catch (IntrospectionException e) {
164+
propertyDescriptorMap = Collections.emptyMap();
165+
}
166+
Map<String, PropertyDescriptor> finalPropertyDescriptorMap = propertyDescriptorMap;
156167
return allFieldsOf(clazz).stream()
157168
.filter(field -> !field.getType().equals(clazz))
158-
.flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix, parentRequired))
169+
.flatMap(f -> fromGetterOfField(clazz, f, fieldNamePrefix, parentRequired, finalPropertyDescriptorMap))
159170
.filter(Objects::nonNull);
160171
}
161172

@@ -168,14 +179,14 @@ private Stream<MethodParameter> extractFrom(Class<?> clazz, String fieldNamePref
168179
* @param parentRequired whether the field that holds the class currently being examined was required or optional
169180
* @return the stream
170181
*/
171-
private Stream<MethodParameter> fromGetterOfField(Class<?> paramClass, Field field, String fieldNamePrefix, boolean parentRequired) {
182+
private Stream<MethodParameter> fromGetterOfField(Class<?> paramClass, Field field, String fieldNamePrefix, boolean parentRequired, Map<String, PropertyDescriptor> propertyDescriptorMap) {
172183
Class<?> type = extractType(paramClass, field);
173184

174185
if (Objects.isNull(type))
175186
return Stream.empty();
176187

177188
if (isSimpleType(type))
178-
return fromSimpleClass(paramClass, field, fieldNamePrefix, parentRequired);
189+
return fromSimpleClass(paramClass, field, fieldNamePrefix, parentRequired, propertyDescriptorMap);
179190
else {
180191
Parameter parameter = field.getAnnotation(Parameter.class);
181192
Schema schema = field.getAnnotation(Schema.class);
@@ -242,33 +253,28 @@ private static Class<?> extractType(Class<?> paramClass, Field field) {
242253
* @param fieldNamePrefix the field name prefix
243254
* @return the stream
244255
*/
245-
private Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Field field, String fieldNamePrefix, boolean parentRequired) {
256+
private Stream<MethodParameter> fromSimpleClass(Class<?> paramClass, Field field, String fieldNamePrefix, boolean parentRequired, Map<String, PropertyDescriptor> propertyDescriptorMap) {
246257
Annotation[] fieldAnnotations = field.getDeclaredAnnotations();
247-
try {
248-
Parameter parameter = field.getAnnotation(Parameter.class);
249-
Schema schema = field.getAnnotation(Schema.class);
250-
boolean fieldRequired = this.schemaUtils.fieldRequired(field, schema, parameter);
258+
Parameter parameter = field.getAnnotation(Parameter.class);
259+
Schema schema = field.getAnnotation(Schema.class);
260+
boolean fieldRequired = this.schemaUtils.fieldRequired(field, schema, parameter);
251261

252-
boolean paramRequired = parentRequired && fieldRequired;
253-
if (paramClass.getSuperclass() != null && paramClass.isRecord()) {
254-
return Stream.of(paramClass.getRecordComponents())
255-
.filter(d -> d.getName().equals(field.getName()))
256-
.map(RecordComponent::getAccessor)
257-
.map(method -> new MethodParameter(method, -1))
258-
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
259-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, field, !paramRequired));
260-
}
261-
else
262-
return Stream.of(Introspector.getBeanInfo(paramClass).getPropertyDescriptors())
263-
.filter(d -> d.getName().equals(field.getName()))
264-
.map(PropertyDescriptor::getReadMethod)
265-
.filter(Objects::nonNull)
266-
.map(method -> new MethodParameter(method, -1))
267-
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
268-
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, field, !paramRequired));
262+
boolean paramRequired = parentRequired && fieldRequired;
263+
if (paramClass.getSuperclass() != null && paramClass.isRecord()) {
264+
return Stream.of(paramClass.getRecordComponents())
265+
.filter(d -> d.getName().equals(field.getName()))
266+
.map(RecordComponent::getAccessor)
267+
.map(method -> new MethodParameter(method, -1))
268+
.map(methodParameter -> DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass))
269+
.map(param -> new DelegatingMethodParameter(param, fieldNamePrefix + field.getName(), fieldAnnotations, param.getMethodAnnotations(), true, field, !paramRequired));
269270
}
270-
catch (IntrospectionException e) {
271-
return Stream.of();
271+
else {
272+
PropertyDescriptor pd = propertyDescriptorMap.get(field.getName());
273+
if (pd == null || pd.getReadMethod() == null)
274+
return Stream.of();
275+
MethodParameter methodParameter = new MethodParameter(pd.getReadMethod(), -1);
276+
methodParameter = DelegatingMethodParameter.changeContainingClass(methodParameter, paramClass);
277+
return Stream.of(new DelegatingMethodParameter(methodParameter, fieldNamePrefix + field.getName(), fieldAnnotations, methodParameter.getMethodAnnotations(), true, field, !paramRequired));
272278
}
273279
}
274280

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.util.function.Consumer;
4646
import java.util.function.Supplier;
4747
import java.util.stream.Collectors;
48+
import java.util.regex.Pattern;
4849
import java.util.stream.Stream;
4950

5051
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -113,6 +114,12 @@ public class OpenAPIService implements ApplicationContextAware {
113114
*/
114115
private static final Logger LOGGER = LoggerFactory.getLogger(OpenAPIService.class);
115116

117+
/**
118+
* The constant CAMEL_CASE_PATTERN.
119+
*/
120+
private static final Pattern CAMEL_CASE_PATTERN = Pattern.compile(
121+
"(?<=[A-Z])(?=[A-Z][a-z])|(?<=[^A-Z])(?=[A-Z])|(?<=[A-Za-z])(?=[^A-Za-z])");
122+
116123
/**
117124
* The Basic error controller.
118125
*/
@@ -220,13 +227,7 @@ public OpenAPIService(Optional<OpenAPI> openAPI, SecurityService securityParser,
220227
* @return the string
221228
*/
222229
public static String splitCamelCase(String str) {
223-
return str.replaceAll(
224-
String.format(
225-
"%s|%s|%s",
226-
"(?<=[A-Z])(?=[A-Z][a-z])",
227-
"(?<=[^A-Z])(?=[A-Z])",
228-
"(?<=[A-Za-z])(?=[^A-Za-z])"),
229-
"-")
230+
return CAMEL_CASE_PATTERN.matcher(str).replaceAll("-")
230231
.toLowerCase(Locale.ROOT);
231232
}
232233

‎springdoc-openapi-starter-common/src/main/java/org/springdoc/core/utils/SpringDocAnnotationsUtils.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,15 +283,20 @@ else if (firstSchema instanceof ComposedSchema) {
283283
*/
284284
@SuppressWarnings("unchecked")
285285
public static boolean isAnnotationToIgnore(MethodParameter parameter) {
286-
boolean annotationFirstCheck = ANNOTATIONS_TO_IGNORE.stream().anyMatch(annotation ->
286+
List<Class> snapshot;
287+
synchronized (ANNOTATIONS_TO_IGNORE) {
288+
snapshot = new ArrayList<>(ANNOTATIONS_TO_IGNORE);
289+
}
290+
boolean annotationFirstCheck = snapshot.stream().anyMatch(annotation ->
287291
(parameter.getParameterIndex() != -1 && AnnotationUtils.findAnnotation(parameter.getMethod().getParameters()[parameter.getParameterIndex()], annotation) != null)
288292
|| AnnotationUtils.findAnnotation(parameter.getParameterType(), annotation) != null);
289293

290-
boolean annotationSecondCheck = Arrays.stream(parameter.getParameterAnnotations()).anyMatch(annotation ->
291-
ANNOTATIONS_TO_IGNORE.contains(annotation.annotationType())
292-
|| ANNOTATIONS_TO_IGNORE.stream().anyMatch(annotationToIgnore -> annotation.annotationType().getDeclaredAnnotation(annotationToIgnore) != null));
294+
if (annotationFirstCheck)
295+
return true;
293296

294-
return annotationFirstCheck || annotationSecondCheck;
297+
return Arrays.stream(parameter.getParameterAnnotations()).anyMatch(annotation ->
298+
snapshot.contains(annotation.annotationType())
299+
|| snapshot.stream().anyMatch(annotationToIgnore -> annotation.annotationType().getDeclaredAnnotation(annotationToIgnore) != null));
295300
}
296301

297302
/**
@@ -301,7 +306,11 @@ public static boolean isAnnotationToIgnore(MethodParameter parameter) {
301306
* @return the boolean
302307
*/
303308
public static boolean isAnnotationToIgnore(Type type) {
304-
return ANNOTATIONS_TO_IGNORE.stream().anyMatch(
309+
List<Class> snapshot;
310+
synchronized (ANNOTATIONS_TO_IGNORE) {
311+
snapshot = new ArrayList<>(ANNOTATIONS_TO_IGNORE);
312+
}
313+
return snapshot.stream().anyMatch(
305314
annotation -> (type instanceof Class
306315
&& AnnotationUtils.findAnnotation((Class<?>) type, annotation) != null));
307316
}

‎springdoc-openapi-starter-common/src/test/java/org/springdoc/api/AbstractOpenApiResourceTest.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,11 @@ properties, springDocProviders, new SpringDocCustomizers(Optional.of(singleton(o
212212
);
213213

214214
// wait for executor to be done
215-
Thread.sleep(1_000);
215+
long deadline = System.currentTimeMillis() + 5_000;
216+
while (openAPIService.getCachedOpenAPI(Locale.getDefault()) == null
217+
&& System.currentTimeMillis() < deadline) {
218+
Thread.sleep(50);
219+
}
216220

217221
// emulate generating base url
218222
String serverBaseUrl = openAPIService.calculateServerBaseUrl(generatedUrl, new MockClientHttpRequest());
@@ -244,7 +248,11 @@ springDocProviders, new SpringDocCustomizers(Optional.empty(), Optional.empty(),
244248
);
245249

246250
// wait for executor to be done
247-
Thread.sleep(1_000);
251+
long deadline = System.currentTimeMillis() + 5_000;
252+
while (openAPIService.getCachedOpenAPI(Locale.getDefault()) == null
253+
&& System.currentTimeMillis() < deadline) {
254+
Thread.sleep(50);
255+
}
248256

249257
Locale locale = Locale.US;
250258

0 commit comments

Comments
 (0)