pr comments

Author: tylerjroach

aws-api-appsync/build.gradle.kts

@@ -38,6 +38,7 @@ dependencies {
     testImplementation(libs.test.junit)
     testImplementation(libs.test.robolectric)
     testImplementation(libs.test.jsonassert)
+    testImplementation(libs.test.kotest.assertions)
     testImplementation(project(":testmodels"))
     testImplementation(project(":testutils"))
 }

aws-api-appsync/src/main/java/com/amplifyframework/core/model/query/predicate/GsonPredicateAdapters.java

@@ -162,6 +162,8 @@ public QueryPredicate deserialize(JsonElement json, Type type, JsonDeserializati
                 case OPERATION:
                     return gson.fromJson(json, QueryPredicateOperation.class);
                 case GROUP:
+                    // We need to manually deserialize Groups to ensure we handle nested groups
+                    // and update _types correctly.
                     return deserializeQueryPredicateGroup(jsonObject);
                 case ALL:
                     return gson.fromJson(json, MatchAllQueryPredicate.class);
@@ -182,6 +184,8 @@ public JsonElement serialize(QueryPredicate predicate, Type type, JsonSerializat
             JsonElement json;
             PredicateType predicateType;
             if (predicate instanceof QueryPredicateGroup) {
+                // We need to manually serialize Groups to ensure we handle nested groups and
+                // update _types correctly.
                 predicateType = PredicateType.GROUP;
                 json = serializeQueryPredicateGroup((QueryPredicateGroup) predicate, context);
             } else {
@@ -201,19 +205,58 @@ public JsonElement serialize(QueryPredicate predicate, Type type, JsonSerializat
             return jsonObject;
         }
 
+        /**
+         * Serializes a QueryPredicateGroup to JSON format.
+         * <p>
+         * This method is necessary because QueryPredicateGroup contains nested QueryPredicate objects
+         * that need to be recursively serialized. We cannot use context.serialize() directly on
+         * QueryPredicateGroup because:
+         * 1. Using context.serialize() would cause infinite recursion back to this adapter
+         * 2. We need to manually construct the JSON structure with proper "_type" fields
+         * <p>
+         * The method handles:
+         * - Serializing the group type (AND, OR, NOT)
+         * - Recursively serializing each nested predicate in the predicates array
+         * - Maintaining the correct JSON structure expected by the deserializer
+         * 
+         * @param group The QueryPredicateGroup to serialize
+         * @param context The serialization context for handling nested QueryOperator objects
+         * @return JsonElement representing the serialized group
+         */
         private JsonElement serializeQueryPredicateGroup(QueryPredicateGroup group, JsonSerializationContext context) {
             JsonObject jsonObject = new JsonObject();
             jsonObject.addProperty("type", group.type().name());
 
             JsonArray predicatesArray = new JsonArray();
             for (QueryPredicate predicate : group.predicates()) {
+                // Recursively serialize nested predicates using this adapter
                 predicatesArray.add(serialize(predicate, QueryPredicate.class, context));
             }
             jsonObject.add("predicates", predicatesArray);
 
             return jsonObject;
         }
 
+        /**
+         * Deserializes a JSON object into a QueryPredicateGroup.
+         * <p>
+         * This method is necessary because QueryPredicateGroup contains nested QueryPredicate objects
+         * that need to be recursively deserialized. We cannot use context.deserialize() directly
+         * because:
+         * 1. Using context.deserialize() would cause infinite recursion back to this adapter
+         * 2. We need to manually parse the JSON structure and handle nested predicates
+         * <p>
+         * The method handles:
+         * - Parsing the group type (AND, OR, NOT) from the "type" field
+         * - Recursively deserializing each predicate in the "predicates" array
+         * - Creating the QueryPredicateGroup with the correct constructor (no builder available)
+         * <p>
+         * This is critical for DataStore sync expressions that contain nested predicate groups,
+         * which caused the "Interfaces can't be instantiated" error before this fix.
+         * 
+         * @param jsonObject The JSON object containing the group data
+         * @return QueryPredicateGroup instance with all nested predicates deserialized
+         */
         private QueryPredicateGroup deserializeQueryPredicateGroup(JsonObject jsonObject) {
             QueryPredicateGroup.Type type = QueryPredicateGroup.Type.valueOf(
                 jsonObject.get("type").getAsString()
@@ -222,10 +265,13 @@ private QueryPredicateGroup deserializeQueryPredicateGroup(JsonObject jsonObject
             List<QueryPredicate> predicates = new ArrayList<>();
             JsonArray predicatesArray = jsonObject.getAsJsonArray("predicates");
             for (JsonElement predicateElement : predicatesArray) {
+                // Recursively deserialize nested predicates using this adapter
+                // Note: Passing null for context since we handle recursion manually
                 QueryPredicate predicate = deserialize(predicateElement, QueryPredicate.class, null);
                 predicates.add(predicate);
             }
 
+            // Use constructor since QueryPredicateGroup doesn't have a builder
             return new QueryPredicateGroup(type, predicates);
         }
     }

aws-api-appsync/src/test/java/com/amplifyframework/core/model/query/predicate/GsonPredicateAdaptersTests.kt

@@ -16,7 +16,7 @@ package com.amplifyframework.core.model.query.predicate
 
 import com.amplifyframework.testmodels.todo.Todo
 import com.google.gson.Gson
-import org.junit.Assert.assertEquals
+import io.kotest.matchers.shouldBe
 import org.junit.Test
 
 class GsonPredicateAdaptersTests {
@@ -32,9 +32,9 @@ class GsonPredicateAdaptersTests {
         val expectedString = """
             {"field":"id","operator":{"value":"123","type":"EQUAL"},"_type":"OPERATION"}
         """.replace("\\s".toRegex(), "")
-        assertEquals(expectedString, queryPredicateString)
+        queryPredicateString shouldBe expectedString
         val deserializedPredicate = gson.fromJson(queryPredicateString, QueryPredicate::class.java)
-        assertEquals(queryPredicate, deserializedPredicate)
+        deserializedPredicate shouldBe queryPredicate
     }
 
     @Test
@@ -51,9 +51,9 @@ class GsonPredicateAdaptersTests {
                 "_type":"GROUP"
             }
         """.replace("\\s".toRegex(), "")
-        assertEquals(expectedString, queryPredicateString)
+        queryPredicateString shouldBe expectedString
         val deserializedPredicate = gson.fromJson(queryPredicateString, QueryPredicate::class.java)
-        assertEquals(queryPredicate, deserializedPredicate)
+        deserializedPredicate shouldBe queryPredicate
     }
 
     @Test
@@ -99,8 +99,8 @@ class GsonPredicateAdaptersTests {
                 "_type": "GROUP"
             }
         """.replace("\\s".toRegex(), "")
-        assertEquals(expectedString, queryPredicateString)
+        queryPredicateString shouldBe expectedString
         val deserializedPredicate = gson.fromJson(queryPredicateString, QueryPredicate::class.java)
-        assertEquals(queryPredicate, deserializedPredicate)
+        deserializedPredicate shouldBe queryPredicate
     }
 }

aws-datastore/build.gradle.kts

@@ -61,4 +61,6 @@ dependencies {
     androidTestImplementation(libs.rxjava)
     androidTestImplementation(libs.okhttp)
     androidTestImplementation(libs.oauth2)
+    androidTestImplementation(libs.test.kotest.assertions)
+
 }

aws-datastore/src/androidTest/java/com/amplifyframework/datastore/storage/sqlite/SQLiteStorageAdapterCreateTest.kt

@@ -20,8 +20,9 @@ import androidx.test.core.app.ApplicationProvider
 import com.amplifyframework.datastore.storage.SynchronousStorageAdapter
 import com.amplifyframework.datastore.syncengine.MigrationFlagsTable
 import com.amplifyframework.testmodels.commentsblog.AmplifyModelProvider
+import io.kotest.assertions.withClue
+import io.kotest.matchers.ints.shouldBePositive
 import org.junit.After
-import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
 
@@ -70,7 +71,9 @@ class SQLiteStorageAdapterCreateTest {
                     MigrationFlagsTable.CLEARED_V2_30_0_AND_BELOW_GROUP_SYNC_EXPRESSIONS
                 )
             ).use { cursor ->
-                Assert.assertTrue("Migration flag row was not created", cursor.count > 0)
+                withClue("Migration flag row was not created") {
+                    cursor.count.shouldBePositive()
+                }
             }
         } finally {
             database.close()