Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove topic.options.autoCreate, add topic.getMetadata #742

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 12 additions & 47 deletions lib/pubsub/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
* }

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

* });
*
* //-
* // <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;
Expand Down Expand Up @@ -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);
};

/**
Expand Down
73 changes: 28 additions & 45 deletions lib/pubsub/topic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
26 changes: 8 additions & 18 deletions system-test/pubsub.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
20 changes: 16 additions & 4 deletions test/pubsub/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
});

Expand All @@ -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();
});
});

Expand Down Expand Up @@ -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) {
Expand Down
Loading