From fef31883d6fac00464b93173ece82d343f896256 Mon Sep 17 00:00:00 2001 From: Arek W Date: Tue, 20 Oct 2015 10:35:40 +1100 Subject: [PATCH 1/8] Disable cache by default. Closes #672 --- Changelog.md | 3 +++ Readme.md | 16 ++++++++-------- lib/Settings.js | 2 +- package.json | 6 +++--- test/integration/association-hasone-reverse.js | 9 ++++++++- test/mocha.opts | 1 + 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/Changelog.md b/Changelog.md index 563ce79a..0c96f840 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,6 @@ +### v2.2.0 +- Singleton/cache disabled by default (#672) + ### v2.1.27 - Fix noisy mysql debug output (#642) diff --git a/Readme.md b/Readme.md index 3a9fc57d..c55cb64f 100755 --- a/Readme.md +++ b/Readme.md @@ -70,7 +70,7 @@ orm.connect("mysql://username:password@host/database", function (err, db) { }); // add the table to the database - db.sync(function(err) { + db.sync(function(err) { if (err) throw err; // add a row to the person table @@ -90,7 +90,7 @@ orm.connect("mysql://username:password@host/database", function (err, db) { // err.msg = "under-age"; }); }); - + }); }); }); @@ -278,7 +278,7 @@ var Person = db.define("person", { Other options: -- `cache` : (default: `true`) Set it to `false` to disable Instance cache ([Singletons](#singleton)) or set a timeout value (in seconds); +- `cache` : (default: `false`) Set it to `true` to enable Instance cache ([Singletons](#singleton)) or set a timeout value (in seconds); - `autoSave` : (default: `false`) Set it to `true` to save an Instance right after changing any property; - `autoFetch` : (default: `false`) Set it to `true` to fetch associations when fetching an instance from the database; - `autoFetchLimit` : (default: `1`) If `autoFetch` is enabled this defines how many hoops (associations of associations) @@ -516,16 +516,16 @@ db.driver.execQuery( ### Caching & Integrity -Model instances are cached. If multiple different queries will result in the same result, you will +Model instances can be cached (turned off by default). If enabled, multiple different queries will result in the same result - you will get the same object. If you have other systems that can change your database (or you're developing and need -to make some manual changes) you should remove this feature by disabling cache. This can be done when you're -defining the Model. +to make some manual changes) you shouldn't use this feature. +It can be enabled/disabled per model: ```js var Person = db.define('person', { name : String }, { - cache : false + cache : true }); ``` @@ -533,7 +533,7 @@ and also globally: ```js orm.connect('...', function(err, db) { - db.settings.set('instance.cache', false); + db.settings.set('instance.cache', true); }); ``` diff --git a/lib/Settings.js b/lib/Settings.js index 1fddd384..c054a672 100644 --- a/lib/Settings.js +++ b/lib/Settings.js @@ -6,7 +6,7 @@ var default_settings = { required : false }, instance : { - cache : true, + cache : false, cacheSaveCheck : true, autoSave : false, autoFetch : false, diff --git a/package.json b/package.json index 1b169278..caaf0178 100644 --- a/package.json +++ b/package.json @@ -43,9 +43,9 @@ "lodash" : "2.4.1" }, "devDependencies": { - "mysql" : "2.5.5", - "pg" : "4.3.0", - "sqlite3" : "3.0.5", + "mysql" : "2.9.0", + "pg" : "4.4.3", + "sqlite3" : "3.1.0", "async" : "0.9.0", "mocha" : "1.13.0", "should" : "1.2.2", diff --git a/test/integration/association-hasone-reverse.js b/test/integration/association-hasone-reverse.js index 2dea61c1..19fb9b37 100644 --- a/test/integration/association-hasone-reverse.js +++ b/test/integration/association-hasone-reverse.js @@ -153,7 +153,11 @@ describe("hasOne", function () { it("should be able to set an array of people as the owner", function (done) { Person.find({ name: ["John Doe", "Jane Doe"] }, function (err, owners) { + should.not.exist(err); + Pet.find({ name: "Fido" }).first(function (err, Fido) { + should.not.exist(err); + Fido.hasOwners(function (err, has_owner) { should.not.exist(err); has_owner.should.be.false; @@ -166,7 +170,10 @@ describe("hasOne", function () { should(Array.isArray(owners)); owners.length.should.equal(2); - if (owners[0] == ownersCopy[0]) { + // Don't know which order they'll be in. + var idProp = common.protocol() == 'mongodb' ? '_id' : 'id' + + if (owners[0][idProp] == ownersCopy[0][idProp]) { owners[0].should.eql(ownersCopy[0]); owners[1].should.eql(ownersCopy[1]); } else { diff --git a/test/mocha.opts b/test/mocha.opts index 5ada47be..c807f3b3 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1 +1,2 @@ --reporter spec +--timeout 5000 From de783ccb6afc2411b092b02c7408ee60d8767811 Mon Sep 17 00:00:00 2001 From: ve Date: Mon, 16 Nov 2015 22:17:27 +0800 Subject: [PATCH 2/8] send the column property to the custom type handler --- lib/Drivers/DML/mysql.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Drivers/DML/mysql.js b/lib/Drivers/DML/mysql.js index b898a4ad..e7edc87d 100644 --- a/lib/Drivers/DML/mysql.js +++ b/lib/Drivers/DML/mysql.js @@ -259,7 +259,7 @@ Driver.prototype.valueToProperty = function (value, property) { default: customType = this.customTypes[property.type]; if(customType && 'valueToProperty' in customType) { - value = customType.valueToProperty(value); + value = customType.valueToProperty(value,property); } } return value; @@ -283,7 +283,7 @@ Driver.prototype.propertyToValue = function (value, property) { default: customType = this.customTypes[property.type]; if(customType && 'propertyToValue' in customType) { - value = customType.propertyToValue(value); + value = customType.propertyToValue(value),property; } } return value; From f6737d52d60ba43ef31f6a841ffe949185cf8440 Mon Sep 17 00:00:00 2001 From: ve Date: Mon, 16 Nov 2015 22:20:51 +0800 Subject: [PATCH 3/8] send the column proterty to the custom type handler --- lib/Drivers/DML/mysql.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Drivers/DML/mysql.js b/lib/Drivers/DML/mysql.js index e7edc87d..7e381b91 100644 --- a/lib/Drivers/DML/mysql.js +++ b/lib/Drivers/DML/mysql.js @@ -283,7 +283,7 @@ Driver.prototype.propertyToValue = function (value, property) { default: customType = this.customTypes[property.type]; if(customType && 'propertyToValue' in customType) { - value = customType.propertyToValue(value),property; + value = customType.propertyToValue(value,property); } } return value; From 4d9755dd105c6e724061c13dee3e76a864e0b535 Mon Sep 17 00:00:00 2001 From: ve Date: Thu, 10 Dec 2015 22:13:41 +0800 Subject: [PATCH 4/8] remove omit prop when create instance --- lib/Instance.js | 50 +++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/lib/Instance.js b/lib/Instance.js index bfca4dea..abf08465 100755 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -493,34 +493,36 @@ function Instance(Model, opts) { } else if (prop && 'defaultValue' in prop) { defaultValue = prop.defaultValue; } + if(opts.data=={} || key in opts.data) + { + setInstanceProperty(key, defaultValue); + + Object.defineProperty(instance, key, { + get: function () { + return opts.data[key]; + }, + set: function (val) { + if (prop.key === true) { + if (prop.type == 'serial' && opts.data[key] != null) { + return; + } else { + opts.originalKeyValues[prop.name] = opts.data[prop.name]; + } + } - setInstanceProperty(key, defaultValue); - - Object.defineProperty(instance, key, { - get: function () { - return opts.data[key]; - }, - set: function (val) { - if (prop.key === true) { - if (prop.type == 'serial' && opts.data[key] != null) { + if (!setInstanceProperty(key, val)) { return; - } else { - opts.originalKeyValues[prop.name] = opts.data[prop.name]; } - } - if (!setInstanceProperty(key, val)) { - return; - } - - if (opts.autoSave) { - saveInstanceProperty(key, val); - } else if (opts.changes.indexOf(key) === -1) { - opts.changes.push(key); - } - }, - enumerable: true - }); + if (opts.autoSave) { + saveInstanceProperty(key, val); + } else if (opts.changes.indexOf(key) === -1) { + opts.changes.push(key); + } + }, + enumerable: true + }); + } }; var addInstanceExtraProperty = function (key) { if (!instance.hasOwnProperty("extra")) { From e45282c388e99ed24da8666d0d43232c859d3c7b Mon Sep 17 00:00:00 2001 From: ve Date: Fri, 11 Dec 2015 14:29:27 +0800 Subject: [PATCH 5/8] fixed 'only' bugs --- lib/Instance.js | 3 ++- lib/Model.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Instance.js b/lib/Instance.js index abf08465..e6ce871d 100755 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -8,6 +8,7 @@ exports.Instance = Instance; function Instance(Model, opts) { opts = opts || {}; opts.data = opts.data || {}; + opts.only = opts.only || []; opts.extra = opts.extra || {}; opts.keys = opts.keys || "id"; opts.changes = (opts.is_new ? Object.keys(opts.data) : []); @@ -493,7 +494,7 @@ function Instance(Model, opts) { } else if (prop && 'defaultValue' in prop) { defaultValue = prop.defaultValue; } - if(opts.data=={} || key in opts.data) + if(opts.only.length==0 || key in opts.only) { setInstanceProperty(key, defaultValue); diff --git a/lib/Model.js b/lib/Model.js index c684c4d2..7af0abbc 100644 --- a/lib/Model.js +++ b/lib/Model.js @@ -418,7 +418,8 @@ function Model(opts) { autoFetchLimit : options.autoFetchLimit, cascadeRemove : options.cascadeRemove, extra : options.extra, - extra_info : options.extra_info + extra_info : options.extra_info, + only : options.only }, cb); }, cb); } From 7df3c039d42cd03f4d2e4df4ff2c08a20d506548 Mon Sep 17 00:00:00 2001 From: ve Date: Tue, 15 Dec 2015 23:04:39 +0800 Subject: [PATCH 6/8] only update v2.2 --- lib/Instance.js | 18 +++---- lib/Model.js | 121 ++++++++++++++++++++++++------------------------ 2 files changed, 70 insertions(+), 69 deletions(-) diff --git a/lib/Instance.js b/lib/Instance.js index e6ce871d..8ce4e7c2 100755 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -48,7 +48,7 @@ function Instance(Model, opts) { var pending = [], errors = [], required; Hook.wait(instance, opts.hooks.beforeValidation, function (err) { - var k, i; + var k, i; if (err) { return saveError(cb, err); } @@ -303,7 +303,7 @@ function Instance(Model, opts) { return; } if (!instance[assoc.name].isInstance) { - instance[assoc.name] = new assoc.model(instance[assoc.name]); + instance[assoc.name] = new assoc.model(instance[assoc.name]); } saveAssociation(assoc.setAccessor, instance[assoc.name]); @@ -375,7 +375,7 @@ function Instance(Model, opts) { var conditions = {}; for (var i = 0; i < opts.keys.length; i++) { - conditions[opts.keys[i]] = opts.data[opts.keys[i]]; + conditions[opts.keys[i]] = opts.data[opts.keys[i]]; } Hook.wait(instance, opts.hooks.beforeRemove, function (err) { @@ -494,7 +494,7 @@ function Instance(Model, opts) { } else if (prop && 'defaultValue' in prop) { defaultValue = prop.defaultValue; } - if(opts.only.length==0 || key in opts.only) + if(opts.only.length==0 || opts.only.indexOf(key) > 0) { setInstanceProperty(key, defaultValue); @@ -537,8 +537,8 @@ function Instance(Model, opts) { setInstanceProperty(key, val); /*if (opts.autoSave) { - saveInstanceProperty(key, val); - }*/if (opts.extrachanges.indexOf(key) === -1) { + saveInstanceProperty(key, val); + }*/if (opts.extrachanges.indexOf(key) === -1) { opts.extrachanges.push(key); } }, @@ -603,9 +603,9 @@ function Instance(Model, opts) { cb = arg; break; default: - var err = new Error("Unknown parameter type '" + (typeof arg) + "' in Instance.save()"); - err.model = Model.table; - throw err; + var err = new Error("Unknown parameter type '" + (typeof arg) + "' in Instance.save()"); + err.model = Model.table; + throw err; } } diff --git a/lib/Model.js b/lib/Model.js index 7af0abbc..098cf548 100644 --- a/lib/Model.js +++ b/lib/Model.js @@ -107,6 +107,7 @@ function Model(opts) { hooks : opts.hooks, methods : opts.methods, validations : opts.validations, + only : inst_opts.only , one_associations : one_associations, many_associations : many_associations, extend_associations : extend_associations, @@ -147,53 +148,53 @@ function Model(opts) { }; var model = function () { - var instance, i; - - var data = arguments.length > 1 ? arguments : arguments[0]; - - if (Array.isArray(opts.keys) && Array.isArray(data)) { - if (data.length == opts.keys.length) { - var data2 = {}; - for (i = 0; i < opts.keys.length; i++) { - data2[opts.keys[i]] = data[i++]; - } - - return createInstance(data2, { isShell: true }); - } - else { - var err = new Error('Model requires ' + opts.keys.length + ' keys, only ' + data.length + ' were provided'); - err.model = opts.table; - - throw err; - } - } - else if (typeof data === "number" || typeof data === "string") { - var data2 = {}; - data2[opts.keys[0]] = data; - - return createInstance(data2, { isShell: true }); - } else if (typeof data === "undefined") { - data = {}; - } - - var isNew = false; - - for (i = 0; i < opts.keys.length; i++) { - if (!data.hasOwnProperty(opts.keys[i])) { - isNew = true; - break; - } - } - - if (keyProperties.length != 1 || (keyProperties.length == 1 && keyProperties[0].type != 'serial')) { - isNew = true; - } - - return createInstance(data, { - is_new: isNew, - autoSave: opts.autoSave, - cascadeRemove: opts.cascadeRemove - }); + var instance, i; + + var data = arguments.length > 1 ? arguments : arguments[0]; + + if (Array.isArray(opts.keys) && Array.isArray(data)) { + if (data.length == opts.keys.length) { + var data2 = {}; + for (i = 0; i < opts.keys.length; i++) { + data2[opts.keys[i]] = data[i++]; + } + + return createInstance(data2, { isShell: true }); + } + else { + var err = new Error('Model requires ' + opts.keys.length + ' keys, only ' + data.length + ' were provided'); + err.model = opts.table; + + throw err; + } + } + else if (typeof data === "number" || typeof data === "string") { + var data2 = {}; + data2[opts.keys[0]] = data; + + return createInstance(data2, { isShell: true }); + } else if (typeof data === "undefined") { + data = {}; + } + + var isNew = false; + + for (i = 0; i < opts.keys.length; i++) { + if (!data.hasOwnProperty(opts.keys[i])) { + isNew = true; + break; + } + } + + if (keyProperties.length != 1 || (keyProperties.length == 1 && keyProperties[0].type != 'serial')) { + isNew = true; + } + + return createInstance(data, { + is_new: isNew, + autoSave: opts.autoSave, + cascadeRemove: opts.cascadeRemove + }); }; model.allProperties = allProperties; @@ -255,7 +256,7 @@ function Model(opts) { var prop; if (typeof cb !== "function") { - throw new ORMError("Missing Model.get() callback", 'MISSING_CALLBACK', { model: opts.table }); + throw new ORMError("Missing Model.get() callback", 'MISSING_CALLBACK', { model: opts.table }); } if (typeof ids[ids.length - 1] === "object" && !Array.isArray(ids[ids.length - 1])) { @@ -267,7 +268,7 @@ function Model(opts) { } if (ids.length !== opts.keys.length) { - throw new ORMError("Model.get() IDs number mismatch (" + opts.keys.length + " needed, " + ids.length + " passed)", 'PARAM_MISMATCH', { model: opts.table }); + throw new ORMError("Model.get() IDs number mismatch (" + opts.keys.length + " needed, " + ids.length + " passed)", 'PARAM_MISMATCH', { model: opts.table }); } for (var i = 0; i < keyProperties.length; i++) { @@ -396,17 +397,17 @@ function Model(opts) { properties : allProperties, keyProperties: keyProperties, newInstance : function (data, cb) { - // We need to do the rename before we construct the UID & do the cache lookup - // because the cache is loaded using propertyName rather than fieldName - Utilities.renameDatastoreFieldsToPropertyNames(data, fieldToPropertyMap); + // We need to do the rename before we construct the UID & do the cache lookup + // because the cache is loaded using propertyName rather than fieldName + Utilities.renameDatastoreFieldsToPropertyNames(data, fieldToPropertyMap); - // Construct UID + // Construct UID var uid = opts.driver.uid + "/" + opts.table + (merge ? "+" + merge.from.table : ""); for (var i = 0; i < opts.keys.length; i++) { uid += "/" + data[opts.keys[i]]; } - - // Now we can do the cache lookup + options.only=this.only + // Now we can do the cache lookup Singleton.get(uid, { cache : options.cache, save_check : opts.settings.get("instance.cacheSaveCheck") @@ -449,7 +450,7 @@ function Model(opts) { } if (cb === null) { - throw new ORMError("Missing Model.one() callback", 'MISSING_CALLBACK', { model: opts.table }); + throw new ORMError("Missing Model.one() callback", 'MISSING_CALLBACK', { model: opts.table }); } // add limit 1 @@ -480,7 +481,7 @@ function Model(opts) { } if (typeof cb !== "function") { - throw new ORMError('MISSING_CALLBACK', "Missing Model.count() callback", { model: opts.table }); + throw new ORMError('MISSING_CALLBACK', "Missing Model.count() callback", { model: opts.table }); } if (conditions) { @@ -529,7 +530,7 @@ function Model(opts) { var cb = ids.pop(); if (typeof cb !== "function") { - throw new ORMError("Missing Model.exists() callback", 'MISSING_CALLBACK', { model: opts.table }); + throw new ORMError("Missing Model.exists() callback", 'MISSING_CALLBACK', { model: opts.table }); } var conditions = {}, i; @@ -696,8 +697,8 @@ function Model(opts) { enumerable: false }); Object.defineProperty(model, "uid", { - value: opts.driver.uid + "/" + opts.table + "/" + opts.keys.join("/"), - enumerable: false + value: opts.driver.uid + "/" + opts.table + "/" + opts.keys.join("/"), + enumerable: false }); // Standardize validations From a610c43c064de933c91dda475f73cdeaf5835d80 Mon Sep 17 00:00:00 2001 From: ve Date: Wed, 16 Dec 2015 14:16:14 +0800 Subject: [PATCH 7/8] fix bug --- lib/Instance.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Instance.js b/lib/Instance.js index 8ce4e7c2..97c666f2 100755 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -494,7 +494,7 @@ function Instance(Model, opts) { } else if (prop && 'defaultValue' in prop) { defaultValue = prop.defaultValue; } - if(opts.only.length==0 || opts.only.indexOf(key) > 0) + if(opts.only.length==0 || opts.only.indexOf(key) >= 0) { setInstanceProperty(key, defaultValue); From fbe2fa48f3353346305965c0c2697c4505fb2fe5 Mon Sep 17 00:00:00 2001 From: ve Date: Tue, 29 Dec 2015 20:52:59 +0800 Subject: [PATCH 8/8] extendsto bug fix --- lib/Associations/Extend.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/lib/Associations/Extend.js b/lib/Associations/Extend.js index a6d42ac1..b65efee2 100644 --- a/lib/Associations/Extend.js +++ b/lib/Associations/Extend.js @@ -143,24 +143,17 @@ function extendInstance(Model, Instance, Driver, association, opts) { if (err) { return cb(err); } + if (!Extension.isInstance) { + Extension = new association.model(Extension); + } - Instance[association.delAccessor](function (err) { - if (err) { - return cb(err); - } - - var fields = Object.keys(association.field); - - if (!Extension.isInstance) { - Extension = new association.model(Extension); - } + var fields = Object.keys(association.field); - for (var i = 0; i < Model.id.length; i++) { - Extension[fields[i]] = Instance[Model.id[i]]; - } + for (var i = 0; i < Model.id.length; i++) { + Extension[fields[i]] = Instance[Model.id[i]]; + } - Extension.save(cb); - }); + Extension.save(cb); }); return this; },