Skip to content

Commit

Permalink
fix: suport complex objects for query params in api explorer
Browse files Browse the repository at this point in the history
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,
swagger-api/swagger-js#1385 and
OAI/OpenAPI-Specification#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.
  • Loading branch information
deepakrkris committed Feb 3, 2020
1 parent 56543fe commit 3394f9a
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,10 @@ describe('Routing metadata for parameters', () => {
const expectedParamSpec = <ParameterObject>{
name: 'filter',
in: 'query',
style: 'deepObject',
explode: true,
schema: {
type: 'object',
additionalProperties: true,
content: {
'application/json': {
schema: {type: 'object', additionalProperties: true},
},
},
};
expectSpecToBeEqual(MyController, expectedParamSpec);
Expand All @@ -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'},
},
},
},
},
};
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 10 additions & 5 deletions packages/openapi-v3/src/decorators/parameter.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -212,9 +215,11 @@ export namespace param {
return param({
name,
in: 'query',
style: 'deepObject',
explode: true,
schema,
content: {
'application/json': {
schema,
},
},
...spec,
});
},
Expand Down
11 changes: 9 additions & 2 deletions packages/rest/src/coercion/coerce-parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3394f9a

Please sign in to comment.