Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/core/plugins/spec/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import assocPath from "lodash/fp/assocPath"
import constant from "lodash/constant"

import { paramToValue, isEmptyValue } from "core/utils"
import { inheritPathItemParameters } from "./path-item-parameters"

// Actions conform to FSA (flux-standard-actions)
// {type: string,payload: Any|Error, meta: obj, error: bool}
Expand Down Expand Up @@ -246,6 +247,15 @@ const debResolveSubtrees = debounce(() => {
specWithCurrentSubtrees: specSelectors.specJS()
}))

// Mirror swagger-client's path-item parameter inheritance for
// path items dereferenced from external files. The generic
// normalize step in swagger-client runs against the still-$ref'd
// root spec, so operations under $ref'd path items never inherit
// their path-level parameters. See issue #5667.
if (batchResult.resultMap && batchResult.resultMap.paths) {
inheritPathItemParameters(batchResult.resultMap.paths)
}

specActions.updateResolvedSubtree([], batchResult.resultMap)
} catch(e) {
console.error(e)
Expand Down
87 changes: 87 additions & 0 deletions src/core/plugins/spec/path-item-parameters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/**
* @prettier
*/

const OPERATION_METHODS = [
"get",
"put",
"post",
"delete",
"options",
"head",
"patch",
"trace",
"query",
]

const isPlainObject = (value) =>
value !== null && typeof value === "object" && !Array.isArray(value)

// Matches the duplicate-detection logic used by swagger-client's
// generic normalizer (`resolver/strategies/generic/normalize.js`)
// so we stay consistent with what the resolver would have produced
// if it had been able to inherit before dereferencing.
const isSameParameter = (a, b) => {
if (!isPlainObject(a) && !isPlainObject(b)) return false
if (a === b) return true
return ["name", "$ref", "$$ref"].some(
(key) =>
typeof a[key] === "string" &&
typeof b[key] === "string" &&
a[key] === b[key]
)
}

/**
* Mutates path items in `paths` so that every operation inherits
* path-level parameters that are not already declared on the operation.
*
* This mirrors the behaviour of swagger-client's generic `normalize` step
* (`resolver/strategies/generic/normalize.js`), which is skipped when a
* path item is dereferenced from an external file via `$ref`. Without
* this inheritance step, operations that do not declare the path-level
* parameters themselves (for example, an `options` operation with no
* `parameters` of its own) render with the path-level parameters missing.
*
* The merge is idempotent: parameters already present on the operation
* are left untouched, using the same duplicate-detection logic as
* swagger-client.
*
* @param {Object} paths the `paths` object from a resolved subtree
* @returns {Object} the same `paths` object, mutated in place
*/
export const inheritPathItemParameters = (paths) => {
if (!isPlainObject(paths)) {
return paths
}

Object.keys(paths).forEach((pathName) => {
const pathItem = paths[pathName]
if (!isPlainObject(pathItem)) return

const pathParameters = pathItem.parameters
if (!Array.isArray(pathParameters) || pathParameters.length === 0) return

OPERATION_METHODS.forEach((method) => {
const operation = pathItem[method]
if (!isPlainObject(operation)) return

if (!Array.isArray(operation.parameters)) {
operation.parameters = []
}

pathParameters.forEach((pathParam) => {
const alreadyPresent = operation.parameters.some((opParam) =>
isSameParameter(opParam, pathParam)
)
if (!alreadyPresent) {
operation.parameters.push(pathParam)
}
})
})
})

return paths
}

export default inheritPathItemParameters
29 changes: 29 additions & 0 deletions test/e2e-cypress/e2e/bugs/5667.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @prettier
*/
// https://github.com/swagger-api/swagger-ui/issues/5667
// Path-level (a.k.a. "global") parameters defined on a path item should
// merge into every operation under that path, even when the path item
// itself was reached through a cross-file $ref.

describe("#5667: Global parameters not visible when referenced through other definition", () => {
beforeEach(() => {
cy.visit("/?url=/documents/bugs/5667-root.yaml")
})

it("should render the path-level parameter on an operation that declares no parameters of its own (options)", () => {
cy.get("#operations-pets-optionsPetById").click()

cy.get("#operations-pets-optionsPetById")
.find("table.parameters .parameter__name")
.should("contain.text", "petId")
})

it("should render the path-level parameter on a sibling operation (get) too", () => {
cy.get("#operations-pets-showPetById").click()

cy.get("#operations-pets-showPetById")
.find("table.parameters .parameter__name")
.should("contain.text", "petId")
})
})
33 changes: 33 additions & 0 deletions test/e2e-cypress/static/documents/bugs/5667-pets.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
openapi: 3.0.4
info:
title: 5667 - External pet store
version: 1.0.0
paths:
/pets/{petId}:
parameters:
- $ref: '#/components/parameters/PetIdPathParam'
get:
summary: Info for a specific pet
operationId: showPetById
tags:
- pets
responses:
'200':
description: ok
options:
summary: Options for a specific pet
operationId: optionsPetById
tags:
- pets
responses:
'200':
description: ok
components:
parameters:
PetIdPathParam:
in: path
name: petId
required: true
description: The id of the pet to retrieve
schema:
type: string
7 changes: 7 additions & 0 deletions test/e2e-cypress/static/documents/bugs/5667-root.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
openapi: 3.0.4
info:
title: 5667 - Global parameters via $ref
version: 1.0.0
paths:
/pets/{petId}:
$ref: ./5667-pets.yaml#/paths/~1pets~1{petId}
170 changes: 170 additions & 0 deletions test/unit/core/plugins/spec/path-item-parameters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/**
* @prettier
*/

import { inheritPathItemParameters } from "core/plugins/spec/path-item-parameters"

describe("spec plugin - path-item-parameters helper", () => {
it("should be a no-op when paths is not a plain object", () => {
expect(inheritPathItemParameters(null)).toBe(null)
expect(inheritPathItemParameters(undefined)).toBe(undefined)
expect(inheritPathItemParameters("nope")).toBe("nope")
expect(inheritPathItemParameters([])).toEqual([])
})

it("should inherit path-level parameters into operations that lack them", () => {
const pathParam = {
in: "path",
name: "petId",
required: true,
schema: { type: "string" },
}
const paths = {
"/pets/{petId}": {
parameters: [pathParam],
get: {
operationId: "showPetById",
parameters: [
{
in: "query",
name: "verbose",
schema: { type: "boolean" },
},
],
},
options: {
operationId: "optionsPetById",
},
},
}

inheritPathItemParameters(paths)

const get = paths["/pets/{petId}"].get
const options = paths["/pets/{petId}"].options

expect(get.parameters).toHaveLength(2)
expect(get.parameters).toEqual(
expect.arrayContaining([
expect.objectContaining({ name: "verbose", in: "query" }),
expect.objectContaining({ name: "petId", in: "path" }),
])
)

expect(options.parameters).toHaveLength(1)
expect(options.parameters[0]).toEqual(
expect.objectContaining({ name: "petId", in: "path" })
)
})

it("should not duplicate parameters that the operation already declares by name", () => {
const pathParam = {
in: "path",
name: "petId",
required: true,
schema: { type: "string" },
}
const paths = {
"/pets/{petId}": {
parameters: [pathParam],
get: {
parameters: [
{
in: "path",
name: "petId",
description: "operation-level override",
schema: { type: "string" },
},
],
},
},
}

inheritPathItemParameters(paths)

expect(paths["/pets/{petId}"].get.parameters).toHaveLength(1)
expect(paths["/pets/{petId}"].get.parameters[0].description).toBe(
"operation-level override"
)
})

it("should not duplicate parameters that share a $ref", () => {
const pathParam = { $ref: "#/components/parameters/PetIdPathParam" }
const paths = {
"/pets/{petId}": {
parameters: [pathParam],
get: {
parameters: [{ $ref: "#/components/parameters/PetIdPathParam" }],
},
},
}

inheritPathItemParameters(paths)

expect(paths["/pets/{petId}"].get.parameters).toHaveLength(1)
})

it("should leave operations untouched when the path item has no parameters", () => {
const paths = {
"/pets": {
get: {
parameters: [
{ in: "query", name: "limit", schema: { type: "integer" } },
],
},
},
}

inheritPathItemParameters(paths)

expect(paths["/pets"].get.parameters).toHaveLength(1)
expect(paths["/pets"].get.parameters[0].name).toBe("limit")
})

it("should ignore non-operation keys on the path item", () => {
const pathParam = {
in: "path",
name: "petId",
schema: { type: "string" },
}
const paths = {
"/pets/{petId}": {
summary: "Operations on a single pet",
description: "Use this group for pet operations",
servers: [{ url: "https://example.com" }],
parameters: [pathParam],
get: { operationId: "showPetById" },
},
}

inheritPathItemParameters(paths)

// summary/description/servers should remain plain values
expect(paths["/pets/{petId}"].summary).toBe("Operations on a single pet")
expect(paths["/pets/{petId}"].description).toBe(
"Use this group for pet operations"
)
expect(Array.isArray(paths["/pets/{petId}"].servers)).toBe(true)

expect(paths["/pets/{petId}"].get.parameters).toHaveLength(1)
})

it("should treat the operation parameters as an array even when missing", () => {
const pathParam = {
in: "path",
name: "petId",
schema: { type: "string" },
}
const paths = {
"/pets/{petId}": {
parameters: [pathParam],
options: {},
},
}

inheritPathItemParameters(paths)

expect(Array.isArray(paths["/pets/{petId}"].options.parameters)).toBe(true)
expect(paths["/pets/{petId}"].options.parameters).toHaveLength(1)
})
})