Added validation for empty string on sqs and sns.

gmetzker · Jan 29, 2017 · 50725d5 · 50725d5
1 parent d63682c
commit 50725d5
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 5 deletions.
diff --git a/src/index.js b/src/index.js
@@ -227,6 +227,10 @@ class Plugin {
   compileFunctionDeadLetterResource(functionName) {
     const functionObj = this.serverless.service.getFunction(functionName);
 
+    if (functionObj.deadLetter === null) {
+      return BbPromise.resolve();
+    }
+
     if (functionObj.deadLetter === undefined) {
       return BbPromise.resolve();
     }
@@ -267,11 +271,15 @@ class Plugin {
 
   compileFunctionDeadLetterQueue(functionName, queueConfig) {
 
-    if (typeof queueConfig !== 'string') {
+    if (typeof queueConfig !== 'string' && queueConfig !== null) {
       throw new Error(`Function property ${functionName}.deadLetter.sqs is an unexpected type.  This must be a or string.`);
     }
 
-    const queueName = queueConfig;
+    const queueName = (queueConfig || '').trim();
+
+    if (queueName.length < 1) {
+      throw new Error(`Function property ${functionName}.deadLetter.sqs must contain one or more characters.`);
+    }
 
     const functionLogicalId = Plugin.GetLogicalIdForFunction(functionName);
     const queueLogicalId = Plugin.GetLogicalIdForDlQueue(functionName);
@@ -319,11 +327,15 @@ class Plugin {
 
   compileFunctionDeadLetterTopic(functionName, topicConfig) {
 
-    if (typeof topicConfig !== 'string') {
+    if (typeof topicConfig !== 'string' && topicConfig !== null) {
       throw new Error(`Function property ${functionName}.deadLetter.sns is an unexpected type.  This must be a or string.`);
     }
 
-    const topicName = topicConfig;
+    const topicName = (topicConfig || '').trim();
+
+    if (topicName.length < 1) {
+      throw new Error(`Function property ${functionName}.deadLetter.sns must contain one or more characters.`);
+    }
 
     const topicLogicalId = Plugin.GetLogicalIdForDlTopic(functionName);
     const resources = this.serverless.service.provider.compiledCloudFormationTemplate.Resources;

diff --git a/tests/index.test.js b/tests/index.test.js
@@ -1125,7 +1125,7 @@ describe('serverless-plugin-lambda-dead-letter', () => {
 
     });
 
-    it('does not call compile resources if deadLetter undefined', () => {
+    it('does not call compile resources if deadLetter is undefined', () => {
 
       // ARRANGE:
 
@@ -1161,6 +1161,42 @@ describe('serverless-plugin-lambda-dead-letter', () => {
 
     });
 
+    it('does not call compile resources if deadLetter is null', () => {
+
+      // ARRANGE:
+
+      const stage = 'test1';
+      const region = 'us-west-42';
+
+      const mockServerless = createMockServerless(createMockRequest(sinon.stub()));
+
+
+      const stubGetFunction = sinon.stub(mockServerless.service, 'getFunction');
+      stubGetFunction.withArgs('f2').returns({ name: 'f2', deadLetter: null });
+
+
+      const plugin = new Plugin(mockServerless, { stage, region });
+
+      const stubCompileFunctionDeadLetterQueue = sinon.stub(plugin, 'compileFunctionDeadLetterQueue');
+      const stubCompileFunctionDeadLetterTopic = sinon.stub(plugin, 'compileFunctionDeadLetterTopic');
+
+      // ACT:
+      const actual = plugin.compileFunctionDeadLetterResource('f2');
+
+      // ASSERT:
+      expect(isPromise(actual)).to.be(true);
+
+      return actual.then(() => {
+
+        expect(stubGetFunction.callCount).to.be(1);
+        expect(stubGetFunction.calledWithExactly('f2')).to.be(true);
+        expect(stubCompileFunctionDeadLetterQueue.callCount).to.be(0);
+        expect(stubCompileFunctionDeadLetterTopic.callCount).to.be(0);
+
+      });
+
+    });
+
     it('calls compileFunctionDeadLetterQueue if deadLetter.sqs is defined', () => {
 
       // ARRANGE:
@@ -1262,6 +1298,30 @@ describe('serverless-plugin-lambda-dead-letter', () => {
 
     });
 
+    const sqsStringValues = [null, '', '  '];
+
+    sqsStringValues.forEach((sqsValue) => {
+
+      it(`throws an error when deadLetter.sqs is '${sqsValue}' i.e. empty`, () => {
+
+        // ARRANGE:
+        const stage = 'test1';
+        const region = 'us-west-42';
+
+        const mockServerless = createMockServerless(createMockRequest(sinon.stub()));
+        const plugin = new Plugin(mockServerless, { stage, region });
+
+        // ACT:
+        const act = () => plugin.compileFunctionDeadLetterQueue('f1', sqsValue);
+
+        // ASSERT:
+        expect(act).throwException((e) => {
+          expect(e.message).to.contain('deadLetter.sqs must contain one or more characters');
+        });
+
+      });
+
+    });
     it('assigns SQS resource', () => {
 
       // ARRANGE:
@@ -1355,6 +1415,30 @@ describe('serverless-plugin-lambda-dead-letter', () => {
 
     });
 
+    const snsStringValues = [null, '', '  '];
+
+    snsStringValues.forEach((snsValue) => {
+
+      it(`throws an error when deadLetter.ns is '${snsValue}' i.e. empty`, () => {
+
+        // ARRANGE:
+        const stage = 'test1';
+        const region = 'us-west-42';
+
+        const mockServerless = createMockServerless(createMockRequest(sinon.stub()));
+        const plugin = new Plugin(mockServerless, { stage, region });
+
+        // ACT:
+        const act = () => plugin.compileFunctionDeadLetterTopic('f1', snsValue);
+
+        // ASSERT:
+        expect(act).throwException((e) => {
+          expect(e.message).to.contain('deadLetter.sns must contain one or more characters');
+        });
+
+      });
+    });
+
     it('assigns SNS resource', () => {
 
       // ARRANGE: