Skip to content

Commit

Permalink
[KeyVault] - Return void from delete methods (#15642)
Browse files Browse the repository at this point in the history
## What

-KeyVaultAccessControlClient#deleteRoleAssignment changes to return `Promise<void>` instead of `Promise<RestResponse>`
-KeyVaultAccessControlClient#deleteRoleDefinition changes to return `Promise<void>` instead of `Promise<RestResponse>`

## Why

Originally we wanted to return the raw response so that customers can access the headers / status code / etc. However, with the 
upcoming effort to core-v2 returning a raw response today would require us to make a breaking change tomorrow as corev2 
does not return the raw response, using a callback instead.

By returning `Promise<void>` today we will be ready to change to corev2 tomorrow without introducing a breaking change to 
these methods.
  • Loading branch information
maorleger authored Jun 9, 2021
1 parent 63b8105 commit 4a6b88c
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 18 deletions.
2 changes: 1 addition & 1 deletion sdk/keyvault/keyvault-admin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
- Renamed `KeyVaultBeginSelectiveRestoreOptions` to `KeyVaultBeginSelectiveKeyRestoreOptions`.
- Renamed `KeyVaultSelectiveRestoreOperationState` to `KeyVaultSelectiveKeyRestoreOperationState`.
- Renamed `KeyVaultSelectiveRestoreResult` to `KeyVaultSelectiveKeyRestoreResult`.
- `deleteRoleAssignment` and `deleteRoleDefinition` will no longer throw an exception when the resource no longer exist, and will return the raw response of the operation.
- `deleteRoleAssignment` and `deleteRoleDefinition` will no longer throw an exception when the resource no longer exist and return no result.

## 4.0.0-beta.3 (2021-04-06)

Expand Down
5 changes: 2 additions & 3 deletions sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import * as coreHttp from '@azure/core-http';
import { PagedAsyncIterableIterator } from '@azure/core-paging';
import { PollerLike } from '@azure/core-lro';
import { PollOperationState } from '@azure/core-lro';
import { RestResponse } from '@azure/core-http';
import { TokenCredential } from '@azure/core-http';

// @public
Expand Down Expand Up @@ -40,8 +39,8 @@ export interface GetRoleDefinitionOptions extends coreHttp.OperationOptions {
export class KeyVaultAccessControlClient {
constructor(vaultUrl: string, credential: TokenCredential, options?: AccessControlClientOptions);
createRoleAssignment(roleScope: KeyVaultRoleScope, name: string, roleDefinitionId: string, principalId: string, options?: CreateRoleAssignmentOptions): Promise<KeyVaultRoleAssignment>;
deleteRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleAssignmentOptions): Promise<RestResponse>;
deleteRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleDefinitionOptions): Promise<RestResponse>;
deleteRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleAssignmentOptions): Promise<void>;
deleteRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleDefinitionOptions): Promise<void>;
getRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: GetRoleAssignmentOptions): Promise<KeyVaultRoleAssignment>;
getRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: GetRoleDefinitionOptions): Promise<KeyVaultRoleDefinition>;
listRoleAssignments(roleScope: KeyVaultRoleScope, options?: ListRoleAssignmentsOptions): PagedAsyncIterableIterator<KeyVaultRoleAssignment>;
Expand Down
19 changes: 9 additions & 10 deletions sdk/keyvault/keyvault-admin/src/accessControlClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import {
isTokenCredential,
signingPolicy,
createPipelineFromOptions,
InternalPipelineOptions,
RestResponse
InternalPipelineOptions
} from "@azure/core-http";
import { PagedAsyncIterableIterator } from "@azure/core-paging";

Expand Down Expand Up @@ -178,10 +177,10 @@ export class KeyVaultAccessControlClient {
roleScope: KeyVaultRoleScope,
name: string,
options: DeleteRoleAssignmentOptions = {}
): Promise<RestResponse> {
return withTrace("deleteRoleAssignment", options, (updatedOptions) =>
this.client.roleAssignments.delete(this.vaultUrl, roleScope, name, updatedOptions)
);
): Promise<void> {
return withTrace("deleteRoleAssignment", options, async (updatedOptions) => {
await this.client.roleAssignments.delete(this.vaultUrl, roleScope, name, updatedOptions);
});
}

/**
Expand Down Expand Up @@ -487,9 +486,9 @@ export class KeyVaultAccessControlClient {
roleScope: KeyVaultRoleScope,
name: string,
options: DeleteRoleDefinitionOptions = {}
): Promise<RestResponse> {
return withTrace("deleteRoleDefinition", options, (updatedOptions) =>
this.client.roleDefinitions.delete(this.vaultUrl, roleScope, name, updatedOptions)
);
): Promise<void> {
return withTrace("deleteRoleDefinition", options, async (updatedOptions) => {
await this.client.roleDefinitions.delete(this.vaultUrl, roleScope, name, updatedOptions);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ describe("KeyVaultAccessControlClient", () => {
});

it("succeeds when deleting a non-existent role definition", async function() {
const response = await client.deleteRoleDefinition(globalScope, "foobar");
assert.equal(404, response._response.status);
await assert.isFulfilled(client.deleteRoleDefinition(globalScope, "foobar"));
});
});

Expand Down Expand Up @@ -277,8 +276,7 @@ describe("KeyVaultAccessControlClient", () => {
});

it("succeeds when deleting a role assignment that doesn't exist", async () => {
const response = await client.deleteRoleAssignment(globalScope, generateFakeUUID());
assert.equal(404, response._response.status);
await assert.isFulfilled(client.deleteRoleAssignment(globalScope, generateFakeUUID()));
});

it("supports tracing", async function() {
Expand Down

0 comments on commit 4a6b88c

Please sign in to comment.