Skip to content

Adding nested object support for deepObject style #1450

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

Closed
wants to merge 7 commits into from

Conversation

DarkSorrow
Copy link

Adding deepObject query parameters serialisation and fix error when a value was null in the object

Description

Created a function to transform the object pass as deepObject stype to query parameters, it uses the same style serialisation as before.

Motivation and Context

My company is using this style of GET method to make filtered query, it seems that many REST API use this method as well to benefit from cache and such. I've read the post such as #1385 and i know that you are talking about asking people from the draft to change this but i know a lot of people are already using this style so i thought i'll just contribute the code that worked for me.
Just like people use nutella to make ice cream instead of just their bread like the picture i suppose it is fine to let them use nested object in their get if they want this :x

How Has This Been Tested?

Started using it in our production environement

Screenshots (if appropriate):

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ethlo
Copy link

ethlo commented Jan 29, 2020

Any progress in this area?

@DarkSorrow
Copy link
Author

Well i'm using my code for deep objects since last year and it seems to be working pretty well for the different use case we had

@ethlo
Copy link

ethlo commented Jan 31, 2020

Thank you. Any reason why this is not merged yet? Having PRs hanging around for a year seems quite counter-productive :-)

@char0n char0n self-assigned this Jul 7, 2020
@char0n char0n self-requested a review July 7, 2020 08:15
@char0n char0n added this to the M4 milestone Jul 7, 2020
@char0n
Copy link
Member

char0n commented Jul 7, 2020

Hi @DarkSorrow,

First of all thank you for contributing this change!

As mentioned in #1385 and subsequent discussion in OAI/OpenAPI-Specification#1706 with no clear decision, at this point even with OAS 3.1 it is still not specified how to serialize deep nested structures using deepObject.

This PR would force everybody using swagger-client to use the style of serialization that your company users. You can understand we are reserved to merge this PR. Ultimately, things like deepObject should be controlled via extensions, then plugins to implement the extended functionality, and should not be part of the core library.

At this point of time we will not proceed with merging this PR.

@char0n char0n closed this Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants