Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

fix: Default operation names no longer cause collisions #332

Merged
merged 2 commits into from
Sep 12, 2019
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
2 changes: 1 addition & 1 deletion src/armTemplates/compositeArmTemplate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArmResourceTemplate, ArmResourceTemplateGenerator, ArmParameters, ArmParamType } from "../models/armTemplates";
import { ArmResourceTemplate, ArmResourceTemplateGenerator, ArmParameters } from "../models/armTemplates";
import { ServerlessAzureConfig } from "../models/serverless";
import { AzureNamingService } from "../services/namingService";
import { Guard } from "../shared/guard";
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/invoke/azureInvokePlugin.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fs from "fs";
import { isAbsolute, join } from "path";
import { isAbsolute } from "path";
import Serverless from "serverless";
import { InvokeService } from "../../services/invokeService";
import { AzureBasePlugin } from "../azureBasePlugin";
Expand Down
19 changes: 13 additions & 6 deletions src/services/apimService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,11 @@ describe("APIM Service", () => {

it("infers APIM operation configuration from HTTP binding", async () => {
const functions = MockFactory.createTestSlsFunctionConfig();

// Remove APIM operation configuration
delete functions.hello.apim;
delete functions.goodbye.apim;

Object.assign(serverless.service, { functions });

let apimResource: ApiManagementServiceResource = {
Expand Down Expand Up @@ -425,11 +430,12 @@ describe("APIM Service", () => {
resourceGroupName,
serviceName,
apiName,
"hello",
"GET-hello",
{
displayName: "hello",
name: "GET-hello",
displayName: "hello (GET)",
description: "",
method: "get",
method: "GET",
urlTemplate: "hello",
templateParameters: [],
responses: [],
Expand All @@ -439,11 +445,12 @@ describe("APIM Service", () => {
resourceGroupName,
serviceName,
apiName,
"goodbye",
"GET-goodbye",
{
displayName: "goodbye",
name: "GET-goodbye",
displayName: "goodbye (GET)",
description: "",
method: "get",
method: "GET",
urlTemplate: "goodbye",
templateParameters: [],
responses: [],
Expand Down
20 changes: 13 additions & 7 deletions src/services/apimService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class ApimService extends BaseService {
return null;
}

this.log("-> Deploying API Operations");
this.log(`-> Deploying API Operations: ${this.apimConfig.name}`);

const deployApiTasks = this.serverless.service
.getAllFunctions()
Expand Down Expand Up @@ -144,12 +144,17 @@ export class ApimService extends BaseService {

const httpConfig = httpEvent["x-azure-settings"];

// Set to GET method by default
if (!httpConfig.methods) {
httpConfig.methods = ["GET"];
}

// Infer APIM operation configuration from HTTP event if not already set
if (!functionConfig.apim) {
const operations = httpConfig.methods.map((method) => {
return {
name: functionConfig.name,
displayName: functionConfig.name,
name: `${method}-${functionConfig.name}`,
displayName: `${functionConfig.name} (${method})`,
urlTemplate: httpConfig.route || functionConfig.name,
method: method,
templateParameters: this.getTemplateParameters(httpConfig.route)
Expand Down Expand Up @@ -214,7 +219,7 @@ export class ApimService extends BaseService {
});

if (this.apimConfig.cors) {
this.log("-> Deploying CORS policy");
this.log(`-> Deploying CORS policy: ${apiContract.name}`);

await this.apimClient.apiPolicy.createOrUpdate(this.resourceGroup, this.apimConfig.name, apiContract.name, {
format: "rawxml",
Expand Down Expand Up @@ -273,6 +278,7 @@ export class ApimService extends BaseService {
const client = new ApiManagementClient(this.credentials, this.subscriptionId);

const operationConfig: OperationContract = {
name: operation.name || functionName,
displayName: operation.displayName || functionName,
description: operation.description || "",
method: operation.method,
Expand All @@ -284,17 +290,17 @@ export class ApimService extends BaseService {
// Ensure a single path seperator in the operation path
const operationPath = `/${api.path}/${operationConfig.urlTemplate}`.replace(/\/+/g, "/");
const operationUrl = `${resource.gatewayUrl}${operationPath}`;
this.log(`--> ${functionName}: [${operationConfig.method.toUpperCase()}] ${operationUrl}`);
this.log(`--> ${operationConfig.name}: [${operationConfig.method.toUpperCase()}] ${operationUrl}`);

const result = await client.apiOperation.createOrUpdate(
this.resourceGroup,
this.apimConfig.name,
api.name,
functionName,
operationConfig.name,
operationConfig,
);

await client.apiOperationPolicy.createOrUpdate(this.resourceGroup, this.apimConfig.name, api.name, functionName, {
await client.apiOperationPolicy.createOrUpdate(this.resourceGroup, this.apimConfig.name, api.name, operationConfig.name, {
format: "rawxml",
value: this.createApiOperationXmlPolicy(backend.name),
});
Expand Down
29 changes: 4 additions & 25 deletions src/services/armService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ describe("Arm Service", () => {
await expect(service.deployTemplate(deployment))
.rejects
.toThrowError(
new RegExp(`.*${errorPattern}.*`,"s")
new RegExp(`.*${errorPattern}.*`, "s")
);
});

Expand Down Expand Up @@ -401,39 +401,18 @@ describe("Arm Service", () => {
expect(call[1]).toMatch(expectedDeploymentNameRegex);
expect(call[2]).toEqual(expectedDeployment);
});

it("Throws original error when there has not been a previous deployment", async () => {
const originalError = new Error("original error message");
Deployments.prototype.createOrUpdate = jest.fn(() => Promise.reject(originalError));
const previousDeploymentError: DeploymentExtendedError = {
code: "DeploymentFailed",
message: "At least one resource deployment operation failed. Please list deployment operations for details. Please see https://aka.ms/arm-debug for usage details.",
details: [
{
code: "ServiceAlreadyExists",
message: "Api service already exists: abc-123-apim"
},
{
code: "StorageAccountAlreadyTaken",
message: "The storage account named ABC123 is already taken."
}
]
}
ResourceService.prototype.getPreviousDeployment = jest.fn(() => Promise.resolve(undefined)) as any;

const deployment: ArmDeployment = {
parameters: MockFactory.createTestParameters(),
template: MockFactory.createTestArmTemplate()
};
deployment.parameters.param1.value = "3"
const { code, message, details } = previousDeploymentError;
let errorPattern = [
code,
message,
details[0].code,
details[0].message,
details[1].code,
details[1].message
].join(".*")

await expect(service.deployTemplate(deployment))
.rejects
.toThrowError(originalError);
Expand Down