-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC showing how to mount an LB3 app and include its REST API in OpenAPI spec [DO NOT MERGE] #2318
Conversation
Quick question first: What's the advantage of mounting an LB3 app on an LB4 app (esp, at this stage) over hosting a stand-alone LB3 app? Who are the target users? |
Good question. Can you @raymondfeng and @richardpringle answer please? In my understanding, when you mount an LB3 app inside an LB4 app, you can more easily migrate your codebase in small increments. For example, you can create LB4 controllers that call out to LB3 models for the actual implementation. Or maybe when adding a new feature, you can implement it in LB4 style (LB4 model, repository & controller), and keep the existing APIs served by the legacy LB3 codebase. |
@hacksparrow, the advantage of mounting an LB3 app inside an LB4 is that everything is packaged together. You don't have to have separate deployments processes etc. I highly doubt that a larger organization that already has experience maintaining multiple legacy applications would see any advantage to loading the two apps together, but for those smaller orgs with less resources, it makes maintenance easier. You can also run both apis on the same port instead of having to set up nginx or proxy based on path prefix. All of these things are fairly straight forward, but each of them contributes to a build up of friction when it comes to migrating over to LB4. Make sense? |
Thank you for thoughtful comments. As I was thinking about this PoC code some more, I realized it's pointing to a missing extension/composition point in LB4. Essentially, we have a custom router (the LB3 app with its REST handler) providing a set of routes, and the accompanying OpenAPI spec describing those routes. In mind, this is similar to existing building blocks we already have in LB4: controller/handler routes managed by our RoutingTable, StaticAssetsRoute managing routes for static assets (with no OpenAPI spec). Here is the request handling pipeline as configured in the top-level Express app:
I am thinking about inserting a "custom routers" extension into this list:
With this infrastructure in place, LB3-to-LB4 bridge can use a standard approach and simply export the custom router powered by LB3. No hacky integration with the REST layer is needed. @raymondfeng what do you think? |
Another use case for a custom router: GraphQL endpoints implemented by oasgraph, see #1905 (comment) |
Thanks @richardpringle. |
0a57cf8
to
ade46d8
Compare
In ade46d8, I introduced a new API The implementation details are wrong, I need to move the code invoking express routers in between LB4 router and static assets. I am not expecting any changes in the user-facing API in |
@raymondfeng @hacksparrow ping 👋
|
Maybe switch the order of IMO, exposing one extensible container for express routers is a good starting point. Ultimately, we still have to make the pipeline more flexible for composition - either phase based or something else. |
3a39c96
to
8e95629
Compare
loopback-getting-started uses externally hosted database and runs automigration at start. I think this creates a race condition where one Travis CI runner deletes old tables while another Travis CI runner is already trying to insert sample (seed) data. |
@bajtos will there be any differences in the mounted app and an app that's running independently, feature and functionality wise? |
@@ -144,14 +136,14 @@ export class RoutingTable { | |||
} | |||
|
|||
// this._staticAssetsRoute will be set only if app.static() was called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once that outdated comment is updated, it looks pretty good. 👏🏻
I believe the behavior should be exactly the same. The only difference I am aware of is the version of OpenAPI spec - LB3 produces OpenAPI v2 (a.k.a Swagger), LB4 produces OpenAPI v3. There may be a subtle difference in how CORS middleware installed by LB4 interacts with the CORS middleware installed by LB3. I don't know enough to be able to provide more details. |
I have created the following follow-up issues and closing this spike as done.
|
I created a new Epic to keep track of all follow-up issues in one place: #2479 |
A proof of concept showing how to expose a LB3 app (loopback-getting-started) via LB4 REST API (see #2301). The solution presented in this pull request is a somewhat alternate approach to #2274, see the discussion in that PR for context.
Under the hood, the solution has the following major parts:
app.mountExpressRouter(basePath, router, spec)
allows LB4 appdevelopers to add arbitrary set of Express routes and provide OpenAPI spec.
Lb3AppBooter
responsible for setting up LoopBack 3 application, obtainingOpenAPI spec describing app's REST API and mounting the app on LB4.
See SPIKE NOTES for further details.
Related: #1849 #2301
Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated