Skip to content

Commit ece6158

Browse files
SirCotareSirCotare
committed
fix nullability customizer
1 parent 6ac1a28 commit ece6158

File tree

3 files changed

+255
-83
lines changed

3 files changed

+255
-83
lines changed

‎src/main/java/it/aboutbits/springboot/toolbox/swagger/customization/default_not_null/NullableCustomizer.java‎

Lines changed: 98 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22

33
import io.swagger.v3.oas.models.OpenAPI;
44
import io.swagger.v3.oas.models.media.Schema;
5+
import org.jspecify.annotations.NullMarked;
56
import org.springdoc.core.customizers.OpenApiCustomizer;
67

8+
import java.lang.annotation.Annotation;
9+
import java.lang.reflect.AnnotatedType;
710
import java.util.ArrayList;
811
import java.util.Map;
912

13+
@NullMarked
1014
public class NullableCustomizer implements OpenApiCustomizer {
11-
public static final String NULLABLE_MARKER = "NULLABLE";
12-
1315
@Override
1416
@SuppressWarnings("unchecked")
1517
public void customise(OpenAPI openApi) {
@@ -21,57 +23,129 @@ public void customise(OpenAPI openApi) {
2123
var requiredProperties = new ArrayList<String>();
2224
if (((Schema<?>) schema).getProperties() != null) {
2325
var properties = ((Schema<?>) schema).getProperties();
24-
processProperties(properties, requiredProperties);
26+
processProperties(schema.getName(), properties, requiredProperties);
2527
}
2628
if (schema.getAllOf() != null) {
2729
schema.getAllOf().forEach(allOfSchema -> {
2830
var allOfSchemaTyped = (Schema<?>) allOfSchema;
2931
if (allOfSchemaTyped.getProperties() != null) {
3032
var properties = allOfSchemaTyped.getProperties();
31-
processProperties(properties, requiredProperties);
33+
processProperties(schema.getName(), properties, requiredProperties);
3234
}
3335
});
3436
}
3537
schema.setRequired(requiredProperties);
3638
});
3739
}
3840

39-
private static void processProperties(Map<String, Schema> properties, ArrayList<String> requiredProperties) {
41+
@SuppressWarnings("rawtypes")
42+
private static void processProperties(
43+
String modelFqn,
44+
Map<String, Schema> properties,
45+
ArrayList<String> requiredProperties
46+
) {
47+
var cls = loadClass(modelFqn);
48+
if (cls == null) {
49+
return;
50+
}
51+
4052
properties.forEach((propertyName, property) -> {
41-
var isNullable = isNullable(property);
53+
var isNullable = isNullable(cls, propertyName);
4254

4355
if (!isNullable) {
4456
requiredProperties.add(propertyName);
4557
} else {
4658
requiredProperties.remove(propertyName);
4759
}
48-
if (property.getTitle() != null && property.getTitle().equals(NULLABLE_MARKER)) {
49-
property.setTitle(null);
50-
}
51-
if (property.get$ref() != null) {
52-
property.set$ref(property.get$ref().replace(NULLABLE_MARKER, ""));
53-
}
54-
if (property.getItems() != null && property.getItems().get$ref() != null) {
55-
property.getItems().set$ref(property.getItems().get$ref().replace(NULLABLE_MARKER, ""));
56-
}
5760
});
5861
}
5962

60-
private static boolean isNullable(Schema<?> property) {
61-
if (property.getTitle() != null && property.getTitle().equals(NULLABLE_MARKER)) {
62-
return true;
63+
@org.jspecify.annotations.Nullable
64+
private static Class<?> loadClass(String fqn) {
65+
try {
66+
return Class.forName(fqn);
67+
} catch (ClassNotFoundException _) {
68+
// if this does not work, we probably have a parameterized type where the fqn is concatenated
6369
}
6470

65-
if (property.get$ref() != null && property.get$ref().endsWith(NULLABLE_MARKER)) {
66-
return true;
71+
var lastDotIndex = -1;
72+
for (var i = 0; i <= fqn.length(); i++) {
73+
if (i == fqn.length() || fqn.charAt(i) == '.') {
74+
var fullPart = fqn.substring(lastDotIndex + 1, i);
75+
if (!fullPart.isEmpty() && Character.isUpperCase(fullPart.charAt(0))) {
76+
// Try the full part first
77+
var baseFqn = fqn.substring(0, i);
78+
try {
79+
return Class.forName(baseFqn);
80+
} catch (ClassNotFoundException _) {
81+
}
82+
83+
// Try stripping capitalized segments from the end of the part
84+
// e.g., LabelAndDescriptionChoiceCom -> try LabelAndDescriptionChoice, then LabelAndDescription, etc.
85+
for (var j = fullPart.length() - 1; j > 0; j--) {
86+
if (Character.isUpperCase(fullPart.charAt(j))) {
87+
var strippedPart = fullPart.substring(0, j);
88+
var candidateFqn = fqn.substring(0, lastDotIndex + 1) + strippedPart;
89+
try {
90+
return Class.forName(candidateFqn);
91+
} catch (ClassNotFoundException _) {
92+
}
93+
}
94+
}
95+
}
96+
lastDotIndex = i;
97+
}
6798
}
99+
return null;
100+
}
68101

69-
if (property.getItems() != null && property.getItems().get$ref() != null && property.getItems()
70-
.get$ref()
71-
.endsWith("?nullable=true")) {
72-
return true;
102+
private static boolean isNullable(Class<?> cls, String propertyName) {
103+
var currentClass = cls;
104+
while (currentClass != null) {
105+
try {
106+
var field = currentClass.getDeclaredField(propertyName);
107+
if (isNullable(field.getAnnotatedType(), field.getAnnotations())) {
108+
return true;
109+
}
110+
} catch (NoSuchFieldException _) {
111+
}
112+
113+
for (var method : currentClass.getDeclaredMethods()) {
114+
if (method.getName().equals(propertyName)
115+
|| method.getName().equals("get" + capitalize(propertyName))
116+
|| method.getName().equals("is" + capitalize(propertyName))) {
117+
if (isNullable(method.getAnnotatedReturnType(), method.getAnnotations())) {
118+
return true;
119+
}
120+
}
121+
}
122+
123+
currentClass = currentClass.getSuperclass();
73124
}
74125

75126
return false;
76127
}
128+
129+
private static boolean isNullable(
130+
AnnotatedType annotatedType,
131+
Annotation[] annotations
132+
) {
133+
if (annotatedType.isAnnotationPresent(org.jspecify.annotations.Nullable.class)) {
134+
return true;
135+
}
136+
for (var annotation : annotations) {
137+
var name = annotation.annotationType().getName();
138+
if (name.equals("org.springframework.lang.Nullable") || name.equals("jakarta.annotation.Nullable")) {
139+
return true;
140+
}
141+
}
142+
return false;
143+
}
144+
145+
private static String capitalize(String str) {
146+
if (str.isEmpty()) {
147+
return str;
148+
}
149+
return str.substring(0, 1).toUpperCase() + str.substring(1);
150+
}
77151
}

‎src/main/java/it/aboutbits/springboot/toolbox/swagger/customization/default_not_null/NullablePropertyCustomizer.java‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

‎src/test/java/it/aboutbits/springboot/toolbox/swagger/customization/default_not_null/NullableCustomizerTest.java‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
package it.aboutbits.springboot.toolbox.swagger.customization.default_not_null;
2+
3+
import io.swagger.v3.oas.models.Components;
4+
import io.swagger.v3.oas.models.OpenAPI;
5+
import io.swagger.v3.oas.models.media.Schema;
6+
import io.swagger.v3.oas.models.media.StringSchema;
7+
import org.jspecify.annotations.NullUnmarked;
8+
import org.junit.jupiter.api.Test;
9+
10+
import java.util.List;
11+
12+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
13+
import static org.junit.jupiter.api.Assertions.assertTrue;
14+
15+
@NullUnmarked
16+
class NullableCustomizerTest {
17+
18+
public static class BaseClass {
19+
@org.springframework.lang.Nullable
20+
private String baseField;
21+
22+
public String getBaseField() {
23+
return baseField;
24+
}
25+
}
26+
27+
public static class SubClass extends BaseClass {
28+
private String subField;
29+
30+
public String getSubField() {
31+
return subField;
32+
}
33+
}
34+
35+
public static class MethodAnnotated {
36+
private String annotatedGetter;
37+
38+
@jakarta.annotation.Nullable
39+
public String getAnnotatedGetter() {
40+
return annotatedGetter;
41+
}
42+
}
43+
44+
public static class DirectMethodAnnotated {
45+
private String directMethod;
46+
47+
@org.jspecify.annotations.Nullable
48+
public String directMethod() {
49+
return directMethod;
50+
}
51+
}
52+
53+
@Test
54+
void shouldFindFieldInSuperClass() {
55+
var customizer = new NullableCustomizer();
56+
var openApi = new OpenAPI();
57+
var components = new Components();
58+
59+
var subClassSchema = new Schema<Object>();
60+
subClassSchema.setName(SubClass.class.getName());
61+
subClassSchema.addProperty("baseField", new StringSchema());
62+
subClassSchema.addProperty("subField", new StringSchema());
63+
64+
components.addSchemas(SubClass.class.getName(), subClassSchema);
65+
openApi.setComponents(components);
66+
67+
assertDoesNotThrow(() -> customizer.customise(openApi));
68+
69+
List<String> required = subClassSchema.getRequired();
70+
assertTrue(required != null && required.contains("subField"), "subField should be required");
71+
assertTrue(required == null || !required.contains("baseField"), "baseField should NOT be required");
72+
}
73+
74+
@Test
75+
void shouldFindAnnotationOnGetter() {
76+
var customizer = new NullableCustomizer();
77+
var openApi = new OpenAPI();
78+
var components = new Components();
79+
80+
var schema = new Schema<Object>();
81+
schema.setName(MethodAnnotated.class.getName());
82+
schema.addProperty("annotatedGetter", new StringSchema());
83+
84+
components.addSchemas(MethodAnnotated.class.getName(), schema);
85+
openApi.setComponents(components);
86+
87+
assertDoesNotThrow(() -> customizer.customise(openApi));
88+
89+
List<String> required = schema.getRequired();
90+
assertTrue(required == null || !required.contains("annotatedGetter"), "annotatedGetter should NOT be required");
91+
}
92+
93+
@Test
94+
void shouldFindAnnotationOnDirectMethod() {
95+
var customizer = new NullableCustomizer();
96+
var openApi = new OpenAPI();
97+
var components = new Components();
98+
99+
var schema = new Schema<Object>();
100+
schema.setName(DirectMethodAnnotated.class.getName());
101+
schema.addProperty("directMethod", new StringSchema());
102+
103+
components.addSchemas(DirectMethodAnnotated.class.getName(), schema);
104+
openApi.setComponents(components);
105+
106+
assertDoesNotThrow(() -> customizer.customise(openApi));
107+
108+
List<String> required = schema.getRequired();
109+
assertTrue(required == null || !required.contains("directMethod"), "directMethod should NOT be required");
110+
}
111+
112+
@Test
113+
void shouldHandleConcatenatedFqns() {
114+
var customizer = new NullableCustomizer();
115+
var openApi = new OpenAPI();
116+
var components = new Components();
117+
118+
var schema = new Schema<Object>();
119+
// Simulating the concatenated FQN pattern described in the issue
120+
var concatenatedFqn = SubClass.class.getName() + "Com.finstral.something";
121+
schema.setName(concatenatedFqn);
122+
schema.addProperty("baseField", new StringSchema());
123+
124+
components.addSchemas(concatenatedFqn, schema);
125+
openApi.setComponents(components);
126+
127+
assertDoesNotThrow(() -> customizer.customise(openApi));
128+
129+
List<String> required = schema.getRequired();
130+
assertTrue(
131+
required == null || !required.contains("baseField"),
132+
"baseField should NOT be required even with concatenated FQN"
133+
);
134+
}
135+
136+
@Test
137+
void shouldNotThrowWhenFieldNotFound() {
138+
var customizer = new NullableCustomizer();
139+
var openApi = new OpenAPI();
140+
var components = new Components();
141+
142+
var schema = new Schema<Object>();
143+
schema.setName(SubClass.class.getName());
144+
schema.addProperty("nonExistent", new StringSchema());
145+
146+
components.addSchemas(SubClass.class.getName(), schema);
147+
openApi.setComponents(components);
148+
149+
assertDoesNotThrow(() -> customizer.customise(openApi));
150+
151+
List<String> required = schema.getRequired();
152+
assertTrue(
153+
required != null && required.contains("nonExistent"),
154+
"nonExistent field should be considered required if not found and not nullable"
155+
);
156+
}
157+
}

0 commit comments

Comments
 (0)