Skip to content

Commit

Permalink
Merge pull request #2274 from nestjs/revert-1949-1711-operation-ids-w…
Browse files Browse the repository at this point in the history 
…ith-version

Revert "feat(swagger): Provide URI version to operationIdFactory"
  • Loading branch information
@kamilmysliwiec
kamilmysliwiec committed Feb 6, 2023
2 parents 2037c61 + 344d392 commit 311597e
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 193 deletions.
14 changes: 1 addition & 13 deletions lib/interfaces/swagger-document-options.interface.ts
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
export type OperationIdFactory =
| ((
controllerKey: string,
methodKey: string,
pathVersionKey?: string
) => string)
| ((
controllerKey: string,
methodKey: string,
pathVersionKey?: string
) => string);

export interface SwaggerDocumentOptions {
/**
* List of modules to include in the specification
Expand All @@ -36,5 +24,5 @@ export interface SwaggerDocumentOptions {
* based on the `controllerKey` and `methodKey`
* @default () => controllerKey_methodKey
*/
operationIdFactory?: OperationIdFactory;
operationIdFactory?: (controllerKey: string, methodKey: string) => string;
}
38 changes: 6 additions & 32 deletions lib/swagger-explorer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
exploreApiTagsMetadata,
exploreGlobalApiTagsMetadata
} from './explorers/api-use-tags.explorer';
import { OperationIdFactory } from './interfaces';
import { DenormalizedDocResolvers } from './interfaces/denormalized-doc-resolvers.interface';
import { DenormalizedDoc } from './interfaces/denormalized-doc.interface';
import {
Expand All @@ -69,10 +68,8 @@ export class SwaggerExplorer {
private readonly mimetypeContentWrapper = new MimetypeContentWrapper();
private readonly metadataScanner = new MetadataScanner();
private readonly schemas: Record<string, SchemaObject> = {};
private operationIdFactory: OperationIdFactory = (
controllerKey: string,
methodKey: string
) => (controllerKey ? `${controllerKey}_${methodKey}` : methodKey);
private operationIdFactory = (controllerKey: string, methodKey: string) =>
controllerKey ? `${controllerKey}_${methodKey}` : methodKey;
private routePathFactory?: RoutePathFactory;

constructor(private readonly schemaObjectFactory: SchemaObjectFactory) {}
Expand All @@ -82,7 +79,7 @@ export class SwaggerExplorer {
applicationConfig: ApplicationConfig,
modulePath?: string | undefined,
globalPrefix?: string | undefined,
operationIdFactory?: OperationIdFactory
operationIdFactory?: (controllerKey: string, methodKey: string) => string
) {
this.routePathFactory = new RoutePathFactory(applicationConfig);
if (operationIdFactory) {
Expand Down Expand Up @@ -278,23 +275,6 @@ export class SwaggerExplorer {
metatype,
applicationConfig.getVersioning()
);

const versionOrVersions = methodVersion ?? controllerVersion;
let versions = [];
if (!!versionOrVersions) {
if (!Array.isArray(versionOrVersions)) {
versions = [versionOrVersions];
} else {
versions = versionOrVersions as string[];
}

const versionConfig = applicationConfig.getVersioning();
if (versionConfig?.type == VersioningType.URI) {
const prefix = this.routePathFactory.getVersionPrefix(versionConfig);
versions = versions.map((v) => `${prefix}${v}`);
}
}

const allRoutePaths = this.routePathFactory.create(
{
methodPath,
Expand All @@ -313,25 +293,19 @@ export class SwaggerExplorer {
DECORATORS.API_EXTENSION,
method
);
const pathVersion = versions?.filter((v) => fullPath.includes(v))[0];
return {
method: RequestMethod[requestMethod].toLowerCase(),
path: fullPath === '' ? '/' : fullPath,
operationId: this.getOperationId(instance, method, pathVersion),
operationId: this.getOperationId(instance, method),
...apiExtension
};
});
}

private getOperationId(
instance: object,
method: Function,
pathVersion?: string
): string {
private getOperationId(instance: object, method: Function): string {
return this.operationIdFactory(
instance.constructor?.name || '',
method.name,
pathVersion
method.name
);
}

Expand Down
8 changes: 2 additions & 6 deletions lib/swagger-scanner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import { ApplicationConfig, NestContainer } from '@nestjs/core';
import { InstanceWrapper } from '@nestjs/core/injector/instance-wrapper';
import { InstanceToken, Module } from '@nestjs/core/injector/module';
import { flatten, isEmpty } from 'lodash';
import {
OpenAPIObject,
OperationIdFactory,
SwaggerDocumentOptions
} from './interfaces';
import { OpenAPIObject, SwaggerDocumentOptions } from './interfaces';
import { ModuleRoute } from './interfaces/module-route.interface';
import {
ReferenceObject,
Expand Down Expand Up @@ -110,7 +106,7 @@ export class SwaggerScanner {
modulePath: string | undefined,
globalPrefix: string | undefined,
applicationConfig: ApplicationConfig,
operationIdFactory?: OperationIdFactory
operationIdFactory?: (controllerKey: string, methodKey: string) => string
): ModuleRoute[] {
const denormalizedArray = [...routes.values()].map((ctrl) =>
this.explorer.exploreController(
Expand Down
191 changes: 49 additions & 142 deletions test/explorer/swagger-explorer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ describe('SwaggerExplorer', () => {
controllerKey: string,
methodKey: string
) => `${controllerKey}.${methodKey}`;
const controllerKeyMethodKeyVersionKeyOperationIdFactory = (
controllerKey: string,
methodKey: string,
versionKey: string
) => `${controllerKey}.${methodKey}.${versionKey}`;

describe('when module only uses metadata', () => {
class Foo {}
Expand Down Expand Up @@ -1306,148 +1301,60 @@ describe('SwaggerExplorer', () => {
defaultVersion: 'THIS_SHOULD_NOT_APPEAR_ANYWHERE'
});
});
describe('and using the default operationIdFactory', () => {
it('should use controller version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix'
);

expect(routes[0].root.path).toEqual(
`/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
);
expect(routes[0].root.operationId).toEqual(
`WithVersionController_foo`
);
});

it('should use route version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix'
);

expect(routes[1].root.path).toEqual(
`/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
);
expect(routes[1].root.operationId).toEqual(
`WithVersionController_bar`
);
});
it('should use controller version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix'
);

it('should use multiple versions defined', () => {
const routes = explorer.exploreController(
{
instance: new WithMultipleVersionsController(),
metatype: WithMultipleVersionsController
} as InstanceWrapper<WithMultipleVersionsController>,
config,
'modulePath',
'globalPrefix'
);

expect(routes[0].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[0] as string
}/modulePath/with-multiple-version`
);
expect(routes[0].root.operationId).toEqual(
`WithMultipleVersionsController_foo`
);
expect(routes[1].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[1] as string
}/modulePath/with-multiple-version`
);
expect(routes[1].root.operationId).toEqual(
`WithMultipleVersionsController_foo`
);
});
expect(routes[0].root.path).toEqual(
`/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
);
});
describe('and has an operationIdFactory that uses the method version', () => {
it('should use controller version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix',
controllerKeyMethodKeyVersionKeyOperationIdFactory
);

expect(routes[0].root.path).toEqual(
`/globalPrefix/v${CONTROLLER_VERSION}/modulePath/with-version`
);
expect(routes[0].root.operationId).toEqual(
`WithVersionController.foo.v${CONTROLLER_VERSION}`
);
});

it('should use route version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix',
controllerKeyMethodKeyVersionKeyOperationIdFactory
);

expect(routes[1].root.path).toEqual(
`/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
);
expect(routes[1].root.operationId).toEqual(
`WithVersionController.bar.v${METHOD_VERSION}`
);
});
it('should use route version defined', () => {
const routes = explorer.exploreController(
{
instance: new WithVersionController(),
metatype: WithVersionController
} as InstanceWrapper<WithVersionController>,
config,
'modulePath',
'globalPrefix'
);

it('should use multiple versions defined', () => {
const routes = explorer.exploreController(
{
instance: new WithMultipleVersionsController(),
metatype: WithMultipleVersionsController
} as InstanceWrapper<WithMultipleVersionsController>,
config,
'modulePath',
'globalPrefix',
controllerKeyMethodKeyVersionKeyOperationIdFactory
);

expect(routes[0].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[0] as string
}/modulePath/with-multiple-version`
);
expect(routes[0].root.operationId).toEqual(
`WithMultipleVersionsController.foo.v${
CONTROLLER_MULTIPLE_VERSIONS[0] as string
}`
);
expect(routes[1].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[1] as string
}/modulePath/with-multiple-version`
);
expect(routes[1].root.operationId).toEqual(
`WithMultipleVersionsController.foo.v${
CONTROLLER_MULTIPLE_VERSIONS[1] as string
}`
);
});
expect(routes[1].root.path).toEqual(
`/globalPrefix/v${METHOD_VERSION}/modulePath/with-version`
);
});

it('should use multiple versions defined', () => {
const routes = explorer.exploreController(
{
instance: new WithMultipleVersionsController(),
metatype: WithMultipleVersionsController
} as InstanceWrapper<WithMultipleVersionsController>,
config,
'modulePath',
'globalPrefix'
);

expect(routes[0].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[0] as string
}/modulePath/with-multiple-version`
);
expect(routes[1].root.path).toEqual(
`/globalPrefix/v${
CONTROLLER_MULTIPLE_VERSIONS[1] as string
}/modulePath/with-multiple-version`
);
});
});

Expand Down

0 comments on commit 311597e

Please sign in to comment.