swift-openapi-generator icon indicating copy to clipboard operation
swift-openapi-generator copied to clipboard

Published 20 hours ago ā€¢ verified publisher icon apple

[Generator] Add support of deepObject style in query params

Open kstefanou52 opened this issue 1 year ago ā€¢ 14 comments

Motivation

The generator changes for: apple/swift-openapi-generator

Depends on https://github.com/apple/swift-openapi-runtime/pull/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)

kstefanou52 avatar Mar 07 '24 13:03 kstefanou52

Thanks @kstefanou52 for taking the time to tackle this. We'll park this PR review until we land the runtime PR.

simonjbeaumont avatar Mar 07 '24 15:03 simonjbeaumont

@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 avatar Apr 16 '24 09:04 czechboy0

@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!

kstefanou52 avatar Apr 16 '24 13:04 kstefanou52

Just checking in on this one. Is it ready for review again? (Thanks for the effort here!)

simonjbeaumont avatar May 21 '24 20:05 simonjbeaumont

Hi @simonjbeaumont! Unfortunately some tests are failing, I'll work on this on weekend and I'll let you know. Cheers!

kstefanou52 avatar May 25 '24 11:05 kstefanou52

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 avatar Jul 22 '24 14:07 czechboy0

@czechboy0 I'm apologising for my late response. I'll try to catch up and complete the PR by the end of the week.

kstefanou52 avatar Sep 04 '24 09:09 kstefanou52

Hi @kstefanou52 - let us know if you need any help with this? I think it just needs to be updated from the main branch and a conflict resolved, but otherwise might be good to go?

czechboy0 avatar Oct 03 '24 07:10 czechboy0

Hello @czechboy0 and @arthurcro, I've made some progress, but I'm facing an issue with one of the unit tests. The code appears to work as expected, but I suspect there may be a problem with the runtime library. Iā€™d appreciate your thoughts on this; more details can be found here.

kstefanou52 avatar Oct 26 '24 13:10 kstefanou52

~~Can you elaborate? The link doesn't really say what the issue is - what are you seeing and what are you expecting?~~

Never mind, found your other comment.

czechboy0 avatar Oct 26 '24 15:10 czechboy0

Hey @czechboy0, I just opened the runtime PR. Please let me know if there are any additional tests we should add. Thanks!

kstefanou52 avatar Oct 29 '24 09:10 kstefanou52