http4k / http4k

The Functional toolkit for Kotlin HTTP applications. http4k provides a simple and uniform way to serve, consume, and test HTTP services.
https://http4k.org
Apache License 2.0
2.62k stars 249 forks source link

OpenApi 3 schema generation is flawed for Maps #1036

Open krissrex opened 10 months ago

krissrex commented 10 months ago

There are several issues with the OpenApi schema generation, and I tried to fix them in my proof-of-concept repo at https://github.com/krissrex/http4k-openapi-annotations/blob/master/src/main/kotlin/no/krex/http4kopenapi/annotations/LifligAutoJsonToJsonSchema.kt . It seems to work, but the code feels slightly messy. (Not ready to PR anything).

The most important issues I found (and fixed) are these:

  1. Correct schema for map-of-maps: mapOf("key" to mapOf("keyInner" to 5)) -> { "additionalProperties": { "additionalProperties": { "type": "number} } } .
  2. Correct schema for map-of-various: mapOf("key" to 1, "key2" to true) -> "additionalProperties": true .

The schema generation is based on the JSON, not the classes. And when a map gets its schema, the example keys are treated like object-properties, not example values, and fed into an Object schema as properties. Instead, we want the values of the example to inform the type of additionalProperties.type. And if the inner values are maps, the schema should not be additionalProperties.properties.additionalProperties, but additionalProperties.additionalProperties.type.


Example map:

// val myMap: Map<String, Map<String, Double>>

  myMap = mapOf(
                        "outerKey1" to mapOf("innerKey1" to 3.14, "innerKey12" to 3.14159),
                        "outerKey2" to mapOf("innerKey2" to 2.71)
                    ),

Ignore any json formatting issues etc, I trimmed the examples to focus on the important parts

Example of current (incorrect) schema:

"myMap": {
            "additionalProperties": {
              "properties": {
                "outerKey1": {
                  "additionalProperties": {
                    "properties": {
                      "innerKey1": {
                        "example": 3.14,
                        "format": "double",
                        "type": "number"
                      },
                      "innerKey12": {
                        "example": 3.14159,
                        "format": "double",
                        "type": "number"
                      }
                    },
                    "example": {
                      "innerKey1": 3.14,
                      "innerKey12": 3.14159
                    },
                    "type": "object"
                  },
                  "type": "object"
                },
                "outerKey2": {
                  "additionalProperties": {
                    "properties": {
                      "innerKey2": {
                        "example": 2.71,
                        "format": "double",
                        "type": "number"
                      }
                    },
                    "example": {
                      "innerKey2": 2.71
                    },
                    "type": "object"
                  },
                  "type": "object"
                }
              },
              "example": {
                "outerKey1": {
                  "innerKey1": 3.14,
                  "innerKey12": 3.14159
                },
                "outerKey2": {
                  "innerKey2": 2.71
                }
              },
              "type": "object"
            },
            "type": "object"
          }

Correct schema:

"myMap": {
  "additionalProperties": {
    "additionalProperties": {
      "example": 3.14,
      "type": "number"
    },
    "type": "object"
  },
  "type": "object"
}

For maps of various types:

// val myMapWithAnything: Map<String, Any>

 myMapWithAnything = mapOf("number" to 4, "bool" to true)

the correct is (not 100% sure about the example-property here):

"myMapWithAnything": {
    "additionalProperties": true,
    "example": {
      "number": 4,
      "bool": true
    },
    "type": "object"
  }

For maps of a primitive type:

// val primitiveMap: Map<String, Int>

mapOf("key" to 1)

the correct is:

"primitiveMap": {
  "additionalProperties": {
    "type": "number"
  }
}
krissrex commented 10 months ago

Seems to be issues with maps of ArrayList too. The keys will get a schema saying object, not array.

tamj0rd2 commented 9 months ago

I really wish it was possible to provide your own schemaGenerator like in the OpenAPI2 implementation. I'm using a library which is able to produce Json schemas which work perfectly for my use case, but am unable to utilise them with http4k

Edit: the library is Kondor. See https://github.com/http4k/http4k/pull/1063

daviddenton commented 9 months ago

I really wish it was possible to provide your own schemaGenerator like in the OpenAPI2 implementation. I'm using a library which is able to produce Json schemas which work perfectly for my use case, but am unable to utilise them with http4k

This is possible - the creation of the Schema is abstracted behind the JsonSchemaCreator interface which is used in both the OpenApi2 and OpenApi3 implementations.

tamj0rd2 commented 9 months ago

I really wish it was possible to provide your own schemaGenerator like in the OpenAPI2 implementation. I'm using a library which is able to produce Json schemas which work perfectly for my use case, but am unable to utilise them with http4k

This is possible - the creation of the Schema is abstracted behind the JsonSchemaCreator interface which is used in both the OpenApi2 and OpenApi3 implementations.

@daviddenton I couldn't see a way to supply this in the OpenApi3 implementation. So I've submitted a PR for that here - https://github.com/http4k/http4k/pull/1058

krissrex commented 9 months ago

I really wish it was possible to provide your own schemaGenerator like in the OpenAPI2 implementation. I'm using a library which is able to produce Json schemas which work perfectly for my use case, but am unable to utilise them with http4k

Mind sharing the name of the library? I might want to use it myself.