From 4a6b88cd8b223ea4cd81c58411900d28445fdcf6 Mon Sep 17 00:00:00 2001 From: Maor Leger Date: Wed, 9 Jun 2021 10:56:54 -0700 Subject: [PATCH] [KeyVault] - Return void from delete methods (#15642) ## What -KeyVaultAccessControlClient#deleteRoleAssignment changes to return `Promise` instead of `Promise` -KeyVaultAccessControlClient#deleteRoleDefinition changes to return `Promise` instead of `Promise` ## 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` today we will be ready to change to corev2 tomorrow without introducing a breaking change to these methods. --- sdk/keyvault/keyvault-admin/CHANGELOG.md | 2 +- .../review/keyvault-admin.api.md | 5 ++--- .../keyvault-admin/src/accessControlClient.ts | 19 +++++++++---------- .../test/public/accessControlClient.spec.ts | 6 ++---- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/sdk/keyvault/keyvault-admin/CHANGELOG.md b/sdk/keyvault/keyvault-admin/CHANGELOG.md index 27cb010cd737..36d775718d9d 100644 --- a/sdk/keyvault/keyvault-admin/CHANGELOG.md +++ b/sdk/keyvault/keyvault-admin/CHANGELOG.md @@ -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) diff --git a/sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md b/sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md index 4e741b3c5364..dfcc077593f7 100644 --- a/sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md +++ b/sdk/keyvault/keyvault-admin/review/keyvault-admin.api.md @@ -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 @@ -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; - deleteRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleAssignmentOptions): Promise; - deleteRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleDefinitionOptions): Promise; + deleteRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleAssignmentOptions): Promise; + deleteRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: DeleteRoleDefinitionOptions): Promise; getRoleAssignment(roleScope: KeyVaultRoleScope, name: string, options?: GetRoleAssignmentOptions): Promise; getRoleDefinition(roleScope: KeyVaultRoleScope, name: string, options?: GetRoleDefinitionOptions): Promise; listRoleAssignments(roleScope: KeyVaultRoleScope, options?: ListRoleAssignmentsOptions): PagedAsyncIterableIterator; diff --git a/sdk/keyvault/keyvault-admin/src/accessControlClient.ts b/sdk/keyvault/keyvault-admin/src/accessControlClient.ts index 4f5c246fa469..00076f18f6be 100644 --- a/sdk/keyvault/keyvault-admin/src/accessControlClient.ts +++ b/sdk/keyvault/keyvault-admin/src/accessControlClient.ts @@ -7,8 +7,7 @@ import { isTokenCredential, signingPolicy, createPipelineFromOptions, - InternalPipelineOptions, - RestResponse + InternalPipelineOptions } from "@azure/core-http"; import { PagedAsyncIterableIterator } from "@azure/core-paging"; @@ -178,10 +177,10 @@ export class KeyVaultAccessControlClient { roleScope: KeyVaultRoleScope, name: string, options: DeleteRoleAssignmentOptions = {} - ): Promise { - return withTrace("deleteRoleAssignment", options, (updatedOptions) => - this.client.roleAssignments.delete(this.vaultUrl, roleScope, name, updatedOptions) - ); + ): Promise { + return withTrace("deleteRoleAssignment", options, async (updatedOptions) => { + await this.client.roleAssignments.delete(this.vaultUrl, roleScope, name, updatedOptions); + }); } /** @@ -487,9 +486,9 @@ export class KeyVaultAccessControlClient { roleScope: KeyVaultRoleScope, name: string, options: DeleteRoleDefinitionOptions = {} - ): Promise { - return withTrace("deleteRoleDefinition", options, (updatedOptions) => - this.client.roleDefinitions.delete(this.vaultUrl, roleScope, name, updatedOptions) - ); + ): Promise { + return withTrace("deleteRoleDefinition", options, async (updatedOptions) => { + await this.client.roleDefinitions.delete(this.vaultUrl, roleScope, name, updatedOptions); + }); } } diff --git a/sdk/keyvault/keyvault-admin/test/public/accessControlClient.spec.ts b/sdk/keyvault/keyvault-admin/test/public/accessControlClient.spec.ts index f77773930c0f..7599d4e74fd8 100644 --- a/sdk/keyvault/keyvault-admin/test/public/accessControlClient.spec.ts +++ b/sdk/keyvault/keyvault-admin/test/public/accessControlClient.spec.ts @@ -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")); }); }); @@ -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() {