Repository: serverless/serverless-azure-functions (ts)

Issue:
sls offline cleanup fails if folder does not exist
<!--
1. Please check if an issue already exists. This bug may have already been documented
2. Check out and follow our Guidelines: https://github.com/serverless/serverless-azure-functions/CONTRIBUTING.md
3. Fill out the whole template so we have a good overview on the issue
4. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the Issue
5. Please follow the template, otherwise we'll have to ask you to update it
-->

# sls offline cleanup fails if folder does not exist

## Description

- What went wrong?

When running sls offline cleanup the command throw an exception if any function folder does not exist within the filesystem.

- What did you expect should have happened?

If any folder doesn't exist it should be skipped and continue onto the next folder

- What was the config you used? N/A
- What stacktrace or error message from your provider did you see? N/A

Similar or dependent issues:

- N/A

## Additional Data

- **_Serverless Framework Version you're using_**: 1.5.2
- **_Serverless CLI Version you're using_**: 1.5.2
- **_Serverless Azure Plugin Version you're using_**: 1.0.0
- **_Operating System_**:
- **_Stack Trace_**:
- **_Provider Error messages_**:


Pull request:
fix: Don't try to clean up non-existent directories
<!--
1. Please check out and follow our Contributing Guidelines: https://github.com/serverless/serverless-azure-functions/CONTRIBUTING.md
2. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the PR
3. Please follow the template, otherwise we'll have to ask you to update it and it will take longer until your PR is merged
-->

## What did you implement:

Check to see if directory exists before trying to delete it

Closes #349 

<!--
Briefly describe the feature if no issue exists for this PR
-->

## How did you implement it:

Added one check to see if directory exists before reading/deleting it

<!--
If this is a nontrivial change please briefly describe your implementation so its easy for us to understand and review your code.
-->

## How can we verify it:

- Create a new project
- Run `sls offline cleanup`

This would have failed before, as it would try to read and then delete directories named after your functions. This check prevents that from happening

<!--
Add any applicable config, commands, screenshots or other resources
to make it easy for us to verify this works. The easier you make it for us
to review a PR, the faster we can review and merge it.

Examples:
* serverless.yml - Fully functioning to easily deploy changes
* Screenshots - Showing the difference between your output and the master
* Cloud Configuration - List cloud resources and show that the correct configuration is in place
* Other - Anything else that comes to mind to help us evaluate
-->

## Todos:

_**Note: Run `npm run test:ci` to run all validation checks on proposed changes**_

- [x] Ensure there are no lint errors.  
       **Validate via `npm run lint`**  
       _Note: Some reported issues can be automatically fixed by running `npm run lint:fix`_
- [x] Write tests and confirm existing functionality is not broken.  
       **Validate via `npm test`**
- [x] Write documentation
- [x] Provide verification config / commands / resources
- [x] Enable "Allow edits from maintainers" for this PR
- [x] Update the messages below

**_Is this ready for review?:_** YES
**_Is it a breaking change?:_** NO


Patch:
diff --git a/jest.config.js b/jest.config.js
index 0e3d8c7..48b1e68 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -2,7 +2,7 @@ module.exports = {
   "collectCoverageFrom": [
     "src/**/*.{js,jsx,ts,tsx}",
     "!src/**/*.d.ts",
-    "!/src/test/"
+    "!src/test/*.ts"
   ],
   "testEnvironment": "node",
   "transform": {
diff --git a/package-lock.json b/package-lock.json
index 11982dc..7eecabf 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.2-16",
+  "version": "1.0.2-18",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
diff --git a/package.json b/package.json
index 455fda7..d59381d 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.2-16",
+  "version": "1.0.2-18",
   "description": "Provider plugin for the Serverless Framework v1.x which adds support for Azure Functions.",
   "license": "MIT",
   "main": "./lib/index.js",
diff --git a/src/plugins/apim/azureApimPlugin.ts b/src/plugins/apim/azureApimPlugin.ts
index 500f441..c43ad7f 100644
--- a/src/plugins/apim/azureApimPlugin.ts
+++ b/src/plugins/apim/azureApimPlugin.ts
@@ -17,7 +17,7 @@ export class AzureApimPlugin extends AzureBasePlugin {
       return Promise.resolve();
     }
 
-    this.serverless.cli.log("Starting APIM service deployment");
+    this.log("Starting APIM service deployment");
 
     const apimService = new ApimService(this.serverless, this.options);
     await apimService.deploy();
diff --git a/src/plugins/deploy/azureDeployPlugin.ts b/src/plugins/deploy/azureDeployPlugin.ts
index 8d7cf3e..7922f82 100644
--- a/src/plugins/deploy/azureDeployPlugin.ts
+++ b/src/plugins/deploy/azureDeployPlugin.ts
@@ -93,7 +93,7 @@ export class AzureDeployPlugin extends AzureBasePlugin<AzureLoginOptions> {
       return Promise.resolve();
     }
 
-    this.serverless.cli.log("Starting APIM service deployment");
+    this.log("Starting APIM service deployment");
 
     const apimService = new ApimService(this.serverless, this.options);
     await apimService.deploy();
diff --git a/src/plugins/login/azureLoginPlugin.ts b/src/plugins/login/azureLoginPlugin.ts
index 137b387..e4f70fd 100644
--- a/src/plugins/login/azureLoginPlugin.ts
+++ b/src/plugins/login/azureLoginPlugin.ts
@@ -36,7 +36,7 @@ export class AzureLoginPlugin extends AzureBasePlugin<AzureLoginOptions> {
       }
       subId = subId || authResult.subscriptions[0].id;
       this.serverless.variables["subscriptionId"] = subId;
-      this.serverless.cli.log(`Using subscription ID: ${subId}`);
+      this.log(`Using subscription ID: ${subId}`);
     }
     catch (e) {
       this.log("Error logging into azure");
diff --git a/src/plugins/login/utils/simpleFileTokenCache.ts b/src/plugins/login/utils/simpleFileTokenCache.ts
index 601e167..f10adb7 100644
--- a/src/plugins/login/utils/simpleFileTokenCache.ts
+++ b/src/plugins/login/utils/simpleFileTokenCache.ts
@@ -57,18 +57,13 @@ export class SimpleFileTokenCache implements adal.TokenCache {
 
   public addSubs(subscriptions: any) {
     this.subscriptions.push(...subscriptions);
-    this.subscriptions = this.subscriptions.reduce((accumulator , current) => {
-      const x = accumulator .find(item => item.id === current.id);
-      if (!x) {
-        return accumulator .concat([current]);
-      } else {
-        return accumulator ;
-      }
+    this.subscriptions = this.subscriptions.reduce((accumulator, current) => {
+      const x = accumulator.find(item => item.id === current.id);
+      return (!x) ? accumulator.concat([current]) : accumulator;
     }, []);
     this.save();
   }
 
-
   public clear(callback: any) {
     this.entries = [];
     this.subscriptions = [];
diff --git a/src/services/armService.ts b/src/services/armService.ts
index f64f7e4..837f56a 100644
--- a/src/services/armService.ts
+++ b/src/services/armService.ts
@@ -101,6 +101,8 @@ export class ArmService extends BaseService {
       return;
     }
 
+    deployment.parameters = deployment.parameters || {};
+    
     for (const key of Object.keys(deployment.parameters)) {
       if (!deployment.parameters[key].value) {
         delete deployment.parameters[key];
@@ -160,6 +162,9 @@ export class ArmService extends BaseService {
 
     const paramsNormalizer = (params: ArmParameters): ArmParameters => {
       const normalized = {};
+      if (!params) {
+        return normalized;
+      }
       const keys = Object.keys(params);
       for (const key of keys) {
         const original = params[key];
@@ -207,6 +212,8 @@ export class ArmService extends BaseService {
    */
   private mergeDefaultParams(parameters: ArmParameters, defaultParameters: ArmParameters): ArmParameters {
     const mergedParams: ArmParameters = {}
+    parameters = parameters || {};
+    defaultParameters = defaultParameters || {};
     Object.keys(defaultParameters).forEach((key) => {
       const defaultParam = defaultParameters[key];
       const param = parameters[key];
@@ -227,7 +234,7 @@ export class ArmService extends BaseService {
     // added to the ARM template used in the deployment.
     const environmentVariables = this.config.provider.environment;
     if (environmentVariables) {
-      this.serverless.cli.log("-> Merging environment configuration");
+      this.log("-> Merging environment configuration");
 
       // This is a json path expression
       // Learn more @ https://goessner.net/articles/JsonPath/index.html#e2
diff --git a/src/services/funcService.ts b/src/services/funcService.ts
index 580f155..dbf5123 100644
--- a/src/services/funcService.ts
+++ b/src/services/funcService.ts
@@ -16,7 +16,7 @@ export class FuncService extends BaseService {
       return;
     }
     if (this.exists(functionName)) {
-      this.serverless.cli.log(`Function ${functionName} already exists`);
+      this.log(`Function ${functionName} already exists`);
       return;
     }
     this.createHandler(functionName);
diff --git a/src/services/functionAppService.ts b/src/services/functionAppService.ts
index 9853302..3cca8af 100644
--- a/src/services/functionAppService.ts
+++ b/src/services/functionAppService.ts
@@ -29,9 +29,9 @@ export class FunctionAppService extends BaseService {
   public async get(): Promise<Site> {
     const response: any = await this.webClient.webApps.get(this.resourceGroup, FunctionAppResource.getResourceName(this.config));
     if (response.error && (response.error.code === "ResourceNotFound" || response.error.code === "ResourceGroupNotFound")) {
-      this.serverless.cli.log(this.resourceGroup);
-      this.serverless.cli.log(FunctionAppResource.getResourceName(this.config));
-      this.serverless.cli.log(JSON.stringify(response));
+      this.log(this.resourceGroup);
+      this.log(FunctionAppResource.getResourceName(this.config));
+      this.log(JSON.stringify(response));
       return null;
     }
 
diff --git a/src/services/invokeService.ts b/src/services/invokeService.ts
index dbb4b52..3a694b9 100644
--- a/src/services/invokeService.ts
+++ b/src/services/invokeService.ts
@@ -27,7 +27,7 @@ export class InvokeService extends BaseService {
     const functionObject = this.configService.getFunctionConfig()[functionName];
     /* accesses the admin key */
     if (!functionObject) {
-      this.serverless.cli.log(`Function ${functionName} does not exist`);
+      this.log(`Function ${functionName} does not exist`);
       return;
     }
 
diff --git a/src/services/packageService.ts b/src/services/packageService.ts
index c92029c..db43ba4 100644
--- a/src/services/packageService.ts
+++ b/src/services/packageService.ts
@@ -51,7 +51,7 @@ export class PackageService extends BaseService {
       }
     });
 
-    this.serverless.cli.log("Copying files for webpack");
+    this.log("Copying files for webpack");
     filesToCopy.forEach((filePath) => {
       const destinationPath = path.join(".webpack", "service", filePath);
       const destinationDirectory = path.dirname(destinationPath);
@@ -59,7 +59,7 @@ export class PackageService extends BaseService {
         fs.mkdirSync(destinationDirectory);
       }
       fs.copyFileSync(filePath, destinationPath);
-      this.serverless.cli.log(`-> ${destinationPath}`);
+      this.log(`-> ${destinationPath}`);
     });
 
     return Promise.resolve();
@@ -76,7 +76,11 @@ export class PackageService extends BaseService {
         fs.unlinkSync(filePath);
       }
 
-      // Delete function folder if empty
+      // Delete function folder if exists and empty
+      if (!fs.existsSync(functionName)) {
+        return;
+      }
+
       const items = fs.readdirSync(functionName);
       if (items.length === 0) {
         fs.rmdirSync(functionName);


Test patch:
diff --git a/src/plugins/func/azureFuncPlugin.test.ts b/src/plugins/func/azureFuncPlugin.test.ts
index 9869dbd..5b01606 100644
--- a/src/plugins/func/azureFuncPlugin.test.ts
+++ b/src/plugins/func/azureFuncPlugin.test.ts
@@ -59,7 +59,7 @@ describe("Azure Func Plugin", () => {
       const plugin = new AzureFuncPlugin(sls, options);
       await invokeHook(plugin, "func:add:add");
 
-      expect(sls.cli.log).toBeCalledWith("Function hello already exists");
+      expect(sls.cli.log).lastCalledWith("Function hello already exists", undefined, undefined);
     });
 
     it("creates function handler and updates serverless.yml", async () => {
diff --git a/src/plugins/login/utils/simpleFileTokenCache.test.ts b/src/plugins/login/utils/simpleFileTokenCache.test.ts
index e9383e4..598b2e7 100644
--- a/src/plugins/login/utils/simpleFileTokenCache.test.ts
+++ b/src/plugins/login/utils/simpleFileTokenCache.test.ts
@@ -13,13 +13,15 @@ describe("Simple File Token Cache", () => {
     subscriptions: []
   };
 
+  beforeEach(() => {
+    mockFs();
+  });
+
   afterEach(() => {
     mockFs.restore();
   });
 
   it("Creates a load file on creation if none", () => {
-    mockFs();
-
     const writeFileSpy = jest.spyOn(fs, "writeFileSync");
     new SimpleFileTokenCache(tokenFilePath);
 
@@ -38,8 +40,6 @@ describe("Simple File Token Cache", () => {
   it("Create .azure default directory if it doesn't exist", () => {
     const expected = path.join(os.homedir(), ".azure");
 
-    mockFs();
-
     const writeFileSpy = jest.spyOn(fs, "writeFileSync");
     writeFileSpy.mockImplementation(() => undefined);
 
@@ -77,13 +77,12 @@ describe("Simple File Token Cache", () => {
   });
 
   it("Saves to file after token is added", () => {
-    mockFs();
-
     const writeFileSpy = jest.spyOn(fs, "writeFileSync");
     const tokenCache = new SimpleFileTokenCache(tokenFilePath);
     const testEntries = MockFactory.createTestTokenCacheEntries();
-
-    tokenCache.add(testEntries);
+    
+    const callback = jest.fn();
+    tokenCache.add(testEntries, callback);
 
     const expected = {
       entries: testEntries,
@@ -95,12 +94,11 @@ describe("Simple File Token Cache", () => {
       tokenFilePath,
       JSON.stringify(expected)
     );
+    expect(callback).toBeCalled();
     writeFileSpy.mockRestore();
   });
 
   it("Saves to file after subscription is added", () => {
-    mockFs();
-
     const writeFileSpy = jest.spyOn(fs, "writeFileSync");
     const testFileCache = new SimpleFileTokenCache(tokenFilePath);
     const testSubs = MockFactory.createTestSubscriptions();
@@ -153,7 +151,8 @@ describe("Simple File Token Cache", () => {
     const testEntries = MockFactory.createTestTokenCacheEntries();
 
     testFileCache.addSubs(testSubs);
-    testFileCache.remove(testEntries);
+    const callback = jest.fn();
+    testFileCache.remove(testEntries, callback);
 
     const expected = {
       entries: [],
@@ -164,6 +163,7 @@ describe("Simple File Token Cache", () => {
       tokenFilePath,
       JSON.stringify(expected)
     );
+    expect(callback).toBeCalled();
     writeFileSpy.mockRestore();
   });
 
@@ -182,4 +182,33 @@ describe("Simple File Token Cache", () => {
     expect(cb).toBeCalledWith(null, result);
     expect(result).toEqual([]);
   });
+
+  it("lists subscriptions", () => {
+    const testFileCache = new SimpleFileTokenCache(tokenFilePath);
+    const testSubs = MockFactory.createTestSubscriptions();
+    testFileCache.addSubs(testSubs);
+    expect(testFileCache.listSubscriptions()).toEqual(testSubs);
+  });
+
+  it("clears cache", () => {
+    const writeFileSpy = jest.spyOn(fs, "writeFileSync");
+
+    const testFileCache = new SimpleFileTokenCache(tokenFilePath);
+    const testSubs = MockFactory.createTestSubscriptions();
+    testFileCache.addSubs(testSubs);
+
+    const testEntries = MockFactory.createTestTokenCacheEntries();
+    testFileCache.add(testEntries);
+
+    expect(testFileCache.isEmpty()).toBe(false);
+    expect(testFileCache.listSubscriptions()).toEqual(testSubs);
+    
+    const callback = jest.fn();
+    testFileCache.clear(callback);
+    
+    expect(callback).toBeCalled();
+    expect(testFileCache.listSubscriptions()).toEqual([]);
+    expect(testFileCache.isEmpty()).toBe(true);
+    expect(writeFileSpy).toBeCalled();
+  });
 });
diff --git a/src/services/armService.test.ts b/src/services/armService.test.ts
index d2baa30..c0283cc 100644
--- a/src/services/armService.test.ts
+++ b/src/services/armService.test.ts
@@ -167,7 +167,16 @@ describe("Arm Service", () => {
         template: MockFactory.createTestArmTemplate()
       };
       await service.deployTemplate(deployment);
-      expect(Deployments.prototype.createOrUpdate).not.toBeCalled()
+      expect(Deployments.prototype.createOrUpdate).not.toBeCalled();
+    });
+
+    it("Does not crash if deployment parameters are undefined", async () => {
+      const deployment: ArmDeployment = {
+        parameters: undefined,
+        template: MockFactory.createTestArmTemplate(),
+      };
+      await service.deployTemplate(deployment);
+      expect(Deployments.prototype.createOrUpdate).toBeCalled();
     });
 
     it("Does not deploy if identity is only difference between deployments", async () => {
diff --git a/src/services/funcService.test.ts b/src/services/funcService.test.ts
index fd0d9d1..c36fdd7 100644
--- a/src/services/funcService.test.ts
+++ b/src/services/funcService.test.ts
@@ -52,7 +52,7 @@ describe("Azure Func Service", () => {
       const service = createService(sls, options);
       await service.add();
 
-      expect(sls.cli.log).toBeCalledWith("Function hello already exists");
+      expect(sls.cli.log).toBeCalledWith("Function hello already exists", undefined, undefined);
     });
 
     it("creates function handler and updates serverless.yml", async () => {
diff --git a/src/services/functionAppService.test.ts b/src/services/functionAppService.test.ts
index 4ef79f6..248efe5 100644
--- a/src/services/functionAppService.test.ts
+++ b/src/services/functionAppService.test.ts
@@ -33,6 +33,7 @@ describe("Function App Service", () => {
   const deleteFunctionMessage = "delete function success";
   const functions = MockFactory.createTestSlsFunctionConfig();
   const functionsResponse = MockFactory.createTestFunctionsResponse(functions);
+  const getFunctionResponse = "hello function";
 
   const baseUrl = "https://management.azure.com"
   const masterKeyUrl = `https://${app.defaultHostName}/admin/host/systemkeys/_master`;
@@ -43,6 +44,8 @@ describe("Function App Service", () => {
   const listFunctionsUrl = `${baseUrl}${app.id}/functions?api-version=2016-08-01`;
   const appSettingsUrl = `${baseUrl}/subscriptions/${subscriptionId}/resourceGroups/${config.provider.resourceGroup}` +
     `/providers/Microsoft.Web/sites/${app.name}/config/appsettings?api-version=2016-08-01`;
+  // "https://management.azure.comappId/functions/hello?api-version=2016-08-01"
+  const getFunctionUrl = `${baseUrl}${app.id}/functions/hello?api-version=2016-08-01`
 
 
   const appSettings = {
@@ -65,6 +68,8 @@ describe("Function App Service", () => {
     axiosMock.onPut(appSettingsUrl).reply(200, appSettings)
     // List Functions
     axiosMock.onGet(listFunctionsUrl).reply(200, { value: functionsResponse });
+    // Get Function
+    axiosMock.onGet(getFunctionUrl).reply(200, { value: getFunctionResponse })
     // Delete Function
     for (const funcName of Object.keys(functions)) {
       axiosMock.onDelete(`${baseUrl}${app.id}/functions/${funcName}?api-version=2016-08-01`)
@@ -191,6 +196,28 @@ describe("Function App Service", () => {
     expect(call[2]).toEqual(30000);
   });
 
+  it("gets function with retry", async () => {
+    const originalRunWithRetry = Utils.runWithRetry;
+    Utils.runWithRetry = jest.fn(() => {
+      return {
+        data: {
+          value: []
+        }
+      }
+    }) as any;
+
+    const service = createService();
+    await service.getFunction(app, "hello");
+    const runWithRetryCalls = (Utils.runWithRetry as any).mock.calls;
+    Utils.runWithRetry = originalRunWithRetry;
+    expect(runWithRetryCalls).toHaveLength(1);
+    const call = runWithRetryCalls[0];
+    const response = await call[0]();
+    expect(response.data.value).toEqual(getFunctionResponse);
+    expect(call[1]).toEqual(30);
+    expect(call[2]).toEqual(30000);
+  });
+
   describe("Deployments", () => {
     const expectedDeployment: ArmDeployment = {
       parameters: {},
@@ -410,7 +437,7 @@ describe("Function App Service", () => {
         return {
           properties: appSettings
         }
-      });
+      }) as any;
       AzureBlobStorageService.prototype.generateBlobSasTokenUrl = jest.fn(() => Promise.resolve(sasUrl));
     });
 
diff --git a/src/services/invokeService.test.ts b/src/services/invokeService.test.ts
index f4b6b8a..a6c6183 100644
--- a/src/services/invokeService.test.ts
+++ b/src/services/invokeService.test.ts
@@ -1,12 +1,13 @@
 import axios from "axios";
 import MockAdapter from "axios-mock-adapter";
+import configConstants from "../config";
 import { MockFactory } from "../test/mockFactory";
+import { FunctionAppService } from "./functionAppService";
 import { InvokeService } from "./invokeService";
+
 jest.mock("@azure/arm-appservice")
 jest.mock("@azure/arm-resources")
 jest.mock("./functionAppService")
-import { FunctionAppService } from "./functionAppService";
-import configConstants from "../config";
 
 describe("Invoke Service ", () => {
   const app = MockFactory.createTestSite();
@@ -81,4 +82,11 @@ describe("Invoke Service ", () => {
     expect(FunctionAppService.prototype.get).not.toBeCalled();
     expect(FunctionAppService.prototype.getMasterKey).not.toBeCalled();
   });
+
+  it("Does not try to invoke a non-existent function", async () => {
+    const service = new InvokeService(sls, options);
+    const fakeName = "fakeFunction";
+    await service.invoke("GET", fakeName);
+    expect(sls.cli.log).lastCalledWith(`Function ${fakeName} does not exist`, undefined, undefined)
+  });
 });
diff --git a/src/services/packageService.test.ts b/src/services/packageService.test.ts
index 8fac111..31fc345 100644
--- a/src/services/packageService.test.ts
+++ b/src/services/packageService.test.ts
@@ -63,6 +63,21 @@ describe("Package Service", () => {
     rmdirSpy.mockRestore();
   });
 
+  it("does not clean up function folder if non-existent", async () => {
+    mockFs({});
+
+    const unlinkSpy = jest.spyOn(fs, "unlinkSync");
+    const rmdirSpy = jest.spyOn(fs, "rmdirSync");
+
+    packageService.cleanUp();
+
+    expect(unlinkSpy).not.toBeCalled();
+    expect(rmdirSpy).not.toBeCalled();
+
+    unlinkSpy.mockRestore();
+    rmdirSpy.mockRestore();
+  });
+
   it("cleans up function.json but does not delete function folder", async () => {
     const fsConfig = {};