From fe74c8019ccde392cd3e2257e6610be5f815d427 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 12 Sep 2014 14:25:35 -0700 Subject: [PATCH 01/20] Tidy up model building from data sources See https://github.com/strongloop/loopback/issues/560 --- lib/datasource.js | 38 ++++++++++++++++---- lib/introspection.js | 6 ++-- lib/model-builder.js | 9 +++-- test/introspection.test.js | 72 ++++++++++++++++++++++++++------------ test/loopback-dl.test.js | 51 +++++++++++++++++---------- 5 files changed, 125 insertions(+), 51 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index edb1d380..572c1276 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -1427,6 +1427,7 @@ DataSource.prototype.discoverSchemasSync = function (modelName, options) { */ DataSource.prototype.discoverAndBuildModels = function (modelName, options, cb) { var self = this; + options = options || {}; this.discoverSchemas(modelName, options, function (err, schemas) { if (err) { cb && cb(err, schemas); @@ -1436,14 +1437,16 @@ DataSource.prototype.discoverAndBuildModels = function (modelName, options, cb) var schemaList = []; for (var s in schemas) { var schema = schemas[s]; + if (options.base) { + schema.options = schema.options || {}; + schema.options.base = options.base; + } schemaList.push(schema); } - var models = self.modelBuilder.buildModels(schemaList); - // Now attach the models to the data source - for (var m in models) { - models[m].attachTo(self); - } + var models = self.modelBuilder.buildModels(schemaList, + self.createModel.bind(self)); + cb && cb(err, models); }); }; @@ -1462,18 +1465,41 @@ DataSource.prototype.discoverAndBuildModels = function (modelName, options, cb) * @param {Object} [options] The options */ DataSource.prototype.discoverAndBuildModelsSync = function (modelName, options) { + options = options || {}; var schemas = this.discoverSchemasSync(modelName, options); var schemaList = []; for (var s in schemas) { var schema = schemas[s]; + if (options.base) { + schema.options = schema.options || {}; + schema.options.base = options.base; + } schemaList.push(schema); } - var models = this.modelBuilder.buildModels(schemaList); + var models = this.modelBuilder.buildModels(schemaList, + this.createModel.bind(this)); + return models; }; +/** + * Introspect a JSON object and build a model class + * @param {String} name Name of the model + * @param {Object} json The json object representing a model instance + * @param {Object} options Options + * @returns {*} + */ +DataSource.prototype.buildModelFromInstance = function (name, json, options) { + + // Introspect the JSON document to generate a schema + var schema = ModelBuilder.introspect(json); + + // Create a model for the generated schema + return this.createModel(name, schema, options); +}; + /** * Check whether migrations needed * This method applies only to SQL connectors. diff --git a/lib/introspection.js b/lib/introspection.js index aa2d5e4a..1ce08198 100644 --- a/lib/introspection.js +++ b/lib/introspection.js @@ -24,12 +24,13 @@ module.exports = function getIntrospector(ModelBuilder) { return 'date'; } + var itemType; if (Array.isArray(value)) { for (var i = 0; i < value.length; i++) { if (value[i] === null || value[i] === undefined) { continue; } - var itemType = introspectType(value[i]); + itemType = introspectType(value[i]); if (itemType) { return [itemType]; } @@ -43,7 +44,7 @@ module.exports = function getIntrospector(ModelBuilder) { var properties = {}; for (var p in value) { - var itemType = introspectType(value[p]); + itemType = introspectType(value[p]); if (itemType) { properties[p] = itemType; } @@ -54,6 +55,7 @@ module.exports = function getIntrospector(ModelBuilder) { return properties; } + ModelBuilder.introspect = introspectType; return introspectType; } diff --git a/lib/model-builder.js b/lib/model-builder.js index df46130e..5962a1a0 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -632,7 +632,7 @@ ModelBuilder.prototype.resolveType = function (type) { * @param {*} schemas The schemas * @returns {Object} A map of model constructors keyed by model name */ -ModelBuilder.prototype.buildModels = function (schemas) { +ModelBuilder.prototype.buildModels = function (schemas, createModel) { var models = {}; // Normalize the schemas to be an array of the schema objects {name: , properties: {}, options: {}} @@ -656,7 +656,12 @@ ModelBuilder.prototype.buildModels = function (schemas) { for (var s in schemas) { var name = this.getSchemaName(schemas[s].name); schemas[s].name = name; - var model = this.define(schemas[s].name, schemas[s].properties, schemas[s].options); + var model; + if(typeof createModel === 'function') { + model = createModel(schemas[s].name, schemas[s].properties, schemas[s].options); + } else { + model = this.define(schemas[s].name, schemas[s].properties, schemas[s].options); + } models[name] = model; relations = relations.concat(model.definition.relations); } diff --git a/test/introspection.test.js b/test/introspection.test.js index 6fda1bcb..720b9e0c 100644 --- a/test/introspection.test.js +++ b/test/introspection.test.js @@ -1,8 +1,29 @@ var assert = require('assert'); -var ModelBuilder = require('../lib/model-builder').ModelBuilder; +var ModelBuilder = require('..').ModelBuilder; +var DataSource = require('../').DataSource; var introspectType = require('../lib/introspection')(ModelBuilder); var traverse = require('traverse'); +var json = { + name: 'Joe', + age: 30, + birthday: new Date(), + vip: true, + address: { + street: '1 Main St', + city: 'San Jose', + state: 'CA', + zipcode: '95131', + country: 'US' + }, + friends: ['John', 'Mary'], + emails: [ + {label: 'work', id: 'x@sample.com'}, + {label: 'home', id: 'x@home.com'} + ], + tags: [] +}; + describe('Introspection of model definitions from JSON', function () { it('should handle simple types', function () { @@ -61,27 +82,6 @@ describe('Introspection of model definitions from JSON', function () { }); it('should build a model from the introspected schema', function (done) { - - var json = { - name: 'Joe', - age: 30, - birthday: new Date(), - vip: true, - address: { - street: '1 Main St', - city: 'San Jose', - state: 'CA', - zipcode: '95131', - country: 'US' - }, - friends: ['John', 'Mary'], - emails: [ - {label: 'work', id: 'x@sample.com'}, - {label: 'home', id: 'x@home.com'} - ], - tags: [] - }; - var copy = traverse(json).clone(); var schema = introspectType(json); @@ -97,5 +97,33 @@ describe('Introspection of model definitions from JSON', function () { assert.deepEqual(obj, copy); done(); }); + + it('should build a model using buildModelFromInstance', function (done) { + var copy = traverse(json).clone(); + + var builder = new ModelBuilder(); + var Model = builder.buildModelFromInstance('MyModel', copy, {idInjection: false}); + + var obj = new Model(json); + obj = obj.toObject(); + assert.deepEqual(obj, copy); + done(); + }); + + it('should build a model using DataSource.buildModelFromInstance', function (done) { + var copy = traverse(json).clone(); + + var builder = new DataSource('memory'); + var Model = builder.buildModelFromInstance('MyModel', copy, + {idInjection: false}); + + assert.equal(Model.dataSource, builder); + + var obj = new Model(json); + obj = obj.toObject(); + assert.deepEqual(obj, copy); + done(); + }); + }); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index bb02bfcf..10c9c10e 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -1418,28 +1418,31 @@ describe('DataAccessObject', function () { }); describe('Load models from json', function () { - it('should be able to define models from json', function () { - var path = require('path'), - fs = require('fs'); - - /** - * Load LDL schemas from a json doc - * @param schemaFile The dataSource json file - * @returns A map of schemas keyed by name - */ - function loadSchemasSync(schemaFile, dataSource) { - // Set up the data source - if (!dataSource) { - dataSource = new DataSource('memory'); - } - - // Read the dataSource JSON file - var schemas = JSON.parse(fs.readFileSync(schemaFile)); - - return dataSource.modelBuilder.buildModels(schemas); + var path = require('path'), + fs = require('fs'); + /** + * Load LDL schemas from a json doc + * @param schemaFile The dataSource json file + * @returns A map of schemas keyed by name + */ + function loadSchemasSync(schemaFile, dataSource) { + var modelBuilder, createModel; + // Set up the data source + if (!dataSource) { + modelBuilder = new ModelBuilder(); + } else { + modelBuilder = dataSource.modelBuilder; + createModel = dataSource.createModel.bind(dataSource); } + // Read the dataSource JSON file + var schemas = JSON.parse(fs.readFileSync(schemaFile)); + return modelBuilder.buildModels(schemas, createModel); + } + + it('should be able to define models from json', function () { + var models = loadSchemasSync(path.join(__dirname, 'test1-schemas.json')); models.should.have.property('AnonymousModel_0'); @@ -1459,6 +1462,16 @@ describe('Load models from json', function () { } }); + it('should be able to define models from json using dataSource', function() { + var ds = new DataSource('memory'); + + var models = loadSchemasSync(path.join(__dirname, 'test2-schemas.json'), ds); + models.should.have.property('Address'); + models.should.have.property('Account'); + models.should.have.property('Customer'); + assert.equal(models.Address.dataSource, ds); + }); + it('should allow customization of default model base class', function () { var modelBuilder = new ModelBuilder(); From 28a661a81a75d0afb149777dcb816a2cf057d2de Mon Sep 17 00:00:00 2001 From: Khashayar Hajian Date: Wed, 17 Sep 2014 17:28:40 +0200 Subject: [PATCH 02/20] Test improvement, shows _targetClass camelCase bug There is an issue where setting _targetClass on hasAndBelongsToMany relations with a camel-case model name, fails. Signed-off-by: Khashayar Hajian --- test/relations.test.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index b99f31ba..48173115 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1604,15 +1604,15 @@ describe('relations', function () { }); describe('hasAndBelongsToMany', function () { - var Article, Tag, ArticleTag; + var Article, TagName, ArticleTag; it('can be declared', function (done) { Article = db.define('Article', {title: String}); - Tag = db.define('Tag', {name: String}); - Article.hasAndBelongsToMany('tags'); - ArticleTag = db.models.ArticleTag; + TagName = db.define('TagName', {name: String}); + Article.hasAndBelongsToMany('tagNames'); + ArticleTag = db.models.ArticleTagName; db.automigrate(function () { Article.destroyAll(function () { - Tag.destroyAll(function () { + TagName.destroyAll(function () { ArticleTag.destroyAll(done) }); }); @@ -1621,11 +1621,11 @@ describe('relations', function () { it('should allow to create instances on scope', function (done) { Article.create(function (e, article) { - article.tags.create({name: 'popular'}, function (e, t) { - t.should.be.an.instanceOf(Tag); + article.tagNames.create({name: 'popular'}, function (e, t) { + t.should.be.an.instanceOf(TagName); ArticleTag.findOne(function (e, at) { should.exist(at); - at.tagId.toString().should.equal(t.id.toString()); + at.tagNameId.toString().should.equal(t.id.toString()); at.articleId.toString().should.equal(article.id.toString()); done(); }); @@ -1635,7 +1635,7 @@ describe('relations', function () { it('should allow to fetch scoped instances', function (done) { Article.findOne(function (e, article) { - article.tags(function (e, tags) { + article.tagNames(function (e, tags) { should.not.exist(e); should.exist(tags); done(); @@ -1645,12 +1645,12 @@ describe('relations', function () { it('should allow to add connection with instance', function (done) { Article.findOne(function (e, article) { - Tag.create({name: 'awesome'}, function (e, tag) { - article.tags.add(tag, function (e, at) { + TagName.create({name: 'awesome'}, function (e, tag) { + article.tagNames.add(tag, function (e, at) { should.not.exist(e); should.exist(at); at.should.be.an.instanceOf(ArticleTag); - at.tagId.should.equal(tag.id); + at.tagNameId.should.equal(tag.id); at.articleId.should.equal(article.id); done(); }); @@ -1660,12 +1660,12 @@ describe('relations', function () { it('should allow to remove connection with instance', function (done) { Article.findOne(function (e, article) { - article.tags(function (e, tags) { + article.tagNames(function (e, tags) { var len = tags.length; tags.should.not.be.empty; - article.tags.remove(tags[0], function (e) { + article.tagNames.remove(tags[0], function (e) { should.not.exist(e); - article.tags(true, function (e, tags) { + article.tagNames(true, function (e, tags) { tags.should.have.lengthOf(len - 1); done(); }); @@ -1675,7 +1675,7 @@ describe('relations', function () { }); it('should set targetClass on scope property', function() { - should.equal(Article.prototype.tags._targetClass, 'Tag'); + should.equal(Article.prototype.tagNames._targetClass, 'TagName'); }); }); From c3df5137128007c661619a30bfdd7cc1a76d97c8 Mon Sep 17 00:00:00 2001 From: Khashayar Hajian Date: Fri, 19 Sep 2014 16:23:39 +0200 Subject: [PATCH 03/20] Fix camel-case issue where relation is 'hasAndBelongsToMany' #304 Signed-off-by: Khashayar Hajian --- lib/scope.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/scope.js b/lib/scope.js index 5124251d..d8ebb7c3 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -162,7 +162,7 @@ function defineScope(cls, targetClass, name, params, methods, options) { f._targetClass = targetModel.modelName; if (f._scope.collect) { - f._targetClass = i8n.capitalize(f._scope.collect); + f._targetClass = i8n.camelize(f._scope.collect); } f.build = build; From d3b30c307154b4edf275e2738ccc872ac192fdac Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Wed, 1 Oct 2014 17:55:02 -0700 Subject: [PATCH 04/20] Add contribution guidelines Uses https://cla.strongloop.com --- CONTRIBUTING.md | 151 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..f091e9e3 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,151 @@ +### Contributing ### + +Thank you for your interest in `loopback-datasource-juggler`, an open source project +administered by StrongLoop. + +Contributing to `loopback-datasource-juggler` is easy. In a few simple steps: + + * Ensure that your effort is aligned with the project's roadmap by + talking to the maintainers, especially if you are going to spend a + lot of time on it. + + * Make something better or fix a bug. + + * Adhere to code style outlined in the [Google C++ Style Guide][] and + [Google Javascript Style Guide][]. + + * Sign the [Contributor License Agreement](https://cla.strongloop.com/strongloop/loopback-datasource-juggler) + + * Submit a pull request through Github. + + +### Contributor License Agreement ### + +``` + Individual Contributor License Agreement + + By signing this Individual Contributor License Agreement + ("Agreement"), and making a Contribution (as defined below) to + StrongLoop, Inc. ("StrongLoop"), You (as defined below) accept and + agree to the following terms and conditions for Your present and + future Contributions submitted to StrongLoop. Except for the license + granted in this Agreement to StrongLoop and recipients of software + distributed by StrongLoop, You reserve all right, title, and interest + in and to Your Contributions. + + 1. Definitions + + "You" or "Your" shall mean the copyright owner or the individual + authorized by the copyright owner that is entering into this + Agreement with StrongLoop. + + "Contribution" shall mean any original work of authorship, + including any modifications or additions to an existing work, that + is intentionally submitted by You to StrongLoop for inclusion in, + or documentation of, any of the products owned or managed by + StrongLoop ("Work"). For purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication + sent to StrongLoop or its representatives, including but not + limited to communication or electronic mailing lists, source code + control systems, and issue tracking systems that are managed by, + or on behalf of, StrongLoop for the purpose of discussing and + improving the Work, but excluding communication that is + conspicuously marked or otherwise designated in writing by You as + "Not a Contribution." + + 2. You Grant a Copyright License to StrongLoop + + Subject to the terms and conditions of this Agreement, You hereby + grant to StrongLoop and recipients of software distributed by + StrongLoop, a perpetual, worldwide, non-exclusive, no-charge, + royalty-free, irrevocable copyright license to reproduce, prepare + derivative works of, publicly display, publicly perform, + sublicense, and distribute Your Contributions and such derivative + works under any license and without any restrictions. + + 3. You Grant a Patent License to StrongLoop + + Subject to the terms and conditions of this Agreement, You hereby + grant to StrongLoop and to recipients of software distributed by + StrongLoop a perpetual, worldwide, non-exclusive, no-charge, + royalty-free, irrevocable (except as stated in this Section) + patent license to make, have made, use, offer to sell, sell, + import, and otherwise transfer the Work under any license and + without any restrictions. The patent license You grant to + StrongLoop under this Section applies only to those patent claims + licensable by You that are necessarily infringed by Your + Contributions(s) alone or by combination of Your Contributions(s) + with the Work to which such Contribution(s) was submitted. If any + entity institutes a patent litigation against You or any other + entity (including a cross-claim or counterclaim in a lawsuit) + alleging that Your Contribution, or the Work to which You have + contributed, constitutes direct or contributory patent + infringement, any patent licenses granted to that entity under + this Agreement for that Contribution or Work shall terminate as + of the date such litigation is filed. + + 4. You Have the Right to Grant Licenses to StrongLoop + + You represent that You are legally entitled to grant the licenses + in this Agreement. + + If Your employer(s) has rights to intellectual property that You + create, You represent that You have received permission to make + the Contributions on behalf of that employer, that Your employer + has waived such rights for Your Contributions, or that Your + employer has executed a separate Corporate Contributor License + Agreement with StrongLoop. + + 5. The Contributions Are Your Original Work + + You represent that each of Your Contributions are Your original + works of authorship (see Section 8 (Submissions on Behalf of + Others) for submission on behalf of others). You represent that to + Your knowledge, no other person claims, or has the right to claim, + any right in any intellectual property right related to Your + Contributions. + + You also represent that You are not legally obligated, whether by + entering into an agreement or otherwise, in any way that conflicts + with the terms of this Agreement. + + You represent that Your Contribution submissions include complete + details of any third-party license or other restriction (including, + but not limited to, related patents and trademarks) of which You + are personally aware and which are associated with any part of + Your Contributions. + + 6. You Don't Have an Obligation to Provide Support for Your Contributions + + You are not expected to provide support for Your Contributions, + except to the extent You desire to provide support. You may provide + support for free, for a fee, or not at all. + + 6. No Warranties or Conditions + + StrongLoop acknowledges that unless required by applicable law or + agreed to in writing, You provide Your Contributions on an "AS IS" + BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, EITHER + EXPRESS OR IMPLIED, INCLUDING, WITHOUT LIMITATION, ANY WARRANTIES + OR CONDITIONS OF TITLE, NON-INFRINGEMENT, MERCHANTABILITY, OR + FITNESS FOR A PARTICULAR PURPOSE. + + 7. Submission on Behalf of Others + + If You wish to submit work that is not Your original creation, You + may submit it to StrongLoop separately from any Contribution, + identifying the complete details of its source and of any license + or other restriction (including, but not limited to, related + patents, trademarks, and license agreements) of which You are + personally aware, and conspicuously marking the work as + "Submitted on Behalf of a Third-Party: [named here]". + + 8. Agree to Notify of Change of Circumstances + + You agree to notify StrongLoop of any facts or circumstances of + which You become aware that would make these representations + inaccurate in any respect. Email us at callback@strongloop.com. +``` + +[Google C++ Style Guide]: https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml +[Google Javascript Style Guide]: https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml From 610866bd7c0f2e8183daa12eecb246acce1ba51b Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 11:13:47 +0200 Subject: [PATCH 05/20] Extract mergeQuery and setScopeValuesFromWhere Related to #274 - in preparation of default scope --- lib/dao.js | 36 +++++++++++++-- lib/model.js | 4 ++ lib/relation-definition.js | 3 +- lib/scope.js | 89 +------------------------------------- lib/utils.js | 88 +++++++++++++++++++++++++++++++++++++ test/loopback-dl.test.js | 2 +- 6 files changed, 129 insertions(+), 93 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index c4236a91..1d308f73 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -13,11 +13,12 @@ var Relation = require('./relations.js'); var Inclusion = require('./include.js'); var List = require('./list.js'); var geo = require('./geo'); -var mergeQuery = require('./scope.js').mergeQuery; var Memory = require('./connectors/memory').Memory; var utils = require('./utils'); var fieldsToArray = utils.fieldsToArray; var removeUndefined = utils.removeUndefined; +var setScopeValuesFromWhere = utils.setScopeValuesFromWhere; +var mergeQuery = utils.mergeQuery; var util = require('util'); var assert = require('assert'); @@ -69,6 +70,18 @@ DataAccessObject._forDB = function (data) { return res; }; +DataAccessObject.applyProperties = function(data) { + var scope = this.definition.settings.scope || {}; + if (typeof scope.where === 'object' + && this.definition.settings.applyProperties !== false) { + setScopeValuesFromWhere(data, scope.where, this); + } +}; + +DataAccessObject.applyScope = function(cond) { + +}; + /** * Create an instance of Model with given data and save to the attached data source. Callback is optional. * Example: @@ -136,6 +149,7 @@ DataAccessObject.create = function (data, callback) { } } + var enforced = {}; var obj; var idValue = getIdValue(this, data); @@ -145,6 +159,10 @@ DataAccessObject.create = function (data, callback) { } else { obj = new Model(data); } + + this.applyProperties(enforced); + obj.setAttributes(enforced); + data = obj.toObject(true); // validation required @@ -220,6 +238,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data inst = new Model(data); } update = inst.toObject(false); + this.applyProperties(update); update = removeUndefined(update); this.getDataSource().connector.updateOrCreate(Model.modelName, update, function (err, data) { var obj; @@ -926,13 +945,17 @@ DataAccessObject.prototype.save = function (options, callback) { if (!('throws' in options)) { options.throws = false; } - + var inst = this; var data = inst.toObject(true); var modelName = Model.modelName; - + + Model.applyProperties(data, this); + if (this.isNewRecord()) { return Model.create(this, callback); + } else { + inst.setAttributes(data); } // validate first @@ -1025,6 +1048,9 @@ DataAccessObject.updateAll = function (where, data, cb) { cb && cb(err); }); } + + this.applyProperties(data); + var connector = this.getDataSource().connector; connector.update(this.modelName, where, data, cb); }; @@ -1075,7 +1101,7 @@ DataAccessObject.prototype.remove = * @param {Mixed} value Value of property */ DataAccessObject.prototype.setAttribute = function setAttribute(name, value) { - this[name] = value; + this[name] = value; // TODO [fabien] - currently not protected by applyProperties }; /** @@ -1101,6 +1127,8 @@ DataAccessObject.prototype.updateAttribute = function updateAttribute(name, valu DataAccessObject.prototype.setAttributes = function setAttributes(data) { if (typeof data !== 'object') return; + this.constructor.applyProperties(data); + var Model = this.constructor; var inst = this; diff --git a/lib/model.js b/lib/model.js index e12b9b4b..7bb19eb7 100644 --- a/lib/model.js +++ b/lib/model.js @@ -60,6 +60,10 @@ ModelBaseClass.prototype._initProperties = function (data, options) { } var properties = _extend({}, ctor.definition.properties); data = data || {}; + + if (typeof ctor.applyProperties === 'function') { + ctor.applyProperties(data); + } options = options || {}; var applySetters = options.applySetters; diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 2590a3a5..1fa299e4 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -3,9 +3,10 @@ */ var assert = require('assert'); var util = require('util'); +var utils = require('./utils'); var i8n = require('inflection'); var defineScope = require('./scope.js').defineScope; -var mergeQuery = require('./scope.js').mergeQuery; +var mergeQuery = utils.mergeQuery; var ModelBaseClass = require('./model.js'); var applyFilter = require('./connectors/memory').applyFilter; var ValidationError = require('./validations.js').ValidationError; diff --git a/lib/scope.js b/lib/scope.js index 6b4dafb1..dfdb364d 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -1,13 +1,14 @@ var i8n = require('inflection'); var utils = require('./utils'); var defineCachedRelations = utils.defineCachedRelations; +var setScopeValuesFromWhere = utils.setScopeValuesFromWhere; +var mergeQuery = utils.mergeQuery; var DefaultModelBaseClass = require('./model.js'); /** * Module exports */ exports.defineScope = defineScope; -exports.mergeQuery = mergeQuery; function ScopeDefinition(definition) { this.isStatic = definition.isStatic; @@ -229,35 +230,6 @@ function defineScope(cls, targetClass, name, params, methods, options) { cls['__count__' + name] = fn_count; - /* - * Extracting fixed property values for the scope from the where clause into - * the data object - * - * @param {Object} The data object - * @param {Object} The where clause - */ - function setScopeValuesFromWhere(data, where, targetModel) { - for (var i in where) { - if (i === 'and') { - // Find fixed property values from each subclauses - for (var w = 0, n = where[i].length; w < n; w++) { - setScopeValuesFromWhere(data, where[i][w], targetModel); - } - continue; - } - var prop = targetModel.definition.properties[i]; - if (prop) { - var val = where[i]; - if (typeof val !== 'object' || val instanceof prop.type - || prop.type.name === 'ObjectID') // MongoDB key - { - // Only pick the {propertyName: propertyValue} - data[i] = where[i]; - } - } - } - } - // and it should have create/build methods with binded thisModelNameId param function build(data) { data = data || {}; @@ -300,60 +272,3 @@ function defineScope(cls, targetClass, name, params, methods, options) { return definition; } - -/*! - * Merge query parameters - * @param {Object} base The base object to contain the merged results - * @param {Object} update The object containing updates to be merged - * @returns {*|Object} The base object - * @private - */ -function mergeQuery(base, update) { - if (!update) { - return; - } - base = base || {}; - if (update.where && Object.keys(update.where).length > 0) { - if (base.where && Object.keys(base.where).length > 0) { - base.where = {and: [base.where, update.where]}; - } else { - base.where = update.where; - } - } - - // Merge inclusion - if (update.include) { - if (!base.include) { - base.include = update.include; - } else { - var saved = base.include; - base.include = {}; - base.include[update.include] = saved; - } - } - if (update.collect) { - base.collect = update.collect; - } - - // set order - if (!base.order && update.order) { - base.order = update.order; - } - - // overwrite pagination - if (update.limit !== undefined) { - base.limit = update.limit; - } - if (update.skip !== undefined) { - base.skip = update.skip; - } - if (update.offset !== undefined) { - base.offset = update.offset; - } - - // Overwrite fields - if (update.fields !== undefined) { - base.fields = update.fields; - } - return base; -} diff --git a/lib/utils.js b/lib/utils.js index c774ecae..bbde56fc 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -7,6 +7,8 @@ exports.mergeSettings = mergeSettings; exports.isPlainObject = isPlainObject; exports.defineCachedRelations = defineCachedRelations; exports.sortObjectsByIds = sortObjectsByIds; +exports.setScopeValuesFromWhere = setScopeValuesFromWhere; +exports.mergeQuery = mergeQuery; var traverse = require('traverse'); @@ -21,6 +23,92 @@ function safeRequire(module) { } } +/* + * Extracting fixed property values for the scope from the where clause into + * the data object + * + * @param {Object} The data object + * @param {Object} The where clause + */ +function setScopeValuesFromWhere(data, where, targetModel) { + for (var i in where) { + if (i === 'and') { + // Find fixed property values from each subclauses + for (var w = 0, n = where[i].length; w < n; w++) { + setScopeValuesFromWhere(data, where[i][w], targetModel); + } + continue; + } + var prop = targetModel.definition.properties[i]; + if (prop) { + var val = where[i]; + if (typeof val !== 'object' || val instanceof prop.type + || prop.type.name === 'ObjectID') // MongoDB key + { + // Only pick the {propertyName: propertyValue} + data[i] = where[i]; + } + } + } +} + +/*! + * Merge query parameters + * @param {Object} base The base object to contain the merged results + * @param {Object} update The object containing updates to be merged + * @returns {*|Object} The base object + * @private + */ +function mergeQuery(base, update) { + if (!update) { + return; + } + base = base || {}; + if (update.where && Object.keys(update.where).length > 0) { + if (base.where && Object.keys(base.where).length > 0) { + base.where = {and: [base.where, update.where]}; + } else { + base.where = update.where; + } + } + + // Merge inclusion + if (update.include) { + if (!base.include) { + base.include = update.include; + } else { + var saved = base.include; + base.include = {}; + base.include[update.include] = saved; + } + } + if (update.collect) { + base.collect = update.collect; + } + + // set order + if (!base.order && update.order) { + base.order = update.order; + } + + // overwrite pagination + if (update.limit !== undefined) { + base.limit = update.limit; + } + if (update.skip !== undefined) { + base.skip = update.skip; + } + if (update.offset !== undefined) { + base.offset = update.offset; + } + + // Overwrite fields + if (update.fields !== undefined) { + base.fields = update.fields; + } + return base; +} + function fieldsToArray(fields, properties) { if (!fields) return; diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 10c9c10e..41d600f6 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -639,7 +639,7 @@ describe('Load models with base', function () { assert(Customer.prototype.instanceMethod === User.prototype.instanceMethod); assert.equal(Customer.base, User); assert.equal(Customer.base, Customer.super_); - + try { var Customer1 = ds.define('Customer1', {vip: Boolean}, {base: 'User1'}); } catch (e) { From 8352ed3afc0e6f6222e063b92d0032a0f4af693d Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 14:09:30 +0200 Subject: [PATCH 06/20] Implemented collection setting for Memory connector --- lib/connectors/memory.js | 70 +++++++++++++++++++++++++--------------- test/memory.test.js | 42 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 26 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index ea9c9d3a..3490dfe3 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -70,6 +70,26 @@ function deserialize(dbObj) { } } +Memory.prototype.getCollection = function(model) { + var modelClass = this._models[model]; + if (modelClass.settings.memory) { + model = modelClass.settings.memory.collection || model; + } + return model; +} + +Memory.prototype.collection = function(model, val) { + model = this.getCollection(model); + if (arguments.length > 1) this.cache[model] = val; + return this.cache[model]; +}; + +Memory.prototype.collectionSeq = function(model, val) { + model = this.getCollection(model); + if (arguments.length > 1) this.ids[model] = val; + return this.ids[model]; +}; + Memory.prototype.loadFromFile = function(callback) { var self = this; var hasLocalStorage = typeof window !== 'undefined' && window.localStorage; @@ -161,36 +181,34 @@ Memory.prototype.saveToFile = function (result, callback) { Memory.prototype.define = function defineModel(definition) { this.constructor.super_.prototype.define.apply(this, [].slice.call(arguments)); var m = definition.model.modelName; - if(!this.cache[m]) { - this.cache[m] = {}; - this.ids[m] = 1; + if(!this.collection(m)) { + this.collection(m, {}); + this.collectionSeq(m, 1); } }; Memory.prototype.create = function create(model, data, callback) { // FIXME: [rfeng] We need to generate unique ids based on the id type // FIXME: [rfeng] We don't support composite ids yet - var currentId = this.ids[model]; - if (currentId === undefined) { - // First time - this.ids[model] = 1; - currentId = 1; + var currentId = this.collectionSeq(model); + if (currentId === undefined) { // First time + currentId = this.collectionSeq(model, 1); } var id = this.getIdValue(model, data) || currentId; if (id > currentId) { // If the id is passed in and the value is greater than the current id currentId = id; } - this.ids[model] = Number(currentId) + 1; + this.collectionSeq(model, Number(currentId) + 1); var props = this._models[model].properties; var idName = this.idName(model); id = (props[idName] && props[idName].type && props[idName].type(id)) || id; this.setIdValue(model, data, id); - if(!this.cache[model]) { - this.cache[model] = {}; + if(!this.collection(model)) { + this.collection(model, {}); } - this.cache[model][id] = serialize(data); + this.collection(model)[id] = serialize(data); this.saveToFile(id, callback); }; @@ -210,30 +228,30 @@ Memory.prototype.updateOrCreate = function (model, data, callback) { Memory.prototype.save = function save(model, data, callback) { var id = this.getIdValue(model, data); - var cachedModels = this.cache[model]; - var modelData = cachedModels && this.cache[model][id]; + var cachedModels = this.collection(model); + var modelData = cachedModels && this.collection(model)[id]; modelData = modelData && deserialize(modelData); if (modelData) { data = merge(modelData, data); } - this.cache[model][id] = serialize(data); + this.collection(model)[id] = serialize(data); this.saveToFile(data, callback); }; Memory.prototype.exists = function exists(model, id, callback) { process.nextTick(function () { - callback(null, this.cache[model] && this.cache[model].hasOwnProperty(id)); + callback(null, this.collection(model) && this.collection(model).hasOwnProperty(id)); }.bind(this)); }; Memory.prototype.find = function find(model, id, callback) { process.nextTick(function () { - callback(null, id in this.cache[model] && this.fromDb(model, this.cache[model][id])); + callback(null, id in this.collection(model) && this.fromDb(model, this.collection(model)[id])); }.bind(this)); }; Memory.prototype.destroy = function destroy(model, id, callback) { - delete this.cache[model][id]; + delete this.collection(model)[id]; this.saveToFile(null, callback); }; @@ -266,8 +284,8 @@ Memory.prototype.fromDb = function (model, data) { Memory.prototype.all = function all(model, filter, callback) { var self = this; - var nodes = Object.keys(this.cache[model]).map(function (key) { - return this.fromDb(model, this.cache[model][key]); + var nodes = Object.keys(this.collection(model)).map(function (key) { + return this.fromDb(model, this.collection(model)[key]); }.bind(this)); if (filter) { @@ -505,7 +523,7 @@ Memory.prototype.destroyAll = function destroyAll(model, where, callback) { callback = where; where = undefined; } - var cache = this.cache[model]; + var cache = this.collection(model); var filter = null; if (where) { filter = applyFilter({where: where}); @@ -516,13 +534,13 @@ Memory.prototype.destroyAll = function destroyAll(model, where, callback) { } }.bind(this)); if (!where) { - this.cache[model] = {}; + this.collection(model, {}); } this.saveToFile(null, callback); }; Memory.prototype.count = function count(model, callback, where) { - var cache = this.cache[model]; + var cache = this.collection(model); var data = Object.keys(cache); if (where) { var filter = {where: where}; @@ -539,7 +557,7 @@ Memory.prototype.count = function count(model, callback, where) { Memory.prototype.update = Memory.prototype.updateAll = function updateAll(model, where, data, cb) { var self = this; - var cache = this.cache[model]; + var cache = this.collection(model); var filter = null; where = where || {}; filter = applyFilter({where: where}); @@ -571,8 +589,8 @@ Memory.prototype.updateAttributes = function updateAttributes(model, id, data, c this.setIdValue(model, data, id); - var cachedModels = this.cache[model]; - var modelData = cachedModels && this.cache[model][id]; + var cachedModels = this.collection(model); + var modelData = cachedModels && this.collection(model)[id]; if (modelData) { this.save(model, data, cb); diff --git a/test/memory.test.js b/test/memory.test.js index 453ac9e8..66ad2346 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -269,6 +269,48 @@ describe('Memory connector', function () { } }); + + it('should use collection setting', function (done) { + var ds = new DataSource({ + connector: 'memory' + }); + + var Product = ds.createModel('Product', { + name: String + }); + + var Tool = ds.createModel('Tool', { + name: String + }, {memory: {collection: 'Product'}}); + + var Widget = ds.createModel('Widget', { + name: String + }, {memory: {collection: 'Product'}}); + + ds.connector.getCollection('Tool').should.equal('Product'); + ds.connector.getCollection('Widget').should.equal('Product'); + + async.series([ + function(next) { + Tool.create({ name: 'Tool A' }, next); + }, + function(next) { + Tool.create({ name: 'Tool B' }, next); + }, + function(next) { + Widget.create({ name: 'Widget A' }, next); + } + ], function(err) { + Product.find(function(err, products) { + should.not.exist(err); + products.should.have.length(3); + products[0].toObject().should.eql({ name: 'Tool A', id: 1 }); + products[1].toObject().should.eql({ name: 'Tool B', id: 2 }); + products[2].toObject().should.eql({ name: 'Widget A', id: 3 }); + done(); + }); + }); + }); }); From ad55681d69743bb9b9c240f35d9c86e04c1b1ab9 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 14:38:57 +0200 Subject: [PATCH 07/20] Properly reset Memory connector cache on automigrate --- lib/connectors/memory.js | 20 +++- lib/dao.js | 21 ++-- lib/model.js | 2 +- lib/utils.js | 35 ++++--- test/default-scope.test.js | 210 +++++++++++++++++++++++++++++++++++++ test/relations.test.js | 4 +- 6 files changed, 259 insertions(+), 33 deletions(-) create mode 100644 test/default-scope.test.js diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 3490dfe3..04b0ed98 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -78,6 +78,11 @@ Memory.prototype.getCollection = function(model) { return model; } +Memory.prototype.initCollection = function(model) { + this.collection(model, {}); + this.collectionSeq(model, 1); +} + Memory.prototype.collection = function(model, val) { model = this.getCollection(model); if (arguments.length > 1) this.cache[model] = val; @@ -181,10 +186,7 @@ Memory.prototype.saveToFile = function (result, callback) { Memory.prototype.define = function defineModel(definition) { this.constructor.super_.prototype.define.apply(this, [].slice.call(arguments)); var m = definition.model.modelName; - if(!this.collection(m)) { - this.collection(m, {}); - this.collectionSeq(m, 1); - } + if(!this.collection(m)) this.initCollection(m); }; Memory.prototype.create = function create(model, data, callback) { @@ -612,6 +614,16 @@ Memory.prototype.buildNearFilter = function (filter) { // noop } +Memory.prototype.automigrate = function (models, cb) { + if (typeof models === 'function') cb = models, models = []; + if (models.length === 0) models = Object.keys(this._models); + var self = this; + models.forEach(function(m) { + self.initCollection(m); + }); + if (cb) cb(); +} + function merge(base, update) { if (!base) { return update; diff --git a/lib/dao.js b/lib/dao.js index 1d308f73..d5277b5e 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -79,7 +79,10 @@ DataAccessObject.applyProperties = function(data) { }; DataAccessObject.applyScope = function(cond) { - + var scope = this.definition.settings.scope; + if (typeof scope === 'object') { + mergeQuery(cond, scope || {}, this.definition.settings.scoping); + } }; /** @@ -330,17 +333,7 @@ DataAccessObject.exists = function exists(id, cb) { */ DataAccessObject.findById = function find(id, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - - this.getDataSource().connector.find(this.modelName, id, function (err, data) { - var obj = null; - if (data) { - if (!getIdValue(this, data)) { - setIdValue(this, data, id); - } - obj = new this(data, {applySetters: false, persisted: true}); - } - cb(err, obj); - }.bind(this)); + this.findOne(byIdQuery(this, id), cb); }; DataAccessObject.findByIds = function(ids, cond, cb) { @@ -702,7 +695,7 @@ DataAccessObject.find = function find(query, cb) { var self = this; query = query || {}; - + try { this._normalize(query); } catch (err) { @@ -711,6 +704,8 @@ DataAccessObject.find = function find(query, cb) { }); } + this.applyScope(query); + var near = query && geo.nearFilter(query.where); var supportsGeo = !!this.getDataSource().connector.buildNearFilter; diff --git a/lib/model.js b/lib/model.js index 7bb19eb7..7e7a66f2 100644 --- a/lib/model.js +++ b/lib/model.js @@ -134,7 +134,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) { } if (properties[p]) { // Managed property - if (applySetters) { + if (applySetters || properties[p].id) { self[p] = propVal; } else { self.__data[p] = propVal; diff --git a/lib/utils.js b/lib/utils.js index bbde56fc..4e317058 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -56,14 +56,17 @@ function setScopeValuesFromWhere(data, where, targetModel) { * Merge query parameters * @param {Object} base The base object to contain the merged results * @param {Object} update The object containing updates to be merged + * @param {Object} spec Optionally specifies parameters to exclude (set to false) * @returns {*|Object} The base object * @private */ -function mergeQuery(base, update) { +function mergeQuery(base, update, spec) { if (!update) { return; } + spec = spec || {}; base = base || {}; + if (update.where && Object.keys(update.where).length > 0) { if (base.where && Object.keys(base.where).length > 0) { base.where = {and: [base.where, update.where]}; @@ -73,7 +76,7 @@ function mergeQuery(base, update) { } // Merge inclusion - if (update.include) { + if (spec.include !== false && update.include) { if (!base.include) { base.include = update.include; } else { @@ -82,30 +85,36 @@ function mergeQuery(base, update) { base.include[update.include] = saved; } } - if (update.collect) { + + if (spec.collect !== false && update.collect) { base.collect = update.collect; } - + + // Overwrite fields + if (spec.fields !== false && update.fields !== undefined) { + base.fields = update.fields; + } + // set order - if (!base.order && update.order) { + if ((!base.order || spec.order === false) && update.order) { base.order = update.order; } // overwrite pagination - if (update.limit !== undefined) { + if (spec.limit !== false && update.limit !== undefined) { base.limit = update.limit; } - if (update.skip !== undefined) { + + var skip = spec.skip !== false && spec.offset !== false; + + if (skip && update.skip !== undefined) { base.skip = update.skip; } - if (update.offset !== undefined) { + + if (skip && update.offset !== undefined) { base.offset = update.offset; } - - // Overwrite fields - if (update.fields !== undefined) { - base.fields = update.fields; - } + return base; } diff --git a/test/default-scope.test.js b/test/default-scope.test.js new file mode 100644 index 00000000..01d218b1 --- /dev/null +++ b/test/default-scope.test.js @@ -0,0 +1,210 @@ +// This test written in mocha+should.js +var should = require('./init.js'); +var async = require('async'); + +var db, Product, Tool, Widget; + +// This test requires a connector that can +// handle a custom collection or table name + +describe('default scope', function () { + + before(function (done) { + db = getSchema(); + + Product = db.define('Product', { + name: String, + kind: String, + description: String + }, { + scope: { order: 'name' }, + }); + + Tool = db.define('Tool', { + name: String, + kind: String, + description: String + }, { + base: 'Product', + scope: { where: { kind: 'tool' }, order: 'name' }, + mongodb: { collection: 'Product' }, + memory: { collection: 'Product' } + }); + + Widget = db.define('Widget', { + name: String, + kind: String, + description: String + }, { + base: 'Product', + scope: { where: { kind: 'widget' }, order: 'name' }, + mongodb: { collection: 'Product' }, + memory: { collection: 'Product' } + }); + + db.automigrate(done); + }); + + describe('manipulation', function() { + + var ids = {}; + + before(function(done) { + db.automigrate(done); + }); + + it('should return a scoped instance', function() { + var p = new Tool({name: 'Product A', kind:'ignored'}); + p.name.should.equal('Product A'); + p.kind.should.equal('tool'); + p.setAttributes({ kind: 'ignored' }); + p.kind.should.equal('tool'); + + p.setAttribute('kind', 'other'); // currently not enforced + p.kind.should.equal('other'); + }); + + it('should create a scoped instance - tool', function(done) { + Tool.create({name: 'Product A', kind: 'ignored'}, function(err, p) { + should.not.exist(err); + p.name.should.equal('Product A'); + p.kind.should.equal('tool'); + ids.productA = p.id; + done(); + }); + }); + + it('should create a scoped instance - widget', function(done) { + Widget.create({name: 'Product B', kind: 'ignored'}, function(err, p) { + should.not.exist(err); + p.name.should.equal('Product B'); + p.kind.should.equal('widget'); + ids.productB = p.id; + done(); + }); + }); + + it('should update a scoped instance - updateAttributes', function(done) { + Tool.findById(ids.productA, function(err, p) { + p.updateAttributes({description: 'A thing...', kind: 'ingored'}, function(err, inst) { + should.not.exist(err); + p.name.should.equal('Product A'); + p.kind.should.equal('tool'); + p.description.should.equal('A thing...'); + done(); + }); + }); + }); + + it('should update a scoped instance - save', function(done) { + Tool.findById(ids.productA, function(err, p) { + p.description = 'Something...'; + p.kind = 'ignored'; + p.save(function(err, inst) { + should.not.exist(err); + p.name.should.equal('Product A'); + p.kind.should.equal('tool'); + p.description.should.equal('Something...'); + Tool.findById(ids.productA, function(err, p) { + p.kind.should.equal('tool'); + done(); + }); + }); + }); + }); + + it('should update a scoped instance - updateOrCreate', function(done) { + var data = {id: ids.productA, description: 'Anything...', kind: 'ingored'}; + Tool.updateOrCreate(data, function(err, p) { + should.not.exist(err); + p.name.should.equal('Product A'); + p.kind.should.equal('tool'); + p.description.should.equal('Anything...'); + done(); + }); + }); + + }); + + describe('queries', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(function(err) { + async.series([ + function(next) { + Tool.create({name: 'Tool Z'}, function(err, inst) { + ids.toolZ = inst.id; + next(); + }); + }, + function(next) { + Widget.create({name: 'Widget Z'}, function(err, inst) { + ids.widgetZ = inst.id; + next(); + }); + }, + function(next) { + Tool.create({name: 'Tool A'}, function(err, inst) { + ids.toolA = inst.id; + next(); + }); + }, + function(next) { + Widget.create({name: 'Widget A'}, function(err, inst) { + ids.widgetA = inst.id; + next(); + }); + } + ], done); + }); + }); + + it('should apply default scope - order', function(done) { + Product.find(function(err, products) { + should.not.exist(err); + products.should.have.length(4); + products[0].name.should.equal('Tool A'); + products[1].name.should.equal('Tool Z'); + products[2].name.should.equal('Widget A'); + products[3].name.should.equal('Widget Z'); + done(); + }); + }); + + it('should apply default scope - order override', function(done) { + Product.find({ order: 'name DESC' }, function(err, products) { + should.not.exist(err); + products.should.have.length(4); + products[0].name.should.equal('Widget Z'); + products[1].name.should.equal('Widget A'); + products[2].name.should.equal('Tool Z'); + products[3].name.should.equal('Tool A'); + done(); + }); + }); + + it('should apply default scope - where + order (tool)', function(done) { + Tool.find(function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].name.should.equal('Tool A'); + products[1].name.should.equal('Tool Z'); + done(); + }); + }); + + it('should apply default scope - where + order (widget)', function(done) { + Widget.find({ order: 'name DESC' }, function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].name.should.equal('Widget Z'); + products[1].name.should.equal('Widget A'); + done(); + }); + }); + + }); + +}); \ No newline at end of file diff --git a/test/relations.test.js b/test/relations.test.js index 12e78d68..5315e677 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -335,7 +335,7 @@ describe('relations', function () { physician.patients.findById(id, function (err, ch) { should.not.exist(err); should.exist(ch); - ch.id.should.equal(id); + ch.id.should.eql(id); done(); }); } @@ -387,7 +387,7 @@ describe('relations', function () { physician.patients.findById(id, function (err, ch) { should.not.exist(err); should.exist(ch); - ch.id.should.equal(id); + ch.id.should.eql(id); ch.name.should.equal('aa'); done(); }); From b136a8fce7cc46a1e097c56fe8da93ac2d86a670 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 19:24:30 +0200 Subject: [PATCH 08/20] Full test CRUD suite for default scope --- lib/connectors/memory.js | 13 +- lib/dao.js | 112 +++++---- test/default-scope.test.js | 468 +++++++++++++++++++++++++++++++++---- 3 files changed, 501 insertions(+), 92 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 04b0ed98..29f25166 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -529,13 +529,12 @@ Memory.prototype.destroyAll = function destroyAll(model, where, callback) { var filter = null; if (where) { filter = applyFilter({where: where}); - } - Object.keys(cache).forEach(function (id) { - if (!filter || filter(this.fromDb(model, cache[id]))) { - delete cache[id]; - } - }.bind(this)); - if (!where) { + Object.keys(cache).forEach(function (id) { + if (!filter || filter(this.fromDb(model, cache[id]))) { + delete cache[id]; + } + }.bind(this)); + } else { this.collection(model, {}); } this.saveToFile(null, callback); diff --git a/lib/dao.js b/lib/dao.js index d5277b5e..2e933a83 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -54,6 +54,14 @@ function setIdValue(m, data, value) { } } +function byIdQuery(m, id) { + var pk = idName(m); + var query = { where: {} }; + query.where[pk] = id; + m.applyScope(query); + return query; +} + DataAccessObject._forDB = function (data) { if (!(this.getDataSource().isRelational && this.getDataSource().isRelational())) { return data; @@ -78,10 +86,10 @@ DataAccessObject.applyProperties = function(data) { } }; -DataAccessObject.applyScope = function(cond) { +DataAccessObject.applyScope = function(query) { var scope = this.definition.settings.scope; if (typeof scope === 'object') { - mergeQuery(cond, scope || {}, this.definition.settings.scoping); + mergeQuery(query, scope || {}, this.definition.settings.scoping); } }; @@ -312,7 +320,9 @@ DataAccessObject.exists = function exists(id, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; if (id !== undefined && id !== null && id !== '') { - this.dataSource.connector.exists(this.modelName, id, cb); + this.count(byIdQuery(this, id).where, function(err, count) { + cb(err, err ? false : count === 1); + }); } else { cb(new Error('Model::exists requires the id argument')); } @@ -716,6 +726,7 @@ DataAccessObject.find = function find(query, cb) { } else if (query.where) { // do in memory query // using all documents + // TODO [fabien] use default scope here? this.getDataSource().connector.all(this.modelName, {}, function (err, data) { var memory = new Memory(); var modelName = self.modelName; @@ -831,34 +842,39 @@ DataAccessObject.findOne = function findOne(query, cb) { * @param {Function} [cb] Callback called with (err) */ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyAll = function destroyAll(where, cb) { - if (stillConnecting(this.getDataSource(), this, arguments)) return; - var Model = this; + if (stillConnecting(this.getDataSource(), this, arguments)) return; + var Model = this; - if (!cb && 'function' === typeof where) { - cb = where; - where = undefined; - } - if (!where) { - this.getDataSource().connector.destroyAll(this.modelName, function (err, data) { - cb && cb(err, data); - if(!err) Model.emit('deletedAll'); - }.bind(this)); - } else { - try { - // Support an optional where object - where = removeUndefined(where); - where = this._coerce(where); - } catch (err) { - return process.nextTick(function() { - cb && cb(err); - }); - } - this.getDataSource().connector.destroyAll(this.modelName, where, function (err, data) { - cb && cb(err, data); - if(!err) Model.emit('deletedAll', where); - }.bind(this)); - } - }; + if (!cb && 'function' === typeof where) { + cb = where; + where = undefined; + } + + var query = { where: where }; + this.applyScope(query); + where = query.where; + + if (!where || (typeof where === 'object' && Object.keys(where).length === 0)) { + this.getDataSource().connector.destroyAll(this.modelName, function (err, data) { + cb && cb(err, data); + if(!err) Model.emit('deletedAll'); + }.bind(this)); + } else { + try { + // Support an optional where object + where = removeUndefined(where); + where = this._coerce(where); + } catch (err) { + return process.nextTick(function() { + cb && cb(err); + }); + } + this.getDataSource().connector.destroyAll(this.modelName, where, function (err, data) { + cb && cb(err, data); + if(!err) Model.emit('deletedAll', where); + }.bind(this)); + } +}; /** * Delete the record with the specified ID. @@ -871,16 +887,16 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA // 'deleteById' will be used as the name for strong-remoting to keep it backward // compatible for angular SDK DataAccessObject.removeById = DataAccessObject.destroyById = DataAccessObject.deleteById = function deleteById(id, cb) { - if (stillConnecting(this.getDataSource(), this, arguments)) return; - var Model = this; - - this.getDataSource().connector.destroy(this.modelName, id, function (err) { - if ('function' === typeof cb) { - cb(err); - } - if(!err) Model.emit('deleted', id); - }.bind(this)); - }; + if (stillConnecting(this.getDataSource(), this, arguments)) return; + var Model = this; + + this.remove(byIdQuery(this, id).where, function(err) { + if ('function' === typeof cb) { + cb(err); + } + if(!err) Model.emit('deleted', id); + }); +}; /** * Return count of matched records. Optional query parameter allows you to count filtered set of model instances. @@ -902,6 +918,11 @@ DataAccessObject.count = function (where, cb) { cb = where; where = null; } + + var query = { where: where }; + this.applyScope(query); + where = query.where; + try { where = removeUndefined(where); where = this._coerce(where); @@ -910,6 +931,7 @@ DataAccessObject.count = function (where, cb) { cb && cb(err); }); } + this.getDataSource().connector.count(this.modelName, cb, where); }; @@ -1034,7 +1056,13 @@ DataAccessObject.updateAll = function (where, data, cb) { assert(typeof where === 'object', 'The where argument should be an object'); assert(typeof data === 'object', 'The data argument should be an object'); assert(cb === null || typeof cb === 'function', 'The cb argument should be a function'); - + + var query = { where: where }; + this.applyScope(query); + this.applyProperties(data); + + where = query.where; + try { where = removeUndefined(where); where = this._coerce(where); @@ -1044,8 +1072,6 @@ DataAccessObject.updateAll = function (where, data, cb) { }); } - this.applyProperties(data); - var connector = this.getDataSource().connector; connector.update(this.modelName, where, data, cb); }; diff --git a/test/default-scope.test.js b/test/default-scope.test.js index 01d218b1..83fb95e9 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -7,6 +7,43 @@ var db, Product, Tool, Widget; // This test requires a connector that can // handle a custom collection or table name +// TODO [fabien] add table for pgsql/mysql + +var setupProducts = function(ids, done) { + async.series([ + function(next) { + Tool.create({name: 'Tool Z'}, function(err, inst) { + ids.toolZ = inst.id; + next(); + }); + }, + function(next) { + Widget.create({name: 'Widget Z'}, function(err, inst) { + ids.widgetZ = inst.id; + next(); + }); + }, + function(next) { + Tool.create({name: 'Tool A', active: false}, function(err, inst) { + ids.toolA = inst.id; + next(); + }); + }, + function(next) { + Widget.create({name: 'Widget A'}, function(err, inst) { + ids.widgetA = inst.id; + next(); + }); + }, + function(next) { + Widget.create({name: 'Widget B', active: false}, function(err, inst) { + ids.widgetB = inst.id; + next(); + }); + } + ], done); +}; + describe('default scope', function () { before(function (done) { @@ -15,18 +52,22 @@ describe('default scope', function () { Product = db.define('Product', { name: String, kind: String, - description: String + description: String, + active: { type: Boolean, default: true } }, { scope: { order: 'name' }, + scopes: { active: { where: { active: true } } } }); Tool = db.define('Tool', { name: String, kind: String, - description: String + description: String, + active: { type: Boolean, default: true } }, { base: 'Product', scope: { where: { kind: 'tool' }, order: 'name' }, + scopes: { active: { where: { active: true } } }, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } }); @@ -34,10 +75,12 @@ describe('default scope', function () { Widget = db.define('Widget', { name: String, kind: String, - description: String + description: String, + active: { type: Boolean, default: true } }, { base: 'Product', scope: { where: { kind: 'widget' }, order: 'name' }, + scopes: { active: { where: { active: true } } }, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } }); @@ -126,49 +169,57 @@ describe('default scope', function () { }); - describe('queries', function() { + describe('findById', function() { var ids = {}; before(function (done) { - db.automigrate(function(err) { - async.series([ - function(next) { - Tool.create({name: 'Tool Z'}, function(err, inst) { - ids.toolZ = inst.id; - next(); - }); - }, - function(next) { - Widget.create({name: 'Widget Z'}, function(err, inst) { - ids.widgetZ = inst.id; - next(); - }); - }, - function(next) { - Tool.create({name: 'Tool A'}, function(err, inst) { - ids.toolA = inst.id; - next(); - }); - }, - function(next) { - Widget.create({name: 'Widget A'}, function(err, inst) { - ids.widgetA = inst.id; - next(); - }); - } - ], done); + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope', function(done) { + Product.findById(ids.toolA, function(err, inst) { + should.not.exist(err); + inst.name.should.equal('Tool A'); + done(); }); }); + it('should apply default scope - tool', function(done) { + Tool.findById(ids.toolA, function(err, inst) { + should.not.exist(err); + inst.name.should.equal('Tool A'); + done(); + }); + }); + + it('should apply default scope (no match)', function(done) { + Widget.findById(ids.toolA, function(err, inst) { + should.not.exist(err); + should.not.exist(inst); + done(); + }); + }); + + }); + + describe('find', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + it('should apply default scope - order', function(done) { Product.find(function(err, products) { should.not.exist(err); - products.should.have.length(4); + products.should.have.length(5); products[0].name.should.equal('Tool A'); products[1].name.should.equal('Tool Z'); products[2].name.should.equal('Widget A'); - products[3].name.should.equal('Widget Z'); + products[3].name.should.equal('Widget B'); + products[4].name.should.equal('Widget Z'); done(); }); }); @@ -176,16 +227,17 @@ describe('default scope', function () { it('should apply default scope - order override', function(done) { Product.find({ order: 'name DESC' }, function(err, products) { should.not.exist(err); - products.should.have.length(4); + products.should.have.length(5); products[0].name.should.equal('Widget Z'); - products[1].name.should.equal('Widget A'); - products[2].name.should.equal('Tool Z'); - products[3].name.should.equal('Tool A'); + products[1].name.should.equal('Widget B'); + products[2].name.should.equal('Widget A'); + products[3].name.should.equal('Tool Z'); + products[4].name.should.equal('Tool A'); done(); }); }); - it('should apply default scope - where + order (tool)', function(done) { + it('should apply default scope - tool', function(done) { Tool.find(function(err, products) { should.not.exist(err); products.should.have.length(2); @@ -195,16 +247,348 @@ describe('default scope', function () { }); }); - it('should apply default scope - where + order (widget)', function(done) { - Widget.find({ order: 'name DESC' }, function(err, products) { + it('should apply default scope - where (widget)', function(done) { + Widget.find({ where: { active: true } }, function(err, products) { should.not.exist(err); products.should.have.length(2); + products[0].name.should.equal('Widget A'); + products[1].name.should.equal('Widget Z'); + done(); + }); + }); + + it('should apply default scope - order (widget)', function(done) { + Widget.find({ order: 'name DESC' }, function(err, products) { + should.not.exist(err); + products.should.have.length(3); products[0].name.should.equal('Widget Z'); - products[1].name.should.equal('Widget A'); + products[1].name.should.equal('Widget B'); + products[2].name.should.equal('Widget A'); done(); }); }); }); -}); \ No newline at end of file + describe('exists', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope', function(done) { + Product.exists(ids.widgetA, function(err, exists) { + should.not.exist(err); + exists.should.be.true; + done(); + }); + }); + + it('should apply default scope - tool', function(done) { + Tool.exists(ids.toolZ, function(err, exists) { + should.not.exist(err); + exists.should.be.true; + done(); + }); + }); + + it('should apply default scope - widget', function(done) { + Widget.exists(ids.widgetA, function(err, exists) { + should.not.exist(err); + exists.should.be.true; + done(); + }); + }); + + it('should apply default scope - tool (no match)', function(done) { + Tool.exists(ids.widgetA, function(err, exists) { + should.not.exist(err); + exists.should.be.false; + done(); + }); + }); + + it('should apply default scope - widget (no match)', function(done) { + Widget.exists(ids.toolZ, function(err, exists) { + should.not.exist(err); + exists.should.be.false; + done(); + }); + }); + + }); + + describe('count', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope - order', function(done) { + Product.count(function(err, count) { + should.not.exist(err); + count.should.equal(5); + done(); + }); + }); + + it('should apply default scope - tool', function(done) { + Tool.count(function(err, count) { + should.not.exist(err); + count.should.equal(2); + done(); + }); + }); + + it('should apply default scope - widget', function(done) { + Widget.count(function(err, count) { + should.not.exist(err); + count.should.equal(3); + done(); + }); + }); + + it('should apply default scope - where', function(done) { + Widget.count({name: 'Widget Z'}, function(err, count) { + should.not.exist(err); + count.should.equal(1); + done(); + }); + }); + + it('should apply default scope - no match', function(done) { + Tool.count({name: 'Widget Z'}, function(err, count) { + should.not.exist(err); + count.should.equal(0); + done(); + }); + }); + + }); + + describe('removeById', function() { + + var ids = {}; + + function isDeleted(id, done) { + Product.exists(id, function(err, exists) { + should.not.exist(err); + exists.should.be.false; + done(); + }); + }; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope', function(done) { + Product.removeById(ids.widgetZ, function(err) { + should.not.exist(err); + isDeleted(ids.widgetZ, done); + }); + }); + + it('should apply default scope - tool', function(done) { + Tool.removeById(ids.toolA, function(err) { + should.not.exist(err); + isDeleted(ids.toolA, done); + }); + }); + + it('should apply default scope - no match', function(done) { + Tool.removeById(ids.widgetA, function(err) { + should.not.exist(err); + Product.exists(ids.widgetA, function(err, exists) { + should.not.exist(err); + exists.should.be.true; + done(); + }); + }); + }); + + it('should apply default scope - widget', function(done) { + Widget.removeById(ids.widgetA, function(err) { + should.not.exist(err); + isDeleted(ids.widgetA, done); + }); + }); + + it('should apply default scope - verify', function(done) { + Product.find(function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].name.should.equal('Tool Z'); + products[1].name.should.equal('Widget B'); + done(); + }); + }); + + }); + + describe('update', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope', function(done) { + Widget.update({active: false},{active: true, kind: 'ignored'}, function(err) { + should.not.exist(err); + Widget.find({where: { active: true }}, function(err, products) { + should.not.exist(err); + products.should.have.length(3); + products[0].name.should.equal('Widget A'); + products[1].name.should.equal('Widget B'); + products[2].name.should.equal('Widget Z'); + done(); + }); + }); + }); + + it('should apply default scope - no match', function(done) { + Tool.update({name: 'Widget A'},{name: 'Ignored'}, function(err) { + should.not.exist(err); + Product.findById(ids.widgetA, function(err, product) { + should.not.exist(err); + product.name.should.equal('Widget A'); + done(); + }); + }); + }); + + it('should have updated within scope', function(done) { + Product.find({where: {active: true}}, function(err, products) { + should.not.exist(err); + products.should.have.length(4); + products[0].name.should.equal('Tool Z'); + products[1].name.should.equal('Widget A'); + products[2].name.should.equal('Widget B'); + products[3].name.should.equal('Widget Z'); + done(); + }); + }); + + }); + + describe('remove', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should apply default scope - custom where', function(done) { + Widget.remove({name: 'Widget A'}, function(err) { + should.not.exist(err); + Product.find(function(err, products) { + products.should.have.length(4); + products[0].name.should.equal('Tool A'); + products[1].name.should.equal('Tool Z'); + products[2].name.should.equal('Widget B'); + products[3].name.should.equal('Widget Z'); + done(); + }); + }); + }); + + it('should apply default scope - custom where (no match)', function(done) { + Tool.remove({name: 'Widget Z'}, function(err) { + should.not.exist(err); + Product.find(function(err, products) { + products.should.have.length(4); + products[0].name.should.equal('Tool A'); + products[1].name.should.equal('Tool Z'); + products[2].name.should.equal('Widget B'); + products[3].name.should.equal('Widget Z'); + done(); + }); + }); + }); + + it('should apply default scope - deleteAll', function(done) { + Tool.deleteAll(function(err) { + should.not.exist(err); + Product.find(function(err, products) { + products.should.have.length(2); + products[0].name.should.equal('Widget B'); + products[1].name.should.equal('Widget Z'); + done(); + }); + }); + }); + + it('should create a scoped instance - tool', function(done) { + Tool.create({name: 'Tool B'}, function(err, p) { + should.not.exist(err); + Product.find(function(err, products) { + products.should.have.length(3); + products[0].name.should.equal('Tool B'); + products[1].name.should.equal('Widget B'); + products[2].name.should.equal('Widget Z'); + done(); + }); + }); + }); + + it('should apply default scope - destroyAll', function(done) { + Widget.destroyAll(function(err) { + should.not.exist(err); + Product.find(function(err, products) { + products.should.have.length(1); + products[0].name.should.equal('Tool B'); + done(); + }); + }); + }); + + }); + + describe('scopes', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(setupProducts.bind(null, ids, done)); + }); + + it('should merge with default scope', function(done) { + Product.active(function(err, products) { + should.not.exist(err); + products.should.have.length(3); + products[0].name.should.equal('Tool Z'); + products[1].name.should.equal('Widget A'); + products[2].name.should.equal('Widget Z'); + done(); + }); + }); + + it('should merge with default scope - tool', function(done) { + Tool.active(function(err, products) { + should.not.exist(err); + products.should.have.length(1); + products[0].name.should.equal('Tool Z'); + done(); + }); + }); + + it('should merge with default scope - widget', function(done) { + Widget.active(function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].name.should.equal('Widget A'); + products[1].name.should.equal('Widget Z'); + done(); + }); + }); + + }); + +}); From 49e2b8b8dc74d6fa9c6b5fa26471fe58ea751f56 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 19:44:58 +0200 Subject: [PATCH 09/20] Allow default scope to be a function --- lib/dao.js | 20 +++++--- test/default-scope.test.js | 102 ++++++++++++++++++++++++++++--------- 2 files changed, 92 insertions(+), 30 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 2e933a83..614ea459 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -78,21 +78,29 @@ DataAccessObject._forDB = function (data) { return res; }; -DataAccessObject.applyProperties = function(data) { - var scope = this.definition.settings.scope || {}; - if (typeof scope.where === 'object' - && this.definition.settings.applyProperties !== false) { - setScopeValuesFromWhere(data, scope.where, this); +DataAccessObject.defaultScope = function(target, isData) { + var scope = this.definition.settings.scope; + if (typeof scope === 'function') { + scope = this.definition.settings.scope.call(this, target, isData); } + return scope; }; DataAccessObject.applyScope = function(query) { - var scope = this.definition.settings.scope; + var scope = this.defaultScope(query, false) || {}; if (typeof scope === 'object') { mergeQuery(query, scope || {}, this.definition.settings.scoping); } }; +DataAccessObject.applyProperties = function(data) { + var scope = this.defaultScope(data, true) || {}; + if (typeof scope.where === 'object' + && this.definition.settings.applyProperties !== false) { + setScopeValuesFromWhere(data, scope.where, this); + } +}; + /** * Create an instance of Model with given data and save to the attached data source. Callback is optional. * Example: diff --git a/test/default-scope.test.js b/test/default-scope.test.js index 83fb95e9..c0efd137 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -2,12 +2,13 @@ var should = require('./init.js'); var async = require('async'); -var db, Product, Tool, Widget; +var db, Product, Tool, Widget, Thing; // This test requires a connector that can // handle a custom collection or table name // TODO [fabien] add table for pgsql/mysql +// TODO [fabien] change model definition - see #293 var setupProducts = function(ids, done) { async.series([ @@ -59,32 +60,33 @@ describe('default scope', function () { scopes: { active: { where: { active: true } } } }); - Tool = db.define('Tool', { - name: String, - kind: String, - description: String, - active: { type: Boolean, default: true } - }, { + Tool = db.define('Tool', Product.definition.properties, { base: 'Product', - scope: { where: { kind: 'tool' }, order: 'name' }, + scope: { where: { kind: 'Tool' }, order: 'name' }, scopes: { active: { where: { active: true } } }, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } }); - Widget = db.define('Widget', { - name: String, - kind: String, - description: String, - active: { type: Boolean, default: true } - }, { + Widget = db.define('Widget', Product.definition.properties, { base: 'Product', - scope: { where: { kind: 'widget' }, order: 'name' }, + scope: { where: { kind: 'Widget' }, order: 'name' }, scopes: { active: { where: { active: true } } }, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } }); - + + var scopeFn = function(target, isData) { + return { where: { kind: this.modelName } }; + }; + + Thing = db.define('Thing', Product.definition.properties, { + base: 'Product', + scope: scopeFn, + mongodb: { collection: 'Product' }, + memory: { collection: 'Product' } + }); + db.automigrate(done); }); @@ -99,9 +101,9 @@ describe('default scope', function () { it('should return a scoped instance', function() { var p = new Tool({name: 'Product A', kind:'ignored'}); p.name.should.equal('Product A'); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); p.setAttributes({ kind: 'ignored' }); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); p.setAttribute('kind', 'other'); // currently not enforced p.kind.should.equal('other'); @@ -111,7 +113,7 @@ describe('default scope', function () { Tool.create({name: 'Product A', kind: 'ignored'}, function(err, p) { should.not.exist(err); p.name.should.equal('Product A'); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); ids.productA = p.id; done(); }); @@ -121,7 +123,7 @@ describe('default scope', function () { Widget.create({name: 'Product B', kind: 'ignored'}, function(err, p) { should.not.exist(err); p.name.should.equal('Product B'); - p.kind.should.equal('widget'); + p.kind.should.equal('Widget'); ids.productB = p.id; done(); }); @@ -132,7 +134,7 @@ describe('default scope', function () { p.updateAttributes({description: 'A thing...', kind: 'ingored'}, function(err, inst) { should.not.exist(err); p.name.should.equal('Product A'); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); p.description.should.equal('A thing...'); done(); }); @@ -146,10 +148,10 @@ describe('default scope', function () { p.save(function(err, inst) { should.not.exist(err); p.name.should.equal('Product A'); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); p.description.should.equal('Something...'); Tool.findById(ids.productA, function(err, p) { - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); done(); }); }); @@ -161,7 +163,7 @@ describe('default scope', function () { Tool.updateOrCreate(data, function(err, p) { should.not.exist(err); p.name.should.equal('Product A'); - p.kind.should.equal('tool'); + p.kind.should.equal('Tool'); p.description.should.equal('Anything...'); done(); }); @@ -591,4 +593,56 @@ describe('default scope', function () { }); + describe('scope function', function() { + + before(function(done) { + db.automigrate(done); + }); + + it('should create a scoped instance - widget', function(done) { + Widget.create({name: 'Product', kind:'ignored'}, function(err, p) { + p.name.should.equal('Product'); + p.kind.should.equal('Widget'); + done(); + }); + }); + + it('should create a scoped instance - thing', function(done) { + Thing.create({name: 'Product', kind:'ignored'}, function(err, p) { + p.name.should.equal('Product'); + p.kind.should.equal('Thing'); + done(); + }); + }); + + it('should find a scoped instance - widget', function(done) { + Widget.findOne({where: {name: 'Product'}}, function(err, p) { + p.name.should.equal('Product'); + p.kind.should.equal('Widget'); + done(); + }); + }); + + it('should find a scoped instance - thing', function(done) { + Thing.findOne({where: {name: 'Product'}}, function(err, p) { + p.name.should.equal('Product'); + p.kind.should.equal('Thing'); + done(); + }); + }); + + it('should find a scoped instance - thing', function(done) { + Product.find({where: {name: 'Product'}}, function(err, products) { + products.should.have.length(2); + products[0].name.should.equal('Product'); + products[1].name.should.equal('Product'); + var kinds = products.map(function(p) { return p.kind; }) + kinds.sort(); + kinds.should.eql(['Thing', 'Widget']); + done(); + }); + }); + + }); + }); From 702796a486c843203553366d3220587143eefe93 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 6 Sep 2014 20:19:20 +0200 Subject: [PATCH 10/20] Test default scope with relations --- test/default-scope.test.js | 130 ++++++++++++++++++++++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-) diff --git a/test/default-scope.test.js b/test/default-scope.test.js index c0efd137..588bb44b 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -2,7 +2,7 @@ var should = require('./init.js'); var async = require('async'); -var db, Product, Tool, Widget, Thing; +var db, Category, Product, Tool, Widget, Thing; // This test requires a connector that can // handle a custom collection or table name @@ -49,6 +49,10 @@ describe('default scope', function () { before(function (done) { db = getSchema(); + + Category = db.define('Category', { + name: String + }); Product = db.define('Product', { name: String, @@ -87,6 +91,16 @@ describe('default scope', function () { memory: { collection: 'Product' } }); + Category.hasMany(Product); + Category.hasMany(Tool, {scope: {order: 'name DESC'}}); + Category.hasMany(Widget); + Category.hasMany(Thing); + + Product.belongsTo(Category); + Tool.belongsTo(Category); + Widget.belongsTo(Category); + Thing.belongsTo(Category); + db.automigrate(done); }); @@ -645,4 +659,118 @@ describe('default scope', function () { }); + describe('relations', function() { + + var ids = {}; + + before(function (done) { + db.automigrate(done); + }); + + before(function (done) { + Category.create({name: 'Category A'}, function(err, cat) { + ids.categoryA = cat.id; + async.series([ + function(next) { + cat.widgets.create({name: 'Widget B', kind: 'ignored'}, next); + }, + function(next) { + cat.widgets.create({name: 'Widget A'}, next); + }, + function(next) { + cat.tools.create({name: 'Tool A'}, next); + }, + function(next) { + cat.things.create({name: 'Thing A'}, next); + } + ], done); + }); + }); + + it('should apply default scope - products', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + should.not.exist(err); + cat.products(function(err, products) { + should.not.exist(err); + products.should.have.length(4); + products[0].should.be.instanceof(Product); + products[0].name.should.equal('Thing A'); + products[1].name.should.equal('Tool A'); + products[2].name.should.equal('Widget A'); + products[3].name.should.equal('Widget B'); + done(); + }); + }); + }); + + it('should apply default scope - widgets', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + should.not.exist(err); + cat.widgets(function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].should.be.instanceof(Widget); + products[0].name.should.equal('Widget A'); + products[1].name.should.equal('Widget B'); + products[0].category(function(err, inst) { + inst.name.should.equal('Category A'); + done(); + }); + }); + }); + }); + + it('should apply default scope - tools', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + should.not.exist(err); + cat.tools(function(err, products) { + should.not.exist(err); + products.should.have.length(1); + products[0].should.be.instanceof(Tool); + products[0].name.should.equal('Tool A'); + products[0].category(function(err, inst) { + inst.name.should.equal('Category A'); + done(); + }); + }); + }); + }); + + it('should apply default scope - things', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + should.not.exist(err); + cat.things(function(err, products) { + should.not.exist(err); + products.should.have.length(1); + products[0].should.be.instanceof(Thing); + products[0].name.should.equal('Thing A'); + products[0].category(function(err, inst) { + inst.name.should.equal('Category A'); + done(); + }); + }); + }); + }); + + it('should create related item with default scope', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + cat.tools.create({name: 'Tool B'}, done); + }); + }); + + it('should use relation scope order', function(done) { + Category.findById(ids.categoryA, function(err, cat) { + should.not.exist(err); + cat.tools(function(err, products) { + should.not.exist(err); + products.should.have.length(2); + products[0].name.should.equal('Tool B'); + products[1].name.should.equal('Tool A'); + done(); + }); + }); + }); + + }); + }); From beeb7c46c9136d7ae15c7604fb41d8c856074b9f Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 7 Sep 2014 14:12:14 +0200 Subject: [PATCH 11/20] applyProperties => properties (object/false) --- lib/dao.js | 12 ++++++++---- test/default-scope.test.js | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 614ea459..7a0c62a2 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -94,10 +94,14 @@ DataAccessObject.applyScope = function(query) { }; DataAccessObject.applyProperties = function(data) { - var scope = this.defaultScope(data, true) || {}; - if (typeof scope.where === 'object' - && this.definition.settings.applyProperties !== false) { - setScopeValuesFromWhere(data, scope.where, this); + var properties = this.definition.settings.properties; + if (typeof properties === 'object') { + util._extend(data, properties); + } else if (properties !== false) { + var scope = this.defaultScope(data, true) || {}; + if (typeof scope.where === 'object') { + setScopeValuesFromWhere(data, scope.where, this); + } } }; diff --git a/test/default-scope.test.js b/test/default-scope.test.js index 588bb44b..8f9d1f6a 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -74,6 +74,7 @@ describe('default scope', function () { Widget = db.define('Widget', Product.definition.properties, { base: 'Product', + properties: { kind: 'Widget' }, scope: { where: { kind: 'Widget' }, order: 'name' }, scopes: { active: { where: { active: true } } }, mongodb: { collection: 'Product' }, From ec05d0b5a82ce757ba5c889ea8678f1732364bda Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 7 Sep 2014 14:24:06 +0200 Subject: [PATCH 12/20] Cleanup, consistency: allow properties to be a function Also, pass along the current instance, for any instance method as a context (save, updateAttributes). --- lib/dao.js | 20 +++++++++++--------- test/default-scope.test.js | 10 +++++++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 7a0c62a2..ee49bc94 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -78,27 +78,29 @@ DataAccessObject._forDB = function (data) { return res; }; -DataAccessObject.defaultScope = function(target, isData) { +DataAccessObject.defaultScope = function(target, inst) { var scope = this.definition.settings.scope; if (typeof scope === 'function') { - scope = this.definition.settings.scope.call(this, target, isData); + scope = this.definition.settings.scope.call(this, target, inst); } return scope; }; -DataAccessObject.applyScope = function(query) { - var scope = this.defaultScope(query, false) || {}; +DataAccessObject.applyScope = function(query, inst) { + var scope = this.defaultScope(query, inst) || {}; if (typeof scope === 'object') { mergeQuery(query, scope || {}, this.definition.settings.scoping); } }; -DataAccessObject.applyProperties = function(data) { +DataAccessObject.applyProperties = function(data, inst) { var properties = this.definition.settings.properties; if (typeof properties === 'object') { util._extend(data, properties); + } else if (typeof properties === 'function') { + util._extend(data, properties.call(this, data, inst) || {}); } else if (properties !== false) { - var scope = this.defaultScope(data, true) || {}; + var scope = this.defaultScope(data, inst) || {}; if (typeof scope.where === 'object') { setScopeValuesFromWhere(data, scope.where, this); } @@ -183,7 +185,7 @@ DataAccessObject.create = function (data, callback) { obj = new Model(data); } - this.applyProperties(enforced); + this.applyProperties(enforced, obj); obj.setAttributes(enforced); data = obj.toObject(true); @@ -261,7 +263,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data inst = new Model(data); } update = inst.toObject(false); - this.applyProperties(update); + this.applyProperties(update, inst); update = removeUndefined(update); this.getDataSource().connector.updateOrCreate(Model.modelName, update, function (err, data) { var obj; @@ -1160,7 +1162,7 @@ DataAccessObject.prototype.updateAttribute = function updateAttribute(name, valu DataAccessObject.prototype.setAttributes = function setAttributes(data) { if (typeof data !== 'object') return; - this.constructor.applyProperties(data); + this.constructor.applyProperties(data, this); var Model = this.constructor; var inst = this; diff --git a/test/default-scope.test.js b/test/default-scope.test.js index 8f9d1f6a..bb44f859 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -81,12 +81,20 @@ describe('default scope', function () { memory: { collection: 'Product' } }); - var scopeFn = function(target, isData) { + // inst is only valid for instance methods + // like save, updateAttributes + + var scopeFn = function(target, inst) { return { where: { kind: this.modelName } }; }; + var propertiesFn = function(target, inst) { + return { kind: this.modelName }; + }; + Thing = db.define('Thing', Product.definition.properties, { base: 'Product', + properties: propertiesFn, scope: scopeFn, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } From 0e49dc94ec1325de4477c0da62553a5f2eddf590 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 7 Sep 2014 14:31:06 +0200 Subject: [PATCH 13/20] Allow `attributes` as an alias for `properties` (for LDL) --- lib/dao.js | 33 +++++++++++++++++++++++++-------- test/default-scope.test.js | 27 +++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index ee49bc94..cff2e52c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -95,6 +95,7 @@ DataAccessObject.applyScope = function(query, inst) { DataAccessObject.applyProperties = function(data, inst) { var properties = this.definition.settings.properties; + properties = properties || this.definition.settings.attributes; if (typeof properties === 'object') { util._extend(data, properties); } else if (typeof properties === 'function') { @@ -107,6 +108,10 @@ DataAccessObject.applyProperties = function(data, inst) { } }; +DataAccessObject.lookupModel = function(data) { + return this; +}; + /** * Create an instance of Model with given data and save to the attached data source. Callback is optional. * Example: @@ -127,8 +132,8 @@ DataAccessObject.create = function (data, callback) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this; - var modelName = Model.modelName; - + var self = this; + if (typeof data === 'function') { callback = data; data = {}; @@ -154,6 +159,7 @@ DataAccessObject.create = function (data, callback) { for (var i = 0; i < data.length; i += 1) { (function (d, i) { + Model = self.lookupModel(d); // data-specific instances.push(Model.create(d, function (err, inst) { if (err) { errors[i] = err; @@ -169,11 +175,15 @@ DataAccessObject.create = function (data, callback) { function modelCreated() { if (--wait === 0) { callback(gotError ? errors : null, instances); - if(!gotError) instances.forEach(Model.emit.bind('changed')); + if(!gotError) { + instances.forEach(function(inst) { + inst.constructor.emit('changed'); + }); + } } } } - + var enforced = {}; var obj; var idValue = getIdValue(this, data); @@ -188,6 +198,9 @@ DataAccessObject.create = function (data, callback) { this.applyProperties(enforced, obj); obj.setAttributes(enforced); + Model = this.lookupModel(data); // data-specific + if (Model !== obj.constructor) obj = new Model(data); + data = obj.toObject(true); // validation required @@ -198,12 +211,13 @@ DataAccessObject.create = function (data, callback) { callback(new ValidationError(obj), obj); } }, data); - + function create() { obj.trigger('create', function (createDone) { obj.trigger('save', function (saveDone) { var _idName = idName(Model); + var modelName = Model.modelName; this._adapter().create(modelName, this.constructor._forDB(obj.toObject(true)), function (err, id, rev) { if (id) { obj.__data[_idName] = id; @@ -251,7 +265,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data if (stillConnecting(this.getDataSource(), this, arguments)) { return; } - + var self = this; var Model = this; if (!getIdValue(this, data)) { return this.create(data, callback); @@ -265,6 +279,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data update = inst.toObject(false); this.applyProperties(update, inst); update = removeUndefined(update); + Model = this.lookupModel(update); this.getDataSource().connector.updateOrCreate(Model.modelName, update, function (err, data) { var obj; if (data && !(data instanceof Model)) { @@ -286,6 +301,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data if (inst) { inst.updateAttributes(data, callback); } else { + Model = self.lookupModel(data); var obj = new Model(data); obj.save(data, callback); } @@ -774,8 +790,9 @@ DataAccessObject.find = function find(query, cb) { this.getDataSource().connector.all(this.modelName, query, function (err, data) { if (data && data.forEach) { data.forEach(function (d, i) { - var obj = new self(d, {fields: query.fields, applySetters: false, persisted: true}); - + var Model = self.lookupModel(d); + var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true}); + if (query && query.include) { if (query.collect) { // The collect property indicates that the query is to return the diff --git a/test/default-scope.test.js b/test/default-scope.test.js index bb44f859..0d510cff 100644 --- a/test/default-scope.test.js +++ b/test/default-scope.test.js @@ -64,6 +64,12 @@ describe('default scope', function () { scopes: { active: { where: { active: true } } } }); + Product.lookupModel = function(data) { + var m = this.dataSource.models[data.kind]; + if (m.base === this) return m; + return this; + }; + Tool = db.define('Tool', Product.definition.properties, { base: 'Product', scope: { where: { kind: 'Tool' }, order: 'name' }, @@ -94,7 +100,7 @@ describe('default scope', function () { Thing = db.define('Thing', Product.definition.properties, { base: 'Product', - properties: propertiesFn, + attributes: propertiesFn, scope: scopeFn, mongodb: { collection: 'Product' }, memory: { collection: 'Product' } @@ -206,6 +212,7 @@ describe('default scope', function () { Product.findById(ids.toolA, function(err, inst) { should.not.exist(err); inst.name.should.equal('Tool A'); + inst.should.be.instanceof(Tool); done(); }); }); @@ -245,6 +252,13 @@ describe('default scope', function () { products[2].name.should.equal('Widget A'); products[3].name.should.equal('Widget B'); products[4].name.should.equal('Widget Z'); + + products[0].should.be.instanceof(Product); + products[0].should.be.instanceof(Tool); + + products[2].should.be.instanceof(Product); + products[2].should.be.instanceof(Widget); + done(); }); }); @@ -702,11 +716,20 @@ describe('default scope', function () { cat.products(function(err, products) { should.not.exist(err); products.should.have.length(4); - products[0].should.be.instanceof(Product); products[0].name.should.equal('Thing A'); products[1].name.should.equal('Tool A'); products[2].name.should.equal('Widget A'); products[3].name.should.equal('Widget B'); + + products[0].should.be.instanceof(Product); + products[0].should.be.instanceof(Thing); + + products[1].should.be.instanceof(Product); + products[1].should.be.instanceof(Tool); + + products[2].should.be.instanceof(Product); + products[2].should.be.instanceof(Widget); + done(); }); }); From 2838158139dde5d6baa918a2e01f7667f12bb342 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 9 Oct 2014 18:18:04 +0200 Subject: [PATCH 14/20] Fix failing test --- test/relations.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/relations.test.js b/test/relations.test.js index 5315e677..f6848d11 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1659,7 +1659,7 @@ describe('relations', function () { should.not.exist(e); should.exist(tags); - article.tags().should.eql(tags); + article.tagNames().should.eql(tags); done(); }); From ca0b3399c85363c35758ef5f095880753d5b875d Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 10 Oct 2014 12:28:39 +0200 Subject: [PATCH 15/20] Implement include scopes --- lib/include.js | 105 ++++++++++++++++++++++++++++++++----------- lib/utils.js | 4 +- test/include.test.js | 36 +++++++++++++++ 3 files changed, 118 insertions(+), 27 deletions(-) diff --git a/lib/include.js b/lib/include.js index dacf78af..d3efc608 100644 --- a/lib/include.js +++ b/lib/include.js @@ -3,6 +3,52 @@ var utils = require('./utils'); var isPlainObject = utils.isPlainObject; var defineCachedRelations = utils.defineCachedRelations; +/*! + * Normalize the include to be an array + * @param include + * @returns {*} + */ +function normalizeInclude(include) { + if (typeof include === 'string') { + return [include]; + } else if (isPlainObject(include)) { + // Build an array of key/value pairs + var newInclude = []; + if (typeof include.relation === 'string' && isPlainObject(include.scope)) { + var obj = {}; + obj[include.relation] = new IncludeScope(include.scope); + newInclude.push(obj); + } else { + for (var key in include) { + var obj = {}; + obj[key] = include[key]; + newInclude.push(obj); + } + } + return newInclude; + } else { + return include; + } +} + +function IncludeScope(scope) { + this._scope = utils.deepMerge({}, scope || {}); + if (this._scope.include) { + this._include = normalizeInclude(this._scope.include); + delete this._scope.include; + } else { + this._include = null; + } +}; + +IncludeScope.prototype.conditions = function() { + return utils.deepMerge({}, this._scope); +}; + +IncludeScope.prototype.include = function() { + return this._include; +}; + /*! * Include mixin for ./model.js */ @@ -59,36 +105,19 @@ Inclusion.include = function (objects, include, cb) { cb && cb(err, objects); }); - - /*! - * Normalize the include to be an array - * @param include - * @returns {*} - */ - function normalizeInclude(include) { - if (typeof include === 'string') { - return [include]; - } else if (isPlainObject(include)) { - // Build an array of key/value pairs - var newInclude = []; - for (var key in include) { - var obj = {}; - obj[key] = include[key]; - newInclude.push(obj); - } - return newInclude; - } else { - return include; - } - } - function processIncludeItem(objs, include, cb) { var relations = self.relations; - var relationName, subInclude; + var relationName; + var subInclude = null, scope = null; if (isPlainObject(include)) { relationName = Object.keys(include)[0]; - subInclude = include[relationName]; + if (include[relationName] instanceof IncludeScope) { + scope = include[relationName]; + subInclude = scope.include(); + } else { + subInclude = include[relationName]; + } } else { relationName = include; subInclude = null; @@ -126,7 +155,31 @@ Inclusion.include = function (objects, include, cb) { var inst = (obj instanceof self) ? obj : new self(obj); // Calling the relation method on the instance - inst[relationName](function (err, result) { + + var related; // relation accessor function + + if (relation.multiple && scope) { + var includeScope = {}; + var filter = scope.conditions(); + + // make sure not to miss any fields for sub includes + if (filter.fields && Array.isArray(subInclude) && relation.modelTo.relations) { + includeScope.fields = []; + subInclude.forEach(function(name) { + var rel = relation.modelTo.relations[name]; + if (rel && rel.type === 'belongsTo') { + includeScope.fields.push(rel.keyFrom); + } + }); + } + + utils.mergeQuery(filter, includeScope, {fields: false}); + related = inst[relationName].bind(inst, filter); + } else { + related = inst[relationName].bind(inst); + } + + related(function (err, result) { if (err) { return callback(err); } else { diff --git a/lib/utils.js b/lib/utils.js index 4e317058..c3fbf841 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -3,7 +3,7 @@ exports.fieldsToArray = fieldsToArray; exports.selectFields = selectFields; exports.removeUndefined = removeUndefined; exports.parseSettings = parseSettings; -exports.mergeSettings = mergeSettings; +exports.mergeSettings = exports.deepMerge = mergeSettings; exports.isPlainObject = isPlainObject; exports.defineCachedRelations = defineCachedRelations; exports.sortObjectsByIds = sortObjectsByIds; @@ -93,6 +93,8 @@ function mergeQuery(base, update, spec) { // Overwrite fields if (spec.fields !== false && update.fields !== undefined) { base.fields = update.fields; + } else if (update.fields !== undefined) { + base.fields = [].concat(base.fields).concat(update.fields); } // set order diff --git a/test/include.test.js b/test/include.test.js index 394862a2..b68c8881 100644 --- a/test/include.test.js +++ b/test/include.test.js @@ -97,6 +97,7 @@ describe('include', function () { user.id.should.equal(p.ownerId); user.__cachedRelations.should.have.property('posts'); user.__cachedRelations.posts.forEach(function (pp) { + pp.should.have.property('id'); pp.userId.should.equal(user.id); pp.should.have.property('author'); pp.__cachedRelations.should.have.property('author'); @@ -108,6 +109,41 @@ describe('include', function () { done(); }); }); + + it('should fetch Passports with include scope on Posts', function (done) { + Passport.find({ + include: {owner: {relation: 'posts', scope:{ + fields: ['title'], include: ['author'], + order: 'title DESC' + }}} + }, function (err, passports) { + should.not.exist(err); + should.exist(passports); + passports.length.should.equal(3); + + var passport = passports[0]; + passport.number.should.equal('1'); + passport.owner().name.should.equal('User A'); + var owner = passport.owner().toObject(); + + var posts = passport.owner().posts(); + posts.should.be.an.array; + posts.should.have.length(3); + + posts[0].title.should.equal('Post C'); + posts[0].should.not.have.property('id'); // omitted + posts[0].author().should.be.instanceOf(User); + posts[0].author().name.should.equal('User A'); + + posts[1].title.should.equal('Post B'); + posts[1].author().name.should.equal('User A'); + + posts[2].title.should.equal('Post A'); + posts[2].author().name.should.equal('User A'); + + done(); + }); + }); it('should fetch User - Posts AND Passports', function (done) { User.find({include: ['posts', 'passports']}, function (err, users) { From 0c28ccedac5bc4acff230b05c47629ede11a3f92 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 10 Oct 2014 14:24:25 +0200 Subject: [PATCH 16/20] Refactored inclusion The syntax is now consistent, regardless of using [] or {} for the include param. --- lib/dao.js | 16 +++++++------- lib/include.js | 15 ++++++++++--- test/include.test.js | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index cff2e52c..2a9b9734 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -803,19 +803,19 @@ DataAccessObject.find = function find(query, cb) { // This handles the case to return parent items including the related // models. For example, Article.find({include: 'tags'}, ...); // Try to normalize the include - var includes = query.include || []; - if (typeof includes === 'string') { - includes = [includes]; - } else if (!Array.isArray(includes) && typeof includes === 'object') { - includes = Object.keys(includes); - } + var includes = Inclusion.normalizeInclude(query.include || []); includes.forEach(function (inc) { + var relationName = inc; + if (utils.isPlainObject(inc)) { + relationName = Object.keys(inc)[0]; + } + // Promote the included model as a direct property - var data = obj.__cachedRelations[inc]; + var data = obj.__cachedRelations[relationName]; if(Array.isArray(data)) { data = new List(data, null, obj); } - obj.__data[inc] = data; + if (data) obj.__data[relationName] = data; }); delete obj.__data.__cachedRelations; } diff --git a/lib/include.js b/lib/include.js index d3efc608..e3307dbb 100644 --- a/lib/include.js +++ b/lib/include.js @@ -63,6 +63,13 @@ module.exports = Inclusion; function Inclusion() { } +/** + * Normalize includes - used in DataAccessObject + * + */ + +Inclusion.normalizeInclude = normalizeInclude; + /** * Enables you to load relations of several objects and optimize numbers of requests. * @@ -98,7 +105,7 @@ Inclusion.include = function (objects, include, cb) { } include = normalizeInclude(include); - + async.each(include, function(item, callback) { processIncludeItem(objects, item, callback); }, function(err) { @@ -107,9 +114,10 @@ Inclusion.include = function (objects, include, cb) { function processIncludeItem(objs, include, cb) { var relations = self.relations; - + var relationName; var subInclude = null, scope = null; + if (isPlainObject(include)) { relationName = Object.keys(include)[0]; if (include[relationName] instanceof IncludeScope) { @@ -122,8 +130,8 @@ Inclusion.include = function (objects, include, cb) { relationName = include; subInclude = null; } - var relation = relations[relationName]; + var relation = relations[relationName]; if (!relation) { cb(new Error('Relation "' + relationName + '" is not defined for ' + self.modelName + ' model')); @@ -183,6 +191,7 @@ Inclusion.include = function (objects, include, cb) { if (err) { return callback(err); } else { + defineCachedRelations(obj); obj.__cachedRelations[relationName] = result; diff --git a/test/include.test.js b/test/include.test.js index b68c8881..9021a724 100644 --- a/test/include.test.js +++ b/test/include.test.js @@ -144,6 +144,57 @@ describe('include', function () { done(); }); }); + + it('should fetch Users with include scope on Posts', function (done) { + User.find({ + include: {relation: 'posts', scope:{ + order: 'title DESC' + }} + }, function (err, users) { + should.not.exist(err); + should.exist(users); + users.length.should.equal(5); + + users[0].name.should.equal('User A'); + users[1].name.should.equal('User B'); + + var posts = users[0].posts(); + posts.should.be.an.array; + posts.should.have.length(3); + + posts[0].title.should.equal('Post C'); + posts[1].title.should.equal('Post B'); + posts[2].title.should.equal('Post A'); + + var posts = users[1].posts(); + posts.should.be.an.array; + posts.should.have.length(1); + posts[0].title.should.equal('Post D'); + + done(); + }); + }); + + it('should fetch Users with include scope on Passports', function (done) { + User.find({ + include: {relation: 'passports', scope:{ + where: { number: '2' } + }} + }, function (err, users) { + should.not.exist(err); + should.exist(users); + users.length.should.equal(5); + + users[0].name.should.equal('User A'); + users[0].passports().should.be.empty; + + users[1].name.should.equal('User B'); + var passports = users[1].passports(); + passports[0].number.should.equal('2'); + + done(); + }); + }); it('should fetch User - Posts AND Passports', function (done) { User.find({include: ['posts', 'passports']}, function (err, users) { From 771d9505cce955dcce42b86078a4b37db266f48a Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 10 Oct 2014 14:32:14 +0200 Subject: [PATCH 17/20] Allow 'rel' and 'relation' --- lib/include.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/include.js b/lib/include.js index e3307dbb..3afa1c02 100644 --- a/lib/include.js +++ b/lib/include.js @@ -14,9 +14,10 @@ function normalizeInclude(include) { } else if (isPlainObject(include)) { // Build an array of key/value pairs var newInclude = []; - if (typeof include.relation === 'string' && isPlainObject(include.scope)) { + var rel = include.rel || include.relation; + if (typeof rel === 'string' && isPlainObject(include.scope)) { var obj = {}; - obj[include.relation] = new IncludeScope(include.scope); + obj[rel] = new IncludeScope(include.scope); newInclude.push(obj); } else { for (var key in include) { From 8d6e3adaabd13e2a65449cd003312ca256cd4072 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 10 Oct 2014 17:17:36 +0200 Subject: [PATCH 18/20] Allow include syntax without scope param --- lib/include.js | 2 +- test/include.test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/include.js b/lib/include.js index 3afa1c02..eddf7a39 100644 --- a/lib/include.js +++ b/lib/include.js @@ -15,7 +15,7 @@ function normalizeInclude(include) { // Build an array of key/value pairs var newInclude = []; var rel = include.rel || include.relation; - if (typeof rel === 'string' && isPlainObject(include.scope)) { + if (typeof rel === 'string') { var obj = {}; obj[rel] = new IncludeScope(include.scope); newInclude.push(obj); diff --git a/test/include.test.js b/test/include.test.js index 9021a724..1773eb2b 100644 --- a/test/include.test.js +++ b/test/include.test.js @@ -79,6 +79,17 @@ describe('include', function () { done(); }); }); + + it('should fetch Passport - Owner - Posts - alternate syntax', function (done) { + Passport.find({include: {owner: {relation: 'posts'}}}, function (err, passports) { + should.not.exist(err); + should.exist(passports); + passports.length.should.be.ok; + var posts = passports[0].owner().posts(); + posts.should.have.length(3); + done(); + }); + }); it('should fetch Passports - User - Posts - User', function (done) { Passport.find({ From 14d178bf69ff5210b6166bb7158ad1a5b6a213f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 10 Oct 2014 13:42:59 +0200 Subject: [PATCH 19/20] Don't inherit settings.base when extending a model Fix the bug in `ModelClass.extend` where the `base` option used in the new class was inherited from ModelClass. As a result the extended model was incorrectly based on ModelClass's parent. Modify `modelBuilder.define` to normalize the property name storing the name of the base model to `settings.base`. --- lib/model-builder.js | 11 +++++++++++ test/model-definition.test.js | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lib/model-builder.js b/lib/model-builder.js index 5962a1a0..87eed664 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -140,6 +140,10 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett var ModelBaseClass = parent || this.defaultModelBaseClass; var baseClass = settings.base || settings['super']; if (baseClass) { + // Normalize base model property + settings.base = baseClass; + delete settings['super']; + if (isModelClass(baseClass)) { ModelBaseClass = baseClass; } else { @@ -337,8 +341,15 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett } // Merge the settings + var originalSubclassSettings = subclassSettings; subclassSettings = mergeSettings(settings, subclassSettings); + // Ensure 'base' is not inherited. Note we don't have to delete 'super' + // as that is removed from settings by modelBuilder.define and thus + // it is never inherited + if (!originalSubclassSettings.base) + delete subclassSettings.base; + // Define the subclass var subClass = modelBuilder.define(className, subclassProperties, subclassSettings, ModelClass); diff --git a/test/model-definition.test.js b/test/model-definition.test.js index e7f22ff1..342dfde3 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -250,6 +250,26 @@ describe('ModelDefinition class', function () { assert(anotherChild.prototype instanceof baseChild); }); + it('should ignore inherited options.base', function() { + var memory = new DataSource({connector: Memory}); + var modelBuilder = memory.modelBuilder; + var base = modelBuilder.define('base'); + var child = base.extend('child', {}, { base: 'base' }); + var grandChild = child.extend('grand-child'); + assert.equal('child', grandChild.base.modelName); + assert(grandChild.prototype instanceof child); + }); + + it('should ignore inherited options.super', function() { + var memory = new DataSource({connector: Memory}); + var modelBuilder = memory.modelBuilder; + var base = modelBuilder.define('base'); + var child = base.extend('child', {}, { super: 'base' }); + var grandChild = child.extend('grand-child'); + assert.equal('child', grandChild.base.modelName); + assert(grandChild.prototype instanceof child); + }); + it('should not serialize hidden properties into JSON', function () { var memory = new DataSource({connector: Memory}); var modelBuilder = memory.modelBuilder; From f2ed39cb2d6c8346b04f6948a908360a7fc610f7 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 13 Oct 2014 08:34:24 -0700 Subject: [PATCH 20/20] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e9ab5236..6a145e8b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.9.0", + "version": "2.10.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",