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.

Sign up for GitHub

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

Jump to bottom

Support deepObject style for query params #259

Open
czechboy0 opened this issue Sep 9, 2023 · 10 comments
Open

Support deepObject style for query params #259

czechboy0 opened this issue Sep 9, 2023 · 10 comments
Labels
area/openapi Adding/updating a feature defined in OpenAPI. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.)
Milestone
Post-1.0

Comments

@czechboy0
Copy link
Contributor

Support deepObject style for query params.

Used by the Stripe API, for example.
@czechboy0 czechboy0 added area/openapi Adding/updating a feature defined in OpenAPI. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.) labels Sep 9, 2023
@czechboy0 czechboy0 added this to the 1.0 milestone Sep 9, 2023
@czechboy0
Copy link
Contributor Author

Not just for query items, but also URL encoded bodies, once we support them.

@tib
Copy link
Contributor

tib commented Oct 26, 2023

I just encountered this issue and I'd love to see better support for query params, let me know if you need help with it. :)

@czechboy0
Copy link
Contributor Author

Yes support for at least deepObject is planned for 1.0, and it would be awesome if you could contribute it. I suspect most of the changes would be in the runtime library's URIEncoder and URIDecoder, please take a look there and see if adding deepObject support would be a project you want to take on. Note that while it's being improved, it might also make sense to just do #258 as well, as the latter might be a natural extension of the existing logic. deepObject will be slightly more involved, but shouldn't be too difficult.

Take a look at how the encoder/decoder works today and see if you can extend it, and I'm here if you have any questions 🙂

@czechboy0
Copy link
Contributor Author

I'll assign you to the issue for now, but don't feel obligated if you decide not to do it, just let me know and I'll unassign it. It's to avoid multiple people working on the same issue 🙂

@czechboy0 czechboy0 mentioned this issue Oct 26, 2023
Open
@tib
Copy link
Contributor

tib commented Oct 26, 2023

Cool, I'll think about this, but can't promise anything, if I have time I can definitely implement this, it'd be cool to eliminate the current workaround from my sample project. 😅

@czechboy0 czechboy0 mentioned this issue Oct 26, 2023
Closed
@czechboy0
Copy link
Contributor Author

A related ask for this: #292

@czechboy0
Copy link
Contributor Author

Ok just let us know if you start working on this @tib, but no pressure. I unassigned it for now to leave it available for others to pick up.

@czechboy0 czechboy0 modified the milestones: 1.0, Post-1.0 Nov 15, 2023
kstefanou52 added a commit to kstefanou52/swift-openapi-runtime that referenced this issue Mar 7, 2024
@kstefanou52
[Runtime] Add support of deepObjects query styling
2529143 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.
kstefanou52 added a commit to kstefanou52/swift-openapi-runtime that referenced this issue Mar 7, 2024
@kstefanou52
[Runtime] Add support of deepObject style in query
f421950 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.
@kstefanou52 kstefanou52 mentioned this issue Mar 7, 2024
Merged
kstefanou52 added a commit to kstefanou52/swift-openapi-generator that referenced this issue Mar 7, 2024
@kstefanou52
[Generator] Add support of deepObject style in query params
82556e6 
### 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)
@kstefanou52 kstefanou52 mentioned this issue Mar 7, 2024
Open
@kstefanou52
Copy link

Hi @czechboy0, I encountered the same issue so I took the initiative to address it.
Excited to hear your thoughts when you get a chance to check my PRs out. Cheers! 🚀

kstefanou52 added a commit to kstefanou52/swift-openapi-runtime that referenced this issue Mar 7, 2024
@kstefanou52
[Runtime] Add support of deepObject style in query params
28265a7 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.
@czechboy0
Copy link
Contributor Author

Hi @kstefanou52 - sure I just came back from vacation, let me take a look shortly.

@Raedalmalki250 Raedalmalki250 mentioned this issue Mar 11, 2024
Open
kstefanou52 added a commit to kstefanou52/swift-openapi-runtime that referenced this issue Apr 4, 2024
@kstefanou52
[Runtime] Add support of deepObject style in query params
c2ab707 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.
kstefanou52 added a commit to kstefanou52/swift-openapi-runtime that referenced this issue Apr 9, 2024
@kstefanou52
[Runtime] Add support of deepObject style in query params
3e3b4e6 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.
czechboy0 added a commit to apple/swift-openapi-runtime that referenced this issue Apr 16, 2024
@kstefanou52 @czechboy0
[Runtime] Add support of deepObject style in query params (#100)
9a8291f 
### Motivation

The runtime changes for:
apple/swift-openapi-generator#259

### 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

These are just the runtime changes, tested together with generated
changes.

---------

Co-authored-by: Honza Dvorsky <honza@apple.com>
@czechboy0
Copy link
Contributor Author

Runtime changes landed in swift-openapi-runtime 1.4.0.

@arthurcro arthurcro mentioned this issue Oct 24, 2024
Draft
kstefanou52 added a commit to kstefanou52/swift-openapi-generator that referenced this issue Oct 26, 2024
@kstefanou52
[Generator] Add support of deepObject style in query params
368d569 
apple#259

~Depends on apple/swift-openapi-runtime#100
landing first and getting released, and the version dependency being
bumped here.~

Added `deepObject` style to serializer & parser in order to support nested keys on query parameters.

Support nested keys on query parameters.

Adapted snippet tests (SnippetBasedReferenceTests)
@czechboy0 czechboy0 mentioned this issue Nov 13, 2024
Merged
czechboy0 added a commit to apple/swift-openapi-runtime that referenced this issue Nov 21, 2024
@czechboy0 @simonjbeaumont
Refactor URIDecoder/URIParser to improve handling of the deepObject s…
5e119a3 
…tyle (#127)

### Motivation

As part of apple/swift-openapi-generator#259,
adding deepObject parameter style support, the initial PR wasn't
complete.

And once we dug more into it, turns out the original implementation of
the URIDecoder/URIParser didn't really lend themselves well for handling
deepObject, and the recent additions of supporting arrays within
dictionaries (#120)
further confused the implementation.

### Modifications

Refactored URIParser/URIDecoder with a clearer understanding of the
current requirements. It's now much easier to follow and embraces the
fact that each of the 7 variants of URI coding we support (form
exploded, form unexploded, simple exploded, simple unexploded, form data
exploded, form data unexploded, and now deepObject exploded) are
similar, but still different in subtle ways.

This new implementation doesn't try as hard to share code between the
implementations, so might at first sight appear to duplicate code. The
original implementation had many methods with many configuration
parameters and utility methods with a high cyclomatic complexity, which
made it very hard to reason about. We did away with that.

While there, I also made some minor improvements to the serialization
path, which allows cleaner round-tripping tests.

### Result

A more maintainable and more correct URI decoder/parser implementation.

### Test Plan

Added many more unit tests that test the full matrix of supported styles
and inputs.

---------

Co-authored-by: Si Beaumont <simonjbeaumont@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/openapi Adding/updating a feature defined in OpenAPI. kind/enhancement Improvements to existing feature. size/S Small task. (A couple of hours of work.)
Projects
None yet
Milestone
Post-1.0
Development

No branches or pull requests
3 participants
@tib @czechboy0 @kstefanou52