Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support blob hierarchy operations with a custom delimiter #10192

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Support blob hierarchy operations with a custom delimiter #10192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 17 additions & 3 deletions src/NuGet.Services.Storage/AzureStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ public class AzureStorage : Storage
private readonly ILogger<AzureStorage> _logger;
private readonly BlobContainerClient _directory;
private readonly string _path;
private readonly string _delimiter;

public AzureStorage(
BlobServiceClient account,
string containerName,
string path,
string delimiter,
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the only delimiter allowed for storage accounts is /, I'd say, there is no point to parameterize it. Having a constant should be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

By any chance it would be \ for non-Windows machines? Not sure about actual implementation of Azure storage machines, under the hood it's it's just another VM from my understanding.

Uri baseAddress,
bool initializeContainer,
bool enablePublicAccess,
Expand All @@ -38,6 +40,7 @@ public AzureStorage(
logger)
{
_path = path;
_delimiter = delimiter;
Copy link
Contributor

Choose a reason for hiding this comment

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

If no delimiter is passed, the blob listing will be flat, and you won't get hierarchical groupings like folders in the results. Now with delimiter, it simulates hierarchy. Should its behavior change have a boolean flag to enable or disable the behavior?

}

private AzureStorage(
Expand Down Expand Up @@ -171,8 +174,14 @@ public override IEnumerable<StorageListItem> List(bool getMetadata)
blobTraits |= BlobTraits.Metadata;
}

foreach (BlobHierarchyItem blob in _directory.GetBlobsByHierarchy(traits: blobTraits, prefix: _path))
foreach (BlobHierarchyItem blob in _directory.GetBlobsByHierarchy(traits: blobTraits, delimiter: _delimiter, prefix: _path))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm running some tests and without the delimiter specified, GetBlobsByHierarchy returns a list of blobs having certain prefix, complete, regardless of directory nesting. On storage accounts with hierarchical namespace enabled, it also includes directories themselves.

When delimiter is passed, it only returns immediate children under prefix AND if prefix ends in /. If prefix does not end with /, it returns a single item. In either case it does not return all blobs with a prefix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest introducing a separate method for listing only immediate children, so there is no ambiguity/confusion as to what it returns.

{
// if prefix then the blob is a directory and we should skip it
if (blob.IsPrefix)
{
continue;
}

yield return GetStorageListItem(_directory.GetBlockBlobClient(blob.Blob.Name));
}
}
Expand All @@ -187,8 +196,13 @@ public override async Task<IEnumerable<StorageListItem>> ListAsync(bool getMetad

var blobList = new List<StorageListItem>();

await foreach (BlobHierarchyItem blob in _directory.GetBlobsByHierarchyAsync(traits: blobTraits, prefix: _path))
await foreach (BlobHierarchyItem blob in _directory.GetBlobsByHierarchyAsync(traits: blobTraits, delimiter: _delimiter, prefix: _path, cancellationToken: cancellationToken))
{
// if prefix then the blob is a directory and we should skip it
if (blob.IsPrefix)
{
continue;
}
blobList.Add(await GetStorageListItemAsync(_directory.GetBlockBlobClient(blob.Blob.Name)));
}

Expand Down Expand Up @@ -364,7 +378,7 @@ protected override async Task<StorageContent> OnLoad(Uri resourceUri, Cancellati
originalStream.Position = 0;
var propertiesResponse = await blob.GetPropertiesAsync();
var properties = propertiesResponse.Value;

MemoryStream content;

if (properties.ContentEncoding == "gzip")
Expand Down
8 changes: 6 additions & 2 deletions src/NuGet.Services.Storage/AzureStorageFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand All @@ -16,6 +16,7 @@ public class AzureStorageFactory : StorageFactory
private Uri _differentBaseAddress = null;
private readonly ILogger<AzureStorage> _azureStorageLogger;
private readonly bool _initializeContainer;
private readonly string _delimiter;

public static string PrepareConnectionString(string connectionString)
{
Expand All @@ -32,14 +33,16 @@ public AzureStorageFactory(
ILogger<AzureStorage> azureStorageLogger,
string path = null,
Uri baseAddress = null,
bool initializeContainer = true)
bool initializeContainer = true,
string delimiter = null)
{
_account = account;
_containerName = containerName;
_enablePublicAccess = enablePublicAccess;
_path = null;
_azureStorageLogger = azureStorageLogger;
_initializeContainer = initializeContainer;
_delimiter = delimiter;

if (path != null)
{
Expand Down Expand Up @@ -94,6 +97,7 @@ public override Storage Create(string name = null)
_account,
_containerName,
path,
_delimiter,
newBase,
_initializeContainer,
_enablePublicAccess,
Expand Down