SMILEY4 / ktor-swagger-ui

Kotlin Ktor plugin to generate OpenAPI and provide Swagger UI
Apache License 2.0
182 stars 33 forks source link

Properties defined as non-nullable with a default value are still marked as required in the swagger schema #121

Closed vincentburesi-es closed 3 months ago

vincentburesi-es commented 3 months ago

As said in the title, I have looked into the code up to io.github.smiley4.schemakenerator.reflection.steps.ReflectionTypeProcessingStep#parseProperty

    private fun parseProperty(
        member: KProperty<*>,
        resolvedTypeParameters: Map<String, TypeParameterData>,
        typeData: MutableList<BaseTypeData>
    ): PropertyData {
        return PropertyData(
            name = member.name,
            type = resolveMemberType(member.returnType, resolvedTypeParameters, typeData).id,
            nullable = member.returnType.isMarkedNullable, // <-- This definition here
            annotations = parseAnnotations(member).toMutableList(),
            kind = PropertyType.PROPERTY,
            visibility = determinePropertyVisibility(member)
        )
    }

and its usage in io.github.smiley4.schemakenerator.swagger.steps.SwaggerSchemaGenerationStep#buildObjectSchema

    private fun buildObjectSchema(typeData: ObjectTypeData, typeDataList: Collection<BaseTypeData>): SwaggerSchema {

        val requiredProperties = mutableSetOf<String>()
        val propertySchemas = mutableMapOf<String, Schema<*>>()

        collectMembers(typeData, typeDataList).forEach { member ->
            propertySchemas[member.name] = schema.referenceSchema(member.type)
            if (!member.nullable) { // <-- Used here to define the property as required
                requiredProperties.add(member.name)
            }
        }

        return SwaggerSchema(
            swagger = schema.objectSchema(propertySchemas, requiredProperties),
            typeData = typeData
        )
    }

So there seem to be no distinction between the notions of "nullable" and "required" in the PropertyData class. It would be great to have a distinct "required" boolean field that takes into account non-nullable fields with default values such as :

data class MyClass(
    val myField: Int = 42
)
vincentburesi-es commented 3 months ago

The only way to identify default values I know of is to find which constructor parameters are marked as isOptional which could be complex to implement in a way that handles every edge case. Another way I could imagine this being solved is through an annotation that could be easily checked in the commented condition in buildObjectSchema()

SMILEY4 commented 3 months ago

Hi, thanks for the suggestion,

i added an automatic detection of optional properties (for reflection only for properties declared via constructor). By default, they are still handled as required during schema-generation, though this can be changed via the config at the generation step:

val result = typeOf<MyExampleClass>()
    .processReflection()
    .generateJsonSchema {
        optionalHandling = OptionalHandling.NON_REQUIRED // same for swagger-schema
    }
    .compileInlining()

Since this works only for constructor parameters, i added two annotations "Optional" and "Required" that are handled in the "handleCoreAnnotations"-step (and overwrite any other behavior, e.g. from nullability or optional from constructor).

The changes will be included in the next version.

I hope these changes make sence. If you have any other idea, please let me know :)