From 54c9c2b86eb61ca9848bcc8a30f4d0906b5ff5d9 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Thu, 24 Oct 2024 12:21:43 -0500 Subject: [PATCH 1/2] feat(storage): multi bucket remove --- .../lib/src/types/storage/remove_options.dart | 9 +- .../example/integration_test/remove_test.dart | 85 +++++++++++++++++++ .../integration_test/utils/object_exists.dart | 24 +++++- .../integration_test/utils/tear_down.dart | 21 +++++ .../service/storage_s3_service_impl.dart | 5 +- 5 files changed, 139 insertions(+), 5 deletions(-) diff --git a/packages/amplify_core/lib/src/types/storage/remove_options.dart b/packages/amplify_core/lib/src/types/storage/remove_options.dart index f898e77190..95be578c1c 100644 --- a/packages/amplify_core/lib/src/types/storage/remove_options.dart +++ b/packages/amplify_core/lib/src/types/storage/remove_options.dart @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import 'package:aws_common/aws_common.dart'; +import 'package:amplify_core/amplify_core.dart'; /// {@template amplify_core.storage.remove_options} /// Configurable options for `Amplify.Storage.remove`. @@ -14,13 +14,17 @@ class StorageRemoveOptions /// {@macro amplify_core.storage.remove_options} const StorageRemoveOptions({ this.pluginOptions, + this.bucket, }); /// {@macro amplify_core.storage.remove_plugin_options} final StorageRemovePluginOptions? pluginOptions; + /// Optionally specify which bucket to target + final StorageBucket? bucket; + @override - List get props => [pluginOptions]; + List get props => [pluginOptions, bucket]; @override String get runtimeTypeName => 'StorageRemoveOptions'; @@ -28,6 +32,7 @@ class StorageRemoveOptions @override Map toJson() => { 'pluginOptions': pluginOptions?.toJson(), + 'bucket': bucket, }; } diff --git a/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart b/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart index a46d68e66e..ac02e87317 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart @@ -64,6 +64,91 @@ void main() { }); }); + group('Multi-bucket', () { + final mainBucket = StorageBucket.fromOutputs( + 'Storage Integ Test main bucket', + ); + final secondaryBucket = StorageBucket.fromOutputs( + 'Storage Integ Test secondary bucket', + ); + final path = 'public/multi-bucket-remove-${uuid()}'; + final storagePath = StoragePath.fromString(path); + setUp(() async { + // upload to main bucket + await Amplify.Storage.uploadData( + data: StorageDataPayload.bytes('data'.codeUnits), + path: storagePath, + bucket: mainBucket, + ).result; + }); + + testWidgets('removes from multiple buckets', (_) async { + // upload to secondary bucket + await Amplify.Storage.uploadData( + data: StorageDataPayload.bytes('data'.codeUnits), + path: storagePath, + bucket: secondaryBucket, + ).result; + expect( + await objectExistsInBuckets( + storagePath, + [mainBucket, secondaryBucket], + ), + true, + ); + + final mainResult = await Amplify.Storage.remove( + path: storagePath, + options: StorageRemoveOptions(bucket: mainBucket), + ).result; + expect(mainResult.removedItem.path, path); + + final secondaryResult = await Amplify.Storage.remove( + path: storagePath, + options: StorageRemoveOptions(bucket: mainBucket), + ).result; + expect(secondaryResult.removedItem.path, path); + + expect( + await objectExistsInBuckets( + storagePath, + [mainBucket, secondaryBucket], + ), + false, + ); + }); + + testWidgets('removes when present in bucket', (_) async { + expect( + await objectExistsInBuckets( + storagePath, + [mainBucket], + ), + true, + ); + final mainResult = await Amplify.Storage.remove( + path: storagePath, + options: StorageRemoveOptions(bucket: mainBucket), + ).result; + expect(mainResult.removedItem.path, path); + expect( + await objectExistsInBuckets( + storagePath, + [mainBucket], + ), + false, + ); + + await expectLater( + Amplify.Storage.remove( + path: storagePath, + options: StorageRemoveOptions(bucket: secondaryBucket), + ).result, + completes, + ); + }); + }); + testWidgets('unauthorized path', (_) async { await expectLater( () => Amplify.Storage.remove( diff --git a/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart b/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart index c6e533f5ab..4f79d32847 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart @@ -3,7 +3,29 @@ import 'package:amplify_core/amplify_core.dart'; /// Returns true if an object exists at the given [path]. Future objectExists(StoragePath path) async { try { - await Amplify.Storage.getProperties(path: path).result; + await Amplify.Storage.getProperties( + path: path, + ).result; + return true; + } on StorageNotFoundException { + return false; + } +} + +/// Returns true if an object exists in multiple [buckets] at [path]. +Future objectExistsInBuckets( + StoragePath path, + List buckets, +) async { + try { + await Future.wait( + buckets.map( + (bucket) => Amplify.Storage.getProperties( + path: path, + options: StorageGetPropertiesOptions(bucket: bucket), + ).result, + ), + ); return true; } on StorageNotFoundException { return false; diff --git a/packages/storage/amplify_storage_s3/example/integration_test/utils/tear_down.dart b/packages/storage/amplify_storage_s3/example/integration_test/utils/tear_down.dart index 5c9bd036f2..319cc05ee9 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/utils/tear_down.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/utils/tear_down.dart @@ -36,6 +36,27 @@ void addTearDownPaths(List paths) { ); } +/// Adds a tear down to remove the same object in multiple [buckets]. +void addTearDownMultiBucket(StoragePath path, List buckets) { + addTearDown( + () { + try { + return Future.wait( + buckets.map( + (bucket) => Amplify.Storage.remove( + path: path, + options: StorageRemoveOptions(bucket: bucket), + ).result, + ), + ); + } on Exception catch (e) { + _logger.warn('Failed to remove files after test', e); + rethrow; + } + }, + ); +} + /// Adds a tear down to delete the current user. void addTearDownCurrentUser() { addTearDown(() { diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart index a6feeb334d..ed0ecaafc8 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart @@ -456,11 +456,12 @@ class StorageS3Service { required StoragePath path, required StorageRemoveOptions options, }) async { + final s3ClientInfo = getS3ClientInfo(storageBucket: options.bucket); final resolvedPath = await _pathResolver.resolvePath(path: path); await _deleteObject( - s3client: _defaultS3Client, - bucket: _storageOutputs.bucketName, + s3client: s3ClientInfo.client, + bucket: s3ClientInfo.bucketName, key: resolvedPath, ); From c1b94269c20fc4c5ff41912c6310b39aa1e461e1 Mon Sep 17 00:00:00 2001 From: Elijah Quartey Date: Fri, 25 Oct 2024 12:54:23 -0500 Subject: [PATCH 2/2] added missing params, fixed tests --- .../example/integration_test/remove_test.dart | 45 ++++++++++++++----- .../integration_test/utils/object_exists.dart | 23 +--------- .../lib/src/amplify_storage_s3_dart_impl.dart | 1 + .../service/storage_s3_service_impl.dart | 2 +- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart b/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart index ac02e87317..6a8d441be7 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/remove_test.dart @@ -83,16 +83,25 @@ void main() { }); testWidgets('removes from multiple buckets', (_) async { + expect( + await objectExists( + storagePath, + bucket: mainBucket, + ), + true, + ); + // upload to secondary bucket await Amplify.Storage.uploadData( data: StorageDataPayload.bytes('data'.codeUnits), path: storagePath, bucket: secondaryBucket, ).result; + expect( - await objectExistsInBuckets( + await objectExists( storagePath, - [mainBucket, secondaryBucket], + bucket: secondaryBucket, ), true, ); @@ -103,16 +112,31 @@ void main() { ).result; expect(mainResult.removedItem.path, path); + // Assert path was only removed from the main bucket + expect( + await objectExists( + storagePath, + bucket: mainBucket, + ), + false, + ); + expect( + await objectExists( + storagePath, + bucket: secondaryBucket, + ), + true, + ); + final secondaryResult = await Amplify.Storage.remove( path: storagePath, - options: StorageRemoveOptions(bucket: mainBucket), + options: StorageRemoveOptions(bucket: secondaryBucket), ).result; expect(secondaryResult.removedItem.path, path); - expect( - await objectExistsInBuckets( + await objectExists( storagePath, - [mainBucket, secondaryBucket], + bucket: secondaryBucket, ), false, ); @@ -120,9 +144,9 @@ void main() { testWidgets('removes when present in bucket', (_) async { expect( - await objectExistsInBuckets( + await objectExists( storagePath, - [mainBucket], + bucket: mainBucket, ), true, ); @@ -132,9 +156,9 @@ void main() { ).result; expect(mainResult.removedItem.path, path); expect( - await objectExistsInBuckets( + await objectExists( storagePath, - [mainBucket], + bucket: mainBucket, ), false, ); @@ -145,6 +169,7 @@ void main() { options: StorageRemoveOptions(bucket: secondaryBucket), ).result, completes, + reason: 'non existent path does not throw', ); }); }); diff --git a/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart b/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart index 4f79d32847..aa3e98f97e 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/utils/object_exists.dart @@ -1,33 +1,14 @@ import 'package:amplify_core/amplify_core.dart'; /// Returns true if an object exists at the given [path]. -Future objectExists(StoragePath path) async { +Future objectExists(StoragePath path, {StorageBucket? bucket}) async { try { await Amplify.Storage.getProperties( path: path, + options: StorageGetPropertiesOptions(bucket: bucket), ).result; return true; } on StorageNotFoundException { return false; } } - -/// Returns true if an object exists in multiple [buckets] at [path]. -Future objectExistsInBuckets( - StoragePath path, - List buckets, -) async { - try { - await Future.wait( - buckets.map( - (bucket) => Amplify.Storage.getProperties( - path: path, - options: StorageGetPropertiesOptions(bucket: bucket), - ).result, - ), - ); - return true; - } on StorageNotFoundException { - return false; - } -} diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart b/packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart index 5134b7a704..cfca3bc0e4 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart @@ -388,6 +388,7 @@ class AmplifyStorageS3Dart extends StoragePluginInterface final s3Options = StorageRemoveOptions( pluginOptions: s3PluginOptions, + bucket: options?.bucket, ); return S3RemoveOperation( diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart index ed0ecaafc8..5848fa3ecd 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/storage_s3_service_impl.dart @@ -201,7 +201,7 @@ class StorageS3Service { return S3GetPropertiesResult( storageItem: S3Item.fromHeadObjectOutput( await headObject( - s3client: _defaultS3Client, + s3client: s3ClientInfo.client, bucket: s3ClientInfo.bucketName, key: resolvedPath, ),