Enforce id (prevent user-set value), fix isNewRecord

This commit is contained in:
Fabien Franzen 2014-09-05 16:35:01 +02:00
parent aea2f64181
commit aebf5e9e6b
4 changed files with 28 additions and 7 deletions

View File

@ -137,8 +137,10 @@ DataAccessObject.create = function (data, callback) {
} }
var obj; var obj;
var idValue = getIdValue(this, data);
// if we come from save // if we come from save
if (data instanceof Model && !getIdValue(this, data)) { if (data instanceof Model && !idValue) {
obj = data; obj = data;
} else { } else {
obj = new Model(data); obj = new Model(data);
@ -315,7 +317,7 @@ DataAccessObject.findById = function find(id, cb) {
if (!getIdValue(this, data)) { if (!getIdValue(this, data)) {
setIdValue(this, data, id); setIdValue(this, data, id);
} }
obj = new this(data, {applySetters: false}); obj = new this(data, {applySetters: false, persisted: true});
} }
cb(err, obj); cb(err, obj);
}.bind(this)); }.bind(this));
@ -732,7 +734,7 @@ DataAccessObject.find = function find(query, cb) {
this.getDataSource().connector.all(this.modelName, query, function (err, data) { this.getDataSource().connector.all(this.modelName, query, function (err, data) {
if (data && data.forEach) { if (data && data.forEach) {
data.forEach(function (d, i) { data.forEach(function (d, i) {
var obj = new self(d, {fields: query.fields, applySetters: false}); var obj = new self(d, {fields: query.fields, applySetters: false, persisted: true});
if (query && query.include) { if (query && query.include) {
if (query.collect) { if (query.collect) {
@ -960,6 +962,7 @@ DataAccessObject.prototype.save = function (options, callback) {
return callback(err, inst); return callback(err, inst);
} }
inst._initProperties(data); inst._initProperties(data);
inst.__persisted = true;
updateDone.call(inst, function () { updateDone.call(inst, function () {
saveDone.call(inst, function () { saveDone.call(inst, function () {
callback(err, inst); callback(err, inst);
@ -1027,7 +1030,7 @@ DataAccessObject.updateAll = function (where, data, cb) {
}; };
DataAccessObject.prototype.isNewRecord = function () { DataAccessObject.prototype.isNewRecord = function () {
return !getIdValue(this.constructor, this); return !this.__persisted;
}; };
/** /**
@ -1168,6 +1171,7 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb
inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst), inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst),
inst.constructor._forDB(typedData), function (err) { inst.constructor._forDB(typedData), function (err) {
if (!err) inst.__persisted = true;
done.call(inst, function () { done.call(inst, function () {
saveDone.call(inst, function () { saveDone.call(inst, function () {
if(cb) cb(err, inst); if(cb) cb(err, inst);

View File

@ -489,6 +489,9 @@ DataSource.prototype.setupDataAccess = function (modelClass, settings) {
var idType = this.connector.getDefaultIdType() || String; var idType = this.connector.getDefaultIdType() || String;
idProp.type = idType; idProp.type = idType;
modelClass.definition.properties[idName].type = idType; modelClass.definition.properties[idName].type = idType;
if (settings.forceId) {
modelClass.validatesAbsenceOf(idName, { if: 'isNewRecord' });
}
} }
if (this.connector.define) { if (this.connector.define) {
// pass control to connector // pass control to connector

View File

@ -47,6 +47,7 @@ function ModelBaseClass(data, options) {
* @param {Object} options An object to control the instantiation * @param {Object} options An object to control the instantiation
* @property {Boolean} applySetters Controls if the setters will be applied * @property {Boolean} applySetters Controls if the setters will be applied
* @property {Boolean} strict Set the instance level strict mode * @property {Boolean} strict Set the instance level strict mode
* @property {Boolean} persisted Whether the instance has been persisted
* @private * @private
*/ */
ModelBaseClass.prototype._initProperties = function (data, options) { ModelBaseClass.prototype._initProperties = function (data, options) {
@ -68,6 +69,8 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
strict = ctor.definition.settings.strict; strict = ctor.definition.settings.strict;
} }
this.__persisted = options.persisted === true;
if (ctor.hideInternalProperties) { if (ctor.hideInternalProperties) {
// Object.defineProperty() is expensive. We only try to make the internal // Object.defineProperty() is expensive. We only try to make the internal
// properties hidden (non-enumerable) if the model class has the // properties hidden (non-enumerable) if the model class has the

View File

@ -16,7 +16,7 @@ describe('manipulation', function () {
age: {type: Number, index: true}, age: {type: Number, index: true},
dob: Date, dob: Date,
createdAt: {type: Number, default: Date.now} createdAt: {type: Number, default: Date.now}
}); }, { forceId: true });
db.automigrate(done); db.automigrate(done);
@ -51,6 +51,17 @@ describe('manipulation', function () {
should.not.exist(person.id); should.not.exist(person.id);
}); });
it('should not allow user-defined value for the id of object', function (done) {
Person.create({ id: 123456 }, function (err, p) {
err.should.be.instanceof(ValidationError);
err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.');
err.statusCode.should.equal(422);
p.should.be.instanceof(Person);
p.id.should.equal(123456);
done();
});
});
it('should work when called without callback', function (done) { it('should work when called without callback', function (done) {
Person.afterCreate = function (next) { Person.afterCreate = function (next) {
this.should.be.an.instanceOf(Person); this.should.be.an.instanceOf(Person);