Reject on PendingConfirmation in findSubscriptionArn() with error…

….code `PENDING`.
martinj · Oct 25, 2016 · a87f35f · a87f35f
1 parent c500955
commit a87f35f
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 12 deletions.
diff --git a/index.js b/index.js
@@ -32,20 +32,26 @@ exports.register = function (server, opts, next) {
 	server.expose('sns', sns);
 
 	const subscribe = Hoek.reach(opts, 'topic.subscribe');
+
+	function requestSubscription() {
+		return sns
+			.subscribe(opts.topic.arn, subscribe.protocol, subscribe.endpoint)
+			.then(() => server.log(['hookido', 'subscribe'], 'Subscription request sent'));
+	}
+
 	if (subscribe) {
+
 		server.ext('onPostStart', (srv, next) => {
 			sns
 				.findSubscriptionArn(opts.topic.arn, subscribe.protocol, subscribe.endpoint)
 				.then(() => server.log(['hookido', 'subscribe'], 'Subscription already exists'))
-				.catch({code: 'NOT_FOUND'}, () => {
-					return sns
-						.subscribe(opts.topic.arn, subscribe.protocol, subscribe.endpoint)
-						.then(() => server.log(['hookido', 'subscribe'], 'Subscription request sent'));
-				})
+				.catch({code: 'NOT_FOUND'}, requestSubscription)
+				.catch({code: 'PENDING'}, requestSubscription)
 				.catch((err) => server.log(['hookido', 'subscribe', 'error'], err));
 
 			next();
 		});
+
 	}
 
 	const topicAttributes = Hoek.reach(opts, 'topic.attributes');

diff --git a/lib/sns.js b/lib/sns.js
@@ -72,6 +72,12 @@ class SNS {
 				});
 
 				if (arn) {
+					if (arn.SubscriptionArn === 'PendingConfirmation') {
+						const err = new Error('Subscription is pending confirmation');
+						err.code = 'PENDING';
+						throw err;
+					}
+
 					return arn.SubscriptionArn;
 				}
 

diff --git a/package.json b/package.json
@@ -6,7 +6,7 @@
   "scripts": {
     "lint": "eslint --ext '.js' lib test *.js",
     "watch": "mocha --watch 'test/**/*.js' 'lib/**/*.js' '*.js' --timeout 500",
-    "test": "npm run lint && NODE_ENV=test istanbul cover _mocha -- -u exports -R spec --timeout 3000 test/**/*.test.js",
+    "test": "npm run lint && NODE_ENV=test istanbul cover _mocha -- -u exports -R spec --timeout 3000 test/*.test.js test/**/*.test.js",
     "mocha-only-detect": "mocha-only-detector-glob **/*.test.js",
     "ci": "npm test --coverage && istanbul report cobertura",
     "changelog": "git log `git describe --tags --abbrev=0`..HEAD --pretty=format:\"  * %s\"",

diff --git a/test/index.test.js b/test/index.test.js
@@ -126,6 +126,56 @@ describe('Hookido Hapi Plugin', () => {
 
 	});
 
+	it('sends new subscribe request onPostStart if subscribe option is set and subscription is pending', (done) => {
+
+		const server = new Hapi.Server();
+		server.connection();
+
+		server.register({
+			register: plugin,
+			options: {
+				topic: {
+					arn: 'foo',
+					subscribe: {
+						endpoint: 'http://foo.com',
+						protocol: 'HTTP'
+					}
+				},
+				handlers: {
+					notification: () => {}
+				}
+			}
+		}, (err) => {
+			if (err) {
+				return done(err);
+			}
+
+			server.plugins.hookido.sns.findSubscriptionArn = (arn, protocol, endpoint) => {
+				const err = new Error();
+				err.code = 'PENDING';
+				return Promise.reject(err);
+			};
+
+			server.plugins.hookido.sns.subscribe = (arn, protocol, endpoint) => {
+
+				expect(arn).to.equal('foo');
+				expect(protocol).to.equal('HTTP');
+				expect(endpoint).to.equal('http://foo.com');
+				return Promise.resolve();
+
+			};
+
+			server.start((err) => {
+				if (err) {
+					return done(err);
+				}
+
+				server.stop(done);
+			});
+		});
+
+	});
+
 	it('sends setTopicAttributes request onPostStart if topicAttributes option is set', (done) => {
 
 		const server = new Hapi.Server();

diff --git a/test/lib/sns.test.js b/test/lib/sns.test.js
@@ -17,26 +17,32 @@ describe('SNS', () => {
 
 	describe('#validatePayload', () => {
 
-		it('rejects on invalid JSON', () => {
+		it('rejects on invalid JSON', (done) => {
 
-			return sns
+			sns
 				.validatePayload('ada')
 				.catch((err) => {
 					expect(err.message).to.equal('Invalid SNS payload: Unexpected token a in JSON at position 0');
 					return sns.validatePayload(null);
 				})
 				.catch((err) => {
+
 					expect(err.message).to.equal('Invalid SNS payload: Not valid JSON');
+					done();
+
 				});
 
 		});
 
-		it('rejects on invalid payload', () => {
+		it('rejects on invalid payload', (done) => {
 
-			return sns
+			sns
 				.validatePayload('{"foo":"bar"}')
 				.catch((err) => {
+
 					expect(err.message).to.equal('Message missing required keys.');
+					done();
+
 				});
 
 		});
@@ -148,7 +154,7 @@ describe('SNS', () => {
 
 		afterEach(() => nock.cleanAll());
 
-		it('rejects with error containing code NOT_FOUND if not found', () => {
+		it('rejects with error containing code NOT_FOUND if not found', (done) => {
 
 			nock('https://sns.eu-west-1.amazonaws.com:443')
 				.post('/', 'Action=ListSubscriptionsByTopic&TopicArn=arn%3Aaws%3Asns%3Aeu-west-1%3A111111111111%3Amytopic&Version=2010-03-31')
@@ -168,16 +174,48 @@ describe('SNS', () => {
 					 </ListSubscriptionsByTopicResponse>
 				`);
 
-			return sns
+			sns
 				.findSubscriptionArn('arn:aws:sns:eu-west-1:111111111111:mytopic', 'HTTP', 'http://foo.com/bar')
 				.catch((err) => {
 
 					expect(err.code).to.equal('NOT_FOUND');
+					done();
 
 				});
 
 		});
 
+		it('rejects with error containing code PENDING if subscription exists but is pending confirmation', (done) => {
+
+			nock('https://sns.eu-west-1.amazonaws.com:443')
+				.post('/', 'Action=ListSubscriptionsByTopic&TopicArn=arn%3Aaws%3Asns%3Aeu-west-1%3A111111111111%3Amytopic&Version=2010-03-31')
+				.reply(200, `
+					<ListSubscriptionsByTopicResponse xmlns="http://sns.amazonaws.com/doc/2010-03-31/">
+						<ListSubscriptionsByTopicResult>
+							<Subscriptions>
+								<member>
+									<TopicArn>arn:aws:sns:us-east-1:123456789012:My-Topic</TopicArn>
+									<Protocol>http</Protocol>
+									<SubscriptionArn>PendingConfirmation</SubscriptionArn>
+									<Owner>123456789012</Owner>
+									<Endpoint>http://foo.com/bar</Endpoint>
+								</member>
+							</Subscriptions>
+						</ListSubscriptionsByTopicResult>
+					 </ListSubscriptionsByTopicResponse>
+				`);
+
+			sns
+				.findSubscriptionArn('arn:aws:sns:eu-west-1:111111111111:mytopic', 'HTTP', 'http://foo.com/bar')
+				.catch((err) => {
+
+					expect(err.code).to.equal('PENDING');
+					done();
+
+				});
+		});
+
+
 		it('handles NextToken for paged results', () => {
 
 			nock('https://sns.eu-west-1.amazonaws.com:443')