Merge pull request #1598 from strongloop/fix/apply-updates-from-hooks

Fix Operation Hooks to propagate data changes
This commit is contained in:
Miroslav Bajtoš 2018-06-28 13:05:43 +02:00 committed by GitHub
commit 3938c3755e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 200 additions and 37 deletions

View File

@ -467,9 +467,10 @@ DataAccessObject.create = function(data, options, cb) {
hookState: hookState,
options: options,
};
Model.notifyObserversOf('persist', context, function(err) {
Model.notifyObserversOf('persist', context, function(err, ctx) {
if (err) return cb(err);
invokeConnectorMethod(connector, 'create', Model, [obj.constructor._forDB(context.data)],
val = ctx.data;
invokeConnectorMethod(connector, 'create', Model, [obj.constructor._forDB(ctx.data)],
options, createCallback);
});
}, obj, cb);
@ -1131,9 +1132,10 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
hookState: hookState,
options: options,
};
Model.notifyObserversOf('loaded', context, function(err) {
Model.notifyObserversOf('loaded', context, function(err, ctx) {
if (err) return cb(err);
data = ctx.data;
var obj, Model = self.lookupModel(data);
if (data) {
@ -2547,9 +2549,10 @@ DataAccessObject.prototype.save = function(options, cb) {
hookState: hookState,
options: options,
};
Model.notifyObserversOf('loaded', context, function(err) {
Model.notifyObserversOf('loaded', context, function(err, ctx) {
if (err) return cb(err);
data = ctx.data;
inst._initProperties(data, {persisted: true});
var context = {
@ -2579,8 +2582,9 @@ DataAccessObject.prototype.save = function(options, cb) {
options: options,
};
Model.notifyObserversOf('persist', context, function(err) {
Model.notifyObserversOf('persist', context, function(err, ctx) {
if (err) return cb(err);
data = ctx.data;
invokeConnectorMethod(connector, 'save', Model, [Model._forDB(data)],
options, saveCallback);
});
@ -2756,6 +2760,7 @@ DataAccessObject.updateAll = function(where, data, options, cb) {
};
Model.notifyObserversOf('persist', context, function(err, ctx) {
if (err) return cb(err);
data = ctx.data;
invokeConnectorMethod(connector, 'update', Model, [where, data], options, updateCallback);
});
}
@ -3084,6 +3089,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
function replaceCallback(err, data) {
if (err) return cb(err);
context.data = data;
if (typeof connector.generateContextData === 'function') {
context = connector.generateContextData(context, data);
}
@ -3132,7 +3138,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
};
Model.notifyObserversOf('persist', ctx, function(err) {
if (err) return cb(err);
invokeConnectorMethod(connector, 'replaceById', Model, [id, Model._forDB(context.data)],
invokeConnectorMethod(connector, 'replaceById', Model, [id, Model._forDB(ctx.data)],
options, replaceCallback);
});
}
@ -3277,6 +3283,7 @@ function(data, options, cb) {
function updateAttributesCallback(err, data) {
if (err) return cb(err);
context.data = data;
if (typeof connector.generateContextData === 'function') {
context = connector.generateContextData(context, data);
}
@ -3330,7 +3337,7 @@ function(data, options, cb) {
Model.notifyObserversOf('persist', ctx, function(err) {
if (err) return cb(err);
invokeConnectorMethod(connector, 'updateAttributes', Model,
[getIdValue(Model, inst), Model._forDB(context.data)],
[getIdValue(Model, inst), Model._forDB(ctx.data)],
options, updateAttributesCallback);
});
}, data, cb);

View File

@ -163,7 +163,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook when near filter is used', function(done) {
GeoModel.observe('loaded', function(ctx, next) {
ctx.data.name = 'Berlin';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'Berlin'});
next();
});
@ -181,8 +182,10 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates to one specific instance from `loaded` hook when near filter is used',
function(done) {
GeoModel.observe('loaded', function(ctx, next) {
if (ctx.data.name === 'Rome')
ctx.data.name = 'Berlin';
if (ctx.data.name === 'Rome') {
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'Berlin'});
}
next();
});
@ -199,7 +202,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook when near filter is not used', function(done) {
TestModel.observe('loaded', function(ctx, next) {
ctx.data.name = 'Paris';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'Paris'});
next();
});
@ -213,8 +217,10 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates to one specific instance from `loaded` hook when near filter is not used',
function(done) {
TestModel.observe('loaded', function(ctx, next) {
if (ctx.data.name === 'first')
ctx.data.name = 'Paris';
if (ctx.data.name === 'first') {
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'Paris'});
}
next();
});
@ -313,7 +319,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
TestModel.find(
@ -472,7 +479,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// By default, the instance passed to create callback is NOT updated
@ -536,7 +544,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// By default, the instance passed to create callback is NOT updated
@ -872,7 +881,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
if (dataSource.connector.findOrCreate) {
it('applies updates from `persist` hook when found', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
TestModel.findOrCreate(
@ -905,7 +915,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `persist` hook when not found', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
TestModel.findOrCreate(
@ -1003,7 +1014,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
if (dataSource.connector.findOrCreate) {
it('applies updates from `loaded` hook when found', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
TestModel.findOrCreate(
@ -1021,7 +1033,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook when not found', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// Unoptimized connector gives a call to `create. But,
@ -1198,7 +1211,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
existingInstance.save(function(err, instance) {
@ -1240,7 +1254,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
existingInstance.save(function(err, instance) {
@ -1364,8 +1379,11 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `before save` hook', function(done) {
TestModel.observe('before save', function(ctx, next) {
ctx.data.extra = 'extra data';
ctx.data.name = 'hooked name';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {
extra: 'extra data',
name: 'hooked name',
});
next();
});
@ -1418,7 +1436,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// By default, the instance passed to updateAttributes callback is NOT updated
@ -1429,7 +1448,11 @@ module.exports = function(dataSource, should, connectorCapabilities) {
existingInstance.updateAttributes({name: 'changed'}, function(err, instance) {
if (err) return done(err);
instance.should.have.property('extra', 'hook data');
done();
TestModel.findById(existingInstance.id, (err, found) => {
if (err) return done(err);
found.should.have.property('extra', 'hook data');
done();
});
});
});
@ -1459,7 +1482,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
ctx.data.address.should.be.type('object');
ctx.data.address.should.not.be.instanceOf(Address);
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// By default, the instance passed to updateAttributes callback is NOT updated
@ -1505,7 +1529,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
if (err) return done(err);
ctxRecorder.records.should.eql(aCtxForModel(TestModel, {
data: {name: 'changed'},
data: {id: existingInstance.id, name: 'changed'},
isNewInstance: false,
}));
@ -1525,7 +1549,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook updateAttributes', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
// By default, the instance passed to updateAttributes callback is NOT updated
@ -1583,7 +1608,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
if (!dataSource.connector.replaceById) {
describe.skip('replaceById - not implemented', function() {});
describe.skip('replaceAttributes - not implemented', function() {});
} else {
describe('PersistedModel.prototype.replaceAttributes', function() {
it('triggers hooks in the correct order', function(done) {
@ -1722,7 +1747,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
ctx.data.address.should.be.type('object');
ctx.data.address.should.not.be.instanceOf(Address);
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
existingUser.replaceAttributes(
@ -1775,7 +1801,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `loaded` hook replaceAttributes', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.name = 'changed in hook';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'changed in hook'});
}));
existingInstance.replaceAttributes({name: 'changed'}, function(err, instance) {
@ -2025,7 +2052,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `before save` hook on update', function(done) {
TestModel.observe('before save', function(ctx, next) {
ctx.data.name = 'hooked';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'hooked'});
next();
});
@ -2043,7 +2071,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
if (ctx.instance) {
ctx.instance.name = 'hooked';
} else {
ctx.data.name = 'hooked';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'hooked'});
}
next();
});
@ -2556,6 +2585,53 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
});
it('applies updates from `persist` hook on create', function(done) {
TestModel.observe('persist', (ctx, next) => {
// it's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
next();
});
// By default, the instance passed to create callback is NOT updated
// with the changes made through persist/loaded hooks. To preserve
// backwards compatibility, we introduced a new setting updateOnLoad,
// which if set, will apply these changes to the model instance too.
TestModel.settings.updateOnLoad = true;
TestModel.replaceOrCreate(
{name: 'a name'},
function(err, instance) {
if (err) return done(err);
instance.should.have.property('extra', 'hook data');
TestModel.findById(instance.id, (err, found) => {
if (err) return done(err);
found.should.have.property('extra', 'hook data');
done();
});
});
});
it('applies updates from `persist` hook on update', function(done) {
TestModel.observe('persist', (ctx, next) => {
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
next();
});
existingInstance.name = 'changed';
const data = existingInstance.toObject();
TestModel.replaceOrCreate(data, function(err, instance) {
if (err) return done(err);
instance.should.have.property('extra', 'hook data');
TestModel.findById(existingInstance.id, (err, found) => {
if (err) return done(err);
found.should.have.property('extra', 'hook data');
done();
});
});
});
it('triggers `loaded` hook on create', function(done) {
TestModel.observe('loaded', ctxRecorder.recordAndNext());
@ -2665,6 +2741,83 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
}
if (!dataSource.connector.replaceById) {
describe.skip('replaceById - not implemented', function() {});
} else {
describe('PersistedModel.replaceById', function() {
it('triggers hooks in the correct order on create', function(done) {
monitorHookExecution();
existingInstance.name = 'replaced name';
TestModel.replaceById(
existingInstance.id,
existingInstance.toObject(),
function(err, record, created) {
if (err) return done(err);
hookMonitor.names.should.eql([
'before save',
'persist',
'loaded',
'after save',
]);
done();
});
});
it('triggers `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext());
existingInstance.name = 'replaced name';
TestModel.replaceById(
existingInstance.id,
existingInstance.toObject(),
function(err, instance) {
if (err) return done(err);
const expected = {
where: {id: existingInstance.id},
data: {
id: existingInstance.id,
name: 'replaced name',
},
currentInstance: {
id: existingInstance.id,
name: 'replaced name',
extra: undefined,
},
};
const expectedContext = aCtxForModel(TestModel, expected);
expectedContext.isNewInstance = false;
ctxRecorder.records.should.eql(expectedContext);
done();
});
});
it('applies updates from `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
existingInstance.name = 'changed';
TestModel.replaceById(
existingInstance.id,
existingInstance.toObject(),
function(err, instance) {
if (err) return done(err);
instance.should.have.property('extra', 'hook data');
TestModel.findById(existingInstance.id, (err, found) => {
if (err) return done(err);
found.should.have.property('extra', 'hook data');
done();
});
});
});
});
}
describe('PersistedModel.deleteAll', function() {
it('triggers `access` hook with query', function(done) {
TestModel.observe('access', ctxRecorder.recordAndNext());
@ -3037,7 +3190,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `persist` hook', function(done) {
TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) {
ctx.data.extra = 'hook data';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {extra: 'hook data'});
}));
TestModel.updateAll(
@ -3282,7 +3436,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
it('applies updates from `before save` hook on update', function(done) {
TestModel.observe('before save', function(ctx, next) {
ctx.data.name = 'hooked';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'hooked'});
next();
});
@ -3300,7 +3455,8 @@ module.exports = function(dataSource, should, connectorCapabilities) {
if (ctx.instance) {
ctx.instance.name = 'hooked';
} else {
ctx.data.name = 'hooked';
// It's crucial to change `ctx.data` reference, not only data props
ctx.data = Object.assign({}, ctx.data, {name: 'hooked'});
}
next();
});