Skip to content

Commit fb2417f

Browse files
0marperez0marperez
committed
feedback
1 parent 71ea5b7 commit fb2417f

File tree

18 files changed

+136
-116
lines changed

18 files changed

+136
-116
lines changed

‎aws-runtime/aws-http/api/aws-http.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ public final class aws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsB
172172
public static final field DDB_MAPPER Laws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetric;
173173
public static final field S3_EXPRESS_BUCKET Laws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetric;
174174
public static final field S3_TRANSFER Laws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetric;
175+
public static final field S3_TRANSFER_DOWNLOAD_DIRECTORY Laws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetric;
176+
public static final field S3_TRANSFER_UPLOAD_DIRECTORY Laws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetric;
175177
public static fun getEntries ()Lkotlin/enums/EnumEntries;
176178
public fun getIdentifier ()Ljava/lang/String;
177179
public fun toString ()Ljava/lang/String;

‎aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/businessmetrics/AwsBusinessMetricsUtils.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ public enum class AwsBusinessMetric(public override val identifier: String) : Bu
6363
S3_EXPRESS_BUCKET("J"),
6464
DDB_MAPPER("d"),
6565
S3_TRANSFER("G"),
66+
S3_TRANSFER_UPLOAD_DIRECTORY("9"),
67+
S3_TRANSFER_DOWNLOAD_DIRECTORY("+"),
6668
;
6769

6870
@InternalApi

‎gradle/libs.versions.toml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@ mockk-version = "1.13.13"
2828
slf4j-version = "2.0.16"
2929
jsoup-version = "1.20.1"
3030

31-
# s3 transfer manager
32-
s3-version = "1.5.62"
33-
3431
[libraries]
3532
aws-kotlin-repo-tools-build-support = { module="aws.sdk.kotlin.gradle:build-support", version.ref = "aws-kotlin-repo-tools-version" }
3633

@@ -129,7 +126,6 @@ kotlinx-serialization-json = { module = "org.jetbrains.kotlinx:kotlinx-serializa
129126
mockk = { module = "io.mockk:mockk", version.ref = "mockk-version" }
130127

131128
ddb-local = { module = "com.amazonaws:DynamoDBLocal", version.ref = "ddb-local-version" }
132-
s3 = { module = "aws.sdk.kotlin:s3", version.ref = "s3-version" }
133129

134130
[bundles]
135131
# bundle of smithy-kotlin dependencies all AWS service clients have

‎hll/s3-transfer-manager/api/s3-transfer-manager.api‎

Lines changed: 31 additions & 28 deletions
Large diffs are not rendered by default.

‎hll/s3-transfer-manager/build.gradle.kts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,32 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import org.gradle.kotlin.dsl.dependencies
7+
import org.gradle.kotlin.dsl.sourceSets
8+
19
/*
210
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
311
* SPDX-License-Identifier: Apache-2.0
412
*/
513

614
description = "S3 Transfer Manager for the AWS SDK for Kotlin"
7-
extra["displayName"] = "AWS :: SDK :: Kotlin :: HLL :: S3TransferManager"
15+
extra["displayName"] = "AWS :: SDK :: Kotlin :: HLL :: S3 Transfer Manager"
816
extra["moduleName"] = "aws.sdk.kotlin.hll.s3transfermanager"
917

1018
kotlin {
1119
sourceSets {
1220
commonMain {
1321
dependencies {
1422
implementation(project(":aws-runtime:aws-http"))
15-
implementation(libs.s3)
23+
implementation(project(":services:s3"))
1624
}
1725
}
1826
jvmTest {
1927
dependencies {
2028
implementation(libs.smithy.kotlin.test.jvm)
2129
implementation(libs.smithy.kotlin.testing.jvm)
22-
implementation(libs.smithy.kotlin.aws.signing.common)
2330
}
2431
}
2532
}

‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/S3TransferManager.kt‎

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55

66
package aws.sdk.kotlin.hll.s3transfermanager
77

8-
import aws.sdk.kotlin.hll.s3transfermanager.model.MultiPartDownloadType
8+
import aws.sdk.kotlin.hll.s3transfermanager.model.MultipartDownloadType
99
import aws.sdk.kotlin.hll.s3transfermanager.model.Part
1010
import aws.sdk.kotlin.hll.s3transfermanager.model.UploadFileRequest
1111
import aws.sdk.kotlin.hll.s3transfermanager.model.UploadFileResponse
12+
import aws.sdk.kotlin.hll.s3transfermanager.utils.S3TransferManagerException
1213
import aws.sdk.kotlin.hll.s3transfermanager.utils.buildCompleteMultipartUploadRequest
1314
import aws.sdk.kotlin.hll.s3transfermanager.utils.buildUploadPartRequest
1415
import aws.sdk.kotlin.hll.s3transfermanager.utils.ceilDiv
@@ -41,9 +42,9 @@ import kotlinx.coroutines.coroutineScope
4142
*/
4243
public class S3TransferManager private constructor(
4344
public val client: S3Client,
44-
public val targePartSize: Long,
45-
public val multipartUploadThreshold: Long,
46-
public val multipartDownloadType: MultiPartDownloadType,
45+
public val partSizeBytes: Long,
46+
public val multipartUploadThresholdBytes: Long,
47+
public val multipartDownloadType: MultipartDownloadType,
4748
public val interceptors: MutableList<TransferInterceptor>,
4849
) {
4950
internal var context: TransferContext = TransferContext()
@@ -55,16 +56,16 @@ public class S3TransferManager private constructor(
5556

5657
public class Builder {
5758
public var client: S3Client? = null
58-
public var targePartSize: Long = 8_000_000
59-
public var multipartUploadThreshold: Long = 16_000_000L
60-
public var multipartDownloadType: MultiPartDownloadType = Part
59+
public var partSizeBytes: Long = 8_000_000
60+
public var multipartUploadThresholdBytes: Long = 16_000_000L
61+
public var multipartDownloadType: MultipartDownloadType = Part
6162
public var interceptors: MutableList<TransferInterceptor> = mutableListOf()
6263

6364
internal fun build(): S3TransferManager =
6465
S3TransferManager(
65-
client = client?.withConfig { interceptors += BusinessMetricInterceptor } ?: error("client must be set"),
66-
targePartSize = targePartSize,
67-
multipartUploadThreshold = multipartUploadThreshold,
66+
client = client?.withConfig { interceptors += S3TransferManagerBusinessMetricInterceptor } ?: error("client must be set"),
67+
partSizeBytes = partSizeBytes,
68+
multipartUploadThresholdBytes = multipartUploadThresholdBytes,
6869
multipartDownloadType = multipartDownloadType,
6970
interceptors = interceptors,
7071
)
@@ -117,17 +118,18 @@ public class S3TransferManager private constructor(
117118
* for large objects as needed.
118119
*
119120
* This function handles the complexity of splitting the data into parts,
120-
* uploading each part, and completing the multipart upload. For object smaller than [multipartUploadThreshold],
121+
* uploading each part, and completing the multipart upload. For object smaller than [multipartUploadThresholdBytes],
121122
* a standard single-part upload is performed automatically.
122123
*
123-
* If the specified [targePartSize] for multipart uploads is too small to allow
124+
* If the specified [partSizeBytes] for multipart uploads is too small to allow
124125
* all parts to fit within S3's limit of 10,000 parts, the part size will be
125126
* automatically increased so that exactly 10,000 parts are uploaded.
126127
*/
127128
public suspend fun uploadFile(uploadFileRequest: UploadFileRequest): Deferred<UploadFileResponse> = coroutineScope {
128-
val multiPartUpload = uploadFileRequest.contentLength >= multipartUploadThreshold
129+
val contentLength = uploadFileRequest.body?.contentLength ?: throw S3TransferManagerException("UploadFileRequest.body.contentLength must be set")
130+
val multiPartUpload = contentLength >= multipartUploadThresholdBytes
129131
val uploadedParts = mutableListOf<CompletedPart>()
130-
var mpuUploadId = "null"
132+
lateinit var mpuUploadId: String
131133

132134
val logger = coroutineContext.logger<S3TransferManager>()
133135

@@ -136,7 +138,7 @@ public class S3TransferManager private constructor(
136138
*/
137139
suspend fun transferInitiated(multiPartUpload: Boolean) {
138140
context.transferredBytes = 0L
139-
context.transferableBytes = uploadFileRequest.contentLength
141+
context.transferableBytes = contentLength
140142
context.request = if (multiPartUpload) {
141143
uploadFileRequest.toCreateMultiPartUploadRequest()
142144
} else {
@@ -145,7 +147,7 @@ public class S3TransferManager private constructor(
145147
operationHook(TransferInitiated) {
146148
if (multiPartUpload) {
147149
context.response = client.createMultipartUpload(context.request as CreateMultipartUploadRequest)
148-
mpuUploadId = (context.response as CreateMultipartUploadResponse).uploadId ?: throw Exception("Missing upload id in create multipart upload response")
150+
mpuUploadId = (context.response as CreateMultipartUploadResponse).uploadId ?: throw S3TransferManagerException("Missing upload id in create multipart upload response")
149151
}
150152
}
151153
}
@@ -156,22 +158,22 @@ public class S3TransferManager private constructor(
156158
suspend fun transferBytes(multiPartUpload: Boolean) {
157159
if (multiPartUpload) {
158160
try {
159-
val partSize = resolvePartSize(uploadFileRequest, this@S3TransferManager, logger)
160-
val numberOfParts = ceilDiv(uploadFileRequest.contentLength, partSize)
161+
val partSize = resolvePartSize(contentLength, this@S3TransferManager, logger)
162+
val numberOfParts = ceilDiv(contentLength, partSize)
161163
val partSource = when (uploadFileRequest.body) {
162164
is ByteStream.Buffer -> uploadFileRequest.body.bytes()
163165
is ByteStream.ChannelStream -> uploadFileRequest.body.readFrom()
164166
is ByteStream.SourceStream -> uploadFileRequest.body.readFrom()
165-
else -> error("Unhandled body type: ${uploadFileRequest.body?.let { it::class.simpleName } ?: "null"}")
167+
else -> throw S3TransferManagerException("Unhandled body type: ${uploadFileRequest.body?.let { it::class.simpleName } ?: "null"}")
166168
}
167169
val partBuffer = SdkBuffer()
168170
var currentPartNumber = 1L
169171

170172
while (context.transferredBytes!! < context.transferableBytes!!) {
171173
partBuffer.getNextPart(partSource, partSize, this@S3TransferManager)
172174
if (currentPartNumber != numberOfParts) {
173-
check(partBuffer.size == partSize) {
174-
"Part #$currentPartNumber size mismatch detected. Expected $partSize, actual: ${partBuffer.size}"
175+
if (partBuffer.size != partSize) {
176+
throw S3TransferManagerException("Part #$currentPartNumber size mismatch detected. Expected $partSize, actual: ${partBuffer.size}")
175177
}
176178
}
177179

@@ -195,8 +197,8 @@ public class S3TransferManager private constructor(
195197
currentPartNumber += 1
196198
}
197199

198-
check(uploadedParts.size == numberOfParts.toInt()) {
199-
"The number of uploaded parts does not match the expected count. Expected $numberOfParts, actual: ${uploadedParts.size}"
200+
if (uploadedParts.size != numberOfParts.toInt()) {
201+
throw S3TransferManagerException("The number of uploaded parts does not match the expected count. Expected $numberOfParts, actual: ${uploadedParts.size}")
200202
}
201203
} catch (uploadPartThrowable: Throwable) {
202204
try {
@@ -207,9 +209,9 @@ public class S3TransferManager private constructor(
207209
requestPayer = uploadFileRequest.requestPayer
208210
uploadId = mpuUploadId
209211
}
210-
throw Exception("Multipart upload failed (ID: $mpuUploadId). One or more parts could not be uploaded", uploadPartThrowable)
212+
throw S3TransferManagerException("Multipart upload failed (ID: $mpuUploadId). One or more parts could not be uploaded", uploadPartThrowable)
211213
} catch (abortThrowable: Throwable) {
212-
throw Exception("Multipart upload failed (ID: $mpuUploadId). Unable to abort multipart upload.", abortThrowable)
214+
throw S3TransferManagerException("Multipart upload failed (ID: $mpuUploadId). Unable to abort multipart upload.", abortThrowable)
213215
}
214216
}
215217
} else {
@@ -232,28 +234,21 @@ public class S3TransferManager private constructor(
232234
try {
233235
context.response = client.completeMultipartUpload(context.request as CompleteMultipartUploadRequest)
234236
} catch (t: Throwable) {
235-
throw Exception("Unable to complete multipart upload with ID: $mpuUploadId", t)
237+
throw S3TransferManagerException("Unable to complete multipart upload with ID: $mpuUploadId", t)
236238
}
237239
}
238240
}
239241
}
240242

241243
async {
242-
checkNotNull(uploadFileRequest.body?.contentLength) {
243-
"UploadFileRequest.body.contentLength must be set"
244-
}
245-
check(uploadFileRequest.body.contentLength == uploadFileRequest.contentLength) {
246-
"contentLength mismatch. uploadFileRequest: ${uploadFileRequest.contentLength}, uploadFileRequest.body.contentLength: ${uploadFileRequest.body.contentLength}"
247-
}
248-
249244
transferInitiated(multiPartUpload)
250245
transferBytes(multiPartUpload)
251246
transferComplete(multiPartUpload)
252247

253248
when (context.response) {
254249
is PutObjectResponse -> (context.response as PutObjectResponse).toUploadFileResponse()
255250
is CompleteMultipartUploadResponse -> (context.response as CompleteMultipartUploadResponse).toUploadFileResponse()
256-
else -> error("Unexpected response: ${context.response?.let { it::class.simpleName } ?: "null"}")
251+
else -> throw S3TransferManagerException("Unexpected response: ${context.response?.let { it::class.simpleName } ?: "null"}")
257252
}
258253
}
259254
}
@@ -263,10 +258,10 @@ public class S3TransferManager private constructor(
263258
* for large objects as needed.
264259
*
265260
* This function handles the complexity of splitting the data into parts,
266-
* uploading each part, and completing the multipart upload. For object smaller than [multipartUploadThreshold],
261+
* uploading each part, and completing the multipart upload. For object smaller than [multipartUploadThresholdBytes],
267262
* a standard single-part upload is performed automatically.
268263
*
269-
* If the specified [targePartSize] for multipart uploads is too small to allow
264+
* If the specified [partSizeBytes] for multipart uploads is too small to allow
270265
* all parts to fit within S3's limit of 10,000 parts, the part size will be
271266
* automatically increased so that exactly 10,000 parts are uploaded.
272267
*/

‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/BusinessMetricInterceptor.kt‎ renamed to ‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/S3TransferManagerBusinessMetricInterceptor.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor
1313
/**
1414
* An interceptor that emits the S3 Transfer Manager business metric
1515
*/
16-
internal object BusinessMetricInterceptor : HttpInterceptor {
16+
internal object S3TransferManagerBusinessMetricInterceptor : HttpInterceptor {
1717
override suspend fun modifyBeforeSerialization(context: RequestInterceptorContext<Any>): Any {
1818
context.executionContext.emitBusinessMetric(AwsBusinessMetric.S3_TRANSFER)
1919
return context.request

‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/TransferInterceptor.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package aws.sdk.kotlin.hll.s3transfermanager
77

8+
import aws.smithy.kotlin.runtime.content.ByteStream
9+
810
// TODO: KDocs
911

1012
public data class TransferContext(
@@ -14,7 +16,7 @@ public data class TransferContext(
1416

1517
// Byte transfers
1618
var transferableBytes: Long? = null,
17-
var currentBytes: ByteArray? = null,
19+
var currentBytes: ByteStream? = null,
1820
var transferredBytes: Long? = null,
1921

2022
// File transfers

‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/model/MultiPartDownloadType.kt‎ renamed to ‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/model/MultipartDownloadType.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ package aws.sdk.kotlin.hll.s3transfermanager.model
77

88
// TODO: KDocs
99

10-
public sealed interface MultiPartDownloadType
11-
public object Range : MultiPartDownloadType
12-
public object Part : MultiPartDownloadType
10+
public sealed interface MultipartDownloadType
11+
public object Range : MultipartDownloadType
12+
public object Part : MultipartDownloadType

‎hll/s3-transfer-manager/common/src/aws/sdk/kotlin/hll/s3transfermanager/model/UploadFileRequest.kt‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ public class UploadFileRequest private constructor(
3030
public val contentDisposition: String?,
3131
public val contentEncoding: String?,
3232
public val contentLanguage: String?,
33-
public val contentLength: Long,
3433
public val contentType: String?,
3534
public val expectedBucketOwner: String?,
3635
public val expires: Instant?,
@@ -46,13 +45,12 @@ public class UploadFileRequest private constructor(
4645
public val objectLockMode: ObjectLockMode?,
4746
public val objectLockRetainUntilDate: Instant?,
4847
public val requestPayer: RequestPayer?,
49-
public val serverSideEncryption: ServerSideEncryption?,
50-
public val source: String?,
5148
public val sseCustomerAlgorithm: String?,
5249
public val sseCustomerKey: String?,
5350
public val sseCustomerKeyMd5: String?,
5451
public val ssekmsEncryptionContext: String?,
5552
public val ssekmsKeyId: String?,
53+
public val serverSideEncryption: ServerSideEncryption?,
5654
public val storageClass: StorageClass?,
5755
public val tagging: String?,
5856
public val websiteRedirectLocation: String?,
@@ -77,7 +75,6 @@ public class UploadFileRequest private constructor(
7775
public var contentDisposition: String? = null
7876
public var contentEncoding: String? = null
7977
public var contentLanguage: String? = null
80-
public var contentLength: Long? = null
8178
public var contentType: String? = null
8279
public var expectedBucketOwner: String? = null
8380
public var expires: Instant? = null
@@ -93,13 +90,12 @@ public class UploadFileRequest private constructor(
9390
public var objectLockMode: ObjectLockMode? = null
9491
public var objectLockRetainUntilDate: Instant? = null
9592
public var requestPayer: RequestPayer? = null
96-
public var source: String? = null
97-
public var serverSideEncryption: ServerSideEncryption? = null
9893
public var sseCustomerAlgorithm: String? = null
9994
public var sseCustomerKey: String? = null
10095
public var sseCustomerKeyMd5: String? = null
10196
public var ssekmsEncryptionContext: String? = null
10297
public var ssekmsKeyId: String? = null
98+
public var serverSideEncryption: ServerSideEncryption? = null
10399
public var storageClass: StorageClass? = null
104100
public var tagging: String? = null
105101
public var websiteRedirectLocation: String? = null
@@ -120,7 +116,6 @@ public class UploadFileRequest private constructor(
120116
contentDisposition,
121117
contentEncoding,
122118
contentLanguage,
123-
contentLength ?: error("contentLength must be set"),
124119
contentType,
125120
expectedBucketOwner,
126121
expires,
@@ -136,13 +131,12 @@ public class UploadFileRequest private constructor(
136131
objectLockMode,
137132
objectLockRetainUntilDate,
138133
requestPayer,
139-
serverSideEncryption,
140-
source,
141134
sseCustomerAlgorithm,
142135
sseCustomerKey,
143136
sseCustomerKeyMd5,
144137
ssekmsEncryptionContext,
145138
ssekmsKeyId,
139+
serverSideEncryption,
146140
storageClass,
147141
tagging,
148142
websiteRedirectLocation,

0 commit comments

Comments
 (0)