PoC: TypeScript types and OpenAPI schema to describe navigational properties (inclusion of related models)

_SPIKE_.md

@@ -0,0 +1,348 @@
+# Spike documentation
+
+See the discussion in
+[#2152](https://github.com/strongloop/loopback-next/issues/2152) for background
+and information on different approaches I have researched and abandoned along
+the way.
+
+## Overview
+
+In this feature branch, I am presenting a proof of concept implementation
+demonstrating how to describe navigational properties for inclusion of related
+models.
+
+The solution consists of three parts:
+
+1. A convention for describing navigational properties (links) via an interface,
+   including support in `CrudRepository` and `DefaultCrudRepository` classes.
+
+2. Improvements in repository-json-schema (`getJsonSchema` and
+   `modelToJsonSchema`):
+
+   - 2.1: A new option `includeRelations` controlling whether navigational
+     properties are included in the generated schema.
+
+   - 2.2: Support for cyclic references.
+
+     For example: `CategoryWithRelations` has a property `products` containing
+     an array of `ProductWithRelations`. `ProductWithRelations` has a property
+     `category` containing `CategoryWithRelations`.
+
+     Please note this example is NOT using the approach for model inclusion,
+     it's just an acceptance criteria.
+
+   - 2.3: A new helper function `modelToJsonSchemaRef` that emits JSON Schema
+     reference and includes definitions of all references models.
+
+3. Improvements in code generating controller OpenAPI spec allowing app
+   developers to use `modelToJsonSchemaRef` to describe response schema.
+
+Additionally, the todo-list example has been updated to show the proposed
+solution in practice.
+
+## Discussion
+
+We have the following requirements on a solution for describing navigational
+properties:
+
+1. A (compile-time) type describing navigational properties of a model. For
+   example, when a `Category` model has many instances of a `Product` model,
+   then we want to define property `products` containing an array of `Product`
+   instances _with product navigational properties included_.
+
+2. Ability to define a compile-time type where all navigational properties are
+   defined as optional. This is needed by Repository implementation.
+
+3. Ability to generate two OpenAPI/JSON Schemas for each model:
+
+   1. Own properties only
+   2. Both own properties and navigational properties
+
+4. SHOULD HAVE: To support JavaScript developers and declarative support, the
+   new types should be optional. At runtime, we should leverage the dynamic
+   nature of JavaScript objects and add navigational properties to an instance
+   of the original model. Specifically, we should not require another model
+   class to represent model with relations.
+
+My proposed solution meets all requirements above. Additionally, it consists of
+several smaller building blocks that can be used beyond the scope of
+navigational properties too.
+
+## Solution details
+
+### Interface describing model navigational properties
+
+To describe navigation properties for TypeScript compiler, application
+developers will define a new interface for each model.
+
+BelongsTo relation:
+
+```ts
+@model()
+class Product extends Entity {
+  @belongsTo(() => Category)
+  categoryId: number;
+}
+
+/**
+ * Navigation properties of the Product model.
+ */
+interface ProductRelations {
+  category?: CategoryWithRelations;
+}
+
+/**
+ * Product's own properties and navigation properties.
+ */
+type ProductWithRelations = Product & ProductRelations;
+```
+
+HasMany relation:
+
+```ts
+@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;
+```
+
+This solution has few important properties I'd like to explicitly point out:
+
+- It is independent on how the relations are defined. `@belongsTo` decorator is
+  applied on the foreign key property, `@hasMany` decorator is applied to a kind
+  of a navigational property. If we decide to apply relational decorators at
+  class level in the future, this solution will support that too.
+
+- It does not trigger circular-reference bug in design:type metadata, see
+  https://github.com/Microsoft/TypeScript/issues/27519
+
+- It makes it easy to define a type where all navigational properties are
+  optional. For example: `Product & Partial<ProductRelations>`
+
+  UPDATE: As it turns out, it's not enough to mark all navigational properties
+  as optional. See the discussion in
+  https://github.com/strongloop/loopback-next/pull/2592#discussion_r267600322
+
+### Integration with CrudRepository APIs
+
+The CRUD-related Repository interfaces and classes are accepting a new generic
+argument `Relations` that's describing navigational properties.
+
+Example use in application-level repositories:
+
+```ts
+export class CategoryRepository extends DefaultCrudRepository<
+  Category,
+  typeof Category.prototype.id,
+  CategoryRelations
+> {
+  // (no changes here)
+}
+```
+
+### OpenAPI Schema generation
+
+When building OpenAPI Schema from a model definition, we provide two modes:
+
+- Own properties only
+- Both own properties and navigational properties
+
+The decision is controlled by a new option passed to `modelToJsonSchema` and
+related methods.
+
+```ts
+// own properties only
+const spec = getJsonSchema(Product);
+// include navigational properties too
+const spec = getJsonSchema(Product, {includeRelations: true});
+```
+
+An example of the produced schema:
+
+```js
+{
+  title: 'CategoryWithRelations',
+  properties: {
+    // own properties
+    id: {type: 'number'},
+    // navigational properties
+    products: {
+      type: 'array',
+      items: {$ref: '#/definitions/ProductWithRelations'},
+    },
+  },
+  definitions: {
+    ProductWithRelations: {
+      title: 'ProductWithRelations',
+      properties: {
+        // own properties
+        id: {type: 'number'},
+        categoryId: {type: 'number'},
+        // navigational properties
+        category: {$ref: '#/definitions/CategoryWithRelations'},
+      },
+    },
+  },
+}
+```
+
+To support integration with OpenAPI spec of controllers, where we want to
+reference a shared definition (component schema), we need a slightly different
+schema. Here is an example as produced by `getJsonSchemaRef`:
+
+```js
+{
+  $ref: '#/definitions/CategoryWithRelations',
+  definitions: {
+    CategoryWithRelations: {
+      title: 'CategoryWithRelations',
+      properties: {
+        id: {type: 'number'},
+        products: {
+          type: 'array',
+          items: {$ref: '#/definitions/ProductWithRelations'},
+        },
+      },
+    }
+    ProductWithRelations: {
+      title: 'ProductWithRelations',
+      properties: {
+        id: {type: 'number'},
+        categoryId: {type: 'number'},
+        category: {$ref: '#/definitions/CategoryWithRelations'},
+      },
+    },
+  },
+}
+```
+
+The first schema defines `CategoryWithRelations` as the top-level schema,
+`definitions` contain only `ProductWithRelations` schema.
+
+The second schema contains only `$ref` entry at the top-level, the actual schema
+for `CategoryWithRelations` is defined in `definitions`.
+
+### Controller spec
+
+The last missing piece is integration with controller spec builder.
+
+At the moment, we use the following implementation of the controller method
+`find`:
+
+```ts
+class CategoryController {
+  // constructor with @inject() decorators
+
+  @get('/categories', {
+    responses: {
+      '200': {
+        description: 'Array of Category model instances',
+        content: {
+          'application/json': {
+            schema: {
+              type: 'array',
+              items: {'x-ts-type': Category},
+            },
+          },
+        },
+      },
+    },
+  })
+  async find(
+    @param.query.object('filter', getFilterSchemaFor(Category)) filter?: Filter,
+  ): Promise<CategoryWithRelations[]> {
+    return await this.categoryRepository.find(filter);
+  }
+}
+```
+
+In my proposal, we replace `x-ts-type` extension with a call to
+`getModelSchemaRef`.
+
+```diff
+   schema: {
+     type: 'array',
+-    items: {'x-ts-type': Category},
++    items: getModelSchemaRef(Category, {includeRelations: true})
+   },
+```
+
+I particularly like how this solution makes it easy to use a different mechanism
+for generating model schema. Consider developers using TypeORM instead of our
+Juggler-based Repository for an example. If we implement my proposal, then it
+will be possible to implement a TypeORM extension for LoopBack that will provide
+a different implementation of `getModelSchemaRef` function, one that will use
+TypeORM metadata instead of juggler's LDL.
+
+Later on, we can explore different ways how to enable `includeRelations` flag
+via OpenAPI spec extensions. For example:
+
+```diff
+   schema: {
+     type: 'array',
+-    items: {'x-ts-type': Category},
++    items: {'x-ts-type': Category, 'x-include-relations': true},
+   },
+```
+
+## Follow-up stories
+
+1. Handle circular references when generating model JSON Schema
+
+   --> https://github.com/strongloop/loopback-next/issues/2628
+
+2. Support `schema: {$ref, definitions}` in resolveControllerSpec
+
+   --> https://github.com/strongloop/loopback-next/issues/2629
+
+3. Enhance `getJsonSchema` to describe navigational properties (introduce
+   `includeRelations` option).
+
+   - Add a new `RelationDefinition` property: `targetsMany: boolean`
+   - Implement support for `includeRelations` in `getJsonSchema` & co.
+
+   --> https://github.com/strongloop/loopback-next/issues/2630
+
+4. Implement `getJsonSchemaRef` and `getModelSchemaRef` helpers
+
+   --> https://github.com/strongloop/loopback-next/issues/2631
+
+5) Modify Repository `find*` method signatures to include navigational
+   properties in the description of the return type
+
+   - Add a new generic parameter `Relations` to CRUD-related Repository
+     interfaces and implementations.
+   - Modify the signature `find` and `findById` to return `T & Relations`
+     instead of `T`. If this requires too many explicit casts, then consider
+     using `T & Partial<Relations>` instead, assuming it improves the situation.
+
+   --> https://github.com/strongloop/loopback-next/issues/2632
+
+6. Update `examples/todo-list` to leverage these new features:
+
+   - Define `{Model}Relations` interfaces and `{Model}WithRelations` types
+   - Update `{Model}Repository` implementations to use these new interfaces
+   - Update repositories to include related models: overwrite `find` and
+     `findById` methods, add a hard-coded retrieval of related models.
+   - Update response schemas for controller methods `find` and `findById`
+
+   --> https://github.com/strongloop/loopback-next/issues/2633
+
+7. Replace our temporary poor-man's relation resolver with a real one, as
+   described in https://github.com/strongloop/loopback-next/pull/2124. Update
+   the example app as part of this work.
+
+   --> https://github.com/strongloop/loopback-next/issues/2634

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

@@ -12,9 +12,9 @@ import {
   toJSON,
 } from '@loopback/testlab';
 import {TodoListApplication} from '../../application';
-import {TodoList} from '../../models/';
-import {TodoListRepository} from '../../repositories/';
-import {givenTodoList} from '../helpers';
+import {Todo, TodoList} from '../../models';
+import {TodoListRepository, TodoRepository} from '../../repositories';
+import {givenTodo, givenTodoList} from '../helpers';
 
 describe('TodoListApplication', () => {
   let app: TodoListApplication;
@@ -178,6 +178,20 @@ describe('TodoListApplication', () => {
       .expect(200, [toJSON(listInBlack)]);
   });
 
+  it('includes Todos in query result', async () => {
+    const list = await givenTodoListInstance();
+    const todo = await givenTodoInstance({todoListId: list.id});
+
+    const response = await client.get('/todo-lists').query({
+      filter: JSON.stringify({include: [{relation: 'todos'}]}),
+    });
+    expect(response.body).to.have.length(1);
+    expect(response.body[0]).to.deepEqual({
+      ...toJSON(list),
+      todos: [toJSON(todo)],
+    });
+  });
+
   /*
    ============================================================================
    TEST HELPERS
@@ -218,6 +232,11 @@ describe('TodoListApplication', () => {
     return await todoListRepo.create(givenTodoList(todoList));
   }
 
+  async function givenTodoInstance(todo?: Partial<Todo>) {
+    const repo = await app.getRepository(TodoRepository);
+    return await repo.create(givenTodo(todo));
+  }
+
   function givenMutlipleTodoListInstances() {
     return Promise.all([
       givenTodoListInstance(),