Skip to content

Add deepObject support #379

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

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

Yarn-e
Copy link
Contributor

@Yarn-e Yarn-e commented Nov 16, 2021

Problem statement

Currently deepObjects are not supported, this PR adds that support.

Description

Example spec

components:
    parameters:
      exampleParameter:
        name: paramObj
        in: query
        required: true
        explode: true
        style: deepObject
        schema:
          type: object
          properties: 
            count: 
              type: integer
            name:
              type: string

Sadly it doesn't really fit in the ParameterDeserializersFactory ideology as it would never reach that step as get_value() in parameters.py would always fail as it tries to find the name of the param_or_header in the specific location. This is because how a deepObjects query param looks like (e.g. someurl.com/?paramObj[count]=1&paramObj[name]=John), the function would try to find paramObj in the location, which it would not find as the QueryParams will be split into a dict like this: {'paramObj[count]': 1, 'paramObj[name]': John}.

In my proposed solution it would split the location dict key into 2 parts, the actual key we want to find paramObj and then create a dict of the 2nd part of the key (whats inside the brackets) with its actual value: {'count': 1, 'name': 'John'}.
Afterwards this dict is checked via the schema of the object.

I highly doubt this solution will get accepted, so if you know a way to make this cleaner/better please lmk 🙏

Todos

  • Tests

Related issue

@p1c2u p1c2u force-pushed the feature/deep-object-support branch 2 times, most recently from d084df1 to 23cc3e4 Compare September 23, 2022 13:56
@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #379 (cc42e15) into master (868c081) will decrease coverage by 0.08%.
The diff coverage is 82.35%.

@@            Coverage Diff             @@
##           master     #379      +/-   ##
==========================================
- Coverage   93.16%   93.08%   -0.09%     
==========================================
  Files          94       94              
  Lines        2311     2327      +16     
  Branches      286      291       +5     
==========================================
+ Hits         2153     2166      +13     
- Misses        123      124       +1     
- Partials       35       37       +2     
Impacted Files Coverage Δ
openapi_core/schema/parameters.py 92.45% <81.25%> (-4.92%) ⬇️
openapi_core/deserializing/parameters/factories.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@p1c2u p1c2u force-pushed the feature/deep-object-support branch from 23cc3e4 to cc42e15 Compare September 23, 2022 13:59
Copy link
Collaborator

@p1c2u p1c2u left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add this as experimental support.

@p1c2u p1c2u merged commit e3da8d3 into python-openapi:master Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants