From 1a4e8863ef1bb2104964146931017f04fd5d94bf Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Tue, 5 Aug 2014 16:16:10 +0200 Subject: [PATCH 01/10] Basic plugin architecture Similar to http://mongoosejs.com/docs/plugins.html Next, loopback-boot should be updated to support loading plugins from dirs. --- index.js | 1 + lib/model-builder.js | 17 ++++++++++ lib/model.js | 5 +++ lib/plugins.js | 69 +++++++++++++++++++++++++++++++++++++++ lib/plugins/timestamps.js | 23 +++++++++++++ test/plugins.test.js | 44 +++++++++++++++++++++++++ 6 files changed, 159 insertions(+) create mode 100644 lib/plugins.js create mode 100644 lib/plugins/timestamps.js create mode 100644 test/plugins.test.js diff --git a/index.js b/index.js index a1e5420e..eca8ba9b 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,7 @@ exports.DataSource = exports.Schema = require('./lib/datasource.js').DataSource; exports.ModelBaseClass = require('./lib/model.js'); exports.GeoPoint = require('./lib/geo.js').GeoPoint; exports.ValidationError = require('./lib/validations.js').ValidationError; +exports.plugins = require('./lib/plugins'); exports.__defineGetter__('version', function () { return require('./package.json').version; diff --git a/lib/model-builder.js b/lib/model-builder.js index aa052dea..9fef88b6 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -10,6 +10,7 @@ var DefaultModelBaseClass = require('./model.js'); var List = require('./list.js'); var ModelDefinition = require('./model-definition.js'); var mergeSettings = require('./utils').mergeSettings; +var plugins = require('./plugins'); // Set up types require('./types')(ModelBuilder); @@ -428,6 +429,22 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett ModelClass.registerProperty(propertyName); } + var pluginSettings = settings.plugins || {}; + var keys = Object.keys(pluginSettings); + var size = keys.length; + for (i = 0; i < size; i++) { + var name = keys[i]; + var plugin = pluginSettings[name]; + if (plugin === true) plugin = {}; + if (typeof plugin === 'object') { + pluginSettings[name] = true; + plugins.apply(name, ModelClass, plugin); + } else { + // for settings metadata + pluginSettings[name] = false; + } + } + ModelClass.emit('defined', ModelClass); return ModelClass; diff --git a/lib/model.js b/lib/model.js index 51a59397..1709a226 100644 --- a/lib/model.js +++ b/lib/model.js @@ -12,6 +12,7 @@ var jutil = require('./jutil'); var List = require('./list'); var Hookable = require('./hooks'); var validations = require('./validations.js'); +var plugins = require('./plugins'); // Set up an object for quick lookup var BASE_TYPES = { @@ -402,5 +403,9 @@ ModelBaseClass.prototype.setStrict = function (strict) { this.__strict = strict; }; +ModelBaseClass.plugin = function (name, options) { + plugins.apply(name, this, options); +}; + jutil.mixin(ModelBaseClass, Hookable); jutil.mixin(ModelBaseClass, validations.Validatable); diff --git a/lib/plugins.js b/lib/plugins.js new file mode 100644 index 00000000..9788aed6 --- /dev/null +++ b/lib/plugins.js @@ -0,0 +1,69 @@ +var fs = require('fs'); +var path = require('path'); +var debug = require('debug')('loopback:plugin'); + +var registry = {}; + +exports.apply = function applyPlugin(name, modelClass, options) { + var fn = registry[name]; + if (typeof fn === 'function') { + if (modelClass.dataSource) { + fn(modelClass, options || {}); + } else { + modelClass.once('dataSourceAttached', function() { + fn(modelClass, options || {}); + }); + } + } else { + debug('Invalid plugin: %s', name); + } +}; + +var definePlugin = exports.define = function definePlugin(name, fn) { + if (typeof fn === 'function') { + if (registry[name]) { + debug('Overwriting plugin: %s', name); + } else { + debug('Defined plugin: %s', name); + } + registry[name] = fn; + } else { + debug('Invalid plugin function: %s', name); + } +}; + +var loadPlugin = exports.load = function loadPlugin(dir) { + var files = tryReadDir(path.resolve(dir)); + files.forEach(function(filename) { + var filepath = path.resolve(path.join(dir, filename)); + var ext = path.extname(filename); + var name = path.basename(filename, ext); + var stats = fs.statSync(filepath); + if (stats.isFile()) { + if (ext in require.extensions) { + var plugin = tryRequire(filepath); + if (typeof plugin === 'function') { + definePlugin(name, plugin); + } + } + } + }); +}; + +loadPlugin(path.join(__dirname, 'plugins')); + +function tryReadDir() { + try { + return fs.readdirSync.apply(fs, arguments); + } catch(e) { + return []; + } +}; + +function tryRequire(file) { + try { + return require(file); + } catch(e) { + } +}; + diff --git a/lib/plugins/timestamps.js b/lib/plugins/timestamps.js new file mode 100644 index 00000000..91a4af0f --- /dev/null +++ b/lib/plugins/timestamps.js @@ -0,0 +1,23 @@ +module.exports = function timestamps(Model, options) { + + Model.defineProperty('createdAt', { type: 'date' }); + Model.defineProperty('updatedAt', { type: 'date' }); + + var originalBeforeSave = Model.beforeSave; + Model.beforeSave = function(next, data) { + Model.applyTimestamps(data, this.isNewRecord()); + if (data.createdAt) this.createdAt = data.createdAt; + if (data.updatedAt) this.updatedAt = data.updatedAt; + if (originalBeforeSave) { + originalBeforeSave.apply(this, arguments); + } else { + next(); + } + }; + + Model.applyTimestamps = function(data, creation) { + data.updatedAt = new Date(); + if (creation) data.createdAt = data.updatedAt; + }; + +}; \ No newline at end of file diff --git a/test/plugins.test.js b/test/plugins.test.js new file mode 100644 index 00000000..353d0046 --- /dev/null +++ b/test/plugins.test.js @@ -0,0 +1,44 @@ +// This test written in mocha+should.js +var should = require('./init.js'); +var assert = require('assert'); + +var jdb = require('../'); +var ModelBuilder = jdb.ModelBuilder; +var DataSource = jdb.DataSource; +var Memory = require('../lib/connectors/memory'); + +var plugins = jdb.plugins; + +describe('Model class', function () { + + it('should define a plugin', function() { + plugins.define('example', function(Model, options) { + Model.prototype.example = function() { + return options; + }; + }); + }) + + it('should apply plugin', function(done) { + var memory = new DataSource({connector: Memory}); + var Item = memory.createModel('Item', { name: 'string' }, { + plugins: { timestamps: true } + }); + + Item.plugin('example', { foo: 'bar' }); + + var def = memory.getModelDefinition('Item'); + var json = def.toJSON(); + var properties = json.properties; + properties.createdAt.should.eql({ type: 'Date' }); + properties.updatedAt.should.eql({ type: 'Date' }); + + Item.create({ name: 'Item 1' }, function(err, inst) { + inst.createdAt.should.be.a.date; + inst.updatedAt.should.be.a.date; + inst.example().should.eql({ foo: 'bar' }); + done(); + }); + }); + +}); \ No newline at end of file From f1f692a8a595ee166d2837ac50b13cfd44198343 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Tue, 5 Aug 2014 16:26:34 +0200 Subject: [PATCH 02/10] Minor touch-ups --- lib/plugins.js | 4 ++-- lib/plugins/timestamps.js | 2 +- test/plugins.test.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/plugins.js b/lib/plugins.js index 9788aed6..77129dc8 100644 --- a/lib/plugins.js +++ b/lib/plugins.js @@ -22,9 +22,9 @@ exports.apply = function applyPlugin(name, modelClass, options) { var definePlugin = exports.define = function definePlugin(name, fn) { if (typeof fn === 'function') { if (registry[name]) { - debug('Overwriting plugin: %s', name); + debug('Duplicate plugin: %s', name); } else { - debug('Defined plugin: %s', name); + debug('Defining plugin: %s', name); } registry[name] = fn; } else { diff --git a/lib/plugins/timestamps.js b/lib/plugins/timestamps.js index 91a4af0f..b6ee77d9 100644 --- a/lib/plugins/timestamps.js +++ b/lib/plugins/timestamps.js @@ -20,4 +20,4 @@ module.exports = function timestamps(Model, options) { if (creation) data.createdAt = data.updatedAt; }; -}; \ No newline at end of file +}; diff --git a/test/plugins.test.js b/test/plugins.test.js index 353d0046..847122d1 100644 --- a/test/plugins.test.js +++ b/test/plugins.test.js @@ -41,4 +41,4 @@ describe('Model class', function () { }); }); -}); \ No newline at end of file +}); From 455ee9ec63ff0986b32e3a0fdb8827eb10496322 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Tue, 5 Aug 2014 16:28:17 +0200 Subject: [PATCH 03/10] Fix typo: loadPlugin(s) --- lib/plugins.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/plugins.js b/lib/plugins.js index 77129dc8..32b10aa9 100644 --- a/lib/plugins.js +++ b/lib/plugins.js @@ -32,7 +32,7 @@ var definePlugin = exports.define = function definePlugin(name, fn) { } }; -var loadPlugin = exports.load = function loadPlugin(dir) { +var loadPlugins = exports.load = function loadPlugins(dir) { var files = tryReadDir(path.resolve(dir)); files.forEach(function(filename) { var filepath = path.resolve(path.join(dir, filename)); @@ -50,7 +50,7 @@ var loadPlugin = exports.load = function loadPlugin(dir) { }); }; -loadPlugin(path.join(__dirname, 'plugins')); +loadPlugins(path.join(__dirname, 'plugins')); function tryReadDir() { try { From 35776311fd47eeaecd73dc5922d6b2decfeca5d8 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Wed, 6 Aug 2014 13:26:47 +0200 Subject: [PATCH 04/10] Unified plugins into mixins Mixin types: module function, module object, LDL json object. --- index.js | 2 +- lib/mixins.js | 87 +++++++++++++++++++ .../timestamps.js => mixins/time-stamp.js} | 0 lib/model-builder.js | 18 ++-- lib/model.js | 12 +-- lib/plugins.js | 69 --------------- test/fixtures/mixins/address.json | 12 +++ test/fixtures/mixins/demo.js | 5 ++ test/fixtures/mixins/other.js | 3 + test/mixins.test.js | 82 +++++++++++++++++ test/plugins.test.js | 44 ---------- 11 files changed, 205 insertions(+), 129 deletions(-) create mode 100644 lib/mixins.js rename lib/{plugins/timestamps.js => mixins/time-stamp.js} (100%) delete mode 100644 lib/plugins.js create mode 100644 test/fixtures/mixins/address.json create mode 100644 test/fixtures/mixins/demo.js create mode 100644 test/fixtures/mixins/other.js create mode 100644 test/mixins.test.js delete mode 100644 test/plugins.test.js diff --git a/index.js b/index.js index eca8ba9b..de88d0ab 100644 --- a/index.js +++ b/index.js @@ -1,9 +1,9 @@ +exports.mixins = require('./lib/mixins'); // require before ModelBuilder below - why? exports.ModelBuilder = exports.LDL = require('./lib/model-builder.js').ModelBuilder; exports.DataSource = exports.Schema = require('./lib/datasource.js').DataSource; exports.ModelBaseClass = require('./lib/model.js'); exports.GeoPoint = require('./lib/geo.js').GeoPoint; exports.ValidationError = require('./lib/validations.js').ValidationError; -exports.plugins = require('./lib/plugins'); exports.__defineGetter__('version', function () { return require('./package.json').version; diff --git a/lib/mixins.js b/lib/mixins.js new file mode 100644 index 00000000..2314a0ae --- /dev/null +++ b/lib/mixins.js @@ -0,0 +1,87 @@ +var fs = require('fs'); +var path = require('path'); +var extend = require('util')._extend; +var inflection = require('inflection'); +var debug = require('debug')('loopback:mixin'); +var ModelBuilder = require('./model-builder').ModelBuilder; + +var registry = exports.registry = {}; +var modelBuilder = new ModelBuilder(); + +exports.apply = function applyMixin(modelClass, name, options) { + name = inflection.classify(name.replace(/-/g, '_')); + var fn = registry[name]; + if (typeof fn === 'function') { + if (modelClass.dataSource) { + fn(modelClass, options || {}); + } else { + modelClass.once('dataSourceAttached', function() { + fn(modelClass, options || {}); + }); + } + } else { + debug('Invalid mixin: %s', name); + } +}; + +var defineMixin = exports.define = function defineMixin(name, mixin, ldl) { + if (typeof mixin === 'function' || typeof mixin === 'object') { + name = inflection.classify(name.replace(/-/g, '_')); + if (registry[name]) { + debug('Duplicate mixin: %s', name); + } else { + debug('Defining mixin: %s', name); + } + if (typeof mixin === 'object' && ldl) { + var model = modelBuilder.define(name, mixin); + registry[name] = function(Model, options) { + Model.mixin(model, options); + }; + } else if (typeof mixin === 'object') { + registry[name] = function(Model, options) { + extend(Model.prototype, mixin); + }; + } else if (typeof mixin === 'function') { + registry[name] = mixin; + } + } else { + debug('Invalid mixin function: %s', name); + } +}; + +var loadMixins = exports.load = function loadMixins(dir) { + var files = tryReadDir(path.resolve(dir)); + files.forEach(function(filename) { + var filepath = path.resolve(path.join(dir, filename)); + var ext = path.extname(filename); + var name = path.basename(filename, ext); + var stats = fs.statSync(filepath); + if (stats.isFile()) { + if (ext in require.extensions) { + var mixin = tryRequire(filepath); + if (typeof mixin === 'function' + || typeof mixin === 'object') { + defineMixin(name, mixin, ext === '.json'); + } + } + } + }); +}; + +loadMixins(path.join(__dirname, 'mixins')); + +function tryReadDir() { + try { + return fs.readdirSync.apply(fs, arguments); + } catch(e) { + return []; + } +}; + +function tryRequire(file) { + try { + return require(file); + } catch(e) { + } +}; + diff --git a/lib/plugins/timestamps.js b/lib/mixins/time-stamp.js similarity index 100% rename from lib/plugins/timestamps.js rename to lib/mixins/time-stamp.js diff --git a/lib/model-builder.js b/lib/model-builder.js index 9fef88b6..e46ce3a6 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -10,7 +10,7 @@ var DefaultModelBaseClass = require('./model.js'); var List = require('./list.js'); var ModelDefinition = require('./model-definition.js'); var mergeSettings = require('./utils').mergeSettings; -var plugins = require('./plugins'); +var mixins = require('./mixins'); // Set up types require('./types')(ModelBuilder); @@ -429,19 +429,19 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett ModelClass.registerProperty(propertyName); } - var pluginSettings = settings.plugins || {}; - var keys = Object.keys(pluginSettings); + var mixinSettings = settings.mixins || {}; + var keys = Object.keys(mixinSettings); var size = keys.length; for (i = 0; i < size; i++) { var name = keys[i]; - var plugin = pluginSettings[name]; - if (plugin === true) plugin = {}; - if (typeof plugin === 'object') { - pluginSettings[name] = true; - plugins.apply(name, ModelClass, plugin); + var mixin = mixinSettings[name]; + if (mixin === true) mixin = {}; + if (typeof mixin === 'object') { + mixinSettings[name] = true; + mixins.apply(ModelClass, name, mixin); } else { // for settings metadata - pluginSettings[name] = false; + mixinSettings[name] = false; } } diff --git a/lib/model.js b/lib/model.js index 1709a226..678226d3 100644 --- a/lib/model.js +++ b/lib/model.js @@ -12,7 +12,7 @@ var jutil = require('./jutil'); var List = require('./list'); var Hookable = require('./hooks'); var validations = require('./validations.js'); -var plugins = require('./plugins'); +var mixins = require('./mixins'); // Set up an object for quick lookup var BASE_TYPES = { @@ -388,7 +388,11 @@ ModelBaseClass.prototype.inspect = function () { }; ModelBaseClass.mixin = function (anotherClass, options) { - return jutil.mixin(this, anotherClass, options); + if (typeof anotherClass === 'string') { + mixins.apply(this, anotherClass, options); + } else { + return jutil.mixin(this, anotherClass, options); + } }; ModelBaseClass.prototype.getDataSource = function () { @@ -403,9 +407,5 @@ ModelBaseClass.prototype.setStrict = function (strict) { this.__strict = strict; }; -ModelBaseClass.plugin = function (name, options) { - plugins.apply(name, this, options); -}; - jutil.mixin(ModelBaseClass, Hookable); jutil.mixin(ModelBaseClass, validations.Validatable); diff --git a/lib/plugins.js b/lib/plugins.js deleted file mode 100644 index 32b10aa9..00000000 --- a/lib/plugins.js +++ /dev/null @@ -1,69 +0,0 @@ -var fs = require('fs'); -var path = require('path'); -var debug = require('debug')('loopback:plugin'); - -var registry = {}; - -exports.apply = function applyPlugin(name, modelClass, options) { - var fn = registry[name]; - if (typeof fn === 'function') { - if (modelClass.dataSource) { - fn(modelClass, options || {}); - } else { - modelClass.once('dataSourceAttached', function() { - fn(modelClass, options || {}); - }); - } - } else { - debug('Invalid plugin: %s', name); - } -}; - -var definePlugin = exports.define = function definePlugin(name, fn) { - if (typeof fn === 'function') { - if (registry[name]) { - debug('Duplicate plugin: %s', name); - } else { - debug('Defining plugin: %s', name); - } - registry[name] = fn; - } else { - debug('Invalid plugin function: %s', name); - } -}; - -var loadPlugins = exports.load = function loadPlugins(dir) { - var files = tryReadDir(path.resolve(dir)); - files.forEach(function(filename) { - var filepath = path.resolve(path.join(dir, filename)); - var ext = path.extname(filename); - var name = path.basename(filename, ext); - var stats = fs.statSync(filepath); - if (stats.isFile()) { - if (ext in require.extensions) { - var plugin = tryRequire(filepath); - if (typeof plugin === 'function') { - definePlugin(name, plugin); - } - } - } - }); -}; - -loadPlugins(path.join(__dirname, 'plugins')); - -function tryReadDir() { - try { - return fs.readdirSync.apply(fs, arguments); - } catch(e) { - return []; - } -}; - -function tryRequire(file) { - try { - return require(file); - } catch(e) { - } -}; - diff --git a/test/fixtures/mixins/address.json b/test/fixtures/mixins/address.json new file mode 100644 index 00000000..b91ff47b --- /dev/null +++ b/test/fixtures/mixins/address.json @@ -0,0 +1,12 @@ +{ + "properties": { + "street": { + "type": "string", + "required": true + }, + "city": { + "type": "string", + "required": true + } + } +} \ No newline at end of file diff --git a/test/fixtures/mixins/demo.js b/test/fixtures/mixins/demo.js new file mode 100644 index 00000000..8971ad1a --- /dev/null +++ b/test/fixtures/mixins/demo.js @@ -0,0 +1,5 @@ +module.exports = function timestamps(Model, options) { + + Model.demoMixin = options.ok; + +}; diff --git a/test/fixtures/mixins/other.js b/test/fixtures/mixins/other.js new file mode 100644 index 00000000..b0656196 --- /dev/null +++ b/test/fixtures/mixins/other.js @@ -0,0 +1,3 @@ +module.exports = { + otherMixin: true +} \ No newline at end of file diff --git a/test/mixins.test.js b/test/mixins.test.js new file mode 100644 index 00000000..afa88887 --- /dev/null +++ b/test/mixins.test.js @@ -0,0 +1,82 @@ +// This test written in mocha+should.js +var should = require('./init.js'); +var assert = require('assert'); +var path = require('path'); + +var jdb = require('../'); +var ModelBuilder = jdb.ModelBuilder; +var DataSource = jdb.DataSource; +var Memory = require('../lib/connectors/memory'); + +var mixins = jdb.mixins; + +describe('Model class', function () { + + it('should define a mixin', function() { + mixins.define('Example', function(Model, options) { + Model.prototype.example = function() { + return options; + }; + }); + }); + + it('should load mixins from directory', function() { + var expected = [ 'TimeStamp', 'Example', 'Address', 'Demo', 'Other' ]; + mixins.load(path.join(__dirname, 'fixtures', 'mixins')); + mixins.registry.should.have.property('TimeStamp'); + mixins.registry.should.have.property('Example'); + mixins.registry.should.have.property('Address'); + mixins.registry.should.have.property('Demo'); + mixins.registry.should.have.property('Other'); + }); + + it('should apply a mixin class', function() { + var memory = new DataSource({connector: Memory}); + var Item = memory.createModel('Item', { name: 'string' }, { + mixins: { TimeStamp: true, demo: true, Address: true } + }); + + var modelBuilder = new ModelBuilder(); + var Address = modelBuilder.define('Address', { + street: { type: 'string', required: true }, + city: { type: 'string', required: true } + }); + + Item.mixin(Address); + + var def = memory.getModelDefinition('Item'); + var properties = def.toJSON().properties; + + // properties.street.should.eql({ type: 'String', required: true }); + // properties.city.should.eql({ type: 'String', required: true }); + }); + + it('should apply mixins', function(done) { + var memory = new DataSource({connector: Memory}); + var Item = memory.createModel('Item', { name: 'string' }, { + mixins: { TimeStamp: true, demo: { ok: true }, Address: true } + }); + + Item.mixin('Example', { foo: 'bar' }); + Item.mixin('other'); + + var def = memory.getModelDefinition('Item'); + var properties = def.toJSON().properties; + properties.createdAt.should.eql({ type: 'Date' }); + properties.updatedAt.should.eql({ type: 'Date' }); + + // properties.street.should.eql({ type: 'String', required: true }); + // properties.city.should.eql({ type: 'String', required: true }); + + Item.demoMixin.should.be.true; + Item.prototype.otherMixin.should.be.true; + + Item.create({ name: 'Item 1' }, function(err, inst) { + inst.createdAt.should.be.a.date; + inst.updatedAt.should.be.a.date; + inst.example().should.eql({ foo: 'bar' }); + done(); + }); + }); + +}); diff --git a/test/plugins.test.js b/test/plugins.test.js deleted file mode 100644 index 847122d1..00000000 --- a/test/plugins.test.js +++ /dev/null @@ -1,44 +0,0 @@ -// This test written in mocha+should.js -var should = require('./init.js'); -var assert = require('assert'); - -var jdb = require('../'); -var ModelBuilder = jdb.ModelBuilder; -var DataSource = jdb.DataSource; -var Memory = require('../lib/connectors/memory'); - -var plugins = jdb.plugins; - -describe('Model class', function () { - - it('should define a plugin', function() { - plugins.define('example', function(Model, options) { - Model.prototype.example = function() { - return options; - }; - }); - }) - - it('should apply plugin', function(done) { - var memory = new DataSource({connector: Memory}); - var Item = memory.createModel('Item', { name: 'string' }, { - plugins: { timestamps: true } - }); - - Item.plugin('example', { foo: 'bar' }); - - var def = memory.getModelDefinition('Item'); - var json = def.toJSON(); - var properties = json.properties; - properties.createdAt.should.eql({ type: 'Date' }); - properties.updatedAt.should.eql({ type: 'Date' }); - - Item.create({ name: 'Item 1' }, function(err, inst) { - inst.createdAt.should.be.a.date; - inst.updatedAt.should.be.a.date; - inst.example().should.eql({ foo: 'bar' }); - done(); - }); - }); - -}); From 71398518ff2dde06d84c7dd7a2cb7824831fa593 Mon Sep 17 00:00:00 2001 From: Laurent Date: Wed, 6 Aug 2014 20:40:24 +0200 Subject: [PATCH 05/10] Fix bug when using multiple include keys --- lib/scope.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/scope.js b/lib/scope.js index 8f8dda1c..7fab605f 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -255,7 +255,7 @@ function mergeQuery(base, update) { } else { var saved = base.include; base.include = {}; - base.include[update.include] = [saved]; + base.include[update.include] = saved; } } if (update.collect) { From f671c9c726761a246fcafc058e940db26b617c13 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 8 Aug 2014 01:20:57 -0700 Subject: [PATCH 06/10] Clean up the mixin processing --- index.js | 1 - lib/mixins.js | 118 +++++++++++++----------------- lib/mixins/time-stamp.js | 23 ------ lib/model-builder.js | 19 +++-- lib/model.js | 18 ++++- test/fixtures/mixins/address.json | 12 --- test/fixtures/mixins/demo.js | 5 -- test/fixtures/mixins/other.js | 3 - test/mixins.test.js | 85 +++++++++++---------- 9 files changed, 120 insertions(+), 164 deletions(-) delete mode 100644 lib/mixins/time-stamp.js delete mode 100644 test/fixtures/mixins/address.json delete mode 100644 test/fixtures/mixins/demo.js delete mode 100644 test/fixtures/mixins/other.js diff --git a/index.js b/index.js index de88d0ab..a1e5420e 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,3 @@ -exports.mixins = require('./lib/mixins'); // require before ModelBuilder below - why? exports.ModelBuilder = exports.LDL = require('./lib/model-builder.js').ModelBuilder; exports.DataSource = exports.Schema = require('./lib/datasource.js').DataSource; exports.ModelBaseClass = require('./lib/model.js'); diff --git a/lib/mixins.js b/lib/mixins.js index 2314a0ae..d16d6561 100644 --- a/lib/mixins.js +++ b/lib/mixins.js @@ -1,16 +1,29 @@ -var fs = require('fs'); -var path = require('path'); -var extend = require('util')._extend; -var inflection = require('inflection'); var debug = require('debug')('loopback:mixin'); -var ModelBuilder = require('./model-builder').ModelBuilder; +var assert = require('assert'); +var DefaultModelBaseClass = require('./model.js'); -var registry = exports.registry = {}; -var modelBuilder = new ModelBuilder(); +function isModelClass(cls) { + if (!cls) { + return false; + } + return cls.prototype instanceof DefaultModelBaseClass; +} -exports.apply = function applyMixin(modelClass, name, options) { - name = inflection.classify(name.replace(/-/g, '_')); - var fn = registry[name]; +module.exports = MixinProvider; + +function MixinProvider(modelBuilder) { + this.modelBuilder = modelBuilder; + this.mixins = {}; +} + +/** + * Apply named mixin to the model class + * @param {Model} modelClass + * @param {String} name + * @param {Object} options + */ +MixinProvider.prototype.applyMixin = function applyMixin(modelClass, name, options) { + var fn = this.mixins[name]; if (typeof fn === 'function') { if (modelClass.dataSource) { fn(modelClass, options || {}); @@ -20,68 +33,35 @@ exports.apply = function applyMixin(modelClass, name, options) { }); } } else { - debug('Invalid mixin: %s', name); - } -}; - -var defineMixin = exports.define = function defineMixin(name, mixin, ldl) { - if (typeof mixin === 'function' || typeof mixin === 'object') { - name = inflection.classify(name.replace(/-/g, '_')); - if (registry[name]) { - debug('Duplicate mixin: %s', name); + // Try model name + var model = this.modelBuilder.getModel(name); + if(model) { + debug('Mixin is resolved to a model: %s', name); + modelClass.mixin(model, options); } else { - debug('Defining mixin: %s', name); - } - if (typeof mixin === 'object' && ldl) { - var model = modelBuilder.define(name, mixin); - registry[name] = function(Model, options) { - Model.mixin(model, options); - }; - } else if (typeof mixin === 'object') { - registry[name] = function(Model, options) { - extend(Model.prototype, mixin); - }; - } else if (typeof mixin === 'function') { - registry[name] = mixin; + debug('Invalid mixin: %s', name); } + } +}; + +/** + * Define a mixin with name + * @param {String} name Name of the mixin + * @param {*) mixin The mixin function or a model + */ +MixinProvider.prototype.define = function defineMixin(name, mixin) { + assert(typeof mixin === 'function', 'The mixin must be a function or model class'); + if (this.mixins[name]) { + debug('Duplicate mixin: %s', name); } else { - debug('Invalid mixin function: %s', name); - } -}; - -var loadMixins = exports.load = function loadMixins(dir) { - var files = tryReadDir(path.resolve(dir)); - files.forEach(function(filename) { - var filepath = path.resolve(path.join(dir, filename)); - var ext = path.extname(filename); - var name = path.basename(filename, ext); - var stats = fs.statSync(filepath); - if (stats.isFile()) { - if (ext in require.extensions) { - var mixin = tryRequire(filepath); - if (typeof mixin === 'function' - || typeof mixin === 'object') { - defineMixin(name, mixin, ext === '.json'); - } - } - } - }); -}; - -loadMixins(path.join(__dirname, 'mixins')); - -function tryReadDir() { - try { - return fs.readdirSync.apply(fs, arguments); - } catch(e) { - return []; - } -}; - -function tryRequire(file) { - try { - return require(file); - } catch(e) { + debug('Defining mixin: %s', name); + } + if (isModelClass(mixin)) { + this.mixins[name] = function (Model, options) { + Model.mixin(mixin, options); + }; + } else if (typeof mixin === 'function') { + this.mixins[name] = mixin; } }; diff --git a/lib/mixins/time-stamp.js b/lib/mixins/time-stamp.js deleted file mode 100644 index b6ee77d9..00000000 --- a/lib/mixins/time-stamp.js +++ /dev/null @@ -1,23 +0,0 @@ -module.exports = function timestamps(Model, options) { - - Model.defineProperty('createdAt', { type: 'date' }); - Model.defineProperty('updatedAt', { type: 'date' }); - - var originalBeforeSave = Model.beforeSave; - Model.beforeSave = function(next, data) { - Model.applyTimestamps(data, this.isNewRecord()); - if (data.createdAt) this.createdAt = data.createdAt; - if (data.updatedAt) this.updatedAt = data.updatedAt; - if (originalBeforeSave) { - originalBeforeSave.apply(this, arguments); - } else { - next(); - } - }; - - Model.applyTimestamps = function(data, creation) { - data.updatedAt = new Date(); - if (creation) data.createdAt = data.updatedAt; - }; - -}; diff --git a/lib/model-builder.js b/lib/model-builder.js index e46ce3a6..73936be5 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -10,7 +10,7 @@ var DefaultModelBaseClass = require('./model.js'); var List = require('./list.js'); var ModelDefinition = require('./model-definition.js'); var mergeSettings = require('./utils').mergeSettings; -var mixins = require('./mixins'); +var MixinProvider = require('./mixins'); // Set up types require('./types')(ModelBuilder); @@ -38,6 +38,7 @@ function ModelBuilder() { // create blank models pool this.models = {}; this.definitions = {}; + this.mixins = new MixinProvider(this); this.defaultModelBaseClass = DefaultModelBaseClass; } @@ -189,7 +190,7 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett // Add metadata to the ModelClass hiddenProperty(ModelClass, 'modelBuilder', modelBuilder); - hiddenProperty(ModelClass, 'dataSource', modelBuilder); // Keep for back-compatibility + hiddenProperty(ModelClass, 'dataSource', null); // Keep for back-compatibility hiddenProperty(ModelClass, 'pluralModelName', pluralName); hiddenProperty(ModelClass, 'relations', {}); hiddenProperty(ModelClass, 'http', { path: '/' + pathName }); @@ -430,18 +431,16 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett } var mixinSettings = settings.mixins || {}; - var keys = Object.keys(mixinSettings); - var size = keys.length; + keys = Object.keys(mixinSettings); + size = keys.length; for (i = 0; i < size; i++) { var name = keys[i]; var mixin = mixinSettings[name]; - if (mixin === true) mixin = {}; + if (mixin === true) { + mixin = {}; + } if (typeof mixin === 'object') { - mixinSettings[name] = true; - mixins.apply(ModelClass, name, mixin); - } else { - // for settings metadata - mixinSettings[name] = false; + modelBuilder.mixins.applyMixin(ModelClass, name, mixin); } } diff --git a/lib/model.js b/lib/model.js index 678226d3..b1c28a99 100644 --- a/lib/model.js +++ b/lib/model.js @@ -12,7 +12,6 @@ var jutil = require('./jutil'); var List = require('./list'); var Hookable = require('./hooks'); var validations = require('./validations.js'); -var mixins = require('./mixins'); // Set up an object for quick lookup var BASE_TYPES = { @@ -202,7 +201,11 @@ ModelBaseClass.prototype._initProperties = function (data, options) { * @param {Object} params Various property configuration */ ModelBaseClass.defineProperty = function (prop, params) { - this.dataSource.defineProperty(this.modelName, prop, params); + if(this.dataSource) { + this.dataSource.defineProperty(this.modelName, prop, params); + } else { + this.modelBuilder.defineProperty(this.modelName, prop, params); + } }; ModelBaseClass.getPropertyType = function (propName) { @@ -389,8 +392,17 @@ ModelBaseClass.prototype.inspect = function () { ModelBaseClass.mixin = function (anotherClass, options) { if (typeof anotherClass === 'string') { - mixins.apply(this, anotherClass, options); + this.modelBuilder.mixins.applyMixin(this, anotherClass, options); } else { + if (anotherClass.prototype instanceof ModelBaseClass) { + var props = anotherClass.definition.properties; + for (var i in props) { + if (this.definition.properties[i]) { + continue; + } + this.defineProperty(i, props[i]); + } + } return jutil.mixin(this, anotherClass, options); } }; diff --git a/test/fixtures/mixins/address.json b/test/fixtures/mixins/address.json deleted file mode 100644 index b91ff47b..00000000 --- a/test/fixtures/mixins/address.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "properties": { - "street": { - "type": "string", - "required": true - }, - "city": { - "type": "string", - "required": true - } - } -} \ No newline at end of file diff --git a/test/fixtures/mixins/demo.js b/test/fixtures/mixins/demo.js deleted file mode 100644 index 8971ad1a..00000000 --- a/test/fixtures/mixins/demo.js +++ /dev/null @@ -1,5 +0,0 @@ -module.exports = function timestamps(Model, options) { - - Model.demoMixin = options.ok; - -}; diff --git a/test/fixtures/mixins/other.js b/test/fixtures/mixins/other.js deleted file mode 100644 index b0656196..00000000 --- a/test/fixtures/mixins/other.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - otherMixin: true -} \ No newline at end of file diff --git a/test/mixins.test.js b/test/mixins.test.js index afa88887..f1dacf9d 100644 --- a/test/mixins.test.js +++ b/test/mixins.test.js @@ -1,14 +1,44 @@ // This test written in mocha+should.js var should = require('./init.js'); -var assert = require('assert'); -var path = require('path'); var jdb = require('../'); var ModelBuilder = jdb.ModelBuilder; var DataSource = jdb.DataSource; var Memory = require('../lib/connectors/memory'); -var mixins = jdb.mixins; +var modelBuilder = new ModelBuilder(); +var mixins = modelBuilder.mixins; + +function timestamps(Model, options) { + + Model.defineProperty('createdAt', { type: Date }); + Model.defineProperty('updatedAt', { type: Date }); + + var originalBeforeSave = Model.beforeSave; + Model.beforeSave = function(next, data) { + Model.applyTimestamps(data, this.isNewRecord()); + if (data.createdAt) { + this.createdAt = data.createdAt; + } + if (data.updatedAt) { + this.updatedAt = data.updatedAt; + } + if (originalBeforeSave) { + originalBeforeSave.apply(this, arguments); + } else { + next(); + } + }; + + Model.applyTimestamps = function(data, creation) { + data.updatedAt = new Date(); + if (creation) { + data.createdAt = data.updatedAt; + } + }; +} + +mixins.define('TimeStamp', timestamps); describe('Model class', function () { @@ -20,56 +50,35 @@ describe('Model class', function () { }); }); - it('should load mixins from directory', function() { - var expected = [ 'TimeStamp', 'Example', 'Address', 'Demo', 'Other' ]; - mixins.load(path.join(__dirname, 'fixtures', 'mixins')); - mixins.registry.should.have.property('TimeStamp'); - mixins.registry.should.have.property('Example'); - mixins.registry.should.have.property('Address'); - mixins.registry.should.have.property('Demo'); - mixins.registry.should.have.property('Other'); - }); - it('should apply a mixin class', function() { - var memory = new DataSource({connector: Memory}); - var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: true, Address: true } - }); - - var modelBuilder = new ModelBuilder(); var Address = modelBuilder.define('Address', { street: { type: 'string', required: true }, city: { type: 'string', required: true } }); + + var memory = new DataSource('mem', {connector: Memory}, modelBuilder); + var Item = memory.createModel('Item', { name: 'string' }, { + mixins: { TimeStamp: true, demo: true, Address: true } + }); + + var properties = Item.definition.properties; - Item.mixin(Address); - - var def = memory.getModelDefinition('Item'); - var properties = def.toJSON().properties; - - // properties.street.should.eql({ type: 'String', required: true }); - // properties.city.should.eql({ type: 'String', required: true }); + properties.street.should.eql({ type: String, required: true }); + properties.city.should.eql({ type: String, required: true }); }); it('should apply mixins', function(done) { - var memory = new DataSource({connector: Memory}); + var memory = new DataSource('mem', {connector: Memory}, modelBuilder); var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: { ok: true }, Address: true } + mixins: { TimeStamp: true, demo: { ok: true } } }); Item.mixin('Example', { foo: 'bar' }); Item.mixin('other'); - var def = memory.getModelDefinition('Item'); - var properties = def.toJSON().properties; - properties.createdAt.should.eql({ type: 'Date' }); - properties.updatedAt.should.eql({ type: 'Date' }); - - // properties.street.should.eql({ type: 'String', required: true }); - // properties.city.should.eql({ type: 'String', required: true }); - - Item.demoMixin.should.be.true; - Item.prototype.otherMixin.should.be.true; + var properties = Item.definition.properties; + properties.createdAt.should.eql({ type: Date }); + properties.updatedAt.should.eql({ type: Date }); Item.create({ name: 'Item 1' }, function(err, inst) { inst.createdAt.should.be.a.date; From 7fe7bf9b9a8c911355842f8d5700bbb3986accd5 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 8 Aug 2014 09:01:24 -0700 Subject: [PATCH 07/10] Add scope definitions to the model class See https://github.com/strongloop/loopback/issues/454 --- lib/scope.js | 21 +++++++++++++-------- test/scope.test.js | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index 7fab605f..7ee30b3f 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -8,11 +8,12 @@ exports.defineScope = defineScope; exports.mergeQuery = mergeQuery; function ScopeDefinition(definition) { - this.sourceModel = definition.sourceModel; - this.targetModel = definition.targetModel || definition.sourceModel; + this.modelFrom = definition.modelFrom || definition.sourceModel; + this.modelTo = definition.modelTo || definition.targetModel; this.name = definition.name; this.params = definition.params; this.methods = definition.methods; + this.options = definition.options; } ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { @@ -40,7 +41,7 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres || actualRefresh) { // It either doesn't hit the cache or refresh is required var params = mergeQuery(actualCond, scopeParams); - return this.targetModel.find(params, function (err, data) { + return this.modelTo.find(params, function (err, data) { if (!err && saveOnCache) { defineCachedRelations(self); self.__cachedRelations[name] = data; @@ -62,7 +63,7 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres * to return the query object * @param methods An object of methods keyed by the method name to be bound to the class */ -function defineScope(cls, targetClass, name, params, methods) { +function defineScope(cls, targetClass, name, params, methods, options) { // collect meta info about scope if (!cls._scopeMeta) { @@ -80,13 +81,17 @@ function defineScope(cls, targetClass, name, params, methods) { } var definition = new ScopeDefinition({ - sourceModel: cls, - targetModel: targetClass, + modelFrom: cls, + modelTo: targetClass, name: name, params: params, - methods: methods + methods: methods, + options: options || {} }); + cls.scopes = cls.scopes || {}; + cls.scopes[name] = definition; + // Define a property for the scope Object.defineProperty(cls, name, { enumerable: false, @@ -115,7 +120,7 @@ function defineScope(cls, targetClass, name, params, methods) { f._scope = typeof definition.params === 'function' ? definition.params.call(self) : definition.params; - f._targetClass = definition.targetModel.modelName; + f._targetClass = definition.modelTo.modelName; if (f._scope.collect) { f._targetClass = i8n.capitalize(f._scope.collect); } diff --git a/test/scope.test.js b/test/scope.test.js index ecaa8727..2060c80e 100644 --- a/test/scope.test.js +++ b/test/scope.test.js @@ -9,6 +9,14 @@ describe('scope', function () { db = getSchema(); Railway = db.define('Railway', { URID: {type: String, index: true} + }, { + scopes: { + highSpeed: { + where: { + highSpeed: true + } + } + } }); Station = db.define('Station', { USID: {type: String, index: true}, @@ -24,9 +32,15 @@ describe('scope', function () { Station.destroyAll(done); }); }); + + it('should define scope using options.scopes', function () { + Railway.scopes.should.have.property('highSpeed'); + Railway.highSpeed.should.be.function; + }); it('should define scope with query', function (done) { Station.scope('active', {where: {isActive: true}}); + Station.scopes.should.have.property('active'); Station.active.create(function (err, station) { should.not.exist(err); should.exist(station); From 3d5ec63c996bcb1369fdb757161f925ae0299f16 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 8 Aug 2014 09:25:49 -0700 Subject: [PATCH 08/10] Pass options into scope --- lib/relation-definition.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index bace4705..1171b998 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -553,7 +553,7 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { } return filter; - }, scopeMethods); + }, scopeMethods, definition.options); }; @@ -1600,7 +1600,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) // Mix the property and scoped methods into the prototype class var scopeDefinition = defineScope(modelFrom.prototype, modelTo, accessorName, function () { return {}; - }, scopeMethods); + }, scopeMethods, definition.options); scopeDefinition.related = scopeMethods.related; }; @@ -2014,7 +2014,7 @@ RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo, // Mix the property and scoped methods into the prototype class var scopeDefinition = defineScope(modelFrom.prototype, modelTo, relationName, function () { return {}; - }, scopeMethods); + }, scopeMethods, definition.options); scopeDefinition.related = scopeMethods.related; // bound to definition }; From 366ff3bf62819f1a3ca4a1b8334571140f601b6e Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 8 Aug 2014 09:30:16 -0700 Subject: [PATCH 09/10] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9920da04..d1ba358b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.2.2", + "version": "2.3.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop", From 950be998bc3d04efe5e1f7ab4b7d28245779f82b Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 8 Aug 2014 09:39:36 -0700 Subject: [PATCH 10/10] Fix the test case so that it works with other DBs --- test/relations.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index 6f827760..d1a2003a 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -284,17 +284,17 @@ describe('relations', function () { Address.create({name: 'z'}, function (err, address) { patient.address(address); patient.save(function() { - verify(physician); + verify(physician, address.id); }); }); }); }); - function verify(physician) { + function verify(physician, addressId) { physician.patients({include: 'address'}, function (err, ch) { should.not.exist(err); should.exist(ch); ch.should.have.lengthOf(1); - ch[0].addressId.should.equal(1); + ch[0].addressId.should.eql(addressId); var address = ch[0].address(); should.exist(address); address.should.be.an.instanceof(Address);