search

hashicorp / terraform-plugin-codegen-openapi

OpenAPI to Terraform Provider Code Generation Specification
Mozilla Public License 2.0
50 stars 9 forks source link

Add support for sets with custom OAS format field #25

Closed austinvalle closed 1 year ago

austinvalle commented 1 year ago

RFC: https://docs.google.com/document/d/15PJokoO7mkMYsc2Z7DN1copykQgH5Kt7nnzb-mcNcTo/edit#heading=h.j9b2jaknykw

This PR adds support for:

I also added documentation to the design doc and added unit/integration tests to cover since this is a custom field that is not present in existing OAS tests.

I wrote the integration tests first, and you can see the diff (list -> set) after implementation here: https://github.com/hashicorp/terraform-plugin-codegen-openapi/pull/25/commits/bc38b31c2fae9491752f653e9210696cac5cec0e#diff-d0cc596b04078cce090146b7654ac6f32f518ba08c68becbfcb93a07257eefdb

Notes

austinvalle commented 1 year ago

Looks good to me 🚀

I presume that uniqueItems will be treated via validation over set handling? Technically, you can have an ordered list with unique items. Can create the coverage issue if you agree.

I actually do not remember coming across uniqueItems during my research for the RFC, thanks for sharing that.

Yeah as you mentioned JSON schema defines arrays as "ordered", however I do see some tools that have made the decision to treat OpenAPI's uniqueItems as a Set:

So if we want to use uniqueItems instead of a custom type, that doesn't seem to be too far of a stretch 🤔

Also related: https://github.com/json-schema-org/json-schema-spec/issues/1197

austinvalle commented 1 year ago

After chatting in our sync today, the plan is to keep the custom format: set proposed in this PR.

JSON schema does not have a concept of unordered arrays, so we'd be making a technically invalid assumption that uniqueItems == unordered AND unique. We can support validation for ordered lists that have unique items with a custom validator.

austinvalle commented 1 year ago

Opened #26 as follow-up

github-actions[bot] commented 3 months ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.