Inclusion of related models [MVP] #1352
Comments
|
I added the following acceptance criteria:
I think we need a spike to determine how to actually implement relation traversal, because right now, a repository for source model does not necessarily know how to obtain an instance of a repository for accessing the target model. /cc @dhmlau |
dhmlau
added
Relations
|
Cross-posting from #1939. I realized this PR is just a starting point to fully support include. There are a few issues:
|
|
@dhmlau The inclusion of related models should work with |
|
I was thinking about this problem and perhaps we can implement this at LB4 Repository level? Let's say "Customer" has many "Order" instances, the relation is called "orders" and we want to fetch a customer with their orders. The way how inclusion works in juggler and LB 3.x, such request creates multiple queries:
For LoopBack 4, I am thinking about making the relation lookup more explicit and letting the Repository to specify a lookup table for inclusion. Similarly to how we explicitly build HasManyRepositoryFactory and BelongsToAccessor now. A mock-up usage to illustrate what I mean: export class TodoListRepository extends DefaultCrudRepository<
TodoList,
typeof TodoList.prototype.id
> {
public readonly todos: HasManyRepositoryFactory<
Todo,
typeof TodoList.prototype.id
>;
constructor(
@inject('datasources.db') dataSource: juggler.DataSource,
@repository.getter(TodoRepository)
protected todoRepositoryGetter: Getter<TodoRepository>,
) {
super(TodoList, dataSource);
this.todos = this._createHasManyRepositoryFactoryFor(
'todos',
todoRepositoryGetter,
);
////// THE FOLLOWING LINE IS NEWLY ADDED
this._registerHasManyInclusion('todos', todoRepositoryGetter);
}
}
export class TodoRepository extends DefaultCrudRepository<
Todo,
typeof Todo.prototype.id
> {
public readonly todoList: BelongsToAccessor<
TodoList,
typeof Todo.prototype.id
>;
constructor(
@inject('datasources.db') dataSource: juggler.DataSource,
@repository.getter('TodoListRepository')
protected todoListRepositoryGetter: Getter<TodoListRepository>,
) {
super(Todo, dataSource);
this.todoList = this._createBelongsToAccessorFor(
'todoList',
todoListRepositoryGetter,
);
////// THE FOLLOWING LINE IS NEWLY ADDED
this._registerBelongsToInclusion('todoList', todoListRepositoryGetter);
}
}Implementation-wise, the base Repository implementation should process class DefaultCrudRepository {
// ...
_inclusions: {[key: string]: InclusionHandler};
async find(filter?: Filter<T>, options?: Options): Promise<T[]> {
const include = filter.include;
filter = Object.assign({}, filter, {include: undefined});
const models = await ensurePromise(
this.modelClass.find(filter as legacy.Filter, options),
);
const entities = this.toEntities(models);
for (relationName of include) {
const handler = this._inclusions[relationName];
if (!(handler)) throw new HttpErrors.BadRequest('Invalid inclusion.');
await handler.fetchIncludedModels(entities, key);
}
}
_registerHasManyInclusion(relationName, targetRepoGetter) {
const meta = this.entityClass.definition.relations[relationName];
this._inclusions[relationName] = new HasManyInclusionHandler(meta, targetRepoGetter);
}
}
class HasManyInclusionHandler {
constructor(public relation, public repositoryGetter) {}
fetchIncludedModels(entities, relationName) {
// see https://github.com/strongloop/loopback-datasource-juggler/blob/f0a6bd146b7ef2f987fd974ffdb5906cf6a584db/lib/include.js#L609-L634
const objIdMap2 = includeUtils.buildOneToOneIdentityMapWithOrigKeys(entities, this.relation.keyFrom);
const filter = {};
filter.where[relation.keyTo] = {
inq: uniq(objIdMap2.getKeys()),
};
const targets = findWithForeignKeysByPage(
await this.repositoryGetter(), filter, this.relation.keyTo, 0);
const targetsIdMap = includeUtils.buildOneToManyIdentityMapWithOrigKeys(
targets, this.relation.keyTo);
// store targets to entities, e.g.
// set entities[0].orders to the target with "customerId=entities[0].id"
} |
|
Hello @bajtos , what's the update on this ? When can we expect this feature in LB4. We are using LB4 since its GA, but this missing feature is making us to not to move to production. Majorly due to performance issues. In order to fetch related models, we have to query other repositories manually to fetch data which in case of find method is too many DB hits. |
|
Is this issue still there? I'm trying to execute a code similar to the todo-list tutorial and it is not working... using MySQL... |
|
Hi, Do you know how to do a where filter on data retrieved by the inclusion resolver ? |
|
@sestienne Hi!
Here is an example for including a
Yes! We've tested it over SQL and NoSQL databases. You can check the Querying related models section in each relation for the usages on our site. You can also check out the acceptance test if you're interested. |
|
@agnes512 Thanks a lot for your response. |
|
@agnes512 So, I launch the todolist example and try request like : |
|
@sestienne sorry I didn't realize that you were trying to filter out In url Unfortunately we don't support inclusion with custom |
|
OK thanks, do you know any planned date for this feature ? |
|
The task #3453 is out of the scope of this MVP. So it probably won't be up that soon. |
|
https://stackoverflow.com/q/58742689/7628381 |
|
@pratikjaiswal15 Hi, I believe that the code you showed in the SO has already setup the inclusion resolver for the relation If you'd like to do CRUD operations at the repository level, here are some examples of usage of inclusion resolver in our test case. If you'd like to use it with REST endpoints, set up the controller as usual, and the should give you the same result as what it gets at the repository level. Reference: inclusion resolver blog. I've update the corresponding docs in #4007. But it seems like the site hasn't updated yet. Let me know if you need more help, thanks. |
|
Thank you for the response. I have just updated loopback-cli version from 1.21.4 to 1.24.0. So for new project inclusion resolver is working fine but for the old project which built with 1.21.4 version is giving error following line
There are some issues with typescript. How to resolve it . thank you |
|
@pratikjaiswal15 the error is because he older project is generated by 1.21.4 and so is the app dependencies. If you want to use the latest cli, you need to upgrade dependencies to match the newer CLI ( you can force it or upgrade deps to versions matching cli 1.24.0. Command After deps upgrade, you might have to fix the project manually for any errors. |
|
Thank you for your reply. Inclusion resolver with has many relations is working pretty well. But with belongs to the relation there are some conservations. In my example, the retailer belongs to users.
`But this extension is not working. It is simply giving retailer data with id = 1 and not any user data. |
|
And another question is how to include three or more models because This extension is giving internal server error. |
|
Sir, does loopback's current version supports inclusion of three models where a model has has many relation with other two? If not then what is alternative. |
|
@pratikjaiswal15 As for question 2, yes it is possible to include multiple relations. with repositories, you can do: Notice that you need to make sure all relation names are unique. And thanks for your comments. They are good suggestions of improving our docs! |
|
Thank you very much. Both solutions working. |
|
@raymondfeng agnes512 hello sir,can you help me, i new in loopback 4 and i stiil get stuck, how to include three or more models : i just have relation for sections thanks ` |
|
@muhrifai7 have you tried the solution that I post above? i.e, modify your Usually we don't set the inclusion in the endpoint. We hand it to the filter to take care to make the endpoint flexible. For example, with the code above, you can include You can also include multiple relations with request: |
thanks for your response sir, it still doesnt works, because the relation is Page hasMany Section hasMany Content, so i want to retrive the data like this :
not like this
|
|
@muhrifai7 I see. So you would like to traverse nested relations not multiple relations. Currently we don't support this feature yet. See #3453. Feel free to join the discussion there :D |
|
Hi, Is it possible to replace the optional attribute with a required attribute for relationships included in openApi? And the Id, he returned by the Api OpenApi.json To avoid in my client to make conditions everywhere to know if "roles" is not undefined, while in any case an empty array will be returned if there is no relation. OpenApi : thanks |
|
@KyDenZ If I understand correctly, your expected schema with related items is "UserWithRelations": {
"title": "UserWithRelations",
"description": "(Schema options: { includeRelations: true })",
"properties": {
"id": {
"type": "number"
},
"name": {
"type": "string"
},
"roles": {
"type": "array",
"items": {
"$ref": "#/components/schemas/RoleWithRelations"
}
}
},
// DIFFERENCE
// `roles` should be added to `required`
"required": [
"name", "roles"
],
"additionalProperties": false
},? |
|
That's exactly, with the Id also in required, but that it is generated automatically |
At the moment, the schema describes the responses that can be returned by the server. If we modify the schema and make a navigational property like Please note that by default, LoopBack does not include any related models. Assuming How do you envision to implement this part? Using the current features provided by LoopBack, my recommendation is to use function composition to apply any tweaks to the model schema produced by LoopBack. For example, you can write a function like this: function withRequiredProps(propNames: string[], schemaRef: SchemaRef) {
const result = _.deepClone(schemaRef);
const title = schemaRef.$ref.match(/[^/]+$/)[0];
const modelSchema = result.components.schemas[title];
if (!modelSchema.required)
modelSchema.required = [];
modelSchema.required.push(...propNames);
return result;
}Then you can use it as follows: @get('/users', {
responses: {
'200': {
description: 'Array of Users model instances',
content: {
'application/json': {
schema: {
type: 'array',
// LOOK HERE
items: withRequiredProps(
['roles'],
getModelSchemaRef(User, {includeRelations: true}),
),
},
},
},
},
},
}) |
|
@bajtos If it interests someone I just had to change some elements for it to work: import { SchemaRef } from '@loopback/rest';
import * as _ from 'lodash';
export function withRequiredProps(propNames: string[], schemaRef: SchemaRef) {
const result = _.cloneDeep(schemaRef); // Replace deepClone by cloneDeep
const title = schemaRef.$ref.match(/[^/]+$/)![0]; // Add !
const modelSchema = result.definitions[title]; // Replace components.schemas by definitions
if (!modelSchema.required) modelSchema.required = [];
modelSchema.required.push(...propNames);
return result;
}It would be interesting to propose as in loopback 3 a "scope" attribute which allows to automatically include relations. This makes it possible to generate a schema with the required relationships for all requests. For a specific route, I like your solution. This function should be added in @ loopback/rest and later supported nested includes, when loopback will support this feature. |
|
This epic is done! Thanks @agnes512 @nabdelgadir @bajtos! |
Description / Steps to reproduce / Feature proposal
Follow up task for PR #1342
Inclusion of related models (same as LB3)
For example,
Customermodel has{include: ['order']}. When query onCustomer, the result should includes theOrderarray.Duplicates:
Follow-up stories:
Acceptance Criteria
MVP scope
2019Q3
keyFromto resolved relation metadata AddkeyFromto resolved relation metadata #3441findByForeignKeyshelper (initial version) AddfindByForeignKeyshelper (initial version) #3443InclusionResolverconcept IntroduceInclusionResolverconcept #3445DefaultCrudRepositoryInclude related models inDefaultCrudRepository#3446todo-listexample to use inclusion resolver Updatetodo-listexample to use inclusion resolver #34502019Q4
lb4 relationCLI Add inclusion resolvers tolb4 relationCLI #3451resolve{Relation}MetadataVerify relation type inresolve{Relation}Metadata#3440Out of MVP scope
inqsplitting infindByForeignKeysSupportinqsplitting infindByForeignKeys#3444See #3585 for the full list.
The text was updated successfully, but these errors were encountered: