From 844147517e0496d5569652fea2d9113ca90e7dbe Mon Sep 17 00:00:00 2001 From: ebarault Date: Thu, 9 Mar 2017 13:34:58 +0100 Subject: [PATCH] first commit for general discussion --- lib/application.js | 134 ++++++++++++++++++++++++++++++++++++++++----- test/user.test.js | 2 +- 2 files changed, 121 insertions(+), 15 deletions(-) diff --git a/lib/application.js b/lib/application.js index 62db7858..a2209ec8 100644 --- a/lib/application.js +++ b/lib/application.js @@ -19,6 +19,7 @@ var classify = require('underscore.string/classify'); var camelize = require('underscore.string/camelize'); var path = require('path'); var util = require('util'); +var Promise = require('bluebird'); /** * The `App` object represents a Loopback application. @@ -404,30 +405,133 @@ app.enableAuth = function(options) { }; app._verifyAuthModelRelations = function() { + let self = this; + // Allow unit-tests (but also LoopBack users) to disable the warnings - if (this.get('_verifyAuthModelRelations') === false) return; + const warnOnBadSetup = self.get('_disableAuthModelRelationsWarnings') !== true; + // Prevent the app from being killed although the configuration is inconsistent + const dismissRejection = self.get('_dismissBadUserConfigRejection') !== true; - const AccessToken = this.registry.findModel('AccessToken'); - const User = this.registry.findModel('User'); - this.models().forEach(Model => { - if (Model === AccessToken || Model.prototype instanceof AccessToken) { - scheduleVerification(Model, verifyAccessTokenRelations); - } + const AccessToken = self.registry.findModel('AccessToken'); + const User = self.registry.findModel('User'); + const models = self.models(); - if (Model === User || Model.prototype instanceof User) { - scheduleVerification(Model, verifyUserRelations); - } + verifyUserModelsSetup().then(()=> { + verifyRelationsSetup(); }); + function verifyUserModelsSetup() { + let userModels = models.filter(model => { + return (model === User || model.prototype instanceof User) || + (model === AccessToken || model.prototype instanceof AccessToken); + }); + + // we use Promise.reflect() here to let all the tests go without throwing, + // this way we can get all the logs for all models we are checking + return Promise.map(userModels, (model) => { + return scheduleVerification(model, hasMultipleUserModelsConfig).reflect(); + }) + .then((inspections) => { + // proceed to next checkups if no error + let hasErrors = inspections.filter(inspection => { + return inspection.isRejected(); + }); + if (!hasErrors.length) { + return; + } + + // else log and eventually kill the app + console.warn('Application setup is inconsistent: some models are set to use ' + + 'the Multiple user models configuration while some other models are set to ' + + 'use the Single user model configuration. The model config is listed below.'); + + // detailed logs + inspections.forEach(inspection => { + let modelSetup = (inspection.value() || inspection.reason()).modelSetup; + console.warn( + 'Model %j of type %j is set to use the %j user model config', + modelSetup.model, + modelSetup.instanceOf, + modelSetup.hasMultipleUserModelsConfig ? 'Multiple' : 'Single' + ); + }); + + delete self.hasMultipleUserModelsConfig; + + if (!dismissRejection) { + throw new Error('Application setup is inconsistent, please see the log for more ' + + 'information'); + } + }); + } + + function verifyRelationsSetup() { + models.forEach(Model => { + if (Model === AccessToken || Model.prototype instanceof AccessToken) { + scheduleVerification(Model, verifyAccessTokenRelations); + } + + if (Model === User || Model.prototype instanceof User) { + scheduleVerification(Model, verifyUserRelations); + } + }); + } + + // instant or scheduled Model verifications, as a promise function scheduleVerification(Model, verifyFn) { - if (Model.dataSource) { - verifyFn(Model); - } else { - Model.on('attached', () => verifyFn(Model)); + return new Promise((resolve, reject) => { + if (Model.dataSource) { + try { + resolve(verifyFn(Model)); + } catch (e) { + reject(e); + } + } else { + Model.on('attached', () => { + scheduleVerification(Model, verifyFn); + }); + } + }); + } + + function hasMultipleUserModelsConfig(model) { + let hasMultipleUserModelsConfig, isInstanceOfUser; + + // check is the model is set to use mutiple users config (with polymorphic relations) + if (model === User || model.prototype instanceof User) { + isInstanceOfUser = true; + const hasManyTokens = model.relations && model.relations.accessTokens; + hasMultipleUserModelsConfig = !!hasManyTokens.polymorphic; + } else if (model === AccessToken || model.prototype instanceof AccessToken) { + const belongsToUser = model.relations && model.relations.user; + // the test on belongsToUser is required as we allow AccessToken model not + // to define the relation with custom user model for backward compatibility + // see https://github.com/strongloop/loopback/pull/3227 + hasMultipleUserModelsConfig = !!(belongsToUser && belongsToUser.polymorphic); } + + let modelSetup = { + model: model.modelName, + instanceOf: isInstanceOfUser ? 'User' : 'AccesToken', + hasMultipleUserModelsConfig, + }; + + // check if model config is consistent with already parsed models' config + if (self.hasMultipleUserModelsConfig === undefined) { + self.hasMultipleUserModelsConfig = hasMultipleUserModelsConfig; + } else if (self.hasMultipleUserModelsConfig !== hasMultipleUserModelsConfig) { + let error = new Error('User models setup is inconsistent'); + error.modelSetup = modelSetup; + throw error; + } + + // if no error, return the modelSetup for further logging purposes + return {modelSetup}; } function verifyAccessTokenRelations(Model) { + if (!warnOnBadSetup) return; + const belongsToUser = Model.relations && Model.relations.user; if (belongsToUser) return; @@ -454,6 +558,8 @@ app._verifyAuthModelRelations = function() { } function verifyUserRelations(Model) { + if (!warnOnBadSetup) return; + const hasManyTokens = Model.relations && Model.relations.accessTokens; if (hasManyTokens) return; diff --git a/test/user.test.js b/test/user.test.js index 396bb89f..cac82d48 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -2442,7 +2442,7 @@ describe('User', function() { it('handles subclassed user with no accessToken relation', () => { // setup a new LoopBack app, we don't want to use shared models app = loopback({localRegistry: true, loadBuiltinModels: true}); - app.set('_verifyAuthModelRelations', false); + app.set('_disableAuthModelRelationsWarnings', true); app.set('remoting', {errorHandler: {debug: true, log: false}}); app.dataSource('db', {connector: 'memory'}); const User = app.registry.createModel({