Skip to content

Commit ffcbb3d

Browse files
committed
fix: suport complex objects for query params in api explorer
BREAKING CHANGE: This fix has modified the api definitions described by the decorator 'param.query.object', to support Open-API's `url-encoded` definition for json query parameters. Previously, such parameters were described with `exploded: true` and `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 simple json objects as below but not for complex objects. ``` http://localhost:3000/todos?filter[limit]=2 ``` To address these issues with exploded queries, this fix switches definition of json query params from the `exploded`, `deep-object` style to the `url-encoded` style definition in Open-API spec. LoopBack already supports receiving url-encoded payload for json query parameters. For instance, to filter api results from the GET '/todo-list' endpoint in the todo-list example with a specific relation, { "include": [ { "relation": "todo" } ] }, the following url-encoded query parameter can be used, ``` http://localhost:3000/todos?filter=%7B%22include%22%3A%5B%7B%22relation%22%3A%22todoList%22%7D%5D%7D ``` The above was possible because the coercion behavior in LoopBack performed json parsing for `deep object` style json query params before this fix. This fix has modified that behavior by removing json parsing. Since the `exploded` `deep-object` definition has been removed from the `param.query.object` decorator, this new behaviour remains just an internal source code aspect as of now. In effect, this fix only modifies the open api definitions generated from LoopBack APIs. This fix removes the 'style' and 'explode' fields from the definition of json query params and moves the 'schema' field under 'content[application/json]'. This is the definition that supports url-encoding as per Open-API spec. ```json { "in": "query" "content": { "application/json": { "schema": {} } } } ``` Certain client libraries (like swagger-ui or LoopBack's api explorer) necessiate using Open-API's `url-encoded` style definition for json query params to support "sending" url-encoded payload. All consumers of LoopBack APIs may need to regenerate api definitions, if their client libraries require them to do so for url-encoding. Otherwise there wouldn't be any significant impact on API consumers. To preserve compatibility with existing REST API clients, this change is backward compatible. All exploded queries like `?filter[limit]=1` will continue to work for json query params, despite the fact that they are described differently in the OpenAPI spec. Existing api clients will continue to work after an upgrade. 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.
1 parent 56543fe commit ffcbb3d

File tree

8 files changed

+142
-36
lines changed

8 files changed

+142
-36
lines changed

examples/todo-list/src/__tests__/acceptance/todo-list.acceptance.ts

+10
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,16 @@ describe('TodoListApplication', () => {
204204
});
205205
});
206206

207+
it('exploded filter conditions work', async () => {
208+
const list = await givenTodoListInstance(todoListRepo);
209+
await givenTodoInstance(todoRepo, {title: 'todo1', todoListId: list.id});
210+
await givenTodoInstance(todoRepo, {title: 'todo2', todoListId: list.id});
211+
await givenTodoInstance(todoRepo, {title: 'todo3', todoListId: list.id});
212+
213+
const response = await client.get('/todos').query('filter[limit]=2');
214+
expect(response.body).to.have.length(2);
215+
});
216+
207217
/*
208218
============================================================================
209219
TEST HELPERS

examples/todo/src/__tests__/acceptance/todo.acceptance.ts

+11
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,17 @@ describe('TodoApplication', () => {
193193
.expect(200, [toJSON(todoInProgress)]);
194194
});
195195

196+
it('exploded filter conditions work', async () => {
197+
await givenTodoInstance({title: 'wake up', isComplete: true});
198+
await givenTodoInstance({
199+
title: 'go to sleep',
200+
isComplete: false,
201+
});
202+
203+
const response = await client.get('/todos').query('filter[limit]=2');
204+
expect(response.body).to.have.length(2);
205+
});
206+
196207
/*
197208
============================================================================
198209
TEST HELPERS

packages/openapi-v3/src/__tests__/unit/decorators/param/param-query.decorator.unit.ts

+18-18
Original file line numberDiff line numberDiff line change
@@ -221,19 +221,18 @@ describe('Routing metadata for parameters', () => {
221221
});
222222

223223
describe('@param.query.object', () => {
224-
it('sets in:query style:deepObject and a default schema', () => {
224+
it('sets in:query and content["application/json"]', () => {
225225
class MyController {
226226
@get('/greet')
227227
greet(@param.query.object('filter') filter: object) {}
228228
}
229229
const expectedParamSpec = <ParameterObject>{
230230
name: 'filter',
231231
in: 'query',
232-
style: 'deepObject',
233-
explode: true,
234-
schema: {
235-
type: 'object',
236-
additionalProperties: true,
232+
content: {
233+
'application/json': {
234+
schema: {type: 'object', additionalProperties: true},
235+
},
237236
},
238237
};
239238
expectSpecToBeEqual(MyController, expectedParamSpec);
@@ -256,13 +255,15 @@ describe('Routing metadata for parameters', () => {
256255
const expectedParamSpec: ParameterObject = {
257256
name: 'filter',
258257
in: 'query',
259-
style: 'deepObject',
260-
explode: true,
261-
schema: {
262-
type: 'object',
263-
properties: {
264-
where: {type: 'object', additionalProperties: true},
265-
limit: {type: 'number'},
258+
content: {
259+
'application/json': {
260+
schema: {
261+
type: 'object',
262+
properties: {
263+
where: {type: 'object', additionalProperties: true},
264+
limit: {type: 'number'},
265+
},
266+
},
266267
},
267268
},
268269
};
@@ -300,11 +301,10 @@ describe('Routing metadata for parameters', () => {
300301
name: 'filter',
301302
in: 'query',
302303
description: 'Search criteria',
303-
style: 'deepObject',
304-
explode: true,
305-
schema: {
306-
type: 'object',
307-
additionalProperties: true,
304+
content: {
305+
'application/json': {
306+
schema: {type: 'object', additionalProperties: true},
307+
},
308308
},
309309
};
310310
expectSpecToBeEqual(MyController, expectedParamSpec);

packages/openapi-v3/src/decorators/parameter.decorator.ts

+10-5
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@ export function param(paramSpec: ParameterObject) {
5050
// generate schema if `paramSpec` has `schema` but without `type`
5151
(isSchemaObject(paramSpec.schema) && !paramSpec.schema.type)
5252
) {
53-
// please note `resolveSchema` only adds `type` and `format` for `schema`
54-
paramSpec.schema = resolveSchema(paramType, paramSpec.schema);
53+
// If content explicitly mentioned do not resolve schema
54+
if (!paramSpec.content) {
55+
// please note `resolveSchema` only adds `type` and `format` for `schema`
56+
paramSpec.schema = resolveSchema(paramType, paramSpec.schema);
57+
}
5558
}
5659
}
5760

@@ -212,9 +215,11 @@ export namespace param {
212215
return param({
213216
name,
214217
in: 'query',
215-
style: 'deepObject',
216-
explode: true,
217-
schema,
218+
content: {
219+
'application/json': {
220+
schema,
221+
},
222+
},
218223
...spec,
219224
});
220225
},

packages/rest/src/__tests__/unit/coercion/paramObject.unit.ts

+25-5
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ import {test} from './utils';
1111
const OPTIONAL_ANY_OBJECT: ParameterObject = {
1212
in: 'query',
1313
name: 'aparameter',
14-
schema: {
15-
type: 'object',
16-
additionalProperties: true,
14+
content: {
15+
'application/json': {
16+
schema: {
17+
type: 'object',
18+
additionalProperties: true,
19+
},
20+
},
1721
},
18-
style: 'deepObject',
19-
explode: true,
2022
};
2123

2224
const REQUIRED_ANY_OBJECT = {
@@ -35,6 +37,15 @@ describe('coerce object param - required', function() {
3537
test(REQUIRED_ANY_OBJECT, {key: 'text'}, {key: 'text'});
3638
});
3739

40+
context('valid string values', () => {
41+
// simple object
42+
test(REQUIRED_ANY_OBJECT, '{"key": "text"}', {key: 'text'});
43+
// nested objects
44+
test(REQUIRED_ANY_OBJECT, '{"include": [{ "relation" : "todoList" }]}', {
45+
include: [{relation: 'todoList'}],
46+
});
47+
});
48+
3849
context('empty values trigger ERROR_BAD_REQUEST', () => {
3950
test(
4051
REQUIRED_ANY_OBJECT,
@@ -90,6 +101,15 @@ describe('coerce object param - optional', function() {
90101
test(OPTIONAL_ANY_OBJECT, 'null', null);
91102
});
92103

104+
context('valid string values', () => {
105+
// simple object
106+
test(OPTIONAL_ANY_OBJECT, '{"key": "text"}', {key: 'text'});
107+
// nested objects
108+
test(OPTIONAL_ANY_OBJECT, '{"include": [{ "relation" : "todoList" }]}', {
109+
include: [{relation: 'todoList'}],
110+
});
111+
});
112+
93113
context('nested values are not coerced', () => {
94114
test(OPTIONAL_ANY_OBJECT, {key: 'undefined'}, {key: 'undefined'});
95115
test(OPTIONAL_ANY_OBJECT, {key: 'null'}, {key: 'null'});

packages/rest/src/__tests__/unit/parser.unit.ts

+32-4
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ describe('operationArgsParser', () => {
300300
expect(args).to.eql(['<html><body><h1>Hello</h1></body></html>']);
301301
});
302302

303-
context('in:query style:deepObject', () => {
303+
context('in:query content:application/json', () => {
304304
it('parses JSON-encoded string value', async () => {
305305
const req = givenRequest({
306306
url: '/?value={"key":"value"}',
@@ -346,6 +346,32 @@ describe('operationArgsParser', () => {
346346
);
347347
});
348348

349+
it('parses complex json object', async () => {
350+
const req = givenRequest({
351+
url: '/?filter={"include": [{"relation": "todoList"}]}',
352+
});
353+
354+
const spec = givenOperationWithObjectParameter('filter');
355+
const route = givenResolvedRoute(spec);
356+
357+
const args = await parseOperationArgs(req, route, requestBodyParser);
358+
359+
expect(args).to.eql([{include: [{relation: 'todoList'}]}]);
360+
});
361+
362+
it('parses url-encoded complex json object', async () => {
363+
const req = givenRequest({
364+
url: '/?filter=%7B"include"%3A%5B%7B"relation"%3A"todoList"%7D%5D%7D',
365+
});
366+
367+
const spec = givenOperationWithObjectParameter('filter');
368+
const route = givenResolvedRoute(spec);
369+
370+
const args = await parseOperationArgs(req, route, requestBodyParser);
371+
372+
expect(args).to.eql([{include: [{relation: 'todoList'}]}]);
373+
});
374+
349375
it('rejects array values encoded as JSON', async () => {
350376
const req = givenRequest({
351377
url: '/?value=[1,2]',
@@ -381,9 +407,11 @@ describe('operationArgsParser', () => {
381407
{
382408
name,
383409
in: 'query',
384-
style: 'deepObject',
385-
explode: true,
386-
schema,
410+
content: {
411+
'application/json': {
412+
schema,
413+
},
414+
},
387415
},
388416
]);
389417
}

packages/rest/src/coercion/coerce-parameter.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,14 @@ export function coerceParameter(
3232
data: string | undefined | object,
3333
spec: ParameterObject,
3434
) {
35-
const schema = spec.schema;
35+
let schema = spec.schema;
36+
37+
// If a query parameter is a url encoded Json object, the schema is defined under content['application/json']
38+
if (!schema && spec.in === 'query' && spec.content) {
39+
const jsonSpec = spec.content['application/json'];
40+
schema = jsonSpec.schema;
41+
}
42+
3643
if (!schema || isReferenceObject(schema)) {
3744
debug(
3845
'The parameter with schema %s is not coerced since schema' +
@@ -172,9 +179,9 @@ function parseJsonIfNeeded(
172179
): string | object | undefined {
173180
if (typeof data !== 'string') return data;
174181

175-
if (spec.in !== 'query' || spec.style !== 'deepObject') {
182+
if (spec.in !== 'query' || (spec.in === 'query' && !spec.content)) {
176183
debug(
177-
'Skipping JSON.parse, argument %s is not in:query style:deepObject',
184+
'Skipping JSON.parse, argument %s is not a url encoded json object query parameter (since content field is missing in parameter schema)',
178185
spec.name,
179186
);
180187
return data;

packages/rest/src/coercion/validator.ts

+26-1
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,34 @@ export class Validator {
6565
isAbsent(value: any) {
6666
if (value === '' || value === undefined) return true;
6767

68+
const spec: ParameterObject = this.ctx.parameterSpec;
6869
const schema: SchemaObject = this.ctx.parameterSpec.schema ?? {};
69-
if (schema.type === 'object' && value === 'null') return true;
70+
const valueIsNull = value === 'null' || value === null;
7071

72+
if (this.isUrlEncodedJsonParam()) {
73+
// is this an url encoded Json object query parameter?
74+
// check for NULL values
75+
if (valueIsNull) return true;
76+
} else if (spec.schema) {
77+
// if parameter spec contains schema object, check if supplied value is NULL
78+
if (schema.type === 'object' && valueIsNull) return true;
79+
}
80+
81+
return false;
82+
}
83+
84+
/**
85+
* Return `true` if defined specification is for a url encoded Json object query parameter
86+
*
87+
* for url encoded Json object query parameters,
88+
* schema is defined under content['application/json']
89+
*/
90+
isUrlEncodedJsonParam() {
91+
const spec: ParameterObject = this.ctx.parameterSpec;
92+
93+
if (spec.in === 'query' && spec.content?.['application/json']?.schema) {
94+
return true;
95+
}
7196
return false;
7297
}
7398
}

0 commit comments

Comments
 (0)