[OAS] Beautify generated operation ids (#198132)

packages/kbn-router-to-openapispec/src/generate_oas.test.fixture.ts

@@ -35,7 +35,7 @@ export const sharedOas = {
       get: {
         deprecated: true,
         'x-discontinued': 'route discontinued version or date',
-        operationId: '%2Fbar#0',
+        operationId: 'get-bar',
         parameters: [
           {
             description: 'The version of the API to use',
@@ -154,7 +154,7 @@ export const sharedOas = {
     '/foo/{id}/{path*}': {
       get: {
         description: 'route description',
-        operationId: '%2Ffoo%2F%7Bid%7D%2F%7Bpath*%7D#0',
+        operationId: 'get-foo-id-path',
         parameters: [
           {
             description: 'The version of the API to use',
@@ -278,7 +278,7 @@ export const sharedOas = {
       },
       post: {
         description: 'route description',
-        operationId: '%2Ffoo%2F%7Bid%7D%2F%7Bpath*%7D#1',
+        operationId: 'post-foo-id-path',
         parameters: [
           {
             description: 'The version of the API to use',

packages/kbn-router-to-openapispec/src/generate_oas.ts

@@ -10,10 +10,9 @@
 import type { CoreVersionedRouter, Router } from '@kbn/core-http-router-server-internal';
 import type { OpenAPIV3 } from 'openapi-types';
 import { OasConverter } from './oas_converter';
-import { createOperationIdCounter } from './operation_id_counter';
 import { processRouter } from './process_router';
 import { processVersionedRouter } from './process_versioned_router';
-import { buildGlobalTags } from './util';
+import { buildGlobalTags, createOpIdGenerator } from './util';
 
 export const openApiVersion = '3.0.0';
 
@@ -40,8 +39,8 @@ export const generateOpenApiDocument = (
 ): OpenAPIV3.Document => {
   const { filters } = opts;
   const converter = new OasConverter();
-  const getOpId = createOperationIdCounter();
   const paths: OpenAPIV3.PathsObject = {};
+  const getOpId = createOpIdGenerator();
   for (const router of appRouters.routers) {
     const result = processRouter(router, converter, getOpId, filters);
     Object.assign(paths, result.paths);

packages/kbn-router-to-openapispec/src/process_router.test.ts

@@ -10,9 +10,9 @@
 import { schema } from '@kbn/config-schema';
 import { Router } from '@kbn/core-http-router-server-internal';
 import { OasConverter } from './oas_converter';
-import { createOperationIdCounter } from './operation_id_counter';
 import { extractResponses, processRouter } from './process_router';
 import { type InternalRouterRoute } from './type';
+import { createOpIdGenerator } from './util';
 
 describe('extractResponses', () => {
   let oasConverter: OasConverter;
@@ -86,18 +86,21 @@ describe('processRouter', () => {
   const testRouter = {
     getRoutes: () => [
       {
+        method: 'get',
         path: '/foo',
         options: { access: 'internal', deprecated: true, discontinued: 'discontinued router' },
         handler: jest.fn(),
         validationSchemas: { request: { body: schema.object({}) } },
       },
       {
+        method: 'get',
         path: '/bar',
         options: {},
         handler: jest.fn(),
         validationSchemas: { request: { body: schema.object({}) } },
       },
       {
+        method: 'get',
         path: '/baz',
         options: {},
         handler: jest.fn(),
@@ -125,20 +128,20 @@ describe('processRouter', () => {
   } as unknown as Router;
 
   it('only provides routes for version 2023-10-31', () => {
-    const result1 = processRouter(testRouter, new OasConverter(), createOperationIdCounter(), {
+    const result1 = processRouter(testRouter, new OasConverter(), createOpIdGenerator(), {
       version: '2023-10-31',
     });
 
     expect(Object.keys(result1.paths!)).toHaveLength(4);
 
-    const result2 = processRouter(testRouter, new OasConverter(), createOperationIdCounter(), {
+    const result2 = processRouter(testRouter, new OasConverter(), createOpIdGenerator(), {
       version: '2024-10-31',
     });
     expect(Object.keys(result2.paths!)).toHaveLength(0);
   });
 
   it('updates description with privileges required', () => {
-    const result = processRouter(testRouter, new OasConverter(), createOperationIdCounter(), {
+    const result = processRouter(testRouter, new OasConverter(), createOpIdGenerator(), {
       version: '2023-10-31',
     });
 

packages/kbn-router-to-openapispec/src/process_router.ts

@@ -24,16 +24,16 @@ import {
   mergeResponseContent,
   prepareRoutes,
   setXState,
+  GetOpId,
 } from './util';
-import type { OperationIdCounter } from './operation_id_counter';
 import type { GenerateOpenApiDocumentOptionsFilters } from './generate_oas';
 import type { CustomOperationObject, InternalRouterRoute } from './type';
 import { extractAuthzDescription } from './extract_authz_description';
 
 export const processRouter = (
   appRouter: Router,
   converter: OasConverter,
-  getOpId: OperationIdCounter,
+  getOpId: GetOpId,
   filters?: GenerateOpenApiDocumentOptionsFilters
 ) => {
   const paths: OpenAPIV3.PathsObject = {};
@@ -91,7 +91,7 @@ export const processRouter = (
           : undefined,
         responses: extractResponses(route, converter),
         parameters,
-        operationId: getOpId(route.path),
+        operationId: getOpId({ path: route.path, method: route.method }),
       };
 
       setXState(route.options.availability, operation);

packages/kbn-router-to-openapispec/src/process_versioned_router.test.ts

@@ -11,13 +11,13 @@ import { schema } from '@kbn/config-schema';
 import type { CoreVersionedRouter } from '@kbn/core-http-router-server-internal';
 import { get } from 'lodash';
 import { OasConverter } from './oas_converter';
-import { createOperationIdCounter } from './operation_id_counter';
 import {
   processVersionedRouter,
   extractVersionedResponses,
   extractVersionedRequestBodies,
 } from './process_versioned_router';
 import { VersionedRouterRoute } from '@kbn/core-http-server';
+import { createOpIdGenerator } from './util';
 
 let oasConverter: OasConverter;
 beforeEach(() => {
@@ -125,7 +125,7 @@ describe('processVersionedRouter', () => {
     const baseCase = processVersionedRouter(
       { getRoutes: () => [createTestRoute()] } as unknown as CoreVersionedRouter,
       new OasConverter(),
-      createOperationIdCounter(),
+      createOpIdGenerator(),
       {}
     );
 
@@ -137,7 +137,7 @@ describe('processVersionedRouter', () => {
     const filteredCase = processVersionedRouter(
       { getRoutes: () => [createTestRoute()] } as unknown as CoreVersionedRouter,
       new OasConverter(),
-      createOperationIdCounter(),
+      createOpIdGenerator(),
       { version: '2023-10-31' }
     );
     expect(Object.keys(get(filteredCase, 'paths["/foo"].get.responses.200.content')!)).toEqual([
@@ -149,7 +149,7 @@ describe('processVersionedRouter', () => {
     const results = processVersionedRouter(
       { getRoutes: () => [createTestRoute()] } as unknown as CoreVersionedRouter,
       new OasConverter(),
-      createOperationIdCounter(),
+      createOpIdGenerator(),
       {}
     );
     expect(results.paths['/foo']).toBeDefined();

packages/kbn-router-to-openapispec/src/process_versioned_router.ts

@@ -18,7 +18,6 @@ import { extractAuthzDescription } from './extract_authz_description';
 import type { GenerateOpenApiDocumentOptionsFilters } from './generate_oas';
 import type { OasConverter } from './oas_converter';
 import { isReferenceObject } from './oas_converter/common';
-import type { OperationIdCounter } from './operation_id_counter';
 import {
   prepareRoutes,
   getPathParameters,
@@ -30,12 +29,13 @@ import {
   mergeResponseContent,
   getXsrfHeaderForMethod,
   setXState,
+  GetOpId,
 } from './util';
 
 export const processVersionedRouter = (
   appRouter: CoreVersionedRouter,
   converter: OasConverter,
-  getOpId: OperationIdCounter,
+  getOpId: GetOpId,
   filters?: GenerateOpenApiDocumentOptionsFilters
 ) => {
   const routes = prepareRoutes(appRouter.getRoutes(), filters);
@@ -121,7 +121,7 @@ export const processVersionedRouter = (
           ? extractVersionedResponse(handler, converter, contentType)
           : extractVersionedResponses(route, converter, contentType),
         parameters,
-        operationId: getOpId(route.path),
+        operationId: getOpId({ path: route.path, method: route.method }),
       };
 
       setXState(route.options.options?.availability, operation);