Repository: serverless/serverless-azure-functions
Pull request #471 (2020-05-27 14:45:05)

fix: Update package service to compile .NET for default OS
<!--
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:

Fixes the check for `windows` and `dotnet` before replacing packaging hook to instead compile the artifact. In the case where the `os` had not been defined (defaults to windows), the `ConfigService` had not yet initialized the default value. This moves the logic to the `ConfigService` when making the check to make sure it has been initialized

<!--
Briefly describe the feature if no issue exists for this PR
-->

## How did you implement it:

- Added a `shouldCompileBeforePublish` method in the `ConfigService` that indicates if hook should be replaced
- Replaced logic in plugin constructor to instead initialize a new `ConfigService` and make the call there instead

<!--
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 .NET serverless azure functions project
- Leave the `os` property undefined
- Run `sls package` - the process will spawn a command to `dotnet` to compile the service

<!--
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?:_** NO  
**_Is it a breaking change?:_** NO



Patch:
diff --git a/src/plugins/package/azurePackagePlugin.ts b/src/plugins/package/azurePackagePlugin.ts
index c8a3123..73ad7cc 100644
--- a/src/plugins/package/azurePackagePlugin.ts
+++ b/src/plugins/package/azurePackagePlugin.ts
@@ -6,6 +6,7 @@ import { PackageService } from "../../services/packageService";
 import { AzureBasePlugin } from "../azureBasePlugin";
 import { isCompiledRuntime, BuildMode, FunctionAppOS } from "../../config/runtime";
 import { CompilerService } from "../../services/compilerService"
+import { ConfigService } from "../../services/configService";
 
 export class AzurePackagePlugin extends AzureBasePlugin {
   private bindingsCreated: boolean = false;
@@ -18,7 +19,8 @@ export class AzurePackagePlugin extends AzureBasePlugin {
       "before:webpack:package:packageModules": this.webpack.bind(this),
       "after:package:finalize": this.finalize.bind(this),
     };
-    if (isCompiledRuntime(this.config.provider.runtime) && this.config.provider.os === FunctionAppOS.WINDOWS) {
+    const configService = new ConfigService(serverless, options);
+    if (configService.shouldCompileBeforePublish()) {
       delete this.serverless.pluginManager.hooks["package:createDeploymentArtifacts"]
       this.hooks["package:createDeploymentArtifacts"] = this.compileArtifact.bind(this);
     }
diff --git a/src/services/configService.ts b/src/services/configService.ts
index e8a802d..7dd0e2f 100644
--- a/src/services/configService.ts
+++ b/src/services/configService.ts
@@ -1,7 +1,7 @@
 import Serverless from "serverless";
 import Service from "serverless/classes/Service";
 import { CliCommand, CliCommandFactory } from "../config/cliCommandFactory";
-import { BuildMode, FunctionAppOS, getRuntimeLanguage, isNodeRuntime, isPythonRuntime, Runtime, RuntimeLanguage, supportedRuntimes, supportedRuntimeSet } from "../config/runtime";
+import { BuildMode, FunctionAppOS, getRuntimeLanguage, isNodeRuntime, isPythonRuntime, Runtime, RuntimeLanguage, supportedRuntimes, supportedRuntimeSet, isCompiledRuntime } from "../config/runtime";
 import { DeploymentConfig, ServerlessAzureConfig, ServerlessAzureFunctionConfig } from "../models/serverless";
 import { constants } from "../shared/constants";
 import { Utils } from "../shared/utils";
@@ -152,6 +152,10 @@ export class ConfigService {
     return this.cliCommandFactory.getCommand(`${runtime}-${mode}`);
   }
 
+  public shouldCompileBeforePublish(): boolean {
+    return isCompiledRuntime(this.config.provider.runtime) && this.config.provider.os === FunctionAppOS.WINDOWS;
+  }
+
   /**
    * Set any default values required for service
    * @param config Current Serverless configuration


Test patch:
diff --git a/src/plugins/package/azurePackagePlugin.test.ts b/src/plugins/package/azurePackagePlugin.test.ts
index dceef18..7035a2a 100644
--- a/src/plugins/package/azurePackagePlugin.test.ts
+++ b/src/plugins/package/azurePackagePlugin.test.ts
@@ -35,6 +35,18 @@ describe("Azure Package Plugin", () => {
     expect(plugin.hooks["package:createDeploymentArtifacts"]).toBeUndefined();
   });
 
+  it("replaces default packaging hook if running dotnet without specifying os", async () => {
+    const service = MockFactory.createTestService();
+    (service as any as ServerlessAzureConfig).provider.runtime = Runtime.DOTNET31;
+    (service as any as ServerlessAzureConfig).provider.os = undefined;
+    const dotnetSls = MockFactory.createTestServerless({ service })
+    const dotnetWindowsPlugin = new AzurePackagePlugin(dotnetSls, MockFactory.createTestServerlessOptions());
+    await invokeHook(dotnetWindowsPlugin, "package:createDeploymentArtifacts");
+    expect(CompilerService.prototype.build).toBeCalledWith(BuildMode.RELEASE);
+    // Default plugins should have this hook undefined because it's created by sls core
+    expect(plugin.hooks["package:createDeploymentArtifacts"]).toBeUndefined();
+  });
+
   it("sets creates function bindings before package:setupProviderConfiguration life cycle event", async () => {
     await invokeHook(plugin, "before:package:setupProviderConfiguration");
     expect(PackageService.prototype.createBindings).toBeCalled();
diff --git a/src/services/configService.test.ts b/src/services/configService.test.ts
index a81a555..5168d02 100644
--- a/src/services/configService.test.ts
+++ b/src/services/configService.test.ts
@@ -152,6 +152,30 @@ describe("Config Service", () => {
       new ConfigService(serverless, {} as any);
       expect(setDefaultValues).not.toBeCalled();
     });
+
+    it("indicates that a configuration with default OS should compile before publish", () => {
+      const sls = MockFactory.createTestServerless();
+      sls.service.provider.runtime = Runtime.DOTNET31;
+      sls.service.provider["os"] = undefined;
+      const service = new ConfigService(sls, {} as any);
+      expect(service.shouldCompileBeforePublish()).toBe(true);
+    });
+
+    it("indicates that a configuration with windows OS should compile before publish", () => {
+      const sls = MockFactory.createTestServerless();
+      sls.service.provider.runtime = Runtime.DOTNET31;
+      sls.service.provider["os"] = FunctionAppOS.WINDOWS;
+      const service = new ConfigService(sls, {} as any);
+      expect(service.shouldCompileBeforePublish()).toBe(true);
+    });
+
+    it("indicates that a configuration with linux OS should not compile before publish", () => {
+      const sls = MockFactory.createTestServerless();
+      sls.service.provider.runtime = Runtime.DOTNET31;
+      sls.service.provider["os"] = FunctionAppOS.LINUX;
+      const service = new ConfigService(sls, {} as any);
+      expect(service.shouldCompileBeforePublish()).toBe(false);
+    });
   
     describe("Service Principal Configuration", () => {
       const cliSubscriptionId = "cli sub id";