Skip to content

Commit 320836b

Browse files
tylerjroachtylerjroach
authored
fix(storage): Prevent missed transfer status updates (#3154)
1 parent 75562b5 commit 320836b

File tree

7 files changed

+65
-33
lines changed

7 files changed

+65
-33
lines changed

‎aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
import com.amplifyframework.storage.s3.service.AWSS3StorageServiceContainer;
9595
import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider;
9696
import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider;
97+
import com.amplifyframework.storage.s3.transfer.TransferDB;
9798
import com.amplifyframework.storage.s3.transfer.TransferObserver;
9899
import com.amplifyframework.storage.s3.transfer.TransferRecord;
99100
import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater;
@@ -154,6 +155,8 @@ public final class AWSS3StoragePlugin extends StoragePlugin<S3Client> {
154155
return defaultStorageService.getClient();
155156
});
156157

158+
private TransferStatusUpdater transferStatusUpdater;
159+
157160
/**
158161
* Constructs the AWS S3 Storage Plugin initializing the executor service.
159162
*/
@@ -176,14 +179,15 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf
176179

177180
@VisibleForTesting
178181
AWSS3StoragePlugin(AuthCredentialsProvider authCredentialsProvider) {
179-
this((context, region, bucket, clientProvider) ->
182+
this((context, region, bucket, clientProvider, transferStatusUpdater) ->
180183
new AWSS3StorageService(
181184
context,
182185
region,
183186
bucket,
184187
authCredentialsProvider,
185188
AWS_S3_STORAGE_PLUGIN_KEY,
186-
clientProvider
189+
clientProvider,
190+
transferStatusUpdater
187191
),
188192
authCredentialsProvider,
189193
new AWSS3StoragePluginConfiguration.Builder().build());
@@ -193,14 +197,15 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf
193197
AWSS3StoragePlugin(AuthCredentialsProvider authCredentialsProvider,
194198
AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration) {
195199

196-
this((context, region, bucket, clientProvider) ->
200+
this((context, region, bucket, clientProvider, transferStatusUpdater) ->
197201
new AWSS3StorageService(
198202
context,
199203
region,
200204
bucket,
201205
authCredentialsProvider,
202206
AWS_S3_STORAGE_PLUGIN_KEY,
203-
clientProvider
207+
clientProvider,
208+
transferStatusUpdater
204209
),
205210
authCredentialsProvider,
206211
awss3StoragePluginConfiguration);
@@ -298,14 +303,20 @@ private void configure(
298303
@NonNull ResolvedStorageBucket bucket
299304
) throws StorageException {
300305
try {
306+
this.transferStatusUpdater = new TransferStatusUpdater(TransferDB.Companion.getInstance(context));
307+
308+
this.awss3StorageServiceContainer = new AWSS3StorageServiceContainer(
309+
context, storageServiceFactory,
310+
(S3StorageTransferClientProvider) clientProvider,
311+
transferStatusUpdater
312+
);
301313
this.defaultStorageService = storageServiceFactory.create(
302314
context,
303315
region,
304316
bucket.getBucketInfo().getBucketName(),
305-
clientProvider);
306-
this.awss3StorageServiceContainer = new AWSS3StorageServiceContainer(
307-
context, storageServiceFactory,
308-
(S3StorageTransferClientProvider) clientProvider);
317+
clientProvider,
318+
transferStatusUpdater
319+
);
309320
this.awss3StorageServiceContainer.put(bucket.getBucketInfo().getBucketName(), this.defaultStorageService);
310321
} catch (RuntimeException exception) {
311322
throw new StorageException(
@@ -927,7 +938,7 @@ public void getTransfer(
927938
TransferObserver transferObserver =
928939
new TransferObserver(
929940
transferRecord.getId(),
930-
defaultStorageService.getTransferManager().getTransferStatusUpdater(),
941+
transferStatusUpdater,
931942
transferRecord.getBucketName(),
932943
transferRecord.getRegion(),
933944
transferRecord.getKey(),

‎aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider
3737
import com.amplifyframework.storage.s3.transfer.TransferManager
3838
import com.amplifyframework.storage.s3.transfer.TransferObserver
3939
import com.amplifyframework.storage.s3.transfer.TransferRecord
40+
import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater
4041
import com.amplifyframework.storage.s3.transfer.UploadOptions
4142
import com.amplifyframework.storage.s3.utils.S3Keys
4243
import java.io.File
@@ -57,13 +58,14 @@ internal class AWSS3StorageService(
5758
private val s3BucketName: String,
5859
private val authCredentialsProvider: AuthCredentialsProvider,
5960
private val awsS3StoragePluginKey: String,
60-
private val clientProvider: StorageTransferClientProvider
61+
private val clientProvider: StorageTransferClientProvider,
62+
private val transferStatusUpdater: TransferStatusUpdater
6163
) : StorageService {
6264

6365
private var s3Client: S3Client = S3StorageTransferClientProvider.getS3Client(awsRegion, authCredentialsProvider)
6466

6567
val transferManager: TransferManager =
66-
TransferManager(context, clientProvider, awsS3StoragePluginKey)
68+
TransferManager(context, clientProvider, awsS3StoragePluginKey, transferStatusUpdater)
6769

6870
/**
6971
* Generate pre-signed URL for an object.
@@ -421,7 +423,8 @@ internal class AWSS3StorageService(
421423
context: Context,
422424
region: String,
423425
bucketName: String,
424-
clientProvider: StorageTransferClientProvider
426+
clientProvider: StorageTransferClientProvider,
427+
transferStatusUpdater: TransferStatusUpdater
425428
): AWSS3StorageService
426429
}
427430
}

‎aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageServiceContainer.kt‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import android.content.Context
1818
import com.amplifyframework.storage.ResolvedStorageBucket
1919
import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider
2020
import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider
21+
import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater
2122
import java.util.concurrent.ConcurrentHashMap
2223

2324
/**
@@ -28,13 +29,15 @@ internal class AWSS3StorageServiceContainer(
2829
private val context: Context,
2930
private val storageServiceFactory: AWSS3StorageService.Factory,
3031
private val clientProvider: StorageTransferClientProvider,
31-
private val awsS3StorageServicesByBucketName: ConcurrentHashMap<String, AWSS3StorageService>
32+
private val awsS3StorageServicesByBucketName: ConcurrentHashMap<String, AWSS3StorageService>,
33+
private val transferStatusUpdater: TransferStatusUpdater
3234
) {
3335
constructor(
3436
context: Context,
3537
storageServiceFactory: AWSS3StorageService.Factory,
36-
clientProvider: S3StorageTransferClientProvider
37-
) : this(context, storageServiceFactory, clientProvider, ConcurrentHashMap())
38+
clientProvider: S3StorageTransferClientProvider,
39+
transferStatusUpdater: TransferStatusUpdater
40+
) : this(context, storageServiceFactory, clientProvider, ConcurrentHashMap(), transferStatusUpdater)
3841

3942
private val lock = Any()
4043

@@ -61,7 +64,8 @@ internal class AWSS3StorageServiceContainer(
6164
var service = awsS3StorageServicesByBucketName.get(bucketName)
6265
if (service == null) {
6366
val region: String = resolvedStorageBucket.bucketInfo.region
64-
service = storageServiceFactory.create(context, region, bucketName, clientProvider)
67+
service =
68+
storageServiceFactory.create(context, region, bucketName, clientProvider, transferStatusUpdater)
6569
awsS3StorageServicesByBucketName[bucketName] = service
6670
}
6771
return service
@@ -78,7 +82,8 @@ internal class AWSS3StorageServiceContainer(
7882
synchronized(lock) {
7983
var service = awsS3StorageServicesByBucketName[bucketName]
8084
if (service == null) {
81-
service = storageServiceFactory.create(context, region, bucketName, clientProvider)
85+
service =
86+
storageServiceFactory.create(context, region, bucketName, clientProvider, transferStatusUpdater)
8287
awsS3StorageServicesByBucketName[bucketName] = service
8388
}
8489

‎aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ internal class TransferManager(
4848
context: Context,
4949
clientProvider: StorageTransferClientProvider,
5050
private val pluginKey: String,
51+
private val transferStatusUpdater: TransferStatusUpdater,
5152
private val workManager: WorkManager = WorkManager.getInstance(context)
5253
) {
5354

5455
private val transferDB: TransferDB = TransferDB.getInstance(context)
55-
val transferStatusUpdater: TransferStatusUpdater = TransferStatusUpdater(transferDB)
5656

5757
private val logger =
5858
Amplify.Logging.logger(

‎aws-storage-s3/src/test/java/com/amplifyframework/storage/AWSS3StorageServiceContainerTest.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import org.junit.Test
3333
class AWSS3StorageServiceContainerTest {
3434

3535
private val storageServiceFactory = mockk<AWSS3StorageService.Factory> {
36-
every { create(any(), any(), any(), any()) } returns mockk<AWSS3StorageService>()
36+
every { create(any(), any(), any(), any(), any()) } returns mockk<AWSS3StorageService>()
3737
}
3838
private val context = mockk<Context>()
3939
private val clientProvider = mockk<StorageTransferClientProvider>()
@@ -50,13 +50,14 @@ class AWSS3StorageServiceContainerTest {
5050
context,
5151
storageServiceFactory,
5252
clientProvider,
53-
serviceContainerHashMap
53+
serviceContainerHashMap,
54+
mockk()
5455
)
5556
}
5657

5758
@Test
5859
fun `put default AWSS3Service in container`() {
59-
val service = storageServiceFactory.create(context, region, bucketName, clientProvider)
60+
val service = storageServiceFactory.create(context, region, bucketName, clientProvider, mockk())
6061
serviceContainer.put(bucketName, service)
6162

6263
serviceContainerHashMap.size shouldBe 1

‎aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
package com.amplifyframework.storage.s3
1717

18+
import android.content.Context
19+
import androidx.test.core.app.ApplicationProvider
1820
import com.amplifyframework.storage.BucketInfo
1921
import com.amplifyframework.storage.InvalidStorageBucketException
2022
import com.amplifyframework.storage.StorageBucket
@@ -29,11 +31,16 @@ import io.mockk.every
2931
import io.mockk.mockk
3032
import io.mockk.verify
3133
import org.junit.Test
34+
import org.junit.runner.RunWith
35+
import org.robolectric.RobolectricTestRunner
3236

37+
@RunWith(RobolectricTestRunner::class)
3338
class AWSS3StoragePluginTest {
3439

40+
var context: Context = ApplicationProvider.getApplicationContext()
41+
3542
private val storageServiceFactory = mockk<AWSS3StorageService.Factory> {
36-
every { create(any(), any(), any(), any()) } returns mockk<AWSS3StorageService>()
43+
every { create(any(), any(), any(), any(), any()) } returns mockk<AWSS3StorageService>()
3744
}
3845

3946
private val plugin = AWSS3StoragePlugin(
@@ -51,10 +58,10 @@ class AWSS3StoragePluginTest {
5158
}
5259
}
5360

54-
plugin.configure(data, mockk())
61+
plugin.configure(data, context)
5562

5663
verify {
57-
storageServiceFactory.create(any(), "test-region", "test-bucket", any())
64+
storageServiceFactory.create(any(), "test-region", "test-bucket", any(), any())
5865
}
5966
}
6067

@@ -65,7 +72,7 @@ class AWSS3StoragePluginTest {
6572
}
6673

6774
shouldThrow<StorageException> {
68-
plugin.configure(data, mockk())
75+
plugin.configure(data, context)
6976
}
7077
}
7178

@@ -83,7 +90,7 @@ class AWSS3StoragePluginTest {
8390
}
8491
}
8592

86-
plugin.configure(data, mockk())
93+
plugin.configure(data, context)
8794
val service = plugin.getStorageService(null)
8895
service shouldNotBe null
8996
}
@@ -102,7 +109,7 @@ class AWSS3StoragePluginTest {
102109
}
103110
}
104111

105-
plugin.configure(data, mockk())
112+
plugin.configure(data, context)
106113
val bucketInfo = BucketInfo("test-bucket", "test-region")
107114
val bucket = StorageBucket.fromBucketInfo(bucketInfo)
108115
val service = plugin.getStorageService(bucket)
@@ -123,7 +130,7 @@ class AWSS3StoragePluginTest {
123130
}
124131
}
125132

126-
plugin.configure(data, mockk())
133+
plugin.configure(data, context)
127134
val bucket = StorageBucket.fromOutputs("test=name")
128135
val service = plugin.getStorageService(bucket)
129136
service shouldNotBe null
@@ -143,7 +150,7 @@ class AWSS3StoragePluginTest {
143150
}
144151
}
145152

146-
plugin.configure(data, mockk())
153+
plugin.configure(data, context)
147154
val bucket = StorageBucket.fromOutputs("myBucket")
148155
val exception = shouldThrow<StorageException> {
149156
plugin.getStorageService(bucket)
@@ -165,7 +172,7 @@ class AWSS3StoragePluginTest {
165172
}
166173
}
167174

168-
plugin.configure(data, mockk())
175+
plugin.configure(data, context)
169176
val bucket = StorageBucket.fromOutputs("test-name")
170177
val result = plugin.getStorageServiceResult(bucket)
171178
val service = result.storageService
@@ -188,7 +195,7 @@ class AWSS3StoragePluginTest {
188195
}
189196
}
190197

191-
plugin.configure(data, mockk())
198+
plugin.configure(data, context)
192199
val bucket = StorageBucket.fromOutputs("myBucket")
193200
val exception = plugin.getStorageServiceResult(bucket).storageException
194201
exception shouldNotBe null

‎aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/StorageComponentTest.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,13 @@ public final class StorageComponentTest {
9090
public void setup() throws AmplifyException {
9191
this.storage = new StorageCategory();
9292
this.storageService = mock(AWSS3StorageService.class);
93-
AWSS3StorageService.Factory storageServiceFactory
94-
= (context, region, bucket, clientProvider) -> (AWSS3StorageService) storageService;
93+
AWSS3StorageService.Factory storageServiceFactory = (
94+
context,
95+
region,
96+
bucket,
97+
clientProvider,
98+
transferStatusUpdater
99+
) -> (AWSS3StorageService) storageService;
95100
AuthCredentialsProvider cognitoAuthProvider = mock(AuthCredentialsProvider.class);
96101
doReturn(RandomString.string()).when(cognitoAuthProvider).getIdentityId(null);
97102
this.storage.addPlugin(new AWSS3StoragePlugin(storageServiceFactory,

0 commit comments

Comments
 (0)