From 3b639064fa11b1dc4ce3714b3a4fc1dad46b576e Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 15:02:49 -0800 Subject: [PATCH 01/10] Add failing tests for _id and : null queries --- test/query.js | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/test/query.js b/test/query.js index c3d5fe5..f9fa4c9 100644 --- a/test/query.js +++ b/test/query.js @@ -50,7 +50,51 @@ module.exports = function() { }); }); - describe('top-level boolean operator', function(done) { + it('filters with top-level _id condition', function(done) { + var snapshots = [ + {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1"}, + {type: 'json0', v: 1, data: {x: 1, y: 2}, id: "test2"}, + {type: 'json0', v: 1, data: {x: 2, y: 2}, id: "test3"} + ]; + var query = {_id: {$in: ['test1', 'test3']}}; + + var db = this.db; + 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.query('testcollection', query, null, null, function(err, results, extra) { + if (err) throw err; + expect(results).eql([snapshots[0], snapshots[2]]); + done(); + }); + }); + }); + + it('filters with null condition', function(done) { + var snapshots = [ + {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1"}, + {type: 'json0', v: 1, data: {x: 1}, id: "test2"}, // y value intentionally omitted + {type: 'json0', v: 1, data: {x: 2, y: 2}, id: "test3"} + ]; + var query = {y: null}; + + var db = this.db; + 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.query('testcollection', query, null, null, function(err, results, extra) { + if (err) throw err; + expect(results).eql([snapshots[1]]); + done(); + }); + }); + }); + + describe('top-level boolean operator', function() { var snapshots = [ {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1"}, {type: 'json0', v: 1, data: {x: 1, y: 2}, id: "test2"}, From 63590e455b10705119f34f9d2d6cdfc7358236fb Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 15:03:43 -0800 Subject: [PATCH 02/10] Upgrade `mingo` dependency --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d6eb82a..e7eff2b 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ }, "homepage": "https://github.com/share/sharedb-mingo-memory#readme", "dependencies": { - "mingo": "^0.6.3", + "mingo": "^2.2.0", "sharedb": "^1.0.0-beta" } } From 7566b43cc8bd88c51866e69b1a2485c42264b357 Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 16:20:52 -0800 Subject: [PATCH 03/10] Better test cases for doc query conditions --- test/query.js | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/test/query.js b/test/query.js index f9fa4c9..6affcbe 100644 --- a/test/query.js +++ b/test/query.js @@ -50,23 +50,40 @@ module.exports = function() { }); }); - it('filters with top-level _id condition', function(done) { + describe('top-level Mongo doc properties', function() { var snapshots = [ {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1"}, {type: 'json0', v: 1, data: {x: 1, y: 2}, id: "test2"}, {type: 'json0', v: 1, data: {x: 2, y: 2}, id: "test3"} ]; - var query = {_id: {$in: ['test1', 'test3']}}; - var db = this.db; - async.each(snapshots, function(snapshot, cb) { - db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, cb); - }, function(err) { - if (err) return done(err); + beforeEach(function(done) { + var db = this.db; + async.each(snapshots, function(snapshot, cb) { + db.commit('testcollection', snapshot.id, {v: 0, create: {}}, snapshot, null, cb); + }, done); + }); - db.query('testcollection', query, null, null, function(err, results, extra) { + it('matches value', function(done) { + this.db.query('testcollection', {_id: 'test1'}, null, null, function(err, results, extra) { if (err) throw err; - expect(results).eql([snapshots[0], snapshots[2]]); + expect(results).eql([snapshots[0]]); + done(); + }); + }); + + it('combines with data conditions', function(done) { + this.db.query('testcollection', {y: 2, _id: {$nin: ['test2']}}, null, null, function(err, results, extra) { + if (err) throw err; + expect(results).eql([snapshots[2]]); + done(); + }); + }); + + it('top-level boolean operator', function(done) { + this.db.query('testcollection', {$or: [{y: 1}, {_id: 'test2'}]}, null, null, function(err, results, extra) { + if (err) throw err; + expect(results).eql([snapshots[0], snapshots[1]]); done(); }); }); From 7825d2a27c34855a5d7392b60e5cdb4b8a2e5ad5 Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 16:22:10 -0800 Subject: [PATCH 04/10] Cast Mongo doc queries to Mingo snapshot queries --- index.js | 47 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index f7b2f7d..40cb468 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,14 @@ var Mingo = require('mingo'); +// Snapshot properties added to the root doc by `castToDoc()` in sharedb-mongo +var MONGO_DOC_PROPERTIES = { + '_id': 'id', + '_v': 'v', + '_type': 'type', + '_m': 'm', + '_o': 'o' +}; + function extendMemoryDB(MemoryDB) { function ShareDBMingo(options) { if (!(this instanceof ShareDBMingo)) return new ShareDBMingo(options); @@ -10,8 +19,11 @@ function extendMemoryDB(MemoryDB) { ShareDBMingo.prototype._querySync = function(snapshots, query, options) { var parsed = parseQuery(query); + var mingoQuery = new Mingo.Query(castToSnapshotQuery(parsed.query)); - var filtered = filter(snapshots, parsed.query); + var filtered = snapshots.filter(function(snapshot) { + return mingoQuery.test(snapshot); + }); if (parsed.sort) sort(filtered, parsed.sort); if (parsed.skip) filtered.splice(0, parsed.skip); if (parsed.limit) filtered = filtered.slice(0, parsed.limit); @@ -23,11 +35,11 @@ function extendMemoryDB(MemoryDB) { }; ShareDBMingo.prototype.queryPollDoc = function(collection, id, query, options, callback) { - var mingoQuery = new Mingo.Query(query); + var mingoQuery = new Mingo.Query(castToSnapshotQuery(query)); this.getSnapshot(collection, id, null, null, function(err, snapshot) { if (err) return callback(err); if (snapshot.data) { - callback(null, mingoQuery.test(snapshot.data)); + callback(null, mingoQuery.test(snapshot)); } else { callback(null, false); } @@ -71,12 +83,29 @@ function extendMemoryDB(MemoryDB) { }; } - // Support exact key match filters only - function filter(snapshots, query) { - var mingoQuery = new Mingo.Query(query); - return snapshots.filter(function(snapshot) { - return snapshot.data && mingoQuery.test(snapshot.data); - }); + // Build a query object that mimics how the query would be executed if it were + // made against snapshots persisted with `sharedb-mongo` + // FIXME: This doesn't handle nested doc properties with dots like: {'_m.mtime': 12300} + function castToSnapshotQuery(query) { + var snapshotQuery = {}; + for (var property in query) { + // Mongo doc property + if (MONGO_DOC_PROPERTIES[property]) { + snapshotQuery[MONGO_DOC_PROPERTIES[property]] = query[property]; + + // top-level boolean operator + } else if (property[0] === '$' && Array.isArray(query[property])) { + snapshotQuery[property] = []; + for (var i = 0; i < query[property].length; i++) { + snapshotQuery[property].push(castToSnapshotQuery(query[property][i])); + } + + // nested `data` document + } else { + snapshotQuery["data." + property] = query[property]; + } + } + return snapshotQuery; } // Support sorting with the Mongo $orderby syntax From 288c8606091c7e9fa1dc6ac0c9fc396c283c8bef Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 16:33:34 -0800 Subject: [PATCH 05/10] Update Node for CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a95b4a5..bec4f9d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: node_js node_js: + - 8 - 5 - 4 - - 0.10 script: "npm run test-cover" # Send coverage data to Coveralls after_script: "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js" From 6d000068677f5b6ce96449b56877ac7a0ddeecc7 Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Mon, 19 Feb 2018 22:44:20 -0800 Subject: [PATCH 06/10] Add basic support for ShareDB projections --- index.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/index.js b/index.js index 40cb468..f7c5b51 100644 --- a/index.js +++ b/index.js @@ -17,6 +17,24 @@ function extendMemoryDB(MemoryDB) { ShareDBMingo.prototype = Object.create(MemoryDB.prototype); + ShareDBMingo.prototype.projectsSnapshots = true; + + // HACK: Wrap MemoryDB functions to project snapshots before returning them. + // This would be cleaner if implemented directly in MemoryDB, or it would be + // more aligned with a drop-in sharedb-mongo replacement if implemented as + // mingo projections. + ShareDBMingo.prototype.getSnapshot = function(collection, id, fields, options, callback) { + MemoryDB.prototype.getSnapshot.call(this, collection, id, fields, options, function (err, snapshot) { + callback(err, projectSnapshot(fields, snapshot)); + }); + } + ShareDBMingo.prototype.query = function(collection, query, fields, options, callback) { + MemoryDB.prototype.query.call(this, collection, query, fields, options, function (err, snapshots, extra) { + var projectSnapshotWithFields = projectSnapshot.bind(null, fields); + callback(err, snapshots.map(projectSnapshotWithFields), extra); + }); + } + ShareDBMingo.prototype._querySync = function(snapshots, query, options) { var parsed = parseQuery(query); var mingoQuery = new Mingo.Query(castToSnapshotQuery(parsed.query)); @@ -83,6 +101,27 @@ function extendMemoryDB(MemoryDB) { }; } + function projectSnapshot(fields, snapshot) { + // Don't project when there's no projection + if (!fields) return snapshot; + // Don't project when called by ShareDB submit + if (fields.$submit) return snapshot; + + var projectedData = {}; + for (var key in snapshot.data) { + if (fields[key]) projectedData[key] = snapshot.data[key]; + } + + return { + id: snapshot.id, + v: snapshot.v, + type: snapshot.type, + m: snapshot.m, + o: snapshot.o, + data: projectedData + }; + } + // Build a query object that mimics how the query would be executed if it were // made against snapshots persisted with `sharedb-mongo` // FIXME: This doesn't handle nested doc properties with dots like: {'_m.mtime': 12300} From 8ffbc6d632e3fc2b9e98ad3b3b8324d45cc1f947 Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Tue, 20 Feb 2018 18:52:18 -0800 Subject: [PATCH 07/10] Handle errors in getSnapshot/query callbacks --- index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index f7c5b51..709c32e 100644 --- a/index.js +++ b/index.js @@ -25,13 +25,15 @@ function extendMemoryDB(MemoryDB) { // mingo projections. ShareDBMingo.prototype.getSnapshot = function(collection, id, fields, options, callback) { MemoryDB.prototype.getSnapshot.call(this, collection, id, fields, options, function (err, snapshot) { - callback(err, projectSnapshot(fields, snapshot)); + if (err) return callback(err); + callback(null, projectSnapshot(fields, snapshot)); }); } ShareDBMingo.prototype.query = function(collection, query, fields, options, callback) { MemoryDB.prototype.query.call(this, collection, query, fields, options, function (err, snapshots, extra) { + if (err) return callback(err); var projectSnapshotWithFields = projectSnapshot.bind(null, fields); - callback(err, snapshots.map(projectSnapshotWithFields), extra); + callback(null, (snapshots || []).map(projectSnapshotWithFields), extra); }); } From c7b540c3f19ae05dea9b409e1908e57e57d26b9d Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Fri, 23 Feb 2018 13:07:19 -0800 Subject: [PATCH 08/10] Update Travis.yml to add Node 6 & 9, remove Node 5 --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index bec4f9d..9a78f15 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,8 @@ language: node_js node_js: + - 9 - 8 - - 5 + - 6 - 4 script: "npm run test-cover" # Send coverage data to Coveralls From a878067c726d8a67935235a7754cc3e19ee10558 Mon Sep 17 00:00:00 2001 From: Zach Millman Date: Fri, 23 Feb 2018 13:24:06 -0800 Subject: [PATCH 09/10] Remove redundant snapshot projection code --- index.js | 41 ----------------------------------------- 1 file changed, 41 deletions(-) diff --git a/index.js b/index.js index 709c32e..40cb468 100644 --- a/index.js +++ b/index.js @@ -17,26 +17,6 @@ function extendMemoryDB(MemoryDB) { ShareDBMingo.prototype = Object.create(MemoryDB.prototype); - ShareDBMingo.prototype.projectsSnapshots = true; - - // HACK: Wrap MemoryDB functions to project snapshots before returning them. - // This would be cleaner if implemented directly in MemoryDB, or it would be - // more aligned with a drop-in sharedb-mongo replacement if implemented as - // mingo projections. - ShareDBMingo.prototype.getSnapshot = function(collection, id, fields, options, callback) { - MemoryDB.prototype.getSnapshot.call(this, collection, id, fields, options, function (err, snapshot) { - if (err) return callback(err); - callback(null, projectSnapshot(fields, snapshot)); - }); - } - ShareDBMingo.prototype.query = function(collection, query, fields, options, callback) { - MemoryDB.prototype.query.call(this, collection, query, fields, options, function (err, snapshots, extra) { - if (err) return callback(err); - var projectSnapshotWithFields = projectSnapshot.bind(null, fields); - callback(null, (snapshots || []).map(projectSnapshotWithFields), extra); - }); - } - ShareDBMingo.prototype._querySync = function(snapshots, query, options) { var parsed = parseQuery(query); var mingoQuery = new Mingo.Query(castToSnapshotQuery(parsed.query)); @@ -103,27 +83,6 @@ function extendMemoryDB(MemoryDB) { }; } - function projectSnapshot(fields, snapshot) { - // Don't project when there's no projection - if (!fields) return snapshot; - // Don't project when called by ShareDB submit - if (fields.$submit) return snapshot; - - var projectedData = {}; - for (var key in snapshot.data) { - if (fields[key]) projectedData[key] = snapshot.data[key]; - } - - return { - id: snapshot.id, - v: snapshot.v, - type: snapshot.type, - m: snapshot.m, - o: snapshot.o, - data: projectedData - }; - } - // Build a query object that mimics how the query would be executed if it were // made against snapshots persisted with `sharedb-mongo` // FIXME: This doesn't handle nested doc properties with dots like: {'_m.mtime': 12300} From 36066196e768329fc104dfba1e3d677097062c45 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Mon, 26 Feb 2018 20:46:11 -0800 Subject: [PATCH 10/10] Add comments to new query tests, add skipped test on _m.mtime --- test/query.js | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/test/query.js b/test/query.js index 6affcbe..93cfdc1 100644 --- a/test/query.js +++ b/test/query.js @@ -50,12 +50,25 @@ module.exports = function() { }); }); - describe('top-level Mongo doc properties', function() { + describe('filtering on special Share properties', function() { + // When sharedb-mongo persists a snapshot into Mongo, any properties + // underneath `data` get "promoted" to top-level, and Share properties + // get underscore-prefixed to avoid name conflicts, like `v` to `_v`. + // + // Query conditions don't undergo this transformation, so if you wanted + // to filter on snapshot version, you'd query with `{_v: 12}`. These tests + // check that sharedb-mingo-memory is consistent with sharedb-mongo for + // queries like those that filter on non-data Share properties. var snapshots = [ - {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1"}, - {type: 'json0', v: 1, data: {x: 1, y: 2}, id: "test2"}, - {type: 'json0', v: 1, data: {x: 2, y: 2}, id: "test3"} + {type: 'json0', v: 1, data: {x: 1, y: 1}, id: "test1", m: {mtime: 1000}}, + {type: 'json0', v: 1, data: {x: 1, y: 2}, id: "test2", m: {mtime: 1001}}, + {type: 'json0', v: 1, data: {x: 2, y: 2}, id: "test3", m: {mtime: 1002}} ]; + var snapshotsNoMeta = snapshots.map(function(snapshot) { + var snapshotCopy = JSON.parse(JSON.stringify(snapshot)); + delete snapshotCopy.m; + return snapshotCopy; + }); beforeEach(function(done) { var db = this.db; @@ -64,18 +77,31 @@ module.exports = function() { }, done); }); - it('matches value', function(done) { + it('condition on Mongo _id (Share id)', function(done) { this.db.query('testcollection', {_id: 'test1'}, null, null, function(err, results, extra) { if (err) throw err; - expect(results).eql([snapshots[0]]); + expect(results).eql([snapshotsNoMeta[0]]); + done(); + }); + }); + + // The simpler query-casting approach doesn't handle queries that filter on + // sub-properties of the Share metadata object. An alternative would be to + // rewrite this module to cast Share snapshots to Mongo docs and vice versa, + // the same way that sharedb-mongo does: + // https://github.com/share/sharedb-mingo-memory/pull/3#pullrequestreview-99017385 + it.skip('condition on sub-property under Share metadata', function(done) { + this.db.query('testcollection', {'_m.mtime': 1001}, null, null, function(err, results, extra) { + if (err) throw err; + expect(results).eql([snapshotsNoMeta[1]]); done(); }); }); - it('combines with data conditions', function(done) { + it('condition on Mongo _id and Share data', function(done) { this.db.query('testcollection', {y: 2, _id: {$nin: ['test2']}}, null, null, function(err, results, extra) { if (err) throw err; - expect(results).eql([snapshots[2]]); + expect(results).eql([snapshotsNoMeta[2]]); done(); }); }); @@ -83,7 +109,7 @@ module.exports = function() { it('top-level boolean operator', function(done) { this.db.query('testcollection', {$or: [{y: 1}, {_id: 'test2'}]}, null, null, function(err, results, extra) { if (err) throw err; - expect(results).eql([snapshots[0], snapshots[1]]); + expect(results).eql([snapshotsNoMeta[0], snapshotsNoMeta[1]]); done(); }); });