search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.07k forks source link

Documentation explaining Multiple Repositories design decision #2110

Open acrodrig opened 5 years ago

acrodrig commented 5 years ago

Feature proposal

Starting to play/understand lb4 ...

I am writing code that inserts several model/entity instances into a DB. My code constantly needs to create repository instances based on the model in question. It seems overkill.

Can you explain in the documentation why this design decision to have one repository per class/model/entity was chosen? Or is it possible to have a multi-class repository? The cli does not seem to allow it.

Thanks.

bajtos commented 5 years ago

cc @raymondfeng

I am writing code that inserts several model/entity instances into a DB. My code constantly needs to create repository instances based on the model in question. It seems overkill.

Can you explain in the documentation why this design decision to have one repository per class/model/entity was chosen? Or is it possible to have a multi-class repository? The cli does not seem to allow it.

I think we need to distinguish two aspects.

(1) We have one repository class per model to make it easy for app developers to add additional behavior to their repository classes. For example, a Product repository may want to introduce API for looking up a product by a name.

export class ProductRepository extends DefaultCrudRepository<
  Product,
  typeof Product.prototype.id
> {
  constructor(@inject('datasources.db') dataSource: juggler.DataSource) {
    super(Product, dataSource);
  }

  findByName(name: string): Promise<Product | undefined> {
    return this.findOne({where:{name}});
  }
}

While this is our recommended approach, advanced users are free to structure their application differently. For example, they can use DefaultCrudRepository class directly.

const productRepo = new DefaultCrudRepository<Product, typeof Product.prototype.id>(
  Product, 
  db
);

(2) We are creating a new repository instance for each incoming request. From my experience, this is a usual approach in systems built using Dependency Injection. The Repository class is a thin wrapper that's cheap to construct. By having one repository instance for each incoming request, the repository can use instance-level properties to e.g. store cached data.

Again, this is a convention, not a requirement. You can reuse the same repository instance for all incoming requests.

Having wrote that, it is not possible to use a single DefaultCrudRepository instance to work with multiple models. It is possible to write a repository class that provides data access for multiple models, but such API will be more complex. It's not enough to say repo.create(data), you also need to tell the repository which model class to use, e.g. repo.create(Product, data) or repo.findById(Product, 123).

LoopBack 4 was designed with extensibility in mind. If a repository supporting multiple models is something you need, then feel free to implement such repository as your own loopback extension.

Our CLI does not allow extensions to contribute custom repository classes yet, see https://github.com/strongloop/loopback-next/issues/2149

Can you explain in the documentation why this design decision to have one repository per class/model/entity was chosen?

@acrodrig Where in our documentation would you like to see this information? Could you please advise us where to put it?

acrodrig commented 5 years ago

Thanks for your answer. Regarding the comment above:

On a closing note I feel more or less the same way about Controllers/REST code. I feels full of boiler-plate code, whereas in LB3 it was completely out of sigh and I could still extend.

I do appreciate the efforts of this new version and it feels cleaner in many ways. Let me know if there is any way I can contribute.

bajtos commented 5 years ago

However ... I just wished my code was not "littered" with repository classes that are almost exact copies of each other. It seems to go against the DRY principle and adds to the complexity of the project in exchange for extensibility that I might not need. I wish we could have both things (extensibility without boilerplate code).

You are not the first one asking for that :)

In https://github.com/strongloop/loopback-next/issues/2036, we are discussing how to create both a repository and a controller, avoiding boiler-plate. What are your requirements - would you like to avoid boilerplate controller code too, or is it just the repository that's bothering you?

Let's continue this discussion to #2036 please.

Regarding (2) it is good to know that the repository class is a thin (and I presume you mean inexpensive to construct) wrapper. That should be in the documentation. I was going through some lengths to understand if I should cache it or not.

Finally, I think the best place to put it is in the Repositories section. There is already a bit there about extensibility, but it would be good to mention 1) why it was taken out of the entity itself (i.e. no more entity.save(), 2) computational cost of constructing repositories, and 3) best practices.

Makes sense. Would you mind contributing this documentation enhancement yourself? I think you should have enough information by now and since you raised these questions, you are best equipped to answer them in a way that will be useful to future readers like yourself.

On a closing note I feel more or less the same way about Controllers/REST code. I feels full of boiler-plate code, whereas in LB3 it was completely out of sigh and I could still extend.

See #2036 :)