Skip to content

Commit 4a70062

Browse files
committed
code refactor src
update tests add back tests copilot comments add more tests add back test
1 parent 81d2a18 commit 4a70062

11 files changed

+481
-601
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)