diff --git a/.changeset/drift-multi-parameter-path-segments.md b/.changeset/drift-multi-parameter-path-segments.md new file mode 100644 index 0000000000..fed5547851 --- /dev/null +++ b/.changeset/drift-multi-parameter-path-segments.md @@ -0,0 +1,7 @@ +--- +'@redocly/cli': patch +--- + +Fixed `drift` and `coverage` failing to match a path template whose segment mixes literal text with parameters, such as `/instances/{worldId}:{instanceId}`. +Only a segment that was entirely one parameter was recognized, so these templates were compiled as literal text and never matched any request. +Affected requests were reported as undocumented by `drift` and left out of the `coverage` figures. diff --git a/packages/cli/src/commands/drift/__tests__/compile-openapi-path.test.ts b/packages/cli/src/commands/drift/__tests__/compile-openapi-path.test.ts new file mode 100644 index 0000000000..f7e83d2039 --- /dev/null +++ b/packages/cli/src/commands/drift/__tests__/compile-openapi-path.test.ts @@ -0,0 +1,58 @@ +import { compileOpenApiPath } from '../utils/http.js'; + +describe('compileOpenApiPath', () => { + it('compiles a segment that is a single parameter', () => { + const { regex, params } = compileOpenApiPath('/users/{userId}'); + + expect(params).toEqual(['userId']); + expect(regex.exec('/users/usr_abc')?.[1]).toBe('usr_abc'); + }); + + it('does not let a parameter span a path separator', () => { + const { regex } = compileOpenApiPath('/users/{userId}'); + + expect(regex.exec('/users/usr_abc/friends')).toBeNull(); + }); + + it('compiles two parameters separated by a literal inside one segment', () => { + const { regex, params } = compileOpenApiPath('/instances/{worldId}:{instanceId}'); + + expect(params).toEqual(['worldId', 'instanceId']); + + const match = regex.exec( + '/instances/wrld_a:85981~group(grp_b)~groupAccessType(public)~region(us)' + ); + expect(match?.[1]).toBe('wrld_a'); + expect(match?.[2]).toBe('85981~group(grp_b)~groupAccessType(public)~region(us)'); + }); + + it('splits on the first separator when the trailing value holds another', () => { + const { regex } = compileOpenApiPath('/instances/{worldId}:{instanceId}'); + const match = regex.exec('/instances/wrld_a:12345~region(us):extra'); + + expect(match?.[1]).toBe('wrld_a'); + expect(match?.[2]).toBe('12345~region(us):extra'); + }); + + it('keeps matching a multi-parameter segment when a suffix segment follows', () => { + const { regex } = compileOpenApiPath('/instances/{worldId}:{instanceId}/shortName'); + + expect(regex.exec('/instances/wrld_a:123~private(usr_b)/shortName')?.[2]).toBe( + '123~private(usr_b)' + ); + }); + + it('ranks a partially literal segment above a bare parameter', () => { + expect(compileOpenApiPath('/instances/{worldId}:{instanceId}').score).toBeGreaterThan( + compileOpenApiPath('/instances/{instanceId}').score + ); + }); + + it('treats a segment with no parameters as a literal', () => { + const { regex, params } = compileOpenApiPath('/instances/recent'); + + expect(params).toEqual([]); + expect(regex.exec('/instances/anything')).toBeNull(); + expect(regex.exec('/instances/recent')).not.toBeNull(); + }); +}); diff --git a/packages/cli/src/commands/drift/utils/http.ts b/packages/cli/src/commands/drift/utils/http.ts index 5e2f4a9397..cee38562b9 100644 --- a/packages/cli/src/commands/drift/utils/http.ts +++ b/packages/cli/src/commands/drift/utils/http.ts @@ -167,14 +167,36 @@ export function compileOpenApiPath(pathTemplate: string): { return ''; } - const paramMatch = segment.match(/^\{([^}]+)\}$/); - if (paramMatch) { + // A segment may hold several parameters around literal text, as in + // `/instances/{worldId}:{instanceId}`. Matching the whole segment as one + // parameter would miss those, and treating it as a literal never matches. + const paramsBefore = params.length; + let compiled = ''; + let literalLength = 0; + let offset = 0; + + for (const paramMatch of segment.matchAll(/\{([^}]+)\}/g)) { + const literal = segment.slice(offset, paramMatch.index); + + compiled += escapeRegex(literal); + literalLength += literal.length; params.push(paramMatch[1]); - return '([^/]+)'; + compiled += '([^/]+?)'; + offset = paramMatch.index + paramMatch[0].length; } - score += 2; - return escapeRegex(segment); + const trailing = segment.slice(offset); + compiled += escapeRegex(trailing); + literalLength += trailing.length; + + if (params.length === paramsBefore) { + score += 2; + } else if (literalLength > 0) { + // More specific than a bare parameter, less so than a whole literal. + score += 1; + } + + return compiled; }) .join('/');