[Storage][DataMovement] Add tests for TransferProgressTracker as well as minor changes (Azure#47894)

jalauzon-msft · web-flow · commit d79be9685545 · 2025-01-21T11:07:04.000-08:00
diff --git a/sdk/storage/Azure.Storage.DataMovement.Blobs/samples/Sample01b_HelloWorldAsync.cs b/sdk/storage/Azure.Storage.DataMovement.Blobs/samples/Sample01b_HelloWorldAsync.cs
@@ -1168,8 +1168,12 @@ async Task<TransferOperation> ListenToProgressAsync(TransferManager transferMana
                 {
                     TransferOptions transferOptions = new()
                     {
-                        // optionally include the below if progress updates on bytes transferred are desired
-                        ProgressHandlerOptions = new(progress, trackBytesTransferred: true)
+                        ProgressHandlerOptions = new()
+                        {
+                            ProgressHandler = progress,
+                            // optionally include the below if progress updates on bytes transferred are desired
+                            TrackBytesTransferred = true,
+                        }
                     };
                     return await transferManager.StartTransferAsync(
                         source,
diff --git a/sdk/storage/Azure.Storage.DataMovement.Blobs/tests/Azure.Storage.DataMovement.Blobs.Tests.csproj b/sdk/storage/Azure.Storage.DataMovement.Blobs/tests/Azure.Storage.DataMovement.Blobs.Tests.csproj
@@ -54,6 +54,7 @@
     <Compile Include="$(AzureStorageDataMovementTestSharedSources)DataMovementTestBase.cs" LinkBase="Shared\DataMovement" />
     <Compile Include="$(AzureStorageDataMovementTestSharedSources)TransferDirection.cs" LinkBase="Shared\DataMovement" />
     <Compile Include="$(AzureStorageDataMovementTestSharedSources)TransferValidator.Local.cs" LinkBase="Shared\DataMovement" />
+    <Compile Include="$(AzureStorageDataMovementTestSharedSources)TestProgressHandler.cs" LinkBase="Shared\DataMovement" />
   </ItemGroup>
   <ItemGroup>
     <Content Include="Resources\**">
diff --git a/sdk/storage/Azure.Storage.DataMovement.Blobs/tests/ProgressHandlerTests.cs b/sdk/storage/Azure.Storage.DataMovement.Blobs/tests/ProgressHandlerTests.cs
@@ -130,7 +130,12 @@ private async Task TransferAndAssertProgress(
 
             TestProgressHandler progressHandler = new TestProgressHandler();
             transferOptions ??= new TransferOptions();
-            transferOptions.ProgressHandlerOptions = new ProgressHandlerOptions(progressHandler, trackBytes);
+            transferOptions.ProgressHandlerOptions = new()
+            {
+                ProgressHandler = progressHandler,
+                TrackBytesTransferred = trackBytes
+            }
+                ;
             transferOptions.CreationPreference = createMode;
 
             TransferOperation transfer = await transferManager.StartTransferAsync(source, destination, transferOptions);
@@ -323,7 +328,11 @@ public async Task ProgressHandler_PauseResume(int delayInMs)
             TestProgressHandler progressHandler = new();
             TransferOptions transferOptions = new()
             {
-                ProgressHandlerOptions = new ProgressHandlerOptions(progressHandler, true)
+                ProgressHandlerOptions = new()
+                {
+                    ProgressHandler = progressHandler,
+                    TrackBytesTransferred = true
+                }
             };
             TestEventsRaised testEventsRaised = new(transferOptions);
 
diff --git a/sdk/storage/Azure.Storage.DataMovement/CHANGELOG.md b/sdk/storage/Azure.Storage.DataMovement/CHANGELOG.md
@@ -16,6 +16,8 @@
     - `DataTransferState` -> `TransferState`
     - `DataTransferStatus` -> `TransferStatus`
     - `DataTransferErrorMode` -> `TransferErrorMode`
+    - `ProgressHandlerOptions` -> `TransferProgressHandlerOptions`
+        - Also removed the constructor since properties are settable.
     - `StorageResourceCheckpointData` -> `StorageResourceCheckpointDetails`
     - `StorageResource.GetDestinationCheckpointData` -> `StorageResource.GetDestinationCheckpointDetails`
     - `StorageResource.GetSourceCheckpointData` -> `StorageResource.GetSourceCheckpointDetails`
diff --git a/sdk/storage/Azure.Storage.DataMovement/README.md b/sdk/storage/Azure.Storage.DataMovement/README.md
@@ -208,8 +208,12 @@ async Task<TransferOperation> ListenToProgressAsync(TransferManager transferMana
 {
     TransferOptions transferOptions = new()
     {
-        // optionally include the below if progress updates on bytes transferred are desired
-        ProgressHandlerOptions = new(progress, trackBytesTransferred: true)
+        ProgressHandlerOptions = new()
+        {
+            ProgressHandler = progress,
+            // optionally include the below if progress updates on bytes transferred are desired
+            TrackBytesTransferred = true,
+        }
     };
     return await transferManager.StartTransferAsync(
         source,
diff --git a/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.net6.0.cs b/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.net6.0.cs
@@ -27,12 +27,6 @@ public LocalFilesStorageResourceProvider() { }
         public Azure.Storage.DataMovement.StorageResourceItem FromFile(string filePath) { throw null; }
         protected internal override System.Threading.Tasks.Task<Azure.Storage.DataMovement.StorageResource> FromSourceAsync(Azure.Storage.DataMovement.TransferProperties properties, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
-    public partial class ProgressHandlerOptions
-    {
-        public ProgressHandlerOptions(System.IProgress<Azure.Storage.DataMovement.TransferProgress> progressHandler, bool trackBytesTransferred = false) { }
-        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
-        public bool TrackBytesTransferred { get { throw null; } set { } }
-    }
     public abstract partial class StorageResource
     {
         protected StorageResource() { }
@@ -201,7 +195,7 @@ public TransferOptions() { }
         public Azure.Storage.DataMovement.StorageResourceCreationMode CreationPreference { get { throw null; } set { } }
         public long? InitialTransferSize { get { throw null; } set { } }
         public long? MaximumTransferChunkSize { get { throw null; } set { } }
-        public Azure.Storage.DataMovement.ProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
+        public Azure.Storage.DataMovement.TransferProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemCompletedEventArgs> ItemTransferCompleted { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemFailedEventArgs> ItemTransferFailed { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemSkippedEventArgs> ItemTransferSkipped { add { } remove { } }
@@ -232,6 +226,12 @@ internal TransferProgress() { }
         public long QueuedCount { get { throw null; } }
         public long SkippedCount { get { throw null; } }
     }
+    public partial class TransferProgressHandlerOptions
+    {
+        public TransferProgressHandlerOptions() { }
+        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
+        public bool TrackBytesTransferred { get { throw null; } set { } }
+    }
     public partial class TransferProperties
     {
         protected internal TransferProperties() { }
diff --git a/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.net8.0.cs b/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.net8.0.cs
@@ -27,12 +27,6 @@ public LocalFilesStorageResourceProvider() { }
         public Azure.Storage.DataMovement.StorageResourceItem FromFile(string filePath) { throw null; }
         protected internal override System.Threading.Tasks.Task<Azure.Storage.DataMovement.StorageResource> FromSourceAsync(Azure.Storage.DataMovement.TransferProperties properties, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
-    public partial class ProgressHandlerOptions
-    {
-        public ProgressHandlerOptions(System.IProgress<Azure.Storage.DataMovement.TransferProgress> progressHandler, bool trackBytesTransferred = false) { }
-        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
-        public bool TrackBytesTransferred { get { throw null; } set { } }
-    }
     public abstract partial class StorageResource
     {
         protected StorageResource() { }
@@ -201,7 +195,7 @@ public TransferOptions() { }
         public Azure.Storage.DataMovement.StorageResourceCreationMode CreationPreference { get { throw null; } set { } }
         public long? InitialTransferSize { get { throw null; } set { } }
         public long? MaximumTransferChunkSize { get { throw null; } set { } }
-        public Azure.Storage.DataMovement.ProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
+        public Azure.Storage.DataMovement.TransferProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemCompletedEventArgs> ItemTransferCompleted { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemFailedEventArgs> ItemTransferFailed { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemSkippedEventArgs> ItemTransferSkipped { add { } remove { } }
@@ -232,6 +226,12 @@ internal TransferProgress() { }
         public long QueuedCount { get { throw null; } }
         public long SkippedCount { get { throw null; } }
     }
+    public partial class TransferProgressHandlerOptions
+    {
+        public TransferProgressHandlerOptions() { }
+        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
+        public bool TrackBytesTransferred { get { throw null; } set { } }
+    }
     public partial class TransferProperties
     {
         protected internal TransferProperties() { }
diff --git a/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.netstandard2.0.cs b/sdk/storage/Azure.Storage.DataMovement/api/Azure.Storage.DataMovement.netstandard2.0.cs
@@ -27,12 +27,6 @@ public LocalFilesStorageResourceProvider() { }
         public Azure.Storage.DataMovement.StorageResourceItem FromFile(string filePath) { throw null; }
         protected internal override System.Threading.Tasks.Task<Azure.Storage.DataMovement.StorageResource> FromSourceAsync(Azure.Storage.DataMovement.TransferProperties properties, System.Threading.CancellationToken cancellationToken) { throw null; }
     }
-    public partial class ProgressHandlerOptions
-    {
-        public ProgressHandlerOptions(System.IProgress<Azure.Storage.DataMovement.TransferProgress> progressHandler, bool trackBytesTransferred = false) { }
-        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
-        public bool TrackBytesTransferred { get { throw null; } set { } }
-    }
     public abstract partial class StorageResource
     {
         protected StorageResource() { }
@@ -201,7 +195,7 @@ public TransferOptions() { }
         public Azure.Storage.DataMovement.StorageResourceCreationMode CreationPreference { get { throw null; } set { } }
         public long? InitialTransferSize { get { throw null; } set { } }
         public long? MaximumTransferChunkSize { get { throw null; } set { } }
-        public Azure.Storage.DataMovement.ProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
+        public Azure.Storage.DataMovement.TransferProgressHandlerOptions ProgressHandlerOptions { get { throw null; } set { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemCompletedEventArgs> ItemTransferCompleted { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemFailedEventArgs> ItemTransferFailed { add { } remove { } }
         public event Azure.Core.SyncAsyncEventHandler<Azure.Storage.DataMovement.TransferItemSkippedEventArgs> ItemTransferSkipped { add { } remove { } }
@@ -232,6 +226,12 @@ internal TransferProgress() { }
         public long QueuedCount { get { throw null; } }
         public long SkippedCount { get { throw null; } }
     }
+    public partial class TransferProgressHandlerOptions
+    {
+        public TransferProgressHandlerOptions() { }
+        public System.IProgress<Azure.Storage.DataMovement.TransferProgress> ProgressHandler { get { throw null; } set { } }
+        public bool TrackBytesTransferred { get { throw null; } set { } }
+    }
     public partial class TransferProperties
     {
         protected internal TransferProperties() { }
diff --git a/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs b/sdk/storage/Azure.Storage.DataMovement/src/JobPartInternal.cs
@@ -308,7 +308,7 @@ await PartTransferStatusEventHandler.RaiseAsync(
 
         protected async ValueTask ReportBytesWrittenAsync(long bytesTransferred)
         {
-            await _progressTracker.IncrementBytesTransferred(bytesTransferred, _cancellationToken).ConfigureAwait(false);
+            await _progressTracker.IncrementBytesTransferredAsync(bytesTransferred, _cancellationToken).ConfigureAwait(false);
         }
 
         public async virtual Task InvokeSingleCompletedArgAsync()
diff --git a/sdk/storage/Azure.Storage.DataMovement/src/TransferOptions.cs b/sdk/storage/Azure.Storage.DataMovement/src/TransferOptions.cs
@@ -40,7 +40,7 @@ public class TransferOptions : IEquatable<TransferOptions>
         /// <summary>
         /// Optional. Options for changing behavior of the ProgressHandler.
         /// </summary>
-        public ProgressHandlerOptions ProgressHandlerOptions { get; set; }
+        public TransferProgressHandlerOptions ProgressHandlerOptions { get; set; }
 
         /// <summary>
         /// Check if two ParallelTransferOptions instances are equal.
diff --git a/sdk/storage/Azure.Storage.DataMovement/src/TransferProgress.cs b/sdk/storage/Azure.Storage.DataMovement/src/TransferProgress.cs
@@ -38,7 +38,7 @@ internal TransferProgress() { }
 
         /// <summary>
         /// Number of bytes transferred across all files.
-        /// Only populated if <see cref="ProgressHandlerOptions.TrackBytesTransferred"/> is set.
+        /// Only populated if <see cref="TransferProgressHandlerOptions.TrackBytesTransferred"/> is set.
         /// </summary>
         public long? BytesTransferred { get; internal set; }
     }
diff --git a/sdk/storage/Azure.Storage.DataMovement/src/TransferProgressHandlerOptions.cs b/sdk/storage/Azure.Storage.DataMovement/src/TransferProgressHandlerOptions.cs
@@ -8,19 +8,8 @@ namespace Azure.Storage.DataMovement
     /// <summary>
     /// Options associated with transfer progress tracking.
     /// </summary>
-    public class ProgressHandlerOptions
+    public class TransferProgressHandlerOptions
     {
-        /// <summary>
-        /// Constructor for ProgressHandlerOptions.
-        /// </summary>
-        /// <param name="progressHandler"></param>
-        /// <param name="trackBytesTransferred"></param>
-        public ProgressHandlerOptions(IProgress<TransferProgress> progressHandler, bool trackBytesTransferred = false)
-        {
-            ProgressHandler = progressHandler;
-            TrackBytesTransferred = trackBytesTransferred;
-        }
-
         /// <summary>
         /// Optional. An <see cref="IProgress{StorageTransferProgress}"/> for tracking progress of the transfer.
         /// See <see cref="TransferProgress"/> for details on what is tracked.
diff --git a/sdk/storage/Azure.Storage.DataMovement/src/TransferProgressTracker.cs b/sdk/storage/Azure.Storage.DataMovement/src/TransferProgressTracker.cs
@@ -14,7 +14,7 @@ namespace Azure.Storage.DataMovement
     /// </summary>
     internal class TransferProgressTracker : IAsyncDisposable
     {
-        private class ProgressEventArgs
+        internal class ProgressEventArgs
         {
             public int CompletedChange { get; set; } = 0;
             public int SkippedChange { get; set; } = 0;
@@ -24,7 +24,7 @@ private class ProgressEventArgs
             public long BytesChange { get; set; } = 0;
         }
 
-        private readonly ProgressHandlerOptions _options;
+        private readonly TransferProgressHandlerOptions _options;
 
         private IProcessor<ProgressEventArgs> _progressProcessor;
         private long _completedCount = 0;
@@ -34,13 +34,20 @@ private class ProgressEventArgs
         private long _queuedCount = 0;
         private long _bytesTransferred = 0;
 
-        public TransferProgressTracker(ProgressHandlerOptions options)
+        public TransferProgressTracker(TransferProgressHandlerOptions options)
         {
             _options = options;
             _progressProcessor = ChannelProcessing.NewProcessor<ProgressEventArgs>(readers: 1);
             _progressProcessor.Process = ProcessProgressEvent;
         }
 
+        internal TransferProgressTracker(IProcessor<ProgressEventArgs> progressProcessor, TransferProgressHandlerOptions options)
+        {
+            _options = options;
+            _progressProcessor = progressProcessor;
+            _progressProcessor.Process = ProcessProgressEvent;
+        }
+
         public async ValueTask DisposeAsync()
         {
             // This will close the channel and block on all pending items being processed
@@ -96,7 +103,7 @@ await QueueProgressEvent(new ProgressEventArgs()
             cancellationToken).ConfigureAwait(false);
         }
 
-        public async ValueTask IncrementBytesTransferred(long bytesTransferred, CancellationToken cancellationToken)
+        public async ValueTask IncrementBytesTransferredAsync(long bytesTransferred, CancellationToken cancellationToken)
         {
             if (_options?.TrackBytesTransferred == true)
             {
diff --git a/sdk/storage/Azure.Storage.DataMovement/tests/Shared/TestProgressHandler.cs b/sdk/storage/Azure.Storage.DataMovement/tests/Shared/TestProgressHandler.cs
diff --git a/sdk/storage/Azure.Storage.DataMovement/tests/TransferManagerTests.cs b/sdk/storage/Azure.Storage.DataMovement/tests/TransferManagerTests.cs
@@ -15,7 +15,6 @@
 using Moq;
 using NUnit.Framework;
 using NUnit.Framework.Constraints;
-using NUnit.Framework.Interfaces;
 
 namespace Azure.Storage.DataMovement.Tests;
 
@@ -571,6 +570,39 @@ public async Task MultipleTransfersAddedCheckpointer(int numJobs)
         Assert.That(jobsProcessor.ItemsInQueue, Is.EqualTo(numJobs), "Error during initial Job queueing.");
     }
 
+    [Test]
+    public async Task BasicTransfer_NoOptions()
+    {
+        Uri srcUri = new("file:///foo/bar");
+        Uri dstUri = new("https://example.com/fizz/buzz");
+
+        (var jobsProcessor, var partsProcessor, var chunksProcessor) = StepProcessors();
+        JobBuilder jobBuilder = new(ArrayPool<byte>.Shared, default, new ClientDiagnostics(ClientOptions.Default));
+        MemoryTransferCheckpointer checkpointer = new();
+
+        await using TransferManager transferManager = new(
+            jobsProcessor,
+            partsProcessor,
+            chunksProcessor,
+            jobBuilder,
+            checkpointer,
+            default);
+
+        int numFiles = 3;
+        Mock<StorageResourceContainer> srcResource = new(MockBehavior.Strict);
+        Mock<StorageResourceContainer> dstResource = new(MockBehavior.Strict);
+        (srcResource, dstResource).BasicSetup(srcUri, dstUri, numFiles);
+
+        // Don't pass options to test default options
+        TransferOperation transfer = await transferManager.StartTransferAsync(srcResource.Object, dstResource.Object);
+
+        Assert.That(await jobsProcessor.StepAll(), Is.EqualTo(1), "Failed to step through jobs queue.");
+        Assert.That(await partsProcessor.StepAll(), Is.EqualTo(numFiles), "Failed to step through parts queue.");
+        await ProcessChunksAssert(chunksProcessor, 1, numFiles, numFiles);
+
+        Assert.That(transfer.Status.HasCompletedSuccessfully);
+    }
+
     /// <summary>
     /// <see cref="TransferStatus"/> is stateful across transfer. This makes it difficult to verify mocks, as verifications
     /// are lazily performed. This captures deep copies of statuses for custom assertion.
diff --git a/sdk/storage/Azure.Storage.DataMovement/tests/TransferProgressTrackerTests.cs b/sdk/storage/Azure.Storage.DataMovement/tests/TransferProgressTrackerTests.cs

Original file line number	Diff line number	Diff line change
`@@ -308,7 +308,7 @@ await PartTransferStatusEventHandler.RaiseAsync(`
`308`	`308`
`309`	`309`	`protected async ValueTask ReportBytesWrittenAsync(long bytesTransferred)`
`310`	`310`	`{`
`311`		`- await _progressTracker.IncrementBytesTransferred(bytesTransferred, _cancellationToken).ConfigureAwait(false);`
	`311`	`+ await _progressTracker.IncrementBytesTransferredAsync(bytesTransferred, _cancellationToken).ConfigureAwait(false);`
`312`	`312`	`}`
`313`	`313`
`314`	`314`	`public async virtual Task InvokeSingleCompletedArgAsync()`