From 3394f9a4052a2502d67bf4b6aa53756d1d405e4f Mon Sep 17 00:00:00 2001 From: DEEPAK RAJAMOHAN Date: Mon, 30 Dec 2019 12:22:15 +0530 Subject: [PATCH] fix: suport complex objects for query params in api explorer BREAKING CHANGE: This fix has modified the schema described by the decorator 'param.query.object', to support Open-API specification's url-encoded format for json query parameters. Previously, such parameters were described with `exploded: true`, `style: deepObject` which turned out to be problematic, as explained and discussed in, https://github.com/swagger-api/swagger-js/issues/1385 and https://github.com/OAI/OpenAPI-Specification/issues/1706 ```json { "in": "query", "style": "deepObject" "explode": "true", "schema": {} } ``` Exploded encoding worked for json query params if the payload was simple as below, but not for complex objects. ``` http://localhost:3000/todos?filter[limit]=2 ``` To address these issues with exploded queries, this fix modifies the definition of JSON query params from `exploded` and `deep-object` style to the `url-encoded` style described in Open-API spec. The 'style' and 'explode' fields are removed. The 'schema' field is moved under 'content[application/json]' (`url-encoded` style) as below, ```json { "in": "query" "content": { "application/json": { "schema": {} } } } ``` for example, to filter api results with the following condition, ```json { "include": [ { "relation": "todo" } ] } ``` the following url-encoded query parameter is used. ``` http://localhost:3000/todos?filter=%7B%22include%22%3A%5B%7B%22relation%22%3A%22todoList%22%7D%5D%7D ``` To preserve compatibility with existing REST API clients, this change is backward compatible with all previously supported formats for json query parameters. Exploded queries like `?filter[limit]=1` will continue to work, despite the fact that they are described differently in the OpenAPI spec. As a result, existing REST API clients will keep working after an upgrade. LoopBack supported receiving `url-encoded` payload for `exploded`, `deep object` style query params on the server side even before this fix, even though the Open-API spec has very clear demarcations for both these styles. In effect, this fix only clarifies the schema contract as per Open-API spec. The signature of the 'param.query.object' decorator has not changed. There is no code changes required in the LoopBack APIs after upgrading to this fix. No method signatures or data structures are impacted. The impact is only on the open api specifications generated from LoopBack APIs. All consumers of LoopBack APIs may need to regenerate the api specifications, if their API clients or client tools (like swagger-ui or LoopBack's api explorer) necessiate adhering to Open-API's url-encoded query parameter definition to use url-encoding. Otherwise there wouldnt be any significant impact on API consumers. --- .../param/param-query.decorator.unit.ts | 34 +++++++++---------- .../src/decorators/parameter.decorator.ts | 15 +++++--- .../rest/src/coercion/coerce-parameter.ts | 11 ++++-- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/packages/openapi-v3/src/__tests__/unit/decorators/param/param-query.decorator.unit.ts b/packages/openapi-v3/src/__tests__/unit/decorators/param/param-query.decorator.unit.ts index 34e8ee0f7720..85d2f50a7b09 100644 --- a/packages/openapi-v3/src/__tests__/unit/decorators/param/param-query.decorator.unit.ts +++ b/packages/openapi-v3/src/__tests__/unit/decorators/param/param-query.decorator.unit.ts @@ -229,11 +229,10 @@ describe('Routing metadata for parameters', () => { const expectedParamSpec = { name: 'filter', in: 'query', - style: 'deepObject', - explode: true, - schema: { - type: 'object', - additionalProperties: true, + content: { + 'application/json': { + schema: {type: 'object', additionalProperties: true}, + }, }, }; expectSpecToBeEqual(MyController, expectedParamSpec); @@ -256,13 +255,15 @@ describe('Routing metadata for parameters', () => { const expectedParamSpec: ParameterObject = { name: 'filter', in: 'query', - style: 'deepObject', - explode: true, - schema: { - type: 'object', - properties: { - where: {type: 'object', additionalProperties: true}, - limit: {type: 'number'}, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + where: {type: 'object', additionalProperties: true}, + limit: {type: 'number'}, + }, + }, }, }, }; @@ -300,11 +301,10 @@ describe('Routing metadata for parameters', () => { name: 'filter', in: 'query', description: 'Search criteria', - style: 'deepObject', - explode: true, - schema: { - type: 'object', - additionalProperties: true, + content: { + 'application/json': { + schema: {type: 'object', additionalProperties: true}, + }, }, }; expectSpecToBeEqual(MyController, expectedParamSpec); diff --git a/packages/openapi-v3/src/decorators/parameter.decorator.ts b/packages/openapi-v3/src/decorators/parameter.decorator.ts index afda71d78644..40bb3ecb52e2 100644 --- a/packages/openapi-v3/src/decorators/parameter.decorator.ts +++ b/packages/openapi-v3/src/decorators/parameter.decorator.ts @@ -50,8 +50,11 @@ export function param(paramSpec: ParameterObject) { // generate schema if `paramSpec` has `schema` but without `type` (isSchemaObject(paramSpec.schema) && !paramSpec.schema.type) ) { - // please note `resolveSchema` only adds `type` and `format` for `schema` - paramSpec.schema = resolveSchema(paramType, paramSpec.schema); + // If content explicitly mentioned do not resolve schema + if (!paramSpec.content) { + // please note `resolveSchema` only adds `type` and `format` for `schema` + paramSpec.schema = resolveSchema(paramType, paramSpec.schema); + } } } @@ -212,9 +215,11 @@ export namespace param { return param({ name, in: 'query', - style: 'deepObject', - explode: true, - schema, + content: { + 'application/json': { + schema, + }, + }, ...spec, }); }, diff --git a/packages/rest/src/coercion/coerce-parameter.ts b/packages/rest/src/coercion/coerce-parameter.ts index 36121d035cab..8ea60a0ebe2c 100644 --- a/packages/rest/src/coercion/coerce-parameter.ts +++ b/packages/rest/src/coercion/coerce-parameter.ts @@ -32,7 +32,14 @@ export function coerceParameter( data: string | undefined | object, spec: ParameterObject, ) { - const schema = spec.schema; + let schema = spec.schema; + + if (!schema && spec.content) { + const content = spec.content; + const jsonSpec = content['application/json']; + schema = jsonSpec.schema; + } + if (!schema || isReferenceObject(schema)) { debug( 'The parameter with schema %s is not coerced since schema' + @@ -172,7 +179,7 @@ function parseJsonIfNeeded( ): string | object | undefined { if (typeof data !== 'string') return data; - if (spec.in !== 'query' || spec.style !== 'deepObject') { + if (spec.in !== 'query' || (spec.style !== 'deepObject' && !spec.content)) { debug( 'Skipping JSON.parse, argument %s is not in:query style:deepObject', spec.name,