diff --git a/packages/oas/src/operation/index.ts b/packages/oas/src/operation/index.ts index 01c9242c..ffd5af22 100644 --- a/packages/oas/src/operation/index.ts +++ b/packages/oas/src/operation/index.ts @@ -350,17 +350,38 @@ export class Operation { * a hash of the path and method will be returned instead. * */ - getOperationId(opts?: { - /** - * Generate a JS method-friendly operation ID when one isn't present. - */ - camelCase: boolean; - }): string { + getOperationId( + opts: { + /** + * Generate a JS method-friendly operation ID when one isn't present. + * + * For backwards compatiblity reasons this option will be indefinitely supported however we + * recommend using `friendlyCase` instead as it's a heavily improved version of this option. + * + * @see {opts.friendlyCase} + * @deprecated + */ + camelCase?: boolean; + + /** + * Generate a human-friendly, but still camelCase, operation ID when one isn't present. The + * difference between this and `camelCase` is that this also ensure that consecutive words are + * not present in the resulting ID. For example, for the endpoint `/candidate/{candidate}` will + * return `getCandidateCandidate` for `camelCase` however `friendlyCase` will return + * `getCandidate`. + * + * The reason this friendliness is just not a part of the `camelCase` option is because we have + * a number of consumers of the old operation ID style and making that change there would a + * breaking change that we don't have any easy way to resolve. + */ + friendlyCase?: boolean; + } = {}, + ): string { function sanitize(id: string) { // We aren't sanitizing underscores here by default in order to preserve operation IDs that // were already generated with this method in the past. return id - .replace(opts?.camelCase ? /[^a-zA-Z0-9_]/g : /[^a-zA-Z0-9]/g, '-') // Remove weird characters + .replace(opts?.camelCase || opts?.friendlyCase ? /[^a-zA-Z0-9_]/g : /[^a-zA-Z0-9]/g, '-') // Remove weird characters .replace(/--+/g, '-') // Remove double --'s .replace(/^-|-$/g, ''); // Don't start or end with - } @@ -373,7 +394,26 @@ export class Operation { } const method = this.method.toLowerCase(); - if (opts?.camelCase) { + if (opts?.camelCase || opts?.friendlyCase) { + if (opts?.friendlyCase) { + // In order to generate friendlier operation IDs we should swap out underscores with spaces + // so the end result will be _slightly_ more camelCase. + operationId = operationId.replaceAll('_', ' '); + + if (!this.hasOperationId()) { + // In another effort to generate friendly operation IDs we should prevent words from + // appearing in consecutive order (eg. `/candidate/{candidate}` should generate + // `getCandidate` not `getCandidateCandidate`). However we only want to do this if we're + // generating the operation ID as if they intentionally added a consecutive word into the + // operation ID then we should respect that. + operationId = operationId + .replace(/[^a-zA-Z0-9_]+(.)/g, (_, chr) => ` ${chr}`) + .split(' ') + .filter((word, i, arr) => word !== arr[i - 1]) + .join(' '); + } + } + operationId = operationId.replace(/[^a-zA-Z0-9_]+(.)/g, (_, chr) => chr.toUpperCase()); if (this.hasOperationId()) { operationId = sanitize(operationId); @@ -398,7 +438,7 @@ export class Operation { } // Because we're merging the `operationId` into an HTTP method we need to reset the first - // character of it back to lowercase so end up with `getBuster`, not `getbuster`. + // character of it back to lowercase so we end up with `getBuster`, not `getbuster`. operationId = operationId.charAt(0).toUpperCase() + operationId.slice(1); return `${method}${operationId}`; } else if (this.hasOperationId()) { diff --git a/packages/oas/test/__fixtures__/create-oas.ts b/packages/oas/test/__fixtures__/create-oas.ts index 6b3bce85..91774153 100644 --- a/packages/oas/test/__fixtures__/create-oas.ts +++ b/packages/oas/test/__fixtures__/create-oas.ts @@ -6,7 +6,7 @@ import Oas from '../../src/index.js'; * @param operation Operation to create a fake API definition and Oas instance for. * @param components Schema components to add into the fake API definition. */ -export default function createOas(operation: RMOAS.OperationObject, components?: RMOAS.ComponentsObject): Oas { +export function createOasForOperation(operation: RMOAS.OperationObject, components?: RMOAS.ComponentsObject): Oas { const schema = { openapi: '3.0.3', info: { title: 'testing', version: '1.0.0' }, @@ -23,3 +23,23 @@ export default function createOas(operation: RMOAS.OperationObject, components?: return new Oas(schema); } + +/** + * @param paths Path objects to create a fake API definition and Oas instance for. + * @param components Schema components to add into the fake API definition. + */ +export function createOasForPaths(paths: RMOAS.PathsObject, components?: RMOAS.ComponentsObject): Oas { + const schema = { + openapi: '3.0.3', + info: { title: 'testing', version: '1.0.0' }, + paths: { + ...paths, + }, + } as RMOAS.OASDocument; + + if (components) { + schema.components = components; + } + + return new Oas(schema); +} diff --git a/packages/oas/test/lib/openapi-to-json-schema.test.ts b/packages/oas/test/lib/openapi-to-json-schema.test.ts index 1ce39387..6427ddf2 100644 --- a/packages/oas/test/lib/openapi-to-json-schema.test.ts +++ b/packages/oas/test/lib/openapi-to-json-schema.test.ts @@ -5,7 +5,7 @@ import { beforeAll, expect, describe, it } from 'vitest'; import Oas from '../../src/index.js'; import { toJSONSchema } from '../../src/lib/openapi-to-json-schema.js'; -import createOas from '../__fixtures__/create-oas.js'; +import { createOasForOperation } from '../__fixtures__/create-oas.js'; import generateJSONSchemaFixture from '../__fixtures__/json-schema.js'; let petstore: Oas; @@ -857,7 +857,7 @@ describe('`description` support', () => { }); it('should add defaults for enums if default is present', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { content: { 'application/json': { diff --git a/packages/oas/test/operation/index.test.ts b/packages/oas/test/operation/index.test.ts index 8552cbf7..71fe51d1 100644 --- a/packages/oas/test/operation/index.test.ts +++ b/packages/oas/test/operation/index.test.ts @@ -6,6 +6,7 @@ import { beforeAll, describe, it, expect } from 'vitest'; import Oas from '../../src/index.js'; import { Operation, Callback } from '../../src/operation/index.js'; +import { createOasForPaths } from '../__fixtures__/create-oas.js'; let petstore: Oas; let callbackSchema: Oas; @@ -1051,127 +1052,184 @@ describe('#getOperationId()', () => { expect(operation.getOperationId()).toBe('post_ac-eq-hazard-18-0'); }); - describe('`camelCase` option', () => { - it('should create a camel cased operationId if one does not exist', () => { - const operation = multipleSecurities.operation('/multiple-combo-auths-duped', 'get'); - expect(operation.getOperationId({ camelCase: true })).toBe('getMultipleComboAuthsDuped'); + describe('options', () => { + describe('given a path without an operationId', () => { + it.each([ + ['camelCase', 'getMultipleComboAuthsDuped'], + ['friendlyCase', 'getMultipleComboAuthsDuped'], + ])('%s', (option, expected) => { + const operation = multipleSecurities.operation('/multiple-combo-auths-duped', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); }); - it('should sanitize underscores in a path', () => { - const spec = Oas.init({ - openapi: '3.1.0', - info: { - title: 'testing', - version: '1.0.0', - }, - paths: { + describe('given a path with underscores', () => { + it.each([ + ['camelCase', 'postAc_eq_hazard180'], + ['friendlyCase', 'postAcEqHazard180'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ '/ac_eq_hazard/18.0': { - post: {}, + post: { + responses: {}, + }, }, - }, - }); + }); - const operation = spec.operation('/ac_eq_hazard/18.0', 'post'); - expect(operation.getOperationId({ camelCase: true })).toBe('postAc_eq_hazard180'); + const operation = spec.operation('/ac_eq_hazard/18.0', 'post'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); }); - it('should not double up on a method prefix if the path starts with the method', () => { - const spec = Oas.init({ - openapi: '3.1.0', - info: { - title: 'testing', - version: '1.0.0', - }, - paths: { + describe('given a path that begins with a prefix matching the http method its associated with', () => { + it.each([ + ['camelCase', 'getPets'], + ['friendlyCase', 'getPets'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ '/get-pets': { get: { - tags: ['dogs'], + responses: {}, }, }, - }, - }); + }); - const operation = spec.operation('/get-pets', 'get'); - expect(operation.getOperationId({ camelCase: true })).toBe('getPets'); + const operation = spec.operation('/get-pets', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); }); - it('should not create an operationId that ends in a hyphen', () => { - const spec = Oas.init({ - openapi: '3.1.0', - info: { - title: 'testing', - version: '1.0.0', - }, - paths: { + describe('given a path that begins with double forward slashes', () => { + it.each([ + ['camelCase', 'getCandidateCandidate_id'], + ['friendlyCase', 'getCandidateId'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ '//candidate/{candidate_id}/': { - get: {}, + get: { + responses: {}, + }, }, - }, + }); + + const operation = spec.operation('/candidate/{candidate_id}/', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); }); + }); + + describe('given an operationId that we already consider to be good enough', () => { + it.each([ + ['camelCase', 'exchangeRESTAPI_GetAccounts'], + ['friendlyCase', 'exchangeRESTAPIGetAccounts'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ + '/anything': { + get: { + // This operationID is already fine to use as a JS method accessor we're just slightly + // modifying it so it fits as a method accessor. + operationId: 'ExchangeRESTAPI_GetAccounts', + responses: {}, + }, + }, + }); - const operation = spec.operation('/candidate/{candidate_id}/', 'get'); - expect(operation.getOperationId({ camelCase: true })).toBe('getCandidateCandidate_id'); + const operation = spec.operation('/anything', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); }); - it.each([ - [ - "should slightly tweak an operationId that's already good to use", - { - // This operationID is already fine to use as a JS method accessor we're just slightly - // modifying it so it fits as a method accessor. - operationId: 'ExchangeRESTAPI_GetAccounts', - expected: 'exchangeRESTAPI_GetAccounts', - }, - ], - [ - 'should clean up an operationId that has non-alphanumeric characters', - { - // This mess of a string is intentionally nasty so we can be sure that we're not - // including anything that wouldn't look right as an operationID for a potential method - // accessor in `api`. - operationId: 'find/?*!@#$%^&*()-=.,<>+[]{}\\|pets-by-status', - expected: 'findPetsByStatus', - }, - ], - [ - 'should clean up an operationId that ends in non-alpha characters', - { - operationId: 'find pets by status (deprecated?*!@#$%^&*()-=.,<>+[]{})', - expected: 'findPetsByStatusDeprecated', - }, - ], - [ - 'should clean up an operationId that starts with in non-alpha characters', - { - operationId: '(?*!@#$%^&*()-=.,<>+[]{}) find pets by status', - expected: 'findPetsByStatus', - }, - ], - [ - 'should clean up an operationId that starts with a number', - { - operationId: '400oD_Browse_by_Date_Feed', - expected: '_400oD_Browse_by_Date_Feed', - }, - ], - ])('%s', (_, { operationId, expected }) => { - const spec = Oas.init({ - openapi: '3.1.0', - info: { - title: 'testing', - version: '1.0.0', - }, - paths: { + describe('given an operationId that contains non-alphanumeric characters', () => { + it.each([ + ['camelCase', 'findPetsByStatus'], + ['friendlyCase', 'findPetsByStatus'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ '/anything': { get: { - operationId, + // This mess of a string is intentionally nasty so we can be sure that we're not + // including anything that wouldn't look right as an operationID for a potential method + // accessor in `api`. + operationId: 'find/?*!@#$%^&*()-=.,<>+[]{}\\|pets-by-status', + responses: {}, }, }, - }, + }); + + const operation = spec.operation('/anything', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); + }); + + describe('given an operationId that ends in non-alphanumeric characters', () => { + it.each([ + ['camelCase', 'findPetsByStatusDeprecated'], + ['friendlyCase', 'findPetsByStatusDeprecated'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ + '/anything': { + get: { + operationId: 'find pets by status (deprecated?*!@#$%^&*()-=.,<>+[]{})', + responses: {}, + }, + }, + }); + + const operation = spec.operation('/anything', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); + }); + + describe('given an operationId that begins with non-alphanumeric characters', () => { + it.each([ + ['camelCase', 'findPetsByStatus'], + ['friendlyCase', 'findPetsByStatus'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ + '/anything': { + get: { + operationId: '(?*!@#$%^&*()-=.,<>+[]{}) find pets by status', + responses: {}, + }, + }, + }); + + const operation = spec.operation('/anything', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); }); + }); - const operation = spec.operation('/anything', 'get'); - expect(operation.getOperationId({ camelCase: true })).toBe(expected); + describe('given an operationId that begins with a number', () => { + it.each([ + ['camelCase', '_400oD_Browse_by_Date_Feed'], + ['friendlyCase', '_400oDBrowseByDateFeed'], + ])('%s', (option, expected) => { + const spec = createOasForPaths({ + '/anything': { + get: { + operationId: '400oD_Browse_by_Date_Feed', + responses: {}, + }, + }, + }); + + const operation = spec.operation('/anything', 'get'); + expect(operation.getOperationId({ [option]: true })).toBe(expected); + }); + }); + + describe('`friendlyCase`', () => { + it('should not create an operationId that includes the same word in a consecutive sequence', () => { + const spec = createOasForPaths({ + '/pet/{pet}/adoption': { + post: { + responses: {}, + }, + }, + }); + + const operation = spec.operation('/pet/{pet}/adoption', 'post'); + expect(operation.getOperationId({ friendlyCase: true })).toBe('postPetAdoption'); + }); }); }); }); diff --git a/packages/oas/test/operation/lib/get-parameters-as-json-schema.test.ts b/packages/oas/test/operation/lib/get-parameters-as-json-schema.test.ts index d8f07f16..5c85259f 100644 --- a/packages/oas/test/operation/lib/get-parameters-as-json-schema.test.ts +++ b/packages/oas/test/operation/lib/get-parameters-as-json-schema.test.ts @@ -4,7 +4,7 @@ import { beforeAll, beforeEach, test, expect, it, describe } from 'vitest'; import { PARAMETER_ORDERING } from '../../../src/extensions.js'; import Oas from '../../../src/index.js'; -import createOas from '../../__fixtures__/create-oas.js'; +import { createOasForOperation } from '../../__fixtures__/create-oas.js'; let ably: Oas; let circular: Oas; @@ -52,8 +52,8 @@ beforeAll(async () => { }); test('it should return with null if there are no parameters', () => { - expect(createOas({ parameters: [] }).operation('/', 'get').getParametersAsJSONSchema()).toBeNull(); - expect(createOas({}).operation('/', 'get').getParametersAsJSONSchema()).toBeNull(); + expect(createOasForOperation({ parameters: [] }).operation('/', 'get').getParametersAsJSONSchema()).toBeNull(); + expect(createOasForOperation({}).operation('/', 'get').getParametersAsJSONSchema()).toBeNull(); }); describe('type sorting', () => { @@ -84,7 +84,7 @@ describe('type sorting', () => { }, }; - const oas = createOas(operation); + const oas = createOasForOperation(operation); const jsonschema = oas.operation('/', 'get').getParametersAsJSONSchema(); expect(jsonschema).toMatchSnapshot(); @@ -105,7 +105,7 @@ describe('type sorting', () => { }, }; - const oas = createOas(operation); + const oas = createOasForOperation(operation); const jsonschema = oas.operation('/', 'get').getParametersAsJSONSchema(); expect(jsonschema).toMatchSnapshot(); @@ -122,7 +122,7 @@ describe('type sorting', () => { [PARAMETER_ORDERING]: ['path', 'header', 'cookie', 'query', 'body', 'form'], }; - const oas = createOas(custom); + const oas = createOasForOperation(custom); const jsonschema = oas.operation('/', 'get').getParametersAsJSONSchema(); expect( @@ -162,7 +162,7 @@ describe('parameters', () => { describe('`content` support', () => { it('should support `content` on parameters', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { name: 'userId', @@ -182,7 +182,7 @@ describe('parameters', () => { }); it('should prioritize `application/json` if present', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { name: 'userId', @@ -203,7 +203,7 @@ describe('parameters', () => { }); it("should prioritize JSON-like content types if they're present", () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { name: 'userId', @@ -228,7 +228,7 @@ describe('parameters', () => { }); it('should use the first content type if `application/json` is not present', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { name: 'userId', @@ -282,7 +282,7 @@ describe('request bodies', () => { }); it('should not return anything for an empty schema', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { description: 'Body description', content: { @@ -297,7 +297,7 @@ describe('request bodies', () => { }); it('should not return anything for a requestBody that has no schema', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { description: 'Body description', content: { @@ -395,7 +395,7 @@ describe('type', () => { describe('request bodies', () => { describe('repair invalid schema that has no `type`', () => { it('should add a missing `type: object` on a schema that is clearly an object', () => { - const oas = createOas( + const oas = createOasForOperation( { requestBody: { content: { @@ -457,7 +457,7 @@ describe('descriptions', () => { it.todo('should pass through description on requestBody'); it('should pass through description on parameters', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { in: 'header', @@ -490,7 +490,7 @@ describe('descriptions', () => { }); it('should pass through description on parameter when referenced as a `$ref` and a `requestBody` is present', async () => { - const oas = createOas( + const oas = createOasForOperation( { parameters: [ { @@ -574,7 +574,7 @@ describe('`example` / `examples` support', () => { }; } - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { in: 'query', @@ -625,7 +625,7 @@ describe('`example` / `examples` support', () => { describe('deprecated', () => { describe('parameters', () => { it('should pass through deprecated on parameters', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { in: 'header', @@ -667,7 +667,7 @@ describe('deprecated', () => { }); it('should pass through deprecated on parameter when referenced as a `$ref` and a `requestBody` is present', async () => { - const oas = createOas( + const oas = createOasForOperation( { parameters: [ { @@ -743,7 +743,7 @@ describe('deprecated', () => { describe('request bodies', () => { it('should pass through deprecated on a request body schema property', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { content: { 'application/json': { @@ -801,7 +801,7 @@ describe('deprecated', () => { describe('polymorphism', () => { it('should pass through deprecated on a (merged) allOf schema', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { content: { 'application/json': { @@ -839,7 +839,7 @@ describe('deprecated', () => { }); it('should be able to merge enums within an allOf schema', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { content: { 'application/json': { @@ -938,7 +938,7 @@ describe('options', () => { describe('retainDeprecatedProperties (default behavior)', () => { it('should support merging `deprecatedProps` together', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { in: 'header', @@ -959,7 +959,7 @@ describe('options', () => { describe('retainDeprecatedProperties', () => { it('should retain deprecated properties within their original schemas', () => { - const oas = createOas({ + const oas = createOasForOperation({ parameters: [ { in: 'header', @@ -987,7 +987,7 @@ describe('options', () => { describe('transformer', () => { it('should be able transform part of a schema', () => { - const oas = createOas({ + const oas = createOasForOperation({ requestBody: { content: { 'application/json': { diff --git a/packages/oas/test/operation/lib/get-response-as-json-schema.test.ts b/packages/oas/test/operation/lib/get-response-as-json-schema.test.ts index fc37dfc1..d71a2611 100644 --- a/packages/oas/test/operation/lib/get-response-as-json-schema.test.ts +++ b/packages/oas/test/operation/lib/get-response-as-json-schema.test.ts @@ -5,7 +5,7 @@ import { beforeAll, describe, test, expect, it } from 'vitest'; import Oas from '../../../src/index.js'; import cloneObject from '../../../src/lib/clone-object.js'; -import createOas from '../../__fixtures__/create-oas.js'; +import { createOasForOperation } from '../../__fixtures__/create-oas.js'; let circular: Oas; let petstore: Oas; @@ -22,11 +22,11 @@ beforeAll(async () => { }); test('it should return with null if there is not a response', () => { - expect(createOas({ responses: {} }).operation('/', 'get').getResponseAsJSONSchema('200')).toBeNull(); + expect(createOasForOperation({ responses: {} }).operation('/', 'get').getResponseAsJSONSchema('200')).toBeNull(); }); test('it should return with null if there is empty content', () => { - const oas = createOas({ + const oas = createOasForOperation({ responses: { 200: { description: 'OK', @@ -99,7 +99,7 @@ describe('content type handling', () => { it("should return JSON Schema for a content type that isn't JSON-compatible", () => { expect( - createOas({ + createOasForOperation({ responses: { 200: { description: 'response level description', @@ -176,7 +176,7 @@ describe('`enum` handling', () => { describe('`headers` support', () => { // https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#responseObject it('should include headers if they exist', () => { - const oas = createOas({ + const oas = createOasForOperation({ responses: { 200: { description: 'response level description',