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

metadata properties spec #147

Merged
merged 2 commits into from
Jul 20, 2021
Merged

metadata properties spec #147

merged 2 commits into from
Jul 20, 2021

Conversation

jho44
Copy link
Contributor

@jho44 jho44 commented Jul 12, 2021

ticket

Tried to add to the metadata schema the specifications listed in the metadata description and the ticket

How it looks like:
image

  • notice that maxProperties isn't showing up, which is apparently an issue that has been reported but never resolved here
  • also notice that the property key is a string resembling a pattern -- this pattern has no real power since I don't think there's a way to put constraints on Object keys in OpenAPI...

@jho44 jho44 marked this pull request as draft July 12, 2021 23:04
@jho44 jho44 marked this pull request as ready for review July 16, 2021 20:10
Copy link
Contributor

@SidneyAllen SidneyAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jho44 - I'm wondering if this property should be oneOf: type number or boolean.

I'm under the impression it's a JSON object with key-value pairs

{"customer_id": "1234"}

Do you have any information about how the type would be anything other than a string?

@SidneyAllen
Copy link
Contributor

@jho44 - I reviewed the description of https://lobsters.atlassian.net/browse/DXP-105

It's my conclusion that the type for this property should be a string which contains up to 20 key-value pairs. The values of the pairs can be string, boolean or number but the property it self is a JSON object stored as a string.

@jho44
Copy link
Contributor Author

jho44 commented Jul 20, 2021

It's my conclusion that the type for this property should be a string which contains up to 20 key-value pairs. The values of the pairs can be string, boolean or number but the property it self is a JSON object stored as a string.

Sorry I just saw these; my Github notifs are botched. I think the descriptions mean that metadata's keys and values are encoded as strings when the content type is URL encoded. This is how a regular sample request with metadata looks
image
and this is info about URL encoded vs JSON content types
image.

From what I understand, URL encoding, by definition, takes a string and turns it into a "safer" string that can be transmitted across the internet. And if we're following a JSON format for the other content types, then the keys are strings while the values are numbers, bools, or strings. I don't think I can specify that keys must be strings in OAS though.

In short, I think you're right about what you said for URL encoded content type requests.

Copy link
Contributor

@SidneyAllen SidneyAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going forward with oneOf where metadata[campaign] = "NYC2021" or "2021" or "TRUE"

@jho44 jho44 merged commit f09bebf into main Jul 20, 2021
@jho44 jho44 deleted the metadata branch July 20, 2021 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Show minProperties/maxProperties constraints for additionalProperties
2 participants