-
Notifications
You must be signed in to change notification settings - Fork 110
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
[Generator] Add support of deepObject style in query params #538
base: main
Are you sure you want to change the base?
Conversation
### Motivation apple#259 ~Depends on apple/swift-openapi-runtime#100 landing first and getting released, and the version dependency being bumped here.~ ### Modifications Added `deepObject` style to serializer & parser in order to support nested keys on query parameters. ### Result Support nested keys on query parameters. ### Test Plan Adapted snippet tests (SnippetBasedReferenceTests)
Thanks @kstefanou52 for taking the time to tackle this. We'll park this PR review until we land the runtime PR. |
@kstefanou52 Ok please bump the runtime dependency to 1.4.0 in Package.swift and you should be able to finish the generator changes. |
@czechboy0 Done! I have created some tests, but I'm not sure if you would like to add more. Could you please suggest any additional tests you think might be necessary? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One code change suggested, and regarding the extra tests - you're right 🙂
Start by adding a similar parameter like you did in the snippet tests, but this time to petstore.yaml
, which makes it available in generated form in PetstoreConsumerTests
. There, please extend one client and one server unit test to verify that the serialization/deserialization is correctly piped through all the way. The relevant test classes are Test_Client
and Test_Server
.
Sources/_OpenAPIGeneratorCore/Translator/Parameters/TypedParameter.swift
Outdated
Show resolved
Hide resolved
Just checking in on this one. Is it ready for review again? (Thanks for the effort here!) |
Hi @simonjbeaumont! Unfortunately some tests are failing, I'll work on this on weekend and I'll let you know. Cheers! |
Hi @kstefanou52 - do you expect to be able to come back to finish this? 🙂 It's also okay if you can't and someone else will pick up where you left off. |
@czechboy0 I'm apologising for my late response. I'll try to catch up and complete the PR by the end of the week. |
Motivation
The generator changes for:
apple/swift-openapi-generator
Depends on apple/swift-openapi-runtime#100
landing first and getting released, and the version dependency being bumped here.
Modifications
Added
deepObject
style to serializer & parser in order to support nested keys on query parameters.Result
Support nested keys on query parameters.
Test Plan
Adapted snippet tests (SnippetBasedReferenceTests)