Skip to content

Commit

Permalink
🥅 Catch falsy agents
Browse files Browse the repository at this point in the history 
We recently made [a change][1], which added checks for if `agent` was
provided to certain backend methods. This added unofficial support for
a falsy `agent`, which we don't really want to support.

In order to formalise this, this change adds an assertion to all public
`Backend` methods, which asserts that `agent` is present, since
behaviour otherwise is undefined.

[1]: #446
  • Loading branch information
@alecgibson
alecgibson committed Apr 7, 2021
1 parent 2606d7f commit 0e71f10
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 14 deletions.
21 changes: 19 additions & 2 deletions lib/backend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ Backend.prototype.trigger = function(action, agent, request, callback) {
// {op:}, {create:} or {del:} field. It should probably contain a v: field (if
// it doesn't, it defaults to the current version).
Backend.prototype.submit = function(agent, index, id, op, options, originalCallback) {
assertAgent(agent);
var backend = this;
var request = new SubmitRequest(this, agent, index, id, op, options);

Expand Down Expand Up @@ -233,6 +234,7 @@ Backend.prototype.submit = function(agent, index, id, op, options, originalCallb
};

Backend.prototype.sanitizeOp = function(agent, index, id, op, callback) {
assertAgent(agent);
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
this._sanitizeOp(agent, projection, collection, id, op, callback);
Expand Down Expand Up @@ -331,6 +333,7 @@ Backend.prototype.getOps = function(agent, index, id, from, to, options, callbac
callback = options;
options = null;
}
assertAgent(agent);
var start = Date.now();
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
Expand All @@ -356,6 +359,7 @@ Backend.prototype.getOpsBulk = function(agent, index, fromMap, toMap, options, c
callback = options;
options = null;
}
assertAgent(agent);
var start = Date.now();
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
Expand All @@ -380,6 +384,7 @@ Backend.prototype.fetch = function(agent, index, id, options, callback) {
callback = options;
options = null;
}
assertAgent(agent);
var start = Date.now();
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
Expand All @@ -392,7 +397,7 @@ Backend.prototype.fetch = function(agent, index, id, options, callback) {
id: id
};
var snapshotOptions = (options && options.snapshotOptions) || {};
if (agent) snapshotOptions.agentCustom = agent.custom;
snapshotOptions.agentCustom = agent.custom;
backend.db.getSnapshot(collection, id, fields, snapshotOptions, function(err, snapshot) {
if (err) return callback(err);
var snapshotProjection = backend._getSnapshotProjection(backend.db, projection);
Expand Down Expand Up @@ -428,6 +433,7 @@ Backend.prototype.fetchBulk = function(agent, index, ids, options, callback) {
callback = options;
options = null;
}
assertAgent(agent);
var start = Date.now();
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
Expand All @@ -440,7 +446,7 @@ Backend.prototype.fetchBulk = function(agent, index, ids, options, callback) {
ids: ids
};
var snapshotOptions = (options && options.snapshotOptions) || {};
if (agent) snapshotOptions.agentCustom = agent.custom;
snapshotOptions.agentCustom = agent.custom;
backend.db.getSnapshotBulk(collection, ids, fields, snapshotOptions, function(err, snapshotMap) {
if (err) return callback(err);
var snapshotProjection = backend._getSnapshotProjection(backend.db, projection);
Expand Down Expand Up @@ -474,6 +480,7 @@ Backend.prototype.subscribe = function(agent, index, id, version, options, callb
callback = options;
options = null;
}
assertAgent(agent);
if (options) {
// We haven't yet implemented the ability to pass options to subscribe. This is because we need to
// add the ability to SubmitRequest.commit to optionally pass the metadata to other clients on
Expand Down Expand Up @@ -543,6 +550,7 @@ Backend.prototype.subscribe = function(agent, index, id, version, options, callb
* ) => void} callback
*/
Backend.prototype.subscribeBulk = function(agent, index, versions, callback) {
assertAgent(agent);
var start = Date.now();
var projection = this.projections[index];
var collection = (projection) ? projection.target : index;
Expand Down Expand Up @@ -610,6 +618,7 @@ function destroyStreams(streams) {
Backend.prototype.queryFetch = function(agent, index, query, options, callback) {
var start = Date.now();
var backend = this;
assertAgent(agent);
backend._triggerQuery(agent, index, query, options, function(err, request) {
if (err) return callback(err);
backend._query(agent, request, function(err, snapshots, extra) {
Expand All @@ -631,6 +640,7 @@ Backend.prototype.queryFetch = function(agent, index, query, options, callback)
Backend.prototype.querySubscribe = function(agent, index, query, options, callback) {
var start = Date.now();
var backend = this;
assertAgent(agent);
backend._triggerQuery(agent, index, query, options, function(err, request) {
if (err) return callback(err);
if (request.db.disableSubscribe) {
Expand Down Expand Up @@ -721,6 +731,7 @@ Backend.prototype.getChannels = function(collection, id) {
};

Backend.prototype.fetchSnapshot = function(agent, index, id, version, callback) {
assertAgent(agent);
var start = Date.now();
var backend = this;
var projection = this.projections[index];
Expand Down Expand Up @@ -777,6 +788,7 @@ Backend.prototype._fetchSnapshot = function(collection, id, version, callback) {
};

Backend.prototype.fetchSnapshotByTimestamp = function(agent, index, id, timestamp, callback) {
assertAgent(agent);
var start = Date.now();
var backend = this;
var projection = this.projections[index];
Expand Down Expand Up @@ -837,6 +849,7 @@ Backend.prototype._buildSnapshotFromOps = function(id, startingSnapshot, ops, ca
};

Backend.prototype.transformPresenceToLatestVersion = function(agent, presence, callback) {
assertAgent(agent);
if (!presence.c || !presence.d) return callback(null, presence);
this.getOps(agent, presence.c, presence.d, presence.v, null, function(error, ops) {
if (error) return callback(error);
Expand Down Expand Up @@ -874,3 +887,7 @@ function filterOpsInPlaceBeforeTimestamp(ops, timestamp) {
}
}
}

function assertAgent(agent) {
if (!agent) throw new Error('agent is required');
}
2 changes: 1 addition & 1 deletion lib/query-emitter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ QueryEmitter.prototype.queryPollDoc = function(id, callback) {
var index = emitter.ids.push(id) - 1;

var snapshotOptions = {};
if (emitter.agent) snapshotOptions.agentCustom = emitter.agent.custom;
snapshotOptions.agentCustom = emitter.agent.custom;

// We can get the result to send to the client async, since there is a
// delay in sending to the client anyway
Expand Down
2 changes: 1 addition & 1 deletion lib/submit-request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ SubmitRequest.prototype.submit = function(callback) {
var fields = {$submit: true};

var snapshotOptions = {};
if (request.agent) snapshotOptions.agentCustom = request.agent.custom;
snapshotOptions.agentCustom = request.agent.custom;
backend.db.getSnapshot(collection, id, fields, snapshotOptions, function(err, snapshot) {
if (err) return callback(err);

Expand Down
18 changes: 9 additions & 9 deletions test/backend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Backend', function() {

describe('getOps', function() {
it('fetches all the ops', function(done) {
backend.getOps(null, 'books', '1984', 0, null, function(error, ops) {
backend.getOps(agent, 'books', '1984', 0, null, function(error, ops) {
if (error) return done(error);
expect(ops).to.have.length(2);
expect(ops[0].create.data).to.eql({title: '1984'});
Expand All @@ -47,7 +47,7 @@ describe('Backend', function() {
var options = {
opsOptions: {metadata: true}
};
backend.getOps(null, 'books', '1984', 0, null, options, function(error, ops) {
backend.getOps(agent, 'books', '1984', 0, null, options, function(error, ops) {
if (error) return done(error);
expect(ops).to.have.length(2);
expect(ops[0].m).to.be.ok;
Expand All @@ -59,7 +59,7 @@ describe('Backend', function() {

describe('fetch', function() {
it('fetches the document', function(done) {
backend.fetch(null, 'books', '1984', function(error, doc) {
backend.fetch(agent, 'books', '1984', function(error, doc) {
if (error) return done(error);
expect(doc.data).to.eql({
title: '1984',
Expand All @@ -73,7 +73,7 @@ describe('Backend', function() {
var options = {
snapshotOptions: {metadata: true}
};
backend.fetch(null, 'books', '1984', options, function(error, doc) {
backend.fetch(agent, 'books', '1984', options, function(error, doc) {
if (error) return done(error);
expect(doc.m).to.be.ok;
done();
Expand Down Expand Up @@ -109,7 +109,7 @@ describe('Backend', function() {

describe('subscribe', function() {
it('subscribes to the document', function(done) {
backend.subscribe(null, 'books', '1984', null, function(error, stream, snapshot) {
backend.subscribe(agent, 'books', '1984', null, function(error, stream, snapshot) {
if (error) return done(error);
expect(stream.open).to.equal(true);
expect(snapshot.data).to.eql({
Expand All @@ -121,7 +121,7 @@ describe('Backend', function() {
expect(data.op).to.eql(op.op);
done();
});
backend.submit(null, 'books', '1984', op, null, function(error) {
backend.submit(agent, 'books', '1984', op, null, function(error) {
if (error) return done(error);
});
});
Expand All @@ -131,7 +131,7 @@ describe('Backend', function() {
var options = {
opsOptions: {metadata: true}
};
backend.subscribe(null, 'books', '1984', null, options, function(error) {
backend.subscribe(agent, 'books', '1984', null, options, function(error) {
expect(error.code).to.equal('ERR_DATABASE_METHOD_NOT_IMPLEMENTED');
done();
});
Expand All @@ -155,7 +155,7 @@ describe('Backend', function() {
});

var op = {op: {p: ['publicationYear'], oi: 1949}};
backend.submit(null, 'books', '1984', op, null, function(error) {
backend.submit(agent, 'books', '1984', op, null, function(error) {
if (error) done(error);
});
});
Expand All @@ -172,7 +172,7 @@ describe('Backend', function() {
});

var op = {op: {p: ['publicationYear'], oi: 1949}};
backend.submit(null, 'books', '1984', op, null, function() {
backend.submit(agent, 'books', '1984', op, null, function() {
// Swallow the error
});
});
Expand Down
4 changes: 3 additions & 1 deletion test/middleware.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ describe('middleware', function() {
});
var op = {create: {type: types.defaultType.uri}};
var options = {testOption: true};
this.backend.submit(null, 'dogs', 'fido', op, options, doneAfter);
var connection = this.backend.connect();
var agent = connection.agent;
this.backend.submit(agent, 'dogs', 'fido', op, options, doneAfter);
});
});
});
Expand Down

0 comments on commit 0e71f10

Please sign in to comment.