Fix behaviuor of required flag for schema class fields. Fixes #2185

bnasslahsen · bnasslahsen · commit c536cef15d31 · 2023-04-01T22:46:55.000+02:00
diff --git a/springdoc-openapi-common/src/main/java/org/springdoc/core/SpringDocConfiguration.java b/springdoc-openapi-common/src/main/java/org/springdoc/core/SpringDocConfiguration.java
@@ -420,6 +420,7 @@ SpringDocProviders springDocProviders(Optional<ActuatorProvider> actuatorProvide
 			Optional<RepositoryRestResourceProvider> repositoryRestResourceProvider, Optional<RouterFunctionProvider> routerFunctionProvider,
 			Optional<SpringWebProvider> springWebProvider, Optional<WebConversionServiceProvider> webConversionServiceProvider,
 			ObjectMapperProvider objectMapperProvider) {
+		objectMapperProvider.jsonMapper().registerModule(new SpringDocRequiredModule());
 		return new SpringDocProviders(actuatorProvider, springCloudFunctionProvider, springSecurityOAuth2Provider, repositoryRestResourceProvider, routerFunctionProvider, springWebProvider, webConversionServiceProvider, objectMapperProvider);
 	}
 
diff --git a/springdoc-openapi-common/src/main/java/org/springdoc/core/SpringDocRequiredModule.java b/springdoc-openapi-common/src/main/java/org/springdoc/core/SpringDocRequiredModule.java
@@ -20,19 +20,20 @@
  *
  */
 
-package org.springdoc.kotlin;
+package org.springdoc.core;
 
 import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
-import com.fasterxml.jackson.databind.introspect.NopAnnotationIntrospector;
 import com.fasterxml.jackson.databind.module.SimpleModule;
+import io.swagger.v3.core.jackson.SwaggerAnnotationIntrospector;
 import io.swagger.v3.oas.annotations.media.Schema;
+import org.apache.commons.lang3.StringUtils;
 
 /**
  * The type Spring doc required module.
  *
  * @author bnasslahsen
  */
-public class SpringDocRequiredModule  extends SimpleModule {
+public class SpringDocRequiredModule extends SimpleModule {
 
 	@Override
 	public void setupModule(SetupContext context) {
@@ -42,11 +43,21 @@ public void setupModule(SetupContext context) {
 	/**
 	 * The type Respect schema required annotation introspector.
 	 */
-	private static class RespectSchemaRequiredAnnotationIntrospector extends NopAnnotationIntrospector {
+	private static class RespectSchemaRequiredAnnotationIntrospector extends SwaggerAnnotationIntrospector {
+
 		@Override
-		public Boolean hasRequiredMarker(AnnotatedMember m) {
-			Schema schemaAnnotation = m.getAnnotation(Schema.class);
-			return schemaAnnotation != null ? schemaAnnotation.required() : null;
+		public Boolean hasRequiredMarker(AnnotatedMember annotatedMember) {
+			Schema schemaAnnotation = annotatedMember.getAnnotation(Schema.class);
+			if (schemaAnnotation != null) {
+				Schema.RequiredMode requiredMode = schemaAnnotation.requiredMode();
+				if (schemaAnnotation.required() || requiredMode == Schema.RequiredMode.REQUIRED) {
+					return true;
+				}
+				else if (requiredMode == Schema.RequiredMode.NOT_REQUIRED || StringUtils.isNotEmpty(schemaAnnotation.defaultValue())) {
+					return false;
+				}
+			}
+			return super.hasRequiredMarker(annotatedMember);
 		}
 	}
 }
diff --git a/springdoc-openapi-common/src/main/java/org/springdoc/core/customizers/DataRestDelegatingMethodParameterCustomizer.java b/springdoc-openapi-common/src/main/java/org/springdoc/core/customizers/DataRestDelegatingMethodParameterCustomizer.java
@@ -24,7 +24,10 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
-import java.util.*;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
diff --git a/springdoc-openapi-common/src/test/java/org/springdoc/core/GenericParameterServiceTest.java b/springdoc-openapi-common/src/test/java/org/springdoc/core/GenericParameterServiceTest.java
@@ -51,7 +51,7 @@
 
 import org.springframework.core.MethodParameter;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
diff --git a/springdoc-openapi-javadoc/src/test/java/org/springdoc/openapi/javadoc/JavadocPropertyCustomizerTest.java b/springdoc-openapi-javadoc/src/test/java/org/springdoc/openapi/javadoc/JavadocPropertyCustomizerTest.java
@@ -50,7 +50,7 @@
 import org.springdoc.core.providers.JavadocProvider;
 import org.springdoc.core.providers.ObjectMapperProvider;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
  * Tests for {@link JavadocPropertyCustomizer}.
diff --git a/springdoc-openapi-kotlin/src/main/java/org/springdoc/kotlin/SpringDocKotlinConfiguration.kt b/springdoc-openapi-kotlin/src/main/java/org/springdoc/kotlin/SpringDocKotlinConfiguration.kt
@@ -45,7 +45,6 @@ class SpringDocKotlinConfiguration(objectMapperProvider: ObjectMapperProvider) {
 			.replaceWithSchema(ByteArray::class.java, ByteArraySchema())
 			.addDeprecatedType(Deprecated::class.java)
 		objectMapperProvider.jsonMapper().registerModule(KotlinModule.Builder().build())
-		objectMapperProvider.jsonMapper().registerModule(SpringDocRequiredModule())
 	}
 
 	/**
diff --git a/springdoc-openapi-kotlin/src/test/java/test/org/springdoc/api/AbstractSpringDocTest.java b/springdoc-openapi-kotlin/src/test/java/test/org/springdoc/api/AbstractSpringDocTest.java
@@ -24,7 +24,6 @@
 import java.nio.file.Paths;
 
 import org.junit.jupiter.api.Test;
-import org.skyscreamer.jsonassert.JSONAssert;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springdoc.core.Constants;
diff --git a/springdoc-openapi-kotlin/src/test/kotlin/test/org/springdoc/api/app9/DemoController.kt b/springdoc-openapi-kotlin/src/test/kotlin/test/org/springdoc/api/app9/DemoController.kt
@@ -18,8 +18,56 @@ data class DemoDto(
     var id: Long,
 )
 
-class DemoRequest {
-	@field:Schema(required = false, description = "Should not be required")
-	val nonNullableWithDefault: String = "a default value"
 
-}
+class DemoRequest (
+
+	@field:Schema(required = true, defaultValue = "a default value")
+	val requiredNullableDefault: String?,
+
+	@field:Schema(required = true)
+	val requiredNullableNoDefault: String?,
+
+	@field:Schema(required = true, defaultValue = "a default value")
+	val requiredNoNullableDefault: String,
+
+	@field:Schema(required = true)
+	val requiredNoNullableNoDefault: String,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.REQUIRED, defaultValue = "a default value")
+	val requiredNullableDefault1: String?,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.REQUIRED)
+	val requiredNullableNoDefault1: String?,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.REQUIRED, defaultValue = "a default value")
+	val requiredNoNullableDefault1: String,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.REQUIRED)
+	val requiredNoNullableNoDefault1: String,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, defaultValue = "a default value")
+	val noRequiredNullableDefault2: String?,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED)
+	val noRequiredNullableNoDefault2: String?,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED, defaultValue = "a default value")
+	val noRequiredNoNullableDefault2: String,
+
+	@field:Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED)
+	val noRequiredNoNullableNoDefault2: String,
+
+	@field:Schema(defaultValue = "a default value")
+	val noRequiredNullableDefault: String?,
+
+	@field:Schema
+	val noRequiredNullableNoDefault: String?,
+
+	@field:Schema(defaultValue = "a default value")
+	val noRequiredNoNullableDefault: String,
+
+	@field:Schema
+	val noRequiredNoNullableNoDefault: String,
+
+
+	)
diff --git a/springdoc-openapi-kotlin/src/test/resources/results/app9.json b/springdoc-openapi-kotlin/src/test/resources/results/app9.json
@@ -45,11 +45,74 @@
   "components": {
     "schemas": {
       "DemoRequest": {
+        "required": [
+          "noRequiredNoNullableNoDefault",
+          "requiredNoNullableDefault",
+          "requiredNoNullableDefault1",
+          "requiredNoNullableNoDefault",
+          "requiredNoNullableNoDefault1",
+          "requiredNullableDefault",
+          "requiredNullableDefault1",
+          "requiredNullableNoDefault",
+          "requiredNullableNoDefault1"
+        ],
         "type": "object",
         "properties": {
-          "nonNullableWithDefault": {
+          "requiredNullableDefault": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "requiredNullableNoDefault": {
+            "type": "string"
+          },
+          "requiredNoNullableDefault": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "requiredNoNullableNoDefault": {
+            "type": "string"
+          },
+          "requiredNullableDefault1": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "requiredNullableNoDefault1": {
+            "type": "string"
+          },
+          "requiredNoNullableDefault1": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "requiredNoNullableNoDefault1": {
+            "type": "string"
+          },
+          "noRequiredNullableDefault2": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "noRequiredNullableNoDefault2": {
+            "type": "string"
+          },
+          "noRequiredNoNullableDefault2": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "noRequiredNoNullableNoDefault2": {
+            "type": "string"
+          },
+          "noRequiredNullableDefault": {
+            "type": "string",
+            "default": "a default value"
+          },
+          "noRequiredNullableNoDefault": {
+            "type": "string"
+          },
+          "noRequiredNoNullableDefault": {
             "type": "string",
-            "description": "Should not be required"
+            "default": "a default value"
+          },
+          "noRequiredNoNullableNoDefault": {
+            "type": "string"
           }
         }
       },
diff --git a/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app145/SpringDocApp1452Test.java b/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app145/SpringDocApp1452Test.java
@@ -36,7 +36,7 @@
 				"springdoc.use-management-port=true",
 				"springdoc.group-configs[0].group=users",
 				"springdoc.group-configs[0].packages-to-scan=test.org.springdoc.api.app145",
-				"management.server.port=9066",
+				"management.server.port=9086",
 				"management.endpoints.web.base-path=/application" })
 public class SpringDocApp1452Test extends AbstractSpringDocActuatorTest {
 
diff --git a/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/HelloController.java b/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/HelloController.java
@@ -23,12 +23,13 @@
 import io.swagger.v3.oas.annotations.media.Content;
 import io.swagger.v3.oas.annotations.responses.ApiResponse;
 import io.swagger.v3.oas.annotations.responses.ApiResponses;
+import reactor.core.publisher.Mono;
+
 import org.springframework.http.MediaType;
 import org.springframework.http.codec.multipart.FilePart;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.bind.annotation.RestController;
-import reactor.core.publisher.Mono;
 
 
 @RestController
diff --git a/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/SpringDocApp189Test.java b/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/SpringDocApp189Test.java
@@ -18,14 +18,15 @@
 
 package test.org.springdoc.api.app189;
 
+import java.time.Duration;
+
 import org.json.JSONException;
 import org.junit.jupiter.api.Test;
 import org.springdoc.core.Constants;
-import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest;
-import org.springframework.test.web.reactive.server.EntityExchangeResult;
 import test.org.springdoc.api.AbstractCommonTest;
 
-import java.time.Duration;
+import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest;
+import org.springframework.test.web.reactive.server.EntityExchangeResult;
 
 import static org.skyscreamer.jsonassert.JSONAssert.assertEquals;
 
diff --git a/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/SpringDocTestApp.java b/springdoc-openapi-webflux-core/src/test/java/test/org/springdoc/api/app189/SpringDocTestApp.java
@@ -24,6 +24,7 @@
 import io.swagger.v3.oas.models.info.License;
 import io.swagger.v3.oas.models.security.SecurityScheme;
 import org.springdoc.core.customizers.OpenApiCustomiser;
+
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
diff --git a/springdoc-openapi-webflux-ui/src/main/java/org/springdoc/webflux/ui/SwaggerResourceResolver.java b/springdoc-openapi-webflux-ui/src/main/java/org/springdoc/webflux/ui/SwaggerResourceResolver.java
@@ -1,14 +1,12 @@
 package org.springdoc.webflux.ui;
 
-import java.io.File;
 import java.util.List;
 
 import org.springdoc.core.SwaggerUiConfigProperties;
 import org.springdoc.ui.AbstractSwaggerResourceResolver;
 import reactor.core.publisher.Mono;
 
 import org.springframework.core.io.Resource;
-import org.springframework.lang.Nullable;
 import org.springframework.web.reactive.resource.ResourceResolver;
 import org.springframework.web.reactive.resource.ResourceResolverChain;
 import org.springframework.web.server.ServerWebExchange;
diff --git a/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/DefaultFlatParamObjectController.java b/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/DefaultFlatParamObjectController.java
@@ -3,11 +3,9 @@
 import org.springdoc.api.annotations.ParameterObject;
 
 import org.springframework.data.domain.Sort;
-import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.web.SortDefault;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 
diff --git a/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/Person.java b/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/Person.java
@@ -1,7 +1,6 @@
 package test.org.springdoc.api.v30.app204;
 
 import javax.validation.constraints.NotBlank;
-import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Size;
 
 public class Person {
diff --git a/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/SpringDocApp204Test.java b/springdoc-openapi-webmvc-core/src/test/java/test/org/springdoc/api/v30/app204/SpringDocApp204Test.java
@@ -22,20 +22,11 @@
 
 package test.org.springdoc.api.v30.app204;
 
-import org.junit.jupiter.api.Test;
-import org.springdoc.core.Constants;
 import test.org.springdoc.api.v30.AbstractSpringDocV30Test;
 
 import org.springframework.boot.autoconfigure.SpringBootApplication;
-import org.springframework.context.annotation.Import;
 import org.springframework.test.context.TestPropertySource;
 
-import static org.hamcrest.Matchers.is;
-import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
-import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
-
 @TestPropertySource(properties = { "springdoc.default-flat-param-object=true" })
 public class SpringDocApp204Test extends AbstractSpringDocV30Test {
 	@SpringBootApplication

Original file line number	Diff line number	Diff line change
`@@ -420,6 +420,7 @@ SpringDocProviders springDocProviders(Optional<ActuatorProvider> actuatorProvide`
`420`	`420`	`Optional<RepositoryRestResourceProvider> repositoryRestResourceProvider, Optional<RouterFunctionProvider> routerFunctionProvider,`
`421`	`421`	`Optional<SpringWebProvider> springWebProvider, Optional<WebConversionServiceProvider> webConversionServiceProvider,`
`422`	`422`	`ObjectMapperProvider objectMapperProvider) {`
	`423`	`+ objectMapperProvider.jsonMapper().registerModule(new SpringDocRequiredModule());`
`423`	`424`	`return new SpringDocProviders(actuatorProvider, springCloudFunctionProvider, springSecurityOAuth2Provider, repositoryRestResourceProvider, routerFunctionProvider, springWebProvider, webConversionServiceProvider, objectMapperProvider);`
`424`	`425`	`}`
`425`	`426`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,6 @@ class SpringDocKotlinConfiguration(objectMapperProvider: ObjectMapperProvider) {`
`45`	`45`	`.replaceWithSchema(ByteArray::class.java, ByteArraySchema())`
`46`	`46`	`.addDeprecatedType(Deprecated::class.java)`
`47`	`47`	`objectMapperProvider.jsonMapper().registerModule(KotlinModule.Builder().build())`
`48`		`- objectMapperProvider.jsonMapper().registerModule(SpringDocRequiredModule())`
`49`	`48`	`}`
`50`	`49`
`51`	`50`	`/**`