FORBIDDEN_ADDITIONAL_PROPERTIES_BY_DEFAULT and sealed classes

[sanyarnd]

Hi "additionalProperties: false" is generated for the following class:

data class SampleConfig(
    val sealed: Sealed
) {
    @Schema(additionalProperties = Schema.AdditionalPropertiesValue.TRUE, anyOf = [Sealed1::class, Sealed2::class])
    sealed interface Sealed

    data class Sealed1(val a: Int) : Sealed
    data class Sealed2(val b: Int) : Sealed
}

which leads to

{
  "$schema" : "https://json-schema.org/draft/2020-12/schema",
  "$ref" : "#/$defs/SampleConfig",
  "$defs" : {
    "SampleConfig" : {
      "type" : "object",
      "properties" : {
        "sealed" : {
          "$ref" : "#/$defs/Sealed",
          "anyOf" : [ {
            "$ref" : "#/$defs/Sealed1"
          }, {
            "$ref" : "#/$defs/Sealed2"
          } ]
        }
      },
      "additionalProperties" : false
    },
    "Sealed" : {
      "type" : "object",
      "additionalProperties" : false
    },
    "Sealed1" : {
      "type" : "object",
      "properties" : {
        "a" : {
          "type" : "integer",
          "format" : "int32"
        }
      },
      "additionalProperties" : false
    },
    "Sealed2" : {
      "type" : "object",
      "properties" : {
        "b" : {
          "type" : "integer",
          "format" : "int32"
        }
      },
      "additionalProperties" : false
    }
  }
}

Using such schema for validation produces:

	Line 1, character 22: false schema always fails
	Keyword: false
	Schema pointer: #/$defs/Sealed/additionalProperties
	Schema location: Line 22, character 32
	Instance pointer: #/sealed/x
	Instance location: Line 1, character 22

I guess we shouldn't generate additionalProperties: false or have a built-in workaround (#352)?

Right now I'm bypassing it with

    val hack: ConfigFunction<TypeScope, Type> = ConfigFunction { scope ->
        when {
            // within a Map<Key, Value> allow additionalProperties of the Value type
            // if no type parameters are defined, this will result in additionalProperties
            // to be omitted (by way of return Object.class)
            scope.type.isInstanceOf(Map::class.java) -> scope.getTypeParameterFor(Map::class.java, 1)
            else -> {
                val ap = scope.type.erasedType.getAnnotation(Schema::class.java)?.additionalProperties
                    ?: return@ConfigFunction null
                if (ap == Schema.AdditionalPropertiesValue.TRUE) {
                    ResolvedObjectType.create(Any::class.java, null, null, null)
                } else {
                    null
                }
            }
        }
    }

which effectively removes "additionalProperties: false", but perhaps there's a better solution

[CarstenWickner]

Hi @sanyarnd,

I've added some basic support for the @Schema(additionalProperties = ...) now. Two thoughts:

• I'm aware that the support of the additionalProperties keyword is not ideal. The challenge is that it only applies to a given subschema, i.e., it doesn't "look into" allOf/anyOf/oneOf or other conditional subschemas. It should therefore be used sparingly.
• The use of @Schema(anyOf = ...) for declaring subtype relations is also not the best way right now. Currently, I'm merely adding those as anyOf on top of the declared type or @Schema(implementation = ...) without replacing the supertype. You might want to consider an alternative way of declaring such subtypes.

[sanyarnd]

Hi, thanks :)

I agree it's kind of hard to support all cases, but anyOf is quite a valid usage, especially if we're talking about Kotlin and Java sealed classes. It'd be nice to not specify it manually and infer it from type info, but of course it's not a deal breaker, I can still do it myself.

Btw, are you planning releasing a version with additionalProperties support? (#352)

[CarstenWickner]

Hi @sanyarnd,

That is exactly what I mean: don't specify the subtypes as anyOf on the Swagger annotation and instead pick those up automatically. I've created an example show-casing exactly that as #356. That utilizes the classgraph library, which Swagger also uses internally. Explicitly indicating those as subtypes, will result in a slightly different schema generation than the Swagger anyOf.

As for the release, I'm not sure when I will find the time and whether it is actually acceptable as-is now.