Skip to content

Commit

Permalink
(#2477) - fix reserved ids in revsDiff
Browse files Browse the repository at this point in the history
  • Loading branch information
nolanlawson committed Jul 19, 2014
1 parent 040d5c5 commit 0d05b9d
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 12 deletions.
26 changes: 20 additions & 6 deletions lib/adapter.js
Expand Up @@ -334,14 +334,19 @@ AbstractPouchDB.prototype.revsDiff =
}
opts = utils.clone(opts);
var ids = Object.keys(req);

if (!ids.length) {
return callback(null, {});
}

var count = 0;
var missing = {};
var missing = new utils.Map();

function addToMissing(id, revId) {
if (!missing[id]) {
missing[id] = {missing: []};
if (!missing.has(id)) {
missing.set(id, {missing: []});
}
missing[id].missing.push(revId);
missing.get(id).missing.push(revId);
}

function processDoc(id, rev_tree) {
Expand Down Expand Up @@ -371,15 +376,24 @@ AbstractPouchDB.prototype.revsDiff =
ids.map(function (id) {
this._getRevisionTree(id, function (err, rev_tree) {
if (err && err.name === 'not_found' && err.message === 'missing') {
missing[id] = {missing: req[id]};
missing.set(id, {missing: req[id]});
} else if (err) {
return callback(err);
} else {
processDoc(id, rev_tree);
}

if (++count === ids.length) {
return callback(null, missing);

// convert LazyMap to object
var missingObj = {};
var iter = missing.entries();
var next;
while (typeof (next = iter.next()) !== 'undefined') {
missingObj[next[0]] = next[1];
}

return callback(null, missingObj);
}
});
}, this);
Expand Down
8 changes: 3 additions & 5 deletions lib/adapters/http.js
Expand Up @@ -882,7 +882,7 @@ function HttpPouch(opts, callback) {
// Given a set of document/revision IDs (given by req), tets the subset of
// those that do NOT correspond to revisions stored in the database.
// See http://wiki.apache.org/couchdb/HttpPostRevsDiff
api.revsDiff = utils.adapterFun('revsDif', function (req, opts, callback) {
api.revsDiff = utils.adapterFun('revsDiff', function (req, opts, callback) {

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

Don't know how this managed to slip through, but I'm guessing the typo was causing this method to never get hit.

// If no options were given, set the callback to be the second parameter
if (typeof opts === 'function') {
callback = opts;
Expand All @@ -894,10 +894,8 @@ function HttpPouch(opts, callback) {
headers: host.headers,
method: 'POST',
url: genDBUrl(host, '_revs_diff'),
body: req
}, function (err, res) {
callback(err, res);
});
body: JSON.stringify(req)

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

necessary because otherwise e.g. {constructor: 'foo'} would get sent as {}

}, callback);
});

api._close = function (callback) {
Expand Down
20 changes: 20 additions & 0 deletions lib/deps/collections.js
Expand Up @@ -11,6 +11,9 @@ LazyMap.prototype.mangle = function (key) {
}
return '$' + key;
};
LazyMap.prototype.unmangle = function (key) {
return key.substring(1);
};
LazyMap.prototype.get = function (key) {
var mangled = this.mangle(key);
if (mangled in this.store) {
Expand All @@ -36,6 +39,23 @@ LazyMap.prototype.delete = function (key) {
}
return false;
};
LazyMap.prototype.entries = function () {
var self = this;
var i = 0;
var keys = Object.keys(self.store).sort();
return {
next: function () {
var key = keys[i++];
if (typeof key === 'undefined') {
return;
} else {
var value = self.store[key];
return [self.unmangle(key), value];
}
}
};
};

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

tried to match the spec, wasn't sure what's supposed to happen when the iterator is out of elements, so I just returned undefined.

This comment has been minimized.

Copy link
@calvinmetcalf

calvinmetcalf via email Jul 19, 2014

Member

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

I cannot seem to find the spec anywhere, and the node 0.10 --harmony version doesn't even implement entries (nor does Firefox 30 or Chrome 36). Let's just keep it as-is until the spec is finalized.

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

nevermind, figured out I can enable it with chrome://flags. Chrome doesn't implement entries(), but there is a forEach(function (value, key)) which seems much more straightforward.

This comment has been minimized.

Copy link
@calvinmetcalf

calvinmetcalf via email Jul 19, 2014

Member

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Jul 19, 2014

Author Member

all right, my implementation of forEach is close enough.

function LazySet() {
this.store = new LazyMap();
}
Expand Down
37 changes: 36 additions & 1 deletion tests/test.revs_diff.js
Expand Up @@ -12,7 +12,7 @@ adapters.forEach(function (adapter) {
testUtils.cleanup([dbs.name], done);
});

after(function (done) {
afterEach(function (done) {
testUtils.cleanup([dbs.name], done);
});

Expand Down Expand Up @@ -101,5 +101,40 @@ adapters.forEach(function (adapter) {
});
});

it('Revs diff with empty revs', function () {
return new PouchDB(dbs.name).then(function (db) {
return db.revsDiff({}).then(function (res) {
should.exist(res);
});
});
});

it('Test revs diff with reserved ID', function (done) {
var db = new PouchDB(dbs.name);
var revs = [];
db.post({
test: 'constructor',
_id: 'constructor'
}, function (err, info) {
revs.push(info.rev);
db.put({
_id: info.id,
_rev: info.rev,
another: 'test'
}, function (err, info2) {
revs.push(info2.rev);
db.revsDiff({ 'constructor': revs }, function (err, results) {
results.should.not.include.keys('constructor');
revs.push('2-randomid');
db.revsDiff({ 'constructor': revs }, function (err, results) {
results.should.include.keys('constructor');
results.constructor.missing.should.have.length(1);
done();
});
});
});
});
});

});
});

0 comments on commit 0d05b9d

Please sign in to comment.