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.

Sign up for GitHub

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

allow operationId be ignored when generating operation names #2043

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+150 −96
Open

allow operationId be ignored when generating operation names #2043

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cd10e9
allow operationId be ignored when generating operation names
askvortcov Feb 19, 2024
c812bd0
change --igrnoreOperationId to --useOperationId; make the flag consis…
askvortcov Feb 20, 2024
af4dfb7
propagate useVersionId to swagger v2 parser
askvortcov Feb 20, 2024
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
2 changes: 2 additions & 0 deletions bin/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const params = program
.option('--exportCore <value>', 'Write core files to disk', true)
.option('--exportServices <value>', 'Write services to disk', true)
.option('--exportModels <value>', 'Write models to disk', true)
.option('--ignoreOperationId <value>', 'Use operation id', true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.option('--ignoreOperationId <value>', 'Use operation id', true)
.option('--ignoreOperationId <value>', 'Ignore operation id', true)

Although I agree with the initial intent – make the flag opt-in and default to true to preserve current functionality

Suggested change
.option('--ignoreOperationId <value>', 'Use operation id', true)
.option('--useOperationId <value>', 'Use operation id', true)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the next commit. Thanks for the review.

.option('--exportSchemas <value>', 'Write schemas to disk', false)
.option('--indent <value>', 'Indentation options [4, 2, tabs]', '4')
.option('--postfixServices <value>', 'Service name postfix', 'Service')
Expand All @@ -41,6 +42,7 @@ if (OpenAPI) {
exportServices: JSON.parse(params.exportServices) === true,
exportModels: JSON.parse(params.exportModels) === true,
exportSchemas: JSON.parse(params.exportSchemas) === true,
ignoreOperationId: JSON.parse(params.ignoreOperationId) === true,
indent: params.indent,
postfixServices: params.postfixServices,
postfixModels: params.postfixModels,
Expand Down
5 changes: 4 additions & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export type Options = {
exportServices?: boolean;
exportModels?: boolean;
exportSchemas?: boolean;
ignoreOperationId?: boolean;
indent?: Indent;
postfixServices?: string;
postfixModels?: string;
Expand All @@ -44,6 +45,7 @@ export type Options = {
* @param exportServices Generate services
* @param exportModels Generate models
* @param exportSchemas Generate schemas
* @param ignoreOperationId Ignore operationId
* @param indent Indentation options (4, 2 or tab)
* @param postfixServices Service name postfix
* @param postfixModels Model name postfix
Expand All @@ -61,6 +63,7 @@ export const generate = async ({
exportServices = true,
exportModels = true,
exportSchemas = false,
ignoreOperationId = false,
indent = Indent.SPACE_4,
postfixServices = 'Service',
postfixModels = '',
Expand Down Expand Up @@ -101,7 +104,7 @@ export const generate = async ({
}

case OpenApiVersion.V3: {
const client = parseV3(openApi);
const client = parseV3(openApi, ignoreOperationId);
const clientFinal = postProcessClient(client);
if (!write) break;
await writeClient(
Expand Down
7 changes: 4 additions & 3 deletions src/openApi/v3/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import { getServiceVersion } from './parser/getServiceVersion';
/**
* Parse the OpenAPI specification to a Client model that contains
* all the models, services and schema's we should output.
* @param openApi The OpenAPI spec that we have loaded from disk.
* @param openApi The OpenAPI spec that we have loaded from disk.
* @param ignoreOperationId should the operationId be ignored when generating operation names
*/
export const parse = (openApi: OpenApi): Client => {
export const parse = (openApi: OpenApi, ignoreOperationId: boolean): Client => {
const version = getServiceVersion(openApi.info.version);
const server = getServer(openApi);
const models = getModels(openApi);
const services = getServices(openApi);
const services = getServices(openApi, ignoreOperationId);

return { version, server, models, services };
};
5 changes: 3 additions & 2 deletions src/openApi/v3/parser/getOperation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ export const getOperation = (
method: string,
tag: string,
op: OpenApiOperation,
pathParams: OperationParameters
pathParams: OperationParameters,
ignoreOperationId: boolean
): Operation => {
const serviceName = getServiceName(tag);
const operationName = getOperationName(url, method, op.operationId);
const operationName = getOperationName(url, method, ignoreOperationId, op.operationId);

// Create a new operation object for this method.
const operation: Operation = {
Expand Down
46 changes: 26 additions & 20 deletions src/openApi/v3/parser/getOperationName.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,32 @@ import { getOperationName } from './getOperationName';

describe('getOperationName', () => {
it('should produce correct result', () => {
expect(getOperationName('/api/v{api-version}/users', 'GET', 'GetAllUsers')).toEqual('getAllUsers');
expect(getOperationName('/api/v{api-version}/users', 'GET', undefined)).toEqual('getApiUsers');
expect(getOperationName('/api/v{api-version}/users', 'POST', undefined)).toEqual('postApiUsers');
expect(getOperationName('/api/v1/users', 'GET', 'GetAllUsers')).toEqual('getAllUsers');
expect(getOperationName('/api/v1/users', 'GET', undefined)).toEqual('getApiV1Users');
expect(getOperationName('/api/v1/users', 'POST', undefined)).toEqual('postApiV1Users');
expect(getOperationName('/api/v1/users/{id}', 'GET', undefined)).toEqual('getApiV1Users');
expect(getOperationName('/api/v1/users/{id}', 'POST', undefined)).toEqual('postApiV1Users');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'GetAllUsers')).toEqual('getAllUsers');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, undefined)).toEqual('getApiUsers');
expect(getOperationName('/api/v{api-version}/users', 'POST', false, undefined)).toEqual('postApiUsers');
expect(getOperationName('/api/v1/users', 'GET', false, 'GetAllUsers')).toEqual('getAllUsers');
expect(getOperationName('/api/v1/users', 'GET', false, undefined)).toEqual('getApiV1Users');
expect(getOperationName('/api/v1/users', 'POST', false, undefined)).toEqual('postApiV1Users');
expect(getOperationName('/api/v1/users/{id}', 'GET', false, undefined)).toEqual('getApiV1UsersById');
expect(getOperationName('/api/v1/users/{id}', 'POST', false, undefined)).toEqual('postApiV1UsersById');

expect(getOperationName('/api/v{api-version}/users', 'GET', 'fooBar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'FooBar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'Foo Bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'foo bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'foo-bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'foo_bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', 'foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', '@foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', '$foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', '_foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', '-foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', '123.foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'fooBar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'FooBar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'Foo Bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'foo bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'foo-bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'foo_bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, 'foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, '@foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, '$foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, '_foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, '-foo.bar')).toEqual('fooBar');
expect(getOperationName('/api/v{api-version}/users', 'GET', false, '123.foo.bar')).toEqual('fooBar');

expect(getOperationName('/api/v1/users', 'GET', true, 'GetAllUsers')).toEqual('getApiV1Users');
expect(getOperationName('/api/v{api-version}/users', 'GET', true, 'fooBar')).toEqual('getApiUsers');
expect(
getOperationName('/api/v{api-version}/users/{userId}/location/{locationId}', 'GET', true, 'fooBar')
).toEqual('getApiUsersByUserIdLocationByLocationId');
});
});
11 changes: 8 additions & 3 deletions src/openApi/v3/parser/getOperationName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ import camelCase from 'camelcase';
* This will use the operation ID - if available - and otherwise fallback
* on a generated name from the URL
*/
export const getOperationName = (url: string, method: string, operationId?: string): string => {
if (operationId) {
export const getOperationName = (
url: string,
method: string,
ignoreOperationId: boolean,
operationId?: string
): string => {
if (operationId && !ignoreOperationId) {
return camelCase(
operationId
.replace(/^[^a-zA-Z]+/g, '')
Expand All @@ -17,7 +22,7 @@ export const getOperationName = (url: string, method: string, operationId?: stri

const urlWithoutPlaceholders = url
.replace(/[^/]*?{api-version}.*?\//g, '')
.replace(/{(.*?)}/g, '')
.replace(/{(.*?)}/g, 'by-$1')
.replace(/\//g, '-');

return camelCase(`${method}-${urlWithoutPlaceholders}`);
Expand Down
38 changes: 21 additions & 17 deletions src/openApi/v3/parser/getServices.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,33 @@
/* eslint-disable */
import { getServices } from './getServices';

describe('getServices', () => {
it('should create a unnamed service if tags are empty', () => {
const services = getServices({
openapi: '3.0.0',
info: {
title: 'x',
version: '1',
},
paths: {
'/api/trips': {
get: {
tags: [],
responses: {
200: {
description: 'x',
},
default: {
description: 'default',
const services = getServices(
{
openapi: '3.0.0',
info: {
title: 'x',
version: '1',
},
paths: {
'/api/trips': {
get: {
tags: [],
responses: {
200: {
description: 'x',
},
default: {
description: 'default',
},
},
},
},
},
},
});
false,
);

expect(services).toHaveLength(1);
expect(services[0].name).toEqual('Default');
Expand Down
12 changes: 10 additions & 2 deletions src/openApi/v3/parser/getServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getOperationParameters } from './getOperationParameters';
/**
* Get the OpenAPI services
*/
export const getServices = (openApi: OpenApi): Service[] => {
export const getServices = (openApi: OpenApi, ignoreOperationId: boolean): Service[] => {
const services = new Map<string, Service>();
for (const url in openApi.paths) {
if (openApi.paths.hasOwnProperty(url)) {
Expand All @@ -30,7 +30,15 @@ export const getServices = (openApi: OpenApi): Service[] => {
const op = path[method]!;
const tags = op.tags?.length ? op.tags.filter(unique) : ['Default'];
tags.forEach(tag => {
const operation = getOperation(openApi, url, method, tag, op, pathParams);
const operation = getOperation(
openApi,
url,
method,
tag,
op,
pathParams,
ignoreOperationId
);

// If we have already declared a service, then we should fetch that and
// append the new method to it. Otherwise we should create a new service object.
Expand Down