Skip to content
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
1 change: 1 addition & 0 deletions sdk/storage/Azure.Storage.Files.DataLake/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Fixed bug where DataLakeFileSystem.SetAccessPolicy() would throw an exception if signed identifier permissions were not in the correct order.
- Added seekability to DataLakeFileClient.OpenRead().
- Added additional info to exception messages.
- Added DataLakeDirectoryClient.GetPaths().

## 12.5.0-preview.1 (2020-09-30)
- Added support for service version 2020-02-10.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public DataLakeDirectoryClient(System.Uri directoryUri, Azure.Storage.StorageSha
public override Azure.Response<Azure.Storage.Files.DataLake.Models.PathAccessControl> GetAccessControl(bool? userPrincipalName = default(bool?), Azure.Storage.Files.DataLake.Models.DataLakeRequestConditions conditions = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public override System.Threading.Tasks.Task<Azure.Response<Azure.Storage.Files.DataLake.Models.PathAccessControl>> GetAccessControlAsync(bool? userPrincipalName = default(bool?), Azure.Storage.Files.DataLake.Models.DataLakeRequestConditions conditions = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Storage.Files.DataLake.DataLakeFileClient GetFileClient(string fileName) { throw null; }
public virtual Azure.Pageable<Azure.Storage.Files.DataLake.Models.PathItem> GetPaths(bool recursive = false, bool userPrincipalName = false, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.AsyncPageable<Azure.Storage.Files.DataLake.Models.PathItem> GetPathsAsync(bool recursive = false, bool userPrincipalName = false, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual new Azure.Response<Azure.Storage.Files.DataLake.Models.PathProperties> GetProperties(Azure.Storage.Files.DataLake.Models.DataLakeRequestConditions conditions = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public override System.Threading.Tasks.Task<Azure.Response<Azure.Storage.Files.DataLake.Models.PathProperties>> GetPropertiesAsync(Azure.Storage.Files.DataLake.Models.DataLakeRequestConditions conditions = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public virtual Azure.Storage.Files.DataLake.DataLakeDirectoryClient GetSubDirectoryClient(string subdirectoryName) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using Azure.Core;
using Azure.Core.Pipeline;
using Azure.Storage.Blobs.Specialized;
using Azure.Storage.Files.DataLake.Models;
using Metadata = System.Collections.Generic.IDictionary<string, string>;

Expand Down Expand Up @@ -2187,5 +2188,95 @@ public virtual async Task<Response> DeleteSubDirectoryAsync(
}
}
#endregion Delete Sub Directory

#region Get Paths
/// <summary>
/// The <see cref="GetPaths"/> operation returns an async sequence
/// of paths in this directory. Enumerating the paths may make
/// multiple requests to the service while fetching all the values.
///
/// For more information, see
/// <see href="https://docs.microsoft.com/rest/api/storageservices/datalakestoragegen2/path/list">
/// List Path(s)</see>.
/// </summary>
/// <param name="recursive">
/// If "true", all paths are listed; otherwise, only paths at the root of the filesystem are listed.
/// </param>
/// <param name="userPrincipalName">
/// Optional. Valid only when Hierarchical Namespace is enabled for the account. If
/// "true", the user identity values returned in the owner and group fields of each list
/// entry will be transformed from Azure Active Directory Object IDs to User Principal
/// Names. If "false", the values will be returned as Azure Active Directory Object IDs.
/// The default value is false. Note that group and application Object IDs are not translated
/// because they do not have unique friendly names.
/// </param>
/// <param name="cancellationToken">
/// Optional <see cref="CancellationToken"/> to propagate
/// notifications that the operation should be cancelled.
/// </param>
/// <returns>
/// An <see cref="Pageable{PathItem}"/>
/// describing the paths in the directory.
/// </returns>
/// <remarks>
/// A <see cref="RequestFailedException"/> will be thrown if
/// a failure occurs.
/// </remarks>
public virtual Pageable<PathItem> GetPaths(
bool recursive = default,
bool userPrincipalName = default,
Comment on lines +2226 to +2227
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be options bag?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the way it is for DataLakeFileSystemClient.GetPaths() too. I do like the idea of an options bag though, but if we do it here, we should go back and do it for the FileSystemClient too and hide the old one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking about this when I did the implementation, I wanted to see what y'all thought. I'll switch both to options bags.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we think this will add a lot of parameters in the future? You currently have 2 and the informal guideline is roughly 6.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible it could grow, but I don't know that there are any specific plans. I added this based on @kasobol-msft and @amnguye's comments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd follow this if there's no formal guidance. Let's not forget cancellationToken when we count parameters.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That link offers a highly opinionated view that clashes with the rest of our (different highly opinionated 😄) style. In general for questions like this we should get them clarified in the guidelines rather than seek elsewhere so Storage libraries look and feel the same as other Azure services. I checked https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-parameters and it does now list 6 parameters:

Simple methods are methods that take up to six parameters, with most of the parameters being simple BCL primitives. ... Simple methods should follow standard .NET Design Guidelines for parameter list and overload design.

Unless we expect this to more than double, I'd suggest using parameters for now.

Copy link
Copy Markdown
Contributor

@kasobol-msft kasobol-msft Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly the https://azure.github.io/azure-sdk/dotnet_introduction.html#dotnet-parameters gives an example of "simple method" that has 3 parameters (2 BCL + cancellation token). In general I think 6 is too much and I think the sdk guide should trim that to 3 (6 parameter method doesn't look good imo).
Anyway in this case since we 2 params + token so let's follow the guideline then and go back to original design.

CancellationToken cancellationToken = default) =>
new GetPathsAsyncCollection(
FileSystemClient,
Path,
recursive,
userPrincipalName,
$"{nameof(DataLakeDirectoryClient)}.{nameof(GetPaths)}")
.ToSyncCollection(cancellationToken);

/// <summary>
/// The <see cref="GetPaths"/> operation returns an async sequence
/// of paths in this directory. Enumerating the paths may make
/// multiple requests to the service while fetching all the values.
///
/// For more information, see
/// <see href="https://docs.microsoft.com/rest/api/storageservices/datalakestoragegen2/path/list">
/// List Path(s)</see>.
/// </summary>
/// <param name="recursive">
/// If "true", all paths are listed; otherwise, only paths at the root of the filesystem are listed.
/// </param>
/// <param name="userPrincipalName">
/// Optional. Valid only when Hierarchical Namespace is enabled for the account. If
/// "true", the user identity values returned in the owner and group fields of each list
/// entry will be transformed from Azure Active Directory Object IDs to User Principal
/// Names. If "false", the values will be returned as Azure Active Directory Object IDs.
/// The default value is false. Note that group and application Object IDs are not translated
/// because they do not have unique friendly names.
/// </param>
/// <param name="cancellationToken">
/// Optional <see cref="CancellationToken"/> to propagate
/// notifications that the operation should be cancelled.
/// </param>
/// <returns>
/// An <see cref="Pageable{PathItem}"/>
/// describing the paths in the directory.
/// </returns>
/// <remarks>
/// A <see cref="RequestFailedException"/> will be thrown if
/// a failure occurs.
/// </remarks>
public virtual AsyncPageable<PathItem> GetPathsAsync(
bool recursive = default,
bool userPrincipalName = default,
CancellationToken cancellationToken = default) =>
new GetPathsAsyncCollection(
FileSystemClient,
Path,
recursive,
userPrincipalName,
$"{nameof(DataLakeDirectoryClient)}.{nameof(GetPaths)}")
.ToAsyncCollection(cancellationToken);
#endregion Get Paths
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using System.Text.Json;
using System.Collections.Generic;
using Azure.Storage.Shared;
using System.ComponentModel;

namespace Azure.Storage.Files.DataLake
{
Expand Down Expand Up @@ -1213,9 +1214,10 @@ public virtual async Task<Response<FileSystemInfo>> SetMetadataAsync(

#region Get Paths
/// <summary>
/// The <see cref="GetPaths"/> operation returns an async sequence
/// of paths in this file system. Enumerating the paths may make
/// multiple requests to the service while fetching all the values.
/// The <see cref="GetPaths"/>
/// operation returns an async sequence of paths in this file system.
/// Enumerating the paths may make multiple requests to the service
/// while fetching all the values.
///
/// For more information, see
/// <see href="https://docs.microsoft.com/rest/api/storageservices/datalakestoragegen2/path/list">
Expand Down Expand Up @@ -1256,12 +1258,14 @@ public virtual Pageable<PathItem> GetPaths(
this,
path,
recursive,
userPrincipalName).ToSyncCollection(cancellationToken);
userPrincipalName,
$"{nameof(DataLakeFileClient)}.{nameof(GetPaths)}")
.ToSyncCollection(cancellationToken);

/// <summary>
/// The <see cref="GetPathsAsync"/> operation returns an async
/// sequence of paths in this file system. Enumerating the paths may
/// make multiple requests to the service while fetching all the
/// The <see cref="GetPathsAsync(string, bool, bool, CancellationToken)"/>
/// operation returns an async sequence of paths in this file system. Enumerating
/// the paths may make multiple requests to the service while fetching all the
/// values.
///
/// For more information, see
Expand Down Expand Up @@ -1302,15 +1306,18 @@ public virtual AsyncPageable<PathItem> GetPathsAsync(
new GetPathsAsyncCollection(this,
path,
recursive,
userPrincipalName).ToAsyncCollection(cancellationToken);
userPrincipalName,
$"{nameof(DataLakeFileClient)}.{nameof(GetPaths)}")
.ToAsyncCollection(cancellationToken);

/// <summary>
/// The <see cref="GetPathsInternal"/> operation returns a
/// single segment of paths in this file system, starting
/// from the specified <paramref name="continuation"/>. Use an empty
/// <paramref name="continuation"/> to start enumeration from the beginning
/// and the <see cref="PathSegment.Continuation"/> if it's not
/// empty to make subsequent calls to <see cref="GetPathsAsync"/>
/// empty to make subsequent calls to
/// <see cref="GetPathsAsync"/>
/// to continue enumerating the paths segment by segment. Paths are
/// ordered lexicographically by name.
///
Expand Down Expand Up @@ -1342,6 +1349,9 @@ public virtual AsyncPageable<PathItem> GetPathsAsync(
/// An optional value that specifies the maximum number of items to return. If omitted or greater than 5,000,
/// the response will include up to 5,000 items.
/// </param>
/// <param name="operationName">
/// The name of the operation.
/// </param>
/// <param name="async">
/// Whether to invoke the operation asynchronously.
/// </param>
Expand All @@ -1363,6 +1373,7 @@ internal async Task<Response<PathSegment>> GetPathsInternal(
bool userPrincipalName,
string continuation,
int? maxResults,
string operationName,
bool async,
CancellationToken cancellationToken)
{
Expand All @@ -1387,6 +1398,7 @@ internal async Task<Response<PathSegment>> GetPathsInternal(
upn: userPrincipalName,
path: path,
async: async,
operationName: operationName,
cancellationToken: cancellationToken)
.ConfigureAwait(false);

Expand Down
35 changes: 33 additions & 2 deletions sdk/storage/Azure.Storage.Files.DataLake/src/DataLakePathClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,25 @@ namespace Azure.Storage.Files.DataLake
public class DataLakePathClient
{
/// <summary>
/// A <see cref="BlockBlobClient"/> associated with the path;
/// A <see cref="BlockBlobClient"/> associated with the path.
/// </summary>
internal readonly BlockBlobClient _blockBlobClient;

/// <summary>
/// BlobClient
/// A <see cref="BlockBlobClient"/> associated with the path.
/// </summary>
internal virtual BlockBlobClient BlobClient => _blockBlobClient;

/// <summary>
/// A <see cref="DataLakeFileSystemClient"/> associated with Directory's parent File System.
/// </summary>
internal readonly DataLakeFileSystemClient _fileSystemClient;

/// <summary>
/// A <see cref="DataLakeFileSystemClient"/> associated with Directory's parent File System.
/// </summary>
internal virtual DataLakeFileSystemClient FileSystemClient => _fileSystemClient;

/// <summary>
/// The paths's primary <see cref="Uri"/> endpoint.
/// </summary>
Expand Down Expand Up @@ -360,6 +370,13 @@ internal DataLakePathClient(Uri pathUri, HttpPipeline pipeline, DataLakeClientOp
_version = options.Version;
_clientDiagnostics = new ClientDiagnostics(options);
_blockBlobClient = BlockBlobClientInternals.Create(_blobUri, _pipeline, Version.AsBlobsVersion(), _clientDiagnostics);

uriBuilder.DirectoryOrFilePath = null;
_fileSystemClient = new DataLakeFileSystemClient(
uriBuilder.ToDfsUri(),
_pipeline,
Version,
ClientDiagnostics);
}

/// <summary>
Expand Down Expand Up @@ -399,6 +416,13 @@ internal DataLakePathClient(
_pipeline,
Version.AsBlobsVersion(),
_clientDiagnostics);

uriBuilder.DirectoryOrFilePath = null;
_fileSystemClient = new DataLakeFileSystemClient(
uriBuilder.ToDfsUri(),
pipeline,
version,
clientDiagnostics);
}

internal DataLakePathClient(
Expand All @@ -423,6 +447,13 @@ internal DataLakePathClient(
_pipeline,
Version.AsBlobsVersion(),
_clientDiagnostics);

uriBuilder.DirectoryOrFilePath = null;
_fileSystemClient = new DataLakeFileSystemClient(
uriBuilder.ToDfsUri(),
pipeline,
version,
clientDiagnostics);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,11 @@ public string DirectoryOrFilePath
set
{
ResetUri();
if (value == "/")
if (value == null)
{
_directoryOrFilePath = null;
}
else if (value == "/")
{
_directoryOrFilePath = value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ internal class GetPathsAsyncCollection : StorageCollectionEnumerator<PathItem>
{
private readonly DataLakeFileSystemClient _client;
private readonly string _path;
private readonly bool _recursive;
private readonly bool _upn;
private readonly bool? _recursive;
private readonly bool? _upn;
private readonly string _operationName;

public GetPathsAsyncCollection(
DataLakeFileSystemClient client,
string path,
bool recursive,
bool upn)
bool? recursive,
bool? upn,
string operationName)
{
_client = client;
_path = path;
_recursive = recursive;
_upn = upn;
_operationName = operationName;
}

public override async ValueTask<Page<PathItem>> GetNextPageAsync(
Expand All @@ -34,10 +37,11 @@ public override async ValueTask<Page<PathItem>> GetNextPageAsync(
{
Response<PathSegment> response = await _client.GetPathsInternal(
_path,
_recursive,
_upn,
_recursive.GetValueOrDefault(),
_upn.GetValueOrDefault(),
continuationToken,
pageSizeHint,
_operationName,
async,
cancellationToken).ConfigureAwait(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,5 +434,19 @@ public async ValueTask DisposeAsync()
}
}
}
}

public string[] PathNames
=> new[]
{
"foo",
"bar",
"baz",
"baz/bar",
"foo/foo",
"foo/bar",
"baz/foo",
"baz/foo/bar",
"baz/bar/foo"
};
}
}
Loading