Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid schemas generated for tuples and (some) Maps #31

Open
brprice opened this issue Nov 11, 2021 · 7 comments · May be fixed by #69
Open

Invalid schemas generated for tuples and (some) Maps #31

brprice opened this issue Nov 11, 2021 · 7 comments · May be fixed by #69

Comments

@brprice
Copy link

brprice commented Nov 11, 2021

Description

It seems we generate invalid schemas for all tuples and Maps with "complex" keys (those that aeson's toJSONKey is ToJSONKeyValue and thus the map is encoded as an array of key-value pairs, rather than as a json object).
This is because Map k v is encoded the same as [(k,v)] for these keys, and (k,v) generates a schema similar to

{
  "minItems": 2,
  "items": [
      {
          "type": "string"
      },
      {
          "type": "boolean"
      }
  ],
  "maxItems": 2,
  "type": "array"
}

According to the OpenAPI3 spec, the items field must not be an array.

items - Value MUST be an object and not an array

More details

Note that even homogenous tuples (e.g. (String,String)) have the same problem, since there is no special handling.
In fact, due to this restriction I don't see how to give a schema for any non-record product type, unless all its fields have the same type.

A full example, generating an invalid spec is

{-# LANGUAGE OverloadedLists #-}
{-# LANGUAGE OverloadedStrings #-}

import Control.Lens
import Data.Aeson.Encode.Pretty (encodePretty)
import Data.ByteString.Lazy.Char8 as BSL
import Data.Map (Map)
import Data.Proxy
import Data.OpenApi

main :: IO ()
main = BSL.putStrLn $ encodePretty $ (mempty :: OpenApi) &
  components . schemas .~ [ ("Buggy", toSchema (Proxy :: Proxy (Map [Int] Bool)))]

(See also this gist for the generated schema, and a few other examples)
The output of this is rejected as invalid by https://validator.swagger.io/validator
and https://editor.swagger.io/ and https://openapi-generator.tech/ (both of which give more useful error messages)

The code that constructs these specs

I expect this code was ported from / inspired by swagger2. The swagger (i.e. openapi2) spec is difficult to read, but I think it does support items being an array. However, openapi3 explicitly does not.

(re swagger/openapi2:

)

The future

I wonder if it is worth adding a custom type error to explain why hetrogenous tuples / "complex" maps cannot be given a spec?
Longer term, it seems that openapi-3.1.0 is out which (if I read (the linked JSON Schema docs) correctly) brings back the ability for items to be an array, under the name prefixItems.
However, this does not seem well supported yet, for instance none of the validators above support it.

@maksbotan
Copy link
Collaborator

Hi! Thanks for such a detailed report.

Unfortunately, OAS 3.0 dropped support for tuple-like arrays, leaving only heterogeneous ones:

items - Value MUST be an object and not an array. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema. items MUST be present if the type is array.

I've however decided to leave the relevant code in openapi3 for two reasons: 1. such types are really useful and it would OK to have them for human readers of the spec, if not for automated tools, and 2. I was being lazy :)

Thanks for the link to OAS 3.1, I will look at it. However, as far as I can tell, 3.0 remains the official version for now, so I'm not sure if it makes sense to migrate to 3.1. Will swagger-ui support it, for example?

@brprice
Copy link
Author

brprice commented Dec 13, 2021

Thanks for the link to OAS 3.1, I will look at it. However, as far as I can tell, 3.0 remains the official version for now

I think 3.1 is officially released, according to https://github.com/OAI/OpenAPI-Specification#current-version---310 and https://www.openapis.org/ (which has a big image saying "3.1.0 released").

so I'm not sure if it makes sense to migrate to 3.1. Will swagger-ui support it, for example?

I don't know much on this front, I'm afraid. My impression is that tool support seems to be lacking at the moment, so I suspect it may not be worth the effort yet (I hope this will change in the future!). Swagger-ui in particular does not support 3.1 yet; there is an open issue swagger-api/swagger-ui#5891, though there doesn't seem to be much activity on this. I have seen that 3.1 support is in the "short-term roadmap" for swagger-api/swagger-parser#1535 (comment), but I don't know an ETA.

@JohanWinther
Copy link
Contributor

JohanWinther commented Oct 18, 2022

I'm reviving this issue, because I think I found a solution for the tuple case.

This tuple representation does not validate:

{
  "minItems": 2,
  "items": [
      {
          "type": "string"
      },
      {
          "$ref": "#/components/schemas/Dog"
      }
  ],
  "maxItems": 2,
  "type": "array"
}

however this does:

{
  "minItems": 2,
  "items": {
    "oneOf": [
      { "type": "string" },
      {
        "$ref": "#/components/schemas/Dog"
      }
    ]
  },
  "maxItems": 2,
  "type": "array"
}

I'm not sure if this means that the schemas are applied to each array item in order though.

@Reykudo Reykudo linked a pull request Nov 12, 2022 that will close this issue
@maksbotan
Copy link
Collaborator

{
  "minItems": 2,
  "items": {
    "oneOf": [
      { "type": "string" },
      {
        "$ref": "#/components/schemas/Dog"
      }
    ]
  },
  "maxItems": 2,
  "type": "array"
}

@JohanWinther sadly, this one defines an array of any length, each item of which can be either string or Dog — not a tuple of two elements, first of which is string, second is Dog.

There is really no way to specify tuples in the OpenAPI 3.0 spec.

@dminuoso
Copy link

dminuoso commented Dec 13, 2022

openapi-generator chokes on these as it abides by OAS 3.0.

Errors: 
	-attribute components.schemas.PoolReportAll.items is missing
	-attribute components.schemas.PoolReportAll.items is not of type `object`
Warnings: 
	-attribute components.schemas.PoolReportAll.items is missing
	-attribute components.schemas.PoolReportAll.items is not of type `object`

Would be nice if we could merge #69 and resolve this

avandecreme pushed a commit to avandecreme/openapi3 that referenced this issue Feb 2, 2023
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
avandecreme pushed a commit to avandecreme/openapi3 that referenced this issue Feb 2, 2023
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
avandecreme pushed a commit to avandecreme/openapi3 that referenced this issue Feb 2, 2023
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
teto pushed a commit to novainsilico/openapi3 that referenced this issue Feb 3, 2023
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
@avandecreme
Copy link

If anyone is stuck using openapi-generator because of this, I implemented the above suggested workaround in novainsilico#2

Note that I used anyOf instead of oneOf because for example if you have [1, 2.1] that you want to load into (Int, Double), 1 matches both Int and Double schemas.

I also made sure to add null in the anyOf list whenever we have a Maybe a.

Finally, I also re-implemented #46 with some tests added. While I don't expect any interest to merge the first commit, let me know if you would like me to open a PR for the second one.

@georgefst
Copy link

@brprice FWIW, the problem here is actually slightly more general than the current thread title suggests, as it also applies to any sum type where a constructor has multiple fields, as Aeson also represents these as a two-element array, just as for tuples.

Also, to save others misreading the previous few comments as I did, the "above suggested workaround" in #31 (comment) refers to #31 (comment), which as pointed out in #31 (comment), is far from a universally-applicable workaround. It just uses a much looser type: List (A | B) instead of Pair A B. This might not be too bad for a client which only creates data of the type in question, but it's very awkward when consuming data of that type, as manual unsafe coercions would be required. The best solution I've thought of so far is to implement a tuple-as-record (e.g. data RecordPair a b = RecordPair {fst :: a, snd :: b}), and use this in place of ordinary Haskell tuples. It's potentially a bit cumbersome, but at least the type is precise, and it's reusable.

jbgour pushed a commit to jbgour/openapi3 that referenced this issue Jun 6, 2024
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
jbgour pushed a commit to jbgour/openapi3 that referenced this issue Jun 20, 2024
Workaround for biocad#31

If the tuples has homogeneous types, the generated schema is strict.
On the other hand if there are heterogeneous types, the schema is not very strict because the order in which the types must come is not specified.

Also, I had to use anyOf instead of oneOf because for example the int in (Int, Float) matches both Integer and Number.

Finally, special care had to be taken to handle nullables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants