Make GsonGraphQLResponseFactory.buildResponse throw on empty string

aws-amplify · Jan 5, 2023 · 0fe15b9 · 0fe15b9
1 parent b85d973
commit 0fe15b9
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 4 deletions.
diff --git a/aws-api/src/main/java/com/amplifyframework/api/aws/GsonGraphQLResponseFactory.java b/aws-api/src/main/java/com/amplifyframework/api/aws/GsonGraphQLResponseFactory.java
@@ -22,6 +22,7 @@
 import com.amplifyframework.api.graphql.GraphQLRequest;
 import com.amplifyframework.api.graphql.GraphQLResponse;
 import com.amplifyframework.api.graphql.PaginatedResult;
+import com.amplifyframework.util.Empty;
 import com.amplifyframework.util.GsonFactory;
 import com.amplifyframework.util.TypeMaker;
 
@@ -56,6 +57,18 @@ final class GsonGraphQLResponseFactory implements GraphQLResponse.Factory {
     @Override
     public <T> GraphQLResponse<T> buildResponse(GraphQLRequest<T> request, String responseJson)
             throws ApiException {
+
+        // On empty strings, Gson returns null instead of throwing JsonSyntaxException. See:
+        // https://github.com/google/gson/issues/457
+        // https://github.com/google/gson/issues/1697
+        if (Empty.check(responseJson)) {
+            throw new ApiException(
+                "Amplify encountered an error while deserializing an object.",
+                new JsonParseException("Empty response."),
+                AmplifyException.TODO_RECOVERY_SUGGESTION
+            );
+        }
+
         Type responseType = TypeMaker.getParameterizedType(GraphQLResponse.class, request.getResponseType());
         try {
             Gson responseGson = gson.newBuilder()

diff --git a/aws-api/src/test/java/com/amplifyframework/api/aws/GsonGraphQLResponseFactoryTest.java b/aws-api/src/test/java/com/amplifyframework/api/aws/GsonGraphQLResponseFactoryTest.java
@@ -114,6 +114,26 @@ public void nonJsonResponseThrowsApiException() throws ApiException {
         });
     }
 
+    /**
+     * Validates that an empty response throws an
+     * ApiException instead of returning a null reference.
+     * @throws ApiException From API configuration
+     */
+    @Test
+    public void emptyResponseThrowsApiException() throws ApiException {
+        // Arrange some empty string from a "server"
+        final String emptyResponse = "";
+
+        // Act! Parse it into a model.
+        Type responseType = TypeMaker.getParameterizedType(PaginatedResult.class, Todo.class);
+        GraphQLRequest<PaginatedResult<Todo>> request = buildDummyRequest(responseType);
+
+        // Assert that the appropriate exception is thrown
+        assertThrows(ApiException.class, () -> {
+            responseFactory.buildResponse(request, emptyResponse);
+        });
+    }
+
     /**
      * Validates that the converter is able to parse a partial GraphQL
      * response into a result. In this case, the result contains some

diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/AWSDataStorePlugin.java b/aws-datastore/src/main/java/com/amplifyframework/datastore/AWSDataStorePlugin.java
@@ -774,6 +774,7 @@ public Builder authModeStrategy(AuthModeStrategyType authModeStrategy) {
 
         /**
          * Enables Retry on DataStore sync engine.
+         * @deprecated This configuration will be deprecated in a future version.
          * @param isSyncRetryEnabled is sync retry enabled.
          * @return An implementation of the {@link ModelProvider} interface.
          */

diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/DataStoreConfiguration.java b/aws-datastore/src/main/java/com/amplifyframework/datastore/DataStoreConfiguration.java
@@ -38,20 +38,20 @@
  * Configuration options for {@link AWSDataStorePlugin}.
  */
 public final class DataStoreConfiguration {
-    private static final Logger LOG = Amplify.Logging.forNamespace("amplify:aws-datastore");
-
     static final String PLUGIN_CONFIG_KEY = "awsDataStorePlugin";
     @VisibleForTesting
     static final long DEFAULT_SYNC_INTERVAL_MINUTES = TimeUnit.DAYS.toMinutes(1);
     @VisibleForTesting
     static final int DEFAULT_SYNC_MAX_RECORDS = 10_000;
-    @VisibleForTesting 
+    @VisibleForTesting
     static final int DEFAULT_SYNC_PAGE_SIZE = 1_000;
     @VisibleForTesting
     static final boolean DEFAULT_DO_SYNC_RETRY = false;
     static final int MAX_RECORDS = 1000;
     static final long MAX_TIME_SEC = 2;
 
+    private static final Logger LOG = Amplify.Logging.forNamespace("amplify:aws-datastore");
+
     private final DataStoreErrorHandler errorHandler;
     private final DataStoreConflictHandler conflictHandler;
     private final Integer syncMaxRecords;
@@ -403,6 +403,8 @@ public Builder syncMaxRecords(@IntRange(from = 0) Integer syncMaxRecords) {
 
         /**
          * Sets the retry enabled on datastore sync.
+         *
+         * @deprecated This configuration will be deprecated in a future version.
          * @param doSyncRetry Is retry enabled on datastore sync
          * @return Current builder instance
          */

diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/SyncProcessor.java b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/SyncProcessor.java
@@ -77,7 +77,7 @@ final class SyncProcessor {
 
     /**
      * The `isSyncRetryEnabled` value is being passed down all the way from the `AWSDataStorePlugin` or the
-     * `DataStoreConfiguration` where it's named `doSyncRetry`
+     * `DataStoreConfiguration` where it's named `doSyncRetry`.
      */
     private final boolean isSyncRetryEnabled;
 

diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/DataStoreConfigurationTest.java b/aws-datastore/src/test/java/com/amplifyframework/datastore/DataStoreConfigurationTest.java
@@ -112,6 +112,8 @@ public void testDefaultOverriddenFromConfigurationAndObject()
 
         DataStoreSyncExpression ownerSyncExpression = () -> BlogOwner.ID.beginsWith(RandomString.string());
         DataStoreSyncExpression postSyncExpression = () -> Post.ID.beginsWith(RandomString.string());
+
+        @SuppressWarnings("deprecation")
         DataStoreConfiguration configObject = DataStoreConfiguration
             .builder()
             .syncMaxRecords(expectedSyncMaxRecords)