Skip to content

Commit d5ebc59

Browse files
jalauzon-msftjalauzon-msft
authored
[Storage][DataMovemnt] Fix resume on resources with special characters (Azure#51584)
1 parent 72a3c27 commit d5ebc59

File tree

9 files changed

+145
-60
lines changed

9 files changed

+145
-60
lines changed

‎sdk/storage/Azure.Storage.DataMovement.Blobs/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
- Fixed an issue that could cause an exception when trying to resume a transfer that contained resources with special characters in the name.
1011

1112
### Other Changes
1213

‎sdk/storage/Azure.Storage.DataMovement.Blobs/samples/Sample01b_HelloWorldAsync.cs‎

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Azure.Storage.Blobs;
1414
using System.Collections.Generic;
1515
using System.Threading;
16+
using System.Linq;
1617

1718
namespace Azure.Storage.DataMovement.Blobs.Samples
1819
{
@@ -844,10 +845,10 @@ public async Task CopyDirectory()
844845
}
845846

846847
/// <summary>
847-
/// Test to show pause and resume tests
848+
/// Basic pause and resume flow from <see cref="TransferManager"/>.
848849
/// </summary>
849850
[Test]
850-
public async Task PauseAndResumeAsync_ManagerId()
851+
public async Task PauseAndResumeAsync_TransferManager()
851852
{
852853
string connectionString = ConnectionString;
853854
string containerName = Randomize("sample-container");
@@ -857,16 +858,12 @@ public async Task PauseAndResumeAsync_ManagerId()
857858
await container.CreateIfNotExistsAsync();
858859
try
859860
{
860-
string downloadPath = CreateTempPath();
861861
// Get a temporary path on disk where we can download the file
862-
//@@ string downloadPath = "hello.jpg";
863-
864-
// Download the public MacBeth copy at https://www.gutenberg.org/cache/epub/1533/pg1533.txt
865-
BlockBlobClient sourceBlob = new BlockBlobClient(new Uri("https://www.gutenberg.org/cache/epub/1533/pg1533.txt"));
866-
await sourceBlob.DownloadToAsync(downloadPath);
862+
string downloadPath = CreateTempPath();
867863

868-
// Create a token credential that can use our Azure Active
869-
// Directory application to authenticate with Azure Storage
864+
// Copy the public MacBeth copy to a Blob
865+
BlockBlobClient sourceBlob = container.GetBlockBlobClient("MacBeth.txt");
866+
await sourceBlob.SyncUploadFromUriAsync(new Uri("https://www.gutenberg.org/cache/epub/1533/pg1533.txt"));
870867

871868
// Create transfer manager
872869
#region Snippet:SetupTransferManagerForResume
@@ -893,11 +890,6 @@ public async Task PauseAndResumeAsync_ManagerId()
893890
await transferManager.PauseTransferAsync(transferId);
894891
#endregion
895892

896-
#region Snippet:ResumeAllTransfers
897-
// Resume all transfers
898-
List<TransferOperation> transfers = await transferManager.ResumeAllTransfersAsync();
899-
#endregion
900-
901893
// Resume a single transfer
902894
#region Snippet:DataMovement_ResumeSingle
903895
TransferOperation resumedTransfer = await transferManager.ResumeTransferAsync(transferId);
@@ -913,10 +905,10 @@ public async Task PauseAndResumeAsync_ManagerId()
913905
}
914906

915907
/// <summary>
916-
/// Test to show pause and resume tests
908+
/// Basic pause and resume flow from <see cref="TransferOperation"/>.
917909
/// </summary>
918910
[Test]
919-
public async Task PauseAndResumeAsync_DataTransferPause()
911+
public async Task PauseAndResumeAsync_TransferOperation()
920912
{
921913
string connectionString = ConnectionString;
922914
TokenCredential tokenCredential = new DefaultAzureCredential();
@@ -927,19 +919,19 @@ public async Task PauseAndResumeAsync_DataTransferPause()
927919
await container.CreateIfNotExistsAsync();
928920
try
929921
{
930-
string downloadPath = CreateTempPath();
931922
// Get a temporary path on disk where we can download the file
932-
//@@ string downloadPath = "hello.jpg";
923+
string downloadPath = CreateTempPath();
933924

934-
// Download the public MacBeth copy at https://www.gutenberg.org/cache/epub/1533/pg1533.txt
935-
BlockBlobClient sourceBlob = new BlockBlobClient(new Uri("https://www.gutenberg.org/cache/epub/1533/pg1533.txt"));
936-
await sourceBlob.DownloadToAsync(downloadPath);
925+
// Copy the public MacBeth copy to a Blob
926+
BlockBlobClient sourceBlob = container.GetBlockBlobClient("MacBeth.txt");
927+
await sourceBlob.SyncUploadFromUriAsync(new Uri("https://www.gutenberg.org/cache/epub/1533/pg1533.txt"));
937928

938929
// Create transfer manager
939930
BlobsStorageResourceProvider blobs = new(tokenCredential);
940-
TransferManagerOptions options = new TransferManagerOptions();
941-
options.ProvidersForResuming = new List<StorageResourceProvider>() { blobs };
942-
TransferManager transferManager = new TransferManager(options);
931+
TransferManager transferManager = new(new TransferManagerOptions()
932+
{
933+
ProvidersForResuming = new List<StorageResourceProvider>() { blobs },
934+
});
943935

944936
// Create source and destination resource
945937
StorageResource sourceResource = BlobsStorageResourceProvider.FromClient(sourceBlob);
@@ -955,8 +947,7 @@ public async Task PauseAndResumeAsync_DataTransferPause()
955947
await transferOperation.PauseAsync();
956948
#endregion
957949

958-
TransferOperation resumedTransfer = await transferManager.ResumeTransferAsync(
959-
transferId: transferOperation.Id);
950+
TransferOperation resumedTransfer = await transferManager.ResumeTransferAsync(transferOperation.Id);
960951

961952
// Wait for download to finish
962953
await resumedTransfer.WaitForCompletionAsync();
@@ -967,6 +958,32 @@ public async Task PauseAndResumeAsync_DataTransferPause()
967958
}
968959
}
969960

961+
/// <summary>
962+
/// Show show to resume all transfers from <see cref="TransferManager"/>.
963+
/// </summary>
964+
/// <returns></returns>
965+
[Test]
966+
public async Task PauseAndResumeAsync_ResumeAll()
967+
{
968+
TokenCredential tokenCredential = new DefaultAzureCredential();
969+
BlobsStorageResourceProvider blobs = new(tokenCredential);
970+
TransferManager transferManager = new(new TransferManagerOptions()
971+
{
972+
ProvidersForResuming = new List<StorageResourceProvider>() { blobs },
973+
});
974+
975+
// ... start multiple transfers
976+
977+
#region Snippet:ResumeAllTransfers
978+
// Resume all transfers
979+
List<TransferOperation> transfers = await transferManager.ResumeAllTransfersAsync();
980+
#endregion
981+
982+
// Wait for all transfers to finish
983+
Task[] resumedTasks = transfers.Select(t => t.WaitForCompletionAsync()).ToArray();
984+
await Task.WhenAll(resumedTasks);
985+
}
986+
970987
/// <summary>
971988
/// Use the <see cref="BlobContainerClient.UploadDirectory"/> extension method to upload an entire directory.
972989
/// </summary>

‎sdk/storage/Azure.Storage.DataMovement.Blobs/tests/BlobStorageResourceContainerTests.cs‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@
66
using System;
77
using System.Collections.Generic;
88
using System.IO;
9-
using System.Linq;
9+
using System.Net;
1010
using System.Threading.Tasks;
1111
using Azure.Core.TestFramework;
1212
using BaseBlobs::Azure.Storage.Blobs;
1313
using BaseBlobs::Azure.Storage.Blobs.Specialized;
1414
using DMBlobs::Azure.Storage.DataMovement.Blobs;
1515
using Moq;
1616
using NUnit.Framework;
17+
using NUnit.Framework.Internal;
1718

1819
namespace Azure.Storage.DataMovement.Blobs.Tests
1920
{
@@ -284,6 +285,39 @@ public async Task GetStorageResourceReferenceAsync_BlobType(string blobResourceI
284285
Assert.AreEqual(blobResourceId, resource.ResourceId);
285286
}
286287

288+
[Test]
289+
public void GetStorageResourceReference_Encoding()
290+
{
291+
string[] prefixes = ["prefix", "pre=fix", "prefix with space"];
292+
string[] tests =
293+
[
294+
"path=true@&#%",
295+
"path%3Dtest%26",
296+
"with space",
297+
"sub=dir/path=true@&#%",
298+
"sub%3Ddir/path=true@&#%",
299+
"sub dir/path=true@&#%"
300+
];
301+
302+
string containerPath = "https://account.blob.core.windows.net/container";
303+
BlobContainerClient containerClient = new(new Uri(containerPath));
304+
305+
foreach (string prefix in prefixes)
306+
{
307+
BlobStorageResourceContainer containerResource = new(containerClient, new()
308+
{
309+
BlobPrefix = prefix,
310+
});
311+
foreach (string test in tests)
312+
{
313+
StorageResourceItem resource = containerResource.GetStorageResourceReference(test, default);
314+
315+
string combined = string.Join("/", containerPath, Uri.EscapeDataString(prefix), Uri.EscapeDataString(test).Replace("%2F", "/"));
316+
Assert.That(resource.Uri.AbsoluteUri, Is.EqualTo(combined));
317+
}
318+
}
319+
}
320+
287321
[Test]
288322
public void GetChildStorageResourceContainer()
289323
{

‎sdk/storage/Azure.Storage.DataMovement.Files.Shares/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
- Fixed an issue that could cause an exception when trying to resume a transfer that contained resources with special characters in the name.
1011

1112
### Other Changes
1213

‎sdk/storage/Azure.Storage.DataMovement.Files.Shares/tests/ShareDirectoryStorageResourceContainerTests.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,36 @@ public void GetChildStorageResourceContainer()
177177
builder.Path = string.Join("/", builder.Path, childPath);
178178
Assert.AreEqual(builder.Uri, childContainer.Uri);
179179
}
180+
181+
[Test]
182+
public void GetStorageResourceReference_Encoding()
183+
{
184+
string[] dirs = ["prefix", "pre=fix", "prefix with space"];
185+
string[] tests =
186+
[
187+
"path=true@&#%",
188+
"path%3Dtest%26",
189+
"with space",
190+
"sub=dir/path=true@&#%",
191+
"sub%3Ddir/path=true@&#%",
192+
"sub dir/path=true@&#%"
193+
];
194+
195+
string sharePath = "https://account.file.core.windows.net/myshare";
196+
ShareClient shareClient = new(new Uri(sharePath));
197+
198+
foreach (string dir in dirs)
199+
{
200+
ShareDirectoryClient directoryClient = shareClient.GetDirectoryClient(dir);
201+
ShareDirectoryStorageResourceContainer containerResource = new(directoryClient, default);
202+
foreach (string test in tests)
203+
{
204+
StorageResourceItem resource = containerResource.GetStorageResourceReference(test, default);
205+
206+
string combined = string.Join("/", sharePath, Uri.EscapeDataString(dir), Uri.EscapeDataString(test).Replace("%2F", "/"));
207+
Assert.That(resource.Uri.AbsoluteUri, Is.EqualTo(combined));
208+
}
209+
}
210+
}
180211
}
181212
}

‎sdk/storage/Azure.Storage.DataMovement/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
- Fixed an issue that could cause an exception when trying to resume a transfer that contained resources with special characters in the name.
1011

1112
### Other Changes
1213

‎sdk/storage/Azure.Storage.DataMovement/src/Shared/DataMovementExtensions.cs‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,8 @@ public static StreamToUriJobPart ToStreamToUriJobPartAsync(
2828
StorageResourceContainer sourceResource,
2929
StorageResourceContainer destinationResource)
3030
{
31-
// If saved path equals the cotnainer Uri, its a single item trasfer.
32-
// Set the resource name to the full Uri.
33-
string childSourcePath = header.SourcePath;
34-
string childSourceName = header.SourcePath == sourceResource.Uri.AbsoluteUri.ToString() ?
35-
childSourcePath :
36-
childSourcePath.Substring(sourceResource.Uri.AbsoluteUri.Length + 1);
37-
string childDestinationPath = header.DestinationPath;
38-
string childDestinationName = header.DestinationPath == destinationResource.Uri.AbsoluteUri.ToString() ?
39-
childDestinationPath :
40-
childDestinationPath.Substring(destinationResource.Uri.AbsoluteUri.Length + 1);
31+
(string childSourceName, string childDestinationName) = GetChildResourceNames(header, sourceResource, destinationResource);
32+
4133
// Override header values if options were specified by user.
4234
long initialTransferSize = baseJob._initialTransferSize ?? header.InitialTransferSize;
4335
long transferChunkSize = baseJob._maximumTransferChunkSize ?? header.ChunkSize;
@@ -67,16 +59,8 @@ public static ServiceToServiceJobPart ToServiceToServiceJobPartAsync(
6759
StorageResourceContainer sourceResource,
6860
StorageResourceContainer destinationResource)
6961
{
70-
// If saved path equals the cotnainer Uri, its a single item trasfer, so the resource name
71-
// does not matter. Just set it to the path.
72-
string childSourcePath = header.SourcePath;
73-
string childSourceName = header.SourcePath == sourceResource.Uri.AbsoluteUri.ToString() ?
74-
childSourcePath :
75-
childSourcePath.Substring(sourceResource.Uri.AbsoluteUri.Length + 1);
76-
string childDestinationPath = header.DestinationPath;
77-
string childDestinationName = header.DestinationPath == destinationResource.Uri.AbsoluteUri.ToString() ?
78-
childDestinationPath :
79-
childDestinationPath.Substring(destinationResource.Uri.AbsoluteUri.Length + 1);
62+
(string childSourceName, string childDestinationName) = GetChildResourceNames(header, sourceResource, destinationResource);
63+
8064
// Override header values if options were specified by user.
8165
long initialTransferSize = baseJob._initialTransferSize ?? header.InitialTransferSize;
8266
long transferChunkSize = baseJob._maximumTransferChunkSize ?? header.ChunkSize;
@@ -106,16 +90,8 @@ public static UriToStreamJobPart ToUriToStreamJobPartAsync(
10690
StorageResourceContainer sourceResource,
10791
StorageResourceContainer destinationResource)
10892
{
109-
// If saved path equals the cotnainer Uri, its a single item trasfer, so the resource name
110-
// does not matter. Just set it to the path.
111-
string childSourcePath = header.SourcePath;
112-
string childSourceName = header.SourcePath == sourceResource.Uri.AbsoluteUri.ToString() ?
113-
childSourcePath :
114-
childSourcePath.Substring(sourceResource.Uri.AbsoluteUri.Length + 1);
115-
string childDestinationPath = header.DestinationPath;
116-
string childDestinationName = header.DestinationPath == destinationResource.Uri.AbsoluteUri.ToString() ?
117-
childDestinationPath :
118-
childDestinationPath.Substring(destinationResource.Uri.AbsoluteUri.Length + 1);
93+
(string childSourceName, string childDestinationName) = GetChildResourceNames(header, sourceResource, destinationResource);
94+
11995
// Override header values if options were specified by user.
12096
long initialTransferSize = baseJob._initialTransferSize ?? header.InitialTransferSize;
12197
long transferChunkSize = baseJob._maximumTransferChunkSize ?? header.ChunkSize;
@@ -197,5 +173,25 @@ internal static void VerifyJobPartPlanHeader(this JobPartInternal jobPart, JobPa
197173
throw Errors.MismatchResumeTransferArguments(nameof(header.DestinationPath), header.DestinationPath, passedDestinationPath);
198174
}
199175
}
176+
177+
private static (string SourceName, string DestinationName) GetChildResourceNames(
178+
JobPartPlanHeader header,
179+
StorageResourceContainer sourceResource,
180+
StorageResourceContainer destinationResource)
181+
{
182+
// If saved path equals the container Uri, it's a single item transfer, so the resource name
183+
// does not matter. Just set it to the path.
184+
string childSourceName = header.SourcePath == sourceResource.Uri.AbsoluteUri.ToString() ?
185+
header.SourcePath :
186+
header.SourcePath.Substring(sourceResource.Uri.AbsoluteUri.Length + 1);
187+
// Decode the resource name as it was pulled from encoded Uri and will be re-encoded.
188+
childSourceName = Uri.UnescapeDataString(childSourceName);
189+
190+
string childDestinationName = header.DestinationPath == destinationResource.Uri.AbsoluteUri.ToString() ?
191+
header.DestinationPath :
192+
header.DestinationPath.Substring(destinationResource.Uri.AbsoluteUri.Length + 1);
193+
childDestinationName = Uri.UnescapeDataString(childDestinationName);
194+
return (childSourceName, childDestinationName);
195+
}
200196
}
201197
}

‎sdk/storage/Azure.Storage.DataMovement/src/TransferJobInternal.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ private async IAsyncEnumerable<JobPartInternal> EnumerateAndCreateJobPartsAsync(
335335
? current.Uri.GetPath()
336336
: current.Uri.GetPath().Substring(containerUriPath.Length + 1);
337337
// Decode the container name as it was pulled from encoded Uri and will be re-encoded on destination.
338-
subContainerPath = WebUtility.UrlDecode(subContainerPath);
338+
subContainerPath = Uri.UnescapeDataString(subContainerPath);
339339
StorageResourceContainer subContainer =
340340
_destinationResourceContainer.GetChildStorageResourceContainer(subContainerPath);
341341

@@ -372,7 +372,7 @@ private async IAsyncEnumerable<JobPartInternal> EnumerateAndCreateJobPartsAsync(
372372
string containerUriPath = _sourceResourceContainer.Uri.GetPath();
373373
sourceName = current.Uri.GetPath().Substring(containerUriPath.Length + 1);
374374
// Decode the resource name as it was pulled from encoded Uri and will be re-encoded on destination.
375-
sourceName = WebUtility.UrlDecode(sourceName);
375+
sourceName = Uri.UnescapeDataString(sourceName);
376376
}
377377

378378
StorageResourceItem sourceItem = (StorageResourceItem)current;

‎sdk/storage/Azure.Storage.DataMovement/tests/TransferManagerTests.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,8 @@ public static void BasicSetup(
644644
items.Destination.SetupGet(r => r.MaxSupportedChunkSize).Returns(Constants.GB);
645645
items.Destination.SetupGet(r => r.MaxSupportedChunkCount).Returns(int.MaxValue);
646646

647+
items.Source.Setup(r => r.ShouldItemTransferAsync(It.IsAny<CancellationToken>())).Returns(Task.FromResult(true));
648+
647649
items.Source.Setup(r => r.GetPropertiesAsync(It.IsAny<CancellationToken>()))
648650
.Returns((CancellationToken cancellationToken) =>
649651
{
@@ -656,6 +658,8 @@ public static void BasicSetup(
656658

657659
items.Destination.Setup(r => r.ValidateTransferAsync(It.IsAny<string>(), It.IsAny<StorageResource>(), It.IsAny<CancellationToken>()))
658660
.Returns(Task.CompletedTask);
661+
items.Destination.Setup(r => r.SetPermissionsAsync(It.IsAny<StorageResourceItem>(), It.IsAny<StorageResourceItemProperties>(), It.IsAny<CancellationToken>()))
662+
.Returns(Task.CompletedTask);
659663

660664
items.Source.Setup(r => r.ReadStreamAsync(It.IsAny<long>(), It.IsAny<long?>(), It.IsAny<CancellationToken>()))
661665
.Returns<long, long?, CancellationToken>((position, length, cancellationToken) =>

0 commit comments

Comments
 (0)