Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage)!: move delimiter to S3ListPluginOptions #4773

Merged
merged 2 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ void main() {
'$uniquePrefix/file1.txt',
'$uniquePrefix/file2.txt',
'$uniquePrefix/subdir/file3.txt',
'$uniquePrefix/subdir2#file4.txt',
];
group('standard config', () {
setUpAll(() async {
Expand Down Expand Up @@ -70,20 +71,46 @@ void main() {
});

group('list() with options', () {
testWidgets(
'should list all files with unique prefix excluding subPaths',
(_) async {
final listResult = await Amplify.Storage.list(
path: StoragePath.fromString('$uniquePrefix/'),
options: const StorageListOptions(
pluginOptions: S3ListPluginOptions(
excludeSubPaths: true,
group('excluding sub paths', () {
testWidgets('default delimiter', (_) async {
final listResult = await Amplify.Storage.list(
path: StoragePath.fromString('$uniquePrefix/'),
options: const StorageListOptions(
pluginOptions: S3ListPluginOptions(
excludeSubPaths: true,
),
),
),
).result;
).result as S3ListResult;

expect(listResult.items.length, 3);
expect(listResult.items.first.path, contains('file1.txt'));

expect(listResult.metadata.subPaths.length, 1);
expect(listResult.metadata.subPaths.first, '$uniquePrefix/subdir/');
expect(listResult.metadata.delimiter, '/');
});

testWidgets('custom delimiter', (_) async {
final listResult = await Amplify.Storage.list(
path: StoragePath.fromString('$uniquePrefix/'),
options: const StorageListOptions(
pluginOptions: S3ListPluginOptions(
excludeSubPaths: true,
delimiter: '#',
),
),
).result as S3ListResult;

expect(listResult.items.length, 2);
expect(listResult.items.first.path, contains('file1.txt'));
expect(listResult.items.length, 3);
expect(listResult.items.first.path, contains('file1.txt'));

expect(listResult.metadata.subPaths.length, 1);
expect(
listResult.metadata.subPaths.first,
'$uniquePrefix/subdir2#',
);
expect(listResult.metadata.delimiter, '#');
});
});

testWidgets('should respect pageSize limitation', (_) async {
Expand Down Expand Up @@ -130,7 +157,7 @@ void main() {
),
).result;

expect(listResult.items.length, 3);
expect(listResult.items.length, uploadedPaths.length);
expect(listResult.nextToken, isNull);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import 'package:amplify_storage_s3_dart/amplify_storage_s3_dart.dart';
/// {@endtemplate}
class AmplifyStorageS3 extends AmplifyStorageS3Dart {
/// {@macro amplify_storage_s3_.amplify_storage_s3_plugin}
AmplifyStorageS3({
super.delimiter,
});
AmplifyStorageS3();

/// A plugin key which can be used with `Amplify.Storage.getPlugin` to retrieve
/// a S3-specific Storage category interface.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ class AmplifyStorageS3Dart extends StoragePluginInterface
with AWSDebuggable, AWSLoggerMixin {
/// {@macro amplify_storage_s3_dart.amplify_storage_s3_plugin_dart}
AmplifyStorageS3Dart({
String? delimiter,
@visibleForTesting DependencyManager? dependencyManagerOverride,
}) : _delimiter = delimiter,
_dependencyManagerOverride = dependencyManagerOverride;
}) : _dependencyManagerOverride = dependencyManagerOverride;

/// {@template amplify_storage_s3_dart.plugin_key}
/// A plugin key which can be used with `Amplify.Storage.getPlugin` to retrieve
Expand All @@ -42,8 +40,6 @@ class AmplifyStorageS3Dart extends StoragePluginInterface
static const StoragePluginKey<AmplifyStorageS3Dart> pluginKey =
_AmplifyStorageS3DartPluginKey();

final String? _delimiter;

final DependencyManager? _dependencyManagerOverride;

@override
Expand Down Expand Up @@ -112,7 +108,6 @@ class AmplifyStorageS3Dart extends StoragePluginInterface
StorageS3Service(
credentialsProvider: credentialsProvider,
s3PluginConfig: s3PluginConfig,
delimiter: _delimiter,
pathResolver: _pathResolver,
logger: logger,
dependencyManager: dependencies,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ class S3ListPluginOptions extends StorageListPluginOptions {
/// {@macro storage.amplify_storage_s3.list_plugin_options}
const S3ListPluginOptions({
bool excludeSubPaths = false,
String delimiter = '/',
}) : this._(
excludeSubPaths: excludeSubPaths,
delimiter: delimiter,
);

const S3ListPluginOptions._({
this.excludeSubPaths = false,
this.delimiter = '/',
this.listAll = false,
});

Expand All @@ -40,6 +43,10 @@ class S3ListPluginOptions extends StorageListPluginOptions {
/// default value is `false`.
final bool excludeSubPaths;

/// The delimiter to use when evaluating sub paths. If [excludeSubPaths] is
/// false, this value has no impact on behavior.
final String delimiter;

/// Whether to list all objects under a given path without pagination. The
/// default value is `false`.
///
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class StorageS3Service {
required AWSIamAmplifyAuthProvider credentialsProvider,
required AWSLogger logger,
required DependencyManager dependencyManager,
String? delimiter,
}) {
final usePathStyle = s3PluginConfig.bucket.contains('.');

Expand All @@ -61,7 +60,6 @@ class StorageS3Service {
credentialsProvider: credentialsProvider,
logger: logger,
dependencyManager: dependencyManager,
delimiter: delimiter,
);
}

Expand All @@ -72,9 +70,7 @@ class StorageS3Service {
required AWSIamAmplifyAuthProvider credentialsProvider,
required AWSLogger logger,
required DependencyManager dependencyManager,
String? delimiter,
}) : _s3PluginConfig = s3PluginConfig,
_delimiter = delimiter ?? '/',
_defaultS3ClientConfig = s3ClientConfig,
// dependencyManager.get() => S3Client is used for unit tests
_defaultS3Client = dependencyManager.get() ??
Expand All @@ -99,7 +95,6 @@ class StorageS3Service {
sigv4.S3ServiceConfiguration(signPayload: false);

final S3PluginConfig _s3PluginConfig;
final String _delimiter;
final smithy_aws.S3ClientConfig _defaultS3ClientConfig;
final s3.S3Client _defaultS3Client;
final S3PathResolver _pathResolver;
Expand Down Expand Up @@ -141,7 +136,9 @@ class StorageS3Service {
..prefix = resolvedPath
..maxKeys = options.pageSize
..continuationToken = options.nextToken
..delimiter = s3PluginOptions.excludeSubPaths ? _delimiter : null;
..delimiter = s3PluginOptions.excludeSubPaths
? s3PluginOptions.delimiter
: null;
});

try {
Expand All @@ -164,7 +161,9 @@ class StorageS3Service {
builder
..bucket = _s3PluginConfig.bucket
..prefix = resolvedPath
..delimiter = s3PluginOptions.excludeSubPaths ? _delimiter : null;
..delimiter = s3PluginOptions.excludeSubPaths
? s3PluginOptions.delimiter
: null;
});

listResult = await _defaultS3Client.listObjectsV2(request).result;
Expand Down
Loading