Skip to content

Commit 3eafd51

Browse files
NikaHsnNikaHsn
authored
feat(storage): update uploadData API to accept optional storage bucket param (#5540)
1 parent d68bb03 commit 3eafd51

19 files changed

+466
-70
lines changed

‎packages/amplify_core/lib/src/category/amplify_storage_category.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ class StorageCategory extends AmplifyCategory<StoragePluginInterface> {
144144
required StoragePath path,
145145
void Function(StorageTransferProgress)? onProgress,
146146
StorageUploadDataOptions? options,
147+
StorageBucket? bucket,
147148
}) {
148149
return identifyCall(
149150
StorageCategoryMethod.uploadData,

‎packages/amplify_core/lib/src/plugin/amplify_storage_plugin_interface.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ abstract class StoragePluginInterface extends AmplifyPluginInterface {
6363
required StorageDataPayload data,
6464
void Function(StorageTransferProgress)? onProgress,
6565
StorageUploadDataOptions? options,
66+
StorageBucket? bucket,
6667
}) {
6768
throw UnimplementedError('uploadData() has not been implemented.');
6869
}

‎packages/amplify_core/lib/src/types/storage/bucket_info.dart

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
1+
import 'package:amplify_core/amplify_core.dart';
2+
13
/// {@template amplify_core.storage.bucket_info}
24
/// Presents a storage bucket information.
35
/// {@endtemplate}
4-
class BucketInfo {
6+
class BucketInfo with AWSEquatable<BucketInfo> {
57
/// {@macro amplify_core.storage.bucket_info}
68
const BucketInfo({required this.bucketName, required this.region});
79
final String bucketName;
810
final String region;
11+
12+
@override
13+
List<Object?> get props => [
14+
bucketName,
15+
region,
16+
];
917
}

‎packages/amplify_core/lib/src/types/storage/storage_bucket_from_outputs.dart

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@ class StorageBucketFromOutputs implements StorageBucket {
1616
BucketInfo resolveBucketInfo(StorageOutputs? storageOutputs) {
1717
assert(
1818
storageOutputs != null,
19-
const InvalidStorageBucketException(
20-
'Amplify Outputs file does not have storage configuration.',
21-
recoverySuggestion:
22-
'Make sure Amplify Storage is configured and the Amplify Outputs '
23-
'file has storage configuration.',
24-
),
19+
'storageOutputs can not be null',
2520
);
2621
final buckets = storageOutputs!.buckets;
2722
if (buckets == null) {

‎packages/amplify_core/test/types/storage/storage_bucket_test.dart

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import 'package:amplify_core/amplify_core.dart';
2+
import 'package:amplify_core/src/config/amplify_outputs/storage/bucket_outputs.dart';
3+
import 'package:amplify_core/src/config/amplify_outputs/storage/storage_outputs.dart';
4+
import 'package:test/test.dart';
5+
6+
void main() {
7+
group('Storage bucket resolve BucketInfo', () {
8+
const defaultBucketOutputs = BucketOutputs(
9+
name: 'default-bucket-friendly-name',
10+
bucketName: 'default-bucket-unique-name',
11+
awsRegion: 'default-bucket-aws-region',
12+
);
13+
const secondBucketOutputs = BucketOutputs(
14+
name: 'second-bucket-friendly-name',
15+
bucketName: 'second-bucket-unique-name',
16+
awsRegion: 'second-bucket-aws-region',
17+
);
18+
final defaultBucketInfo = BucketInfo(
19+
bucketName: defaultBucketOutputs.bucketName,
20+
region: defaultBucketOutputs.awsRegion,
21+
);
22+
final secondBucketInfo = BucketInfo(
23+
bucketName: secondBucketOutputs.bucketName,
24+
region: secondBucketOutputs.awsRegion,
25+
);
26+
final testStorageOutputsMultiBucket = StorageOutputs(
27+
awsRegion: defaultBucketOutputs.awsRegion,
28+
bucketName: defaultBucketOutputs.bucketName,
29+
buckets: [
30+
defaultBucketOutputs,
31+
secondBucketOutputs,
32+
],
33+
);
34+
final testStorageOutputsSingleBucket = StorageOutputs(
35+
awsRegion: defaultBucketOutputs.awsRegion,
36+
bucketName: defaultBucketOutputs.bucketName,
37+
);
38+
39+
test(
40+
'should return same bucket info when storage bucket is created from'
41+
' a bucket info', () {
42+
final storageBucket = StorageBucket.fromBucketInfo(
43+
defaultBucketInfo,
44+
);
45+
final bucketInfo = storageBucket.resolveBucketInfo(null);
46+
expect(bucketInfo, defaultBucketInfo);
47+
});
48+
49+
test(
50+
'should return bucket info when storage bucket is created from'
51+
' buckets in storage outputs', () {
52+
final storageBucket = StorageBucket.fromOutputs(secondBucketOutputs.name);
53+
final bucketInfo =
54+
storageBucket.resolveBucketInfo(testStorageOutputsMultiBucket);
55+
expect(bucketInfo, secondBucketInfo);
56+
});
57+
58+
test(
59+
'should throw assertion error when storage bucket is created from'
60+
' outputs and storage outputs is null', () {
61+
final storageBucket =
62+
StorageBucket.fromOutputs(defaultBucketOutputs.name);
63+
expect(
64+
() => storageBucket.resolveBucketInfo(null),
65+
throwsA(isA<AssertionError>()),
66+
);
67+
});
68+
test(
69+
'should throw exception when storage bucket is created from outputs and'
70+
' storage outputs does not have buckets', () {
71+
final storageBucket = StorageBucket.fromOutputs('bucket-name');
72+
expect(
73+
() => storageBucket.resolveBucketInfo(testStorageOutputsSingleBucket),
74+
throwsA(isA<InvalidStorageBucketException>()),
75+
);
76+
});
77+
test(
78+
'should throw exception when storage bucket is created from outputs and'
79+
' bucket name does not match any bucket in storage outputs', () {
80+
final storageBucket = StorageBucket.fromOutputs('invalid-bucket-name');
81+
expect(
82+
() => storageBucket.resolveBucketInfo(testStorageOutputsMultiBucket),
83+
throwsA(isA<InvalidStorageBucketException>()),
84+
);
85+
});
86+
});
87+
}

‎packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ class AmplifyStorageS3Dart extends StoragePluginInterface
274274
required StoragePath path,
275275
void Function(S3TransferProgress)? onProgress,
276276
StorageUploadDataOptions? options,
277+
StorageBucket? bucket,
277278
}) {
278279
final s3PluginOptions = reifyPluginOptions(
279280
pluginOptions: options?.pluginOptions,
@@ -290,6 +291,7 @@ class AmplifyStorageS3Dart extends StoragePluginInterface
290291
dataPayload: data,
291292
options: s3Options,
292293
onProgress: onProgress,
294+
bucket: bucket,
293295
);
294296

295297
return S3UploadDataOperation(

‎packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/s3_client_info.dart

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,15 @@ import 'package:smithy_aws/smithy_aws.dart';
55
/// It holds Amazon S3 client information.
66
@internal
77
class S3ClientInfo {
8-
const S3ClientInfo({required this.client, required this.config});
8+
const S3ClientInfo({
9+
required this.client,
10+
required this.config,
11+
required this.bucketName,
12+
required this.awsRegion,
13+
});
914

1015
final S3Client client;
1116
final S3ClientConfig config;
17+
final String bucketName;
18+
final String awsRegion;
1219
}

‎packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -330,15 +330,13 @@ class StorageS3Service {
330330
FutureOr<void> Function()? onError,
331331
StorageBucket? bucket,
332332
}) {
333-
// ignore: invalid_use_of_internal_member
334-
final bucketName = bucket?.resolveBucketInfo(_storageOutputs).bucketName ??
335-
_storageOutputs.bucketName;
336-
final s3ClientInfo = _getS3ClientInfo(bucket);
333+
final s3ClientInfo = getS3ClientInfo(storageBucket: bucket);
337334
final uploadDataTask = S3UploadTask.fromDataPayload(
338335
dataPayload,
339336
s3Client: s3ClientInfo.client,
340-
defaultS3ClientConfig: s3ClientInfo.config,
341-
bucket: bucketName,
337+
s3ClientConfig: s3ClientInfo.config,
338+
bucket: s3ClientInfo.bucketName,
339+
awsRegion: s3ClientInfo.awsRegion,
342340
path: path,
343341
options: options,
344342
pathResolver: _pathResolver,
@@ -376,8 +374,9 @@ class StorageS3Service {
376374
final uploadDataTask = S3UploadTask.fromAWSFile(
377375
localFile,
378376
s3Client: _defaultS3Client,
379-
defaultS3ClientConfig: _defaultS3ClientConfig,
377+
s3ClientConfig: _defaultS3ClientConfig,
380378
bucket: _storageOutputs.bucketName,
379+
awsRegion: _storageOutputs.awsRegion,
381380
path: path,
382381
options: uploadDataOptions,
383382
pathResolver: _pathResolver,
@@ -604,29 +603,42 @@ class StorageS3Service {
604603
Future<void> abortIncompleteMultipartUploads() async {
605604
final records = await _transferDatabase
606605
.getMultipartUploadRecordsCreatedBefore(_serviceStartingTime);
607-
608606
for (final record in records) {
607+
final bucketInfo = BucketInfo(
608+
bucketName: record.bucketName ?? _storageOutputs.bucketName,
609+
region: record.awsRegion ?? _storageOutputs.awsRegion,
610+
);
609611
final request = s3.AbortMultipartUploadRequest.build((builder) {
610612
builder
611-
..bucket = _storageOutputs.bucketName
613+
..bucket = bucketInfo.bucketName
612614
..key = record.objectKey
613615
..uploadId = record.uploadId;
614616
});
617+
final s3Client = getS3ClientInfo(
618+
storageBucket: StorageBucket.fromBucketInfo(bucketInfo),
619+
).client;
615620

616621
try {
617-
await _defaultS3Client.abortMultipartUpload(request).result;
622+
await s3Client.abortMultipartUpload(request).result;
618623
await _transferDatabase.deleteTransferRecords(record.uploadId);
619624
} on Exception catch (error) {
620625
_logger.error('Failed to abort multipart upload due to: $error');
621626
}
622627
}
623628
}
624629

625-
S3ClientInfo _getS3ClientInfo(StorageBucket? storageBucket) {
630+
/// Creates and caches [S3ClientInfo] given the optional [storageBucket]
631+
/// parameter. If the optional parameter is not provided it uses
632+
/// StorageOutputs default bucket to create the [S3ClientInfo].
633+
@internal
634+
@visibleForTesting
635+
S3ClientInfo getS3ClientInfo({StorageBucket? storageBucket}) {
626636
if (storageBucket == null) {
627637
return S3ClientInfo(
628638
client: _defaultS3Client,
629639
config: _defaultS3ClientConfig,
640+
bucketName: _storageOutputs.bucketName,
641+
awsRegion: _storageOutputs.awsRegion,
630642
);
631643
}
632644
// ignore: invalid_use_of_internal_member
@@ -658,6 +670,8 @@ class StorageS3Service {
658670
final s3ClientInfo = S3ClientInfo(
659671
client: s3Client,
660672
config: s3ClientConfig,
673+
bucketName: bucketInfo.bucketName,
674+
awsRegion: bucketInfo.region,
661675
);
662676
_s3ClientsInfo[bucketInfo.bucketName] = s3ClientInfo;
663677
return s3ClientInfo;

‎packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/task/s3_upload_task.dart

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ const fallbackContentType = 'application/octet-stream';
4848
class S3UploadTask {
4949
S3UploadTask._({
5050
required s3.S3Client s3Client,
51-
required smithy_aws.S3ClientConfig defaultS3ClientConfig,
51+
required smithy_aws.S3ClientConfig s3ClientConfig,
5252
required S3PathResolver pathResolver,
5353
required String bucket,
54+
required String awsRegion,
5455
required StoragePath path,
5556
required StorageUploadDataOptions options,
5657
S3DataPayload? dataPayload,
@@ -59,9 +60,10 @@ class S3UploadTask {
5960
required AWSLogger logger,
6061
required transfer.TransferDatabase transferDatabase,
6162
}) : _s3Client = s3Client,
62-
_defaultS3ClientConfig = defaultS3ClientConfig,
63+
_s3ClientConfig = s3ClientConfig,
6364
_pathResolver = pathResolver,
6465
_bucket = bucket,
66+
_awsRegion = awsRegion,
6567
_path = path,
6668
_options = options,
6769
_dataPayload = dataPayload,
@@ -81,19 +83,21 @@ class S3UploadTask {
8183
S3UploadTask.fromDataPayload(
8284
S3DataPayload dataPayload, {
8385
required s3.S3Client s3Client,
84-
required smithy_aws.S3ClientConfig defaultS3ClientConfig,
86+
required smithy_aws.S3ClientConfig s3ClientConfig,
8587
required S3PathResolver pathResolver,
8688
required String bucket,
89+
required String awsRegion,
8790
required StoragePath path,
8891
required StorageUploadDataOptions options,
8992
void Function(S3TransferProgress)? onProgress,
9093
required AWSLogger logger,
9194
required transfer.TransferDatabase transferDatabase,
9295
}) : this._(
9396
s3Client: s3Client,
94-
defaultS3ClientConfig: defaultS3ClientConfig,
97+
s3ClientConfig: s3ClientConfig,
9598
pathResolver: pathResolver,
9699
bucket: bucket,
100+
awsRegion: awsRegion,
97101
path: path,
98102
dataPayload: dataPayload,
99103
options: options,
@@ -108,19 +112,21 @@ class S3UploadTask {
108112
S3UploadTask.fromAWSFile(
109113
AWSFile localFile, {
110114
required s3.S3Client s3Client,
111-
required smithy_aws.S3ClientConfig defaultS3ClientConfig,
115+
required smithy_aws.S3ClientConfig s3ClientConfig,
112116
required S3PathResolver pathResolver,
113117
required String bucket,
118+
required String awsRegion,
114119
required StoragePath path,
115120
required StorageUploadDataOptions options,
116121
void Function(S3TransferProgress)? onProgress,
117122
required AWSLogger logger,
118123
required transfer.TransferDatabase transferDatabase,
119124
}) : this._(
120125
s3Client: s3Client,
121-
defaultS3ClientConfig: defaultS3ClientConfig,
126+
s3ClientConfig: s3ClientConfig,
122127
pathResolver: pathResolver,
123128
bucket: bucket,
129+
awsRegion: awsRegion,
124130
path: path,
125131
localFile: localFile,
126132
options: options,
@@ -135,9 +141,10 @@ class S3UploadTask {
135141
final Completer<S3Item> _uploadCompleter = Completer();
136142

137143
final s3.S3Client _s3Client;
138-
final smithy_aws.S3ClientConfig _defaultS3ClientConfig;
144+
final smithy_aws.S3ClientConfig _s3ClientConfig;
139145
final S3PathResolver _pathResolver;
140146
final String _bucket;
147+
final String _awsRegion;
141148
final StoragePath _path;
142149
final StorageUploadDataOptions _options;
143150
final void Function(S3TransferProgress)? _onProgress;
@@ -191,7 +198,7 @@ class S3UploadTask {
191198
/// Should be used only internally.
192199
Future<void> start() async {
193200
if (_s3PluginOptions.useAccelerateEndpoint &&
194-
_defaultS3ClientConfig.usePathStyle) {
201+
_s3ClientConfig.usePathStyle) {
195202
_completeUploadWithError(s3_exception.accelerateEndpointUnusable);
196203
return;
197204
}
@@ -328,7 +335,7 @@ class S3UploadTask {
328335
try {
329336
_putObjectOperation = _s3Client.putObject(
330337
putObjectRequest,
331-
s3ClientConfig: _defaultS3ClientConfig.copyWith(
338+
s3ClientConfig: _s3ClientConfig.copyWith(
332339
useAcceleration: _s3PluginOptions.useAccelerateEndpoint,
333340
),
334341
);
@@ -497,6 +504,8 @@ class S3UploadTask {
497504
TransferRecord(
498505
uploadId: uploadId,
499506
objectKey: _resolvedPath,
507+
bucketName: _bucket,
508+
awsRegion: _awsRegion,
500509
createdAt: DateTime.now(),
501510
),
502511
);
@@ -651,7 +660,7 @@ class S3UploadTask {
651660
try {
652661
final operation = _s3Client.uploadPart(
653662
request,
654-
s3ClientConfig: _defaultS3ClientConfig.copyWith(
663+
s3ClientConfig: _s3ClientConfig.copyWith(
655664
useAcceleration: _s3PluginOptions.useAccelerateEndpoint,
656665
),
657666
);

0 commit comments

Comments
 (0)