Uh oh!
There was an error while loading. Please reload this page.
Load middleware and phases from middleware.json - #66
Conversation
bajtos
commented
Nov 12, 2014
In the light of our recent discussion in SlackChat and the request strongloop/loopback#794, we cannot use middleware path as a key, as it must be possible to load multiple instances of the same middleware. I'll rework this pull request to support that. |
There was a problem hiding this comment.
factoryFile is a bit weird... I'm not 100% against it... but it just doesn't look right. Perhaps middlewareFile?
There was a problem hiding this comment.
How about sourceFile? It is already nested in middleware section, I feel it's not necessary to repeat middleware in the name.
ritch
commented
Nov 12, 2014
@bajtos I took a look and didn't find anything worth mentioning besides |
bajtos
commented
Nov 13, 2014
I have reworked the patch to support strongloop/loopback#794, the file
Example: {
"auth": {
"oauth2": [
{
"args": "first-domain-config"
},
{
"args": "second-domain-config"
}
]
},
"routes:before": {
"morgan": {
"args": ["dev", { "threshold": 128 }]
}
},
"routes": {
"loopback/server/middleware/rest": {
}
},
"subapps": {
"./adminer": {
},
}
}Once we have strongloop/loopback#794 implemented, we can use a config similar to the following example to configure OAuth2 protection for different paths: {
"auth": {
"loopback-component-oauth2/server/middleware/authorize": [
{
"paths": ["/restricted", "/api"],
"args": { "session": false, "scope": "demo"}
},
{
"paths": ["/admin"],
"args": { "session": true, "scope": "intranet" }
},
]
}
}@ritch please review the new version |
bajtos
commented
Nov 13, 2014
The new version depends on loopback changes proposed in strongloop/loopback#796. |
8154c70 to
6a6c707Compareraymondfeng
commented
Nov 13, 2014
@bajtos I'm trying to figure out what is in the oauth2 component and what is in the main app. For the oauth2 component, I assume we will implement (and declare) the middleware. Let's take the token middleware (validates the access token and stores the accessToken object in the req and/or context) as an example.
In the main app,
Can you confirm? |
ritch
commented
Nov 13, 2014
Can we hide the structure of the component somehow by handling this a bit differently: Perhaps reference this as |
ritch
commented
Nov 13, 2014
|
ritch
commented
Nov 13, 2014
@bajtos do you have ideas on how to implement this in workspace / the generators? The format in general is nice / readable, but looks like it will be difficult to implement in the workspace. |
raymondfeng
commented
Nov 13, 2014
Let's walk through the developer experience. For the component developer that implements the middleware
module.exports=function(options){returnfunctionfooMiddleware(req,res,next){// foo logic ...next();}}
exports.foo=require('./lib/foo-middleware.js');
varfoo=require('my-component').foo;app.use(foo(options));
For the application developer uses the middleware from my-component.
varoptions={ttl: 60};// or read from a config filevarfoo=require('my-component').foo;app.use(foo(options));@bajtos Your proposal seems to automate step 3 for the application (importing the middleware from my-component). We can start from there. What's your take on the component side (exporting the middleware)? |
bajtos
commented
Nov 14, 2014
@raymondfeng@ritch thank you for thoughtful comments! Just to make it clear: the first version of middleware support will not include tooling ( However, it is still a good idea to discuss the tooling to ensure that it will be possible to implement the necessary changes in the future. Keeping that in mind, here are my answers to the questions raised above.
As I am envisioning the final solution, the runtime (loopback-boot) does not need any middleware declaration, as it has all necessary information in
The current name is Middleware exported by a module (component) should be described in However, if both of you and @ritch prefer
Yes, your description is correct. In the future, we should automate some part of that process via
Yes. Additionally, my proposal handles the following:
For the initial version, the component does not have to do anything to export a middleware. Just put the javascript file to In the future, the component should include middleware description in the component metadata specified in package.json, e.g. {"name": "loopback-component-foo","version": "1.0.0","loopback-component": {"common": {/* ... */},"server": {"connectors": /*...*/,/* models? */"middleware": {"token": {"description": "The token middleware validates the access token and stores the accessToken object in the req and/or context","sourceFile": "server/middleware/token","recommendedPhase": "auth","options": {"ttl": {"type": "number","default": 60,"description": "Time to live for the tokens."}}}}}}}Storing the component metadata in
Definitely not
So far, loopback-boot is trying to avoid any opinions about the layout of 3rd party modules, for example I would like to preserve that trait for the middleware registration too and keep support for "require()"-style identifiers in addition to any shorthand convention-based notation we can come up with. Let's move the discussion about the shorthand notation to a new github issue #68 and implement the feature as an incremental addition once this PR is landed.
The format is an implementation detail of the filesystem storage, it does not have to drive the design of workspace entities. Here is a quick outline of a possible implementation in loopback-workspace:
|
bajtos
commented
Nov 14, 2014
Let's move this discussion to strongloop/loopback#796 |
bajtos
commented
Nov 14, 2014
This should be automated by |
bajtos
commented
Nov 18, 2014
@raymondfeng@ritch any more comments? I need to finish this patch before I can start working on follow-up stories scheduled for Sprint 58. |
raymondfeng
commented
Nov 18, 2014
@bajtos Can you confirm if the following middleware.json can be loaded by your PR? https://gist.github.com/raymondfeng/25f3dc5acd3ce60dc98f I have a few things around the middleware key. In my case, I have: session ==> loopback.session |
bajtos
commented
Nov 18, 2014
It looks like it should be loadable, possibly with minor tweaks in the syntax.
No, this won't be implemented, as
#68 proposes
Yes, this will work. The path is relative to |
raymondfeng
commented
Nov 18, 2014
Sounds good. For the relative path, I would prefer to be resolved against the containing file as it won't leave any ambiguity to developers to specify the path. |
ritch
commented
Nov 18, 2014
👍 |
bajtos
commented
Nov 19, 2014
For posterity, I have amended the code to make this more clear and to guide future developers in that direction. // not configurable yetvarmiddlewareRootDir=appRootDir;varmiddlewareConfig=options.middleware||ConfigLoader.loadMiddleware(middlewareRootDir,env);varmiddlewareInstructions=buildMiddlewareInstructions(middlewareRootDir,middlewareConfig); |
f5f1144 to
352dad7Comparebajtos
commented
Nov 19, 2014
@slnode test please |
352dad7 to
3ba151fCompareSample JSON:
{
"routes:before": {
"morgan": {
"params": ["dev"]
}
},
"routes": {
"loopback/server/middleware/rest": {
}
},
"subapps": {
"./adminer": {
},
}
}
The JSON file can be customized using the usual conventions:
- middleware.local.{js|json}
- middleware.{env}.{js|json}
It is also possible to mount the same middleware in the same phase
multiple times with different configuration.
Example config:
{
"auth": {
"oauth2": [
{
"params": "first"
},
{
"params": "second"
}
]
},
});3ba151f to
1114bc9CompareLoad middleware and phases from `middleware.json`
Sample JSON:
The JSON file can be customized using the usual conventions:
It is also possible to mount the same middleware in the same phase
multiple times with different configuration.
Example config:
A part of #44
/to @raymondfeng please review
/cc @ritch