From 5e341b7c4cb015198cba8cb1b10048cc0d67370c Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 27 Jan 2021 11:43:18 -0500 Subject: [PATCH 01/27] Add the ability to trigger and to use middleware --- index.js | 39 ++++++++++++++++++++++++++++++--------- src/actions.js | 3 +++ src/middleware.js | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 src/actions.js create mode 100644 src/middleware.js diff --git a/index.js b/index.js index 9c9fb83a..0709e5f8 100644 --- a/index.js +++ b/index.js @@ -66,6 +66,8 @@ function ShareDbMongo(mongo, options) { } else { throw new Error('deprecated: pass mongo as url string or function with callback'); } + + this.middleware = {}; }; ShareDbMongo.prototype = Object.create(DB.prototype); @@ -215,14 +217,24 @@ ShareDbMongo.prototype.close = function(callback) { ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, options, callback) { var self = this; + var request = { + collectionName: collectionName, + id: id, + op: op, + snapshot: snapshot + }; + if (options) { + request.agent = options.agent; + } this._writeOp(collectionName, id, op, snapshot, function(err, result) { if (err) return callback(err); var opId = result.insertedId; - self._writeSnapshot(collectionName, id, snapshot, opId, function(err, succeeded) { + request.opId = opId; + self._writeSnapshot(request, function(err, succeeded) { if (succeeded) return callback(err, succeeded); // Cleanup unsuccessful op if snapshot write failed. This is not // neccessary for data correctness, but it gets rid of clutter - self._deleteOp(collectionName, opId, function(removeErr) { + self._deleteOp(request.collectionName, request.opId, function(removeErr) { callback(err || removeErr, succeeded); }); }); @@ -250,10 +262,11 @@ ShareDbMongo.prototype._deleteOp = function(collectionName, opId, callback) { }); }; -ShareDbMongo.prototype._writeSnapshot = function(collectionName, id, snapshot, opLink, callback) { - this.getCollection(collectionName, function(err, collection) { +ShareDbMongo.prototype._writeSnapshot = function(request, callback) { + var self = this; + this.getCollection(request.collectionName, function(err, collection) { if (err) return callback(err); - var doc = castToDoc(id, snapshot, opLink); + var doc = castToDoc(request.id, request.snapshot, request.opLink); if (doc._v === 1) { collection.insertOne(doc, function(err) { if (err) { @@ -267,10 +280,16 @@ ShareDbMongo.prototype._writeSnapshot = function(collectionName, id, snapshot, o callback(null, true); }); } else { - collection.replaceOne({_id: id, _v: doc._v - 1}, doc, function(err, result) { - if (err) return callback(err); - var succeeded = !!result.modifiedCount; - callback(null, succeeded); + request.queryFilter = {_id: request.id, _v: doc._v - 1}; + self.trigger(self.MIDDLEWARE_ACTIONS.replaceOne, null, request, function(middlewareErr) { + if (middlewareErr) { + return callback(middlewareErr); + } + collection.replaceOne(request.queryFilter, doc, function(err, result) { + if (err) return callback(err); + var succeeded = !!result.modifiedCount; + callback(null, succeeded); + }); }); } }); @@ -1576,3 +1595,5 @@ ShareDbMongo.parseQueryError = function(err) { err.code = 5104; return err; }; + +require('./src/middleware')(ShareDbMongo); diff --git a/src/actions.js b/src/actions.js new file mode 100644 index 00000000..41fe8a29 --- /dev/null +++ b/src/actions.js @@ -0,0 +1,3 @@ +module.exports = { + replaceOne: 'replaceOne' +}; diff --git a/src/middleware.js b/src/middleware.js new file mode 100644 index 00000000..2abc8e65 --- /dev/null +++ b/src/middleware.js @@ -0,0 +1,46 @@ +module.exports = extendWithMiddleware; +var MIDDLEWARE_ACTIONS = require('./actions'); + +function extendWithMiddleware(ShareDbMongo) { + /** + * Add middleware to an action or array of actions + */ + ShareDbMongo.prototype.use = function(action, fn) { + if (Array.isArray(action)) { + for (var i = 0; i < action.length; i++) { + this.use(action[i], fn); + } + return this; + } + var fns = this.middleware[action] || (this.middleware[action] = []); + fns.push(fn); + return this; + }; + + /** + * Passes request through the middleware stack + * + * Middleware may modify the request object. After all middleware have been + * invoked we call `callback` with `null` and the modified request. If one of + * the middleware resturns an error the callback is called with that error. + */ + ShareDbMongo.prototype.trigger = function(action, agent, request, callback) { + request.action = action; + if (agent) request.agent = agent; + + var fns = this.middleware[action]; + if (!fns) return callback(); + + // Copying the triggers we'll fire so they don't get edited while we iterate. + fns = fns.slice(); + var next = function(err) { + if (err) return callback(err); + var fn = fns.shift(); + if (!fn) return callback(); + fn(request, next); + }; + next(); + }; + + ShareDbMongo.prototype.MIDDLEWARE_ACTIONS = MIDDLEWARE_ACTIONS; +}; From 5632c4cfae9181436b0fc413bc75ba90b1dcaa00 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 27 Jan 2021 11:50:26 -0500 Subject: [PATCH 02/27] Add some comments for talking points --- index.js | 3 +++ src/actions.js | 2 ++ src/middleware.js | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 0709e5f8..f7530c96 100644 --- a/index.js +++ b/index.js @@ -217,12 +217,15 @@ ShareDbMongo.prototype.close = function(callback) { ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, options, callback) { var self = this; + // Unsure of the naming of "request" but we need an object that contains all the properties + // that should be passed through middleware var request = { collectionName: collectionName, id: id, op: op, snapshot: snapshot }; + // Maybe we use options to serve as "agent" or maybe we just pass the entire options on the request? if (options) { request.agent = options.agent; } diff --git a/src/actions.js b/src/actions.js index 41fe8a29..2a399bde 100644 --- a/src/actions.js +++ b/src/actions.js @@ -1,3 +1,5 @@ module.exports = { + // Could also be "beforeReplaceOne" + // Wondering about what the usual pattern is for middleware actions + naming replaceOne: 'replaceOne' }; diff --git a/src/middleware.js b/src/middleware.js index 2abc8e65..2e4abfa9 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -3,8 +3,8 @@ var MIDDLEWARE_ACTIONS = require('./actions'); function extendWithMiddleware(ShareDbMongo) { /** - * Add middleware to an action or array of actions - */ + * Add middleware to an action or array of actions + */ ShareDbMongo.prototype.use = function(action, fn) { if (Array.isArray(action)) { for (var i = 0; i < action.length; i++) { From 914b2c610746ae1878b45e98d8e99c7dae712f0b Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Mon, 1 Feb 2021 10:20:23 -0500 Subject: [PATCH 03/27] Some cleanup and documentation updates --- index.js | 9 +++------ src/actions.js | 5 ++--- src/middleware.js | 15 +++++++++++++-- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index f7530c96..6dc49ba7 100644 --- a/index.js +++ b/index.js @@ -223,12 +223,9 @@ ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, optio collectionName: collectionName, id: id, op: op, - snapshot: snapshot + snapshot: snapshot, + options: options }; - // Maybe we use options to serve as "agent" or maybe we just pass the entire options on the request? - if (options) { - request.agent = options.agent; - } this._writeOp(collectionName, id, op, snapshot, function(err, result) { if (err) return callback(err); var opId = result.insertedId; @@ -284,7 +281,7 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { }); } else { request.queryFilter = {_id: request.id, _v: doc._v - 1}; - self.trigger(self.MIDDLEWARE_ACTIONS.replaceOne, null, request, function(middlewareErr) { + self.trigger(self.MIDDLEWARE_ACTIONS.beforeWrite, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } diff --git a/src/actions.js b/src/actions.js index 2a399bde..75a26cd1 100644 --- a/src/actions.js +++ b/src/actions.js @@ -1,5 +1,4 @@ module.exports = { - // Could also be "beforeReplaceOne" - // Wondering about what the usual pattern is for middleware actions + naming - replaceOne: 'replaceOne' + beforeWrite: 'beforeWrite', + beforeQuery: 'beforeQuery' }; diff --git a/src/middleware.js b/src/middleware.js index 2e4abfa9..77e96a40 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -4,6 +4,14 @@ var MIDDLEWARE_ACTIONS = require('./actions'); function extendWithMiddleware(ShareDbMongo) { /** * Add middleware to an action or array of actions + * + * @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeWrite') + * @param fn The function to call when this middleware is triggered + * The fn receives a request object with information on the triggered action (e.g. the snapshot to write) + * and a next function to call once the middleware is complete + * + * NOTE: It is recommended not to add async or long running tasks to the sharedb-mongo middleware as it will + * be called very frequently during sensitive operations. It may have a significant performance impact. */ ShareDbMongo.prototype.use = function(action, fn) { if (Array.isArray(action)) { @@ -23,10 +31,13 @@ function extendWithMiddleware(ShareDbMongo) { * Middleware may modify the request object. After all middleware have been * invoked we call `callback` with `null` and the modified request. If one of * the middleware resturns an error the callback is called with that error. + * + * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeWrite') + * @param request Request details such as the snapshot to write, depends on the triggered action + * @param callback Function to call once the middleware has been processed. */ - ShareDbMongo.prototype.trigger = function(action, agent, request, callback) { + ShareDbMongo.prototype.trigger = function(action, request, callback) { request.action = action; - if (agent) request.agent = agent; var fns = this.middleware[action]; if (!fns) return callback(); From 1a7760fc0af7140dd7fc3227b7015f700295a09b Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 10:38:26 -0500 Subject: [PATCH 04/27] Fix naming of opLink to opId --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 6dc49ba7..becbcfc3 100644 --- a/index.js +++ b/index.js @@ -266,7 +266,7 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { var self = this; this.getCollection(request.collectionName, function(err, collection) { if (err) return callback(err); - var doc = castToDoc(request.id, request.snapshot, request.opLink); + var doc = castToDoc(request.id, request.snapshot, request.opId); if (doc._v === 1) { collection.insertOne(doc, function(err) { if (err) { From 14f7b0c3476776cd0e5244c4566d87346686e05e Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 11:39:30 -0500 Subject: [PATCH 05/27] Add test for middleware --- test/test_mongo_middleware.js | 79 +++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 test/test_mongo_middleware.js diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js new file mode 100644 index 00000000..89888177 --- /dev/null +++ b/test/test_mongo_middleware.js @@ -0,0 +1,79 @@ +var expect = require('chai').expect; +var ShareDbMongo = require('..'); +var getQuery = require('sharedb-mingo-memory/get-query'); +var async = require('async'); + +var mongoUrl = process.env.TEST_MONGO_URL || 'mongodb://localhost:27017/test'; + +function create(callback) { + var db = new ShareDbMongo(mongoUrl); + db.getDbs(function(err, mongo) { + if (err) return callback(err); + mongo.dropDatabase(function(err) { + if (err) return callback(err); + callback(null, db, mongo); + }); + }); +}; + +describe('mongo db', function() { + beforeEach(function(done) { + var self = this; + create(function(err, db, mongo) { + if (err) return done(err); + self.db = db; + self.mongo = mongo; + done(); + }); + }); + + afterEach(function(done) { + this.db.close(done); + }); + + describe.only('middleware', function() { + it('should augment query filter', function(done) { + var db = this.db; + db.use('beforeWrite', function(request, next) { + request.queryFilter.foo = 'bar'; + next(); + }); + db.use('beforeWrite', function(request, next) { + expect(request.queryFilter).to.deep.equal({ + _id: 'test1', + _v: 1, + foo: 'bar' + }); + next(); + }); + + var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; + var query = {_id: 'test1'}; + + function findsTest1(valueOfFoo, cb) { + db.query('testcollection', query, null, null, function(err, results) { + if (err) return done(err); + + expect(results[0].data).eql({ + foo: valueOfFoo + }); + + cb(); + }); + }; + + var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + findsTest1('bar', function() { + db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { + if (err) return done(err); + findsTest1('fuzz', done); + }); + }); + }); + }); + }); +}); From 1f87aee5aabb4a08fbafa47568acffe4dc694c36 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 11:53:22 -0500 Subject: [PATCH 06/27] Add tests --- test/test_mongo_middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 89888177..d01d1bc6 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -31,7 +31,7 @@ describe('mongo db', function() { this.db.close(done); }); - describe.only('middleware', function() { + describe('middleware', function() { it('should augment query filter', function(done) { var db = this.db; db.use('beforeWrite', function(request, next) { From 15eab510c2a9fde09e132f670c5e6ab9450a00ec Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 13:51:35 -0500 Subject: [PATCH 07/27] Fix linting issues --- test/test_mongo_middleware.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index d01d1bc6..9e6f901d 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -1,7 +1,5 @@ var expect = require('chai').expect; var ShareDbMongo = require('..'); -var getQuery = require('sharedb-mingo-memory/get-query'); -var async = require('async'); var mongoUrl = process.env.TEST_MONGO_URL || 'mongodb://localhost:27017/test'; @@ -16,7 +14,7 @@ function create(callback) { }); }; -describe('mongo db', function() { +describe('mongo db middleware', function() { beforeEach(function(done) { var self = this; create(function(err, db, mongo) { @@ -31,13 +29,20 @@ describe('mongo db', function() { this.db.close(done); }); - describe('middleware', function() { - it('should augment query filter', function(done) { + describe('beforeWrite', function() { + it('should augment query filter and write to the document when commit is called', function(done) { var db = this.db; + // Augment the queryFilter. The original query looks up the document by id, wheras this middleware + // changes it to use the `foo` property. The end result still returns the same document. The next + // middleware ensures we attached it to the request. + // We can't truly change which document is returned from the query because MongoDB will not allow + // the immutable fields such as `_id` to be changed. db.use('beforeWrite', function(request, next) { request.queryFilter.foo = 'bar'; next(); }); + // Attach this middleware to check that the original one is passing the context + // correctly. Commit will be called after this. db.use('beforeWrite', function(request, next) { expect(request.queryFilter).to.deep.equal({ _id: 'test1', @@ -70,6 +75,7 @@ describe('mongo db', function() { findsTest1('bar', function() { db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { if (err) return done(err); + // Ensure the value is updated as expected findsTest1('fuzz', done); }); }); From b1e60404f2a1b58983314128a74c7590c08d948c Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 15:13:31 -0500 Subject: [PATCH 08/27] Add support for middleware on snapshot retrieval --- index.js | 81 ++++++++++++--------- src/actions.js | 6 +- src/middleware.js | 4 +- test/test_mongo_middleware.js | 132 +++++++++++++++++++++++++++++----- 4 files changed, 169 insertions(+), 54 deletions(-) diff --git a/index.js b/index.js index becbcfc3..d6359aa7 100644 --- a/index.js +++ b/index.js @@ -217,15 +217,7 @@ ShareDbMongo.prototype.close = function(callback) { ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, options, callback) { var self = this; - // Unsure of the naming of "request" but we need an object that contains all the properties - // that should be passed through middleware - var request = { - collectionName: collectionName, - id: id, - op: op, - snapshot: snapshot, - options: options - }; + var request = createRequestForMiddleware(collectionName, id, op, snapshot, options); this._writeOp(collectionName, id, op, snapshot, function(err, result) { if (err) return callback(err); var opId = result.insertedId; @@ -241,6 +233,17 @@ ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, optio }); }; +function createRequestForMiddleware(collectionName, id, op, snapshot, options) { + // Create a new request object which will be passed to helper functions and middleware + return { + collectionName: collectionName, + id: id, + op: op, + snapshot: snapshot, + options: options + }; +} + ShareDbMongo.prototype._writeOp = function(collectionName, id, op, snapshot, callback) { if (typeof op.v !== 'number') { var err = ShareDbMongo.invalidOpVersionError(collectionName, id, op.v); @@ -266,9 +269,9 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { var self = this; this.getCollection(request.collectionName, function(err, collection) { if (err) return callback(err); - var doc = castToDoc(request.id, request.snapshot, request.opId); - if (doc._v === 1) { - collection.insertOne(doc, function(err) { + request.doc = castToDoc(request.id, request.snapshot, request.opId); + if (request.doc._v === 1) { + collection.insertOne(request.doc, function(err) { if (err) { // Return non-success instead of duplicate key error, since this is // expected to occur during simultaneous creates on the same id @@ -280,12 +283,12 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { callback(null, true); }); } else { - request.queryFilter = {_id: request.id, _v: doc._v - 1}; - self.trigger(self.MIDDLEWARE_ACTIONS.beforeWrite, request, function(middlewareErr) { + request.queryFilter = {_id: request.id, _v: request.doc._v - 1}; + self.trigger(self.MIDDLEWARE_ACTIONS.beforeEdit, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } - collection.replaceOne(request.queryFilter, doc, function(err, result) { + collection.replaceOne(request.queryFilter, request.doc, function(err, result) { if (err) return callback(err); var succeeded = !!result.modifiedCount; callback(null, succeeded); @@ -299,36 +302,50 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { // **** Snapshot methods ShareDbMongo.prototype.getSnapshot = function(collectionName, id, fields, options, callback) { + var self = this; this.getCollection(collectionName, function(err, collection) { if (err) return callback(err); var query = {_id: id}; var projection = getProjection(fields, options); - collection.find(query).limit(1).project(projection).next(function(err, doc) { - if (err) return callback(err); - var snapshot = (doc) ? castToSnapshot(doc) : new MongoSnapshot(id, 0, null, undefined); - callback(null, snapshot); + var request = createRequestForMiddleware(collectionName, id, null, null, options); + request.query = query; + self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { + if (middlewareErr) return callback(middlewareErr); + + collection.find(request.query).limit(1).project(projection).next(function(err, doc) { + if (err) return callback(err); + var snapshot = (doc) ? castToSnapshot(doc) : new MongoSnapshot(id, 0, null, undefined); + callback(null, snapshot); + }); }); }); }; ShareDbMongo.prototype.getSnapshotBulk = function(collectionName, ids, fields, options, callback) { + var self = this; this.getCollection(collectionName, function(err, collection) { if (err) return callback(err); var query = {_id: {$in: ids}}; var projection = getProjection(fields, options); - collection.find(query).project(projection).toArray(function(err, docs) { - if (err) return callback(err); - var snapshotMap = {}; - for (var i = 0; i < docs.length; i++) { - var snapshot = castToSnapshot(docs[i]); - snapshotMap[snapshot.id] = snapshot; - } - for (var i = 0; i < ids.length; i++) { - var id = ids[i]; - if (snapshotMap[id]) continue; - snapshotMap[id] = new MongoSnapshot(id, 0, null, undefined); - } - callback(null, snapshotMap); + var request = createRequestForMiddleware(collectionName, ids, null, null, options); + request.query = query; + self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { + if (middlewareErr) return callback(middlewareErr); + + collection.find(request.query).project(projection).toArray(function(err, docs) { + if (err) return callback(err); + var snapshotMap = {}; + for (var i = 0; i < docs.length; i++) { + var snapshot = castToSnapshot(docs[i]); + snapshotMap[snapshot.id] = snapshot; + } + for (var i = 0; i < ids.length; i++) { + var id = ids[i]; + if (snapshotMap[id]) continue; + snapshotMap[id] = new MongoSnapshot(id, 0, null, undefined); + } + callback(null, snapshotMap); + }); }); }); }; diff --git a/src/actions.js b/src/actions.js index 75a26cd1..36c9f2c4 100644 --- a/src/actions.js +++ b/src/actions.js @@ -1,4 +1,6 @@ module.exports = { - beforeWrite: 'beforeWrite', - beforeQuery: 'beforeQuery' + // Triggers before the call to replace a document is made to Mongo + beforeEdit: 'beforeEdit', + // Triggers directly before the call to issue a query to Mongo for a single snapshot by ID + beforeSnapshotLookup: 'beforeSnapshotLookup' }; diff --git a/src/middleware.js b/src/middleware.js index 77e96a40..80a6efac 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -5,7 +5,7 @@ function extendWithMiddleware(ShareDbMongo) { /** * Add middleware to an action or array of actions * - * @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeWrite') + * @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') * @param fn The function to call when this middleware is triggered * The fn receives a request object with information on the triggered action (e.g. the snapshot to write) * and a next function to call once the middleware is complete @@ -32,7 +32,7 @@ function extendWithMiddleware(ShareDbMongo) { * invoked we call `callback` with `null` and the modified request. If one of * the middleware resturns an error the callback is called with that error. * - * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeWrite') + * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') * @param request Request details such as the snapshot to write, depends on the triggered action * @param callback Function to call once the middleware has been processed. */ diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 9e6f901d..2399a01f 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -1,3 +1,4 @@ +var async = require('async'); var expect = require('chai').expect; var ShareDbMongo = require('..'); @@ -14,7 +15,7 @@ function create(callback) { }); }; -describe('mongo db middleware', function() { +describe.only('mongo db middleware', function() { beforeEach(function(done) { var self = this; create(function(err, db, mongo) { @@ -29,7 +30,7 @@ describe('mongo db middleware', function() { this.db.close(done); }); - describe('beforeWrite', function() { + describe('beforeEdit', function() { it('should augment query filter and write to the document when commit is called', function(done) { var db = this.db; // Augment the queryFilter. The original query looks up the document by id, wheras this middleware @@ -37,13 +38,13 @@ describe('mongo db middleware', function() { // middleware ensures we attached it to the request. // We can't truly change which document is returned from the query because MongoDB will not allow // the immutable fields such as `_id` to be changed. - db.use('beforeWrite', function(request, next) { + db.use('beforeEdit', function(request, next) { request.queryFilter.foo = 'bar'; next(); }); // Attach this middleware to check that the original one is passing the context // correctly. Commit will be called after this. - db.use('beforeWrite', function(request, next) { + db.use('beforeEdit', function(request, next) { expect(request.queryFilter).to.deep.equal({ _id: 'test1', _v: 1, @@ -53,33 +54,128 @@ describe('mongo db middleware', function() { }); var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; - var query = {_id: 'test1'}; - function findsTest1(valueOfFoo, cb) { - db.query('testcollection', query, null, null, function(err, results) { + var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + expectDocumentToContainFoo(db, 'bar', function() { + db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { + if (err) return done(err); + // Ensure the value is updated as expected + expectDocumentToContainFoo(db, 'fuzz', done); + }); + }); + }); + }); + }); + + it('should augment the written document when commit is called', function(done) { + var db = this.db; + // Change the written value of foo to be `fuzz` + db.use('beforeEdit', function(request, next) { + request.doc.foo = 'fuzz'; + next(); + }); + + var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; + + // Issue a commit to change `bar` to `baz` + var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + expectDocumentToContainFoo(db, 'bar', function() { + db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { + if (err) return done(err); + // Ensure the value is updated as expected + expectDocumentToContainFoo(db, 'fuzz', done); + }); + }); + }); + }); + + describe('beforeSnapshotLookup', function() { + it('should augment the query when getSnapshot is called', function(done) { + var db = this.db; + + var snapshots = [ + {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}, + {type: 'json0', id: 'test2', v: 1, data: {foo: 'baz'}} + ]; + + async.each(snapshots, function(snapshot, cb) { + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, cb); + }, function(err) { + if (err) return done(err); + db.getSnapshot('testcollection', 'test1', null, null, function(err, doc) { if (err) return done(err); + expect(doc.data).eql({ + foo: 'bar' + }); - expect(results[0].data).eql({ - foo: valueOfFoo + // Change the query to look for baz and not bar + db.use('beforeSnapshotLookup', function(request, next) { + request.query = {_id: 'test2'}; + next(); }); - cb(); + db.getSnapshot('testcollection', 'test1', null, null, function(err, doc) { + if (err) return done(err); + expect(doc.data).eql({ + foo: 'baz' + }); + done(); + }); }); - }; + }); + }); - var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; - var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + it('should augment the query when getSnapshotBulk is called', function(done) { + var db = this.db; - db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + var snapshots = [ + {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}, + {type: 'json0', id: 'test2', v: 1, data: {foo: 'baz'}} + ]; + + async.each(snapshots, function(snapshot, cb) { + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, cb); + }, function(err) { if (err) return done(err); - findsTest1('bar', function() { - db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { + db.getSnapshotBulk('testcollection', ['test1', 'test2'], null, null, function(err, docs) { + if (err) return done(err); + expect(docs.test1.data.foo).to.equal('bar'); + expect(docs.test2.data.foo).to.equal('baz'); + + // Change the query to look for baz and not bar + db.use('beforeSnapshotLookup', function(request, next) { + request.query = {_id: {$in: ['test2']}}; + next(); + }); + + db.getSnapshotBulk('testcollection', ['test1', 'test2'], null, null, function(err, docs) { if (err) return done(err); - // Ensure the value is updated as expected - findsTest1('fuzz', done); + expect(docs.test1.data).not.to.exist; + expect(docs.test2.data.foo).to.equal('baz'); + done(); }); }); }); }); }); + + function expectDocumentToContainFoo(db, valueOfFoo, cb) { + var query = {_id: 'test1'}; + + db.query('testcollection', query, null, null, function(err, results) { + if (err) return done(err); + expect(results[0].data).eql({ + foo: valueOfFoo + }); + cb(); + }); + }; }); From e7b30a816b574a1a6a43aed68f3d4b4829e9da91 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 15:16:16 -0500 Subject: [PATCH 09/27] Remove only on test --- test/test_mongo_middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 2399a01f..1932fa68 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -15,7 +15,7 @@ function create(callback) { }); }; -describe.only('mongo db middleware', function() { +describe('mongo db middleware', function() { beforeEach(function(done) { var self = this; create(function(err, db, mongo) { From 3b6d2273924781436d9fc8e4ab1d835910a4e6e7 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 15:50:04 -0500 Subject: [PATCH 10/27] Reduce information that is not currently needed on the request --- index.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index d6359aa7..0c470dee 100644 --- a/index.js +++ b/index.js @@ -217,31 +217,29 @@ ShareDbMongo.prototype.close = function(callback) { ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, options, callback) { var self = this; - var request = createRequestForMiddleware(collectionName, id, op, snapshot, options); + var request = createRequestForMiddleware(options, collectionName, op); this._writeOp(collectionName, id, op, snapshot, function(err, result) { if (err) return callback(err); var opId = result.insertedId; - request.opId = opId; - self._writeSnapshot(request, function(err, succeeded) { + self._writeSnapshot(request, id, snapshot, opId, function(err, succeeded) { if (succeeded) return callback(err, succeeded); // Cleanup unsuccessful op if snapshot write failed. This is not // neccessary for data correctness, but it gets rid of clutter - self._deleteOp(request.collectionName, request.opId, function(removeErr) { + self._deleteOp(request.collectionName, opId, function(removeErr) { callback(err || removeErr, succeeded); }); }); }); }; -function createRequestForMiddleware(collectionName, id, op, snapshot, options) { +function createRequestForMiddleware(options, collectionName, op) { // Create a new request object which will be passed to helper functions and middleware - return { - collectionName: collectionName, - id: id, - op: op, - snapshot: snapshot, - options: options + var request = { + options: options, + collectionName: collectionName }; + if (op) request.op = op; + return request; } ShareDbMongo.prototype._writeOp = function(collectionName, id, op, snapshot, callback) { @@ -265,11 +263,11 @@ ShareDbMongo.prototype._deleteOp = function(collectionName, opId, callback) { }); }; -ShareDbMongo.prototype._writeSnapshot = function(request, callback) { +ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, callback) { var self = this; this.getCollection(request.collectionName, function(err, collection) { if (err) return callback(err); - request.doc = castToDoc(request.id, request.snapshot, request.opId); + request.doc = castToDoc(id, snapshot, opId); if (request.doc._v === 1) { collection.insertOne(request.doc, function(err) { if (err) { @@ -283,7 +281,7 @@ ShareDbMongo.prototype._writeSnapshot = function(request, callback) { callback(null, true); }); } else { - request.queryFilter = {_id: request.id, _v: request.doc._v - 1}; + request.queryFilter = {_id: id, _v: request.doc._v - 1}; self.trigger(self.MIDDLEWARE_ACTIONS.beforeEdit, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); @@ -307,7 +305,7 @@ ShareDbMongo.prototype.getSnapshot = function(collectionName, id, fields, option if (err) return callback(err); var query = {_id: id}; var projection = getProjection(fields, options); - var request = createRequestForMiddleware(collectionName, id, null, null, options); + var request = createRequestForMiddleware(options, collectionName); request.query = query; self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { if (middlewareErr) return callback(middlewareErr); From 38ca0ef2de651a5e9dc2c2e67344a9d19eef9bb7 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 15:57:02 -0500 Subject: [PATCH 11/27] Rename queryFilter to query --- index.js | 4 ++-- test/test_mongo_middleware.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 0c470dee..76937fd8 100644 --- a/index.js +++ b/index.js @@ -281,12 +281,12 @@ ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, ca callback(null, true); }); } else { - request.queryFilter = {_id: id, _v: request.doc._v - 1}; + request.query = {_id: id, _v: request.doc._v - 1}; self.trigger(self.MIDDLEWARE_ACTIONS.beforeEdit, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } - collection.replaceOne(request.queryFilter, request.doc, function(err, result) { + collection.replaceOne(request.query, request.doc, function(err, result) { if (err) return callback(err); var succeeded = !!result.modifiedCount; callback(null, succeeded); diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 1932fa68..c1571221 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -33,19 +33,19 @@ describe('mongo db middleware', function() { describe('beforeEdit', function() { it('should augment query filter and write to the document when commit is called', function(done) { var db = this.db; - // Augment the queryFilter. The original query looks up the document by id, wheras this middleware + // Augment the query. The original query looks up the document by id, wheras this middleware // changes it to use the `foo` property. The end result still returns the same document. The next // middleware ensures we attached it to the request. // We can't truly change which document is returned from the query because MongoDB will not allow // the immutable fields such as `_id` to be changed. db.use('beforeEdit', function(request, next) { - request.queryFilter.foo = 'bar'; + request.query.foo = 'bar'; next(); }); // Attach this middleware to check that the original one is passing the context // correctly. Commit will be called after this. db.use('beforeEdit', function(request, next) { - expect(request.queryFilter).to.deep.equal({ + expect(request.query).to.deep.equal({ _id: 'test1', _v: 1, foo: 'bar' From 03b3436f0f27da61089e59a00afa50cff58f463b Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 16:20:15 -0500 Subject: [PATCH 12/27] Add tests for definition of request object --- index.js | 2 +- test/test_mongo_middleware.js | 88 ++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 76937fd8..6225fd4e 100644 --- a/index.js +++ b/index.js @@ -325,7 +325,7 @@ ShareDbMongo.prototype.getSnapshotBulk = function(collectionName, ids, fields, o if (err) return callback(err); var query = {_id: {$in: ids}}; var projection = getProjection(fields, options); - var request = createRequestForMiddleware(collectionName, ids, null, null, options); + var request = createRequestForMiddleware(options, collectionName); request.query = query; self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { if (middlewareErr) return callback(middlewareErr); diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index c1571221..ae8c6485 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -31,6 +31,39 @@ describe('mongo db middleware', function() { }); describe('beforeEdit', function() { + it('has the expected properties on the request object', function(done) { + var db = this.db; + db.use('beforeEdit', function(request, next) { + expect(request).to.have.all.keys([ + 'action', + 'collectionName', + 'doc', + 'op', + 'options', + 'query' + ]); + expect(request.action).to.equal('beforeEdit'); + expect(request.collectionName).to.equal('testcollection'); + expect(request.doc.foo).to.equal('fuzz'); + expect(request.op.op).to.exist; + expect(request.options.testOptions).to.equal('yes'); + expect(request.query._id).to.equal('test1'); + next(); + done(); + }); + + var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; + var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + db.commit('testcollection', snapshot.id, editOp, newSnapshot, {testOptions: 'yes'}, function(err) { + if (err) return done(err); + }); + }); + }); + it('should augment query filter and write to the document when commit is called', function(done) { var db = this.db; // Augment the query. The original query looks up the document by id, wheras this middleware @@ -54,7 +87,6 @@ describe('mongo db middleware', function() { }); var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; - var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; @@ -98,6 +130,60 @@ describe('mongo db middleware', function() { }); describe('beforeSnapshotLookup', function() { + it('has the expected properties on the request object before getting a single snapshot', function(done) { + var db = this.db; + db.use('beforeSnapshotLookup', function(request, next) { + expect(request).to.have.all.keys([ + 'action', + 'collectionName', + 'options', + 'query' + ]); + expect(request.action).to.equal('beforeSnapshotLookup'); + expect(request.collectionName).to.equal('testcollection'); + expect(request.options.testOptions).to.equal('yes'); + expect(request.query._id).to.equal('test1'); + next(); + done(); + }); + + var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + db.getSnapshot('testcollection', 'test1', null, {testOptions: 'yes'}, function(err, doc) { + if (err) return done(err); + expect(doc).to.exist; + }); + }); + }); + + it('has the expected properties on the request object before getting bulk snapshots', function(done) { + var db = this.db; + db.use('beforeSnapshotLookup', function(request, next) { + expect(request).to.have.all.keys([ + 'action', + 'collectionName', + 'options', + 'query' + ]); + expect(request.action).to.equal('beforeSnapshotLookup'); + expect(request.collectionName).to.equal('testcollection'); + expect(request.options.testOptions).to.equal('yes'); + expect(request.query._id).to.deep.equal({$in: ['test1']}); + next(); + done(); + }); + + var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { + if (err) return done(err); + db.getSnapshotBulk('testcollection', ['test1'], null, {testOptions: 'yes'}, function(err, doc) { + if (err) return done(err); + expect(doc).to.exist; + }); + }); + }); + it('should augment the query when getSnapshot is called', function(done) { var db = this.db; From 96ed2aaadbbf331a7949eba0cfadaacdac5afb18 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 16:22:35 -0500 Subject: [PATCH 13/27] Small wording fix in comments --- src/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions.js b/src/actions.js index 36c9f2c4..bac56ee2 100644 --- a/src/actions.js +++ b/src/actions.js @@ -1,6 +1,6 @@ module.exports = { // Triggers before the call to replace a document is made to Mongo beforeEdit: 'beforeEdit', - // Triggers directly before the call to issue a query to Mongo for a single snapshot by ID + // Triggers directly before the call to issue a query to Mongo for snapshots by ID beforeSnapshotLookup: 'beforeSnapshotLookup' }; From ca7173718e84753051ab14a78b62e90b84d56d9d Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 16:26:35 -0500 Subject: [PATCH 14/27] Fix confusion on baz vs fuzz in commit tests --- test/test_mongo_middleware.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index ae8c6485..e44301b8 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -88,7 +88,7 @@ describe('mongo db middleware', function() { var snapshot = {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}; var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; - var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'baz'}}; db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { if (err) return done(err); @@ -96,7 +96,7 @@ describe('mongo db middleware', function() { db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { if (err) return done(err); // Ensure the value is updated as expected - expectDocumentToContainFoo(db, 'fuzz', done); + expectDocumentToContainFoo(db, 'baz', done); }); }); }); @@ -115,7 +115,7 @@ describe('mongo db middleware', function() { // Issue a commit to change `bar` to `baz` var editOp = {v: 2, op: [{p: ['foo'], oi: 'bar', oi: 'baz'}], m: {ts: Date.now()}}; - var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'fuzz'}}; + var newSnapshot = {type: 'json0', id: 'test1', v: 2, data: {foo: 'baz'}}; db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { if (err) return done(err); From 2f68ab92f3c2681da4c6596a3b2118468d1a3784 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 16:43:09 -0500 Subject: [PATCH 15/27] Update documentation --- README.md | 102 ++++++++++++++++++++++++++++++------------------- src/actions.js | 4 +- 2 files changed, 65 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index d4d9bc17..6ee8ceb5 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,13 @@ # sharedb-mongo - [![NPM Version](https://img.shields.io/npm/v/sharedb-mongo.svg)](https://npmjs.org/package/sharedb-mongo) - [![Build Status](https://travis-ci.org/share/sharedb-mongo.svg?branch=master)](https://travis-ci.org/share/sharedb-mongo) - [![Coverage Status](https://coveralls.io/repos/github/share/sharedb-mongo/badge.svg?branch=master)](https://coveralls.io/github/share/sharedb-mongo?branch=master) +[![NPM Version](https://img.shields.io/npm/v/sharedb-mongo.svg)](https://npmjs.org/package/sharedb-mongo) +[![Build Status](https://travis-ci.org/share/sharedb-mongo.svg?branch=master)](https://travis-ci.org/share/sharedb-mongo) +[![Coverage Status](https://coveralls.io/repos/github/share/sharedb-mongo/badge.svg?branch=master)](https://coveralls.io/github/share/sharedb-mongo?branch=master) MongoDB database adapter for [sharedb](https://github.com/share/sharedb). This driver can be used both as a snapshot store and oplog. -Snapshots are stored where you'd expect (the named collection with _id=id). In +Snapshots are stored where you'd expect (the named collection with \_id=id). In addition, operations are stored in `o_COLLECTION`. For example, if you have a `users` collection, the operations are stored in `o_users`. @@ -17,38 +17,36 @@ the form of `_v` and `_type`). It is safe to query documents directly with the MongoDB driver or command line. Any read only mongo features, including find, aggregate, and map reduce are safe to perform concurrent with ShareDB. -However, you must *always* use ShareDB to edit documents. Never use the +However, you must _always_ use ShareDB to edit documents. Never use the MongoDB driver or command line to directly modify any documents that ShareDB might create or edit. ShareDB must be used to properly persist operations together with snapshots. - ## Usage `sharedb-mongo` uses the [MongoDB NodeJS Driver](https://github.com/mongodb/node-mongodb-native), and it supports the same configuration options. There are two ways to instantiate a sharedb-mongo wrapper: -1. The simplest way is to invoke the module and pass in your mongo DB -arguments as arguments to the module function. For example: - - ```javascript - const db = require('sharedb-mongo')('mongodb://localhost:27017/test', {mongoOptions: {...}}); - const backend = new ShareDB({db}); - ``` +1. The simplest way is to invoke the module and pass in your mongo DB + arguments as arguments to the module function. For example: -2. If you'd like to reuse a mongo db connection or handle mongo driver -instantiation yourself, you can pass in a function that calls back with -a mongo instance. + ```javascript + const db = require('sharedb-mongo')('mongodb://localhost:27017/test', {mongoOptions: {...}}); + const backend = new ShareDB({db}); + ``` - ```javascript - const mongodb = require('mongodb'); - const db = require('sharedb-mongo')({mongo: function(callback) { - mongodb.connect('mongodb://localhost:27017/test', callback); - }}); - const backend = new ShareDB({db}); - ``` +2. If you'd like to reuse a mongo db connection or handle mongo driver + instantiation yourself, you can pass in a function that calls back with + a mongo instance. + ```javascript + const mongodb = require('mongodb'); + const db = require('sharedb-mongo')({mongo: function(callback) { + mongodb.connect('mongodb://localhost:27017/test', callback); + }}); + const backend = new ShareDB({db}); + ``` ## Queries @@ -172,7 +170,7 @@ failed ops: - v4: collision 4 - v5: unique - v6: unique -... + ... - v1000: unique If I want to fetch ops v1-v3, then we: @@ -190,6 +188,32 @@ In the case where a valid op cannot be determined, we still fall back to fetching all ops and working backwards from the current version. +### Middlewares + +Middlewares let you hook into the `sharedb-mongo` pipeline for certain actions. They are distinct from [middleware in `ShareDB`](https://github.com/share/sharedb) as they are closer the concrete calls that are made to `MongoDB` itself. + +The original intent for middleware on `sharedb-mongo` is to support running in a sharded `MongoDB` cluster to satisfy the requirements on shard keys for versions 4.2 and greater of `MongoDB`. For more information see [the MongoDB docs](https://docs.mongodb.com/manual/core/sharding-shard-key/#shard-keys). + +`share.use(action, fn)` +Register a new middleware. + +- `action` _(String)_ + One of: + - `'beforeEdit'`: directly before the call to replace a document, can include edits as well as deletions + - `'beforeSnapshotLookup'`: directly before the call to issue a query for snapshots by ID +- `fn` _(Function(context, callback))_ + Call this function at the time specified by `action`. + - `context` will always have the following properties: + - `action`: The action this middleware is handling + - `collectionName`: The collection name being handled + - `options`: Original options as they were passed into the relevant function that triggered the action + - `'beforeEdit'` actions have additional context properties: + - `doc` - The document to be written + - `op` - The op that represents the changes that will be made to the document + - `query` - A filter that will be used to lookup the document that is about to be edited + - `'beforeSnapshotLookup'` actions have additional context properties: + - `query` - A filter that will be used to lookup the snapshot + ### Limitations #### Integrity @@ -226,26 +250,26 @@ Mongo errors are passed back directly. Additional error codes: #### 4100 -- Bad request - DB -* 4101 -- Invalid op version -* 4102 -- Invalid collection name -* 4103 -- $where queries disabled -* 4104 -- $mapReduce queries disabled -* 4105 -- $aggregate queries disabled -* 4106 -- $query property deprecated in queries -* 4107 -- Malformed query operator -* 4108 -- Only one collection operation allowed -* 4109 -- Only one cursor operation allowed -* 4110 -- Cursor methods can't run after collection method +- 4101 -- Invalid op version +- 4102 -- Invalid collection name +- 4103 -- $where queries disabled +- 4104 -- $mapReduce queries disabled +- 4105 -- $aggregate queries disabled +- 4106 -- $query property deprecated in queries +- 4107 -- Malformed query operator +- 4108 -- Only one collection operation allowed +- 4109 -- Only one cursor operation allowed +- 4110 -- Cursor methods can't run after collection method #### 5100 -- Internal error - DB -* 5101 -- Already closed -* 5102 -- Snapshot missing last operation field -* 5103 -- Missing ops from requested version -* 5104 -- Failed to parse query - +- 5101 -- Already closed +- 5102 -- Snapshot missing last operation field +- 5103 -- Missing ops from requested version +- 5104 -- Failed to parse query ## MIT License + Copyright (c) 2015 by Joseph Gentle and Nate Smith Permission is hereby granted, free of charge, to any person obtaining a copy diff --git a/src/actions.js b/src/actions.js index bac56ee2..39c2105d 100644 --- a/src/actions.js +++ b/src/actions.js @@ -1,6 +1,6 @@ module.exports = { - // Triggers before the call to replace a document is made to Mongo + // Triggers before the call to replace a document is made beforeEdit: 'beforeEdit', - // Triggers directly before the call to issue a query to Mongo for snapshots by ID + // Triggers directly before the call to issue a query for snapshots by ID beforeSnapshotLookup: 'beforeSnapshotLookup' }; From d4a552edd1305723fbd2d87e8f37d1d0178c4a5a Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 3 Feb 2021 16:45:23 -0500 Subject: [PATCH 16/27] Minor docs updates --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6ee8ceb5..08c1594c 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,8 @@ Middlewares let you hook into the `sharedb-mongo` pipeline for certain actions. The original intent for middleware on `sharedb-mongo` is to support running in a sharded `MongoDB` cluster to satisfy the requirements on shard keys for versions 4.2 and greater of `MongoDB`. For more information see [the MongoDB docs](https://docs.mongodb.com/manual/core/sharding-shard-key/#shard-keys). +#### Usage + `share.use(action, fn)` Register a new middleware. @@ -202,7 +204,7 @@ Register a new middleware. - `'beforeEdit'`: directly before the call to replace a document, can include edits as well as deletions - `'beforeSnapshotLookup'`: directly before the call to issue a query for snapshots by ID - `fn` _(Function(context, callback))_ - Call this function at the time specified by `action`. + Call this function at the time specified by `action` - `context` will always have the following properties: - `action`: The action this middleware is handling - `collectionName`: The collection name being handled From 12418b08a06fd2693959967ae92e65e0bf415a55 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 09:29:54 -0500 Subject: [PATCH 17/27] Update README.md Co-authored-by: Alec Gibson <12036746+alecgibson@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 08c1594c..847fe4be 100644 --- a/README.md +++ b/README.md @@ -190,7 +190,7 @@ current version. ### Middlewares -Middlewares let you hook into the `sharedb-mongo` pipeline for certain actions. They are distinct from [middleware in `ShareDB`](https://github.com/share/sharedb) as they are closer the concrete calls that are made to `MongoDB` itself. +Middlewares let you hook into the `sharedb-mongo` pipeline for certain actions. They are distinct from [middleware in `ShareDB`](https://github.com/share/sharedb) as they are closer to the concrete calls that are made to `MongoDB` itself. The original intent for middleware on `sharedb-mongo` is to support running in a sharded `MongoDB` cluster to satisfy the requirements on shard keys for versions 4.2 and greater of `MongoDB`. For more information see [the MongoDB docs](https://docs.mongodb.com/manual/core/sharding-shard-key/#shard-keys). From 3ed9f54e678cc0e10d552167137f36e22068129e Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 09:52:31 -0500 Subject: [PATCH 18/27] Update middleware handler to use OOP approach instead of mixin --- index.js | 19 +++++++++---- src/middleware.js | 57 -------------------------------------- src/middlewareHandler.js | 60 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 63 deletions(-) delete mode 100644 src/middleware.js create mode 100644 src/middlewareHandler.js diff --git a/index.js b/index.js index 6225fd4e..68a4d46d 100644 --- a/index.js +++ b/index.js @@ -2,6 +2,7 @@ var async = require('async'); var mongodb = require('mongodb'); var DB = require('sharedb').DB; var OpLinkValidator = require('./op-link-validator'); +var MiddlewareHandler = require('./src/middlewareHandler'); module.exports = ShareDbMongo; @@ -67,7 +68,7 @@ function ShareDbMongo(mongo, options) { throw new Error('deprecated: pass mongo as url string or function with callback'); } - this.middleware = {}; + this._middleware = new MiddlewareHandler(); }; ShareDbMongo.prototype = Object.create(DB.prototype); @@ -233,7 +234,7 @@ ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, optio }; function createRequestForMiddleware(options, collectionName, op) { - // Create a new request object which will be passed to helper functions and middleware + // Create a new request object which will be passed to helper functions and _middleware var request = { options: options, collectionName: collectionName @@ -282,7 +283,7 @@ ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, ca }); } else { request.query = {_id: id, _v: request.doc._v - 1}; - self.trigger(self.MIDDLEWARE_ACTIONS.beforeEdit, request, function(middlewareErr) { + self._middleware.trigger(MiddlewareHandler.Actions.beforeEdit, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } @@ -307,7 +308,7 @@ ShareDbMongo.prototype.getSnapshot = function(collectionName, id, fields, option var projection = getProjection(fields, options); var request = createRequestForMiddleware(options, collectionName); request.query = query; - self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { + self._middleware.trigger(MiddlewareHandler.Actions.beforeSnapshotLookup, request, function(middlewareErr) { if (middlewareErr) return callback(middlewareErr); collection.find(request.query).limit(1).project(projection).next(function(err, doc) { @@ -327,7 +328,7 @@ ShareDbMongo.prototype.getSnapshotBulk = function(collectionName, ids, fields, o var projection = getProjection(fields, options); var request = createRequestForMiddleware(options, collectionName); request.query = query; - self.trigger(self.MIDDLEWARE_ACTIONS.beforeSnapshotLookup, request, function(middlewareErr) { + self._middleware.trigger(MiddlewareHandler.Actions.beforeSnapshotLookup, request, function(middlewareErr) { if (middlewareErr) return callback(middlewareErr); collection.find(request.query).project(projection).toArray(function(err, docs) { @@ -1611,4 +1612,10 @@ ShareDbMongo.parseQueryError = function(err) { return err; }; -require('./src/middleware')(ShareDbMongo); +// Middleware + +ShareDbMongo.prototype.use = function(action, fn) { + this._middleware.use(action, fn); +}; + +ShareDbMongo.MiddlewareActions = MiddlewareHandler.Actions; diff --git a/src/middleware.js b/src/middleware.js deleted file mode 100644 index 80a6efac..00000000 --- a/src/middleware.js +++ /dev/null @@ -1,57 +0,0 @@ -module.exports = extendWithMiddleware; -var MIDDLEWARE_ACTIONS = require('./actions'); - -function extendWithMiddleware(ShareDbMongo) { - /** - * Add middleware to an action or array of actions - * - * @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') - * @param fn The function to call when this middleware is triggered - * The fn receives a request object with information on the triggered action (e.g. the snapshot to write) - * and a next function to call once the middleware is complete - * - * NOTE: It is recommended not to add async or long running tasks to the sharedb-mongo middleware as it will - * be called very frequently during sensitive operations. It may have a significant performance impact. - */ - ShareDbMongo.prototype.use = function(action, fn) { - if (Array.isArray(action)) { - for (var i = 0; i < action.length; i++) { - this.use(action[i], fn); - } - return this; - } - var fns = this.middleware[action] || (this.middleware[action] = []); - fns.push(fn); - return this; - }; - - /** - * Passes request through the middleware stack - * - * Middleware may modify the request object. After all middleware have been - * invoked we call `callback` with `null` and the modified request. If one of - * the middleware resturns an error the callback is called with that error. - * - * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') - * @param request Request details such as the snapshot to write, depends on the triggered action - * @param callback Function to call once the middleware has been processed. - */ - ShareDbMongo.prototype.trigger = function(action, request, callback) { - request.action = action; - - var fns = this.middleware[action]; - if (!fns) return callback(); - - // Copying the triggers we'll fire so they don't get edited while we iterate. - fns = fns.slice(); - var next = function(err) { - if (err) return callback(err); - var fn = fns.shift(); - if (!fn) return callback(); - fn(request, next); - }; - next(); - }; - - ShareDbMongo.prototype.MIDDLEWARE_ACTIONS = MIDDLEWARE_ACTIONS; -}; diff --git a/src/middlewareHandler.js b/src/middlewareHandler.js new file mode 100644 index 00000000..f822c115 --- /dev/null +++ b/src/middlewareHandler.js @@ -0,0 +1,60 @@ +var MIDDLEWARE_ACTIONS = require('./actions'); + +function MiddlewareHandler() { + this._middleware = {}; +} + +/** +* Add middleware to an action or array of actions +* +* @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') +* @param fn The function to call when this middleware is triggered +* The fn receives a request object with information on the triggered action (e.g. the snapshot to write) +* and a next function to call once the middleware is complete +* +* NOTE: It is recommended not to add async or long running tasks to the sharedb-mongo middleware as it will +* be called very frequently during sensitive operations. It may have a significant performance impact. +*/ +MiddlewareHandler.prototype.use = function(action, fn) { + if (Array.isArray(action)) { + for (var i = 0; i < action.length; i++) { + this.use(action[i], fn); + } + return this; + } + var fns = this._middleware[action] || (this._middleware[action] = []); + fns.push(fn); + return this; +}; + +/** + * Passes request through the middleware stack + * + * Middleware may modify the request object. After all middleware have been + * invoked we call `callback` with `null` and the modified request. If one of + * the middleware resturns an error the callback is called with that error. + * + * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') + * @param request Request details such as the snapshot to write, depends on the triggered action + * @param callback Function to call once the middleware has been processed. + */ +MiddlewareHandler.prototype.trigger = function(action, request, callback) { + request.action = action; + + var fns = this._middleware[action]; + if (!fns) return callback(); + + // Copying the triggers we'll fire so they don't get edited while we iterate. + fns = fns.slice(); + var next = function(err) { + if (err) return callback(err); + var fn = fns.shift(); + if (!fn) return callback(); + fn(request, next); + }; + next(); +}; + +MiddlewareHandler.Actions = MIDDLEWARE_ACTIONS; + +module.exports = MiddlewareHandler; From 5ec4873fe81b3c9090424b5720c53874dc8e8d11 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 09:53:45 -0500 Subject: [PATCH 19/27] Update to move middleware to subpath in src --- index.js | 2 +- src/{ => middleware}/actions.js | 0 src/{ => middleware}/middlewareHandler.js | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename src/{ => middleware}/actions.js (100%) rename src/{ => middleware}/middlewareHandler.js (100%) diff --git a/index.js b/index.js index 68a4d46d..4609f8b0 100644 --- a/index.js +++ b/index.js @@ -2,7 +2,7 @@ var async = require('async'); var mongodb = require('mongodb'); var DB = require('sharedb').DB; var OpLinkValidator = require('./op-link-validator'); -var MiddlewareHandler = require('./src/middlewareHandler'); +var MiddlewareHandler = require('./src/middleware/middlewareHandler'); module.exports = ShareDbMongo; diff --git a/src/actions.js b/src/middleware/actions.js similarity index 100% rename from src/actions.js rename to src/middleware/actions.js diff --git a/src/middlewareHandler.js b/src/middleware/middlewareHandler.js similarity index 100% rename from src/middlewareHandler.js rename to src/middleware/middlewareHandler.js From 6291841c22fb0acd42a685089853a50914ea57ca Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 09:58:08 -0500 Subject: [PATCH 20/27] Don't save db on the test context --- test/test_mongo_middleware.js | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index e44301b8..1e719db7 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -16,23 +16,22 @@ function create(callback) { }; describe('mongo db middleware', function() { + var db; + beforeEach(function(done) { - var self = this; - create(function(err, db, mongo) { + create(function(err, createdDb, createdMongo) { if (err) return done(err); - self.db = db; - self.mongo = mongo; + db = createdDb; done(); }); }); afterEach(function(done) { - this.db.close(done); + db.close(done); }); describe('beforeEdit', function() { it('has the expected properties on the request object', function(done) { - var db = this.db; db.use('beforeEdit', function(request, next) { expect(request).to.have.all.keys([ 'action', @@ -65,7 +64,6 @@ describe('mongo db middleware', function() { }); it('should augment query filter and write to the document when commit is called', function(done) { - var db = this.db; // Augment the query. The original query looks up the document by id, wheras this middleware // changes it to use the `foo` property. The end result still returns the same document. The next // middleware ensures we attached it to the request. @@ -92,11 +90,11 @@ describe('mongo db middleware', function() { db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { if (err) return done(err); - expectDocumentToContainFoo(db, 'bar', function() { + expectDocumentToContainFoo('bar', function() { db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { if (err) return done(err); // Ensure the value is updated as expected - expectDocumentToContainFoo(db, 'baz', done); + expectDocumentToContainFoo('baz', done); }); }); }); @@ -104,7 +102,6 @@ describe('mongo db middleware', function() { }); it('should augment the written document when commit is called', function(done) { - var db = this.db; // Change the written value of foo to be `fuzz` db.use('beforeEdit', function(request, next) { request.doc.foo = 'fuzz'; @@ -119,11 +116,11 @@ describe('mongo db middleware', function() { db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, function(err) { if (err) return done(err); - expectDocumentToContainFoo(db, 'bar', function() { + expectDocumentToContainFoo('bar', function() { db.commit('testcollection', snapshot.id, editOp, newSnapshot, null, function(err) { if (err) return done(err); // Ensure the value is updated as expected - expectDocumentToContainFoo(db, 'fuzz', done); + expectDocumentToContainFoo('fuzz', done); }); }); }); @@ -131,7 +128,6 @@ describe('mongo db middleware', function() { describe('beforeSnapshotLookup', function() { it('has the expected properties on the request object before getting a single snapshot', function(done) { - var db = this.db; db.use('beforeSnapshotLookup', function(request, next) { expect(request).to.have.all.keys([ 'action', @@ -158,7 +154,6 @@ describe('mongo db middleware', function() { }); it('has the expected properties on the request object before getting bulk snapshots', function(done) { - var db = this.db; db.use('beforeSnapshotLookup', function(request, next) { expect(request).to.have.all.keys([ 'action', @@ -185,8 +180,6 @@ describe('mongo db middleware', function() { }); it('should augment the query when getSnapshot is called', function(done) { - var db = this.db; - var snapshots = [ {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}, {type: 'json0', id: 'test2', v: 1, data: {foo: 'baz'}} @@ -220,8 +213,6 @@ describe('mongo db middleware', function() { }); it('should augment the query when getSnapshotBulk is called', function(done) { - var db = this.db; - var snapshots = [ {type: 'json0', id: 'test1', v: 1, data: {foo: 'bar'}}, {type: 'json0', id: 'test2', v: 1, data: {foo: 'baz'}} @@ -253,7 +244,7 @@ describe('mongo db middleware', function() { }); }); - function expectDocumentToContainFoo(db, valueOfFoo, cb) { + function expectDocumentToContainFoo(valueOfFoo, cb) { var query = {_id: 'test1'}; db.query('testcollection', query, null, null, function(err, results) { From 3a53f562d50c72cb59e616bf22785105c60a9426 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 10:15:01 -0500 Subject: [PATCH 21/27] Add error handling for the `use` function --- src/middleware/middlewareHandler.js | 6 ++++++ test/test_mongo_middleware.js | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/middleware/middlewareHandler.js b/src/middleware/middlewareHandler.js index f822c115..fec63cda 100644 --- a/src/middleware/middlewareHandler.js +++ b/src/middleware/middlewareHandler.js @@ -22,6 +22,12 @@ MiddlewareHandler.prototype.use = function(action, fn) { } return this; } + if (!action) throw new Error('Expected action to be defined'); + if (!fn) throw new Error('Expected fn to be defined'); + if (!Object.values(MIDDLEWARE_ACTIONS).includes(action)) { + throw new Error('Unrecognized action name ' + action); + } + var fns = this._middleware[action] || (this._middleware[action] = []); fns.push(fn); return this; diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 1e719db7..884f01da 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -30,6 +30,33 @@ describe('mongo db middleware', function() { db.close(done); }); + describe('error handling', function() { + it('throws error when no action is given', function() { + function invalidAction() { + db.use(null, function(request, next) { + next(); + }); + } + expect(invalidAction).to.throw(); + }); + + it('throws error when no handler is given', function() { + function invalidAction() { + db.use('someAction'); + } + expect(invalidAction).to.throw(); + }); + + it('throws error on unrecognized action name', function() { + function invalidAction() { + db.use('someAction', function(request, next) { + next(); + }); + } + expect(invalidAction).to.throw(); + }); + }); + describe('beforeEdit', function() { it('has the expected properties on the request object', function(done) { db.use('beforeEdit', function(request, next) { From 825d1fdc18eb68ec5718464d307b871cab8fa3d4 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 10:25:57 -0500 Subject: [PATCH 22/27] Minor refactor on variable names and linting issues --- test/test_mongo_middleware.js | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 884f01da..1fb3d6eb 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -3,6 +3,8 @@ var expect = require('chai').expect; var ShareDbMongo = require('..'); var mongoUrl = process.env.TEST_MONGO_URL || 'mongodb://localhost:27017/test'; +var BEFORE_EDIT = ShareDbMongo.MiddlewareActions.beforeEdit; +var BEFORE_SNAPSHOT_LOOKUP = ShareDbMongo.MiddlewareActions.beforeSnapshotLookup; function create(callback) { var db = new ShareDbMongo(mongoUrl); @@ -19,7 +21,7 @@ describe('mongo db middleware', function() { var db; beforeEach(function(done) { - create(function(err, createdDb, createdMongo) { + create(function(err, createdDb) { if (err) return done(err); db = createdDb; done(); @@ -33,7 +35,7 @@ describe('mongo db middleware', function() { describe('error handling', function() { it('throws error when no action is given', function() { function invalidAction() { - db.use(null, function(request, next) { + db.use(null, function(_request, next) { next(); }); } @@ -49,7 +51,7 @@ describe('mongo db middleware', function() { it('throws error on unrecognized action name', function() { function invalidAction() { - db.use('someAction', function(request, next) { + db.use('someAction', function(_request, next) { next(); }); } @@ -57,9 +59,9 @@ describe('mongo db middleware', function() { }); }); - describe('beforeEdit', function() { + describe(BEFORE_EDIT, function() { it('has the expected properties on the request object', function(done) { - db.use('beforeEdit', function(request, next) { + db.use(BEFORE_EDIT, function(request, next) { expect(request).to.have.all.keys([ 'action', 'collectionName', @@ -68,7 +70,7 @@ describe('mongo db middleware', function() { 'options', 'query' ]); - expect(request.action).to.equal('beforeEdit'); + expect(request.action).to.equal(BEFORE_EDIT); expect(request.collectionName).to.equal('testcollection'); expect(request.doc.foo).to.equal('fuzz'); expect(request.op.op).to.exist; @@ -96,13 +98,13 @@ describe('mongo db middleware', function() { // middleware ensures we attached it to the request. // We can't truly change which document is returned from the query because MongoDB will not allow // the immutable fields such as `_id` to be changed. - db.use('beforeEdit', function(request, next) { + db.use(BEFORE_EDIT, function(request, next) { request.query.foo = 'bar'; next(); }); // Attach this middleware to check that the original one is passing the context // correctly. Commit will be called after this. - db.use('beforeEdit', function(request, next) { + db.use(BEFORE_EDIT, function(request, next) { expect(request.query).to.deep.equal({ _id: 'test1', _v: 1, @@ -130,7 +132,7 @@ describe('mongo db middleware', function() { it('should augment the written document when commit is called', function(done) { // Change the written value of foo to be `fuzz` - db.use('beforeEdit', function(request, next) { + db.use(BEFORE_EDIT, function(request, next) { request.doc.foo = 'fuzz'; next(); }); @@ -153,16 +155,16 @@ describe('mongo db middleware', function() { }); }); - describe('beforeSnapshotLookup', function() { + describe(BEFORE_SNAPSHOT_LOOKUP, function() { it('has the expected properties on the request object before getting a single snapshot', function(done) { - db.use('beforeSnapshotLookup', function(request, next) { + db.use(BEFORE_SNAPSHOT_LOOKUP, function(request, next) { expect(request).to.have.all.keys([ 'action', 'collectionName', 'options', 'query' ]); - expect(request.action).to.equal('beforeSnapshotLookup'); + expect(request.action).to.equal(BEFORE_SNAPSHOT_LOOKUP); expect(request.collectionName).to.equal('testcollection'); expect(request.options.testOptions).to.equal('yes'); expect(request.query._id).to.equal('test1'); @@ -181,14 +183,14 @@ describe('mongo db middleware', function() { }); it('has the expected properties on the request object before getting bulk snapshots', function(done) { - db.use('beforeSnapshotLookup', function(request, next) { + db.use(BEFORE_SNAPSHOT_LOOKUP, function(request, next) { expect(request).to.have.all.keys([ 'action', 'collectionName', 'options', 'query' ]); - expect(request.action).to.equal('beforeSnapshotLookup'); + expect(request.action).to.equal(BEFORE_SNAPSHOT_LOOKUP); expect(request.collectionName).to.equal('testcollection'); expect(request.options.testOptions).to.equal('yes'); expect(request.query._id).to.deep.equal({$in: ['test1']}); @@ -223,7 +225,7 @@ describe('mongo db middleware', function() { }); // Change the query to look for baz and not bar - db.use('beforeSnapshotLookup', function(request, next) { + db.use(BEFORE_SNAPSHOT_LOOKUP, function(request, next) { request.query = {_id: 'test2'}; next(); }); @@ -255,7 +257,7 @@ describe('mongo db middleware', function() { expect(docs.test2.data.foo).to.equal('baz'); // Change the query to look for baz and not bar - db.use('beforeSnapshotLookup', function(request, next) { + db.use(BEFORE_SNAPSHOT_LOOKUP, function(request, next) { request.query = {_id: {$in: ['test2']}}; next(); }); From bdb11cc82e85d37069338f2ccb70d3c9f14ce772 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 10:31:49 -0500 Subject: [PATCH 23/27] Rename doc -> documentToWrite --- README.md | 2 +- index.js | 10 +++++----- test/test_mongo_middleware.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 847fe4be..d5278d0f 100644 --- a/README.md +++ b/README.md @@ -210,7 +210,7 @@ Register a new middleware. - `collectionName`: The collection name being handled - `options`: Original options as they were passed into the relevant function that triggered the action - `'beforeEdit'` actions have additional context properties: - - `doc` - The document to be written + - `documentToWrite` - The document to be written - `op` - The op that represents the changes that will be made to the document - `query` - A filter that will be used to lookup the document that is about to be edited - `'beforeSnapshotLookup'` actions have additional context properties: diff --git a/index.js b/index.js index 4609f8b0..6a0b47c7 100644 --- a/index.js +++ b/index.js @@ -268,9 +268,9 @@ ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, ca var self = this; this.getCollection(request.collectionName, function(err, collection) { if (err) return callback(err); - request.doc = castToDoc(id, snapshot, opId); - if (request.doc._v === 1) { - collection.insertOne(request.doc, function(err) { + request.documentToWrite = castToDoc(id, snapshot, opId); + if (request.documentToWrite._v === 1) { + collection.insertOne(request.documentToWrite, function(err) { if (err) { // Return non-success instead of duplicate key error, since this is // expected to occur during simultaneous creates on the same id @@ -282,12 +282,12 @@ ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, ca callback(null, true); }); } else { - request.query = {_id: id, _v: request.doc._v - 1}; + request.query = {_id: id, _v: request.documentToWrite._v - 1}; self._middleware.trigger(MiddlewareHandler.Actions.beforeEdit, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } - collection.replaceOne(request.query, request.doc, function(err, result) { + collection.replaceOne(request.query, request.documentToWrite, function(err, result) { if (err) return callback(err); var succeeded = !!result.modifiedCount; callback(null, succeeded); diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 1fb3d6eb..10037e0f 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -65,14 +65,14 @@ describe('mongo db middleware', function() { expect(request).to.have.all.keys([ 'action', 'collectionName', - 'doc', + 'documentToWrite', 'op', 'options', 'query' ]); expect(request.action).to.equal(BEFORE_EDIT); expect(request.collectionName).to.equal('testcollection'); - expect(request.doc.foo).to.equal('fuzz'); + expect(request.documentToWrite.foo).to.equal('fuzz'); expect(request.op.op).to.exist; expect(request.options.testOptions).to.equal('yes'); expect(request.query._id).to.equal('test1'); @@ -133,7 +133,7 @@ describe('mongo db middleware', function() { it('should augment the written document when commit is called', function(done) { // Change the written value of foo to be `fuzz` db.use(BEFORE_EDIT, function(request, next) { - request.doc.foo = 'fuzz'; + request.documentToWrite.foo = 'fuzz'; next(); }); From 3d69a062b3fb012fb760f1435ca2658425a7506b Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Fri, 5 Feb 2021 10:35:48 -0500 Subject: [PATCH 24/27] Update index.js --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 6a0b47c7..cb95ca70 100644 --- a/index.js +++ b/index.js @@ -234,7 +234,7 @@ ShareDbMongo.prototype.commit = function(collectionName, id, op, snapshot, optio }; function createRequestForMiddleware(options, collectionName, op) { - // Create a new request object which will be passed to helper functions and _middleware + // Create a new request object which will be passed to helper functions and middleware var request = { options: options, collectionName: collectionName From d81dfda77a920f470154a273a171ff7fb7d67466 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 10 Feb 2021 12:15:27 -0500 Subject: [PATCH 25/27] Rename beforeEdit -> beforeOverwrite --- README.md | 4 ++-- index.js | 2 +- src/middleware/actions.js | 5 +++-- src/middleware/middlewareHandler.js | 4 ++-- test/test_mongo_middleware.js | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d5278d0f..50bb0772 100644 --- a/README.md +++ b/README.md @@ -201,7 +201,7 @@ Register a new middleware. - `action` _(String)_ One of: - - `'beforeEdit'`: directly before the call to replace a document, can include edits as well as deletions + - `'beforeOverwrite'`: directly before the call to replace a document, can include edits as well as deletions - `'beforeSnapshotLookup'`: directly before the call to issue a query for snapshots by ID - `fn` _(Function(context, callback))_ Call this function at the time specified by `action` @@ -209,7 +209,7 @@ Register a new middleware. - `action`: The action this middleware is handling - `collectionName`: The collection name being handled - `options`: Original options as they were passed into the relevant function that triggered the action - - `'beforeEdit'` actions have additional context properties: + - `'beforeOverwrite'` actions have additional context properties: - `documentToWrite` - The document to be written - `op` - The op that represents the changes that will be made to the document - `query` - A filter that will be used to lookup the document that is about to be edited diff --git a/index.js b/index.js index 6a0b47c7..edcfcdfa 100644 --- a/index.js +++ b/index.js @@ -283,7 +283,7 @@ ShareDbMongo.prototype._writeSnapshot = function(request, id, snapshot, opId, ca }); } else { request.query = {_id: id, _v: request.documentToWrite._v - 1}; - self._middleware.trigger(MiddlewareHandler.Actions.beforeEdit, request, function(middlewareErr) { + self._middleware.trigger(MiddlewareHandler.Actions.beforeOverwrite, request, function(middlewareErr) { if (middlewareErr) { return callback(middlewareErr); } diff --git a/src/middleware/actions.js b/src/middleware/actions.js index 39c2105d..3e33c646 100644 --- a/src/middleware/actions.js +++ b/src/middleware/actions.js @@ -1,6 +1,7 @@ module.exports = { // Triggers before the call to replace a document is made - beforeEdit: 'beforeEdit', - // Triggers directly before the call to issue a query for snapshots by ID + beforeOverwrite: 'beforeOverwrite', + // Triggers directly before the call to issue a query for snapshots + // Applies for both a single lookup by ID and bulk lookups by a list of IDs beforeSnapshotLookup: 'beforeSnapshotLookup' }; diff --git a/src/middleware/middlewareHandler.js b/src/middleware/middlewareHandler.js index fec63cda..18e3aa97 100644 --- a/src/middleware/middlewareHandler.js +++ b/src/middleware/middlewareHandler.js @@ -7,7 +7,7 @@ function MiddlewareHandler() { /** * Add middleware to an action or array of actions * -* @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') +* @param action The action to use from MIDDLEWARE_ACTIONS (e.g. 'beforeOverwrite') * @param fn The function to call when this middleware is triggered * The fn receives a request object with information on the triggered action (e.g. the snapshot to write) * and a next function to call once the middleware is complete @@ -40,7 +40,7 @@ MiddlewareHandler.prototype.use = function(action, fn) { * invoked we call `callback` with `null` and the modified request. If one of * the middleware resturns an error the callback is called with that error. * - * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeEdit') + * @param action The action to trigger from MIDDLEWARE_ACTIONS (e.g. 'beforeOverwrite') * @param request Request details such as the snapshot to write, depends on the triggered action * @param callback Function to call once the middleware has been processed. */ diff --git a/test/test_mongo_middleware.js b/test/test_mongo_middleware.js index 10037e0f..50b624fa 100644 --- a/test/test_mongo_middleware.js +++ b/test/test_mongo_middleware.js @@ -3,7 +3,7 @@ var expect = require('chai').expect; var ShareDbMongo = require('..'); var mongoUrl = process.env.TEST_MONGO_URL || 'mongodb://localhost:27017/test'; -var BEFORE_EDIT = ShareDbMongo.MiddlewareActions.beforeEdit; +var BEFORE_EDIT = ShareDbMongo.MiddlewareActions.beforeOverwrite; var BEFORE_SNAPSHOT_LOOKUP = ShareDbMongo.MiddlewareActions.beforeSnapshotLookup; function create(callback) { From 546fead8c62ad2df70ce5450f9939fd56f8787c1 Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 10 Feb 2021 12:21:55 -0500 Subject: [PATCH 26/27] Apply suggestions from code review --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 50bb0772..4afa551b 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ Register a new middleware. - `action` _(String)_ One of: - `'beforeOverwrite'`: directly before the call to replace a document, can include edits as well as deletions - - `'beforeSnapshotLookup'`: directly before the call to issue a query for snapshots by ID + - `'beforeSnapshotLookup'`: directly before the call to issue a query for one or more snapshots by ID - `fn` _(Function(context, callback))_ Call this function at the time specified by `action` - `context` will always have the following properties: @@ -214,7 +214,7 @@ Register a new middleware. - `op` - The op that represents the changes that will be made to the document - `query` - A filter that will be used to lookup the document that is about to be edited - `'beforeSnapshotLookup'` actions have additional context properties: - - `query` - A filter that will be used to lookup the snapshot + - `query` - A filter that will be used to lookup the snapshot. When a single snapshot is looked up the query will take the shape `{_id: docId}` while a bulk lookup by a list of IDs will resemble `{_id: {$in: docIdsArray}}`. ### Limitations From d82a7dca1c2f1e91f07bc0b50fb2bced0406fa6c Mon Sep 17 00:00:00 2001 From: Christina Burger Date: Wed, 10 Feb 2021 12:26:31 -0500 Subject: [PATCH 27/27] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4afa551b..2c66759a 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,7 @@ Register a new middleware. - `'beforeOverwrite'` actions have additional context properties: - `documentToWrite` - The document to be written - `op` - The op that represents the changes that will be made to the document - - `query` - A filter that will be used to lookup the document that is about to be edited + - `query` - A filter that will be used to lookup the document that is about to be edited, which should always include an ID and snapshot version e.g. `{_id: 'uuid', _v: 1}` - `'beforeSnapshotLookup'` actions have additional context properties: - `query` - A filter that will be used to lookup the snapshot. When a single snapshot is looked up the query will take the shape `{_id: docId}` while a bulk lookup by a list of IDs will resemble `{_id: {$in: docIdsArray}}`.