[Spike] Explore the way to resolve inclusion property

[jannyHou]

Description / Steps to reproduce / Feature proposal

A follow-up story of the relation traverse spike #1952

The previous spike story turns out that we could improve our design of describing&defining model properties.

Copied the discussion and proposal from it:

Thanks for all the feedback!

I understand that the best outcome of a spike is a concrete solution, but I am afraid the inclusion story is more complicated than we expect, and it would be better to create separate stories to explorer different potential approaches.

Like what @raymondfeng points out, GraphQL's query system has two features we could borrow:

• use resolver to describe a complicated object and calculate its value
• use different query types to describe the filter of a model

And a overall design question to consider (from @bajtos ) :

How are we going to generate JSON Schema and OpenAPI schema for payloads containing both model properties (Customer's id, name, email) and data of related model (Customer's orders, addresses, etc.). At the moment, we are leveraging @Property metadata. If we choose a different approach for describing model+relations in TypeScript, then we also need a new way how to obtain JSON Schema for such new types.

Here is a proposal of the next steps:

• Explorer the approach "Define relation property as a resolver"
  • The prototype would be:

       import { TodoList } from './TodoList';
       class Todo extends Entity {
          @inclusion()
          TodoList: ()=>TodoList
        }

• Explorer the approach "Use a type wrapper to prevent the circular reference problem"
  • The prototype would be:

        export type Related<T> = T | undefined;
        class Todo extends Entity {
          // ...
          todoList: Related<TodoList>;
        }
        class TodoList extends Entity {
         // ...
          todos: Related<Todo>;
        }

• Explorer "Generate decorator based filter types"
  • Inspired by the query/mutation type in GraphQL, I am trying to define different partial model types described by decorators
  • The prototype:

        // model.ts file
       class Todo extends Entity {
         @property({name: 'id', type: 'string'})
         id: string,
         @property({name: 'desc', type: 'string'})
         desc?: string
         @inclusion()
         TodoList: ()=>TodoList
       }
       // model.query.ts file
       type TodoMutation = Pick<Todo, a union of properties decorated by @property except id>
       type TodoQuery = TodoMutation & Inclusion<Todo>

• Explorer the approach "Define multiple model classes for different purpose"
  • Prototype:

      class Customer extends Entity {
        // ...
      }
    
      class CustomerWithRelations extends Customer {
       orders?: Order[]
      }

[bajtos]

use resolver to describe a complicated object and calculate its value
Explorer the approach "Define relation property as a resolver"

As I understand LB4 design, the Model class is describing shape of the data only, it does not deal with obtaining that data (fetching it from a database or resolving a model relation). It's the Repository that deals with resolving model relations and fetching related data.

We are following this design in HasMany/BelongsTo relation too. You can think of HasManyRepository.prototype.find and BelongsToAccessor as resolver functions.

I feel that your proposal proposal to move resolvers to model classes is violating this design constraint and we should investigate different approaches.

Re-posting the code sample from the problematic proposal:

   import { TodoList } from './TodoList';
   class Todo extends Entity {
      @inclusion()
      TodoList: ()=>TodoList
    }

[dhmlau]

@jannyHou , could you please add the acceptance criteria? Thanks!

[bajtos]

Relevant discussions: #2442 #2256

Reposting #2442 (comment)

It sounds like we don't want to mix the property and belongsTo decorators. The solution provided by @raymondfeng might work but it adds redundant info to the db and schema which will be confusing to the user.

Here is one reason why we should not mix the property and the relation decorator:

• When creating a new model instance, or updating an existing model instance, we don't want the data type to include the navigational property. addressRepo.create(data) does not support updating the AddressBook instance we are belonging to, therefore data type should exclude addressBook property.
• Navigational properties are needed only when querying data via repo.find.
• This difference must be propagated to OpenAPI schemas used by REST API endpoints.

For long term, I would like to propose following API at conceptual level (implementation details will be most likely different):

// Relations are defined at model-level
@belongsTo(() => AddressBook)
@model()
class Address extends Entity {
  // the foreign key is defined explicitly
  @foreignKey(() => AddressBook, 'id')
  @property({
    length: 36,
    postgresql: {
      dataType: 'uuid',
    },
  })
  addressBookId: string;
}

// A custom class or interface describes format
// of data returned by a query
@model()
class AddressWithRelations extends Address {
  @property()
  addressBook?: AddressBook;
}

// Repository API
class DefaultCrudRepository {
  find(filter: Filter<Address>): Promise<AddressWithRelations>;
  create(data: Address): Promise<Address>;
  // ...
}

[bajtos]

Generate decorator based filter types"
Inspired by the query/mutation type in GraphQL, I am trying to define different partial model types described by decorators
The prototype:

   // model.ts file
  class Todo extends Entity {
    @property({name: 'id', type: 'string'})
    id: string,
    @property({name: 'desc', type: 'string'})
    desc?: string
    @inclusion()
    TodoList: ()=>TodoList
  }
  // model.query.ts file
  type TodoMutation = Pick<Todo, a union of properties decorated by @property except id>
  type TodoQuery = TodoMutation & Inclusion<Todo>

The part Inclusion<Todo> is problematic to implement at the type level. Here is what I tried:

     @model()
      class Product {
        @property()
        name: string;
      }

      @model()
      class Category {
        @property()
        name: string;
      }

      // tslint:disable-next-line:no-any
      type Include<Target, As extends keyof any> = {
        readonly [P in As]?: Target
      };

      type CategoryQuery = Category & Include<ProductQuery[], 'products'>;
      type ProductQuery = Product & Include<CategoryQuery, 'category'>;

TypeScript is returning the following errors:

ts(2456): Type alias 'CategoryQuery' circularly references itself.
ts(2456): Type alias 'ProductQuery' circularly references itself. 

The following definition works well:

      type CategoryQuery = Category & {readonly products?: ProductQuery[]};
      type ProductQuery = Product & {readonly category?: CategoryQuery};

Personally, I like this approach better because it's easier to understand.

---

Regarding TodoMutation type:

Please note that we need multiple types to describe mutations.

• A type used for creating new instances, this type typically excludes properties set by the database (e.g. id, _rev, etc.)

  type NewTodo = Pick<Todo,  Exclude<keyof Product, 'id'>>;

• A type used for performing a patch (partial update) operation, this type has all properties optional and probably should exclude properties controlled by the database

  type TodoPatch = Partial<NewTodo>;

• A type used to perform a full replace of the entire model instance, this is the original model.

Now that we know how to deal with types, we need to figure out how to generate OpenAPI schema for these different models. That's out of scope of this spike though, see #1722 and related issues.

[bajtos]

In 312be08 (see https://github.com/strongloop/loopback-next/tree/spike/relation-inclusion-properties-as-model), I looked into approach based on defining a new model class for model with relation links.

@model()
export class TodoListImage extends Entity {
  // ...
}

@model()
export class TodoListImageWithRelations extends TodoListImage {
  @property(() => TodoList)
  todoList?: Value<TodoList>;
}

While this approach makes it easy to generate OpenAPI spec (once we fix our generator to correctly handle circular references), it's not great in many other aspects.

• In Repository methods, we want to say that find methods return model instances with optional relation links. This is non-trivial to express, because we cannot enforce types like TodoListImageWithRelations to have all properties optional.
• I feel this approach is also introducing too much of redundancy. We already know what are the relation methods and their target types, this information is provided in relation definition. Surely our model JSON schema builder can take it from there?
• We have two distinct classes: TodoList and TodoListImageWithRelations. I find this as going against the spirit of JavaScript and its dynamic nature. Ideally, relations should be added on top of an existing TodoList instance, there is no need to create a new class. Especially when writing the code in vanilla JavaScript.

Next, I'd like to investigate approach where the relation links are described in an interface and JSON schema is built from relation metadata.

[bajtos]

Next, I'd like to investigate approach where the relation links are described in an interface and JSON schema is built from relation metadata.

I am quite pleased with the results, see
https://github.com/strongloop/loopback-next/tree/spike/relation-inclusion-properties-as-iface

Now I need to write some dev documentation & describe the proposed changes, to make the PoC code ready for review.

[bajtos]

Opened a pull request showing a PoC and creating space for further discussion: #2592

[bajtos]

I am closing the spike as done.

Cross-posting #2592 (comment)

I have converted Inclusion of related models #1352 into an Epic and added the following new stories:

• Fix repository-json-schema to handle circular references Fix repository-json-schema to handle circular references #2628
• Allow controllers to provide definitions of models referenced in operation spec Allow controllers to provide definitions of models referenced in operation spec #2629
• Enhance getJsonSchema to describe navigational properties Enhance getJsonSchema to describe navigational properties #2630
• Implement getJsonSchemaRef and getModelSchemaRef helpers Implement getJsonSchemaRef and getModelSchemaRef helpers #2631
• Modify Repository find* methods to include navigational properties Modify Repository find* methods to include navigational properties #2632
• Add navigational properties to examples/todo-list Add navigational properties to examples/todo-list #2633
• Spike: Resolver for inclusion of related models Spike: Resolver for inclusion of related models #2634

[AnanthGopal]

Hi @bajtos ,

is released relation link feature? We are waiting to use this feature.

[bajtos]

@AnanthGopal not yet. We need to implement the issues listed in my previous comment, plus any additional items discovered by #2634. Would you like to help us and contribute some of the work needed to make this feature happen? We are happy to help you along the way. See https://loopback.io/doc/en/contrib/index.html and https://loopback.io/doc/en/lb4/submitting_a_pr.html to get started.