Skip to content

Commit 04178b1

Browse files
lawmichatylerjroach
lawmicha
and
tylerjroach
authored
fix(DataStore): should not crash on missing version metadata (#2849)
Co-authored-by: Tyler Roach <[email protected]>
1 parent f2fd6f7 commit 04178b1

File tree

6 files changed

+45
-61
lines changed

6 files changed

+45
-61
lines changed

‎aws-datastore/src/main/java/com/amplifyframework/datastore/appsync/AppSync.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ <T extends Model> Cancelable create(
112112
<T extends Model> Cancelable update(
113113
@NonNull T model,
114114
@NonNull ModelSchema modelSchema,
115-
@NonNull Integer version,
115+
@Nullable Integer version,
116116
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
117117
@NonNull Consumer<DataStoreException> onFailure
118118
);
@@ -132,7 +132,7 @@ <T extends Model> Cancelable update(
132132
<T extends Model> Cancelable update(
133133
@NonNull T model,
134134
@NonNull ModelSchema modelSchema,
135-
@NonNull Integer version,
135+
@Nullable Integer version,
136136
@NonNull QueryPredicate predicate,
137137
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
138138
@NonNull Consumer<DataStoreException> onFailure
@@ -152,7 +152,7 @@ <T extends Model> Cancelable update(
152152
<T extends Model> Cancelable delete(
153153
@NonNull T model,
154154
@NonNull ModelSchema modelSchema,
155-
@NonNull Integer version,
155+
@Nullable Integer version,
156156
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
157157
@NonNull Consumer<DataStoreException> onFailure
158158
);
@@ -172,7 +172,7 @@ <T extends Model> Cancelable delete(
172172
<T extends Model> Cancelable delete(
173173
@NonNull T model,
174174
@NonNull ModelSchema modelSchema,
175-
@NonNull Integer version,
175+
@Nullable Integer version,
176176
@NonNull QueryPredicate predicate,
177177
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
178178
@NonNull Consumer<DataStoreException> onFailure

‎aws-datastore/src/main/java/com/amplifyframework/datastore/appsync/AppSyncClient.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public <T extends Model> Cancelable create(
159159
public <T extends Model> Cancelable update(
160160
@NonNull T model,
161161
@NonNull ModelSchema modelSchema,
162-
@NonNull Integer version,
162+
@Nullable Integer version,
163163
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
164164
@NonNull Consumer<DataStoreException> onFailure) {
165165
return update(model, modelSchema, version, QueryPredicates.all(), onResponse, onFailure);
@@ -170,7 +170,7 @@ public <T extends Model> Cancelable update(
170170
public <T extends Model> Cancelable update(
171171
@NonNull T model,
172172
@NonNull ModelSchema modelSchema,
173-
@NonNull Integer version,
173+
@Nullable Integer version,
174174
@NonNull QueryPredicate predicate,
175175
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
176176
@NonNull Consumer<DataStoreException> onFailure) {
@@ -207,7 +207,7 @@ public <T extends Model> Cancelable update(
207207
public <T extends Model> Cancelable delete(
208208
@NonNull T model,
209209
@NonNull ModelSchema modelSchema,
210-
@NonNull Integer version,
210+
@Nullable Integer version,
211211
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
212212
@NonNull Consumer<DataStoreException> onFailure) {
213213
return delete(model, modelSchema, version, QueryPredicates.all(), onResponse, onFailure);
@@ -218,7 +218,7 @@ public <T extends Model> Cancelable delete(
218218
public <T extends Model> Cancelable delete(
219219
@NonNull T model,
220220
@NonNull ModelSchema modelSchema,
221-
@NonNull Integer version,
221+
@Nullable Integer version,
222222
@NonNull QueryPredicate predicate,
223223
@NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
224224
@NonNull Consumer<DataStoreException> onFailure) {

‎aws-datastore/src/main/java/com/amplifyframework/datastore/appsync/AppSyncRequestFactory.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ static <M extends Model> AppSyncGraphQLRequest<ModelWithMetadata<M>> buildDeleti
167167
throws DataStoreException {
168168
try {
169169
Map<String, Object> inputMap = new HashMap<>();
170-
inputMap.put("_version", version);
170+
if (version != null) {
171+
inputMap.put("_version", version);
172+
}
171173
inputMap.putAll(GraphQLRequestHelper.getDeleteMutationInputMap(schema, model));
172174
return buildMutation(schema, inputMap, predicate, MutationType.DELETE, strategyType);
173175
} catch (AmplifyException amplifyException) {
@@ -184,7 +186,9 @@ static <M extends Model> AppSyncGraphQLRequest<ModelWithMetadata<M>> buildUpdate
184186
AuthModeStrategyType strategyType) throws DataStoreException {
185187
try {
186188
Map<String, Object> inputMap = new HashMap<>();
187-
inputMap.put("_version", version);
189+
if (version != null) {
190+
inputMap.put("_version", version);
191+
}
188192
inputMap.putAll(GraphQLRequestHelper.getMapOfFieldNameAndValues(schema, model, MutationType.UPDATE));
189193
return buildMutation(schema, inputMap, predicate, MutationType.UPDATE, strategyType);
190194
} catch (AmplifyException amplifyException) {

‎aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/MutationProcessor.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,8 @@ private <T extends Model> Single<ModelWithMetadata<T>> update(PendingMutation<T>
290290
this.schemaRegistry.getModelSchemaForModelClass(updatedItem.getModelName());
291291
return versionRepository.findModelVersion(updatedItem).flatMap(version ->
292292
publishWithStrategy(mutation, (model, onSuccess, onError) ->
293-
appSync.update(model, updatedItemSchema, version, mutation.getPredicate(), onSuccess, onError)
293+
appSync.update(
294+
model, updatedItemSchema, version.orElse(null), mutation.getPredicate(), onSuccess, onError)
294295
)
295296
);
296297
}
@@ -312,7 +313,7 @@ private <T extends Model> Single<ModelWithMetadata<T>> delete(PendingMutation<T>
312313
return versionRepository.findModelVersion(deletedItem).flatMap(version ->
313314
publishWithStrategy(mutation, (model, onSuccess, onError) ->
314315
appSync.delete(
315-
deletedItem, deletedItemSchema, version, mutation.getPredicate(), onSuccess, onError
316+
deletedItem, deletedItemSchema, version.orElse(null), mutation.getPredicate(), onSuccess, onError
316317
)
317318
)
318319
);

‎aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/VersionRepository.kt‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import com.amplifyframework.datastore.extensions.getMetadataSQLitePrimaryKey
2929
import com.amplifyframework.datastore.storage.LocalStorageAdapter
3030
import io.reactivex.rxjava3.core.Single
3131
import io.reactivex.rxjava3.core.SingleEmitter
32+
import java.util.Optional
3233
import kotlin.coroutines.resume
3334
import kotlin.coroutines.resumeWithException
3435
import kotlin.coroutines.suspendCoroutine
@@ -48,8 +49,8 @@ internal class VersionRepository(private val localStorageAdapter: LocalStorageAd
4849
* @param <T> Type of model
4950
* @return Current version known locally
5051
</T> */
51-
fun <T : Model> findModelVersion(model: T): Single<Int> {
52-
return Single.create { emitter: SingleEmitter<Int> ->
52+
fun <T : Model> findModelVersion(model: T): Single<Optional<Int>> {
53+
return Single.create { emitter: SingleEmitter<Optional<Int>> ->
5354
// The ModelMetadata for the model uses the same ID as an identifier.
5455
localStorageAdapter.query(
5556
ModelMetadata::class.java,
@@ -129,32 +130,27 @@ internal class VersionRepository(private val localStorageAdapter: LocalStorageAd
129130
* @param metadataIterator An iterator of ModelMetadata; the metadata is associated with the provided model
130131
* @param <T> The type of model
131132
* @return The version of the model, if available
132-
* @throws DataStoreException If there is no version for the model, or if the version cannot be obtained
133+
* @throws DataStoreException If there is no metadata for the model
133134
</T> */
134135
@Throws(DataStoreException::class)
135136
private fun <T : Model> extractVersion(
136137
model: T,
137138
metadataIterator: Iterator<ModelMetadata>
138-
): Int {
139+
): Optional<Int> {
139140
val results: MutableList<ModelMetadata> =
140141
ArrayList()
141142
while (metadataIterator.hasNext()) {
142143
results.add(metadataIterator.next())
143144
}
144-
145145
// There should be only one metadata for the model....
146146
if (results.size != 1) {
147-
throw DataStoreException(
148-
"Wanted 1 metadata for item with id = " + model.primaryKeyString + ", but had " + results.size +
149-
".",
150-
"This is likely a bug. please report to AWS."
147+
LOG.warn(
148+
"Wanted 1 metadata for item with id = " + model.primaryKeyString + ", but had " + results.size + "."
151149
)
150+
return Optional.empty()
151+
} else {
152+
return Optional.ofNullable(results[0].version)
152153
}
153-
return results[0].version
154-
?: throw DataStoreException(
155-
"Metadata for item with id = " + model.primaryKeyString + " had null version.",
156-
"This is likely a bug. Please report to AWS."
157-
)
158154
}
159155

160156
companion object {

‎aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/VersionRepositoryTest.kt‎

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import com.amplifyframework.datastore.appsync.ModelWithMetadata
2121
import com.amplifyframework.datastore.storage.InMemoryStorageAdapter
2222
import com.amplifyframework.datastore.storage.SynchronousStorageAdapter
2323
import com.amplifyframework.testmodels.commentsblog.BlogOwner
24-
import java.util.Locale
24+
import java.util.Optional
2525
import java.util.Random
2626
import java.util.concurrent.TimeUnit
2727
import kotlin.time.Duration.Companion.seconds
@@ -55,12 +55,12 @@ class VersionRepositoryTest {
5555

5656
/**
5757
* When you try to get a model version, but there's no metadata for that model,
58-
* this should fail with an [DataStoreException].
58+
* this should return empty.
5959
* @throws InterruptedException If interrupted while awaiting terminal result in test observer
6060
*/
6161
@Test
6262
@Throws(InterruptedException::class)
63-
fun emitsErrorForNoMetadataInRepo() {
63+
fun emitsSuccessWithEmptyValueForNoMetadataInRepo() {
6464
// Arrange: no metadata is in the repo.
6565
val blogOwner = BlogOwner.builder()
6666
.name("Jameson Williams")
@@ -73,33 +73,22 @@ class VersionRepositoryTest {
7373
val observer = versionRepository.findModelVersion(blogOwner).test()
7474
assertTrue(observer.await(REASONABLE_WAIT_TIME, TimeUnit.MILLISECONDS))
7575

76-
// Assert: this failed. There was no version available.
77-
observer.assertError { error: Throwable ->
78-
if (error !is DataStoreException) {
79-
return@assertError false
80-
}
81-
val expectedMessage = String.format(
82-
Locale.US,
83-
"Wanted 1 metadata for item with id = %s, but had 0.",
84-
blogOwner.id
85-
)
86-
expectedMessage == error.message
87-
}
76+
// Assert: we got a empty version
77+
observer
78+
.assertNoErrors()
79+
.assertComplete()
80+
.assertValue(Optional.empty())
8881
}
8982

9083
/**
9184
* When you try to get the version for a model, and there is metadata for the model
9285
* in the DataStore, BUT the version info is not populated, this should return an
93-
* [DataStoreException].
94-
* @throws DataStoreException
95-
* NOT EXPECTED. This happens on failure to arrange data before test action.
96-
* The expected DataStoreException is communicated via callback, not thrown
97-
* on the calling thread. It's a different thing than this.
86+
* empty optional.
9887
* @throws InterruptedException If interrupted while awaiting terminal result in test observer
9988
*/
10089
@Test
10190
@Throws(DataStoreException::class, InterruptedException::class)
102-
fun emitsErrorWhenMetadataHasNullVersion() {
91+
fun emitsSuccessWithEmptyValueWhenMetadataHasNullVersion() {
10392
// Arrange a model an metadata into the store, but the metadtaa doesn't contain a valid version
10493
val blogOwner = BlogOwner.builder()
10594
.name("Jameson")
@@ -114,18 +103,11 @@ class VersionRepositoryTest {
114103
val observer = versionRepository.findModelVersion(blogOwner).test()
115104
assertTrue(observer.await(REASONABLE_WAIT_TIME, TimeUnit.MILLISECONDS))
116105

117-
// Assert: the single emitted a DataStoreException.
118-
observer.assertError { error: Throwable ->
119-
if (error !is DataStoreException) {
120-
return@assertError false
121-
}
122-
val expectedMessage = String.format(
123-
Locale.US,
124-
"Metadata for item with id = %s had null version.",
125-
blogOwner.id
126-
)
127-
expectedMessage == error.message
128-
}
106+
// Assert: we got a empty version
107+
observer
108+
.assertNoErrors()
109+
.assertComplete()
110+
.assertValue(Optional.empty())
129111
}
130112

131113
/**
@@ -142,12 +124,13 @@ class VersionRepositoryTest {
142124
.name("Jameson")
143125
.build()
144126
val maxRandomVersion = 1000
145-
val expectedVersion = Random().nextInt(maxRandomVersion)
127+
val randomVersion = Random().nextInt(maxRandomVersion)
128+
val expectedVersion = Optional.ofNullable(randomVersion)
146129
storageAdapter.save(
147130
ModelMetadata(
148131
owner.modelName + "|" + owner.id,
149132
false,
150-
expectedVersion,
133+
randomVersion,
151134
Temporal.Timestamp.now()
152135
)
153136
)

0 commit comments

Comments
 (0)