From 70dbc495bacf3c0ae36be06d017eaf4165087464 Mon Sep 17 00:00:00 2001 From: Xiaoning Liu Date: Thu, 2 Jul 2020 16:16:45 +0800 Subject: [PATCH] [Storage] Add comments for recursive ACL parameters; Resolve review comments --- .../storage-file-datalake/CHANGELOG.md | 2 +- .../review/storage-file-datalake.api.md | 17 +++--- .../storage-file-datalake/src/clients.ts | 15 ++--- .../storage-file-datalake/src/models.ts | 57 +++++++++++++++++-- .../test/node/pathclient.spec.ts | 20 +++---- 5 files changed, 78 insertions(+), 33 deletions(-) diff --git a/sdk/storage/storage-file-datalake/CHANGELOG.md b/sdk/storage/storage-file-datalake/CHANGELOG.md index 6a948bf803a0..53a44c5ea20c 100644 --- a/sdk/storage/storage-file-datalake/CHANGELOG.md +++ b/sdk/storage/storage-file-datalake/CHANGELOG.md @@ -5,7 +5,7 @@ - Increased the maximum block size for file from 100MiB to 4000MiB(~4GB). And thereby supporting ~200TB maximum size for file. - Added more mappings for Blob and DFS endpoints. [issue #8744](https://github.com/Azure/azure-sdk-for-js/issues/8744). - Added convenience methods `createIfNotExists`, `deleteIfExists` to `DataLakeFileSystemClient`, `DataLakePathClient`, `DataLakeDirectoryClient`, and `DataLakeFileClient`. -- Supported set access control list recursively. +- Added support to set access control list recursively. ## 12.0.1 (2020.05) diff --git a/sdk/storage/storage-file-datalake/review/storage-file-datalake.api.md b/sdk/storage/storage-file-datalake/review/storage-file-datalake.api.md index f9997cf2c078..13f72fdd5ad3 100644 --- a/sdk/storage/storage-file-datalake/review/storage-file-datalake.api.md +++ b/sdk/storage/storage-file-datalake/review/storage-file-datalake.api.md @@ -56,7 +56,7 @@ export interface AccessControlChanges { aggregateCounters: AccessControlChangeCounters; batchCounters: AccessControlChangeCounters; batchFailures: AccessControlChangeFailure[]; - continuation?: string; + continuationToken?: string; } // @public (undocumented) @@ -874,8 +874,10 @@ export interface PathAccessControl { } // @public (undocumented) -export interface PathAccessControlItem extends RemovePathAccessControlItem { - // (undocumented) +export interface PathAccessControlItem { + accessControlType: AccessControlType; + defaultScope: boolean; + entityId: string; permissions: RolePermissions; } @@ -891,14 +893,14 @@ export interface PathAppendDataHeaders { export interface PathChangeAccessControlRecursiveOptions extends CommonOptions { abortSignal?: AbortSignalLike; batchSize?: number; - continuation?: string; + continuationToken?: string; maxBatches?: number; onProgress?: (progress: AccessControlChanges) => void; } // @public export interface PathChangeAccessControlRecursiveResponse { - continuation?: string; + continuationToken?: string; counters: AccessControlChangeCounters; } @@ -1466,12 +1468,9 @@ export interface RawAccessPolicy { // @public (undocumented) export interface RemovePathAccessControlItem { - // (undocumented) accessControlType: AccessControlType; - // (undocumented) defaultScope: boolean; - // (undocumented) - entityId: string; + entityId?: string; } export { RequestPolicy } diff --git a/sdk/storage/storage-file-datalake/src/clients.ts b/sdk/storage/storage-file-datalake/src/clients.ts index 03cdf8d47c24..a2bac0f9180d 100644 --- a/sdk/storage/storage-file-datalake/src/clients.ts +++ b/sdk/storage/storage-file-datalake/src/clients.ts @@ -61,6 +61,7 @@ import { PathSetMetadataResponse, PathSetPermissionsOptions, PathSetPermissionsResponse, + RemovePathAccessControlItem, } from "./models"; import { newPipeline, Pipeline, StoragePipelineOptions } from "./Pipeline"; import { StorageClient } from "./StorageClient"; @@ -148,11 +149,11 @@ export class DataLakePathClient extends StorageClient { changedDirectoriesCount: 0, changedFilesCount: 0 }, - continuation: undefined + continuationToken: undefined }; try { - let continuation = options.continuation; + let continuationToken = options.continuationToken; let batchCounter = 0; let reachMaxBatches = false; do { @@ -160,14 +161,14 @@ export class DataLakePathClient extends StorageClient { ...options, acl: toAclString(acl as PathAccessControlItem[]), maxRecords: options.batchSize, - continuation, + continuation: continuationToken, spanOptions }); batchCounter++; - continuation = response.continuation; + continuationToken = response.continuation; // Update result - result.continuation = continuation; + result.continuationToken = continuationToken; result.counters.failedChangesCount += response.failureCount || 0; result.counters.changedDirectoriesCount += response.directoriesSuccessful || 0; result.counters.changedFilesCount += response.filesSuccessful || 0; @@ -182,14 +183,14 @@ export class DataLakePathClient extends StorageClient { changedFilesCount: response.filesSuccessful || 0 }, aggregateCounters: result.counters, - continuation + continuationToken: continuationToken }; options.onProgress(progress); } reachMaxBatches = options.maxBatches === undefined ? false : batchCounter >= options.maxBatches; - } while (continuation && !reachMaxBatches); + } while (continuationToken && !reachMaxBatches); return result; } catch (e) { diff --git a/sdk/storage/storage-file-datalake/src/models.ts b/sdk/storage/storage-file-datalake/src/models.ts index 3f68458514ec..4db720ba1fd5 100644 --- a/sdk/storage/storage-file-datalake/src/models.ts +++ b/sdk/storage/storage-file-datalake/src/models.ts @@ -3,7 +3,6 @@ import { HttpResponse, TransferProgressEvent } from "@azure/core-http"; import { LeaseAccessConditions, ModifiedAccessConditions, UserDelegationKeyModel } from "@azure/storage-blob"; import { - FileSystemListPathsHeaders, FileSystemListPathsHeaders, PathCreateResponse, PathDeleteResponse, @@ -404,12 +403,58 @@ export interface PathPermissions { export type AccessControlType = "user" | "group" | "mask" | "other"; export interface RemovePathAccessControlItem { + /** + * Indicates whether this is the default entry for the ACL. + * + * @type {boolean} + * @memberof RemovePathAccessControlItem + */ defaultScope: boolean; + /** + * Specifies which role this entry targets. + * + * @type {AccessControlType} + * @memberof RemovePathAccessControlItem + */ accessControlType: AccessControlType; - entityId: string; + /** + * Specifies the entity for which this entry applies. + * Must be omitted for types mask or other. It must also be omitted when the user or group is the owner. + * + * @type {string} + * @memberof RemovePathAccessControlItem + */ + entityId?: string; } -export interface PathAccessControlItem extends RemovePathAccessControlItem { +export interface PathAccessControlItem { + /** + * Indicates whether this is the default entry for the ACL. + * + * @type {boolean} + * @memberof PathAccessControlItem + */ + defaultScope: boolean; + /** + * Specifies which role this entry targets. + * + * @type {AccessControlType} + * @memberof PathAccessControlItem + */ + accessControlType: AccessControlType; + /** + * Specifies the entity for which this entry applies. + * + * @type {string} + * @memberof PathAccessControlItem + */ + entityId: string; + /** + * Access control permissions. + * + * @type {RolePermissions} + * @memberof PathAccessControlItem + */ permissions: RolePermissions; } @@ -519,7 +564,7 @@ export interface PathChangeAccessControlRecursiveOptions extends CommonOptions { * @type {string} * @memberof PathChangeAccessControlRecursiveOptions */ - continuation?: string; + continuationToken?: string; /** * Callback where caller can track progress of the operation * as well as collect paths that failed to change Access Control. @@ -594,7 +639,7 @@ export interface AccessControlChanges { * @type {string} * @memberof AccessControlChanges */ - continuation?: string; + continuationToken?: string; } /** @@ -644,7 +689,7 @@ export interface PathChangeAccessControlRecursiveResponse { * @type {string} * @memberof PathChangeAccessControlRecursiveResponse */ - continuation?: string; + continuationToken?: string; } export interface PathSetPermissionsOptions extends CommonOptions { diff --git a/sdk/storage/storage-file-datalake/test/node/pathclient.spec.ts b/sdk/storage/storage-file-datalake/test/node/pathclient.spec.ts index e35c0f0bb267..b91ddcf6439a 100644 --- a/sdk/storage/storage-file-datalake/test/node/pathclient.spec.ts +++ b/sdk/storage/storage-file-datalake/test/node/pathclient.spec.ts @@ -325,7 +325,7 @@ describe("DataLakePathClient Node.js only", () => { }); }); -describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { +describe.only("DataLakePathClient setAccessControlRecursive Node.js only", () => { let fileSystemName: string; let fileSystemClient: DataLakeFileSystemClient; let fileName: string; @@ -388,7 +388,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { assert.deepStrictEqual(3, result.counters.changedDirectoriesCount); assert.deepStrictEqual(4, result.counters.changedFilesCount); assert.deepStrictEqual(0, result.counters.failedChangesCount); - assert.deepStrictEqual(undefined, result.continuation); + assert.deepStrictEqual(undefined, result.continuationToken); }); it("setAccessControlRecursive should work with options - maxBatches", async () => { @@ -432,7 +432,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { ); assert.deepStrictEqual(1, batchCounter); - assert.notDeepStrictEqual(undefined, result.continuation); + assert.notDeepStrictEqual(undefined, result.continuationToken); }); it("setAccessControlRecursive should work with options - batchSize", async () => { @@ -509,7 +509,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { assert.deepStrictEqual(3, result.counters.changedDirectoriesCount); assert.deepStrictEqual(4, result.counters.changedFilesCount); assert.deepStrictEqual(0, result.counters.failedChangesCount); - assert.deepStrictEqual(undefined, result.continuation); + assert.deepStrictEqual(undefined, result.continuationToken); assert.deepStrictEqual(true, batchCounter > 3); }); @@ -551,7 +551,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { batchSize: 2, onProgress: (progress) => { midProgress = progress; - continuation = progress.continuation; + continuation = progress.continuationToken; aborter.abort(); }, abortSignal: aborter.signal @@ -567,7 +567,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { "user::rwx,user:ec3595d6-2c17-4696-8caa-7e139758d24a:rw-,group::rw-,mask::rwx,other::---" ), { - continuation + continuationToken: continuation } ); @@ -583,7 +583,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { 0, result.counters.failedChangesCount + midProgress!.batchCounters.failedChangesCount ); - assert.deepStrictEqual(undefined, result.continuation); + assert.deepStrictEqual(undefined, result.continuationToken); }); it("updateAccessControlRecursive should work", async () => { @@ -621,7 +621,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { assert.deepStrictEqual(3, result.counters.changedDirectoriesCount); assert.deepStrictEqual(4, result.counters.changedFilesCount); assert.deepStrictEqual(0, result.counters.failedChangesCount); - assert.deepStrictEqual(undefined, result.continuation); + assert.deepStrictEqual(undefined, result.continuationToken); }); it("removeAccessControlRecursive should work", async () => { @@ -659,7 +659,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { assert.deepStrictEqual(3, result.counters.changedDirectoriesCount); assert.deepStrictEqual(4, result.counters.changedFilesCount); assert.deepStrictEqual(0, result.counters.failedChangesCount); - assert.deepStrictEqual(undefined, result.continuation); + assert.deepStrictEqual(undefined, result.continuationToken); const removeResult = await directoryClient.removeAccessControlRecursive( toRemoveAcl( @@ -673,7 +673,7 @@ describe("DataLakePathClient setAccessControlRecursive Node.js only", () => { assert.deepStrictEqual(3, removeResult.counters.changedDirectoriesCount); assert.deepStrictEqual(4, removeResult.counters.changedFilesCount); assert.deepStrictEqual(0, removeResult.counters.failedChangesCount); - assert.deepStrictEqual(undefined, removeResult.continuation); + assert.deepStrictEqual(undefined, removeResult.continuationToken); }); it("setAccessControlRecursive should work with progress failures - TODO", async () => {});