From 26abb43ad43118472e0d57caa83722accd0f4264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 21 Oct 2014 10:06:28 +0200 Subject: [PATCH 1/2] Skip definitions of built-in loopback models LoopBack built-in models are special: they follow the loopback-boot structure and provide `common/models/{name}.json` files, but they are also automatically loaded (created) by loopback. This change modifies `executor` to recognize built-in models and do not redefine them. --- lib/executor.js | 13 +++++++++++++ test/executor.test.js | 16 ++++++++++++++++ test/fixtures/browser-app/model-config.json | 6 ++++++ 3 files changed, 35 insertions(+) diff --git a/lib/executor.js b/lib/executor.js index ceef61f..61d38e8 100644 --- a/lib/executor.js +++ b/lib/executor.js @@ -156,6 +156,10 @@ function defineModels(app, instructions) { throw new Error('Cannot configure unknown model ' + name); } debug('Configuring existing model %s', name); + } else if (isBuiltinLoopBackModel(app, data)) { + model = app.loopback[name]; + assert(model, 'app.loopback[model] should have been defined'); + debug('Configuring built-in LoopBack model %s', name); } else { debug('Creating new model %s %j', name, data.definition); model = app.loopback.createModel(data.definition); @@ -176,6 +180,15 @@ function defineModels(app, instructions) { }); } +function isBuiltinLoopBackModel(app, data) { + // 1. Built-in models are exposed on the loopback object + if (!app.loopback[data.name]) return false; + + // 2. Built-in models have a script file `loopback/{facet}/models/{name}.js` + return data.sourceFile && + /node_modules\/loopback\/[^\/]+\/models\/[^\/]+\.js$/.test(data.sourceFile); +} + function forEachKeyedObject(obj, fn) { if(typeof obj !== 'object') return; diff --git a/test/executor.test.js b/test/executor.test.js index 4a63929..c4f248c 100644 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -160,6 +160,22 @@ describe('executor', function() { expect(actual).to.equal('not attached'); }); + it('skips definition of already defined LoopBack models', function() { + var builtinModel = { + name: 'User', + definition: fs.readJsonFileSync( + require.resolve('loopback/common/models/user.json') + ), + config: { dataSource: 'db' }, + sourceFile: require.resolve('loopback/common/models/user.js') + }; + builtinModel.definition.redefined = true; + + boot.execute(app, someInstructions({ models: [ builtinModel ] })); + + expect(app.models.User.settings.redefined, 'redefined').to.not.equal(true); + }); + describe('with boot and models files', function() { beforeEach(function() { process.bootFlags = process.bootFlags || []; diff --git a/test/fixtures/browser-app/model-config.json b/test/fixtures/browser-app/model-config.json index 3566c55..d895c8a 100644 --- a/test/fixtures/browser-app/model-config.json +++ b/test/fixtures/browser-app/model-config.json @@ -1,4 +1,10 @@ { + "_meta": { + "sources": [ + "./models", + "../../../node_modules/loopback/common/models" + ] + }, "Customer": { "dataSource": "db" } From aa4cbdd80ffbd3effd43a0daac16e0d9fd30c879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 21 Oct 2014 16:51:17 +0200 Subject: [PATCH 2/2] compiler: support module-relative model sources Interpret model sources in the same way how `require.resolve` interprets the path: - values starting with `./` and `../` are relative to the file where they are specified - other values are relative to node modules folders This way it's possible to specify a source `loopback/common/models` and have it resolved to whatever place the loopback is installed. --- lib/compiler.js | 38 ++++++++++++++++++++- test/compiler.test.js | 24 +++++++++++++ test/fixtures/browser-app/model-config.json | 2 +- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/lib/compiler.js b/lib/compiler.js index d5d870a..3e1df63 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -5,6 +5,7 @@ var path = require('path'); var toposort = require('toposort'); var ConfigLoader = require('./config-loader'); var debug = require('debug')('loopback:boot:compiler'); +var Module = require('module'); /** * Gather all bootstrap-related configuration data and compile it into @@ -245,7 +246,12 @@ function findModelDefinitions(rootDir, sources) { var registry = {}; sources.forEach(function(src) { - var srcDir = path.resolve(rootDir, src); + var srcDir = resolveSourceDir(rootDir, src); + if (!srcDir) { + debug('Skipping unknown module source dir %j', src); + return; + } + var files = tryReadDir(srcDir); files .filter(function(f) { @@ -267,6 +273,36 @@ function findModelDefinitions(rootDir, sources) { return registry; } +function resolveSourceDir(rootDir, sourceDir) { + var srcDir = path.resolve(rootDir, sourceDir); + if (fs.existsSync(srcDir)) + return srcDir; + + // Handle module-relative path, e.g. `loopback/common/models` + var start = sourceDir.substring(0, 2); + if (start !== './' && start !== '..') { + // Module.globalPaths is a list of globally configured paths like + // [ env.NODE_PATH values, $HOME/.node_modules, etc. ] + // Module._nodeModulePaths(rootDir) returns a list of paths like + // [ rootDir/node_modules, rootDir/../node_modules, etc. ] + var modulePaths = Module.globalPaths + .concat(Module._nodeModulePaths(rootDir)); + + srcDir = modulePaths + .map(function(candidateDir) { + return path.join(candidateDir, sourceDir); + }) + .filter(function(candidate) { + return fs.existsSync(candidate); + }) + [0]; + if (srcDir) + return srcDir; + } + + return undefined; +} + function loadModelDefinition(rootDir, jsonFile) { var definition = require(jsonFile); diff --git a/test/compiler.test.js b/test/compiler.test.js index 45e17ec..f902140 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -501,6 +501,30 @@ describe('compiler', function() { }); }); + it('supports sources relative to node_modules', function() { + appdir.createConfigFilesSync({}, {}, { + User: { dataSource: 'db' } + }); + + var instructions = boot.compile({ + appRootDir: appdir.PATH, + modelSources: [ + 'loopback/common/models', + 'loopback/common/dir-does-not-exist' + ] + }); + + expect(instructions.models).to.have.length(1); + expect(instructions.models[0]).to.eql({ + name: 'User', + config: { + dataSource: 'db' + }, + definition: require('loopback/common/models/user.json'), + sourceFile: require.resolve('loopback/common/models/user.js') + }); + }); + it('handles model definitions with no code', function() { appdir.createConfigFilesSync({}, {}, { Car: { dataSource: 'db' } diff --git a/test/fixtures/browser-app/model-config.json b/test/fixtures/browser-app/model-config.json index d895c8a..c0686a2 100644 --- a/test/fixtures/browser-app/model-config.json +++ b/test/fixtures/browser-app/model-config.json @@ -2,7 +2,7 @@ "_meta": { "sources": [ "./models", - "../../../node_modules/loopback/common/models" + "loopback/common/models" ] }, "Customer": {