Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.Schema;
import org.jspecify.annotations.NullMarked;
import org.springdoc.core.customizers.OpenApiCustomizer;

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedType;
import java.util.ArrayList;
import java.util.Map;

@NullMarked
public class NullableCustomizer implements OpenApiCustomizer {
public static final String NULLABLE_MARKER = "NULLABLE";

@Override
@SuppressWarnings("unchecked")
public void customise(OpenAPI openApi) {
Expand All @@ -21,57 +23,129 @@ public void customise(OpenAPI openApi) {
var requiredProperties = new ArrayList<String>();
if (((Schema<?>) schema).getProperties() != null) {
var properties = ((Schema<?>) schema).getProperties();
processProperties(properties, requiredProperties);
processProperties(schema.getName(), properties, requiredProperties);
}
if (schema.getAllOf() != null) {
schema.getAllOf().forEach(allOfSchema -> {
var allOfSchemaTyped = (Schema<?>) allOfSchema;
if (allOfSchemaTyped.getProperties() != null) {
var properties = allOfSchemaTyped.getProperties();
processProperties(properties, requiredProperties);
processProperties(schema.getName(), properties, requiredProperties);
}
});
}
schema.setRequired(requiredProperties);
});
}

private static void processProperties(Map<String, Schema> properties, ArrayList<String> requiredProperties) {
@SuppressWarnings("rawtypes")
private static void processProperties(
String modelFqn,
Map<String, Schema> properties,
ArrayList<String> requiredProperties
) {
var cls = loadClass(modelFqn);
if (cls == null) {
return;
}

properties.forEach((propertyName, property) -> {
var isNullable = isNullable(property);
var isNullable = isNullable(cls, propertyName);

if (!isNullable) {
requiredProperties.add(propertyName);
} else {
requiredProperties.remove(propertyName);
}
if (property.getTitle() != null && property.getTitle().equals(NULLABLE_MARKER)) {
property.setTitle(null);
}
if (property.get$ref() != null) {
property.set$ref(property.get$ref().replace(NULLABLE_MARKER, ""));
}
if (property.getItems() != null && property.getItems().get$ref() != null) {
property.getItems().set$ref(property.getItems().get$ref().replace(NULLABLE_MARKER, ""));
}
});
}

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

if (property.get$ref() != null && property.get$ref().endsWith(NULLABLE_MARKER)) {
return true;
var lastDotIndex = -1;
for (var i = 0; i <= fqn.length(); i++) {
if (i == fqn.length() || fqn.charAt(i) == '.') {
var fullPart = fqn.substring(lastDotIndex + 1, i);
if (!fullPart.isEmpty() && Character.isUpperCase(fullPart.charAt(0))) {
// Try the full part first
var baseFqn = fqn.substring(0, i);
try {
return Class.forName(baseFqn);
} catch (ClassNotFoundException _) {
}

// Try stripping capitalized segments from the end of the part
// e.g., LabelAndDescriptionChoiceCom -> try LabelAndDescriptionChoice, then LabelAndDescription, etc.
for (var j = fullPart.length() - 1; j > 0; j--) {
if (Character.isUpperCase(fullPart.charAt(j))) {
var strippedPart = fullPart.substring(0, j);
var candidateFqn = fqn.substring(0, lastDotIndex + 1) + strippedPart;
try {
return Class.forName(candidateFqn);
} catch (ClassNotFoundException _) {
}
}
}
}
lastDotIndex = i;
}
}
return null;
}

if (property.getItems() != null && property.getItems().get$ref() != null && property.getItems()
.get$ref()
.endsWith("?nullable=true")) {
return true;
private static boolean isNullable(Class<?> cls, String propertyName) {
var currentClass = cls;
while (currentClass != null) {
try {
var field = currentClass.getDeclaredField(propertyName);
if (isNullable(field.getAnnotatedType(), field.getAnnotations())) {
return true;
}
} catch (NoSuchFieldException _) {
}

for (var method : currentClass.getDeclaredMethods()) {
if (method.getName().equals(propertyName)
|| method.getName().equals("get" + capitalize(propertyName))
|| method.getName().equals("is" + capitalize(propertyName))) {
if (isNullable(method.getAnnotatedReturnType(), method.getAnnotations())) {
return true;
}
}
}

currentClass = currentClass.getSuperclass();
}

return false;
}

private static boolean isNullable(
AnnotatedType annotatedType,
Annotation[] annotations
) {
if (annotatedType.isAnnotationPresent(org.jspecify.annotations.Nullable.class)) {
return true;
}
for (var annotation : annotations) {
var name = annotation.annotationType().getName();
if (name.equals("org.springframework.lang.Nullable") || name.equals("jakarta.annotation.Nullable")) {
return true;
}
}
return false;
}

private static String capitalize(String str) {
if (str.isEmpty()) {
return str;
}
return str.substring(0, 1).toUpperCase() + str.substring(1);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package it.aboutbits.springboot.toolbox.swagger.customization.default_not_null;

import io.swagger.v3.oas.models.Components;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.StringSchema;
import org.jspecify.annotations.NullUnmarked;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertTrue;

@NullUnmarked
class NullableCustomizerTest {

public static class BaseClass {
@org.springframework.lang.Nullable
private String baseField;

public String getBaseField() {
return baseField;
}
}

public static class SubClass extends BaseClass {
private String subField;

public String getSubField() {
return subField;
}
}

public static class MethodAnnotated {
private String annotatedGetter;

@jakarta.annotation.Nullable
public String getAnnotatedGetter() {
return annotatedGetter;
}
}

public static class DirectMethodAnnotated {
private String directMethod;

@org.jspecify.annotations.Nullable
public String directMethod() {
return directMethod;
}
}
Comment on lines +18 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you have a mix of different @Nullable annotations.


@Test
void shouldFindFieldInSuperClass() {
var customizer = new NullableCustomizer();
var openApi = new OpenAPI();
var components = new Components();

var subClassSchema = new Schema<Object>();
subClassSchema.setName(SubClass.class.getName());
subClassSchema.addProperty("baseField", new StringSchema());
subClassSchema.addProperty("subField", new StringSchema());

components.addSchemas(SubClass.class.getName(), subClassSchema);
openApi.setComponents(components);

assertDoesNotThrow(() -> customizer.customise(openApi));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we usually use AssertJ
assertThatCode(() -> call()).doesNotThrowAnyException();
instead of this JUnit one?

Same also for the ones below and assertTrue.


List<String> required = subClassSchema.getRequired();
assertTrue(required != null && required.contains("subField"), "subField should be required");
assertTrue(required == null || !required.contains("baseField"), "baseField should NOT be required");
}

@Test
void shouldFindAnnotationOnGetter() {
var customizer = new NullableCustomizer();
var openApi = new OpenAPI();
var components = new Components();

var schema = new Schema<Object>();
schema.setName(MethodAnnotated.class.getName());
schema.addProperty("annotatedGetter", new StringSchema());

components.addSchemas(MethodAnnotated.class.getName(), schema);
openApi.setComponents(components);

assertDoesNotThrow(() -> customizer.customise(openApi));

List<String> required = schema.getRequired();
assertTrue(required == null || !required.contains("annotatedGetter"), "annotatedGetter should NOT be required");
}

@Test
void shouldFindAnnotationOnDirectMethod() {
var customizer = new NullableCustomizer();
var openApi = new OpenAPI();
var components = new Components();

var schema = new Schema<Object>();
schema.setName(DirectMethodAnnotated.class.getName());
schema.addProperty("directMethod", new StringSchema());

components.addSchemas(DirectMethodAnnotated.class.getName(), schema);
openApi.setComponents(components);

assertDoesNotThrow(() -> customizer.customise(openApi));

List<String> required = schema.getRequired();
assertTrue(required == null || !required.contains("directMethod"), "directMethod should NOT be required");
}

@Test
void shouldHandleConcatenatedFqns() {
var customizer = new NullableCustomizer();
var openApi = new OpenAPI();
var components = new Components();

var schema = new Schema<Object>();
// Simulating the concatenated FQN pattern described in the issue
var concatenatedFqn = SubClass.class.getName() + "Com.finstral.something";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like you have done it in PR commit https://github.com/aboutbits/spring-boot-toolbox/pull/47/changes#diff-ad71dc1ec29d6eb4c590a1e3e61049b7de72b369f976eabf6485d7eef1c210c6R121

Suggested change
var concatenatedFqn = SubClass.class.getName() + "Com.finstral.something";
var concatenatedFqn = SubClass.class.getName() + "It.aboutbits.something";

schema.setName(concatenatedFqn);
schema.addProperty("baseField", new StringSchema());

components.addSchemas(concatenatedFqn, schema);
openApi.setComponents(components);

assertDoesNotThrow(() -> customizer.customise(openApi));

List<String> required = schema.getRequired();
assertTrue(
required == null || !required.contains("baseField"),
"baseField should NOT be required even with concatenated FQN"
);
}

@Test
void shouldNotThrowWhenFieldNotFound() {
var customizer = new NullableCustomizer();
var openApi = new OpenAPI();
var components = new Components();

var schema = new Schema<Object>();
schema.setName(SubClass.class.getName());
schema.addProperty("nonExistent", new StringSchema());

components.addSchemas(SubClass.class.getName(), schema);
openApi.setComponents(components);

assertDoesNotThrow(() -> customizer.customise(openApi));

List<String> required = schema.getRequired();
assertTrue(
required != null && required.contains("nonExistent"),
"nonExistent field should be considered required if not found and not nullable"
);
}
}