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

Merge pull request #898 from strongloop/fix_notify_find_bugs 

Fix notify for find method
This commit is contained in:
Amir-61 2016-04-13 16:17:23 -04:00
parent c9388fa955 e9afb46eda
commit 281f554365
2 changed files with 100 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
lib/dao.js
View File

@ -1631,16 +1631,26 @@ DataAccessObject.find = function find(query, options, cb) {
// do in memory query
// using all documents
// TODO [fabien] use default scope here?
if (options.notify === false) {
queryGeo(query);
} else {
withNotifyGeo();
}
var context = {
Model: self,
query: query,
hookState: hookState,
options: options,
};
self.notifyObserversOf('access', context, function(err, ctx) {
if (err) return cb(err);
function withNotifyGeo() {
var context = {
Model: self,
query: query,
hookState: hookState,
options: options,
};
self.notifyObserversOf('access', context, function(err, ctx) {
if (err) return cb(err);
queryGeo(ctx.query);
});
}
function queryGeo(query) {
function geoCallback(err, data) {
var memory = new Memory();
var modelName = self.modelName;
@ -1661,7 +1671,7 @@ DataAccessObject.find = function find(query, options, cb) {
});
// FIXME: apply "includes" and other transforms - see allCb below
memory.all(modelName, ctx.query, options, cb);
memory.all(modelName, query, options, cb);
} else {
cb(null, []);
}
@ -1672,8 +1682,7 @@ DataAccessObject.find = function find(query, options, cb) {
} else {
connector.all(self.modelName, {}, geoCallback);
}
});
}
// already handled
return cb.promise;
}
@ -1685,18 +1694,14 @@ DataAccessObject.find = function find(query, options, cb) {
async.each(data, function(item, callback) {
var d = item;//data[i];
var Model = self.lookupModel(d);
if (options.notify === false) {
buildResult(d);
} else {
withNotify();
}
context = {
Model: Model,
data: d,
isNewInstance: false,
hookState: hookState,
options: options,
};
Model.notifyObserversOf('loaded', context, function(err) {
if (err) return callback(err);
d = context.data;
function buildResult(data) {
d = data;
var ctorOpts = {
fields: query.fields,
@ -1742,7 +1747,22 @@ DataAccessObject.find = function find(query, options, cb) {
} else {
callback();
}
});
}
function withNotify() {
context = {
Model: Model,
data: d,
isNewInstance: false,
hookState: hookState,
options: options,
};
Model.notifyObserversOf('loaded', context, function(err) {
if (err) return callback(err);
buildResult(context.data);
});
}
},
function(err) {
if (err) return cb(err);

146
test/persistence-hooks.suite.js
View File

@ -86,6 +86,45 @@ module.exports = function(dataSource, should, connectorCapabilities) {
});
});
it('should not trigger hooks, if notify is false', function(done) {
monitorHookExecution();
TestModel.find(
{ where: { id: '1' }},
{ notify: false },
function(err, list) {
if (err) return done(err);
hookMonitor.names.should.be.empty();
done();
});
});
it('should not trigger hooks for geo queries, if notify is false',
function(done) {
monitorHookExecution();
TestModel.find(
{ where: { geo: { near: '10,20' }}},
{ notify: false },
function(err, list) {
if (err) return done(err);
hookMonitor.names.should.be.empty();
done();
});
});
it('should apply updates from `access` hook', function(done) {
TestModel.observe('access', function(ctx, next) {
ctx.query = { where: { name: 'second' }};
next();
});
TestModel.find({ name: 'first' }, function(err, list) {
if (err) return done(err);
list.map(get('name')).should.eql(['second']);
done();
});
});
it('triggers `access` hook', function(done) {
TestModel.observe('access', ctxRecorder.recordAndNext());
@ -1664,24 +1703,13 @@ module.exports = function(dataSource, should, connectorCapabilities) {
{ id: existingInstance.id, name: 'new name' },
function(err, record, created) {
if (err) return done(err);
if (dataSource.connector.updateOrCreate) {
hookMonitor.names.should.eql([
'access',
'before save',
'persist',
'loaded',
'after save',
]);
} else {
hookMonitor.names.should.eql([
'access',
'loaded',
'before save',
'persist',
'loaded',
'after save',
]);
}
hookMonitor.names.should.eql([
'access',
'before save',
'persist',
'loaded',
'after save',
]);
done();
});
});
@ -2017,25 +2045,14 @@ module.exports = function(dataSource, should, connectorCapabilities) {
isNewInstance: false,
}));
} else {
// For Unoptimized connector, the callback function `contextRecorder.recordAndNext`
// is called twice. As a result, contextRecorder.records
// returns an array and NOT a single instance.
ctxRecorder.records.should.eql([
aCtxForModel(TestModel, {
data: {
id: existingInstance.id,
name: 'first',
},
isNewInstance: false,
options: { notify: false },
}),
ctxRecorder.records.should.eql(
aCtxForModel(TestModel, {
data: {
id: existingInstance.id,
name: 'updated name',
},
}),
]);
})
);
}
done();
});
@ -2119,33 +2136,13 @@ module.exports = function(dataSource, should, connectorCapabilities) {
{ id: existingInstance.id, name: 'new name' },
function(err, record, created) {
if (err) return done(err);
if (dataSource.connector.replaceOrCreate) {
hookMonitor.names.should.eql([
'access',
'before save',
'persist',
'loaded',
'after save',
]);
} else {
// TODO: Please see loopback-datasource-juggler/issues#836
//
// loaded hook is triggered twice in non-atomic version:
// 1) It gets triggered once by "find()" in this chain:
// "replaceORCreate()->findOne()->find()",
// which is a bug; Please see this ticket:
// loopback-datasource-juggler/issues#836.
// 2) It, also, gets triggered in "replaceAttributes()"
// in this chain replaceORCreate()->replaceAttributes()
hookMonitor.names.should.eql([
'access',
'loaded',
'before save',
'persist',
'loaded',
'after save',
]);
};
hookMonitor.names.should.eql([
'access',
'before save',
'persist',
'loaded',
'after save',
]);
done();
});
});
@ -2455,36 +2452,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
connectorCapabilities.replaceOrCreateReportsNewInstance ?
false : undefined;
if (dataSource.connector.replaceOrCreate) {
ctxRecorder.records.should.eql(aCtxForModel(TestModel, expected));
} else {
// TODO: Please see loopback-datasource-juggler/issues#836
//
// loaded hook is triggered twice in non-atomic version:
// 1) It gets triggered once by "find()" in this chain:
// "replaceORCreate()->findOne()->find()",
// which is a bug; Please see this ticket:
// loopback-datasource-juggler/issues#836.
// 2) It, also, gets triggered in "replaceAttributes()"
// in this chain replaceORCreate()->replaceAttributes()
ctxRecorder.records.should.eql([
aCtxForModel(TestModel, {
data: {
id: existingInstance.id,
name: 'first',
},
isNewInstance: false,
options: { notify: false },
}),
aCtxForModel(TestModel, {
data: {
id: existingInstance.id,
name: 'replaced name',
},
isNewInstance: false,
}),
]);
}
ctxRecorder.records.should.eql(aCtxForModel(TestModel, expected));
done();
});
});