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

Improve create message route #1

Open
A5rocks opened this issue Aug 2, 2023 · 6 comments
Open

Improve create message route #1

A5rocks opened this issue Aug 2, 2023 · 6 comments

Comments

@A5rocks
Copy link

A5rocks commented Aug 2, 2023

Currently, the create message route is... kinda sucky:

"properties": {
"files[0]": {
"type": "string",
"contentEncoding": "binary"
},
"files[1]": {
"type": "string",
"contentEncoding": "binary"
},
"files[2]": {
"type": "string",
"contentEncoding": "binary"
},
"files[3]": {
"type": "string",
"contentEncoding": "binary"
},
"files[4]": {
"type": "string",
"contentEncoding": "binary"
},
"files[5]": {
"type": "string",
"contentEncoding": "binary"
},
"files[6]": {
"type": "string",
"contentEncoding": "binary"
},
"files[7]": {
"type": "string",
"contentEncoding": "binary"
},
"files[8]": {
"type": "string",
"contentEncoding": "binary"
},
"files[9]": {
"type": "string",
"contentEncoding": "binary"
}
}

This could be better expressed as:

"patternProperties": {
  "^files\\[[0-9]+\\]$": {
    "type": "string",
    "contentEncoding": "binary"
  }
}

using patternProperties.

@IanMitchell
Copy link

🤔 pretty sure the ID can be any snowflake as well

@A5rocks
Copy link
Author

A5rocks commented Aug 2, 2023

Yeah this uses a regex to say the number is any sequence of digits. I would use \d but that's not in the restricted subset of regex JSON schema recommends :(

@infinitestory
Copy link

This is a technical limitation needed to express that we don't accept unlimited uploads in a single request. (In general, for requests, I'd prefer that the spec expresses a subset rather than a superset of allowed inputs, if it can't be exactly precise).

@A5rocks
Copy link
Author

A5rocks commented Aug 2, 2023

How about using maxProperties then? IIRC you can only send the payload_json key for the form body (... maybe I'm misremembering?) so just set the max number of properties to the max amount of files + 1? (... or maybe without the +1?).

@infinitestory
Copy link

i think we currently accept body properties as form properties (but if payload_json is present, they all have to be in there instead)? i could be wrong as well

@A5rocks
Copy link
Author

A5rocks commented Aug 3, 2023

I checked, you're correct. Maybe there should be an oneOf where one branch allows only these fixed names and no payload_json, and the other branch allows any file numbers and only an extra payload_json field?

(I imagine "the other branch" is the significantly more common case; that's how I've implemented things before :P)

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

No branches or pull requests

3 participants