Skip to content

Commit

Permalink
Create lro-location-header rule to disable LroLocationHeader Lint…
Browse files Browse the repository at this point in the history
…Diff rule (#514)

Closes Azure/typespec-azure-pr#3896

**REST API Specs** (9 violations)
Azure/azure-rest-api-specs#28505

---------

Co-authored-by: Timothee Guerin <[email protected]>
  • Loading branch information
tjprescott and timotheeguerin authored Mar 28, 2024
1 parent 8d9d99a commit f1d30de
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

Add `lro-location-header` rule.
1 change: 1 addition & 0 deletions docs/libraries/azure-resource-manager/reference/linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator` | Each resource interface must have an @armResourceOperations decorator. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb` | Actions must be HTTP Post operations. |
| `@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. |
| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](/libraries/azure-resource-manager/rules/lro-location-header.md) | A 202 response should include a Location response header. |
| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Azure services should not use enums. |
| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. |
| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. |
Expand Down
27 changes: 27 additions & 0 deletions docs/libraries/azure-resource-manager/rules/lro-location-header.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
title: lro-location-header
---

```text title=- Full name-
@azure-tools/typespec-azure-resource-manager/lro-location-header
```

Long-running (LRO) operations with 202 responses must have a "Location" response header.

#### ❌ Incorrect

```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteWithoutOkAsync<Employee, EmployeeProperties, LroHeaders = {}>;
}
```

#### ✅ Correct

```tsp
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteWithoutOkAsync<Employee, EmployeeProperties>;
}
```
1 change: 1 addition & 0 deletions packages/typespec-azure-resource-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator` | Each resource interface must have an @armResourceOperations decorator. |
| `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb` | Actions must be HTTP Post operations. |
| `@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation` | Tenant and Extension resources should not define a list by subscription operation. |
| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/lro-location-header) | A 202 response should include a Location response header. |
| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/missing-x-ms-identifiers) | Azure services should not use enums. |
| `@azure-tools/typespec-azure-resource-manager/no-response-body` | The body of 202 response should be empty. |
| `@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint` | Check for missing Operations interface. |
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-azure-resource-manager/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { coreOperationsRule } from "./rules/core-operations.js";
import { deleteOperationMissingRule } from "./rules/delete-operation.js";
import { envelopePropertiesRules } from "./rules/envelope-properties.js";
import { listBySubscriptionRule } from "./rules/list-operation.js";
import { lroLocationHeaderRule } from "./rules/lro-location-header.js";
import { missingXmsIdentifiersRule } from "./rules/missing-x-ms-identifiers.js";
import { noResponseBodyRule } from "./rules/no-response-body.js";
import { operationsInterfaceMissingRule } from "./rules/operations-interface-missing.js";
Expand Down Expand Up @@ -51,6 +52,7 @@ const rules = [
interfacesRule,
invalidActionVerbRule,
listBySubscriptionRule,
lroLocationHeaderRule,
missingXmsIdentifiersRule,
noResponseBodyRule,
operationsInterfaceMissingRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { ModelProperty, Operation, createRule } from "@typespec/compiler";
import { getHttpOperation } from "@typespec/http";

function getCaseInsensitiveHeader(
headers: Record<string, ModelProperty> | undefined,
key: string
): string | undefined {
if (!headers) {
return undefined;
}
for (const header of Object.keys(headers)) {
if (header.toLowerCase() === key.toLowerCase()) {
return header;
}
}
return undefined;
}

/**
* Ensure that LRO 202 responses have a Location Header.
*/
export const lroLocationHeaderRule = createRule({
name: "lro-location-header",
severity: "warning",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/lro-location-header",
description: "A 202 response should include a Location response header.",
messages: {
default: `A 202 response should include a Location response header.`,
},
create(context) {
return {
operation: (op: Operation) => {
const [httpOperation, _] = getHttpOperation(context.program, op);
const responses = httpOperation.responses;
for (const response of responses) {
if (response.statusCodes !== 202) {
continue;
}
for (const resp of response.responses) {
if (getCaseInsensitiveHeader(resp.headers, "Location") === undefined) {
context.reportDiagnostic({
target: op,
});
return;
}
}
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import {
BasicTestRunner,
LinterRuleTester,
createLinterRuleTester,
} from "@typespec/compiler/testing";
import { beforeEach, it } from "vitest";
import { lroLocationHeaderRule } from "../../src/rules/lro-location-header.js";
import { createAzureResourceManagerTestRunner } from "../test-host.js";

let runner: BasicTestRunner;
let tester: LinterRuleTester;

beforeEach(async () => {
runner = await createAzureResourceManagerTestRunner();
tester = createLinterRuleTester(
runner,
lroLocationHeaderRule,
"@azure-tools/typespec-azure-resource-manager"
);
});

it("Emits a warning for LRO 202 response that does not contain a Location header", async () => {
await tester
.expect(
`
@armProviderNamespace
@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1)
namespace Microsoft.Contoso;
model Employee is ProxyResource<EmployeeProperties> {
@doc("Name of employee")
@pattern("^[a-zA-Z0-9-]{3,24}$")
@key("employeeName")
@path
@segment("employees")
name: string;
}
model EmployeeProperties {}
@armResourceOperations
interface Employees {
@armResourceDelete(Employee)
delete is ArmResourceDeleteWithoutOkAsync<Employee, EmployeeProperties, LroHeaders = {}>;
}
`
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-azure-resource-manager/lro-location-header",
});
});

it("Emits a warning for custom 202 response that does not contain a Location header", async () => {
await tester
.expect(
`
@armProviderNamespace
@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1)
namespace Microsoft.Contoso;
model Employee is ProxyResource<EmployeeProperties> {
@doc("Name of employee")
@pattern("^[a-zA-Z0-9-]{3,24}$")
@key("employeeName")
@path
@segment("employees")
name: string;
}
model EmployeeProperties {}
@armResourceOperations
interface Employees {
@armResourceDelete(Employee)
delete(): { @statusCode _: 202 };
}
`
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-azure-resource-manager/lro-location-header",
});
});

it("Does not emit a warning for LRO 202 response that contains the Location response header", async () => {
await tester
.expect(
`
@armProviderNamespace
@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1)
namespace Microsoft.Contoso;
model Employee is ProxyResource<EmployeeProperties> {
@doc("Name of employee")
@pattern("^[a-zA-Z0-9-]{3,24}$")
@key("employeeName")
@path
@segment("employees")
name: string;
}
model EmployeeProperties {}
@armResourceOperations
interface Employees {
delete is ArmResourceDeleteWithoutOkAsync<Employee, EmployeeProperties>;
}`
)
.toBeValid();
});

0 comments on commit f1d30de

Please sign in to comment.