fix(DataStore): should not crash on missing version metadata

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

@@ -112,7 +112,7 @@ <T extends Model> Cancelable create(
     <T extends Model> Cancelable update(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure
     );
@@ -132,7 +132,7 @@ <T extends Model> Cancelable update(
     <T extends Model> Cancelable update(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull QueryPredicate predicate,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure
@@ -152,7 +152,7 @@ <T extends Model> Cancelable update(
     <T extends Model> Cancelable delete(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure
     );
@@ -172,7 +172,7 @@ <T extends Model> Cancelable delete(
     <T extends Model> Cancelable delete(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull QueryPredicate predicate,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure

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

@@ -159,7 +159,7 @@ public <T extends Model> Cancelable create(
     public <T extends Model> Cancelable update(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure) {
         return update(model, modelSchema, version, QueryPredicates.all(), onResponse, onFailure);
@@ -170,7 +170,7 @@ public <T extends Model> Cancelable update(
     public <T extends Model> Cancelable update(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull QueryPredicate predicate,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure) {
@@ -207,7 +207,7 @@ public <T extends Model> Cancelable update(
     public <T extends Model> Cancelable delete(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure) {
         return delete(model, modelSchema, version, QueryPredicates.all(), onResponse, onFailure);
@@ -218,7 +218,7 @@ public <T extends Model> Cancelable delete(
     public <T extends Model> Cancelable delete(
             @NonNull T model,
             @NonNull ModelSchema modelSchema,
-            @NonNull Integer version,
+            @Nullable Integer version,
             @NonNull QueryPredicate predicate,
             @NonNull Consumer<GraphQLResponse<ModelWithMetadata<T>>> onResponse,
             @NonNull Consumer<DataStoreException> onFailure) {

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

@@ -167,7 +167,9 @@ static <M extends Model> AppSyncGraphQLRequest<ModelWithMetadata<M>> buildDeleti
             throws DataStoreException {
         try {
             Map<String, Object> inputMap = new HashMap<>();
-            inputMap.put("_version", version);
+            if (version != null) {
+                inputMap.put("_version", version);
+            }
             inputMap.putAll(GraphQLRequestHelper.getDeleteMutationInputMap(schema, model));
             return buildMutation(schema, inputMap, predicate, MutationType.DELETE, strategyType);
         } catch (AmplifyException amplifyException) {
@@ -184,7 +186,9 @@ static <M extends Model> AppSyncGraphQLRequest<ModelWithMetadata<M>> buildUpdate
             AuthModeStrategyType strategyType) throws DataStoreException {
         try {
             Map<String, Object> inputMap = new HashMap<>();
-            inputMap.put("_version", version);
+            if (version != null) {
+                inputMap.put("_version", version);
+            }
             inputMap.putAll(GraphQLRequestHelper.getMapOfFieldNameAndValues(schema, model, MutationType.UPDATE));
             return buildMutation(schema, inputMap, predicate, MutationType.UPDATE, strategyType);
         } catch (AmplifyException amplifyException) {

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

@@ -290,7 +290,8 @@ private <T extends Model> Single<ModelWithMetadata<T>> update(PendingMutation<T>
             this.schemaRegistry.getModelSchemaForModelClass(updatedItem.getModelName());
         return versionRepository.findModelVersion(updatedItem).flatMap(version ->
             publishWithStrategy(mutation, (model, onSuccess, onError) ->
-                appSync.update(model, updatedItemSchema, version, mutation.getPredicate(), onSuccess, onError)
+                appSync.update(
+                    model, updatedItemSchema, version.orElse(null), mutation.getPredicate(), onSuccess, onError)
             )
         );
     }
@@ -312,7 +313,7 @@ private <T extends Model> Single<ModelWithMetadata<T>> delete(PendingMutation<T>
         return versionRepository.findModelVersion(deletedItem).flatMap(version ->
             publishWithStrategy(mutation, (model, onSuccess, onError) ->
                 appSync.delete(
-                    deletedItem, deletedItemSchema, version, mutation.getPredicate(), onSuccess, onError
+                    deletedItem, deletedItemSchema, version.orElse(null), mutation.getPredicate(), onSuccess, onError
                 )
             )
         );

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

@@ -29,6 +29,7 @@ import com.amplifyframework.datastore.extensions.getMetadataSQLitePrimaryKey
 import com.amplifyframework.datastore.storage.LocalStorageAdapter
 import io.reactivex.rxjava3.core.Single
 import io.reactivex.rxjava3.core.SingleEmitter
+import java.util.Optional
 import kotlin.coroutines.resume
 import kotlin.coroutines.resumeWithException
 import kotlin.coroutines.suspendCoroutine
@@ -48,8 +49,8 @@ internal class VersionRepository(private val localStorageAdapter: LocalStorageAd
      * @param <T> Type of model
      * @return Current version known locally
      </T> */
-    fun <T : Model> findModelVersion(model: T): Single<Int> {
-        return Single.create { emitter: SingleEmitter<Int> ->
+    fun <T : Model> findModelVersion(model: T): Single<Optional<Int>> {
+        return Single.create { emitter: SingleEmitter<Optional<Int>> ->
             // The ModelMetadata for the model uses the same ID as an identifier.
             localStorageAdapter.query(
                 ModelMetadata::class.java,
@@ -129,32 +130,27 @@ internal class VersionRepository(private val localStorageAdapter: LocalStorageAd
      * @param metadataIterator An iterator of ModelMetadata; the metadata is associated with the provided model
      * @param <T> The type of model
      * @return The version of the model, if available
-     * @throws DataStoreException If there is no version for the model, or if the version cannot be obtained
+     * @throws DataStoreException If there is no metadata for the model
      </T> */
     @Throws(DataStoreException::class)
     private fun <T : Model> extractVersion(
         model: T,
         metadataIterator: Iterator<ModelMetadata>
-    ): Int {
+    ): Optional<Int> {
         val results: MutableList<ModelMetadata> =
             ArrayList()
         while (metadataIterator.hasNext()) {
             results.add(metadataIterator.next())
         }
-
         // There should be only one metadata for the model....
         if (results.size != 1) {
-            throw DataStoreException(
-                "Wanted 1 metadata for item with id = " + model.primaryKeyString + ", but had " + results.size +
-                    ".",
-                "This is likely a bug. please report to AWS."
+            LOG.warn(
+                "Wanted 1 metadata for item with id = " + model.primaryKeyString + ", but had " + results.size + "."
             )
+            return Optional.empty()
+        } else {
+            return Optional.ofNullable(results[0].version)
         }
-        return results[0].version
-            ?: throw DataStoreException(
-                "Metadata for item with id = " + model.primaryKeyString + " had null version.",
-                "This is likely a bug. Please report to AWS."
-            )
     }
 
     companion object {

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

@@ -21,7 +21,7 @@ import com.amplifyframework.datastore.appsync.ModelWithMetadata
 import com.amplifyframework.datastore.storage.InMemoryStorageAdapter
 import com.amplifyframework.datastore.storage.SynchronousStorageAdapter
 import com.amplifyframework.testmodels.commentsblog.BlogOwner
-import java.util.Locale
+import java.util.Optional
 import java.util.Random
 import java.util.concurrent.TimeUnit
 import kotlin.time.Duration.Companion.seconds
@@ -55,12 +55,12 @@ class VersionRepositoryTest {
 
     /**
      * When you try to get a model version, but there's no metadata for that model,
-     * this should fail with an [DataStoreException].
+     * this should return empty.
      * @throws InterruptedException If interrupted while awaiting terminal result in test observer
      */
     @Test
     @Throws(InterruptedException::class)
-    fun emitsErrorForNoMetadataInRepo() {
+    fun emitsSuccessWithEmptyValueForNoMetadataInRepo() {
         // Arrange: no metadata is in the repo.
         val blogOwner = BlogOwner.builder()
             .name("Jameson Williams")
@@ -73,33 +73,22 @@ class VersionRepositoryTest {
         val observer = versionRepository.findModelVersion(blogOwner).test()
         assertTrue(observer.await(REASONABLE_WAIT_TIME, TimeUnit.MILLISECONDS))
 
-        // Assert: this failed. There was no version available.
-        observer.assertError { error: Throwable ->
-            if (error !is DataStoreException) {
-                return@assertError false
-            }
-            val expectedMessage = String.format(
-                Locale.US,
-                "Wanted 1 metadata for item with id = %s, but had 0.",
-                blogOwner.id
-            )
-            expectedMessage == error.message
-        }
+        // Assert: we got a empty version
+        observer
+            .assertNoErrors()
+            .assertComplete()
+            .assertValue(Optional.empty())
     }
 
     /**
      * When you try to get the version for a model, and there is metadata for the model
      * in the DataStore, BUT the version info is not populated, this should return an
-     * [DataStoreException].
-     * @throws DataStoreException
-     * NOT EXPECTED. This happens on failure to arrange data before test action.
-     * The expected DataStoreException is communicated via callback, not thrown
-     * on the calling thread. It's a different thing than this.
+     * empty optional.
      * @throws InterruptedException If interrupted while awaiting terminal result in test observer
      */
     @Test
     @Throws(DataStoreException::class, InterruptedException::class)
-    fun emitsErrorWhenMetadataHasNullVersion() {
+    fun emitsSuccessWithEmptyValueWhenMetadataHasNullVersion() {
         // Arrange a model an metadata into the store, but the metadtaa doesn't contain a valid version
         val blogOwner = BlogOwner.builder()
             .name("Jameson")
@@ -114,18 +103,11 @@ class VersionRepositoryTest {
         val observer = versionRepository.findModelVersion(blogOwner).test()
         assertTrue(observer.await(REASONABLE_WAIT_TIME, TimeUnit.MILLISECONDS))
 
-        // Assert: the single emitted a DataStoreException.
-        observer.assertError { error: Throwable ->
-            if (error !is DataStoreException) {
-                return@assertError false
-            }
-            val expectedMessage = String.format(
-                Locale.US,
-                "Metadata for item with id = %s had null version.",
-                blogOwner.id
-            )
-            expectedMessage == error.message
-        }
+        // Assert: we got a empty version
+        observer
+            .assertNoErrors()
+            .assertComplete()
+            .assertValue(Optional.empty())
     }
 
     /**
@@ -142,12 +124,13 @@ class VersionRepositoryTest {
             .name("Jameson")
             .build()
         val maxRandomVersion = 1000
-        val expectedVersion = Random().nextInt(maxRandomVersion)
+        val randomVersion = Random().nextInt(maxRandomVersion)
+        val expectedVersion = Optional.ofNullable(randomVersion)
         storageAdapter.save(
             ModelMetadata(
                 owner.modelName + "|" + owner.id,
                 false,
-                expectedVersion,
+                randomVersion,
                 Temporal.Timestamp.now()
             )
         )