apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.21k stars 87 forks source link

Empty arrays are omitted from query params #570

Closed jpsim closed 2 weeks ago

jpsim commented 2 weeks ago

Description

For an OpenAPI route with the following get parameter:

{
  "in": "query",
  "name": "colors",
  "required": false,
  "schema": {
    "items": {
      "enum": [
        "BLUE",
        "RED",
        "ORANGE"
      ]
    },
    "type": "array"
  }
}

If one were to pass colors: [] in the generated code, the URISerializer from swift-openapi-runtime would completely omit the query parameter instead of passing say ?colors[]= to specify that the array is empty.

Reproduction

See above

Package version(s)

Expected behavior

[] vs nil should be differentiated somehow.

Environment

Swift 5.10

Additional information

No response

jpsim commented 2 weeks ago

I'm also noticing that this is the last query param, then the URL ends with a dangling &.

/foo?a=false&b=true&
jpsim commented 2 weeks ago

Alternatively, we may want to go with the ?colors= syntax, which means that there's no distinction between [""] and [] which isn't great, but right now there's no distinction between [] and nil which is worse, because at least in the case where the values in the array are defined by an enum, [""] doesn't map to a real value anyway.

This would have the added advantage of being more conformant, and servers that look up the colors key get a value, whereas if they try to access colors[] they may not know that it maps to colors.

czechboy0 commented 2 weeks ago

I'm also noticing that this is the last query param, then the URL ends with a dangling &.

/foo?a=false&b=true&

Can you please file a separate issue for this? Should be easy to fix hopefully.

czechboy0 commented 2 weeks ago

Please double check me on this, but from my reading of the specifications, omitting any keys for an empty array seems like the correct behavior.

OpenAPI 3.1 form exploded query items: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#style-values

Delegates the rules to https://datatracker.ietf.org/doc/html/rfc6570#section-3.2.8

Which adds the key only for items in the array. So if there are no items in the array, there are no keys in the URL.

Putting array= has a different meaning, that'd mean a single element of an empty string, which has a different meaning and could fail e.g. enum validation.

So to your point: "there's no distinction between [] and nil" - yes, that's my reading of RFC6570.

jpsim commented 2 weeks ago

So to your point: "there's no distinction between [] and nil" - yes, that's my reading of RFC6570.

In the last 24 hours I’ve come to realize that this is both correct and also very unfortunate.

Thanks for bringing receipts and actually referencing the specifications too!