diff --git a/lib/pubsub/index.js b/lib/pubsub/index.js index 57db7cff0755..e88b41c41886 100644 --- a/lib/pubsub/index.js +++ b/lib/pubsub/index.js @@ -20,6 +20,8 @@ 'use strict'; +var extend = require('extend'); + /** * @type {module:pubsub/subscription} * @private @@ -229,7 +231,6 @@ PubSub.prototype.createTopic = function(name, callback) { * * @param {module:pubsub/topic|string} - topic - The Topic to create a * subscription to. - * @param {string} subName - The name of the subscription. * @param {object=} options - Configuration object. * @param {number} options.ackDeadlineSeconds - The maximum time after receiving * a message that you must ack a message before it is redelivered. @@ -237,6 +238,7 @@ PubSub.prototype.createTopic = function(name, callback) { * it's pulled. (default: false) * @param {number} options.interval - Interval in milliseconds to check for new * messages. (default: 10) + * @param {string} options.name - The name of the subscription. * @param {boolean} options.reuseExisting - If the subscription already exists, * reuse it. The options of the existing subscription are not changed. If * false, attempting to create a subscription that already exists will fail. @@ -250,18 +252,16 @@ PubSub.prototype.createTopic = function(name, callback) { * //- * // Subscribe to a topic. (Also see {module:pubsub/topic#subscribe}). * //- - * var topic = 'messageCenter'; - * var name = 'newMessages'; + * var topicName = 'message-center'; * - * pubsub.subscribe(topic, name, function(err, subscription, apiResponse) {}); + * pubsub.subscribe(topicName, function(err, subscription, apiResponse) {}); * * //- * // Customize the subscription. * //- - * pubsub.subscribe(topic, name, { - * ackDeadlineSeconds: 90, + * pubsub.subscribe(topic, { * autoAck: true, - * interval: 30 + * name: 'my-new-subscription' * }, function(err, subscription, apiResponse) {}); * * //- @@ -273,18 +273,16 @@ PubSub.prototype.createTopic = function(name, callback) { * * var topic = anotherProject.topic('messageCenter'); * - * pubsub.subscribe(topic, name, function(err, subscription, apiResponse) {}); + * pubsub.subscribe(topic, function(err, subscription, apiResponse) {}); */ -PubSub.prototype.subscribe = function(topic, subName, options, callback) { +PubSub.prototype.subscribe = function(topic, options, callback) { + var self = this; + if (!util.is(topic, 'string') && !(topic instanceof Topic)) { throw new Error('A Topic is required for a new subscription.'); } - if (!util.is(subName, 'string')) { - throw new Error('A subscription name is required for a new subscription.'); - } - - if (!callback) { + if (util.is(options, 'function')) { callback = options; options = {}; } @@ -303,14 +301,22 @@ PubSub.prototype.subscribe = function(topic, subName, options, callback) { body.ackDeadlineSeconds = options.ackDeadlineSeconds; } - var subscription = this.subscription(subName, options); + var newSubName = ''; - this.makeReq_('PUT', subscription.name, null, body, function(err, result) { + if (options.name) { + newSubName = Subscription.formatName_(self.projectId, options.name); + delete options.name; + } + + this.makeReq_('PUT', newSubName, null, body, function(err, result) { if (err && !(err.code === 409 && options.reuseExisting)) { callback(err, null, result); return; } + var opts = extend({}, result, options); + var subscription = self.subscription(result.name, opts); + callback(null, subscription, result); }); }; diff --git a/lib/pubsub/topic.js b/lib/pubsub/topic.js index e610909babbc..cc514964e017 100644 --- a/lib/pubsub/topic.js +++ b/lib/pubsub/topic.js @@ -274,6 +274,7 @@ Topic.prototype.getSubscriptions = function(options, callback) { * once it's pulled. (default: false) * @param {number=} options.interval - Interval in milliseconds to check for new * messages. (default: 10) + * @param {string} options.name - The name of the subscription. * @param {boolean=} options.reuseExisting - If the subscription already exists, * reuse it. The options of the existing subscription are not changed. If * false, attempting to create a subscription that already exists will fail. @@ -282,17 +283,16 @@ Topic.prototype.getSubscriptions = function(options, callback) { * * @example * // Without specifying any options. - * topic.subscribe('newMessages', function(err, subscription, apiResponse) {}); + * topic.subscribe(function(err, subscription, apiResponse) {}); * * // With options. - * topic.subscribe('newMessages', { - * ackDeadlineSeconds: 90, + * topic.subscribe({ * autoAck: true, - * interval: 30 + * name: 'my-new-subscription' * }, function(err, subscription, apiResponse) {}); */ -Topic.prototype.subscribe = function(subName, options, callback) { - this.pubsub.subscribe(this, subName, options, callback); +Topic.prototype.subscribe = function(options, callback) { + this.pubsub.subscribe(this, options, callback); }; /** diff --git a/system-test/pubsub.js b/system-test/pubsub.js index 3cc751e2fca6..2bad4c94c89e 100644 --- a/system-test/pubsub.js +++ b/system-test/pubsub.js @@ -145,11 +145,11 @@ describe('pubsub', function() { var SUBSCRIPTIONS = [ { name: SUB_NAMES[0], - options: { ackDeadlineSeconds: 30 } + ackDeadlineSeconds: 30 }, { name: SUB_NAMES[1], - options: { ackDeadlineSeconds: 60 } + ackDeadlineSeconds: 60 } ]; @@ -162,9 +162,7 @@ describe('pubsub', function() { topic = newTopic; // Create subscriptions. - async.parallel(SUBSCRIPTIONS.map(function(sub) { - return topic.subscribe.bind(topic, sub.name, sub.options); - }), done); + async.each(SUBSCRIPTIONS, topic.subscribe.bind(topic), done); }); }); @@ -227,11 +225,24 @@ describe('pubsub', function() { }); }); - it('should allow creation and deletion of a subscription', function(done) { + it('should create and delete a named subscription', function(done) { var subName = generateSubName(); - topic.subscribe(subName, function(err, sub) { + var fullSubName = Subscription.formatName_(pubsub.projectId, subName); + + topic.subscribe({ + name: subName + }, function(err, sub) { assert.ifError(err); assert(sub instanceof Subscription); + assert.strictEqual(sub.name, fullSubName); + sub.delete(done); + }); + }); + + it.skip('should automatically assign a subscription name', function(done) { + topic.subscribe(function(err, sub) { + assert.ifError(err); + assert.notStrictEqual(sub.name, ''); sub.delete(done); }); }); diff --git a/test/pubsub/index.js b/test/pubsub/index.js index 5089f4a44c4c..d1d5af223dff 100644 --- a/test/pubsub/index.js +++ b/test/pubsub/index.js @@ -17,6 +17,7 @@ 'use strict'; var assert = require('assert'); +var extend = require('extend'); var mockery = require('mockery'); var request = require('request'); var Topic = require('../../lib/pubsub/topic.js'); @@ -30,6 +31,12 @@ function Subscription(a, b) { return new OverrideFn(a, b); } +var formatNameOverride; +Subscription.formatName_ = function() { + var fn = formatNameOverride || SubscriptionCached.formatName_; + return fn.apply(null, arguments); +}; + var requestCached = request; var requestOverride; function fakeRequest() { @@ -80,9 +87,10 @@ describe('PubSub', function() { beforeEach(function() { SubscriptionOverride = null; requestOverride = null; + formatNameOverride = null; pubsub = new PubSub({ projectId: PROJECT_ID }); pubsub.makeReq_ = function(method, path, q, body, callback) { - callback(); + callback(null, {}); }; }); @@ -366,132 +374,136 @@ describe('PubSub', function() { name: 'projects/' + PROJECT_ID + '/subscriptions/' + SUB_NAME }; + beforeEach(function() { + pubsub.makeReq_ = function(method, path, query, body, callback) { + callback(null, { name: SUB_NAME }); + }; + }); + it('should throw if no Topic is provided', function() { assert.throws(function() { pubsub.subscribe(); }, /A Topic is required.*/); }); - it('should throw if no sub name is provided', function() { - assert.throws(function() { - pubsub.subscribe('topic'); - }, /A subscription name is required.*/); - }); - it('should not require configuration options', function(done) { - pubsub.makeReq_ = function(method, path, qs, body, callback) { - callback(); - }; - - pubsub.subscribe(TOPIC_NAME, SUB_NAME, done); + pubsub.subscribe(TOPIC_NAME, done); }); - it('should cretae a topic object from a string', function(done) { + it('should create a Topic from a string', function(done) { pubsub.topic = function(topicName) { assert.equal(topicName, TOPIC_NAME); done(); return TOPIC; }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, assert.ifError); + pubsub.subscribe(TOPIC_NAME, assert.ifError); }); - it('should create a subscription object from a string', function(done) { - var opts = {}; + it('should build full subscription name if one is given', function(done) { + var opts = { name: SUB_NAME }; - pubsub.subscription = function(subName, options) { - assert.equal(subName, SUB_NAME); - assert.deepEqual(options, opts); + formatNameOverride = function(projectId, subName) { + assert.strictEqual(projectId, PROJECT_ID); + assert.strictEqual(subName, opts.name); done(); - return SUBSCRIPTION; }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, opts, assert.ifError); + pubsub.subscribe(TOPIC_NAME, opts, assert.ifError); }); - it('should pass options to a created subscription object', function(done) { - var opts = { a: 'b', c: 'd' }; - - pubsub.subscription = function(subName, options) { - assert.equal(subName, SUB_NAME); - assert.deepEqual(options, opts); - done(); - return SUBSCRIPTION; + it('should send correct request', function(done) { + var opts = { + name: SUB_NAME, + ackDeadlineSeconds: 90 }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, opts, assert.ifError); - }); - - it('should send correct request', function(done) { pubsub.makeReq_ = function(method, path, query, body) { - assert.equal(method, 'PUT'); - assert.equal(path, SUBSCRIPTION.name); - assert.equal(body.topic, TOPIC.name); + assert.strictEqual(method, 'PUT'); + assert.strictEqual(path, SUBSCRIPTION.name); + assert.strictEqual(body.topic, TOPIC.name); + assert.strictEqual(body.ackDeadlineSeconds, opts.ackDeadlineSeconds); done(); }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, assert.ifError); + pubsub.subscribe(TOPIC_NAME, opts, assert.ifError); }); - it('should re-use existing subscription if specified', function(done) { - pubsub.subscription = function() { - return SUBSCRIPTION; - }; - - pubsub.makeReq_ = function(method, path, query, body, callback) { - callback({ code: 409 }); - }; + describe('error', function() { + var error = new Error('Error.'); + var apiResponse = { name: SUB_NAME }; - // Don't re-use an existing subscription (error if one exists). - pubsub.subscribe(TOPIC_NAME, SUB_NAME, function(err) { - assert.equal(err.code, 409); + beforeEach(function() { + pubsub.makeReq_ = function(method, path, query, body, callback) { + callback(error, apiResponse); + }; }); - // Re-use an existing subscription (ignore error if one exists). - var opts = { reuseExisting: true }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, opts, function(err, sub) { - assert.ifError(err); - assert.deepEqual(sub, SUBSCRIPTION); + it('should re-use existing subscription if specified', function(done) { + var error = new Error('Error'); + error.code = 409; - done(); - }); - }); + pubsub.makeReq_ = function(method, path, query, body, callback) { + callback(error, apiResponse); + }; - it('should return an api error to the callback', function(done) { - var error = new Error('Error.'); + // Don't re-use an existing subscription (error if one exists). + pubsub.subscribe(TOPIC_NAME, function(err) { + assert.strictEqual(err, error); + }); - pubsub.makeReq_ = function(method, path, query, body, callback) { - callback(error); - }; + // Re-use an existing subscription (ignore error if one exists). + var opts = { reuseExisting: true }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, function(err) { - assert.equal(err, error); - done(); - }); - }); + pubsub.subscription = function() { + return SUBSCRIPTION; + }; - it('should return apiResponse to the callback', function(done) { - var resp = { success: true }; + pubsub.subscribe(TOPIC_NAME, opts, function(err, sub) { + assert.ifError(err); + assert.strictEqual(sub, SUBSCRIPTION); - pubsub.makeReq_ = function(method, path, query, body, callback) { - callback(null, resp); - }; + done(); + }); + }); - pubsub.subscribe(TOPIC_NAME, SUB_NAME, function(err, sub, apiResponse) { - assert.deepEqual(resp, apiResponse); - done(); + it('should execute callback with error & API response', function(done) { + pubsub.subscribe(TOPIC_NAME, function(err, sub, apiResponse_) { + assert.strictEqual(err, error); + assert.strictEqual(sub, null); + assert.strictEqual(apiResponse_, apiResponse); + + done(); + }); }); }); - it('should pass options to the api request', function(done) { - var opts = { ackDeadlineSeconds: 90 }; + describe('success', function() { + var apiResponse = { name: SUB_NAME }; - pubsub.makeReq_ = function(method, path, query, body) { - assert.strictEqual(body.ackDeadlineSeconds, opts.ackDeadlineSeconds); - done(); - }; + beforeEach(function() { + pubsub.makeReq_ = function(method, path, query, body, callback) { + callback(null, apiResponse); + }; + }); + + it('should exec callback w/ Subscription & API response', function(done) { + var opts = { ackDeadlineSeconds: 90 }; + var expectedOptions = extend({}, apiResponse, opts); + + pubsub.subscription = function(subName, opts_) { + assert.strictEqual(subName, SUB_NAME); + assert.deepEqual(opts_, expectedOptions); + return SUBSCRIPTION; + }; - pubsub.subscribe(TOPIC_NAME, SUB_NAME, opts, assert.ifError); + pubsub.subscribe(TOPIC_NAME, opts, function(err, sub, apiResponse_) { + assert.ifError(err); + assert.strictEqual(sub, SUBSCRIPTION); + assert.strictEqual(apiResponse_, apiResponse); + done(); + }); + }); }); }); diff --git a/test/pubsub/topic.js b/test/pubsub/topic.js index 57905d9c8e95..d6b270363033 100644 --- a/test/pubsub/topic.js +++ b/test/pubsub/topic.js @@ -263,17 +263,15 @@ describe('Topic', function() { describe('subscribe', function() { it('should pass correct arguments to pubsub#subscribe', function(done) { - var subscriptionName = 'subName'; var opts = {}; - topic.pubsub.subscribe = function(t, subName, options, callback) { - assert.deepEqual(t, topic); - assert.equal(subName, subscriptionName); - assert.deepEqual(options, opts); + topic.pubsub.subscribe = function(topic_, opts_, callback) { + assert.strictEqual(topic_, topic); + assert.deepEqual(opts_, opts); callback(); }; - topic.subscribe(subscriptionName, opts, done); + topic.subscribe(opts, done); }); });