From 9edd3d6df70cbc8acd381c5ba244192da2b94725 Mon Sep 17 00:00:00 2001
From: Stephen Sawchuk <sawchuk@gmail.com>
Date: Sat, 25 Jul 2015 13:46:12 -0400
Subject: [PATCH] pubsub: remove autoCreate, add getMetadata
---
lib/pubsub/index.js | 59 +++----------
lib/pubsub/topic.js | 73 ++++++----------
system-test/pubsub.js | 26 ++----
test/pubsub/index.js | 20 ++++-
test/pubsub/topic.js | 199 ++++++++++++++----------------------------
5 files changed, 128 insertions(+), 249 deletions(-)
diff --git a/lib/pubsub/index.js b/lib/pubsub/index.js
index 1de3f6622b5..57db7cff075 100644
--- a/lib/pubsub/index.js
+++ b/lib/pubsub/index.js
@@ -174,9 +174,9 @@ PubSub.prototype.getTopics = function(query, callback) {
return;
}
var topics = (result.topics || []).map(function(item) {
- return new Topic(self, {
- name: item.name
- });
+ var topicInstance = self.topic(item.name);
+ topicInstance.metadata = item;
+ return topicInstance;
});
var nextQuery = null;
if (result.nextPageToken) {
@@ -199,16 +199,10 @@ PubSub.prototype.getTopics = function(query, callback) {
*
* @example
* pubsub.createTopic('my-new-topic', function(err, topic, apiResponse) {
- * topic.publish({
- * data: 'New message!'
- * }, function(err) {});
+ * if (!err) {
+ * // The topic was created successfully.
+ * }
* });
- *
- * //-
- * // <strong>Note:</strong> For cases like the one above, it is simpler to use
- * // {module:pubsub#topic}, which will create the topic for you at the time you
- * // publish a message.
- * //-
*/
PubSub.prototype.createTopic = function(name, callback) {
callback = callback || util.noop;
@@ -359,56 +353,27 @@ PubSub.prototype.subscription = function(name, options) {
};
/**
- * Create a Topic object to reference an existing topic.
+ * Create a Topic object to reference an existing topic. See
+ * {module:pubsub/createTopic} to create a topic.
*
* @throws {Error} If a name is not provided.
*
* @param {string} name - The name of the topic.
- * @param {object=} options - Configuration object.
- * @param {boolean} options.autoCreate - Automatically create topic if it
- * doesn't exist. Note that messages published to a topic with no
- * subscribers will not be delivered. Default: true.
* @return {module:pubsub/topic}
*
* @example
- * //-
- * // By default, it isn't required to specify a topic that already exists. The
- * // first time you publish a message, the topic will be created for you.
- * //
- * // This will only cost one additional API request at the time of publishing.
- * // If the topic doesn't need to be created, there is no performance penalty.
- * //-
- * var topic = pubsub.topic('my-topic');
+ * var topic = pubsub.topic('my-existing-topic');
*
* topic.publish({
* data: 'New message!'
* }, function(err) {});
- *
- * //-
- * // If you prefer an error when trying to publish to a topic that doesn't
- * // exist, set `autoCreate` to `false`.
- * //-
- * var nonExistentTopic = pubsub.topic('my-non-existent-topic', {
- * autoCreate: false
- * });
- *
- * nonExistentTopic.publish({
- * data: 'New message!'
- * }, function(err) {
- * if (err) {
- * // API error from trying to publish a message to a non-existent topic.
- * }
- * });
*/
-PubSub.prototype.topic = function(name, options) {
+PubSub.prototype.topic = function(name) {
if (!name) {
throw new Error('A name must be specified for a new topic.');
}
- options = options || {};
- return new Topic(this, {
- name: name,
- autoCreate: options.autoCreate
- });
+
+ return new Topic(this, name);
};
/**
diff --git a/lib/pubsub/topic.js b/lib/pubsub/topic.js
index 602ce68f810..e610909babb 100644
--- a/lib/pubsub/topic.js
+++ b/lib/pubsub/topic.js
@@ -29,11 +29,7 @@ var util = require('../common/util.js');
/*! Developer Documentation
*
* @param {module:pubsub} pubsub - PubSub object.
- * @param {object} options - Configuration object.
- * @param {boolean=} options.autoCreate - Automatically create topic if it
- * doesn't exist. Note that messages published to a topic with no
- * subscribers will not be delivered. Default: true.
- * @param {string} options.name - Name of the topic.
+ * @param {string} name - Name of the topic.
*/
/**
* A Topic object allows you to interact with a Google Cloud Pub/Sub topic.
@@ -48,13 +44,14 @@ var util = require('../common/util.js');
*
* var topic = pubsub.topic('my-topic');
*/
-function Topic(pubsub, options) {
- this.autoCreate = options.autoCreate !== false;
- this.name = Topic.formatName_(pubsub.projectId, options.name);
+function Topic(pubsub, name) {
+ this.name = Topic.formatName_(pubsub.projectId, name);
this.projectId = pubsub.projectId;
this.pubsub = pubsub;
- this.unformattedName = options.name;
+ this.unformattedName = name;
+
+ this.makeReq_ = this.pubsub.makeReq_.bind(this.pubsub);
}
/**
@@ -181,6 +178,28 @@ Topic.prototype.delete = function(callback) {
this.makeReq_('DELETE', this.name, null, null, callback);
};
+/**
+ * Get the official representation of this topic from the API.
+ *
+ * @param {function} callback - The callback function.
+ * @param {?error} callback.err - An error returned while making this request.
+ * @param {object} callback.metadata - The metadata of the Topic.
+ * @param {object} callback.apiResponse - The full API response.
+ */
+Topic.prototype.getMetadata = function(callback) {
+ var self = this;
+
+ this.makeReq_('GET', this.name, null, null, function(err, resp) {
+ if (err) {
+ callback(err, null, resp);
+ return;
+ }
+
+ self.metadata = resp;
+ callback(null, self.metadata, resp);
+ });
+};
+
/**
* Get a list of the subscriptions registered to this topic. You may optionally
* provide a query object as the first argument to customize the response.
@@ -305,40 +324,4 @@ Topic.prototype.subscription = function(name, options) {
return this.pubsub.subscription(name, options);
};
-/**
- * Make an API request using the parent PubSub object's `makeReq_`. If the Topic
- * instance has `autoCreate: true` set, this method will first try to create the
- * Topic in the event of a 404.
- *
- * @private
- *
- * @param {string} method - Action.
- * @param {string} path - Request path.
- * @param {*} query - Request query object.
- * @param {*} body - Request body contents.
- * @param {function} callback - The callback function.
- */
-Topic.prototype.makeReq_ = function(method, path, query, body, callback) {
- var self = this;
-
- function createTopicThenRetryRequest() {
- self.pubsub.createTopic(self.unformattedName, function(err, topic, res) {
- if (err) {
- callback(err, null, res);
- return;
- }
-
- self.pubsub.makeReq_(method, path, query, body, callback);
- });
- }
-
- this.pubsub.makeReq_(method, path, query, body, function(err, res) {
- if (self.autoCreate && err && err.code === 404 && method !== 'DELETE') {
- createTopicThenRetryRequest();
- } else {
- callback(err, res);
- }
- });
-};
-
module.exports = Topic;
diff --git a/system-test/pubsub.js b/system-test/pubsub.js
index f772251acf6..3cc751e2fca 100644
--- a/system-test/pubsub.js
+++ b/system-test/pubsub.js
@@ -115,30 +115,20 @@ describe('pubsub', function() {
});
});
- it('should lazily create by default', function(done) {
- var newTopicName = generateTopicName();
- var newTopic = pubsub.topic(newTopicName);
-
- newTopic.publish({ data: 'message from me' }, function(err) {
+ it('should publish a message', function(done) {
+ var topic = pubsub.topic(TOPIC_NAMES[0]);
+ topic.publish({ data: 'message from me' }, function(err, messageIds) {
assert.ifError(err);
-
- pubsub.getTopics(function(err, topics) {
- assert.ifError(err);
-
- assert(topics.some(function(topic) {
- return topic.name.indexOf(newTopicName) > -1;
- }));
-
- newTopic.delete(done);
- });
+ assert.equal(messageIds.length, 1);
+ done();
});
});
- it('should publish a message', function(done) {
+ it('should get the metadata of a topic', function(done) {
var topic = pubsub.topic(TOPIC_NAMES[0]);
- topic.publish({ data: 'message from me' }, function(err, messageIds) {
+ topic.getMetadata(function(err, metadata) {
assert.ifError(err);
- assert.equal(messageIds.length, 1);
+ assert.strictEqual(metadata.name, topic.name);
done();
});
});
diff --git a/test/pubsub/index.js b/test/pubsub/index.js
index 68901156a35..5089f4a44c4 100644
--- a/test/pubsub/index.js
+++ b/test/pubsub/index.js
@@ -99,9 +99,12 @@ describe('PubSub', function() {
});
describe('getTopics', function() {
+ var topicName = 'fake-topic';
+ var apiResponse = { topics: [{ name: topicName }]};
+
beforeEach(function() {
pubsub.makeReq_ = function(method, path, q, body, callback) {
- callback(null, { topics: [{ name: 'fake-topic' }] });
+ callback(null, apiResponse);
};
});
@@ -121,10 +124,19 @@ describe('PubSub', function() {
pubsub.getTopics(function() {});
});
- it('should return Topic instances', function() {
+ it('should return Topic instances with metadata', function(done) {
+ var topic = {};
+
+ pubsub.topic = function(name) {
+ assert.strictEqual(name, topicName);
+ return topic;
+ };
+
pubsub.getTopics(function(err, topics) {
assert.ifError(err);
- assert(topics[0] instanceof Topic);
+ assert.strictEqual(topics[0], topic);
+ assert.strictEqual(topics[0].metadata, apiResponse.topics[0]);
+ done();
});
});
@@ -237,7 +249,7 @@ describe('PubSub', function() {
'projects/' + PROJECT_ID + '/topics/' + TOPIC_NAME + '/subscriptions';
before(function() {
- TOPIC = new Topic(pubsub, { name: TOPIC_NAME });
+ TOPIC = new Topic(pubsub, TOPIC_NAME);
});
it('should subscribe to a topic by string', function(done) {
diff --git a/test/pubsub/topic.js b/test/pubsub/topic.js
index 3a057fd9ac5..57905d9c8e9 100644
--- a/test/pubsub/topic.js
+++ b/test/pubsub/topic.js
@@ -30,7 +30,7 @@ describe('Topic', function() {
var topic;
beforeEach(function() {
- topic = new Topic(pubsubMock, { name: TOPIC_NAME });
+ topic = new Topic(pubsubMock, TOPIC_NAME);
});
describe('initialization', function() {
@@ -40,7 +40,7 @@ describe('Topic', function() {
Topic.formatName_ = formatName_;
done();
};
- new Topic(pubsubMock, { name: TOPIC_NAME });
+ new Topic(pubsubMock, TOPIC_NAME);
});
it('should assign projectId to `this`', function() {
@@ -50,18 +50,6 @@ describe('Topic', function() {
it('should assign pubsub object to `this`', function() {
assert.deepEqual(topic.pubsub, pubsubMock);
});
-
- it('should set `autoCreate` to true by default', function() {
- assert.strictEqual(topic.autoCreate, true);
- });
-
- it('should allow overriding autoCreate', function() {
- var topic = new Topic(pubsubMock, {
- name: TOPIC_NAME,
- autoCreate: false
- });
- assert.strictEqual(topic.autoCreate, false);
- });
});
describe('formatMessage_', function() {
@@ -186,6 +174,68 @@ describe('Topic', function() {
});
});
+ describe('getMetadata', function() {
+ it('should make the correct API request', function(done) {
+ topic.makeReq_ = function(method, path, query, body) {
+ assert.strictEqual(method, 'GET');
+ assert.strictEqual(path, topic.name);
+ assert.strictEqual(query, null);
+ assert.strictEqual(body, null);
+
+ done();
+ };
+
+ topic.getMetadata(assert.ifError);
+ });
+
+ describe('error', function() {
+ var error = new Error('Error.');
+ var apiResponse = { a: 'b', c: 'd' };
+
+ beforeEach(function() {
+ topic.makeReq_ = function(method, path, query, body, callback) {
+ callback(error, apiResponse);
+ };
+ });
+
+ it('should execute callback with error & API response', function(done) {
+ topic.getMetadata(function(err, metadata, apiResponse_) {
+ assert.strictEqual(err, error);
+ assert.strictEqual(metadata, null);
+ assert.strictEqual(apiResponse_, apiResponse);
+ done();
+ });
+ });
+ });
+
+ describe('success', function() {
+ var apiResponse = { a: 'b', c: 'd' };
+
+ beforeEach(function() {
+ topic.makeReq_ = function(method, path, query, body, callback) {
+ callback(null, apiResponse);
+ };
+ });
+
+ it('should assign the response to the metadata property', function(done) {
+ topic.getMetadata(function(err) {
+ assert.ifError(err);
+ assert.strictEqual(topic.metadata, apiResponse);
+ done();
+ });
+ });
+
+ it('should exec callback with metadata & API response', function(done) {
+ topic.getMetadata(function(err, metadata, apiResponse_) {
+ assert.ifError(err);
+ assert.strictEqual(metadata, apiResponse);
+ assert.strictEqual(apiResponse_, apiResponse);
+ done();
+ });
+ });
+ });
+ });
+
describe('getSubscriptions', function() {
it('should accept just a callback', function(done) {
topic.pubsub.getSubscriptions = function(options, callback) {
@@ -250,125 +300,4 @@ describe('Topic', function() {
doneFn();
});
});
-
- describe('makeReq_', function() {
- var method = 'POST';
- var path = '/path';
- var query = 'query';
- var body = 'body';
-
- it('should call through to pubsub.makeReq_', function(done) {
- topic.pubsub.makeReq_ = function(m, p, q, b) {
- assert.equal(m, method);
- assert.equal(p, path);
- assert.equal(q, query);
- assert.equal(b, body);
-
- done();
- };
-
- topic.makeReq_(method, path, query, body, util.noop);
- });
-
- describe('autoCreate: false', function() {
- it('should execute callback with response', function(done) {
- var error = new Error('Error.');
- var apiResponse = { a: 'b', c: 'd' };
-
- topic.pubsub.makeReq_ = function(method, path, query, body, callback) {
- callback(error, apiResponse);
- };
-
- topic.makeReq_(method, path, query, body, function(err, apiResp) {
- assert.deepEqual(err, error);
- assert.deepEqual(apiResp, apiResponse);
-
- done();
- });
- });
- });
-
- describe('autoCreate: true', function() {
- it('should not create a topic if doing a DELETE', function(done) {
- var topicCreated = false;
-
- topic.pubsub.createTopic = function() {
- topicCreated = true;
- };
-
- topic.pubsub.makeReq_ = function(method, path, query, body, callback) {
- callback({ code: 404 });
- };
-
- topic.makeReq_('DELETE', path, query, body, function() {
- assert.strictEqual(topicCreated, false);
-
- done();
- });
- });
-
- it('should create a non-DELETE API request returns 404', function(done) {
- topic.pubsub.createTopic = function(topicName) {
- assert.equal(topicName, TOPIC_NAME);
-
- done();
- };
-
- topic.pubsub.makeReq_ = function(method, path, query, body, callback) {
- callback({ code: 404 });
- };
-
- topic.makeReq_(method, path, query, body, util.noop);
- });
-
- describe('creating topic failed', function() {
- var error = new Error('Error.');
- var apiResponse = { a: 'b', c: 'd' };
-
- beforeEach(function() {
- topic.pubsub.createTopic = function(topicName, callback) {
- callback(error, null, apiResponse);
- };
-
- topic.pubsub.makeReq_ = function(m, p, q, b, callback) {
- callback({ code: 404 });
- };
- });
-
- it('should execute the callback with error & apiResp', function(done) {
- topic.makeReq_(method, path, query, body, function(err, t, apiResp) {
- assert.deepEqual(err, error);
- assert.strictEqual(t, null);
- assert.deepEqual(apiResp, apiResponse);
-
- done();
- });
- });
- });
-
- describe('creating topic succeeded', function() {
- it('should call makeReq_ again with the original args', function(done) {
- topic.pubsub.createTopic = function(topicName, callback) {
- callback();
- };
-
- topic.pubsub.makeReq_ = function(m, p, q, b, callback) {
- // Overwrite the method to confirm it is called again.
- topic.pubsub.makeReq_ = function(m, p, q, b, callback) {
- assert.equal(m, method);
- assert.equal(p, path);
- assert.equal(q, query);
- assert.equal(b, body);
-
- callback(); // (should be the done function)
- };
-
- callback({ code: 404 });
- };
-
- topic.makeReq_(method, path, query, body, done);
- });
- });
- });
- });
});