From 36ba0ad206cb1440ab792e8aa34292bb350c69ce Mon Sep 17 00:00:00 2001 From: Kha Truong <64438356+khatruong2009@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:43:50 -0700 Subject: [PATCH] feat: update list API (#4600) * feat: list API init commit * chore: removed / from beginning of paths * chore: fix formatting * chore: fix formatting and tests * chore: fix formatting in storage_s3_service_test.dart * chore: remove commented code and and / before paths * Apply suggestions from code review Co-authored-by: Jordan Nelson * chore: remove accessLevel from StorageListOptions * chore: removed prefix formatter and fixed formatting * chore: removed unused field * chore: remove prefix resolver and fix classes * chore: change variable name Co-Authored-By: Jordan Nelson <20613561+Jordan-Nelson@users.noreply.github.com> * chore: fix formatting * chore: add deleted documentation back in * chore: fixed comment placement --------- Co-authored-by: Jordan Nelson Co-authored-by: Jordan Nelson <20613561+Jordan-Nelson@users.noreply.github.com> --- .../category/amplify_storage_category.dart | 2 +- .../amplify_storage_plugin_interface.dart | 2 +- .../lib/src/types/storage/list_options.dart | 4 +- .../lib/src/types/storage/list_request.dart | 4 +- .../lib/src/types/storage/storage_item.dart | 6 +- .../integration_test/use_case_test.dart | 13 +-- .../amplify_storage_s3/example/lib/main.dart | 2 +- .../example/bin/example.dart | 26 +++--- .../lib/src/amplify_storage_s3_dart_impl.dart | 4 +- .../lib/src/model/s3_item.dart | 32 +------ .../lib/src/model/s3_item.g.dart | 9 +- .../lib/src/model/s3_list_options.dart | 7 -- .../lib/src/model/s3_list_plugin_options.dart | 37 +-------- .../lib/src/model/s3_list_result.dart | 50 +++-------- .../service/storage_s3_service_impl.dart | 23 +---- .../service/task/s3_download_task.dart | 2 +- .../test/amplify_storage_s3_dart_test.dart | 21 ++--- .../test/model/storage_s3_item_test.dart | 6 +- .../download_file_html_test.dart | 2 +- .../platform_impl/download_file_io_test.dart | 2 +- .../storage_s3_service_test.dart | 83 +++++-------------- .../task/s3_download_task_test.dart | 4 +- 22 files changed, 87 insertions(+), 254 deletions(-) diff --git a/packages/amplify_core/lib/src/category/amplify_storage_category.dart b/packages/amplify_core/lib/src/category/amplify_storage_category.dart index 9f60f81a70..594189ee51 100644 --- a/packages/amplify_core/lib/src/category/amplify_storage_category.dart +++ b/packages/amplify_core/lib/src/category/amplify_storage_category.dart @@ -37,7 +37,7 @@ class StorageCategory extends AmplifyCategory { /// returns a [StorageListOperation]. /// {@endtemplate} StorageListOperation list({ - String? path, + required StoragePath path, StorageListOptions? options, }) { return identifyCall( diff --git a/packages/amplify_core/lib/src/plugin/amplify_storage_plugin_interface.dart b/packages/amplify_core/lib/src/plugin/amplify_storage_plugin_interface.dart index 9c3da9aef9..c4d874d8a0 100644 --- a/packages/amplify_core/lib/src/plugin/amplify_storage_plugin_interface.dart +++ b/packages/amplify_core/lib/src/plugin/amplify_storage_plugin_interface.dart @@ -16,7 +16,7 @@ abstract class StoragePluginInterface extends AmplifyPluginInterface { /// {@macro amplify_core.amplify_storage_category.list} StorageListOperation list({ - String? path, + required StoragePath path, StorageListOptions? options, }) { throw UnimplementedError('list() has not been implemented.'); diff --git a/packages/amplify_core/lib/src/types/storage/list_options.dart b/packages/amplify_core/lib/src/types/storage/list_options.dart index 82b29851a4..80b6127287 100644 --- a/packages/amplify_core/lib/src/types/storage/list_options.dart +++ b/packages/amplify_core/lib/src/types/storage/list_options.dart @@ -14,7 +14,6 @@ class StorageListOptions extends StorageOperationOptions AWSDebuggable { /// {@macro amplify_core.storage.list_options} const StorageListOptions({ - super.accessLevel, this.pageSize = 1000, this.nextToken, this.pluginOptions, @@ -30,14 +29,13 @@ class StorageListOptions extends StorageOperationOptions final StorageListPluginOptions? pluginOptions; @override - List get props => [accessLevel, pageSize, nextToken, pluginOptions]; + List get props => [pageSize, nextToken, pluginOptions]; @override String get runtimeTypeName => 'StorageListOptions'; @override Map toJson() => { - 'accessLevel': accessLevel?.name, 'pageSize': pageSize, 'nextToken': nextToken, 'pluginOptions': pluginOptions?.toJson(), diff --git a/packages/amplify_core/lib/src/types/storage/list_request.dart b/packages/amplify_core/lib/src/types/storage/list_request.dart index 9513b69dba..c3965152cc 100644 --- a/packages/amplify_core/lib/src/types/storage/list_request.dart +++ b/packages/amplify_core/lib/src/types/storage/list_request.dart @@ -9,12 +9,12 @@ import 'package:amplify_core/amplify_core.dart'; class StorageListRequest { /// {@macro amplify_core.storage.list_request} const StorageListRequest({ - this.path, + required this.path, this.options, }); /// Path to list objects under. - final String? path; + final StoragePath path; /// Configurable options of the [StorageListRequest]. final StorageListOptions? options; diff --git a/packages/amplify_core/lib/src/types/storage/storage_item.dart b/packages/amplify_core/lib/src/types/storage/storage_item.dart index 5a83d382f2..6675967eeb 100644 --- a/packages/amplify_core/lib/src/types/storage/storage_item.dart +++ b/packages/amplify_core/lib/src/types/storage/storage_item.dart @@ -7,17 +7,13 @@ class StorageItem { /// {@macro amplify_core.storage.storage_item} const StorageItem({ - this.key, - // TODO(Jordan-Nelson): make required - this.path = '', + required this.path, this.size, this.lastModified, this.eTag, this.metadata = const {}, }); - // TODO(Jordan-Nelson): Remove key - final String? key; final String path; final int? size; final DateTime? lastModified; diff --git a/packages/storage/amplify_storage_s3/example/integration_test/use_case_test.dart b/packages/storage/amplify_storage_s3/example/integration_test/use_case_test.dart index 39825b696a..89aeae684c 100644 --- a/packages/storage/amplify_storage_s3/example/integration_test/use_case_test.dart +++ b/packages/storage/amplify_storage_s3/example/integration_test/use_case_test.dart @@ -416,7 +416,7 @@ void main() { ), ).result; - expect(result.copiedItem.key, testObject3CopyKey); + expect(result.copiedItem.path, testObject3CopyKey); expect(result.copiedItem.eTag, isNotEmpty); }); @@ -683,7 +683,6 @@ void main() { (WidgetTester tester) async { const filesToUpload = 2; const filesToList = 1; - const accessLevel = StorageAccessLevel.private; final uploadedKeys = []; // Upload some files to test. for (var i = filesToUpload; i > 0; i--) { @@ -706,8 +705,10 @@ void main() { // Call list() and ensure length of result matches pageSize. final listResult = await Amplify.Storage.list( + path: StoragePath.withIdentityId( + (identityId) => 'private/$identityId/', + ), options: const StorageListOptions( - accessLevel: accessLevel, pageSize: filesToList, ), ).result; @@ -729,7 +730,6 @@ void main() { (WidgetTester tester) async { const filesToUpload = 2; const filesToList = 1; - const accessLevel = StorageAccessLevel.private; final keyPrefix = 'testObjectList${uuid()}'; final uploadedKeys = []; // Upload some files to test. @@ -758,12 +758,13 @@ void main() { do { // Call list() until nextToken is null and ensure we paginated expected times. final listResult = await Amplify.Storage.list( + path: StoragePath.withIdentityId( + (identityId) => 'private/$identityId/', + ), options: StorageListOptions( - accessLevel: accessLevel, pageSize: filesToList, nextToken: lastNextToken, ), - path: keyPrefix, ).result; lastNextToken = listResult.nextToken; timesCalled++; diff --git a/packages/storage/amplify_storage_s3/example/lib/main.dart b/packages/storage/amplify_storage_s3/example/lib/main.dart index 0c7d26dac7..ef0a408a19 100644 --- a/packages/storage/amplify_storage_s3/example/lib/main.dart +++ b/packages/storage/amplify_storage_s3/example/lib/main.dart @@ -161,8 +161,8 @@ class _HomeScreenState extends State { Future _listAllPublicFiles() async { try { final result = await Amplify.Storage.list( + path: const StoragePath.fromString('public'), options: const StorageListOptions( - accessLevel: StorageAccessLevel.guest, pluginOptions: S3ListPluginOptions.listAll(), ), ).result; diff --git a/packages/storage/amplify_storage_s3_dart/example/bin/example.dart b/packages/storage/amplify_storage_s3_dart/example/bin/example.dart index 2750322cc5..8ddfaeca0d 100644 --- a/packages/storage/amplify_storage_s3_dart/example/bin/example.dart +++ b/packages/storage/amplify_storage_s3_dart/example/bin/example.dart @@ -76,9 +76,6 @@ Future main() async { Future listOperation() async { final path = prompt('Enter a path to list objects for: '); - final accessLevel = promptStorageAccessLevel( - 'Choose the storage access level associated with the path: ', - ); final listAll = prompt('List with pagination? (Y/n): ').toLowerCase() == 'n'; const pageSize = 5; @@ -86,16 +83,14 @@ Future listOperation() async { // get plugin with plugin key to gain S3 specific interface final s3Plugin = Amplify.Storage.getPlugin(AmplifyStorageS3Dart.pluginKey); final options = listAll - ? StorageListOptions( - accessLevel: accessLevel, - pluginOptions: const S3ListPluginOptions.listAll(), + ? const StorageListOptions( + pluginOptions: S3ListPluginOptions.listAll(), ) - : StorageListOptions( - accessLevel: accessLevel, + : const StorageListOptions( pageSize: pageSize, ); final operation = s3Plugin.list( - path: path, + path: StoragePath.fromString(path), options: options, ); @@ -116,7 +111,7 @@ Future listOperation() async { stdout.writeln('Listed ${result.items.length} objects.'); stdout.writeln('Sub directories: ${result.metadata.subPaths}'); result.items.asMap().forEach((index, item) { - stdout.writeln('$index. key: ${item.key} | size: ${item.size}'); + stdout.writeln('$index. path: ${item.path} | size: ${item.size}'); }); if (!result.hasNextPage) { @@ -133,9 +128,8 @@ Future listOperation() async { result = await s3Plugin .list( - path: path, + path: StoragePath.fromString(path), options: StorageListOptions( - accessLevel: accessLevel, pageSize: pageSize, nextToken: result.nextToken, ), @@ -155,7 +149,7 @@ Future getPropertiesOperation() async { stdout ..writeln('Got properties: ') - ..writeln('key: ${result.storageItem.key}') + ..writeln('path: ${result.storageItem.path}') ..writeln('size: ${result.storageItem.size}') ..writeln('lastModified: ${result.storageItem.lastModified}') ..writeln('eTag: ${result.storageItem.eTag}') @@ -289,7 +283,7 @@ Future uploadDataUrlOperation() async { final result = await uploadDataOperation.result; stdout ..writeln('Uploaded data url: ') - ..writeln('key: ${result.uploadedItem.key}') + ..writeln('path: ${result.uploadedItem.path}') ..writeln('size: ${result.uploadedItem.size}') ..writeln('lastModified: ${result.uploadedItem.lastModified}') ..writeln('eTag: ${result.uploadedItem.eTag}'); @@ -367,7 +361,7 @@ Future copyOperation() async { final result = await copyOperation.result; stdout ..writeln('Copied object: ') - ..writeln('key: ${result.copiedItem.key}') + ..writeln('path: ${result.copiedItem.path}') ..writeln('size: ${result.copiedItem.size}') ..writeln('lastModified: ${result.copiedItem.lastModified}') ..writeln('eTag: ${result.copiedItem.eTag}') @@ -391,7 +385,7 @@ Future removeOperation() async { final result = await removeOperation.result; stdout ..writeln('Remove completed.') - ..writeln('Removed object: ${result.removedItem.key}'); + ..writeln('Removed object: ${result.removedItem.path}'); } on Exception catch (error) { stderr ..writeln('Something went wrong...') 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 e17e302e77..b8547c8b94 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 @@ -123,7 +123,6 @@ class AmplifyStorageS3Dart extends StoragePluginInterface credentialsProvider: credentialsProvider, s3PluginConfig: s3PluginConfig, delimiter: _delimiter, - prefixResolver: _prefixResolver!, pathResolver: _pathResolver, logger: logger, dependencyManager: dependencies, @@ -141,7 +140,7 @@ class AmplifyStorageS3Dart extends StoragePluginInterface @override S3ListOperation list({ - String? path, + required StoragePath path, StorageListOptions? options, }) { final s3PluginOptions = reifyPluginOptions( @@ -149,7 +148,6 @@ class AmplifyStorageS3Dart extends StoragePluginInterface defaultPluginOptions: const S3ListPluginOptions(), ); final s3Options = StorageListOptions( - accessLevel: options?.accessLevel, pluginOptions: s3PluginOptions, nextToken: options?.nextToken, pageSize: options?.pageSize ?? 1000, diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.dart b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.dart index e9977cdcce..eb816e0ae3 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.dart @@ -19,9 +19,7 @@ class S3Item extends StorageItem AWSDebuggable { /// {@macro storage.amplify_storage_s3.storage_s3_item} S3Item({ - super.key, - // TODO(Jordan-Nelson): make required - super.path, + required super.path, super.size, super.lastModified, super.eTag, @@ -35,7 +33,6 @@ class S3Item extends StorageItem return storageItem is S3Item ? storageItem : S3Item( - key: storageItem.key, path: storageItem.path, size: storageItem.size, lastModified: storageItem.lastModified, @@ -50,9 +47,8 @@ class S3Item extends StorageItem /// Creates a [S3Item] from [s3.S3Object] provided by S3 Client. @internal factory S3Item.fromS3Object( - s3.S3Object object, { - required String prefixToDrop, - }) { + s3.S3Object object, + ) { final key = object.key; // Sanity check, key property should never be null in a S3Object returned @@ -64,13 +60,7 @@ class S3Item extends StorageItem ); } - final keyDroppedPrefix = dropPrefixFromKey( - prefixToDrop: prefixToDrop, - key: key, - ); - return S3Item( - key: keyDroppedPrefix, path: key, size: object.size?.toInt(), lastModified: object.lastModified, @@ -83,12 +73,9 @@ class S3Item extends StorageItem @internal factory S3Item.fromHeadObjectOutput( s3.HeadObjectOutput headObjectOutput, { - // TODO(Jordan-Nelson): remove key - String? key, required String path, }) { return S3Item( - key: key, path: path, lastModified: headObjectOutput.lastModified, eTag: headObjectOutput.eTag, @@ -104,12 +91,9 @@ class S3Item extends StorageItem @internal factory S3Item.fromGetObjectOutput( s3.GetObjectOutput getObjectOutput, { - // TODO(Jordan-Nelson): remove key - String? key, required String path, }) { return S3Item( - key: key, path: path, lastModified: getObjectOutput.lastModified, eTag: getObjectOutput.eTag, @@ -120,15 +104,6 @@ class S3Item extends StorageItem ); } - /// Removes [prefixToDrop] from [key] string. - @internal - static String dropPrefixFromKey({ - required String prefixToDrop, - required String key, - }) { - return key.replaceRange(0, prefixToDrop.length, ''); - } - /// Object `versionId`, may be available when S3 bucket versioning is enabled. final String? versionId; @@ -137,7 +112,6 @@ class S3Item extends StorageItem @override List get props => [ - key, path, size, lastModified, diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.g.dart b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.g.dart index 5a4c48b825..6ba820001c 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.g.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_item.g.dart @@ -7,8 +7,7 @@ part of 's3_item.dart'; // ************************************************************************** S3Item _$S3ItemFromJson(Map json) => S3Item( - key: json['key'] as String?, - path: json['path'] as String? ?? '', + path: json['path'] as String, size: json['size'] as int?, lastModified: json['lastModified'] == null ? null @@ -23,7 +22,9 @@ S3Item _$S3ItemFromJson(Map json) => S3Item( ); Map _$S3ItemToJson(S3Item instance) { - final val = {}; + final val = { + 'path': instance.path, + }; void writeNotNull(String key, dynamic value) { if (value != null) { @@ -31,8 +32,6 @@ Map _$S3ItemToJson(S3Item instance) { } } - writeNotNull('key', instance.key); - val['path'] = instance.path; writeNotNull('size', instance.size); writeNotNull('lastModified', instance.lastModified?.toIso8601String()); writeNotNull('eTag', instance.eTag); diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_options.dart b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_options.dart index 20163109f9..e76da00643 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_options.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_options.dart @@ -15,12 +15,10 @@ class S3ListOptions extends StorageListOptions { 'use StorageListOptions(pluginOptions: S3ListPluginOptions(...)) instead.', ) const S3ListOptions({ - StorageAccessLevel accessLevel = StorageAccessLevel.guest, int pageSize = 1000, bool excludeSubPaths = false, String? nextToken, }) : this._( - accessLevel: accessLevel, pageSize: pageSize, nextToken: nextToken, excludeSubPaths: excludeSubPaths, @@ -30,7 +28,6 @@ class S3ListOptions extends StorageListOptions { 'use StorageListOptions(pluginOptions: S3ListPluginOptions(...)) instead.', ) const S3ListOptions._({ - super.accessLevel = StorageAccessLevel.guest, super.pageSize = 1000, super.nextToken, this.targetIdentityId, @@ -51,7 +48,6 @@ class S3ListOptions extends StorageListOptions { bool excludeSubPaths = false, String? nextToken, }) : this._( - accessLevel: StorageAccessLevel.protected, pageSize: pageSize, targetIdentityId: targetIdentityId, nextToken: nextToken, @@ -65,10 +61,8 @@ class S3ListOptions extends StorageListOptions { 'use StorageListOptions(pluginOptions: S3ListPluginOptions(...)) instead.', ) const S3ListOptions.listAll({ - StorageAccessLevel accessLevel = StorageAccessLevel.guest, bool excludeSubPaths = false, }) : this._( - accessLevel: accessLevel, excludeSubPaths: excludeSubPaths, listAll: true, ); @@ -85,7 +79,6 @@ class S3ListOptions extends StorageListOptions { String targetIdentityId, { bool excludeSubPaths = false, }) : this._( - accessLevel: StorageAccessLevel.protected, targetIdentityId: targetIdentityId, excludeSubPaths: excludeSubPaths, ); diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_plugin_options.dart b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_plugin_options.dart index 5b4bfca883..2f19cc2e5c 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_plugin_options.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_plugin_options.dart @@ -18,23 +18,10 @@ class S3ListPluginOptions extends StorageListPluginOptions { ); const S3ListPluginOptions._({ - this.targetIdentityId, this.excludeSubPaths = false, this.listAll = false, }); - /// {@macro storage.amplify_storage_s3.list_plugin_options} - /// - /// Use this to list objects that belongs to other user (identified by - /// [targetIdentityId]) rather than the currently signed in user. - const S3ListPluginOptions.forIdentity( - String targetIdentityId, { - bool excludeSubPaths = false, - }) : this._( - targetIdentityId: targetIdentityId, - excludeSubPaths: excludeSubPaths, - ); - /// {@macro storage.amplify_storage_s3.list_plugin_options} /// /// Use this to list all objects without pagination. @@ -45,37 +32,18 @@ class S3ListPluginOptions extends StorageListPluginOptions { listAll: true, ); - /// {@macro storage.amplify_storage_s3.list_plugin_options} - /// - /// Use this to list all objects without pagination when the objects belong - /// to other user (identified by [targetIdentityId]) rather than the currently - /// signed in user. - const S3ListPluginOptions.listAllForIdentity( - String targetIdentityId, { - bool excludeSubPaths = false, - }) : this._( - targetIdentityId: targetIdentityId, - excludeSubPaths: excludeSubPaths, - ); - /// {@macro storage.amplify_storage_s3.list_plugin_options} factory S3ListPluginOptions.fromJson(Map json) => _$S3ListPluginOptionsFromJson(json); - /// The identity ID of another user who uploaded the objects. - /// - /// This can be set by using [S3ListPluginOptions.forIdentity]. - final String? targetIdentityId; - /// Whether to exclude objects under the sub paths of the path to list. The /// default value is `false`. final bool excludeSubPaths; /// Whether to list all objects under a given path without pagination. The /// default value is `false`. - /// - /// This can be set by using [S3ListPluginOptions.listAll] or - /// [S3ListPluginOptions.listAllForIdentity]. + /// + /// This can be set by using [S3ListPluginOptions.listAll]. /// /// Use with caution if numerous objects are under the given path. final bool listAll; @@ -84,7 +52,6 @@ class S3ListPluginOptions extends StorageListPluginOptions { List get props => [ excludeSubPaths, listAll, - targetIdentityId, ]; @override diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_result.dart b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_result.dart index f8b6cef2ad..f2b30cfcc6 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_result.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/model/s3_list_result.dart @@ -23,21 +23,19 @@ class S3ListResult extends StorageListResult { /// smithy. This named constructor should be used internally only. @internal factory S3ListResult.fromPaginatedResult( - PaginatedResult paginatedResult, { - required String prefixToDrop, - }) { + PaginatedResult paginatedResult, + ) { final output = paginatedResult.items; - final metadata = S3ListMetadata.fromS3CommonPrefixes( - prefixToDrop: prefixToDrop, - commonPrefixes: output.commonPrefixes?.toList(), + final metadata = S3ListMetadata( + subPaths: output.commonPrefixes + ?.map((commonPrefix) => commonPrefix.prefix) + .whereType() + .toList(), delimiter: output.delimiter, ); final items = output.contents ?.map( - (s3.S3Object item) => S3Item.fromS3Object( - item, - prefixToDrop: prefixToDrop, - ), + S3Item.fromS3Object, ) .toList() ?? const []; @@ -70,34 +68,8 @@ class S3ListResult extends StorageListResult { class S3ListMetadata { /// Creates a S3ListMetadata from the `commonPrefix` and `delimiter` /// properties of the [s3.ListObjectsV2Output]. - factory S3ListMetadata.fromS3CommonPrefixes({ - required String prefixToDrop, - List? commonPrefixes, - String? delimiter, - }) { - final extractedSubPath = []; - - if (commonPrefixes != null) { - for (final commonPrefix in commonPrefixes) { - final prefix = commonPrefix.prefix; - if (prefix != null) { - extractedSubPath.add( - S3Item.dropPrefixFromKey( - prefixToDrop: prefixToDrop, - key: prefix, - ), - ); - } - } - } - - return S3ListMetadata._( - subPaths: extractedSubPath, - delimiter: delimiter, - ); - } - - S3ListMetadata._({ + @visibleForTesting + S3ListMetadata({ List? subPaths, this.delimiter, }) : subPaths = subPaths ?? const []; @@ -105,7 +77,7 @@ class S3ListMetadata { /// Merges two instances of [S3ListMetadata] into one. S3ListMetadata merge(S3ListMetadata other) { final subPaths = [...this.subPaths, ...other.subPaths]; - return S3ListMetadata._(subPaths: subPaths, delimiter: other.delimiter); + return S3ListMetadata(subPaths: subPaths, delimiter: other.delimiter); } /// Sub paths under the `path` parameter calling the `list` API. 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 adb00e8ff3..eface8e80c 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 @@ -32,7 +32,6 @@ class StorageS3Service { /// {@macro amplify_storage_s3.storage_s3_service} factory StorageS3Service({ required S3PluginConfig s3PluginConfig, - required S3PrefixResolver prefixResolver, required S3PathResolver pathResolver, required AWSIamAmplifyAuthProvider credentialsProvider, required AWSLogger logger, @@ -58,7 +57,6 @@ class StorageS3Service { return StorageS3Service._( s3PluginConfig: s3PluginConfig, s3ClientConfig: s3ClientConfig, - prefixResolver: prefixResolver, pathResolver: pathResolver, credentialsProvider: credentialsProvider, logger: logger, @@ -70,7 +68,6 @@ class StorageS3Service { StorageS3Service._({ required S3PluginConfig s3PluginConfig, required smithy_aws.S3ClientConfig s3ClientConfig, - required S3PrefixResolver prefixResolver, required S3PathResolver pathResolver, required AWSIamAmplifyAuthProvider credentialsProvider, required AWSLogger logger, @@ -88,7 +85,6 @@ class StorageS3Service { client: AmplifyHttpClient(dependencyManager) ..supportedProtocols = SupportedProtocols.http1, ), - _prefixResolver = prefixResolver, _pathResolver = pathResolver, _logger = logger, // dependencyManager.get() => sigv4.AWSSigV4Signer is used for unit tests @@ -106,7 +102,6 @@ class StorageS3Service { final String _delimiter; final smithy_aws.S3ClientConfig _defaultS3ClientConfig; final s3.S3Client _defaultS3Client; - final S3PrefixResolver _prefixResolver; final S3PathResolver _pathResolver; final AWSLogger _logger; final sigv4.AWSSigV4Signer _awsSigV4Signer; @@ -131,25 +126,19 @@ class StorageS3Service { /// service returned [smithy.UnknownSmithyHttpException] if any. /// {@endtemplate} Future list({ - String? path, + required StoragePath path, required StorageListOptions options, }) async { final s3PluginOptions = options.pluginOptions as S3ListPluginOptions? ?? const S3ListPluginOptions(); - final resolvedPrefix = await getResolvedPrefix( - prefixResolver: _prefixResolver, - logger: _logger, - accessLevel: options.accessLevel ?? _s3PluginConfig.defaultAccessLevel, - identityId: s3PluginOptions.targetIdentityId, - ); - final listTargetPrefix = '$resolvedPrefix${path ?? ''}'; + final resolvedPath = await _pathResolver.resolvePath(path: path); if (!s3PluginOptions.listAll) { final request = s3.ListObjectsV2Request.build((builder) { builder ..bucket = _s3PluginConfig.bucket - ..prefix = listTargetPrefix + ..prefix = resolvedPath ..maxKeys = options.pageSize ..continuationToken = options.nextToken ..delimiter = s3PluginOptions.excludeSubPaths ? _delimiter : null; @@ -158,7 +147,6 @@ class StorageS3Service { try { return S3ListResult.fromPaginatedResult( await _defaultS3Client.listObjectsV2(request).result, - prefixToDrop: resolvedPrefix, ); } on smithy.UnknownSmithyHttpException catch (error) { // S3Client.headObject may return 403 error @@ -175,14 +163,13 @@ class StorageS3Service { final request = s3.ListObjectsV2Request.build((builder) { builder ..bucket = _s3PluginConfig.bucket - ..prefix = listTargetPrefix + ..prefix = resolvedPath ..delimiter = s3PluginOptions.excludeSubPaths ? _delimiter : null; }); listResult = await _defaultS3Client.listObjectsV2(request).result; recursiveResult = S3ListResult.fromPaginatedResult( listResult, - prefixToDrop: resolvedPrefix, ); while (listResult.hasNext) { @@ -190,7 +177,6 @@ class StorageS3Service { recursiveResult = recursiveResult.merge( S3ListResult.fromPaginatedResult( listResult, - prefixToDrop: resolvedPrefix, ), ); } @@ -515,7 +501,6 @@ class StorageS3Service { output.deleted?.toList().map( (removedObject) => S3Item.fromS3Object( s3.S3Object(key: removedObject.key), - prefixToDrop: '', ), ) ?? [], diff --git a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/task/s3_download_task.dart b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/task/s3_download_task.dart index 6a2160b3ce..3415447a3a 100644 --- a/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/task/s3_download_task.dart +++ b/packages/storage/amplify_storage_s3_dart/lib/src/storage_s3_service/service/task/s3_download_task.dart @@ -296,7 +296,7 @@ class S3DownloadTask { // is set to `false`. _s3PluginOptions.getProperties ? _downloadedS3Item - : S3Item(key: _downloadedS3Item.key), + : S3Item(path: _downloadedS3Item.path), ); } on Exception catch (error, stackTrace) { await _completeDownloadWithError(error, stackTrace); diff --git a/packages/storage/amplify_storage_s3_dart/test/amplify_storage_s3_dart_test.dart b/packages/storage/amplify_storage_s3_dart/test/amplify_storage_s3_dart_test.dart index 7711466f60..5ab8b9e71c 100644 --- a/packages/storage/amplify_storage_s3_dart/test/amplify_storage_s3_dart_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/amplify_storage_s3_dart_test.dart @@ -18,7 +18,6 @@ const testPath = StoragePath.fromString('some/path.txt'); void main() { const testDefaultStorageAccessLevel = StorageAccessLevel.guest; - const testAccessLevelProtected = StorageAccessLevel.protected; const testConfig = AmplifyConfig( storage: StorageConfig( plugins: { @@ -134,13 +133,12 @@ void main() { }); group('list()', () { - const testPath = 'some/path'; + const testPath = StoragePath.fromString('some/path'); final testResult = S3ListResult( [], hasNextPage: false, - metadata: S3ListMetadata.fromS3CommonPrefixes( - commonPrefixes: [], - prefixToDrop: 'prefix', + metadata: S3ListMetadata( + subPaths: [], ), ); @@ -189,7 +187,6 @@ void main() { test('should forward options to StorageS3Service.list() API', () async { const testOptions = StorageListOptions( - accessLevel: testAccessLevelProtected, pluginOptions: S3ListPluginOptions(excludeSubPaths: true), nextToken: 'next-token-123', pageSize: 2, @@ -235,7 +232,7 @@ void main() { const testKey = 'some-object-key'; final testResult = S3GetPropertiesResult( storageItem: S3Item( - key: testKey, + path: testKey, lastModified: DateTime(2022, 9, 19), eTag: '12345', size: 1024, @@ -435,7 +432,7 @@ void main() { group('downloadData() API', () { const testKey = 'some-object-key'; final testItem = S3Item( - key: testKey, + path: testKey, lastModified: DateTime(2022, 9, 19), eTag: '12345', size: 1024, @@ -569,7 +566,7 @@ void main() { const testKey = 'object-upload-to'; final testData = S3DataPayload.string('Hello S3.'); final testItem = S3Item( - key: testKey, + path: testKey, lastModified: DateTime(2022, 10, 14), eTag: '12345', size: 1024, @@ -741,7 +738,7 @@ void main() { const testKey = 'object-upload-to'; final testLocalFile = AWSFile.fromData([101]); final testItem = S3Item( - key: testKey, + path: testKey, lastModified: DateTime(2022, 10, 14), eTag: '12345', size: 1024, @@ -994,7 +991,7 @@ void main() { const testKey = 'object-to-remove'; final testResult = S3RemoveResult( removedItem: S3Item( - key: testKey, + path: testKey, ), ); @@ -1085,7 +1082,7 @@ void main() { ); final testPaths = testKeys.map(StoragePath.fromString).toList(); final resultRemoveItems = - testKeys.map((key) => S3Item(key: key)).toList(); + testKeys.map((key) => S3Item(path: key)).toList(); final testResult = S3RemoveManyResult( removedItems: resultRemoveItems, ); diff --git a/packages/storage/amplify_storage_s3_dart/test/model/storage_s3_item_test.dart b/packages/storage/amplify_storage_s3_dart/test/model/storage_s3_item_test.dart index 93d6e77086..ad82f0dff9 100644 --- a/packages/storage/amplify_storage_s3_dart/test/model/storage_s3_item_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/model/storage_s3_item_test.dart @@ -25,13 +25,12 @@ void main() { final storageS3Item = S3Item.fromS3Object( testS3Object, - prefixToDrop: testPrefixToDrop, ); expect(storageS3Item.eTag, testETag); expect( - storageS3Item.key, - testKey.replaceRange(0, testPrefixToDrop.length, ''), + storageS3Item.path, + testKey, ); expect(storageS3Item.lastModified, testLastModified); expect(storageS3Item.size, testSize.toInt()); @@ -47,7 +46,6 @@ void main() { try { S3Item.fromS3Object( testS3Object, - prefixToDrop: testPrefixToDrop, ); fail("Expected exception wasn't thrown."); } on StorageException catch (error) { diff --git a/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_html_test.dart b/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_html_test.dart index 21badaba10..e8f743b964 100644 --- a/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_html_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_html_test.dart @@ -35,7 +35,7 @@ void main() { region: 'west-2', ); final testItem = S3Item( - key: testKey, + path: testKey, lastModified: DateTime(2022, 9, 19), eTag: '12345', size: 1024, diff --git a/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_io_test.dart b/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_io_test.dart index 68c469fdd8..cb2ccdd74f 100644 --- a/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_io_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/platform_impl/download_file_io_test.dart @@ -35,7 +35,7 @@ void main() { ); const testKey = 'upload-key.text'; const testFileContent = 'Hello world!'; - final testItem = S3Item(key: testKey); + final testItem = S3Item(path: testKey); final testFileBytes = utf8.encode(testFileContent); late S3TransferProgress expectedProgress; diff --git a/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/storage_s3_service_test.dart b/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/storage_s3_service_test.dart index 2f056c3002..cf82833c39 100644 --- a/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/storage_s3_service_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/storage_s3_service_test.dart @@ -42,7 +42,6 @@ void main() { const s3PluginConfig = S3PluginConfig(bucket: testBucket, region: testRegion); - final testPrefixResolver = TestPrefixResolver(); final pathResolver = TestPathResolver(); late DependencyManager dependencyManager; late S3Client s3Client; @@ -59,7 +58,6 @@ void main() { ..addInstance(awsSigV4Signer); storageS3Service = StorageS3Service( s3PluginConfig: s3PluginConfig, - prefixResolver: testPrefixResolver, pathResolver: pathResolver, credentialsProvider: TestIamAuthProvider(), logger: logger, @@ -73,7 +71,6 @@ void main() { bucket: 'bucket.name.has.dots.com', region: 'us-west-2', ), - prefixResolver: testPrefixResolver, pathResolver: pathResolver, credentialsProvider: TestIamAuthProvider(), logger: logger, @@ -85,35 +82,13 @@ void main() { expect(message, contains('Since your bucket name contains dots')); }); - group('_getResolvedPrefix()', () { - test( - 'should throw a StorageException if supplied prefix resolver throws an exception', - () async { - const testOptions = StorageListOptions( - pageSize: 1000, - pluginOptions: - S3ListPluginOptions.forIdentity('throw exception for me'), - ); - //StorageListOption<>(S3ListOptions.forIdentity('throw exception for me'); - - await expectLater( - storageS3Service.list(path: 'a path', options: testOptions), - throwsA(isA()), - ); - - verify(() => logger.error(any(), any(), any())).called(1); - }); - }); - group('list() API', () { late S3ListResult listResult; const testNextContinuationToken = 'get-next-page'; const testPageSize = 100; const testBucketName = 'a-bucket'; - const testStorageAccessLevel = StorageAccessLevel.protected; - final testPrefixToDrop = - '${testStorageAccessLevel.defaultPrefix}$testDelimiter'; - final testCommonPrefix = CommonPrefix(prefix: testPrefixToDrop); + const testPrefix = 'public$testDelimiter'; + final testCommonPrefix = CommonPrefix(prefix: testPrefix); setUpAll(() { registerFallbackValue(ListObjectsV2Request(bucket: 'fake bucket')); @@ -121,25 +96,20 @@ void main() { test('should invoke S3Client.listObjectsV2 with expected parameters', () async { - final testPrefixToDrop = - '${s3PluginConfig.defaultAccessLevel.defaultPrefix}$testDelimiter'; - final testCommonPrefix = CommonPrefix(prefix: testPrefixToDrop); + final testCommonPrefix = CommonPrefix(prefix: testPrefix); final testS3Objects = [1, 2, 3, 4, 5] .map( (e) => S3Object( - key: '${testPrefixToDrop}object-$e', + key: '${testPrefix}object-$e', size: Int64(100 * 4), eTag: 'object-$e-eTag', ), ) .toList(); - const testPath = 'album'; - const testTargetIdentityId = 'someone-else-id'; + const testPath = StoragePath.fromString('album'); const testOptions = StorageListOptions( pageSize: testPageSize, - pluginOptions: S3ListPluginOptions.forIdentity( - testTargetIdentityId, - ), + pluginOptions: S3ListPluginOptions(), ); final testPaginatedResult = @@ -182,18 +152,14 @@ void main() { expect(request.bucket, testBucket); expect( request.prefix, - '${await testPrefixResolver.resolvePrefix( - accessLevel: s3PluginConfig.defaultAccessLevel, - identityId: testTargetIdentityId, - )}$testPath', + TestPathResolver.path, ); expect(request.maxKeys, testPageSize); }); test('should return correct StorageS3ListResult', () async { listResult.items.asMap().forEach((index, item) { - expect(item.key, isNot(contains(testPrefixToDrop))); - expect(item.key, 'object-${index + 1}'); + expect(item.path, '${testPrefix}object-${index + 1}'); }); expect(listResult.hasNextPage, true); expect(listResult.nextToken, testNextContinuationToken); @@ -205,15 +171,14 @@ void main() { final testS3Objects = [1, 2, 3, 4, 5] .map( (e) => S3Object( - key: '${testPrefixToDrop}object-$e', + key: '${testPrefix}object-$e', size: Int64(100 * 4), eTag: 'object-$e-eTag', ), ) .toList(); - const testPath = 'album'; + const testPath = StoragePath.fromString('album'); const testOptions = StorageListOptions( - accessLevel: testStorageAccessLevel, pageSize: testPageSize, pluginOptions: S3ListPluginOptions(excludeSubPaths: true), ); @@ -227,10 +192,10 @@ void main() { ListObjectsV2Output( contents: testS3Objects, commonPrefixes: [ - CommonPrefix(prefix: '$testPrefixToDrop${testSubPaths[0]}'), - CommonPrefix(prefix: '$testPrefixToDrop${testSubPaths[1]}'), + CommonPrefix(prefix: testSubPaths[0]), + CommonPrefix(prefix: testSubPaths[1]), CommonPrefix( - prefix: '$testPrefixToDrop${testSubPaths[2]}', + prefix: testSubPaths[2], ), ], delimiter: testDelimiter, @@ -278,7 +243,7 @@ void main() { expect( storageS3Service.list( - path: 'a path', + path: const StoragePath.fromString('apath'), options: testOptions, ), throwsA(isA()), @@ -291,15 +256,14 @@ void main() { final testS3Objects = List.generate(2001, (index) => '$index') .map( (e) => S3Object( - key: '${testPrefixToDrop}object-$e', + key: '${testPrefix}object-$e', size: Int64(100 * 4), eTag: 'object-$e-eTag', ), ) .toList(); - const testPath = 'album'; + const testPath = StoragePath.fromString('album'); const testOptions = StorageListOptions( - accessLevel: StorageAccessLevel.private, pageSize: testPageSize, pluginOptions: S3ListPluginOptions.listAll(), ); @@ -378,11 +342,10 @@ void main() { expect( capturedRequest, isA().having( - (o) => o.prefix, - 'prefix', - '${await testPrefixResolver.resolvePrefix( - accessLevel: testOptions.accessLevel!, - )}$testPath'), + (o) => o.prefix, + 'prefix', + TestPathResolver.path, + ), ); expect(listResult.hasNextPage, false); @@ -395,8 +358,7 @@ void main() { }); test('should handle AWSHttpException and throw NetworkException', () { - const testOptions = - StorageListOptions(accessLevel: StorageAccessLevel.guest); + const testOptions = StorageListOptions(); final testException = AWSHttpException( AWSHttpRequest(method: AWSHttpMethod.get, uri: Uri()), ); @@ -407,7 +369,7 @@ void main() { expect( storageS3Service.list( - path: 'a path', + path: const StoragePath.fromString('apath'), options: testOptions, ), throwsA(isA()), @@ -837,7 +799,6 @@ void main() { ..addInstance(pathStyleAwsSigV4Signer); pathStyleStorageS3Service = StorageS3Service( s3PluginConfig: pathStyleS3PluginConfig, - prefixResolver: testPrefixResolver, pathResolver: pathResolver, credentialsProvider: TestIamAuthProvider(), logger: MockAWSLogger(), diff --git a/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/task/s3_download_task_test.dart b/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/task/s3_download_task_test.dart index 6bae2c4882..ae1ea95f3d 100644 --- a/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/task/s3_download_task_test.dart +++ b/packages/storage/amplify_storage_s3_dart/test/storage_s3_service/task/s3_download_task_test.dart @@ -656,13 +656,13 @@ void main() { description: 'should include metadata when getPropertiesValue is set to true', getPropertiesValue: true, - expectedS3Item: S3Item(key: testKey, metadata: testMetadata), + expectedS3Item: S3Item(path: testKey, metadata: testMetadata), ), GetPropertiesTestItem( description: 'should NOT include metadata when getPropertiesValue is set to false', getPropertiesValue: false, - expectedS3Item: S3Item(key: testKey, metadata: {}), + expectedS3Item: S3Item(path: testKey, metadata: {}), ), ];