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.

Sign up for GitHub

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

chore: fix storage list with multiple buckets #2908

Merged
merged 4 commits into from
Sep 5, 2024
Merged

chore: fix storage list with multiple buckets #2908

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3891445
chore: fix storage list with multiple buckets
phantumcode Sep 4, 2024
efc8cdf
chore: refactor to not start operation if there's a storage exception
phantumcode Sep 5, 2024
f7ac1ee
chore: add unit tests
phantumcode Sep 5, 2024
768fc0c
chore: fix lint error
phantumcode Sep 5, 2024
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
168 changes: 84 additions & 84 deletions aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.amplifyframework.auth.CognitoCredentialsProvider;
import com.amplifyframework.core.Consumer;
import com.amplifyframework.core.NoOpConsumer;
import com.amplifyframework.core.async.AmplifyOperation;
import com.amplifyframework.core.configuration.AmplifyOutputsData;
import com.amplifyframework.storage.BucketInfo;
import com.amplifyframework.storage.InvalidStorageBucketException;
Expand Down Expand Up @@ -377,23 +378,19 @@ public StorageGetUrlOperation<?> getUrl(
validateObjectExistence
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageGetPresignedUrlOperation operation =
new AWSS3StorageGetPresignedUrlOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
awsS3StoragePluginConfiguration,
onSuccess,
onError);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand All @@ -419,22 +416,18 @@ public StorageGetUrlOperation<?> getUrl(
validateObjectExistence
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathGetPresignedUrlOperation operation =
new AWSS3StoragePathGetPresignedUrlOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
onSuccess,
onError);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -513,15 +506,10 @@ public StorageDownloadFileOperation<?> downloadFile(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageDownloadFileOperation operation = new AWSS3StorageDownloadFileOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
Expand All @@ -530,7 +518,8 @@ public StorageDownloadFileOperation<?> downloadFile(
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand All @@ -555,23 +544,19 @@ public StorageDownloadFileOperation<?> downloadFile(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathDownloadFileOperation operation = new AWSS3StoragePathDownloadFileOperation(
request,
storageService,
result.storageService,
executorService,
authCredentialsProvider,
onProgress,
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -654,15 +639,10 @@ public StorageUploadFileOperation<?> uploadFile(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
Expand All @@ -671,7 +651,8 @@ public StorageUploadFileOperation<?> uploadFile(
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -699,23 +680,19 @@ public StorageUploadFileOperation<?> uploadFile(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathUploadFileOperation operation = new AWSS3StoragePathUploadFileOperation(
request,
storageService,
result.storageService,
executorService,
authCredentialsProvider,
onProgress,
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -796,15 +773,10 @@ public StorageUploadInputStreamOperation<?> uploadInputStream(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
awsS3StoragePluginConfiguration,
Expand All @@ -813,7 +785,8 @@ public StorageUploadInputStreamOperation<?> uploadInputStream(
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -841,24 +814,20 @@ public StorageUploadInputStreamOperation<?> uploadInputStream(
useAccelerateEndpoint
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathUploadInputStreamOperation operation =
new AWSS3StoragePathUploadInputStreamOperation(
request,
storageService,
result.storageService,
executorService,
authCredentialsProvider,
onProgress,
onSuccess,
onError
);
operation.start();

handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -901,24 +870,19 @@ public StorageRemoveOperation<?> remove(
options.getTargetIdentityId()
);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageRemoveOperation operation =
new AWSS3StorageRemoveOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
awsS3StoragePluginConfiguration,
onSuccess,
onError);

operation.start();
handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand All @@ -933,23 +897,18 @@ public StorageRemoveOperation<?> remove(
) {
AWSS3StoragePathRemoveRequest request = new AWSS3StoragePathRemoveRequest(path);

AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(options.getBucket());
} catch (StorageException exception) {
onError.accept(exception);
}
GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathRemoveOperation operation =
new AWSS3StoragePathRemoveOperation(
storageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
onSuccess,
onError);

operation.start();
handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand Down Expand Up @@ -1096,17 +1055,19 @@ public StorageListOperation<?> list(@NonNull String path,
options.getNextToken(),
options.getSubpathStrategy());

GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StorageListOperation operation =
new AWSS3StorageListOperation(
defaultStorageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
awsS3StoragePluginConfiguration,
onSuccess,
onError);

operation.start();
handleGetStorageServiceResult(onError, result, operation);

return operation;
}
Expand All @@ -1125,20 +1086,47 @@ public StorageListOperation<?> list(
options.getNextToken(),
options.getSubpathStrategy());

GetStorageServiceResult result = getStorageServiceResult(options.getBucket());

AWSS3StoragePathListOperation operation =
new AWSS3StoragePathListOperation(
defaultStorageService,
result.storageService,
executorService,
authCredentialsProvider,
request,
onSuccess,
onError);

operation.start();
handleGetStorageServiceResult(onError, result, operation);

return operation;
}

private static void handleGetStorageServiceResult(
@NonNull Consumer<StorageException> onError,
GetStorageServiceResult result,
AmplifyOperation<?> operation
) {
if (result.storageException == null) {
operation.start();
} else {
onError.accept(result.storageException);
}
}

@VisibleForTesting
@NonNull
GetStorageServiceResult getStorageServiceResult(@Nullable StorageBucket bucket) {
StorageException storageException = null;
AWSS3StorageService storageService = defaultStorageService;
try {
storageService = getStorageService(bucket);
} catch (StorageException exception) {
storageException = exception;
}
return new GetStorageServiceResult(storageService, storageException);
}

@SuppressLint("UnsafeOptInUsageError")
@VisibleForTesting
@NonNull
Expand Down Expand Up @@ -1209,4 +1197,16 @@ public String getConfigurationKey() {
return configurationKey;
}
}

@VisibleForTesting
@SuppressWarnings("checkstyle:VisibilityModifier")
static class GetStorageServiceResult {
final AWSS3StorageService storageService;
final StorageException storageException;

GetStorageServiceResult(AWSS3StorageService storageService, StorageException exception) {
this.storageService = storageService;
this.storageException = exception;
}
}
}
Loading