From db52b9a0479464dc3e05cefe7c736e6b09e6eb77 Mon Sep 17 00:00:00 2001 From: Justin King Date: Thu, 2 Mar 2017 11:46:48 -0800 Subject: [PATCH 1/3] pubsub: add ability to pass timeout to ack --- packages/pubsub/src/subscription.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/pubsub/src/subscription.js b/packages/pubsub/src/subscription.js index e49f6479c3d..4ab4168f894 100644 --- a/packages/pubsub/src/subscription.js +++ b/packages/pubsub/src/subscription.js @@ -409,6 +409,9 @@ Subscription.generateName_ = function() { * @throws {Error} If at least one ackId is not provided. * * @param {string|string[]} ackIds - An ackId or array of ackIds. + * @param {options=} options - Configuration object. + * @param {number} options.timeout - Set a maximum amount of time in + * milliseconds before giving up if no response is received. * @param {function=} callback - The callback function. * * @example @@ -423,7 +426,7 @@ Subscription.generateName_ = function() { * var apiResponse = data[0]; * }); */ -Subscription.prototype.ack = function(ackIds, callback) { +Subscription.prototype.ack = function(ackIds, options, callback) { var self = this; ackIds = arrify(ackIds); @@ -434,13 +437,24 @@ Subscription.prototype.ack = function(ackIds, callback) { ].join('')); } - callback = callback || common.util.noop; + if (!callback) { + if (is.fn(options)) { + callback = options; + options = {}; + } else { + callback = common.util.noop; + } + } var protoOpts = { service: 'Subscriber', method: 'acknowledge' }; + if (options && is.number(options.timeout)) { + protoOpts.timeout = options.timeout; + } + var reqOpts = { subscription: this.name, ackIds: ackIds From d0accde4624d1c91dda70e009bc6a2725c7d0f61 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 27 Mar 2017 11:43:56 -0400 Subject: [PATCH 2/3] tests --- packages/pubsub/test/subscription.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/pubsub/test/subscription.js b/packages/pubsub/test/subscription.js index 4f8002615bd..13705e309c1 100644 --- a/packages/pubsub/test/subscription.js +++ b/packages/pubsub/test/subscription.js @@ -410,6 +410,19 @@ describe('Subscription', function() { subscription.ack(IDS, assert.ifError); }); + it('should honor the timeout setting', function(done) { + var options = { + timeout: 10 + }; + + subscription.request = function(protoOpts) { + assert.strictEqual(protoOpts.timeout, options.timeout); + done(); + }; + + subscription.ack('abc', options, assert.ifError); + }); + it('should unmark the ack ids as being in progress', function(done) { subscription.request = function(protoOpts, reqOpts, callback) { callback(); From ed4f719130bfc7bbd0919b72e04f6379b12b337d Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Mon, 27 Mar 2017 13:43:56 -0400 Subject: [PATCH 3/3] fix coverage decrease --- packages/pubsub/src/subscription.js | 13 ++++++------- packages/pubsub/test/subscription.js | 12 +++++++++++- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/pubsub/src/subscription.js b/packages/pubsub/src/subscription.js index 4ab4168f894..7d8e40b3ee7 100644 --- a/packages/pubsub/src/subscription.js +++ b/packages/pubsub/src/subscription.js @@ -437,15 +437,14 @@ Subscription.prototype.ack = function(ackIds, options, callback) { ].join('')); } - if (!callback) { - if (is.fn(options)) { - callback = options; - options = {}; - } else { - callback = common.util.noop; - } + if (is.fn(options)) { + callback = options; + options = {}; } + options = options || {}; + callback = callback || common.util.noop; + var protoOpts = { service: 'Subscriber', method: 'acknowledge' diff --git a/packages/pubsub/test/subscription.js b/packages/pubsub/test/subscription.js index 13705e309c1..4f365cc4342 100644 --- a/packages/pubsub/test/subscription.js +++ b/packages/pubsub/test/subscription.js @@ -53,7 +53,8 @@ describe('Subscription', function() { var SUB_NAME = 'test-subscription'; var SUB_FULL_NAME = 'projects/' + PROJECT_ID + '/subscriptions/' + SUB_NAME; var PUBSUB = { - projectId: PROJECT_ID + projectId: PROJECT_ID, + request: util.noop }; var message = 'howdy'; var messageBuffer = new Buffer(message).toString('base64'); @@ -423,6 +424,15 @@ describe('Subscription', function() { subscription.ack('abc', options, assert.ifError); }); + it('should not require a callback', function() { + assert.doesNotThrow(function() { + subscription.ack('abc'); + subscription.ack('abc', { + timeout: 10 + }); + }); + }); + it('should unmark the ack ids as being in progress', function(done) { subscription.request = function(protoOpts, reqOpts, callback) { callback();