[scala] Support for Set when array has uniqueItems=true #4926
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This builds upon community contribution for better Set support in Scala. It makes property + model work as expected with Set and default values across all Scala generators. Included tests to account for new changes. This also reverts the community contribution to remove ListBuffer imports and change the default for array to ListBuffer. Users should use the instantiation types map to modify the desired array instantiation type. Any new default should target a new minor release after community discussion, as it affects all existing SDKs generated with openapi-generator.
|
@ramzimaalej thanks for the review. I added another commit to use default values for monadic |
Scala Play defaulted to List and should continue to do so.
|
I'll go ahead and merge. If there are concerns about it breaking functionality, we can revert and retarget for 4.3.0. But I think the impact is minimal and considered a bug by those who expected Set anyway. |
|
@jimschubert agreed with your change 👍 |
5 tasks
jimschubert
added a commit
to jimschubert/openapi-generator
that referenced
this pull request
Jan 5, 2020
* master: (275 commits) Initial CODEOWNERS (OpenAPITools#4924) [scala] Support for Set when array has uniqueItems=true (OpenAPITools#4926) remove nodejs server samples, scripts (OpenAPITools#4919) Added ability to work with `defaultHeaders` and fixed authentication for code generated by openapi-generator for typescript-node (OpenAPITools#4896) replace petstore_api with packageName (OpenAPITools#4921) remove base_object_spec.mustache from ruby client (OpenAPITools#4918) Add an link to Ada article (OpenAPITools#4920) avoid using hardcode prefix in example (OpenAPITools#4917) [dart-dio] Fix basepath (OpenAPITools#4911) [java][client] jackson update (OpenAPITools#4907) [Swift] Minor improvements to swift 5 generator (OpenAPITools#4910) [cpp-restbed] Added "out-of-the-box" functionality (OpenAPITools#4759) New generator swift5 (OpenAPITools#4086) [dart-dio] Adds support for multipart form data post body (OpenAPITools#4797) [go][client] fix when schema have multiple servers (OpenAPITools#4901) Unables CI tests of python-flask-python2 (OpenAPITools#4889) [C-libcurl] The JSON key name in request/response body should not be escaped even though it is a C key word. (OpenAPITools#4893) upgrade to JUnit 4.13 (OpenAPITools#4899) [r] Ignoring README.md in Rbuildignore (OpenAPITools#4898) update samples ...
jimschubert
added a commit
to jimschubert/openapi-generator
that referenced
this pull request
Jan 11, 2020
* master: (187 commits) [core] Initial FeatureSet structures and definitions (OpenAPITools#3614) Add Cisco to the user list (OpenAPITools#4971) comment out php slim4 in ensure-up-to-date update samples [Python] Allow models to have properties of type self (OpenAPITools#4888) Add npmRepository option to javascript generators (OpenAPITools#4956) [Slim4] Add ref support to Data Mocker (OpenAPITools#4932) Fix auto-labeler for jax-rs (OpenAPITools#4943) [doc] full generator details (OpenAPITools#4941) comment out python flask 2 test (OpenAPITools#4949) [jaxrs-spec][quarkus] update to version 1.1.1.Final (OpenAPITools#4935) [cli] Full config help details (OpenAPITools#4928) Add RequestFile to typescript-node model template (OpenAPITools#4903) [csharp] enum suffix changes enumValueNameSuffix to enumValueSuffix (OpenAPITools#4927) [C#] allow customization of generated enum suffixes (OpenAPITools#4301) [Kotlin] Correct isInherited flag for Kotlin generators (OpenAPITools#4254) [Rust Server] Fix panic handling headers (OpenAPITools#4877) Initial CODEOWNERS (OpenAPITools#4924) [scala] Support for Set when array has uniqueItems=true (OpenAPITools#4926) remove nodejs server samples, scripts (OpenAPITools#4919) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses my comments discussed in #4872
Specifically, I override type/property evaluation and default values so that Set is used consistently across models and properties. I've also added tests.
If CI passes here, I think we can close #4872 and merge into master. I would consider Arrays with unique items generating as List rather than Set to be a bug rather than a breaking change. @wing328 would you agree?
PR checklist
./bin/(or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).master,4.3.x,5.0.x. Default:master.cc @clasnake @shijinkui @ramzimaalej