feat: simplify request body decorator

packages/openapi-v3/_spike-request-body.md

@@ -0,0 +1,105 @@
+## Improve the UX of @requestBody()
+
+The original discussion is tracked in issue
+[Spike: simplify requestBody annotation with schema options](https://github.com/strongloop/loopback-next/issues/2654).
+
+The current @requestBody() can only
+
+- takes in an entire request body specification with very nested media type
+  objects or
+- generate the schema inferred from the parameter type
+
+To simplify the signature, this spike PR introduces two more parameters
+`modelCtor` and `schemaOptions` to configure the schema. The new decorator
+`requestBody2()`(let's discuss a better name later, see section
+[Naming](#Naming) is written in file 'request-body.spike.decorator.ts'
+
+### Signature
+
+The new decorator's signature is
+`@requestBody2(spec, modelCtor, schemaOptions)`.
+
+```ts
+export function requestBody2(
+  specOrModelOrOptions?: Partial<RequestBodyObject> | Function | SchemaOptions,
+  modelOrOptions?: Function | SchemaOptions,
+  schemaOptions?: SchemaOptions,
+) {
+  // implementation
+}
+```
+
+All the 3 parameters are optional, therefore in the PoC, the real implementation
+are in `_requestBody2()`, `requestBody2()` is a wrapper that resolves different
+combination of the parameters.
+
+_Please note TypeScript doesn't support function overloading with different
+number of parameters(like requestBody(spec), requestBody(model, options)).
+Therefore we have to create a wrapper function to resolve different signatures
+from the caller_
+
+My PoC PR only adds 2 unit tests for different signatures, the real
+implementation should test all the combinations.
+
+### Create - exclude properties
+
+Take "Create a new product with excluded properties" as an example:
+
+```ts
+// The decorator takes in the option without having a nested content object
+class MyController1 {
+  @post('/Product')
+  create(
+    @requestBody2(
+      // Provide the description as before
+      {
+        description: 'Create a product',
+        required: true,
+      },
+      // Using advanced ts types like `Exclude<>`, `Partial<>` results in
+      // `MetadataInspector.getDesignTypeForMethod(target, member)` only
+      // returns `Object` as the param type, which loses the model type's info.
+      // Therefore user must provide the model type in options.
+      Product,
+      // Provide the options that configure your schema
+      {
+        // The excluded properties
+        exclude: ['id'],
+      },
+    )
+    product: Exclude<Product, ['id']>,
+  ) {}
+}
+```
+
+### Update - partial properties
+
+```ts
+class MyController2 {
+  @put('/Product')
+  update(
+    @requestBody2(
+      {description: 'Update a product', required: true},
+      Product,
+      {partial: true},
+    )
+    product: Partial<Product>,
+  ) {}
+}
+```
+
+## Naming
+
+From @jannyHou: I think we can keep the name `requestBody` unchanged. I enabled
+the existing `@requestBody()`'s tests but applied `requestBody2()` decorator,
+all tests pass, which means there is no breaking change.
+
+## Follow-up Stories
+
+_I will create stories if we agree on the plan_
+
+- [ ] Modify the current `@requestBody()` according to the spike code, pass
+      existing tests across all repos.
+- [ ] Add more unit tests to verify all the signatures.
+- [ ] Upgrade examples to provide the descriptive configs(model, options) using
+      the new decorator.

packages/openapi-v3/src/__tests__/unit/decorators/request-body/request-body.decorator.unit.ts

@@ -3,7 +3,7 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {post, requestBody, getControllerSpec} from '../../../..';
+import {post, requestBody2, getControllerSpec} from '../../../..';
 import {expect} from '@loopback/testlab';
 import {model, property} from '@loopback/repository';
 
@@ -16,7 +16,7 @@ describe('requestBody decorator', () => {
       };
       class MyController {
         @post('/greeting')
-        greet(@requestBody(requestSpec) name: string) {}
+        greet(@requestBody2(requestSpec) name: string) {}
       }
 
       const requestBodySpec = getControllerSpec(MyController).paths[
@@ -35,7 +35,7 @@ describe('requestBody decorator', () => {
       };
       class MyController {
         @post('/greeting')
-        greet(@requestBody(requestSpec) name: string) {}
+        greet(@requestBody2(requestSpec) name: string) {}
       }
 
       const requestBodySpec = getControllerSpec(MyController).paths[
@@ -60,7 +60,7 @@ describe('requestBody decorator', () => {
       class MyController {
         @post('/MyModel')
         createMyModel(
-          @requestBody({content: {'application/text': {}}}) inst: MyModel,
+          @requestBody2({content: {'application/text': {}}}) inst: MyModel,
         ) {}
       }
 
@@ -81,7 +81,9 @@ describe('requestBody decorator', () => {
 
       class MyController {
         @post('/MyModel')
-        createMyModel(@requestBody({content: expectedContent}) inst: MyModel) {}
+        createMyModel(
+          @requestBody2({content: expectedContent}) inst: MyModel,
+        ) {}
       }
 
       const requestBodySpec = getControllerSpec(MyController).paths['/MyModel'][
@@ -102,7 +104,7 @@ describe('requestBody decorator', () => {
       class MyController {
         @post('/MyModel')
         createMyModel(
-          @requestBody({content: expectedContent}) inst: Partial<MyModel>,
+          @requestBody2({content: expectedContent}) inst: Partial<MyModel>,
         ) {}
       }
 
@@ -115,7 +117,7 @@ describe('requestBody decorator', () => {
     it('reports error if more than one requestBody are found for the same method', () => {
       class MyController {
         @post('/greeting')
-        greet(@requestBody() name: string, @requestBody() foo: number) {}
+        greet(@requestBody2() name: string, @requestBody2() foo: number) {}
       }
       expect(() => getControllerSpec(MyController)).to.throwError(
         /An operation should only have one parameter decorated by @requestBody/,

packages/openapi-v3/src/__tests__/unit/decorators/request-body/request-body.spike.decorator.unit.ts

@@ -0,0 +1,223 @@
+// Copyright IBM Corp. 2019. All Rights Reserved.
+// Node module: @loopback/openapi-v3
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+import {
+  belongsTo,
+  Entity,
+  hasMany,
+  model,
+  property,
+} from '@loopback/repository';
+import {expect} from '@loopback/testlab';
+import {getControllerSpec, post, put} from '../../../..';
+import {requestBody2} from '../../../../decorators/request-body.spike.decorator';
+
+describe('spike - requestBody decorator', () => {
+  context('CRUD', () => {
+    @model()
+    class Product extends Entity {
+      @property({
+        type: 'string',
+      })
+      name: string;
+      @belongsTo(() => Category)
+      categoryId: number;
+
+      constructor(data?: Partial<Product>) {
+        super(data);
+      }
+    }
+
+    /**
+     * Navigation properties of the Product model.
+     */
+    interface ProductRelations {
+      category?: CategoryWithRelations;
+    }
+    /**
+     * Product's own properties and navigation properties.
+     */
+    type ProductWithRelations = Product & ProductRelations;
+
+    @model()
+    class Category extends Entity {
+      @hasMany(() => Product)
+      products?: Product[];
+    }
+    /**
+     * Navigation properties of the Category model.
+     */
+    interface CategoryRelations {
+      products?: ProductWithRelations[];
+    }
+    /**
+     * Category's own properties and navigation properties.
+     */
+    type CategoryWithRelations = Category & CategoryRelations;
+
+    it('create - generates schema with excluded properties', () => {
+      class MyController1 {
+        @post('/Product')
+        create(
+          @requestBody2(
+            {description: 'Create a product', required: true},
+            Product,
+            {exclude: ['id']},
+          )
+          product: Exclude<Product, ['id']>,
+        ) {}
+      }
+
+      const spec1 = getControllerSpec(MyController1);
+
+      const requestBodySpecForCreate =
+        spec1.paths['/Product']['post'].requestBody;
+
+      const referenceSchema = spec1.components!.schemas![
+        'ProductExcluding[id]'
+      ];
+
+      expect(requestBodySpecForCreate).to.have.properties({
+        description: 'Create a product',
+        required: true,
+        content: {
+          'application/json': {
+            schema: {
+              $ref: '#/components/schemas/ProductExcluding[id]',
+            },
+          },
+        },
+      });
+
+      // The feature that generates schemas according to
+      // different options is TO BE DONE and out of the
+      // scope of this spike, so that the schema `PartialProduct`
+      // here is still the same as `Product`
+      expect(referenceSchema).to.have.properties({
+        title: 'ProductExcluding[id]',
+        properties: {
+          categoryId: {type: 'number'},
+          name: {type: 'string'},
+        },
+      });
+    });
+
+    it('update - generates schema with partial properties', () => {
+      class MyController2 {
+        @put('/Product')
+        update(
+          @requestBody2(
+            {description: 'Update a product', required: true},
+            Product,
+            {partial: true},
+          )
+          product: Partial<Product>,
+        ) {}
+      }
+
+      const spec2 = getControllerSpec(MyController2);
+
+      const requestBodySpecForCreate =
+        spec2.paths['/Product']['put'].requestBody;
+
+      const referenceSchema = spec2.components!.schemas!.ProductPartial;
+
+      expect(requestBodySpecForCreate).to.have.properties({
+        description: 'Update a product',
+        required: true,
+        content: {
+          'application/json': {
+            schema: {
+              $ref: '#/components/schemas/ProductPartial',
+            },
+          },
+        },
+      });
+
+      // The feature that generates schemas according to
+      // different options is TO BE DONE and out of the
+      // scope of this spike, so that the schema `PartialProduct`
+      // here is still the same as `Product`
+      expect(referenceSchema).to.have.properties({
+        title: 'ProductPartial',
+        properties: {
+          categoryId: {type: 'number'},
+          name: {type: 'string'},
+        },
+      });
+    });
+  });
+  context(
+    'different signatures(More tests TBD in the real implementation)',
+    () => {
+      @model()
+      class Test extends Entity {
+        @property({
+          type: 'string',
+        })
+        name: string;
+        constructor(data?: Partial<Test>) {
+          super(data);
+        }
+      }
+      it('default', () => {
+        class TestController {
+          @post('/Test')
+          create(@requestBody2() test: Test) {}
+        }
+
+        const testSpec1 = getControllerSpec(TestController);
+
+        const requestBodySpecForCreate =
+          testSpec1.paths['/Test']['post'].requestBody;
+        expect(requestBodySpecForCreate).to.have.properties({
+          content: {
+            'application/json': {
+              schema: {
+                $ref: '#/components/schemas/Test',
+              },
+            },
+          },
+        });
+
+        const referenceSchema = testSpec1.components!.schemas!.Test;
+        expect(referenceSchema).to.have.properties({
+          title: 'Test',
+          properties: {
+            name: {type: 'string'},
+          },
+        });
+      });
+      it('omits the 1st parameter', () => {
+        class TestController {
+          @post('/Test')
+          create(@requestBody2(Test, {partial: true}) test: Partial<Test>) {}
+        }
+
+        const testSpec1 = getControllerSpec(TestController);
+
+        const requestBodySpecForCreate =
+          testSpec1.paths['/Test']['post'].requestBody;
+        expect(requestBodySpecForCreate).to.have.properties({
+          content: {
+            'application/json': {
+              schema: {
+                $ref: '#/components/schemas/TestPartial',
+              },
+            },
+          },
+        });
+
+        const referenceSchema = testSpec1.components!.schemas!.TestPartial;
+        expect(referenceSchema).to.have.properties({
+          title: 'TestPartial',
+          properties: {
+            name: {type: 'string'},
+          },
+        });
+      });
+    },
+  );
+});