verdnatura-mirror
/
loopback-datasource-juggler
mirror of https://github.com/loopbackio/loopback-datasource-juggler
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Flag id as updateOnly when forceId is in effect (#1453) 

* updateOnly, forceId changes

* support getUpdateOnlyProperties

* fixup! fix updateOrCreate in forceId mode

The contract of `updateOrCreate` is expecting a full object instance
to be passed to the callback.

The current implementation was creating an empty instance and
calling updateAttributes under the hood. As a result, the callback
was called with the attributes being updated only.

In order to preserve existing behaviour, we have to always build
a full initial instance by calling `findById`.

See the following discussion for more context:
https://github.com/strongloop/loopback-datasource-juggler/issues/966

* fixup! fix tests of upsert validation

* move forceId to model-builder

* remove TODO comment

* revert refactoring and test fixes

* Remove duplicate test

* change testcase names

* change to ModeClass.settingse

* forceId setup from datasource to model-builder

* fix inheritance of auto-generated forceId

* Fixed failing tests for auto change

* fixed a comment
This commit is contained in:
Rashmi Hunt 2017-08-22 13:09:45 -07:00 committed by GitHub
parent aebbcd2f15
commit 6c6df15286
7 changed files with 133 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/dao.js
View File

@ -537,16 +537,11 @@ DataAccessObject.upsert = function(data, options, cb) {
if (forceId) {
options = Object.create(options);
options.validate = !!doValidate;
if (doValidate) {
Model.findById(id, options, function(err, model) {
if (err) return cb(err);
if (!model) return cb(errorModelNotFound(id));
model.updateAttributes(data, options, cb);
});
} else {
const model = new Model({id: id}, {persisted: true});
Model.findById(id, options, function(err, model) {
if (err) return cb(err);
if (!model) return cb(errorModelNotFound(id));
model.updateAttributes(data, options, cb);
}
});
return cb.promise;
}

7
lib/datasource.js
View File

@ -628,13 +628,6 @@ DataSource.prototype.setupDataAccess = function(modelClass, settings) {
idProp.type = idType;
modelClass.definition.rawProperties[idName].type = idType;
modelClass.definition.properties[idName].type = idType;
var forceId = settings.forceId;
if (idProp.generated && forceId !== false) {
forceId = true;
}
if (forceId) {
modelClass.validatesAbsenceOf(idName, {if: 'isNewRecord'});
}
}
if (this.connector.define) {
// pass control to connector

26
lib/model-builder.js
View File

@ -342,6 +342,32 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett
});
}
// updateOnly property is added to indicate that this property will appear in
// the model for update/updateorcreate operations but and not for create operation.
var forceId = ModelClass.settings.forceId;
if (idNames.length > 0) {
var idName = modelDefinition.idName();
idProp = ModelClass.definition.rawProperties[idName];
if (idProp.generated && forceId !== false) {
forceId = 'auto';
} else if (!idProp.generated && forceId === 'auto') {
// One of our parent models has enabled forceId because
// it uses an auto-generated id property. However,
// this particular model does not use auto-generated id,
// therefore we need to disable `forceId`.
forceId = false;
}
if (forceId) {
ModelClass.validatesAbsenceOf(idName, {if: 'isNewRecord'});
}
ModelClass.definition.properties[idName].updateOnly = !!forceId;
ModelClass.definition.rawProperties[idName].updateOnly = !!forceId;
ModelClass.settings.forceId = forceId;
}
// A function to loop through the properties
ModelClass.forEachProperty = function(cb) {
var props = ModelClass.definition.properties;

12
lib/model.js
View File

@ -827,6 +827,18 @@ ModelBaseClass.getMergePolicy = function(options) {
return mergePolicy;
};
/**
* Gets properties defined with 'updateOnly' flag set to true from the model. This flag is also set to true
* internally for the id property, if this property is generated and IdInjection is true.
* @returns {updateOnlyProps} List of properties with updateOnly set to true.
*/
ModelBaseClass.getUpdateOnlyProperties = function() {
var Model = this;
const props = this.definition.properties;
return Object.keys(props).filter(key => props[key].updateOnly);
};
// Mixin observer
jutil.mixin(ModelBaseClass, require('./observer'));

64
test/loopback-dl.test.js
View File

@ -645,7 +645,7 @@ describe('DataSource define model', function() {
var User = ds.define('User', {});
assert.deepEqual(User.definition.properties.id,
{type: Number, id: 1, generated: true});
{type: Number, id: 1, generated: true, updateOnly: true});
done();
});
@ -663,13 +663,13 @@ describe('DataSource define model', function() {
var User = builder.define('User', {id: {type: String, generated: true, id: true}});
assert.deepEqual(User.definition.properties.id,
{type: String, id: 1, generated: true});
{type: String, id: 1, generated: true, updateOnly: true});
var ds = new DataSource('memory');// define models
User.attachTo(ds);
assert.deepEqual(User.definition.properties.id,
{type: Number, id: 1, generated: true});
{type: Number, id: 1, generated: true, updateOnly: true});
done();
});
@ -1911,3 +1911,61 @@ describe('ModelBuilder options.models', function() {
assert.deepEqual(codes.number, ['unknown-property']);
});
});
describe('updateOnly', function() {
it('sets forceId to true when model id is generated', function(done) {
var ds = new DataSource('memory');
var Post = ds.define('Post', {
title: {type: String, length: 255},
date: {type: Date, default: function() {
return new Date();
}},
});
// check if forceId is added as true in ModelClass's settings[] explicitly,
// if id a generated (default) and forceId in from the model is
// true(unspecified is 'true' which is the default).
Post.settings.should.have.property('forceId').eql('auto');
done();
});
it('flags id as updateOnly when forceId is undefined', function(done) {
var ds = new DataSource('memory');
var Post = ds.define('Post', {
title: {type: String, length: 255},
date: {type: Date, default: function() {
return new Date();
}},
});
// check if method getUpdateOnlyProperties exist in ModelClass and check if
// the Post has 'id' in updateOnlyProperties list
Post.should.have.property('getUpdateOnlyProperties');
Post.getUpdateOnlyProperties().should.eql(['id']);
done();
});
it('does not flag id as updateOnly when forceId is false', function(done) {
var ds = new DataSource('memory');
var Person = ds.define('Person', {
name: String,
gender: String,
}, {forceId: false});
// id should not be there in updateOnly properties list if forceId is set
// to false
Person.should.have.property('getUpdateOnlyProperties');
Person.getUpdateOnlyProperties().should.eql([]);
done();
});
it('flags id as updateOnly when forceId is true', function(done) {
var ds = new DataSource('memory');
var Person = ds.define('Person', {
name: String,
gender: String,
}, {forceId: true});
// id should be there in updateOnly properties list if forceId is set
// to true
Person.should.have.property('getUpdateOnlyProperties');
Person.getUpdateOnlyProperties().should.eql(['id']);
done();
});
});

3
test/model-inheritance.test.js
View File

@ -239,6 +239,8 @@ describe('Model class inheritance', function() {
);
assert.deepEqual(User.settings, {
// forceId is set to 'auto' in memory if idProp.generated && forceId !== false
forceId: 'auto',
defaultPermission: 'ALLOW',
acls: [
{
@ -257,6 +259,7 @@ describe('Model class inheritance', function() {
});
assert.deepEqual(Customer.settings, {
forceId: false,
defaultPermission: 'DENY',
acls: [
{

44
test/validations.test.js
View File

@ -205,9 +205,12 @@ describe('validations', function() {
it('should ignore errors on upsert by default', function(done) {
delete User.validations;
User.validatesPresenceOf('name');
// It's important to pass an id value, otherwise DAO falls back
// to regular create()
User.updateOrCreate({id: 999}, done);
// It's important to pass an existing id value to updateOrCreate,
// otherwise DAO falls back to regular create()
User.create({name: 'a-name'}, (err, u) => {
if (err) return done(err);
User.updateOrCreate({id: u.id}, done);
});
});
it('should be skipped by upsert when disabled via settings', function(done) {
@ -215,26 +218,33 @@ describe('validations', function() {
Customer.attachTo(db);
db.autoupdate(function(err) {
if (err) return done(err);
Customer.prototype.isValid = function() {
throw new Error('isValid() should not be called at all');
};
Customer.settings.validateUpsert = false;
// It's important to pass an id value, otherwise DAO falls back
// to regular create()
Customer.updateOrCreate({id: 999}, done);
// It's important to pass an existing id value,
// otherwise DAO falls back to regular create()
Customer.create({name: 'a-name'}, (err, u) => {
if (err) return done(err);
Customer.prototype.isValid = function() {
throw new Error('isValid() should not be called at all');
};
Customer.settings.validateUpsert = false;
Customer.updateOrCreate({id: u.id, name: ''}, done);
});
});
});
it('should work on upsert when enabled via settings', function(done) {
delete User.validations;
User.validatesPresenceOf('name');
User.settings.validateUpsert = true;
// It's important to pass an id value, otherwise DAO falls back
// to regular create()
User.upsert({id: 999}, function(err, u) {
if (!err) return done(new Error('Validation should have failed.'));
err.should.be.instanceOf(ValidationError);
done();
// It's important to pass an existing id value,
// otherwise DAO falls back to regular create()
User.create({name: 'a-name'}, (err, u) => {
if (err) return done(err);
User.upsert({id: u.id, name: ''}, function(err, u) {
if (!err) return done(new Error('Validation should have failed.'));
err.should.be.instanceOf(ValidationError);
done();
});
});
});