Skip to content

Commit 2c7b2c9

Browse files
GarrettBeattyGarrettBeatty
committed
update ordering of cancellation token
1 parent d13eb85 commit 2c7b2c9

File tree

6 files changed

+27
-27
lines changed

6 files changed

+27
-27
lines changed

‎sdk/src/Services/S3/Custom/Transfer/Internal/BufferedMultipartStream.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public async Task InitializeAsync(CancellationToken cancellationToken)
123123
_discoveryResult.TotalParts,
124124
_discoveryResult.IsSinglePart);
125125

126-
await _downloadCoordinator.StartDownloadsAsync(_discoveryResult, cancellationToken)
126+
await _downloadCoordinator.StartDownloadsAsync(_discoveryResult, null, cancellationToken)
127127
.ConfigureAwait(false);
128128

129129
_initialized = true;

‎sdk/src/Services/S3/Custom/Transfer/Internal/IDownloadManager.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ internal interface IDownloadManager : IDisposable
4545
/// Starts concurrent downloads with HTTP concurrency control and part range calculations.
4646
/// </summary>
4747
/// <param name="discoveryResult">Results from the discovery phase.</param>
48-
/// <param name="cancellationToken">A token to cancel the download operation.</param>
4948
/// <param name="progressCallback">Optional callback for progress tracking events.</param>
49+
/// <param name="cancellationToken">A token to cancel the download operation.</param>
5050
/// <returns>A task that completes when all downloads finish or an error occurs.</returns>
51-
Task StartDownloadsAsync(DownloadDiscoveryResult discoveryResult, CancellationToken cancellationToken, EventHandler<WriteObjectProgressArgs> progressCallback = null);
51+
Task StartDownloadsAsync(DownloadDiscoveryResult discoveryResult, EventHandler<WriteObjectProgressArgs> progressCallback, CancellationToken cancellationToken);
5252

5353
/// <summary>
5454
/// Exception that occurred during downloads, if any.

‎sdk/src/Services/S3/Custom/Transfer/Internal/MultipartDownloadManager.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public async Task<DownloadDiscoveryResult> DiscoverDownloadStrategyAsync(Cancell
144144
}
145145

146146
/// <inheritdoc/>
147-
public async Task StartDownloadsAsync(DownloadDiscoveryResult discoveryResult, CancellationToken cancellationToken, EventHandler<WriteObjectProgressArgs> progressCallback = null)
147+
public async Task StartDownloadsAsync(DownloadDiscoveryResult discoveryResult, EventHandler<WriteObjectProgressArgs> progressCallback, CancellationToken cancellationToken)
148148
{
149149
ThrowIfDisposed();
150150

‎sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartDownloadCommand.async.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public override async Task<TransferUtilityDownloadResponse> ExecuteAsync(Cancell
7373

7474
// Step 2: Start concurrent downloads for all parts
7575
Logger.DebugFormat("Starting downloads for {0} part(s)", discoveryResult.TotalParts);
76-
await coordinator.StartDownloadsAsync(discoveryResult, cancellationToken, DownloadPartProgressEventCallback)
76+
await coordinator.StartDownloadsAsync(discoveryResult, DownloadPartProgressEventCallback, cancellationToken)
7777
.ConfigureAwait(false);
7878

7979
// Step 2b: Wait for all downloads to complete before returning

‎sdk/test/Services/S3/UnitTests/Custom/BufferedMultipartStreamTests.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ private async Task<BufferedMultipartStream> CreateInitializedStreamAsync(
7373

7474
_mockCoordinator.Setup(x => x.DiscoverDownloadStrategyAsync(It.IsAny<CancellationToken>()))
7575
.ReturnsAsync(discoveryResult);
76-
_mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()))
76+
_mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()))
7777
.Returns(Task.CompletedTask);
7878

7979
var stream = CreateStream();
@@ -199,7 +199,7 @@ public async Task InitializeAsync_SinglePart_CallsStartDownloads()
199199
var mockCoordinator = new Mock<IDownloadManager>();
200200
mockCoordinator.Setup(x => x.DiscoverDownloadStrategyAsync(It.IsAny<CancellationToken>()))
201201
.ReturnsAsync(discoveryResult);
202-
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()))
202+
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()))
203203
.Returns(Task.CompletedTask);
204204

205205
var mockBufferManager = new Mock<IPartBufferManager>();
@@ -211,7 +211,7 @@ public async Task InitializeAsync_SinglePart_CallsStartDownloads()
211211

212212
// Assert
213213
mockCoordinator.Verify(
214-
x => x.StartDownloadsAsync(discoveryResult, It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()),
214+
x => x.StartDownloadsAsync(discoveryResult, It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()),
215215
Times.Once);
216216
}
217217

@@ -233,7 +233,7 @@ public async Task InitializeAsync_Multipart_UsesMultipartHandler()
233233
var mockCoordinator = new Mock<IDownloadManager>();
234234
mockCoordinator.Setup(x => x.DiscoverDownloadStrategyAsync(It.IsAny<CancellationToken>()))
235235
.ReturnsAsync(discoveryResult);
236-
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()))
236+
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()))
237237
.Returns(Task.CompletedTask);
238238

239239
var mockBufferManager = new Mock<IPartBufferManager>();
@@ -261,7 +261,7 @@ public async Task InitializeAsync_Multipart_StartsDownloads()
261261
var mockCoordinator = new Mock<IDownloadManager>();
262262
mockCoordinator.Setup(x => x.DiscoverDownloadStrategyAsync(It.IsAny<CancellationToken>()))
263263
.ReturnsAsync(discoveryResult);
264-
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()))
264+
mockCoordinator.Setup(x => x.StartDownloadsAsync(It.IsAny<DownloadDiscoveryResult>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()))
265265
.Returns(Task.CompletedTask);
266266

267267
var mockBufferManager = new Mock<IPartBufferManager>();
@@ -273,7 +273,7 @@ public async Task InitializeAsync_Multipart_StartsDownloads()
273273

274274
// Assert
275275
mockCoordinator.Verify(
276-
x => x.StartDownloadsAsync(discoveryResult, It.IsAny<CancellationToken>(), It.IsAny<EventHandler<WriteObjectProgressArgs>>()),
276+
x => x.StartDownloadsAsync(discoveryResult, It.IsAny<EventHandler<WriteObjectProgressArgs>>(), It.IsAny<CancellationToken>()),
277277
Times.Once);
278278
}
279279

‎sdk/test/Services/S3/UnitTests/Custom/MultipartDownloadManagerTests.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ public async Task StartDownloadsAsync_SinglePart_ReturnsImmediately()
577577
var mockBufferManager = new Mock<IPartBufferManager>();
578578

579579
// Act
580-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
580+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
581581

582582
// Assert - should complete without any downloads
583583
mockClient.Verify(x => x.GetObjectAsync(It.IsAny<GetObjectRequest>(), It.IsAny<CancellationToken>()), Times.Never);
@@ -594,7 +594,7 @@ public async Task StartDownloadsAsync_WithNullDiscoveryResult_ThrowsArgumentNull
594594
var coordinator = new MultipartDownloadManager(mockClient.Object, request, config, CreateMockDataHandler().Object);
595595

596596
// Act
597-
await coordinator.StartDownloadsAsync(null, CancellationToken.None);
597+
await coordinator.StartDownloadsAsync(null, null, CancellationToken.None);
598598
}
599599

600600
#endregion
@@ -617,7 +617,7 @@ public async Task Validation_Failures_ThrowInvalidOperationException(
617617
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
618618

619619
// Act & Assert (exception expected via attribute)
620-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
620+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
621621
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
622622
}
623623

@@ -641,7 +641,7 @@ public async Task Validation_ETag_Matching_Succeeds()
641641
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
642642

643643
// Act - should succeed with matching ETags
644-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
644+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
645645

646646
// Assert - no exception thrown
647647
}
@@ -683,7 +683,7 @@ public async Task Validation_ContentRange_ValidRange_Succeeds()
683683
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
684684

685685
// Act - should succeed with valid ranges
686-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
686+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
687687

688688
// Assert - no exception thrown
689689
}
@@ -824,7 +824,7 @@ public async Task StartDownloadsAsync_WhenCancelledBeforeStart_ThrowsOperationCa
824824
cts.Cancel();
825825

826826
// Act
827-
await coordinator.StartDownloadsAsync(discoveryResult, cts.Token);
827+
await coordinator.StartDownloadsAsync(discoveryResult, null, cts.Token);
828828
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
829829
}
830830

@@ -865,7 +865,7 @@ public async Task StartDownloadsAsync_WhenCancelledDuringDownloads_NotifiesBuffe
865865
// Act
866866
try
867867
{
868-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
868+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
869869
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
870870
}
871871
catch (OperationCanceledException)
@@ -910,7 +910,7 @@ public async Task StartDownloadsAsync_WhenCancelled_SetsDownloadException()
910910
// Act
911911
try
912912
{
913-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
913+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
914914
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
915915
}
916916
catch (OperationCanceledException)
@@ -944,7 +944,7 @@ public async Task StartDownloadsAsync_PassesCancellationTokenToBufferManager()
944944
var cts = new CancellationTokenSource();
945945

946946
// Act
947-
await coordinator.StartDownloadsAsync(discoveryResult, cts.Token);
947+
await coordinator.StartDownloadsAsync(discoveryResult, null, cts.Token);
948948

949949
// Assert - The cancellation token was passed through to the data handler
950950
Assert.IsNotNull(discoveryResult);
@@ -970,7 +970,7 @@ public async Task StartDownloadsAsync_SinglePart_DoesNotThrowOnCancellation()
970970
cts.Cancel();
971971

972972
// Act - should complete without throwing even though token is cancelled
973-
await coordinator.StartDownloadsAsync(discoveryResult, cts.Token);
973+
await coordinator.StartDownloadsAsync(discoveryResult, null, cts.Token);
974974

975975
// Assert - no exception thrown, no S3 calls made
976976
mockClient.Verify(x => x.GetObjectAsync(It.IsAny<GetObjectRequest>(), It.IsAny<CancellationToken>()), Times.Never);
@@ -1021,7 +1021,7 @@ public async Task StartDownloadsAsync_CancellationPropagatesAcrossConcurrentDown
10211021
// Act
10221022
try
10231023
{
1024-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
1024+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
10251025
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
10261026
}
10271027
catch (OperationCanceledException)
@@ -1098,7 +1098,7 @@ public async Task StartDownloadsAsync_RangeStrategy_CancellationDuringDownloads(
10981098
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
10991099

11001100
// Act
1101-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
1101+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
11021102
await coordinator.DownloadCompletionTask; // Wait for background task to observe exceptions
11031103
}
11041104

@@ -1153,7 +1153,7 @@ public async Task StartDownloadsAsync_ReturnsImmediately_PreventsDeadlock()
11531153

11541154
// Act - StartDownloadsAsync should return immediately (not wait for all downloads)
11551155
var stopwatch = System.Diagnostics.Stopwatch.StartNew();
1156-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
1156+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
11571157
stopwatch.Stop();
11581158

11591159
// Assert - StartDownloadsAsync should return almost immediately
@@ -1202,7 +1202,7 @@ public async Task StartDownloadsAsync_SinglePart_ReturnsImmediatelyWithoutBackgr
12021202

12031203
// Act
12041204
var stopwatch = System.Diagnostics.Stopwatch.StartNew();
1205-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None);
1205+
await coordinator.StartDownloadsAsync(discoveryResult, null, CancellationToken.None);
12061206
stopwatch.Stop();
12071207

12081208
// DownloadCompletionTask should be completed immediately (no background work)
@@ -1761,7 +1761,7 @@ public async Task ProgressCallback_ConcurrentCompletion_FiresOnlyOneCompletionEv
17611761
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
17621762

17631763
// Act
1764-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None, progressCallback);
1764+
await coordinator.StartDownloadsAsync(discoveryResult, progressCallback, CancellationToken.None);
17651765

17661766
// Wait for async progress events to complete
17671767
var success = await WaitForProgressEventsAsync(progressEvents, progressLock, totalObjectSize);
@@ -1817,7 +1817,7 @@ public async Task ProgressCallback_MultiplePartsComplete_AggregatesCorrectly()
18171817
var discoveryResult = await coordinator.DiscoverDownloadStrategyAsync(CancellationToken.None);
18181818

18191819
// Act
1820-
await coordinator.StartDownloadsAsync(discoveryResult, CancellationToken.None, progressCallback);
1820+
await coordinator.StartDownloadsAsync(discoveryResult, progressCallback, CancellationToken.None);
18211821

18221822
// Wait for async progress events to complete
18231823
var success = await WaitForProgressEventsAsync(progressEvents, progressLock, totalObjectSize);

0 commit comments

Comments
 (0)