Merge pull request #2171 from swagger-api/issue-2125

issue-2125: fix deserialization messages for schemas default values when nullable is set

Author: djankows

modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/OpenAPIV3Parser.java

@@ -176,7 +176,7 @@ public SwaggerParseResult readContents(String swaggerAsString, List<Authorizatio
             SwaggerParseResult result;
             if (options != null) {
                 result = parseJsonNode(location, rootNode, options);
-            }else {
+            } else {
                 result = parseJsonNode(location, rootNode);
             }
             if (result.getOpenAPI() != null) {

modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java

@@ -1879,7 +1879,6 @@ public BigDecimal getBigDecimal(String key, ObjectNode node, boolean required, S
 		return value;
 	}
 
-
 	public Integer getInteger(String key, ObjectNode node, boolean required, String location, ParseResult result) {
 		Integer value = null;
 		JsonNode v = node.get(key);
@@ -2747,7 +2746,7 @@ at the moment path passed as string (basePath) from upper components can be both
 				}
 			}
 			schema = items;
-		}else if (itemsNode != null){
+		} else if (itemsNode != null) {
 			Schema items = new Schema();
 			if (itemsNode.getNodeType().equals(JsonNodeType.OBJECT)) {
 				items.setItems(getSchema(itemsNode, location, result));
@@ -2905,7 +2904,7 @@ at the moment path passed as string (basePath) from upper components can be both
 
 		Map<String, Schema> properties = new LinkedHashMap<>();
 		ObjectNode propertiesObj = getObject("properties", node, false, location, result);
-		Schema property = null;
+		Schema property;
 
 		Set<String> keys = getKeys(propertiesObj);
 		for (String name : keys) {
@@ -2925,10 +2924,24 @@ at the moment path passed as string (basePath) from upper components can be both
 			schema.setProperties(properties);
 		}
 
+        bool = getBoolean("nullable", node, false, location, result);
+        if (bool != null) {
+            schema.setNullable(bool);
+        }
+
 		//sets default value according to the schema type
 		if (node.get("default") != null && result.isInferSchemaType()) {
+            boolean nullable = schema.getNullable() == null || schema.getNullable();
+            boolean isDefaultNodeTypeNull = node.get("default") != null && node.get("default").isNull();
             if (!StringUtils.isBlank(schema.getType())) {
-                if (schema.getType().equals("array")) {
+                if (isDefaultNodeTypeNull) {
+                    if (nullable) {
+                        schema.setDefault(null);
+                    } else {
+                        String expectedType = String.format("non-null %s", schema.getType());
+                        result.invalidType(location, "default", expectedType, node);
+                    }
+                } else if (schema.getType().equals("array")) {
                     ArrayNode array = getArray("default", node, false, location, result);
                     if (array != null) {
                         schema.setDefault(array);
@@ -2975,15 +2988,10 @@ at the moment path passed as string (basePath) from upper components can be both
             if (defaultObject != null) {
                 schema.setDefault(defaultObject);
             }
-		}else{
+		} else {
 			schema.setDefault(null);
 		}
 
-		bool = getBoolean("nullable", node, false, location, result);
-		if (bool != null) {
-			schema.setNullable(bool);
-		}
-
 		Map<String, Object> extensions = getExtensions(node);
 		if (extensions != null && extensions.size() > 0) {
 			schema.setExtensions(extensions);

modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OAIDeserializationTest.java

@@ -11,7 +11,6 @@
 import io.swagger.v3.oas.models.media.ComposedSchema;
 import io.swagger.v3.oas.models.media.Content;
 import io.swagger.v3.oas.models.media.MediaType;
-import io.swagger.v3.oas.models.media.ObjectSchema;
 import io.swagger.v3.oas.models.media.StringSchema;
 import io.swagger.v3.oas.models.parameters.RequestBody;
 import io.swagger.v3.oas.models.responses.ApiResponse;
@@ -137,4 +136,33 @@ public void testDeserializeYamlDefinitionMissingSchema_Issue1951() throws Except
         assertTrue(result.getMessages().contains("attribute components.responses.ErrorObj.content.'application/json'.schema.NotAddedYet is missing"));
         assertTrue(result.getMessages().contains("attribute paths.'/thingy'(post).requestBody.content.'application/json'.schema.#/components/schemas/ThingRequest is missing"));
     }
+
+    @Test
+    public void testDeserializeSchemaWithDefaultProperty() {
+        SwaggerParseResult result = new OpenAPIV3Parser().readLocation("/schemas-default-value/default.yaml", null, null);
+        assertEquals(result.getMessages().size(),0);
+        assertNotNull(result.getOpenAPI());
+    }
+
+    @Test
+    public void testDeserializeSchemaWithNullDefaultProperty() {
+        SwaggerParseResult result = new OpenAPIV3Parser().readLocation("/schemas-default-value/defaultNull.yaml", null, null);
+        assertEquals(result.getMessages().size(),0);
+        assertNotNull(result.getOpenAPI());
+    }
+
+    @Test
+    public void testDeserializeSchemaWithNullDefaultAndNullableTrueProperty() {
+        SwaggerParseResult result = new OpenAPIV3Parser().readLocation("/schemas-default-value/defaultNullAndNullableTrue.yaml", null, null);
+        assertEquals(result.getMessages().size(),0);
+        assertNotNull(result.getOpenAPI());
+    }
+
+    @Test
+    public void testDeserializeSchemaWithNullDefaultAndNullableFalseProperty() {
+        SwaggerParseResult result = new OpenAPIV3Parser().readLocation("/schemas-default-value/defaultNullAndNullableFalse.yaml", null, null);
+        assertEquals(result.getMessages().size(),1);
+        assertEquals(result.getMessages().get(0), "attribute components.schemas.Test.default is not of type `non-null string`");
+        assertNotNull(result.getOpenAPI());
+    }
 }

modules/swagger-parser-v3/src/test/resources/issue_2125.yaml

@@ -0,0 +1,16 @@
+openapi: '3.0.0'
+
+info:
+  title: Test
+  version: 0.1.0
+
+paths: {}
+
+components:
+  schemas:
+    Test:
+      type: string
+      items: 
+        type: string
+      default: null
+      nullable: false

modules/swagger-parser-v3/src/test/resources/schemas-default-value/default.yaml

@@ -0,0 +1,13 @@
+openapi: '3.0.0'
+
+info:
+  title: Test
+  version: 0.1.0
+
+paths: {}
+
+components:
+  schemas:
+    Test:
+      type: string
+      default: string

modules/swagger-parser-v3/src/test/resources/schemas-default-value/defaultNull.yaml

@@ -0,0 +1,13 @@
+openapi: '3.0.0'
+
+info:
+  title: Test
+  version: 0.1.0
+
+paths: {}
+
+components:
+  schemas:
+    Test:
+      type: string
+      default: null

modules/swagger-parser-v3/src/test/resources/schemas-default-value/defaultNullAndNullableFalse.yaml

@@ -0,0 +1,14 @@
+openapi: '3.0.0'
+
+info:
+  title: Test
+  version: 0.1.0
+
+paths: {}
+
+components:
+  schemas:
+    Test:
+      type: string
+      default: null
+      nullable: false

modules/swagger-parser-v3/src/test/resources/schemas-default-value/defaultNullAndNullableTrue.yaml

@@ -0,0 +1,14 @@
+openapi: '3.0.0'
+
+info:
+  title: Test
+  version: 0.1.0
+
+paths: {}
+
+components:
+  schemas:
+    Test:
+      type: string
+      default: null
+      nullable: true