mirror of
				https://github.com/datahub-project/datahub.git
				synced 2025-10-31 10:49:00 +00:00 
			
		
		
		
	fix(openapi): openapi 3.1.0 - use oneOf to mark nullable primitive props as well (#14103)
This commit is contained in:
		
							parent
							
								
									5471be6d4d
								
							
						
					
					
						commit
						e7f45b1b49
					
				| @ -1108,7 +1108,7 @@ public class OpenAPIV3Generator { | |||||||
|                   // Set enums to "string". |                   // Set enums to "string". | ||||||
|                   if (s.getEnum() != null && !s.getEnum().isEmpty()) { |                   if (s.getEnum() != null && !s.getEnum().isEmpty()) { | ||||||
|                     if (s.getNullable() != null && s.getNullable()) { |                     if (s.getNullable() != null && s.getNullable()) { | ||||||
|                       nullableSchema(s, TYPE_STRING_NULLABLE); |                       s = toNullablePrimitive(s, TYPE_STRING); | ||||||
|                     } else { |                     } else { | ||||||
|                       s.setType(TYPE_STRING); |                       s.setType(TYPE_STRING); | ||||||
|                     } |                     } | ||||||
| @ -1119,53 +1119,58 @@ public class OpenAPIV3Generator { | |||||||
|                             .orElse(new HashSet()); |                             .orElse(new HashSet()); | ||||||
| 
 | 
 | ||||||
|                     Map<String, Schema> properties = |                     Map<String, Schema> properties = | ||||||
|                         Optional.ofNullable(s.getProperties()).orElse(new HashMap<>()); |                         Optional.ofNullable(s.getProperties()).orElseGet(HashMap::new); | ||||||
|                     properties.forEach( |                     for (Map.Entry<String, Schema> entry : properties.entrySet()) { | ||||||
|                         (name, schema) -> { |                       String name = entry.getKey(); | ||||||
|                           schema.specVersion(SpecVersion.V31); |                       Schema<?> prop = entry.getValue(); | ||||||
|                           String $ref = schema.get$ref(); |                       prop.specVersion(SpecVersion.V31); | ||||||
| 
 | 
 | ||||||
|                           boolean isNameRequired = requiredNames.contains(name); |                       String $ref = prop.get$ref(); | ||||||
|  |                       boolean isRequired = requiredNames.contains(name); | ||||||
| 
 | 
 | ||||||
|                           if (definitions.has(n)) { |                       // ---- default means "not required" ----------------------------------- | ||||||
|                             JsonNode field = definitions.get(n); |                       if (definitions.has(n)) { | ||||||
|                             boolean hasDefault = |                         JsonNode field = definitions.get(n); | ||||||
|                                 field.get("properties").get(name).has("default") |                         boolean hasDefault = | ||||||
|                                     && !field.get("properties").get(name).get("default").isNull(); |                             field.get("properties").get(name).has("default") | ||||||
|                             if (hasDefault) { |                                 && !field.get("properties").get(name).get("default").isNull(); | ||||||
|                               // A default value means it is not required, regardless of nullability |                         if (hasDefault) { | ||||||
|                               s.getRequired().remove(name); |                           s.getRequired().remove(name); | ||||||
|                               if (s.getRequired().isEmpty()) { |                           if (s.getRequired().isEmpty()) { | ||||||
|                                 s.setRequired(null); |                             s.setRequired(null); | ||||||
|                               } |  | ||||||
|                             } |  | ||||||
|                           } |                           } | ||||||
|  |                         } | ||||||
|  |                       } | ||||||
| 
 | 
 | ||||||
|                           if ($ref != null && !isNameRequired) { |                       // ---- optional $ref  → oneOf($ref, null) ----------------------------- | ||||||
|                             // A non-required $ref property must be wrapped in a { oneOf: [ $ref, |                       if ($ref != null && !isRequired) { | ||||||
|                             // null ] } |                         prop.setType(null); | ||||||
|                             // object to allow the |                         prop.set$ref(null); | ||||||
|                             // property to be marked as nullable |                         prop.setOneOf(List.of(newSchema().$ref($ref), newSchema().type(TYPE_NULL))); | ||||||
|                             schema.setType(null); |                         continue; | ||||||
|                             schema.set$ref(null); |                       } | ||||||
|                             schema.setOneOf( |  | ||||||
|                                 List.of(newSchema().$ref($ref), newSchema().type(TYPE_NULL))); |  | ||||||
|                           } |  | ||||||
| 
 | 
 | ||||||
|                           if ($ref == null) { |                       // -------------------------------------------------------------------- | ||||||
|                             if (schema.getEnum() != null && !schema.getEnum().isEmpty()) { |                       // optional ENUM / primitive ‑->  ["string","null"] | ||||||
|                               if ((schema.getNullable() != null && schema.getNullable()) |                       // required ENUM / primitive  ‑->  "type": "string" | ||||||
|                                   || !isNameRequired) { |                       // -------------------------------------------------------------------- | ||||||
|                                 nullableSchema(schema, TYPE_STRING_NULLABLE); |                       if ($ref == null) { | ||||||
|                               } else { |                         boolean isEnum = prop.getEnum() != null && !prop.getEnum().isEmpty(); | ||||||
|                                 schema.setType(TYPE_STRING); |                         if (!isRequired) { | ||||||
|                               } |                           // OPTIONAL  --> allow explicit null | ||||||
|                             } else if (schema.getEnum() == null && !isNameRequired) { |                           String scalar = isEnum ? TYPE_STRING : prop.getType(); | ||||||
|                               nullableSchema(schema, Set.of(schema.getType(), TYPE_NULL)); |                           Schema<?> nullable = toNullablePrimitive(prop, scalar); | ||||||
|                             } |                           entry.setValue(nullable); | ||||||
|                           } |                         } else { | ||||||
|                         }); |                           // REQUIRED  --> keep scalar type, ensure it's present | ||||||
|  |                           prop.setType(isEnum ? TYPE_STRING : prop.getType()); | ||||||
|  |                           prop.setTypes(null); | ||||||
|  |                         } | ||||||
|  |                       } | ||||||
|  |                     } | ||||||
|                   } |                   } | ||||||
|  |                   // Clear out the previous state | ||||||
|  |                   s.setJsonSchema(null); | ||||||
|                   components.addSchemas(n, s); |                   components.addSchemas(n, s); | ||||||
|                 } catch (Exception e) { |                 } catch (Exception e) { | ||||||
|                   throw new RuntimeException(e); |                   throw new RuntimeException(e); | ||||||
| @ -2022,16 +2027,143 @@ public class OpenAPIV3Generator { | |||||||
|     return new Schema().specVersion(SPEC_VERSION); |     return new Schema().specVersion(SPEC_VERSION); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   private static Schema nullableSchema(Schema origin, Set<String> types) { |   /** | ||||||
|     if (origin == null) { |    * Replace a StringSchema / IntegerSchema / … with a base Schema that has oneOf: [<scalar>, null] | ||||||
|       return newSchema().types(types); |    * while preserving all other attributes (format, description, examples, …). This is because | ||||||
|  |    * StringSchema / IntegerSchema / etc will hard-code the type attributes which breaks OAS 3.1.0 | ||||||
|  |    * nullability for primitives. | ||||||
|  |    * | ||||||
|  |    * @param original the original typed schema (StringSchema, IntegerSchema, etc.) | ||||||
|  |    * @param scalarType the scalar type string (e.g., "string", "integer", "number", "boolean", | ||||||
|  |    *     "object") | ||||||
|  |    * @return a new Schema with proper OAS 3.1.0 nullable oneOf structure | ||||||
|  |    * @throws IllegalArgumentException if original schema or scalarType is null/empty | ||||||
|  |    */ | ||||||
|  |   private static <T> Schema<T> toNullablePrimitive(Schema<T> original, String scalarType) { | ||||||
|  |     if (original == null) { | ||||||
|  |       throw new IllegalArgumentException("Original schema cannot be null"); | ||||||
|  |     } | ||||||
|  |     if (scalarType == null || scalarType.trim().isEmpty()) { | ||||||
|  |       throw new IllegalArgumentException("Scalar type cannot be null or empty"); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     String nonNullType = types.stream().filter(t -> !"null".equals(t)).findFirst().orElse(null); |     try { | ||||||
|  |       // Create a deep clone without modifying the original | ||||||
|  |       Schema<T> clone = Json.mapper().convertValue(original, Schema.class); | ||||||
| 
 | 
 | ||||||
|     origin.setType(nonNullType); |       // Create scalar schema and transfer ALL type-specific properties | ||||||
|     origin.setTypes(types); |       Schema<T> scalarSchema = new Schema<>(); | ||||||
|  |       scalarSchema.setType(scalarType.trim()); | ||||||
|  |       scalarSchema.setSpecVersion(SPEC_VERSION); | ||||||
| 
 | 
 | ||||||
|     return origin; |       // Transfer type-specific properties to scalar schema | ||||||
|  |       // String-specific properties | ||||||
|  |       if (clone.getFormat() != null) { | ||||||
|  |         scalarSchema.setFormat(clone.getFormat()); | ||||||
|  |         clone.setFormat(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMinLength() != null) { | ||||||
|  |         scalarSchema.setMinLength(clone.getMinLength()); | ||||||
|  |         clone.setMinLength(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMaxLength() != null) { | ||||||
|  |         scalarSchema.setMaxLength(clone.getMaxLength()); | ||||||
|  |         clone.setMaxLength(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getPattern() != null) { | ||||||
|  |         scalarSchema.setPattern(clone.getPattern()); | ||||||
|  |         clone.setPattern(null); | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       // Number-specific properties | ||||||
|  |       if (clone.getMinimum() != null) { | ||||||
|  |         scalarSchema.setMinimum(clone.getMinimum()); | ||||||
|  |         clone.setMinimum(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMaximum() != null) { | ||||||
|  |         scalarSchema.setMaximum(clone.getMaximum()); | ||||||
|  |         clone.setMaximum(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getExclusiveMinimum() != null) { | ||||||
|  |         scalarSchema.setExclusiveMinimum(clone.getExclusiveMinimum()); | ||||||
|  |         clone.setExclusiveMinimum(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getExclusiveMaximum() != null) { | ||||||
|  |         scalarSchema.setExclusiveMaximum(clone.getExclusiveMaximum()); | ||||||
|  |         clone.setExclusiveMaximum(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMultipleOf() != null) { | ||||||
|  |         scalarSchema.setMultipleOf(clone.getMultipleOf()); | ||||||
|  |         clone.setMultipleOf(null); | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       // Object-specific properties | ||||||
|  |       if (clone.getProperties() != null) { | ||||||
|  |         scalarSchema.setProperties(clone.getProperties()); | ||||||
|  |         clone.setProperties(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getAdditionalProperties() != null) { | ||||||
|  |         scalarSchema.setAdditionalProperties(clone.getAdditionalProperties()); | ||||||
|  |         clone.setAdditionalProperties(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getRequired() != null) { | ||||||
|  |         scalarSchema.setRequired(clone.getRequired()); | ||||||
|  |         clone.setRequired(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMinProperties() != null) { | ||||||
|  |         scalarSchema.setMinProperties(clone.getMinProperties()); | ||||||
|  |         clone.setMinProperties(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMaxProperties() != null) { | ||||||
|  |         scalarSchema.setMaxProperties(clone.getMaxProperties()); | ||||||
|  |         clone.setMaxProperties(null); | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       // Array-specific properties | ||||||
|  |       if (clone.getItems() != null) { | ||||||
|  |         scalarSchema.setItems(clone.getItems()); | ||||||
|  |         clone.setItems(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMinItems() != null) { | ||||||
|  |         scalarSchema.setMinItems(clone.getMinItems()); | ||||||
|  |         clone.setMinItems(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getMaxItems() != null) { | ||||||
|  |         scalarSchema.setMaxItems(clone.getMaxItems()); | ||||||
|  |         clone.setMaxItems(null); | ||||||
|  |       } | ||||||
|  |       if (clone.getUniqueItems() != null) { | ||||||
|  |         scalarSchema.setUniqueItems(clone.getUniqueItems()); | ||||||
|  |         clone.setUniqueItems(null); | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       // Enum (applies to any type) | ||||||
|  |       if (clone.getEnum() != null) { | ||||||
|  |         scalarSchema.setEnum(clone.getEnum()); | ||||||
|  |         clone.setEnum(null); | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       // Clear conflicting type information from parent | ||||||
|  |       clone.setType(null); | ||||||
|  |       clone.setNullable(null); | ||||||
|  |       clone.setTypes(null); | ||||||
|  | 
 | ||||||
|  |       // Create null schema | ||||||
|  |       Schema<?> nullSchema = new Schema<>(); | ||||||
|  |       nullSchema.setType(TYPE_NULL); | ||||||
|  |       nullSchema.setSpecVersion(SPEC_VERSION); | ||||||
|  | 
 | ||||||
|  |       // Set oneOf on parent schema | ||||||
|  |       clone.setOneOf(List.of(scalarSchema, nullSchema)); | ||||||
|  |       clone.setSpecVersion(SPEC_VERSION); | ||||||
|  | 
 | ||||||
|  |       return clone; | ||||||
|  | 
 | ||||||
|  |     } catch (Exception e) { | ||||||
|  |       throw new RuntimeException( | ||||||
|  |           String.format( | ||||||
|  |               "Failed to convert schema to nullable primitive with type '%s'", scalarType), | ||||||
|  |           e); | ||||||
|  |     } | ||||||
|   } |   } | ||||||
| } | } | ||||||
|  | |||||||
| @ -233,16 +233,18 @@ public class OpenAPIV3GeneratorTest { | |||||||
|     Schema customProperties = properties.get("customProperties"); |     Schema customProperties = properties.get("customProperties"); | ||||||
|     assertNull(datasetPropertiesSchema.getRequired()); // not required due to defaults |     assertNull(datasetPropertiesSchema.getRequired()); // not required due to defaults | ||||||
|     assertNull(customProperties.getNullable()); |     assertNull(customProperties.getNullable()); | ||||||
|     assertEquals(customProperties.getType(), "object"); |  | ||||||
|     assertEquals( |     assertEquals( | ||||||
|         customProperties.getTypes(), |         customProperties.getType(), | ||||||
|         Set.of("object")); // it is however still not optional, therefore null is not allowed |         "object"); // it is however still not optional, therefore null is not allowed | ||||||
| 
 | 
 | ||||||
|     // Assert non-required properties are nullable |     // Assert non-required properties are nullable | ||||||
|     Schema name = properties.get("name"); |     Schema name = properties.get("name"); | ||||||
|     assertNull(name.getNullable()); |     assertNull(name.getNullable()); | ||||||
|     assertEquals(name.getType(), "string"); |     assertNull(name.getType()); | ||||||
|     assertEquals(name.getTypes(), Set.of("string", "null")); |     assertNull(name.getTypes()); | ||||||
|  |     assertEquals(name.getOneOf().size(), 2); | ||||||
|  |     assertTrue(name.getOneOf().stream().anyMatch(s -> "string".equals(((Schema<?>) s).getType()))); | ||||||
|  |     assertTrue(name.getOneOf().stream().anyMatch(s -> "null".equals(((Schema<?>) s).getType()))); | ||||||
| 
 | 
 | ||||||
|     // Assert non-required $ref properties are replaced by nullable { anyOf: [ $ref ] } objects |     // Assert non-required $ref properties are replaced by nullable { anyOf: [ $ref ] } objects | ||||||
|     Schema created = properties.get("created"); |     Schema created = properties.get("created"); | ||||||
| @ -297,8 +299,8 @@ public class OpenAPIV3GeneratorTest { | |||||||
| 
 | 
 | ||||||
|     Schema titleSchema = properties.get("title"); |     Schema titleSchema = properties.get("title"); | ||||||
|     assertNull(titleSchema.getNullable()); |     assertNull(titleSchema.getNullable()); | ||||||
|     assertEquals(titleSchema.getTypes(), Set.of("string")); // null is not allowed |     assertEquals(titleSchema.getType(), "string"); // null is not allowed | ||||||
|     assertEquals(titleSchema.getType(), "string"); |     assertNull(titleSchema.getTypes()); | ||||||
| 
 | 
 | ||||||
|     Schema changeAuditStampsSchema = properties.get("changeAuditStamps"); |     Schema changeAuditStampsSchema = properties.get("changeAuditStamps"); | ||||||
|     assertNull(changeAuditStampsSchema.getNullable()); |     assertNull(changeAuditStampsSchema.getNullable()); | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 Kevin Chun
						Kevin Chun