Skip to content
This repository has been archived by the owner on Nov 19, 2023. It is now read-only.

Check anyOf and oneOf schemas in nullable validation #300

Merged
merged 1 commit into from
Jul 4, 2023
Merged

Check anyOf and oneOf schemas in nullable validation #300

merged 1 commit into from
Jul 4, 2023

Conversation

arttuperala
Copy link
Contributor

test_is_nullable() is executed before the normal test_schema_section() processing, including the anyOf and oneOf checking, takes place, and if your nullable definition comes from inside either anyOf or oneOf, the validation will fail.

@JonasKs JonasKs requested a review from sondrelg July 3, 2023 10:11
@codecov
Copy link

codecov bot commented Jul 3, 2023

Codecov Report

Merging #300 (7eb51c0) into master (b2aa34e) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff           @@
##           master    #300   +/-   ##
======================================
  Coverage    98.6%   98.6%           
======================================
  Files           9       9           
  Lines         522     528    +6     
  Branches       93      97    +4     
======================================
+ Hits          515     521    +6     
  Misses          4       4           
  Partials        3       3           
Impacted Files Coverage Δ
openapi_tester/schema_tester.py 99.3% <100.0%> (+<0.1%) ⬆️

Copy link
Member

@sondrelg sondrelg left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Have to admit, it's been a while since I looked at this code, so it's hard to evaluate the PR. Could you elaborate a little, and/or give an example, on what would fail previously that now passes?

@arttuperala
Copy link
Contributor Author

arttuperala commented Jul 4, 2023

It basically just allows the test_is_nullable() to pass without issues if the nullable: true comes from within an anyOf or oneOf.

We're basically working with a JSON:API schema that looks a bit something like this:

components:
  schemas:
    reltoone:
      type: object
      properties:
        data:
          $ref: '#/components/schemas/relationshipToOne'
    relationshipToOne:
      anyOf:
      - $ref: '#/components/schemas/nulltype'
      - $ref: '#/components/schemas/linkage'
    nulltype:
      type: object
      nullable: true
      default: null
    linkage:
      type: object
      properties:
        type:
          $ref: '#/components/schemas/type'
        id:
          $ref: '#/components/schemas/id'
        meta:
          $ref: '#/components/schemas/meta'
    ABC:
      type: object
      properties:
        relationships:
          type: object
          properties:
            xyz:
              $ref: '#/components/schemas/reltoone'

And then we'd feed the validator a response like this:

{
    "data": {
        "relationships": {
            "xyz": {
                "data": null
            }
        }
    }
}

Even though /data/relationships/xyz/data is a relationshipToOne, where one of its possible schemas is nulltype with nullable: true, it'd fail the validation because the current code could not peek inside the anyOf array to figure out if one of the possible values is nullable.

@sondrelg sondrelg merged commit 8cd209a into snok:master Jul 4, 2023
16 checks passed
@sondrelg
Copy link
Member

sondrelg commented Jul 4, 2023

Looks good to me! I'll release a fix shortly 👍

@arttuperala
Copy link
Contributor Author

I might have another PR coming up also, since there's also another issue that I'm having with the validators.

@arttuperala arttuperala deleted the anyof-nullable branch July 4, 2023 07:19
@sondrelg
Copy link
Member

sondrelg commented Jul 4, 2023

Ah sorry, missed this. No worries though, I can do another release 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants