HasManyRepository should implement EntityCrudRepository?

[Salketer]

Suggestion

The HasManyRepository is missing the *ById, exists and count methods. They would prove to be very handy...

Use Cases

Currently we have to forge the where clause to target entities by ID when working on a relation. This is error prone and more work than it should be.

For the *ById methods:

While routes like GET /project/12/milestone/3 could definitely be narrowed down to /milestone/3
Relation can be used in a multi-tenant architecture like so in a controller:

    this.tenantRepository.milestones(currentUserProfile.tenantId);

To find a milestone, making sure we return it only if it is owned by the current tenant:

async findById(
    @inject(SecurityBindings.USER) currentUserProfile: SscUserProfile,
    @param.path.number('id') id: number,
    @param.filter(CrmContact, {exclude: 'where'})
    filter?: FilterExcludingWhere<CrmContact>,
  ): Promise<CrmContact> {
    filter = filter || {};
    filter.limit = 1;
    const newFilter = Object.assign({}, filter, {where: {id}});
    return (await this.tenantRepository.milestones(currentUserProfile.tenantId).find(newFilter))[0];
  }

This gets cumbersome even if there's better ways to build the where filter like using constraints (?).

For the count method

Another use case if for the count method. We cannot efficiently use the relation to count as we need to use find for it! A worker could be to use the right repository and constrain it to the relation key...

// ... the Where filter should be modified not just thown away but meh
   return this.mileStoneRepository.count({tenantId:currentUserProfile.tenantId});
//or
   const newFilter = {where,fields:{id:true}}; //include the least fields possible.
   return (await this.tenantRepository.milestones(currentUserProfile.tenantId).find(newFilter)).length;

Examples

return this.tenantRepository.milestones(currentUserProfile.tenantId).findById(id);
return this.tenantRepository.milestones(currentUserProfile.tenantId).count(where);

Acceptance criteria

• implement count, exists and *ById for hasManyRepository
• implement exists and *ById for hasOneRepository

[agnes512]

@Salketer thanks for the suggestion! I think these expected behaviors is similar to what inclusion does for relations.
For example, for request

await this.tenantRepository.milestones(currentUserProfile.tenantId).find(newFilter)

I assume tenantRepository is the hasMany repository. And Project has many Users, and the relation is named milestones.
If you do

const filter = {
  where:{id: ...}
  include: [
    {
      relation: 'milestones',
      scope: {
        _ newFilter_,
      },
    },
  ],
};

await projectRepository.findById(filter);
// or projectRepository.find(filter)

Notice we are using projectRepository, not the hasManyRepository class. Under the hood, it finds the related models(User) via relation milestones, then apply the newFilter to those User instances it just found. The different is that the result it returns is
a Project instance including Users. e.g

{
    projectId: 1,
    description: 'a project instance',
    milestones: [
       {userid: .., des: 'a user instance'},
       {.....}
    ]
}

And you need to extract the information you need from the result.

I can see that how your suggestion simplifies the hasManyRepository. I am not sure if it was an internal decision to keep it that way. What do you think about traversing data with inclusion? Is it closer what you suggested?

[Salketer]

Thanks @agnes512 for your thoughts. The problem with your suggestion is that it queries the source model while including the model we want. To my understanding, the relation Repository is there to prevent the need to use the source model if you just need the related ones.

It also does not answer the concerns for the count method.

I realize my example wasn't that clear. In the situation I was pointing out, the tenant Repository hasMany projects, and many milestones. While milestones also belong to projects.

[raymondfeng]

I think it should be good to implement count and other missing methods.

[agnes512]

My bad, I thought you were talking about traversing nested relations via hasMany repository 🙈

I will update the acceptance criteria shortly. Feel free to submit a PR tho😄

[bajtos]

We have intentionally kept the initial implementation of relation repositories small, to avoid implementing APIs that nobody will use and we will have to maintain.

While I am not sure if relations are robust enough to support multi-tenancy, I guess the use case described by @Salketer is reasonable and it makes sense to implement the suggested methods like count, exists and *ById.

To my understanding, the relation Repository is there to prevent the need to use the source model if you just need the related ones.

This is not necessarily true. Take the request GET /project/12/milestone/3 as an example. In order to verify that project 12 actually exists, we may need to query projects first, before we proceed to query related milestones. See the discussion in #1718 for more details.

[stale]

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.