From cbcd3bcb8fbff0c78d07d347d021d876c712d6fe Mon Sep 17 00:00:00 2001 From: Dmitry Kokovtsev Date: Wed, 15 Apr 2020 10:45:29 +0300 Subject: [PATCH 1/2] fix: query string is incorrectly serialized for primitive parameters (#103) --- .../2.0/serializers/operation-object.ts | 12 ++-- .../typescript/3.0/bundled/openapi-3-utils.ts | 8 +-- .../__tests__/operation-object.spec.ts | 71 +++++++++++++++++++ .../3.0/serializers/operation-object.ts | 8 +-- .../3.0/serializers/parameter-object.ts | 20 ++++-- .../common/data/serialized-fragment.ts | 14 +++- 6 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts diff --git a/src/language/typescript/2.0/serializers/operation-object.ts b/src/language/typescript/2.0/serializers/operation-object.ts index fd8f208..4058396 100644 --- a/src/language/typescript/2.0/serializers/operation-object.ts +++ b/src/language/typescript/2.0/serializers/operation-object.ts @@ -411,8 +411,8 @@ export const serializeQueryParameterObject = ( case 'number': case 'boolean': { const f = serializedFragment( - `value => encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(value)`, - [], + `value => some(encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(value))`, + [serializedDependency('some', 'fp-ts/lib/Option')], [], ); return right(getSerializedOptionCallFragment(!required, f, encoded)); @@ -430,16 +430,16 @@ export const serializeQueryParameterObject = ( case 'pipes': { const s = getCollectionSeparator(collectionFormat); const f = serializedFragment( - `value => encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(value.join('${s}'))`, - [], + `value => some(encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(value.join('${s}')))`, + [serializedDependency('some', 'fp-ts/lib/Option')], [], ); return right(getSerializedOptionCallFragment(!required, f, encoded)); } case 'multi': { const f = serializedFragment( - `value => value.map(item => encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(item)).join('&')`, - [], + `value => some(value.map(item => encodeURIComponent('${parameter.name}') + '=' + encodeURIComponent(item)).join('&'))`, + [serializedDependency('some', 'fp-ts/lib/Option')], [], ); return right(getSerializedOptionCallFragment(!required, f, encoded)); diff --git a/src/language/typescript/3.0/bundled/openapi-3-utils.ts b/src/language/typescript/3.0/bundled/openapi-3-utils.ts index a7bb11b..a0d75c8 100644 --- a/src/language/typescript/3.0/bundled/openapi-3-utils.ts +++ b/src/language/typescript/3.0/bundled/openapi-3-utils.ts @@ -14,16 +14,16 @@ const content = ` export const serializePrimitiveParameter = (style: string, name: string, value: unknown): Either => { switch (style) { case 'matrix': { - return right(\`;\${name}=\${value}\`); + return right(\`;\${name}=\${encodeURIComponent(String(value))}\`); } case 'label': { - return right(\`.\${value}\`); + return right(\`.\${encodeURIComponent(String(value))}\`); } case 'form': { - return right(\`\${name}=\${value}\`); + return right(\`\${name}=\${encodeURIComponent(String(value))}\`); } case 'simple': { - return right(\`\${value}\`); + return right(\`\${encodeURIComponent(String(value))}\`); } } return left(new Error(\`Unsupported style "\${style}" for parameter "\${name}"\`)); diff --git a/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts b/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts new file mode 100644 index 0000000..f23d704 --- /dev/null +++ b/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts @@ -0,0 +1,71 @@ +import { getParameters as createGetParameters } from '../operation-object'; +import { constant } from 'fp-ts/lib/function'; +import { left } from 'fp-ts/lib/Either'; +import { fromString } from '../../../../../utils/ref'; +import { PathItemObjectCodec } from '../../../../../schema/3.0/path-item-object'; +import { pipe } from 'fp-ts/lib/pipeable'; +import { either, option } from 'fp-ts'; +import { OperationObjectCodec } from '../../../../../schema/3.0/operation-object'; +import { sequenceTEither } from '@devexperts/utils/dist/adt/either.utils'; + +describe('OperationObject', () => { + describe('getParameters', () => { + const getParameters = createGetParameters({ + resolveRef: constant(left(new Error('Refs not supported'))), + }); + + it('should correctly handle primitive query parameters', () => { + const operation = pipe( + OperationObjectCodec.decode({ + responses: {}, + parameters: [ + { + in: 'query', + name: 'offset', + required: false, + schema: { + type: 'number', + }, + }, + { + in: 'query', + name: 'limit', + required: true, + schema: { + type: 'number', + }, + }, + ], + }), + either.mapLeft(constant(new Error())), + ); + + const pathItem = pipe(PathItemObjectCodec.decode({}), either.mapLeft(constant(new Error()))); + + const result = pipe( + sequenceTEither(fromString('#/test'), operation, pathItem), + either.chain(([ref, operation, pathItem]) => getParameters(ref, operation, pathItem)), + ); + + const generated = pipe( + result, + option.fromEither, + option.chain(result => result.serializedQueryString), + option.fold(constant(''), fragment => fragment.value.replace(/\s+/g, ' ')), + ); + + expect(generated).toEqual( + `compact([pipe( + optionFromNullable(number).encode(parameters.query.offset), + option.fromNullable, + option.chain(value => option.fromEither(serializePrimitiveParameter('form', 'offset', value))), + ),pipe( + number.encode(parameters.query.limit), + value => option.fromEither(serializePrimitiveParameter('form', 'limit', value)), + )]).join('&')` + .trim() + .replace(/\s+/g, ' '), + ); + }); + }); +}); diff --git a/src/language/typescript/3.0/serializers/operation-object.ts b/src/language/typescript/3.0/serializers/operation-object.ts index 95e5ae5..3fe8c46 100644 --- a/src/language/typescript/3.0/serializers/operation-object.ts +++ b/src/language/typescript/3.0/serializers/operation-object.ts @@ -12,7 +12,7 @@ import { getParameterObjectSchema, isRequired, serializeParameterObject, - serializeParameterToTemplate, + serializeQueryParameterToTemplate, } from './parameter-object'; import { pipe } from 'fp-ts/lib/pipeable'; import { getSerializedKindDependency, serializedDependency } from '../../common/data/serialized-dependency'; @@ -70,7 +70,7 @@ const eqParameterByNameAndIn: Eq = getStructEq({ }); const contains = array.elem(eqParameterByNameAndIn); -const getParameters = combineReader( +export const getParameters = combineReader( ask(), e => (from: Ref, operation: OperationObject, pathItem: PathItemObject): Either => { const processedParameters: ParameterObject[] = []; @@ -142,7 +142,7 @@ const getParameters = combineReader( return resolvedSchema; } - const queryStringFragment = serializeParameterToTemplate( + const queryStringFragment = serializeQueryParameterToTemplate( from, resolved.right, resolvedSchema.right, @@ -209,7 +209,7 @@ const getParameters = combineReader( option.map(f => combineFragmentsK(f, c => serializedFragment( - `encodeURIComponent(compact([${c}]).join('&'))`, + `compact([${c}]).join('&')`, [serializedDependency('compact', 'fp-ts/lib/Array')], [], ), diff --git a/src/language/typescript/3.0/serializers/parameter-object.ts b/src/language/typescript/3.0/serializers/parameter-object.ts index 4037694..53ec65a 100644 --- a/src/language/typescript/3.0/serializers/parameter-object.ts +++ b/src/language/typescript/3.0/serializers/parameter-object.ts @@ -85,7 +85,7 @@ const getParameterExplode = (parameter: ParameterObject): boolean => option.getOrElse(() => getParameterObjectStyle(parameter) === 'form'), ); -export const serializeParameterToTemplate = ( +export const serializeQueryParameterToTemplate = ( from: Ref, parameter: ParameterObject, parameterSchema: SchemaObject, @@ -121,8 +121,11 @@ const getFn = ( if (PrimitiveSchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => serializePrimitiveParameter('${style}', '${parameter.name}', value)`, - [serializedDependency('serializePrimitiveParameter', pathToUtils)], + `value => option.fromEither(serializePrimitiveParameter('${style}', '${parameter.name}', value))`, + [ + serializedDependency('option', 'fp-ts'), + serializedDependency('serializePrimitiveParameter', pathToUtils), + ], [], ), ); @@ -130,8 +133,8 @@ const getFn = ( if (ArraySchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => serializeArrayParameter('${style}', '${parameter.name}', value, ${explode})`, - [serializedDependency('serializeArrayParameter', pathToUtils)], + `value => option.fromEither(serializeArrayParameter('${style}', '${parameter.name}', value, ${explode}))`, + [serializedDependency('option', 'fp-ts'), serializedDependency('serializeArrayParameter', pathToUtils)], [], ), ); @@ -139,8 +142,11 @@ const getFn = ( if (ObjectSchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => serializeObjectParameter('${style}', '${parameter.name}', value, ${explode})`, - [serializedDependency('serializeObjectParameter', pathToUtils)], + `value => option.fromEither(serializeObjectParameter('${style}', '${parameter.name}', value, ${explode}))`, + [ + serializedDependency('option', 'fp-ts'), + serializedDependency('serializeObjectParameter', pathToUtils), + ], [], ), ); diff --git a/src/language/typescript/common/data/serialized-fragment.ts b/src/language/typescript/common/data/serialized-fragment.ts index b8089da..256e3e5 100644 --- a/src/language/typescript/common/data/serialized-fragment.ts +++ b/src/language/typescript/common/data/serialized-fragment.ts @@ -125,6 +125,9 @@ export function combineFragmentsK( return serializedFragment(fragment.value, dependencies.concat(fragment.dependencies), refs.concat(fragment.refs)); } +/** + * @param f returns an Option + */ export const getSerializedOptionCallFragment = ( nullable: boolean, f: SerializedFragment, @@ -136,12 +139,19 @@ export const getSerializedOptionCallFragment = ( `pipe( ${a}, option.fromNullable, - option.map(${fn}), + option.chain(${fn}), )`, [serializedDependency('option', 'fp-ts'), serializedDependency('pipe', 'fp-ts/lib/pipeable')], [], ) - : serializedFragment(`some((${fn})(${a}))`, [serializedDependency('some', 'fp-ts/lib/Option')], []), + : serializedFragment( + `pipe( + ${a}, + ${fn}, + )`, + [serializedDependency('pipe', 'fp-ts/lib/pipeable')], + [], + ), ); export const commaFragment = serializedFragment(', ', [], []); From 18f6062b41e2ce1b7cce66e8915362183e446b9a Mon Sep 17 00:00:00 2001 From: Dmitry Kokovtsev Date: Fri, 17 Apr 2020 10:13:43 +0300 Subject: [PATCH 2/2] fix: more granular dependency on fromEither --- .../__tests__/operation-object.spec.ts | 4 ++-- .../3.0/serializers/parameter-object.ts | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts b/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts index f23d704..e3af1aa 100644 --- a/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts +++ b/src/language/typescript/3.0/serializers/__tests__/operation-object.spec.ts @@ -58,10 +58,10 @@ describe('OperationObject', () => { `compact([pipe( optionFromNullable(number).encode(parameters.query.offset), option.fromNullable, - option.chain(value => option.fromEither(serializePrimitiveParameter('form', 'offset', value))), + option.chain(value => fromEither(serializePrimitiveParameter('form', 'offset', value))), ),pipe( number.encode(parameters.query.limit), - value => option.fromEither(serializePrimitiveParameter('form', 'limit', value)), + value => fromEither(serializePrimitiveParameter('form', 'limit', value)), )]).join('&')` .trim() .replace(/\s+/g, ' '), diff --git a/src/language/typescript/3.0/serializers/parameter-object.ts b/src/language/typescript/3.0/serializers/parameter-object.ts index 53ec65a..809ad51 100644 --- a/src/language/typescript/3.0/serializers/parameter-object.ts +++ b/src/language/typescript/3.0/serializers/parameter-object.ts @@ -121,9 +121,9 @@ const getFn = ( if (PrimitiveSchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => option.fromEither(serializePrimitiveParameter('${style}', '${parameter.name}', value))`, + `value => fromEither(serializePrimitiveParameter('${style}', '${parameter.name}', value))`, [ - serializedDependency('option', 'fp-ts'), + serializedDependency('fromEither', 'fp-ts/lib/Option'), serializedDependency('serializePrimitiveParameter', pathToUtils), ], [], @@ -133,8 +133,11 @@ const getFn = ( if (ArraySchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => option.fromEither(serializeArrayParameter('${style}', '${parameter.name}', value, ${explode}))`, - [serializedDependency('option', 'fp-ts'), serializedDependency('serializeArrayParameter', pathToUtils)], + `value => fromEither(serializeArrayParameter('${style}', '${parameter.name}', value, ${explode}))`, + [ + serializedDependency('fromEither', 'fp-ts/lib/Option'), + serializedDependency('serializeArrayParameter', pathToUtils), + ], [], ), ); @@ -142,9 +145,9 @@ const getFn = ( if (ObjectSchemaObjectCodec.is(schema)) { return right( serializedFragment( - `value => option.fromEither(serializeObjectParameter('${style}', '${parameter.name}', value, ${explode}))`, + `value => fromEither(serializeObjectParameter('${style}', '${parameter.name}', value, ${explode}))`, [ - serializedDependency('option', 'fp-ts'), + serializedDependency('fromEither', 'fp-ts/lib/Option'), serializedDependency('serializeObjectParameter', pathToUtils), ], [],