From fcaf19a1d204ad27dfa1f6fbaa25e9e5dac9d126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 Feb 2015 10:31:45 +0100 Subject: [PATCH] Rename hook "query" to "access" The name "query" creates incorrect assumption that hook handlers may return the result of a query to bypass database access. That is far from true, since this hook is called also by methods like `deleteAll` or `updateAll` that don't perform any SELECT query. --- lib/dao.js | 14 ++--- test/persistence-hooks.suite.js | 98 ++++++++++++++++----------------- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index e002bf2e..27688a57 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -290,7 +290,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data return this.create(data, callback); } - Model.notifyObserversOf('query', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); + Model.notifyObserversOf('access', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); function doUpdateOrCreate(err, ctx) { if (err) return callback(err); @@ -828,7 +828,7 @@ DataAccessObject.find = function find(query, options, cb) { // using all documents // TODO [fabien] use default scope here? - self.notifyObserversOf('query', { Model: self, query: query }, function(err, ctx) { + self.notifyObserversOf('access', { Model: self, query: query }, function(err, ctx) { if (err) return cb(err); self.getDataSource().connector.all(self.modelName, {}, function (err, data) { @@ -916,7 +916,7 @@ DataAccessObject.find = function find(query, options, cb) { if (options.notify === false) { self.getDataSource().connector.all(self.modelName, query, allCb); } else { - this.notifyObserversOf('query', { Model: this, query: query }, function(err, ctx) { + this.notifyObserversOf('access', { Model: this, query: query }, function(err, ctx) { if (err) return cb(err); var query = ctx.query; self.getDataSource().connector.all(self.modelName, query, allCb); @@ -991,7 +991,7 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA doDelete(where); } else { query = { where: whereIsEmpty(where) ? {} : where }; - Model.notifyObserversOf('query', + Model.notifyObserversOf('access', { Model: Model, query: query }, function(err, ctx) { if (err) return cb(err); @@ -1099,7 +1099,7 @@ DataAccessObject.count = function (where, cb) { } var Model = this; - this.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + this.notifyObserversOf('access', { Model: Model, query: { where: where } }, function(err, ctx) { if (err) return cb(err); where = ctx.query.where; Model.getDataSource().connector.count(Model.modelName, cb, where); @@ -1242,7 +1242,7 @@ DataAccessObject.updateAll = function (where, data, cb) { var Model = this; - Model.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + Model.notifyObserversOf('access', { Model: Model, query: { where: where } }, function(err, ctx) { if (err) return cb && cb(err); Model.notifyObserversOf( 'before save', @@ -1311,7 +1311,7 @@ DataAccessObject.prototype.remove = var id = getIdValue(this.constructor, this); Model.notifyObserversOf( - 'query', + 'access', { Model: Model, query: byIdQuery(Model, id) }, function(err, ctx) { if (err) return cb(err); diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 9a523d6a..1df8699d 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -43,8 +43,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.find', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.find({ where: { id: '1' } }, function(err, list) { if (err) return done(err); @@ -55,8 +55,8 @@ module.exports = function(dataSource, should) { }); }); - it('aborts when `query` hook fails', function(done) { - TestModel.observe('query', nextWithError(expectedError)); + it('aborts when `access` hook fails', function(done) { + TestModel.observe('access', nextWithError(expectedError)); TestModel.find(function(err, list) { [err].should.eql([expectedError]); @@ -64,8 +64,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: existingInstance.id } }; next(); }); @@ -77,8 +77,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook for geo queries', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook for geo queries', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.find({ where: { geo: { near: '10,20' }}}, function(err, list) { if (err) return done(err); @@ -89,8 +89,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook for geo queries', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook for geo queries', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: existingInstance.id } }; next(); }); @@ -256,8 +256,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.findOrCreate', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.findOrCreate( { where: { name: 'new-record' } }, @@ -339,7 +339,7 @@ module.exports = function(dataSource, should) { function(err, record, created) { if (err) return done(err); triggered.should.eql([ - 'query', + 'access', 'before save', 'after save' ]); @@ -347,8 +347,8 @@ module.exports = function(dataSource, should) { }); }); - it('aborts when `query` hook fails', function(done) { - TestModel.observe('query', nextWithError(expectedError)); + it('aborts when `access` hook fails', function(done) { + TestModel.observe('access', nextWithError(expectedError)); TestModel.findOrCreate( { where: { id: 'does-not-exist' } }, @@ -403,8 +403,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.count', function(done) { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.count({ id: existingInstance.id }, function(err, count) { if (err) return done(err); @@ -415,8 +415,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query.where = { id: existingInstance.id }; next(); }); @@ -614,8 +614,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.updateOrCreate', function() { - it('triggers `query` hook on create', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook on create', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { id: 'not-found', name: 'not found' }, @@ -628,8 +628,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook on update', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook on update', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { id: existingInstance.id, name: 'new name' }, @@ -642,8 +642,8 @@ module.exports = function(dataSource, should) { }); }); - it('does not trigger `query` on missing id', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('does not trigger `access` on missing id', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { name: 'new name' }, @@ -654,8 +654,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook when found', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook when found', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -675,8 +675,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook when not found', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook when not found', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: 'not-found' } }; next(); }); @@ -698,10 +698,10 @@ module.exports = function(dataSource, should) { }); it('triggers hooks only once', function(done) { - TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('access', pushNameAndNext('access')); TestModel.observe('before save', pushNameAndNext('before save')); - TestModel.observe('query', function(ctx, next) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -710,7 +710,7 @@ module.exports = function(dataSource, should) { { id: 'ignored', name: 'new name' }, function(err, instance) { if (err) return done(err); - observersCalled.should.eql(['query', 'before save']); + observersCalled.should.eql(['access', 'before save']); done(); }); }); @@ -855,8 +855,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.deleteAll', function() { - it('triggers `query` hook with query', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook with query', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.deleteAll({ name: existingInstance.name }, function(err) { if (err) return done(err); @@ -867,8 +867,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook without query', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook without query', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.deleteAll(function(err) { if (err) return done(err); @@ -877,8 +877,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -977,8 +977,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.prototype.delete', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); existingInstance.delete(function(err) { if (err) return done(err); @@ -989,8 +989,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updated from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updated from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -1080,24 +1080,24 @@ module.exports = function(dataSource, should) { }); it('triggers hooks only once', function(done) { - TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('access', pushNameAndNext('access')); TestModel.observe('after delete', pushNameAndNext('after delete')); - TestModel.observe('query', function(ctx, next) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); existingInstance.delete(function(err) { if (err) return done(err); - observersCalled.should.eql(['query', 'after delete']); + observersCalled.should.eql(['access', 'after delete']); done(); }); }); }); describe('PersistedModel.updateAll', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateAll( { name: 'searched' }, @@ -1111,8 +1111,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); });