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.

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

Swagger spec is not normalized if structure is hidden behind a reference #1396

Open
biochimia opened this issue Oct 16, 2018 · 3 comments · May be fixed by jetboard/swagger-editor#5
Open

Swagger spec is not normalized if structure is hidden behind a reference #1396

biochimia opened this issue Oct 16, 2018 · 3 comments · May be fixed by jetboard/swagger-editor#5

Comments

@biochimia
Copy link

biochimia commented Oct 16, 2018

I tracked down a regression in swagger-ui to an interaction between swagger normalization and subtree resolution introduced in #1274.

With that change, normalization happens only before subtree resolution, and this breaks in cases where the structure of the references hides the structure of the swagger file from normalization.

I boiled it down to this this test master...biochimia:regression/normalize-before-resolution

Q&A (please complete the following information)

  • OS: [e.g. macOS] macOS
  • Environment: [e.g. Chrome 59, Node.js v10.0.0] Node.js v10.12.0
  • Method of installation: [e.g. npm, unpkg] npm
  • Swagger-Client version: [e.g. 3.8.0] >3.6.0, <=3.8.21
  • Swagger/OpenAPI version: [e.g. Swagger 2.0, OpenAPI 3.0] OpenAPI 3.0

Content & configuration

Test case: master...biochimia:regression/normalize-before-resolution

Swagger/OpenAPI definition:

paths.yaml:
  root:
    parameters:
      - $ref: '#/parameters.yaml/accept-header'
    get:
      parameters:
        - $ref: '#/parameters.yaml/name'
        - $ref: '#/parameters.yaml/status'
parameters.yaml:
  accept-header:
    name: accept
    in: header
    description: content-type for response
    required: true
    type: string
  name:
    name: name
    in: formData
    description: name of the pet
    required: false
    type: string
  status:
    name: status
    in: formData
    description: status of the pet
    required: false
    type: string
paths:
  /:
    $ref: '#/paths.yaml/root'

Swagger-Client usage:

SwaggerClient({
  // your config options here
})

Describe the bug you're encountering

Because normalizeSwagger() doesn't get to see the structure of the path object before resolution, if doesn't copy path parameters into individual methods. Later when the subtrees are resolved normalization is already disabled and is not performed.

To reproduce...

Steps to reproduce the behavior:

  1. ...

Expected behavior

Screenshots

Additional context or thoughts

@char0n
Copy link
Member

char0n commented Jan 3, 2023

The same bug will be part of normalization that will be introduced for OpenAPI 3.1 in #2738.

This issue needs to be processed after introducting OpenAPI 3.1.0 support in swagger-client for both OpenAPI 2.0 - 3.0.x and OpenAPI 3.1.0 normalizations.

@char0n
Copy link
Member

char0n commented May 23, 2024

We will not be addressing this bug in old specmap based dereferencing algorithm, but we will address it in new ApiDOM based dereferencing. ApiDOM based dereferencing is currently handling OpenAPI 3.1.0 exclusively. Old specmap based dereferencing is handling OpenAPI 2.0 and OpenAPI 3.0.x. In future ApiDOM based dereferencing will completely replace specmap based dereferencing so this bug will be remediated fully. Later solution proposal addressed ApiDOM based dereferencing only.

Solution

We can reduce resolve and resolveSubtree to single use-case, as the latter one is just a specialization of resolve function using pathDistriminator option.

pathDiscriminator is NOT set

resolve performs full document normalization after the dereferencing on skipNormalization=false .
If $$normalized field is set, we skip the full document normalization to avoid double normalizing the definition.
And that's it. Normalizing fully resolved document is simple as that.

$$normalized field will look like the example below after the full document normalization. $normalized field is maintaining list of passed pathDiscriminators which are translated to JSON Pointers. If pathDiscriminator is not set, it's translated implicitly to / JSONPointer, which represents entire document.

$$normalized: [
  "/", // => represents JSON Pointer
]

Additional metadata for individual plugins will be maintained on additional fields:

$$normalized-servers: [
  "/servers", // => means that defaults Server Object have been added (if no Server Object is present)
]

pathDiscriminator is set

If pathDiscriminator is set and $$normalized field is not yet set (this also means that no metadata exists for individual plugins), then we perform full document normalization after the dereferencing.

if pathDiscriminator is set and $$normalized field and individual plugin metadata fields are set, we limit the scope of the normalization to pathDiscriminator, because we assume that the full document normalization was already performed.

Required characteristics

All normalization plugins MUST implement idempotence. This means that each plugin with maintain information about what has already been normalized, and if it traverse the already normalized piece of data, it must skip the normalization. Implication is that we can run normalization multiple times on whole documents.

$$normalized field CAN be maintained outside of the dispatching of normalize plugins or inside of the integrating code. Probably it is easier to maintain it inside the integration code and make normalize plugins unaware of it. Normalize plugins only require scope option represented by pathDiscirminator. pathDiscriminator can be set to empty list, which represents JSONPointer(/) which effectively disables the scoping.

@char0n
Copy link
Member

char0n commented May 29, 2024

Next job consists of 3 steps:

  1. Integrated the new ApiDOM release in swagger-js
  2. Always perform normalization with every subTreeResolve call
  3. integrate refractor plugin into overall dereference traversal to avoid additional traversal pass

These steps are performed after next ApiDOM release is issued.

char0n added a commit that referenced this issue Jul 15, 2024
swagger-bot pushed a commit that referenced this issue Aug 9, 2024
## [3.28.3](v3.28.2...v3.28.3) (2024-08-09)

### Bug Fixes

* **resolver:** disable Server Objects normalization for OpenAPI 3.1.0 ([#3628](#3628)) ([d63f33f](d63f33f)), closes [#3627](#3627)
* **resolver:** don't skip normalization of OpenAPI 3.1.0 specs ([#3575](#3575)) ([bc4a40c](bc4a40c)), closes [#1396](#1396)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants