[WIP] [RFC] feat: Add REST mapping for repository interfaces

[raymondfeng]

Description

This PR illustrates how we can use base controller classes to expose Repository interfaces over REST so that we can provision REST APIs for persistent models.

Related issues

• Depends on fix(rest): Improve rest metadata inheritance #746

Checklist

• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide

[bajtos]

Thank you for this pull request, I think it's a good illustration of how to expose repositories via REST 👍

In my (very strong) opinion, we should not be packaging these controllers for consumption via inheritance. Instead, we should write CLI tool to generate these controllers.

I have already explained this in my older comment #485 (comment), which I am cross-posting below

You may ask why to code-generate all those controller files? In my experience, one of the most frequently sought feature of LB 3.x is the ability to control and customize the public API. Users start with a wide built-in CRUD API provided by the LoopBack, but soon enough then need to lock the API down, expose only subset of endpoints, and customize how exactly these endpoints work.

In LoopBack 3.x, this was a complex process that required hooks at many different levels (middleware/strong-remoting phases, remoting hooks, operation hooks) and unintuitive API like disableRemoteMethodByName.

In my proposal, customization of the REST API is as easy as editing the generated code.

• Need to hide a built-in (generated) endpoint? Just delete the controller method!
• Need to modify the request arguments before executing the underlying CRUD method? Add that code into the controller method.
• Need to add more parameters to a built-in (generated) endpoint? Just modify the controller method.
• Need to post-process the value returned from the model? Add the transformation after you awaited the result of the model method.

[kjdelisle]

+1 to @bajtos ' review

[raymondfeng]

Using the base controllers and having CLI generate sub controllers do not have to be exclusive.

@bajtos All of your requirements can be achieved with the base controllers too. See:

https://github.com/strongloop/loopback-next/blob/d63bf883643b57c18916db84f15242e4828758b3/packages/repository-rest/test/acceptance/crud-controller.ts#L50

There are a few possible cases:

1. Generate or add your own controller to extend base CRUD controllers
2. Mixin CRUD endpoints to a controller class
3. Automagically build controllers for repositories and register them with LoopBack (I guess this is the part that you have concerns).

[raymondfeng]

@bajtos @kjdelisle PTAL

[bajtos]

An example to disable an HTTP route exposed by the base method
https://github.com/strongloop/loopback-next/blob/d63bf883643b57c18916db84f15242e4828758b3/packages/repository-rest/test/acceptance/crud-controller.ts#L73-L79

Can you confirm that this is removing the endpoint from the OpenAPI spec too?

An example to add pre/post processing logic for the base method
https://github.com/strongloop/loopback-next/blob/d63bf883643b57c18916db84f15242e4828758b3/packages/repository-rest/test/acceptance/crud-controller.ts#L60-L71

What if users want to add additional endpoint parameters? How can they do that? Do they need to copy all existing decorators from the base class method? I find that too difficult compared to the alternative approach where the controller source file in user's project already contains all decorators and is ready to be modified.

---

Sorry @raymondfeng, I think you did not understand my point. I agree that using the base controllers and having CLI generate sub controllers do not have to be exclusive.

What I am saying is that "base controllers" approach require too much effort to develop, maintain and support. It also has a steeper learning curve, because developers must understand how the base controllers are built internally, what are the mechanisms for controlling the API (e.g. hiding methods) and customizing the behavior (e.g. via remoting hooks or sequence action(s) or overriding base controller methods). Compare this with the alternative approach where the full source code of controllers is generated. Any JS developer should be able to immediately understand what each controller method does under the hood and how it can be tweaked (by editing its body) or disabled (by removing the method).

For me, just the steeper learning curve is enough justification for rejecting this approach.

If that's not enough, then please consider how much difficulties we already have in supporting all existing features in LB 3.x and all database connectors, with a team of only about five people. Let's not repeat the mistakes we made in LB 1.x-3.x that led to this struggle, and make the new codebase less demanding for maintainers time.

Let me quote Antoine de Saint Exupéry:

In anything at all, perfection is finally attained not when there is no longer anything to add, but when there is no longer anything to take away, when a body has been stripped down to its nakedness.

and Edsger Dijkstra:

My point today is that, if we wish to count lines of code, we should not regard them as “lines produced” but as “lines spent”: the current conventional wisdom is so foolish as to book that count on the wrong side of the ledger.

My point here is that by avoiding base controllers, we can avoid a lot of extra code that we will have to maintain (API for hiding methods inherited from base controllers, pre/after/onError hooks).

[bajtos]

(In case it's not clear from my previous comment, I am proposing to close this pull request as rejected.)

[raymondfeng]

@bajtos Thank you for the comments. I have a different view here:

In summary, there are two styles to promote a convention/pattern to map repository interfaces to REST endpoints:

• By template (code generation from the template, enforced at build time)
• By inheritance (inheritance from the base class, enforced at both build and runtime)

What are in common is that we need come up a template to define the mappings from repository methods to REST operations. Please confirm if you agree.

The difference is the mechanism. It's quite similar as C++ templates vs inheritance. IMO, there are pros and cons for each approach. For example:

1. Template based code generation gives the application developer full control afterward. But it will be hard for tooling support and enforcement of REST API styles by a team/company. Since the generated code don't share a common base, it's not easy to implement non-functional concerns such as logging, authentication/authorization, and metrics. Refactoring can be hard too.

2. Inheritance based approaches gives the application developer base implementation that can be overridden. With TypeScript and IDEs like VSCode, it's pretty easy to find out the super method signature. Application developers have to obey the inheritance rules and lose some control. But the base class gives the framework or team/company a control point to add/enforce common capabilities. It is also more friendly for metadata driven approach, such as adding controllers based on the discovered database schema, or provisioning new controllers in a hosted service.

I see three styles from the user experience perspective:

1. Code generation of controllers based on a template that defines Repository (various flavors) to REST mapping without extending a base class

2. Code generation of controllers based on a template that defines Repository (various flavors) to REST mapping by extending a base class

3. Automagic provision and registration of controllers based on metadata without code generation

While I'm fine with the idea to allow some application developers to choose 1, I think the other styles should be supported too.

I would also like to hear opinions from LoopBack 3.x users. It will be useful to get comments from IBM app connect team who uses LoopBack as the platform and requires dynamic activation/deactivation of certain artifacts.

[kjdelisle]

I object to the concept of the Repository interfaces in general, since it implies that there's inherent value not only in the concept, but in implementing this concept. My original attempt to create an implementation of the EntityCrudRepository and CrudConnector led me to the idea that even though I could build implementations of those functions, it would be easier to simply avoid them entirely.

This is why I began the spike for a TypeORM mixin. Allowing the conventions of the target ORM to dictate the shape of some of the artifacts feels clean and tangible, IMO, and directing our efforts there feels like a better way to get people what they want with popular offerings, rather than attempting to create a "universal" interface that can't possibly accommodate every edge case.

Tangling the REST API with the data layer feels like an anti-pattern. While I understand that some may want a magic generation of everything, we need to be careful about what we offer here since it leaves us with the responsibility of supporting all sorts of advanced use cases that won't easily fit into these patterns.

I also think we should close this PR in favour of code generation via the CLI.

[raymondfeng]

I object to the concept of the Repository interfaces in general, since it implies that there's inherent value not only in the concept, but in implementing this concept. My original attempt to create an implementation of the EntityCrudRepository and CrudConnector led me to the idea that even though I could build implementations of those functions, it would be easier to simply avoid them entirely.

Repository is common pattern implemented by a lot of frameworks. See:

• Spring Data - https://docs.spring.io/spring-data/data-commons/docs/1.6.1.RELEASE/reference/html/repositories.html
• Entity Framework - https://msdn.microsoft.com/en-us/library/ff649690.aspx

We cannot afford losing out-of-box data access capabilities in LoopBack 4. Being able to CRUD typical DBs and expose REST APIs by convention is one of the key features of LoopBack and it differentiate LoopBack from other frameworks such as Express, Hapi or Resify. If the integration is removed from LoopBack 4, I don't see why LoopBack is better than https://github.com/thiagobustamante/typescript-rest.

This is why I began the spike for a TypeORM mixin. Allowing the conventions of the target ORM to dictate the shape of some of the artifacts feels clean and tangible, IMO, and directing our efforts there feels like a better way to get people what they want with popular offerings, rather than attempting to create a "universal" interface that can't possibly accommodate every edge case.

IMO, it's contradictory to your statement that you object to Repository interfaces. TypeORM indeed supports repository pattern. I'm open to integrate with other ORMs. That's why I see there are different flavors of Repository interfaces. Having a consistent CRUD operations and unified query and mutation in JSON format is a key factor behind the success of LoopBack.

Tangling the REST API with the data layer feels like an anti-pattern. While I understand that some may want a magic generation of everything, we need to be careful about what we offer here since it leaves us with the responsibility of supporting all sorts of advanced use cases that won't easily fit into these patterns.

I strongly disagree. First of all, what's proposed in this PR is creating a bridge between controllers and repositories and it's not tangling at all. Model oriented resources fit REST paradigm pretty naturally. The mapping from repository to REST by convention finds its way in many popular frameworks. See:

• https://spring.io/guides/gs/accessing-data-rest/
• https://docs.microsoft.com/en-us/aspnet/web-api/overview/data/using-web-api-with-entity-framework/part-2
• LoopBack 3.x
• Ruby on rails

BTW, no matter how we support controllers with CRUD (code gen or base class), the REST mapping for Repository (-like) operations is always required, either in the template or base class. The debate is about how we serve the pattern for developers.

[raymondfeng]

@emckean @cfjedimaster @svennam92 Would you like to chime in?

[kjdelisle]

@raymondfeng

I object to the concept of the Repository interfaces in general...

To clarify: I meant that I don't feel comfortable with our current set of interfaces like EntityCrudRepository and the like, because there's no way to utilize them with other ORMs that isn't more easily replaced by using those ORMs directly (hence the explanation about how I already tried to use those interfaces to implement an adapter).

Now, it might be that there's missing code that would bridge the gap for actually constructing the datasource instances for another ORM, but even so, I can't see a compelling reason for an extension developer to implement those interfaces for their ORM of choice other than to avoid changing controller logic and calls to our CRUD functions (findById, updateById, replaceById, etc)

(EDIT: I should also point out that when I attempted to adapt TypeORM to our interfaces, there was no equivalent to replaceById, which meant I had to make multiple separate calls to TypeORM's repository, which creates a race condition!)

I certainly do not object to the general principle of a Repository as a design pattern. I just don't think there's a way to make a universal Repository interface that will function well with all known data sources.

Tangling the REST API with the data layer feels like an anti-pattern...

This was meant within the context of our "universal" interface. There's definitely some immediate value in being able to mount a REST API to your database, as long as everyone's aware of the limitations and potential risks that come with this approach, and how to mitigate them.

The interfaces we've already made make me feel uneasy because they provide a false sense of equivalence; being able to mount any ORM to them when all of the necessary pieces are made doesn't mean that the quality of that experience will be equivalent; in fact, it almost guarantees that only the ORM we build in-house will provide the best experience with the rest of the framework.

This is why I'm pushing to avoid this sort of lock-in. If we can treat our ORM (if we even make one!) the exact same way as all the others, it lets the community gravitate to whichever implementations they find most satisfying.

We cannot afford losing out-of-box data access capabilities in LoopBack 4. Being able to CRUD typical DBs and expose REST APIs by convention is one of the key features of LoopBack and it differentiate LoopBack from other frameworks such as Express, Hapi or Resify. If the integration is removed from LoopBack 4, I don't see why LoopBack is better than https://github.com/thiagobustamante/typescript-rest.

I'm not suggesting that we surrender out-of-the-box data capabilities in LoopBack, but rather we change the approach we take to get there. I think that there's a middle ground somewhere between frameworks like Express/Hapi/typescript-rest and something like LoopBack 3.

By providing generated code and mixins that work with that code, we can still give people a nicely sewn up experience, but one that's unique to which path they choose at the beginning (be it the ORM they select at the prompt, or something else).

Rather than having to provide end-to-end support for an ORM and a variety of connectors, we would only need to ensure that our templates and mixins are compatible with the latest implementations they're made to work with. In exchange, this leaves us with more free time to write the documentation, tutorials, blog posts and other materials that can help those devs find their way to the goal.

We currently support 32 connectors for the Juggler, and it was taxing in the days before we shifted to loopback-next. It pains me to see how many things we've had to let slip (or just plain missed) when it comes to those connectors and I don't want to be stuck in that position indefinitely.

Being able to CRUD typical DBs and expose REST APIs by convention is one of the key features of LoopBack and it differentiate LoopBack from other frameworks such as Express, Hapi or Resify

I want to touch on this point specifically.

We had this issue (strongloop/loopback#683) open for years with no satisfactory resolution, and tons of feedback from developers asking us to add JOIN support. The trouble is that there's no easy way to add "simple" support for things like join, which is why ORMs like TypeORM and knex.js provide query builders to give developers the ability to construct SQL queries programmatically to suit their needs.

Having a quick way to reach your data over REST is a satisfying first step, but most users that decide to use the framework want to go much further, and they expect us to not only provide these sorts of advanced features, but to do it efficiently. If we're going to provide our own ORM, then I would expect us to take on the burden of building these advanced features into it, for every single database we claim to support, SQL or otherwise.

I'm only going to get on board with the idea of building our own ORM if it can perform at least as well as some of its competitors in these more advanced examples, and to be blunt, I doubt that we're going to get that far in the timeframe we originally intended with as many developers as we have.

[bajtos]

@raymondfeng

What are in common is that we need come up a template to define the mappings from repository methods to REST operations. Please confirm if you agree.

Yes, I agree!

Template based code generation gives the application developer full control afterward. But it will be hard for tooling support and enforcement of REST API styles by a team/company. Since the generated code don't share a common base, it's not easy to implement non-functional concerns such as logging, authentication/authorization, and metrics. Refactoring can be hard too.

In my experience, every team has different non-functional concerns they need to enforce in all controllers. It's very difficult (if possible at all) to provide a single universal base class with extension points that will accommodate all possible non-functional concerns.

Instead, I am proposing to advise teams to build their own base controller classes with whatever non-functional requirement they have, and share these base controller classes in their project/team/company - for example as an npm package.

In that light, it makes sense to me to require our CLI tooling to support controllers inheriting from a base class template, but asking the CLI users to provide those base classes! In other words, we need to make the following easy:

• As an app developer, I want to build and publish a template controller to be used as a base class in my project(s).
• When creating a new project, I want to specify what npm package & base class name to use for my controllers.
• When running lb4 controller (or similar command), I want the CLI to use the configuration from the previous step to correctly scaffold my controllers to inherit from my custom base class.

[cfjedimaster]

I'm only chiming in as I was asked to. ;) I have no idea what a repository is in terms of LoopBack. I've read the first few comments a few times, and frankly, I just can't connect it to my knowledge of LB3. :)

[raymondfeng]

@cfjedimaster In LoopBack 3, we use models to deal with modeling, persistence, and remoting. LoopBack 4 decouple the responsibilities into a few artifacts:

1. controller (accepting REST/HTTP requests and producing responses)
2. repository (a binding of model and datasource, providing CRUD operations)
3. model (plain data modeling)

To help LB 3.x users to have one class to connect CRUD with REST, this proposal defines a base controller class with decorators to map various CRUD methods to REST (similar as LB 3.x PersistedModel) and depends on the repository for the CRUD implementation.

Without this, a developer will have to define a controller class, add CRUD methods with decorators for REST, and implement each method by delegating a repository instance (can be injected into the controller as a dependency).

[cfjedimaster]

So being able to customize at the level you describe sounds nice, but I'd not use it most of the time as I've been more than satisfied with how LB3 handles things out of the box. (Or I add a few custom remote properties.) So yes - I'd say LB4 has to have this or I'd have to start asking why I'm using LB if I have to do more work to get simple stuff done. :)

[kjdelisle]

Given that we've settled on the Juggler approach for our new version, this PR seems like a no-brainer to me, but I would like to rename it so that we can have a more generic CrudController beneath this layer that provides the repository mappings without the protocol-specific decorators.

[kjdelisle]

I would rename this to RestController; that way we can keep the idea of a CrudController open as a base class for a RestController, a GrpcController, etc.

[kjdelisle]

Same here, I'd call it EntityRestController

[bajtos]

@raymondfeng is this pull request still relevant?

[raymondfeng]

I'll keep it open as an option till the LB3 functional parity is settled.

[bajtos]

Sounds good. I think we will need something similar to what is proposed here for the following user story: From model definition to REST API with no repository/controller classes #2036. Besides a built-in controller for CRUD APIs, we will also need a controller for KeyValue style.

[bajtos]

Created #2736 to keep track of this finishing this work.

I am proposing to call the extension package @loopback/rest-crud, so that we can later add @loopback/rest-key-value for KeyValue repositories.

Similarly, I am proposing to call the controller class CrudRestController, to differentiate from KeyValueRestController (KeyValue over REST) and in the future from CrudGrpcController, etc.

[bajtos]

Closing in favor of #3582