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

Empty arrays are omitted from query params #570

Closed
jpsim opened this issue May 3, 2024 · 5 comments
Closed

Empty arrays are omitted from query params #570

jpsim opened this issue May 3, 2024 · 5 comments
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.

Comments

@jpsim
Copy link

jpsim commented May 3, 2024

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)

  • swift-openapi-generator: 1.2.1
  • swift-openapi-runtime: 1.4.0
  • swift-openapi-urlsession: 1.0.1

Expected behavior

[] vs nil should be differentiated somehow.

Environment

Swift 5.10

Additional information

No response

@jpsim jpsim added kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue. labels May 3, 2024
jpsim added a commit to jpsim/swift-openapi-runtime that referenced this issue May 3, 2024
@jpsim
Copy link
Author

jpsim commented May 3, 2024

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

/foo?a=false&b=true&

@jpsim
Copy link
Author

jpsim commented May 3, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

jpsim commented May 3, 2024

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!

@jpsim jpsim closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants