Integrate phases with boot(...)

[strongloop]

Integrate phases with boot(...)

[chandadharap]

What is actually implemented by this story? Does it need a breakdown, as in, a checklist?

[raymondfeng]

Let's borrow some ideas from JAX-RS:

https://jersey.java.net/documentation/latest/filters-and-interceptors.html

In particular:

• Types of phases (middleware, remoting, model, connector)
• Predefined phases
• Execution order
• Binding of handlers (global/name binding/dynamic binding)

[ritch]

@raymondfeng here is the code scratch pad we used to pair program some of the basic implementation ideas.

var loopback = require('loopback');
var boot = require('loopback-boot');

var app = module.exports = loopback();

// Set up the /favicon.ico
app.use(loopback.favicon());

// request pre-processing middleware
app.use(loopback.compress());

// -- Add your pre-processing middleware here --

// boot scripts mount components like REST API
boot(app, __dirname);

var root = app.createPhase('root');

foreach(phase in config.phases) {

}

foreach(phase in app.phases()) {
  foreach(handler in handlersForGivenPhase) {
    app.register(phase.name, hander)
  }

  root.add(phase);
}

// juggler
User.create = function(data, cb) {
  User.SOMETHING.execPhase('validate', function() {
    // the validate phase!
    // ... and so on...
  });
}

// hook into your method
Phase.wrapSync('yourMethod', ...);

MyModel.register('beforeYourMethod', function(ctx) {
  ctx.arguments
})

MyModel.yourMethod = function proxy() {
  // executes concrete yourMethod
  // before phase
  // if sync
  //  executes the after phase
  // async
  //   wrap the callback
  //      exec the after phase
}

Model.wrap = function(methodName, options) {
  var methodPhase = Model.createPhase(key);
  model[methodName] = function() {
    var args = arguments;
    if(isSync) {
      methodPhase.add('before', methodName, function() {
        originalMethod(args);
        methodPhase.add('after', methodName);
      });
    } else {
      var cb = getCb(args);
      methodPhase.add('before', ..., function() {
        methodPhase.add('after', methodName);
      });
    }
  }
}

app.register('foo:*', function(cb) {
  console.log(ctx.phase.name);
  cb();
});

MyModel.register('afterYourMethod', function(ctx) {
  ctx.arguments
})

app.models.User.register('validate', function(ctx, done) {
  done(new Error('nothing is valid!'));
});

// new imp for enable auth
app.register('auth', function(ctx) {
  // ...
  loopback.Context
  ctx.remotingContext.req
});

var explorer;
try {
  explorer = require('loopback-explorer');
} catch(err) {
  console.log(
    'Run `npm install loopback-explorer` to enable the LoopBack explorer'
  );
  return;
}

var restApiRoot = server.get('restApiRoot');

var explorerApp = explorer(server, { basePath: restApiRoot });
server.use('/explorer', explorerApp);
server.once('started', function() {
  var baseUrl = server.get('url').replace(/\/$/, '');
  // express 4.x (loopback 2.x) uses `mountpath`
  // express 3.x (loopback 1.x) uses `route`
  var explorerPath = explorerApp.mountpath || explorerApp.route;
  console.log('Browse your REST API at %s%s', baseUrl, explorerPath);
});

var restApiRoot = server.get('restApiRoot');
server.use(restApiRoot, server.loopback.rest());

// -- Mount static files here--
// All static middleware should be registered at the end, as all requests
// passing the static middleware are hitting the file system
// Example:
//   app.use(loopback.static(path.resolve(__dirname', '../client')));
var websitePath = require('path').resolve(__dirname, '../client');
app.use(loopback.static(websitePath));

// Requests that get this far won't be handled
// by any middleware. Convert them into a 404 error
// that will be handled later down the chain.
app.use(loopback.urlNotFound());

// The ultimate error handler.
app.use(loopback.errorHandler());

app.start = function() {
  // start the web server
  return app.listen(function() {
    app.emit('started');
    console.log('Web server listening at: %s', app.get('url'));
  });
};

// start the server if `$ node server.js`
if (require.main === module) {
  app.start();
}

[bajtos]

I have reviewed the code from the scratchpad. Few comments:

• I find the names of the new methods little bit confusing, we should come up with a more descriptive name (e.g. onPhase(name, fn) instead of register(name, fn))
• app.createPhase and root.add(phase) seem rather arbitrary to me, I don't understand what is the purpose. It's not a problem, I am just saying that I cannot asses this part.
• The proposal does not handle phase ordering, nor Binding of handlers (global/name binding/dynamic binding)

Other than that, this looks like a sensible approach to me.

It would be great if we can build this incrementally in small steps. Start with something small yet complete, then add more stuff building on top of that. Perhaps start by replacing beforeValidate and afterValidate hooks with the new implementation?

I don't understand what is the relation of this feature with the middleware registration and ordering, if there is any relation. In particular, how will be phase.toMiddleware() mounted on the app?

[bajtos]

@ritch could you please update the spec in wiki to match the final implementation?

The check-list in the issue description contains items that were not implemented AFAICT. Are you sure this task is done? ;-)

[ritch]

@bajtos good catch... I though this was the PR. This should stay open until it is implemented in boot.

[chandadharap]

reestimate, rescope

[bajtos]

IMO the middleware (request-processing) phases should be defined directly in loopback. That way we preserve the current design where loopback-boot is just a convention-based wrapper around features provided by loopback core.

[bajtos]

The more I think about integrating phases & middleware components into loopback and loopback-boot, the more confused I am about what is the actual plan.

As I see it, there are many places where we can use phases:

1. Bootstrapping phases: applyAppConfig, setupDataSources, setupModels, runBootScripts, etc. However, using phases is not very practical here, as the boot scripts that can register phase callbacks are run towards the end, when most of the phases were already executed.

2. Bootstrapping phases for middleware registration. After runBootScripts is done, the bootstrapper can run phases to execute app.use in the correct order.

   // boot/token.js
   module.exports = function(app) {
     app.boot.middleware.find('auth').use(function() {
       app.use(app.loopback.token());
     });
   };
   
   // loopback-boot
   async.series([
     function(done) {
        runBootScripts(app, instructions, done);
     },
     function(done) {
        app.boot.middleware.run(app, done);
     },
     function(done) {
       enableAnonymousSwagger(app, instructions);
       done();
     }], callback)

3. LoopBack runtime can provide a phase list that is executed as part of request handling. A simplified implementation:

   // boot/token.js
   module.exports = function(app) {
     app.middleware.find('auth').use(app.loopback.token());
   };
   
   // lib/application.js
   app.handlers = function(type) {
     // todo: memoize (cache) the final handler
     var remotingHandler = this.remotes().handler;
   
     // we need to inject strong-remoting handler into `route` phase
     // to avoid duplicate calls of the handler, we create a new clone 
     // of the original phase list 
     var phases = app.middleware.clone();
     phases.find('route').use(function(ctx, cb) {
       remotingHandler(ctx.req, ctx.res, cb);
     });
   
     return function(req, res, next) {
       phases.run({ req: req, res: res }, next);
     }
   };

4. Replace remoting hooks with phases

5. Replace Model hooks like beforeCreate or afterSave with phases. Possible merge remoting hooks with these new phases.

Only the items 1 & 2 require changes in loopback-boot, items 3-5 affect loopback core.

@ritch @raymondfeng What's your opinion? What did I missed? Which items do we want to implement?

This is blocking my work on #44, which in turns blocks other stories in the sprint. Unfortunately I won't have much time to discuss this in person today. Could you please leave a comment here so that I can make a better decision on what to work tomorrow?

[ritch]

Bootstrapping phases: applyAppConfig, setupDataSources, setupModels, runBootScripts, etc. However, using phases is not very practical here, as the boot scripts that can register phase callbacks are run towards the end, when most of the phases were already executed.

You are assuming the only way you can add a boot phase handler is in a boot script... Why? I think we should make it clear where you can add them that makes sense (eg. server.js).

[ritch]

I think 3 is the priority and the only real blocker for other stories.

[ritch]

I'd like to implement most of these. Here is the priority IMO:

• 3. a phase list that is executed as part of request handling (for mounting middleware in order)
• 1. a boot phase list executed during the boot process (for components to add boot tasks)
• 5. hooks for methods built on phases (for apps and components)

[ritch]

The more I think about integrating phases & middleware components into loopback and loopback-boot, the more confused I am about what is the actual plan.

Do you have specific questions about 3?

[raymondfeng]

I think the other important perspective for phases is to allow additional logic to plug in. For the boot process, if we anticipate custom code to be executed when an app is starting, we should phasify the steps. When a phase is run, we could emit before/use/after events too. This way, some logic can be added as event listeners too.

[bajtos]

Bootstrapping phases: applyAppConfig, setupDataSources, setupModels, runBootScripts, etc. However, using phases is not very practical here, as the boot scripts that can register phase callbacks are run towards the end, when most of the phases were already executed.

You are assuming the only way you can add a boot phase handler is in a boot script... Why? I think we should make it clear where you can add them that makes sense (eg. server.js).

server.js should be as small as possible and preferably users should not touch it. There are many reasons for that, one of them is to allow generator-loopback and Studio to add/remove stuff without having to parse arbitrary javascript code in server.js.

We can add a concept of "preboot" scripts that are run before anything else, but that adds even more complexity to the already complicated boot process.

I think 3 is the priority and the only real blocker for other stories.
Do you have specific questions about 3?

I'd say the specific question will arise when we get down to implementation. I have few suspicions on what may not work great, but let's wait until we have a code to discuss.

The question is - who is going to implement it? I am happy to volunteer. We should create a new story in loopback to track this task.

What is the scope of this issue in loopback-boot? Is it "1. a boot phase list executed during the boot process (for components to add boot tasks)"?

When a phase is run, we could emit before/use/after events too. This way, some logic can be added as event listeners too.

Phases already provide before/use/after semantics and as opposed to event emitters, it support asynchronous listener functions. I'd rather keep the matter simple and let users install before/use/after callback instead of adding event emitter into the mix.

[bajtos]

The question is - who is going to implement it? I am happy to volunteer. We should create a new story in loopback to track this task.

strongloop/loopback#739

[dhmlau]

Since the last comment was almost 4 years ago, closing due to inactivity. Please create a new issue if you are still running into problems.