fix: code review comments

loopbackio · Jan 31, 2020 · 206edb2 · 206edb2
1 parent c7f42be
commit 206edb2
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 14 deletions.
diff --git a/examples/todo-list/src/__tests__/acceptance/todo-list.acceptance.ts b/examples/todo-list/src/__tests__/acceptance/todo-list.acceptance.ts
@@ -204,6 +204,16 @@ describe('TodoListApplication', () => {
     });
   });
 
+  it('exploded filter conditions work', async () => {
+    const list = await givenTodoListInstance(todoListRepo);
+    await givenTodoInstance(todoRepo, {title: 'todo1', todoListId: list.id});
+    await givenTodoInstance(todoRepo, {title: 'todo2', todoListId: list.id});
+    await givenTodoInstance(todoRepo, {title: 'todo3', todoListId: list.id});
+
+    const response = await client.get('/todos').query('filter[limit]=2');
+    expect(response.body).to.have.length(2);
+  });
+
   /*
    ============================================================================
    TEST HELPERS

diff --git a/examples/todo/src/__tests__/acceptance/todo.acceptance.ts b/examples/todo/src/__tests__/acceptance/todo.acceptance.ts
@@ -193,6 +193,17 @@ describe('TodoApplication', () => {
       .expect(200, [toJSON(todoInProgress)]);
   });
 
+  it('exploded filter conditions work', async () => {
+    await givenTodoInstance({title: 'wake up', isComplete: true});
+    await givenTodoInstance({
+      title: 'go to sleep',
+      isComplete: false,
+    });
+
+    const response = await client.get('/todos').query('filter[limit]=2');
+    expect(response.body).to.have.length(2);
+  });
+
   /*
    ============================================================================
    TEST HELPERS

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
@@ -221,7 +221,7 @@ describe('Routing metadata for parameters', () => {
   });
 
   describe('@param.query.object', () => {
-    it('sets in:query style:deepObject and a default schema', () => {
+    it('sets in:query and content["application/json"]', () => {
       class MyController {
         @get('/greet')
         greet(@param.query.object('filter') filter: object) {}

diff --git a/packages/rest/src/__tests__/unit/coercion/paramObject.unit.ts b/packages/rest/src/__tests__/unit/coercion/paramObject.unit.ts
@@ -35,6 +35,12 @@ describe('coerce object param - required', function() {
     test(REQUIRED_ANY_OBJECT, {key: 'undefined'}, {key: 'undefined'});
     test(REQUIRED_ANY_OBJECT, {key: 'null'}, {key: 'null'});
     test(REQUIRED_ANY_OBJECT, {key: 'text'}, {key: 'text'});
+  });
+
+  context('valid string values', () => {
+    // simple object
+    test(REQUIRED_ANY_OBJECT, '{"key": "text"}', {key: 'text'});
+    // nested objects
     test(REQUIRED_ANY_OBJECT, '{"include": [{ "relation" : "todoList" }]}', {
       include: [{relation: 'todoList'}],
     });
@@ -95,6 +101,15 @@ describe('coerce object param - optional', function() {
     test(OPTIONAL_ANY_OBJECT, 'null', null);
   });
 
+  context('valid string values', () => {
+    // simple object
+    test(OPTIONAL_ANY_OBJECT, '{"key": "text"}', {key: 'text'});
+    // nested objects
+    test(OPTIONAL_ANY_OBJECT, '{"include": [{ "relation" : "todoList" }]}', {
+      include: [{relation: 'todoList'}],
+    });
+  });
+
   context('nested values are not coerced', () => {
     test(OPTIONAL_ANY_OBJECT, {key: 'undefined'}, {key: 'undefined'});
     test(OPTIONAL_ANY_OBJECT, {key: 'null'}, {key: 'null'});

diff --git a/packages/rest/src/__tests__/unit/parser.unit.ts b/packages/rest/src/__tests__/unit/parser.unit.ts
@@ -346,6 +346,19 @@ describe('operationArgsParser', () => {
       );
     });
 
+    it('parses complex json object', async () => {
+      const req = givenRequest({
+        url: '/?filter={"include": [{"relation": "todoList"}]}',
+      });
+
+      const spec = givenOperationWithObjectParameter('filter');
+      const route = givenResolvedRoute(spec);
+
+      const args = await parseOperationArgs(req, route, requestBodyParser);
+
+      expect(args).to.eql([{include: [{relation: 'todoList'}]}]);
+    });
+
     it('parses url-encoded complex json object', async () => {
       const req = givenRequest({
         url: '/?filter=%7B"include"%3A%5B%7B"relation"%3A"todoList"%7D%5D%7D',

diff --git a/packages/rest/src/coercion/validator.ts b/packages/rest/src/coercion/validator.ts
@@ -69,22 +69,30 @@ export class Validator {
     const schema: SchemaObject = this.ctx.parameterSpec.schema ?? {};
     const valueIsNull = value === 'null' || value === null;
 
-    // if parameter spec contains schema object, check if supplied value is NULL
-    if (spec.schema) {
+    if (this.isUrlEncodedJsonParam()) {
+      // is this an url encoded Json object query parameter?
+      // check for NULL values
+      if (valueIsNull) return true;
+    } else if (spec.schema) {
+      // if parameter spec contains schema object, check if supplied value is NULL
       if (schema.type === 'object' && valueIsNull) return true;
-    } else {
-      // for url encoded Json object query parameters, schema is defined under content['application/json']
-      if (
-        spec.in === 'query' &&
-        spec.content &&
-        spec.content['application/json'] &&
-        spec.content['application/json'].schema
-      ) {
-        // check for NULL values in url encoded Json objects
-        if (valueIsNull) return true;
-      }
     }
 
     return false;
   }
+
+  /**
+   * Return `true` if defined specification is for a url encoded Json object query parameter
+   *
+   * for url encoded Json object query parameters,
+   * schema is defined under content['application/json']
+   */
+  isUrlEncodedJsonParam() {
+    const spec: ParameterObject = this.ctx.parameterSpec;
+
+    if (spec.in === 'query' && spec.content?.['application/json']?.schema) {
+      return true;
+    }
+    return false;
+  }
 }