Skip to content

Commit

Permalink
fix: Dereference path parameters (#962)
Browse files Browse the repository at this point in the history
The OpenAPI spec loader has a `discoverRoutes` method which explores an OpenAPI document
and gathers information about the paths and parameters used.
The list of discovered path parameters is used to install parameter-specific middleware in `src/openapi.validator.ts#installPathParams`
Path parameters declared with `$ref` were not detected in the `discoverRoutes` implementation, leading to the un-coerced values being used.
By dereferencing each path parameter when building this list, we should see the same behavior for referenced path parameters and for inline path parameters.

Closes #803
  • Loading branch information
duncanbeevers authored Sep 5, 2024
1 parent 0a8dc2f commit 0aebe5d
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
19 changes: 11 additions & 8 deletions src/framework/openapi.spec.loader.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { dereferenceParameter } from '../middlewares/parsers/util';
import { OpenAPIFramework } from './index';
import {
OpenAPIFrameworkAPIContext,
Expand Down Expand Up @@ -68,13 +69,15 @@ export class OpenApiSpecLoader {
) {
continue;
}
const pathParams = new Set<string>();
const parameters = [...schema.parameters ?? [], ...methods.parameters ?? []]
for (const param of parameters) {
if (param.in === 'path') {
pathParams.add(param.name);
}
}

const pathParams = [
...(schema.parameters ?? []),
...(methods.parameters ?? []),
]
.map(param => dereferenceParameter(apiDoc, param))
.filter(param => param.in === 'path')
.map(param => param.name);

const openApiRoute = `${bp}${path}`;
const expressRoute = `${openApiRoute}`
.split(':')
Expand All @@ -86,7 +89,7 @@ export class OpenApiSpecLoader {
expressRoute,
openApiRoute,
method: method.toUpperCase(),
pathParams: Array.from(pathParams),
pathParams: Array.from(new Set(pathParams)),
});
}
}
Expand Down
8 changes: 8 additions & 0 deletions test/operation.handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,12 @@ describe('custom operation handler', () => {
});
});

it('should coerce path parameters', async () => {
return request(app)
.get(`${basePath}/users/123/info`)
.expect(200)
.then((r) => {
expect(r.text).to.be.equal('{"id":123}');
});
});
});
30 changes: 30 additions & 0 deletions test/resources/eov-operations.modulepath.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,35 @@ paths:
properties:
success:
type: boolean
/users/{userID}/info:
get:
description: Get info about a User
summary: Get info about a User
operationId: user.info
security: []
parameters:
- $ref: '#/components/parameters/userID'
responses:
'200':
description: Returns info about a User.
content:
'application/json':
schema:
type: object
properties:
id:
$ref: '#/components/schemas/UserID'

components:
parameters:
userID:
name: userID
in: path
required: true
schema:
$ref: '#/components/schemas/UserID'
# type: number

schemas:
Pet:
required:
Expand All @@ -210,6 +237,9 @@ components:
- dog
- cat

UserID:
type: number

Error:
required:
- code
Expand Down
3 changes: 3 additions & 0 deletions test/resources/routes/user.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
info: (req, res) => res.status(200).send({ id: req.params.userID })
};

0 comments on commit 0aebe5d

Please sign in to comment.