Skip to content

Commit

Permalink
chore: update path validation logic
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan-Nelson committed Mar 22, 2024
1 parent 2cb93bf commit d2b59a0
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 61 deletions.
4 changes: 2 additions & 2 deletions packages/amplify_core/lib/src/types/storage/storage_path.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class StoragePath {
/// ### Example
/// {@template amplify_core.storage.storage_path.from_string.example}
/// ```
/// const p = StoragePath.fromString('/path/to/object.png');
/// const p = StoragePath.fromString('path/to/object.png');
/// ```
/// {@endtemplate}
const StoragePath.fromString(String path) : _path = path;
Expand All @@ -32,7 +32,7 @@ class StoragePath {
/// ### Example
/// {@template amplify_core.storage.storage_path.with_identity_id.example}
/// ```
/// const p = StoragePath.withIdentityId((String identityId) => '/users/$identityId/object.png');
/// const p = StoragePath.withIdentityId((String identityId) => 'users/$identityId/object.png');
/// ```
/// {@endtemplate}
factory StoragePath.withIdentityId(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void testContentTypeInferTest({
tearDownAll(() async {
await Amplify.Storage.removeMany(
paths: testUploadKeys
.map((key) => StoragePath.fromString('/private/$key'))
.map((key) => StoragePath.fromString('private/$key'))
.toList(),
).result;
});
Expand All @@ -53,7 +53,7 @@ void testContentTypeInferTest({
final result = await s3Plugin
.uploadFile(
localFile: file,
path: StoragePath.fromString('/private/${testUploadKeys[index]}'),
path: StoragePath.fromString('private/${testUploadKeys[index]}'),
options: const StorageUploadFileOptions(
pluginOptions: S3UploadFilePluginOptions(
getProperties: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void testContentTypeInferTest({
tearDownAll(() async {
await Amplify.Storage.removeMany(
paths: testUploadKeys
.map((key) => StoragePath.fromString('/private/$key'))
.map((key) => StoragePath.fromString('private/$key'))
.toList(),
).result;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ void main() {
final key = 'Test - ${DateTime.now()}';
await Amplify.Storage.uploadData(
data: S3DataPayload.bytes('hello'.codeUnits),
path: StoragePath.fromString('/public/$key'),
path: StoragePath.fromString('public/$key'),
).result;

final getUrlResult = await Amplify.Storage.getUrl(
path: StoragePath.fromString('/public/$key'),
path: StoragePath.fromString('public/$key'),
).result;
final uri = getUrlResult.url;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ void main() {
testBytes,
contentType: testContentType,
),
path: StoragePath.fromString('/public/$testObjectKey1'),
path: StoragePath.fromString('public/$testObjectKey1'),
options: const StorageUploadDataOptions(
metadata: {
'filename': testObjectFileName1,
Expand Down Expand Up @@ -213,7 +213,7 @@ void main() {
.uploadData(
data: S3DataPayload.dataUrl(testDataUrl),
path: StoragePath.withIdentityId(
(identityId) => '/protected/$identityId/$testObjectKey2',
(identityId) => 'protected/$identityId/$testObjectKey2',
),
options: const StorageUploadDataOptions(
metadata: {
Expand Down Expand Up @@ -252,7 +252,7 @@ void main() {
.uploadFile(
localFile: AWSFile.fromData(testLargeFileBytes),
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey3',
(identityId) => 'private/$identityId/$testObjectKey3',
),
options: const StorageUploadFileOptions(
metadata: {
Expand Down Expand Up @@ -288,7 +288,7 @@ void main() {
' currently signed in user', (WidgetTester tester) async {
final result = await Amplify.Storage.getUrl(
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey3',
(identityId) => 'private/$identityId/$testObjectKey3',
),
options: const StorageGetUrlOptions(
pluginOptions: S3GetUrlPluginOptions(
Expand All @@ -306,7 +306,7 @@ void main() {
(WidgetTester tester) async {
final result = Amplify.Storage.getUrl(
path: const StoragePath.fromString(
'/public/random/non-existent/object.png',
'public/random/non-existent/object.png',
),
options: const StorageGetUrlOptions(
pluginOptions: S3GetUrlPluginOptions(
Expand Down Expand Up @@ -406,10 +406,10 @@ void main() {
(WidgetTester tester) async {
final result = await Amplify.Storage.copy(
source: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey3',
(identityId) => 'private/$identityId/$testObjectKey3',
),
destination: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObject3CopyKey',
(identityId) => 'private/$identityId/$testObject3CopyKey',
),
options: const StorageCopyOptions(
pluginOptions: S3CopyPluginOptions(
Expand All @@ -427,7 +427,7 @@ void main() {
(WidgetTester tester) async {
final result = await Amplify.Storage.remove(
path: StoragePath.withIdentityId(
(id) => '/private/$id/$testObject3CopyMoveKey',
(id) => 'private/$id/$testObject3CopyMoveKey',
),
).result;

Expand Down Expand Up @@ -487,7 +487,7 @@ void main() {
testBytes,
contentType: testContentType,
),
path: StoragePath.fromString('/public/$objectKey'),
path: StoragePath.fromString('public/$objectKey'),
options: StorageUploadDataOptions(
metadata: testMetadata,
pluginOptions: const S3UploadDataPluginOptions(
Expand Down Expand Up @@ -515,7 +515,7 @@ void main() {
expect(metadata, equals(testMetadata));

await s3Plugin
.remove(path: StoragePath.fromString('/public/$objectKey'))
.remove(path: StoragePath.fromString('public/$objectKey'))
.result;
});
});
Expand All @@ -537,7 +537,7 @@ void main() {
testWidgets('get properties of object with access level guest',
(WidgetTester tester) async {
final result = await Amplify.Storage.getProperties(
path: StoragePath.fromString('/public/$testObjectKey1'),
path: StoragePath.fromString('public/$testObjectKey1'),
).result;

expect(result.storageItem.eTag, object1Etag);
Expand All @@ -548,7 +548,7 @@ void main() {
(WidgetTester tester) async {
final result = await Amplify.Storage.getProperties(
path: StoragePath.fromString(
'/protected/$user1IdentityId/$testObjectKey2',
'protected/$user1IdentityId/$testObjectKey2',
),
).result;

Expand All @@ -561,7 +561,7 @@ void main() {
(WidgetTester tester) async {
final operation = Amplify.Storage.getProperties(
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey3',
(identityId) => 'private/$identityId/$testObjectKey3',
),
);

Expand All @@ -574,7 +574,7 @@ void main() {
testWidgets('get url of object with access level guest',
(WidgetTester tester) async {
final operation = Amplify.Storage.getUrl(
path: StoragePath.fromString('/public/$testObjectKey1'),
path: StoragePath.fromString('public/$testObjectKey1'),
options: const StorageGetUrlOptions(
pluginOptions: S3GetUrlPluginOptions(
validateObjectExistence: true,
Expand All @@ -590,7 +590,7 @@ void main() {
(WidgetTester tester) async {
final operation = Amplify.Storage.getUrl(
path: StoragePath.withIdentityId(
(identityId) => '/protected/$identityId/$testObjectKey2',
(identityId) => 'protected/$identityId/$testObjectKey2',
),
options: const StorageGetUrlOptions(
pluginOptions: S3GetUrlPluginOptions(),
Expand All @@ -606,7 +606,7 @@ void main() {
(WidgetTester tester) async {
final operation = Amplify.Storage.getUrl(
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey3',
(identityId) => 'private/$identityId/$testObjectKey3',
),
options: const StorageGetUrlOptions(
pluginOptions: S3GetUrlPluginOptions(
Expand Down Expand Up @@ -670,7 +670,7 @@ void main() {
'/protected/$user1IdentityId/$testObjectKey2',
),
destination: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/$testObjectKey2',
(identityId) => 'private/$identityId/$testObjectKey2',
),
options: const StorageCopyOptions(
pluginOptions: S3CopyPluginOptions(
Expand Down Expand Up @@ -698,7 +698,7 @@ void main() {
testBytes,
contentType: 'text/plain',
),
path: StoragePath.fromString('/private/$fileKey'),
path: StoragePath.fromString('private/$fileKey'),
options: StorageUploadDataOptions(
metadata: {
'filename': fileNameTemp,
Expand All @@ -719,7 +719,7 @@ void main() {
// Clean up files from this test.
await Amplify.Storage.removeMany(
paths: uploadedKeys
.map((key) => StoragePath.fromString('/private/$key'))
.map((key) => StoragePath.fromString('private/$key'))
.toList(),
).result;
});
Expand All @@ -741,7 +741,7 @@ void main() {
testBytes,
contentType: 'text/plain',
),
path: StoragePath.fromString('/private/$fileKey'),
path: StoragePath.fromString('private/$fileKey'),
options: StorageUploadDataOptions(
metadata: {
'filename': fileNameTemp,
Expand Down Expand Up @@ -769,7 +769,7 @@ void main() {
// Clean up files from this test.
await Amplify.Storage.removeMany(
paths: uploadedKeys
.map((key) => StoragePath.fromString('/private/$key'))
.map((key) => StoragePath.fromString('private/$key'))
.toList(),
).result;
});
Expand All @@ -780,14 +780,14 @@ void main() {
final result1 = await Amplify.Storage.uploadData(
data: HttpPayload.string('obj1'),
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/remove-test/obj1',
(identityId) => 'private/$identityId/remove-test/obj1',
),
).result;

final result2 = await Amplify.Storage.uploadData(
data: HttpPayload.string('obj2'),
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/remove-test/obj2',
(identityId) => 'private/$identityId/remove-test/obj2',
),
).result;

Expand Down Expand Up @@ -826,14 +826,14 @@ void main() {
final result1 = await Amplify.Storage.uploadData(
data: HttpPayload.string('obj1'),
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/remove-test-user-a/obj1',
(identityId) => 'private/$identityId/remove-test-user-a/obj1',
),
).result;

final result2 = await Amplify.Storage.uploadData(
data: HttpPayload.string('obj2'),
path: StoragePath.withIdentityId(
(identityId) => '/private/$identityId/remove-test-user-a/obj2',
(identityId) => 'private/$identityId/remove-test-user-a/obj2',
),
).result;

Expand Down
4 changes: 2 additions & 2 deletions packages/storage/amplify_storage_s3/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class _HomeScreenState extends State<HomeScreen> {
platformFile.readStream!,
size: platformFile.size,
),
path: StoragePath.fromString('/public/${platformFile.name}'),
path: StoragePath.fromString('public/${platformFile.name}'),
onProgress: (p) =>
_logger.debug('Uploading: ${p.transferredBytes}/${p.totalBytes}'),
).result;
Expand Down Expand Up @@ -212,7 +212,7 @@ class _HomeScreenState extends State<HomeScreen> {
}) async {
try {
await Amplify.Storage.remove(
path: StoragePath.fromString('/public/$key'),
path: StoragePath.fromString('public/$key'),
).result;
setState(() {
// set the imageUrl to empty if the deleted file is the one being displayed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ class S3Item extends StorageItem

return S3Item(
key: keyDroppedPrefix,
// TODO(Jordan-Nelson): remove when path validation logic is updated
path: '/$key',
path: key,
size: object.size?.toInt(),
lastModified: object.lastModified,
eTag: object.eTag,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ class S3PathResolver {
// ignore: invalid_use_of_internal_member
_ => path.resolvePath()
};
if (!resolvedPath.startsWith('/')) {
if (resolvedPath.startsWith('/')) {
throw const StoragePathValidationException(
'StoragePath must start with a leading "/"',
recoverySuggestion: 'Update the provided path to include a leading "/"',
'StoragePath cannot start with a leading "/"',
recoverySuggestion: 'Remove the leading "/" from the StoragePath',
);
}
return resolvedPath;
Expand All @@ -47,16 +47,13 @@ class S3PathResolver {
paths.whereType<StoragePathWithIdentityId>().isNotEmpty;
final identityId =
requiredIdentityId ? await _identityProvider.getIdentityId() : null;
return (await Future.wait(
return Future.wait(
paths.map(
(path) => resolvePath(
path: path,
identityId: identityId,
),
),
))
// TODO(Jordan-Nelson): remove once path validation logic is updated.
.map((path) => path.substring(1))
.toList();
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'test_utils/test_custom_prefix_resolver.dart';
import 'test_utils/test_path_resolver.dart';
import 'test_utils/test_token_provider.dart';

const testPath = StoragePath.fromString('/some/path.txt');
const testPath = StoragePath.fromString('some/path.txt');

void main() {
const testDefaultStorageAccessLevel = StorageAccessLevel.guest;
Expand Down Expand Up @@ -913,9 +913,9 @@ void main() {
});

group('copy() API', () {
const testSource = StoragePath.fromString('/public/source-key');
const testSource = StoragePath.fromString('public/source-key');
final testDestination = StoragePath.withIdentityId(
(identityId) => '/protected/$identityId/destination-key',
(identityId) => 'protected/$identityId/destination-key',
);

final testResult = S3CopyResult(
Expand All @@ -925,7 +925,7 @@ void main() {
setUpAll(() {
registerFallbackValue(const StorageCopyOptions());
registerFallbackValue(
const StoragePath.fromString('/public/source-key'),
const StoragePath.fromString('public/source-key'),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,23 @@ void main() {
);

test('should resolve static strings', () async {
const path = StoragePath.fromString('/foo/bar/picture.png');
const path = StoragePath.fromString('foo/bar/picture.png');
expect(
await pathResolver.resolvePath(path: path),
'/foo/bar/picture.png',
'foo/bar/picture.png',
);
});

test('should resolve path with identity Id', () async {
final path = StoragePath.withIdentityId((id) => '/foo/$id/picture.png');
final path = StoragePath.withIdentityId((id) => 'foo/$id/picture.png');
expect(
await pathResolver.resolvePath(path: path),
'/foo/mock-id/picture.png',
'foo/mock-id/picture.png',
);
});

test('should throw if the path does not start with a leading "/"',
() async {
const path = StoragePath.fromString('foo/bar/picture.png');
test('should throw if the path starts with a leading "/"', () async {
const path = StoragePath.fromString('/foo/bar/picture.png');
expect(
() => pathResolver.resolvePath(path: path),
throwsA(isA<StoragePathValidationException>()),
Expand Down
Loading

0 comments on commit d2b59a0

Please sign in to comment.