Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(json-schema-2020-12): Fix OAS 3.1 additionalProperties false #1

Conversation

adavis444
Copy link
Owner

@adavis444 adavis444 commented Jul 16, 2023

(Local mirror of swagger-api#9023)

Description

swagger-ui incorrectly adds "additionalProp1": {} to samples for schemas where additionalProperties is false in OpenAPI 3.1.0, even though this was previously correct in OpenAPI 3.0.x.

This fixes a change in swagger-api#8910 that did not handle the case that additionalProperties is false, which broadened the condition additionalProperties === true to isBooleanJSONSchema(additionalProperties) with the same handling, treating an additionalProperties value of false identically to a value of true.

This can be remedied by changing isBooleanJSONSchema(additionalProperties) to isBooleanJSONSchema(additionalProperties) && additionalProperties === true.

This restores some parity between src/core/plugins/json-schema-2020-12/samples-extensions/fn/main.js and src/core/plugins/samples/fn/index.js.

Motivation and Context

No additional properties should be added to samples for schemas where additionalProperties is false, maintaining consistency with the behavior in OpenAPI 3.0.x.

Fixes swagger-api#9022

Refs swagger-api#8577

How Has This Been Tested?

Following the steps in "To reproduce..." in swagger-api#9022 using that openapi.json example, on master we have a sample body for a POST / request of

{
  "bar": "string",
  "additionalProp1": {}
}

On this branch, it is

{
  "bar": "string"
}

I've added identical tests to test/unit/core/plugins/samples/fn/index.js and to test/unit/core/plugins/json-schema-2020-12/samples-extensions/fn.js.

However, as seen in the unit test results when tests are added without code changes, on master it is expected that the tests for test/unit/core/plugins/samples/fn/index.js will pass whereas the tests for test/unit/core/plugins/json-schema-2020-12/samples-extensions/fn.js will fail.

Screenshots (if appropriate):

On master
image

On this branch
image

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@adavis444 adavis444 marked this pull request as draft July 16, 2023 19:22
@adavis444 adavis444 closed this Jul 24, 2023
@adavis444 adavis444 deleted the bug/9022-fix-OAS-3.1-additionalProperties-false-samples branch July 24, 2023 13:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAPI 3.1.0 support: additionalProperties: false adds additionalProp1 to samples
2 participants