Skip to content

Commit 6fa4e5a

Browse files
committed
code refactor src
update tests add back tests copilot comments add more tests add back test add back test move tests add test back
1 parent 77c7972 commit 6fa4e5a

11 files changed

+492
-528
lines changed

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,16 @@ internal class BufferedMultipartStream : Stream
4343

4444
private bool _initialized = false;
4545
private bool _disposed = false;
46-
private DownloadDiscoveryResult _discoveryResult;
46+
private DownloadResult _discoveryResult;
4747
private long _totalBytesRead = 0;
4848

4949
private readonly Logger _logger = Logger.GetLogger(typeof(BufferedMultipartStream));
5050

5151
/// <summary>
52-
/// Gets the <see cref="DownloadDiscoveryResult"/> containing metadata from the initial GetObject response.
52+
/// Gets the <see cref="DownloadResult"/> containing metadata from the initial GetObject response.
5353
/// Available after <see cref="InitializeAsync"/> completes successfully.
5454
/// </summary>
55-
public DownloadDiscoveryResult DiscoveryResult => _discoveryResult;
55+
public DownloadResult DiscoveryResult => _discoveryResult;
5656

5757
/// <summary>
5858
/// Creates a new <see cref="BufferedMultipartStream"/> with dependency injection.
@@ -112,16 +112,14 @@ public async Task InitializeAsync(CancellationToken cancellationToken)
112112

113113
_logger.DebugFormat("BufferedMultipartStream: Starting initialization");
114114

115-
_discoveryResult = await _downloadCoordinator.DiscoverDownloadStrategyAsync(cancellationToken)
116-
.ConfigureAwait(false);
117-
118-
_logger.DebugFormat("BufferedMultipartStream: Discovery completed - ObjectSize={0}, TotalParts={1}, IsSinglePart={2}",
115+
// Start unified download operation (discovers strategy and starts downloads)
116+
_discoveryResult = await _downloadCoordinator.StartDownloadAsync(null, cancellationToken)
117+
.ConfigureAwait(false);
118+
119+
_logger.DebugFormat("BufferedMultipartStream: Download started - ObjectSize={0}, TotalParts={1}, IsSinglePart={2}",
119120
_discoveryResult.ObjectSize,
120121
_discoveryResult.TotalParts,
121122
_discoveryResult.IsSinglePart);
122-
123-
await _downloadCoordinator.StartDownloadsAsync(_discoveryResult, null, cancellationToken)
124-
.ConfigureAwait(false);
125123

126124
_initialized = true;
127125
_logger.DebugFormat("BufferedMultipartStream: Initialization completed successfully");

sdk/src/Services/S3/Custom/Transfer/Internal/BufferedPartDataHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public BufferedPartDataHandler(
7171
_config = config ?? throw new ArgumentNullException(nameof(config));
7272
}
7373

74-
public Task PrepareAsync(DownloadDiscoveryResult discoveryResult, CancellationToken cancellationToken)
74+
public Task PrepareAsync(DownloadResult discoveryResult, CancellationToken cancellationToken)
7575
{
7676
// No preparation needed for buffered handler - buffers are created on demand
7777
return Task.CompletedTask;

sdk/src/Services/S3/Custom/Transfer/Internal/FilePartDataHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public FilePartDataHandler(FileDownloadConfiguration config)
5555
}
5656

5757
/// <inheritdoc/>
58-
public Task PrepareAsync(DownloadDiscoveryResult discoveryResult, CancellationToken cancellationToken)
58+
public Task PrepareAsync(DownloadResult discoveryResult, CancellationToken cancellationToken)
5959
{
6060
// Create temporary file once during preparation phase
6161
_tempFilePath = _fileHandler.CreateTemporaryFile(_config.DestinationFilePath);

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

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,23 +32,29 @@ namespace Amazon.S3.Transfer.Internal
3232
internal interface IDownloadManager : IDisposable
3333
{
3434
/// <summary>
35-
/// Discovers whether the object requires single-part or multipart downloading.
35+
/// Discovers the download strategy and starts concurrent downloads in a single operation.
36+
/// This unified method eliminates resource leakage by managing HTTP slots and buffer capacity
37+
/// internally throughout the entire download lifecycle.
3638
/// </summary>
37-
/// <param name="cancellationToken">A token to cancel the discovery operation.</param>
38-
/// <returns>
39-
/// A task containing discovery results including total parts, object size,
40-
/// and initial response data if single-part.
41-
/// </returns>
42-
Task<DownloadDiscoveryResult> DiscoverDownloadStrategyAsync(CancellationToken cancellationToken);
43-
44-
/// <summary>
45-
/// Starts concurrent downloads with HTTP concurrency control and part range calculations.
46-
/// </summary>
47-
/// <param name="discoveryResult">Results from the discovery phase.</param>
4839
/// <param name="progressCallback">Optional callback for progress tracking events.</param>
4940
/// <param name="cancellationToken">A token to cancel the download operation.</param>
50-
/// <returns>A task that completes when all downloads finish or an error occurs.</returns>
51-
Task StartDownloadsAsync(DownloadDiscoveryResult discoveryResult, EventHandler<WriteObjectProgressArgs> progressCallback, CancellationToken cancellationToken);
41+
/// <returns>
42+
/// A task containing download results including total parts, object size,
43+
/// and initial response data.
44+
/// </returns>
45+
/// <remarks>
46+
/// This method performs both discovery and download operations atomically:
47+
/// 1. Acquires HTTP slot and buffer capacity
48+
/// 2. Makes initial GetObject request to discover download strategy
49+
/// 3. Processes Part 1 immediately
50+
/// 4. Starts background downloads for remaining parts (if multipart)
51+
/// 5. Returns after Part 1 is processed, allowing consumer to begin reading
52+
///
53+
/// Resources (HTTP slots, buffer capacity) are managed internally and released
54+
/// at the appropriate times, eliminating the awkward resource holding that existed
55+
/// with the previous two-method API.
56+
/// </remarks>
57+
Task<DownloadResult> StartDownloadAsync(EventHandler<WriteObjectProgressArgs> progressCallback, CancellationToken cancellationToken);
5258

5359
/// <summary>
5460
/// Exception that occurred during downloads, if any.
@@ -57,9 +63,9 @@ internal interface IDownloadManager : IDisposable
5763
}
5864

5965
/// <summary>
60-
/// Download discovery results with metadata for determining download strategy.
66+
/// Download results with metadata about the completed discovery and initial download.
6167
/// </summary>
62-
internal class DownloadDiscoveryResult
68+
internal class DownloadResult
6369
{
6470
/// <summary>
6571
/// Total parts needed (1 = single-part, >1 = multipart).
@@ -72,7 +78,8 @@ internal class DownloadDiscoveryResult
7278
public long ObjectSize { get; set; }
7379

7480
/// <summary>
75-
/// GetObjectResponse obtained during download initialization, containing the ResponseStream. Represents the complete object for single-part downloads or the first range/part for multipart downloads.
81+
/// GetObjectResponse obtained during download initialization, containing the ResponseStream.
82+
/// Represents the complete object for single-part downloads or the first range/part for multipart downloads.
7683
/// </summary>
7784
public GetObjectResponse InitialResponse { get; set; }
7885

sdk/src/Services/S3/Custom/Transfer/Internal/IPartDataHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ internal interface IPartDataHandler : IDisposable
4040
/// <param name="discoveryResult">Discovery result containing object metadata</param>
4141
/// <param name="cancellationToken">Cancellation token</param>
4242
/// <returns>Task that completes when preparation is done</returns>
43-
Task PrepareAsync(DownloadDiscoveryResult discoveryResult, CancellationToken cancellationToken);
43+
Task PrepareAsync(DownloadResult discoveryResult, CancellationToken cancellationToken);
4444

4545
/// <summary>
4646
/// Process a downloaded part from the GetObjectResponse.

0 commit comments

Comments
 (0)