Extension: CrudRestController

[bajtos]

This pull request adds CrudRestController extension as described in #2736. See the top-level EPIC describing the big picture & plan: #2036 From model definition to REST API with no custom repository/controller classes.

Example use of the new controller:

1. Create a base REST CRUD controller class for your model.

   const CrudRestController = defineCrudRestController<
     Product,
     typeof Product.prototype.id,
     'id'
   >(Product, {basePath: '/products'});

2. Create a new subclass of the base controller class to configure repository
   injection.

   class ProductController extends CrudRestController {
     constructor(@repository(ProductRepository) repo: ProductRepository) {
       super(repo);
     }
   }

3. Register the controller with your application.

   app.controller(ProductController);

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• API Documentation in code was updated
• Documentation in /docs/site was updated
• Affected artifact templates in packages/cli were updated
• Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

[bajtos]

Added docs and implemented the missing operations.

The pull request is ready for review. /cc @raymondfeng @strongloop/loopback-maintainers

[raymondfeng]

Is it feasible to define a CrudRestController from a Repository (instance or class) directly? Ideally, if a developer adds a repository using lb4 repository to connect a model to a data source, we should allows him/her to expose the repository as CRUD REST APIs out of box.

[bajtos]

That's an interesting idea, we can explore it later if you like. This new module is in 0.x version, so even if we need to make breaking changes to support your idea, then that's ok.

Our current plan is to allow developers to go from lb4 model to REST API with no custom repository class needed - see #2036

The idea is that the user will tell us that let's say Product model should be exposed via CRUD REST API (as opposed to e.g. KeyValue REST API) using datasource db, and the framework will take care of the rest.

[raymondfeng]

The current implementation requires a repository class to be defined. I added a few comments inline. I'm open to incremental improvements.

[nabdelgadir]

Is it possible to make this optional and use a default basePath if one isn't provided?

[bajtos]

That would require us to implement some logic to convert a model name (e.g. Product) to a path (e.g. /products). This gets quickly tricky - do we want /people or /persons for Person model?

I'd prefer to keep basePath required for this initial implementation, we can make it optional later.

[bajtos]

Perhaps the purpose of this field is not clear enough and we should improve the api docs? Can you suggest a better wording? Would an example help?

[nabdelgadir]

That would require us to implement some logic to convert a model name (e.g. Product) to a path (e.g. /products). This gets quickly tricky - do we want /people or /persons for Person model?

I was thinking we could use something similar to how the controller generator does it which in this scenario would make it /people.

I'd prefer to keep basePath required for this initial implementation, we can make it optional later.

That's fine with me 👍

Perhaps the purpose of this field is not clear enough and we should improve the api docs? Can you suggest a better wording? Would an example help?

It's clear to me. My suggestion was so that we could make options optional and be able to exclude it from the function call:

   const CrudRestController = defineCrudRestController<
     Product,
     typeof Product.prototype.id,
     'id'
   >(Product, {basePath: '/products'});

vs.

   const CrudRestController = defineCrudRestController<
     Product,
     typeof Product.prototype.id,
     'id'
   >(Product);

[bajtos]

I was thinking we could use something similar to how the controller generator does it which in this scenario would make it /people.

Makes sense! I had mixed experience with auto-generated base path for CRUD endpoints in LB3. Some people found the pluralization rules confusing ("people" vs. "persons"), some wanted different mechanism for converting pascal-cased model names (e.g. "ProductReview") to HTTP paths (e.g. "/ProductReviews", "/product-reviews", etc.).

Let's keep basePath optional for now and wait until we learn more about this area while working on #2036.

[raymondfeng]

Is it required? IIUC, you want to skip Repository class so that a model can be used to expose REST CRUD APIs with a datasource.

[bajtos]

A custom repository class is not required, this is just to make the documentation easier to follow.

This new component is meant to be a low-level building block to support #2036, I am expecting that most people most people will use #2036 and don't leverage the current API directly.

For the initial docs, I choose a scenario that's easy to understand (because it's using existing concepts) and that's matching the scenario used for testing.

Depending on the outcome of the next tasks (especially the spike #2738), we may add more "meat" to this component and update or even rework the README along the way.

[raymondfeng]

If we skip the repository, can we inject a datasource instead?

[bajtos]

Yes, I believe injecting a datasource should be a viable option too. The actual implementation does not prescribe any IoC setup, the base controller class is expecting to receive a repository instance. It's up to the code extending the controller class to decide how to obtain the repository.

I believe the following solution should work as an alternative approach too:

class ProductController extends CrudRestController {
  constructor(
    @inject('datasources.db') dataSource: juggler.DataSource,
  ) {
    const repo = new DefaultCrudRepository<
      Product,
      typeof Product.prototype.id
    >(Product, db);

    super(repo);
  }
}

@raymondfeng Would you like me to open a new issue to add a test & documentation for this approach?

[raymondfeng]

I left a few more comments, which can be addressed in follow-up tasks.

[jannyHou]

👍 LGTM, a few comments about the status code.

[bajtos]

@raymondfeng @nabdelgadir @jannyHou thank you for the review and thoughtful comments. I am going to land this pull request as it is now. Please let me know if you would like me to improve any parts of this initial implementation as part of the story #2736.