Skip to content

Commit

Permalink
fix determining optional path params logic
Browse files Browse the repository at this point in the history
  • Loading branch information
jloleysens committed Jan 11, 2024
1 parent b57f994 commit 281d946
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 27 deletions.
22 changes: 13 additions & 9 deletions packages/kbn-generate-oas/src/generate_oas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,8 @@ const prepareRoutes = <R extends { path: string; options: { access?: 'public' |
routes
// TODO: Make this smarter?
.filter(pathStartsWith ? (route) => route.path.startsWith(pathStartsWith) : () => true)
// TODO: Figure out how we can scope which routes we generate OAS for
// .filter((route) => route.options.access === 'public')
.map((route) => ({
...route,
path: route.path.replace('?', ''),
}))
// TODO: Figure out how we can scope which routes we generate OAS for
// .filter((route) => route.options.access === 'public')
);
};

Expand Down Expand Up @@ -186,7 +182,7 @@ const processVersionedRouter = (
},
};

paths[route.path] = { ...paths[route.path], ...path };
assignToPathsObject(paths, route.path, path);
} catch (e) {
// Enrich the error message with a bit more context
e.message = `Error generating OpenAPI for route '${route.path}' using version '${version}': ${e.message}`;
Expand Down Expand Up @@ -259,8 +255,7 @@ const processRouter = (appRouter: Router, pathStartsWith?: string): OpenAPIV3.Pa
operationId: getOperationId(route.path),
},
};

paths[route.path] = { ...paths[route.path], ...path };
assignToPathsObject(paths, route.path, path);
} catch (e) {
// Enrich the error message with a bit more context
e.message = `Error generating OpenAPI for route '${route.path}': ${e.message}`;
Expand All @@ -270,6 +265,15 @@ const processRouter = (appRouter: Router, pathStartsWith?: string): OpenAPIV3.Pa
return paths;
};

const assignToPathsObject = (
paths: OpenAPIV3.PathsObject,
path: string,
pathObject: OpenAPIV3.PathItemObject
): void => {
const pathName = path.replace('?', '');
paths[pathName] = { ...paths[pathName], ...pathObject };
};

const getOpenApiPathsObject = (
appRouter: Router | CoreVersionedRouter,
pathStartsWith?: string
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-generate-oas/src/oas_converters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import type { OpenAPIV3 } from 'openapi-types';
import { OpenAPIConverter } from '../type';
import { KnownParameters, OpenAPIConverter } from '../type';

import { zodConverter } from './zod';
import { kbnConfigSchemaConverter } from './kbn_config_schema/kbn_config_schema';
Expand All @@ -24,7 +24,7 @@ export const convert = (schema: unknown): OpenAPIV3.SchemaObject => {

export const convertPathParameters = (
schema: unknown,
pathParameters: string[]
pathParameters: KnownParameters
): OpenAPIV3.ParameterObject[] => {
return getConverter(schema).convertPathParameters(schema, pathParameters);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import joi from 'joi';
import { isConfigSchema, Type } from '@kbn/config-schema';
import { get } from 'lodash';
import type { OpenAPIV3 } from 'openapi-types';
import type { OpenAPIConverter } from '../../type';
import type { KnownParameters, OpenAPIConverter } from '../../type';
import { isReferenceObject } from '../common';
import * as mutations from './post_process_mutations';

Expand Down Expand Up @@ -115,6 +115,7 @@ const convert = (kbnConfigSchema: unknown): OpenAPIV3.BaseSchemaObject => {

const convertObjectMembersToParameterObjects = (
schema: joi.Schema,
knownParameters: KnownParameters = {},
isPathParameter = false
): OpenAPIV3.ParameterObject[] => {
let properties: Exclude<OpenAPIV3.SchemaObject['properties'], undefined>;
Expand All @@ -129,7 +130,6 @@ const convertObjectMembersToParameterObjects = (
throw createError(`Expected record, object or nullable object type, but got '${schema.type}'`);
}

const isRequired = isSchemaRequired(schema);
return Object.entries(properties).map(([schemaKey, schemaObject]) => {
const isSubSchemaRequired = isSchemaRequired(schemaObject);
if (isReferenceObject(schemaObject)) {
Expand All @@ -141,7 +141,7 @@ const convertObjectMembersToParameterObjects = (
return {
name: schemaKey,
in: isPathParameter ? 'path' : 'query',
required: isPathParameter || (isRequired && isSubSchemaRequired),
required: isPathParameter ? !knownParameters[schemaKey].optional : isSubSchemaRequired,
schema: openApiSchemaObject,
description,
};
Expand All @@ -150,12 +150,12 @@ const convertObjectMembersToParameterObjects = (

const convertQuery = (kbnConfigSchema: unknown): OpenAPIV3.ParameterObject[] => {
const schema = unwrapKbnConfigSchema(kbnConfigSchema);
return convertObjectMembersToParameterObjects(schema, false);
return convertObjectMembersToParameterObjects(schema, {}, false);
};

const convertPathParameters = (
kbnConfigSchema: unknown,
pathParameters: string[]
knownParameters: { [paramName: string]: { optional: boolean } }
): OpenAPIV3.ParameterObject[] => {
const schema = unwrapKbnConfigSchema(kbnConfigSchema);

Expand All @@ -170,7 +170,7 @@ const convertPathParameters = (
throw createError('Input parser for path params expected to be an object schema');
}

return convertObjectMembersToParameterObjects(schema, true);
return convertObjectMembersToParameterObjects(schema, knownParameters, true);
};

const is = (schema: unknown): boolean => {
Expand Down
15 changes: 9 additions & 6 deletions packages/kbn-generate-oas/src/oas_converters/zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { z } from '@kbn/zod';
import type { OpenAPIV3 } from 'openapi-types';
import zodToJsonSchema from 'zod-to-json-schema';
import { OpenAPIConverter } from '../type';
import { KnownParameters, OpenAPIConverter } from '../type';
import { validatePathParameters } from './common';

// Adapted from from https://github.com/jlalmes/trpc-openapi/blob/aea45441af785518df35c2bc173ae2ea6271e489/src/utils/zod.ts#L1
Expand Down Expand Up @@ -142,7 +142,8 @@ const instanceofZodTypeCoercible = (_type: z.ZodTypeAny): _type is ZodTypeCoerci
const convertObjectMembersToParameterObjects = (
shape: z.ZodRawShape,
isRequired: boolean,
isPathParameter = false
isPathParameter = false,
knownParameters: KnownParameters = {}
): OpenAPIV3.ParameterObject[] => {
return Object.entries(shape).map(([shapeKey, subShape]) => {
const isSubShapeRequired = !subShape.isOptional();
Expand All @@ -164,7 +165,7 @@ const convertObjectMembersToParameterObjects = (
return {
name: shapeKey,
in: isPathParameter ? 'path' : 'query',
required: isPathParameter || (isRequired && isSubShapeRequired),
required: isPathParameter ? !knownParameters[shapeKey]?.optional : isSubShapeRequired,
schema: openApiSchemaObject,
description,
};
Expand All @@ -184,19 +185,21 @@ const convertQuery = (schema: unknown): OpenAPIV3.ParameterObject[] => {

const convertPathParameters = (
schema: unknown,
pathParameters: string[]
knownParameters: KnownParameters
): OpenAPIV3.ParameterObject[] => {
assertInstanceOfZodType(schema);
const unwrappedSchema = unwrapZodType(schema, true);
if (pathParameters.length === 0 && instanceofZodTypeLikeVoid(unwrappedSchema)) {
const paramKeys = Object.keys(knownParameters);
const paramsCount = paramKeys.length;
if (paramsCount === 0 && instanceofZodTypeLikeVoid(unwrappedSchema)) {
return [];
}
if (!instanceofZodTypeObject(unwrappedSchema)) {
throw createError('Parameters schema must be an _object_ schema validator!');
}
const shape = unwrappedSchema.shape;
const schemaKeys = Object.keys(shape);
validatePathParameters(pathParameters, schemaKeys);
validatePathParameters(paramKeys, schemaKeys);
const isRequired = !schema.isOptional();
return convertObjectMembersToParameterObjects(shape, isRequired, true);
};
Expand Down
9 changes: 8 additions & 1 deletion packages/kbn-generate-oas/src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,15 @@

import type { OpenAPIV3 } from 'openapi-types';

export interface KnownParameters {
[paramName: string]: { optional: boolean };
}

export interface OpenAPIConverter {
convertPathParameters(schema: unknown, pathParameters: string[]): OpenAPIV3.ParameterObject[];
convertPathParameters(
schema: unknown,
knownPathParameters: KnownParameters
): OpenAPIV3.ParameterObject[];

convertQuery(schema: unknown): OpenAPIV3.ParameterObject[];

Expand Down
10 changes: 7 additions & 3 deletions packages/kbn-generate-oas/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@

import { VersionedRouterRoute } from '@kbn/core-http-router-server-internal/src/versioned_router/types';
import { RouterRoute, RouteValidatorConfig } from '@kbn/core-http-server';
import { KnownParameters } from './type';

// https://github.com/jlalmes/trpc-openapi/blob/aea45441af785518df35c2bc173ae2ea6271e489/src/utils/path.ts#L5
export const getPathParameters = (path: string): string[] => {
return Array.from(path.matchAll(/\{(.+?)\}/g)).map(([_, key]) => key!);
export const getPathParameters = (path: string): KnownParameters => {
return Array.from(path.matchAll(/\{(.+?)\}/g)).reduce<KnownParameters>((acc, [_, key]) => {
const optional = key.endsWith('?');
acc[optional ? key.slice(0, key.length - 1) : key] = { optional };
return acc;
}, {});
};

export const extractValidationSchemaFromVersionedHandler = (
Expand Down

0 comments on commit 281d946

Please sign in to comment.