diff --git a/metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java b/metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java index 70ac351e03..a91a7e199c 100644 --- a/metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java +++ b/metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java @@ -1108,7 +1108,7 @@ public class OpenAPIV3Generator { // Set enums to "string". if (s.getEnum() != null && !s.getEnum().isEmpty()) { if (s.getNullable() != null && s.getNullable()) { - nullableSchema(s, TYPE_STRING_NULLABLE); + s = toNullablePrimitive(s, TYPE_STRING); } else { s.setType(TYPE_STRING); } @@ -1119,53 +1119,58 @@ public class OpenAPIV3Generator { .orElse(new HashSet()); Map properties = - Optional.ofNullable(s.getProperties()).orElse(new HashMap<>()); - properties.forEach( - (name, schema) -> { - schema.specVersion(SpecVersion.V31); - String $ref = schema.get$ref(); + Optional.ofNullable(s.getProperties()).orElseGet(HashMap::new); + for (Map.Entry entry : properties.entrySet()) { + String name = entry.getKey(); + Schema prop = entry.getValue(); + prop.specVersion(SpecVersion.V31); - boolean isNameRequired = requiredNames.contains(name); + String $ref = prop.get$ref(); + boolean isRequired = requiredNames.contains(name); - if (definitions.has(n)) { - JsonNode field = definitions.get(n); - boolean hasDefault = - field.get("properties").get(name).has("default") - && !field.get("properties").get(name).get("default").isNull(); - if (hasDefault) { - // A default value means it is not required, regardless of nullability - s.getRequired().remove(name); - if (s.getRequired().isEmpty()) { - s.setRequired(null); - } - } + // ---- default means "not required" ----------------------------------- + if (definitions.has(n)) { + JsonNode field = definitions.get(n); + boolean hasDefault = + field.get("properties").get(name).has("default") + && !field.get("properties").get(name).get("default").isNull(); + if (hasDefault) { + s.getRequired().remove(name); + if (s.getRequired().isEmpty()) { + s.setRequired(null); } + } + } - if ($ref != null && !isNameRequired) { - // A non-required $ref property must be wrapped in a { oneOf: [ $ref, - // null ] } - // object to allow the - // property to be marked as nullable - schema.setType(null); - schema.set$ref(null); - schema.setOneOf( - List.of(newSchema().$ref($ref), newSchema().type(TYPE_NULL))); - } + // ---- optional $ref → oneOf($ref, null) ----------------------------- + if ($ref != null && !isRequired) { + prop.setType(null); + prop.set$ref(null); + prop.setOneOf(List.of(newSchema().$ref($ref), newSchema().type(TYPE_NULL))); + continue; + } - if ($ref == null) { - if (schema.getEnum() != null && !schema.getEnum().isEmpty()) { - if ((schema.getNullable() != null && schema.getNullable()) - || !isNameRequired) { - nullableSchema(schema, TYPE_STRING_NULLABLE); - } else { - schema.setType(TYPE_STRING); - } - } else if (schema.getEnum() == null && !isNameRequired) { - nullableSchema(schema, Set.of(schema.getType(), TYPE_NULL)); - } - } - }); + // -------------------------------------------------------------------- + // optional ENUM / primitive ‑-> ["string","null"] + // required ENUM / primitive ‑-> "type": "string" + // -------------------------------------------------------------------- + if ($ref == null) { + boolean isEnum = prop.getEnum() != null && !prop.getEnum().isEmpty(); + if (!isRequired) { + // OPTIONAL --> allow explicit null + String scalar = isEnum ? TYPE_STRING : prop.getType(); + 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); } catch (Exception e) { throw new RuntimeException(e); @@ -2022,16 +2027,143 @@ public class OpenAPIV3Generator { return new Schema().specVersion(SPEC_VERSION); } - private static Schema nullableSchema(Schema origin, Set types) { - if (origin == null) { - return newSchema().types(types); + /** + * Replace a StringSchema / IntegerSchema / … with a base Schema that has oneOf: [, null] + * 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 Schema toNullablePrimitive(Schema 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 clone = Json.mapper().convertValue(original, Schema.class); - origin.setType(nonNullType); - origin.setTypes(types); + // Create scalar schema and transfer ALL type-specific properties + Schema 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); + } } } diff --git a/metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java b/metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java index 640b26d8e5..d9d697e2cd 100644 --- a/metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java +++ b/metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java @@ -233,16 +233,18 @@ public class OpenAPIV3GeneratorTest { Schema customProperties = properties.get("customProperties"); assertNull(datasetPropertiesSchema.getRequired()); // not required due to defaults assertNull(customProperties.getNullable()); - assertEquals(customProperties.getType(), "object"); assertEquals( - customProperties.getTypes(), - Set.of("object")); // it is however still not optional, therefore null is not allowed + customProperties.getType(), + "object"); // it is however still not optional, therefore null is not allowed // Assert non-required properties are nullable Schema name = properties.get("name"); assertNull(name.getNullable()); - assertEquals(name.getType(), "string"); - assertEquals(name.getTypes(), Set.of("string", "null")); + assertNull(name.getType()); + 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 Schema created = properties.get("created"); @@ -297,8 +299,8 @@ public class OpenAPIV3GeneratorTest { Schema titleSchema = properties.get("title"); assertNull(titleSchema.getNullable()); - assertEquals(titleSchema.getTypes(), Set.of("string")); // null is not allowed - assertEquals(titleSchema.getType(), "string"); + assertEquals(titleSchema.getType(), "string"); // null is not allowed + assertNull(titleSchema.getTypes()); Schema changeAuditStampsSchema = properties.get("changeAuditStamps"); assertNull(changeAuditStampsSchema.getNullable());