Save eventually issue (#831)

* - Added a failing test for #827
- The eventually queue should be cleared at each test execution but Parse.getEventuallyQueue().clear() crashes, that's why line 1585 is commented

* Removed unused imports

* Fixed ParseUserTest.testSaveEventuallyWhenSessionIsInvalid() so it reproduce exactly issue #827

* Fixed server response mocking in ParseUserTest.testSaveEventuallyWhenSessionIsInvalid()

* Failing test for #827 fails when the deadlock at ParseObject.java:L1604 occurs

* * Suggestion of fix for #827
* Improved test for #827 :
- Generalize to any server error that is different to CONNECTION_FAILED (not only INVALID_SESSION_TOKEN)
- Added isDirty() checks
- fixed test tearDown

* Working fix suggestion for #827

Author: cjosepha

Parse/src/main/java/com/parse/Parse.java

@@ -248,6 +248,10 @@ public static boolean isLocalDatastoreEnabled() {
    * @param configuration The configuration for your application.
    */
   public static void initialize(Configuration configuration) {
+    initialize(configuration, null);
+  }
+
+  static void initialize(Configuration configuration, ParsePlugins parsePlugins) {
     if (isInitialized()) {
       PLog.w(TAG, "Parse is already initialized");
       return;
@@ -256,7 +260,11 @@ public static void initialize(Configuration configuration) {
     // isLocalDataStoreEnabled() to perform additional behavior.
     isLocalDatastoreEnabled = configuration.localDataStoreEnabled;
 
-    ParsePlugins.initialize(configuration.context, configuration);
+    if (parsePlugins == null) {
+      ParsePlugins.initialize(configuration.context, configuration);
+    } else {
+      ParsePlugins.set(parsePlugins);
+    }
 
     try {
       ParseRESTCommand.server = new URL(configuration.server);
@@ -311,6 +319,8 @@ public Void then(Task<Void> task) throws Exception {
   }
 
   static void destroy() {
+    ParseObject.unregisterParseSubclasses();
+
     ParseEventuallyQueue queue;
     synchronized (MUTEX) {
       queue = eventuallyQueue;
@@ -322,6 +332,8 @@ static void destroy() {
 
     ParseCorePlugins.getInstance().reset();
     ParsePlugins.reset();
+
+    setLocalDatastore(null);
   }
 
   /**

Parse/src/main/java/com/parse/ParseObject.java

@@ -1420,6 +1420,20 @@ private ParseRESTObjectCommand currentSaveEventuallyCommand(
       final ParseObject.State result, final ParseOperationSet operationsBeforeSave) {
     Task<Void> task = Task.forResult(null);
 
+    /*
+     * If this object is in the offline store, then we need to make sure that we pull in any dirty
+     * changes it may have before merging the server data into it.
+     */
+    final OfflineStore store = Parse.getLocalDatastore();
+    if (store != null) {
+      task = task.onSuccessTask(new Continuation<Void, Task<Void>>() {
+        @Override
+        public Task<Void> then(Task<Void> task) throws Exception {
+          return store.fetchLocallyAsync(ParseObject.this).makeVoid();
+        }
+      });
+    }
+
     final boolean success = result != null;
     synchronized (mutex) {
       // Find operationsBeforeSave in the queue so that we can remove it and move to the next
@@ -1433,24 +1447,22 @@ private ParseRESTObjectCommand currentSaveEventuallyCommand(
         // Merge the data from the failed save into the next save.
         ParseOperationSet nextOperation = opIterator.next();
         nextOperation.mergeFrom(operationsBeforeSave);
+        if (store != null) {
+          task = task.continueWithTask(new Continuation<Void, Task<Void>>() {
+            @Override
+            public Task<Void> then(Task<Void> task) throws Exception {
+              if (task.isFaulted()) {
+                return Task.forResult(null);
+              } else {
+                return store.updateDataForObjectAsync(ParseObject.this);
+              }
+            }
+          });
+        }
         return task;
       }
     }
 
-    /*
-     * If this object is in the offline store, then we need to make sure that we pull in any dirty
-     * changes it may have before merging the server data into it.
-     */
-    final OfflineStore store = Parse.getLocalDatastore();
-    if (store != null) {
-      task = task.onSuccessTask(new Continuation<Void, Task<Void>>() {
-        @Override
-        public Task<Void> then(Task<Void> task) throws Exception {
-          return store.fetchLocallyAsync(ParseObject.this).makeVoid();
-        }
-      });
-    }
-
     // fetchLocallyAsync will return an error if this object isn't in the LDS yet and that's ok
     task = task.continueWith(new Continuation<Void, Void>() {
       @Override

Parse/src/test/java/com/parse/ParseInstallationTest.java

@@ -146,7 +146,7 @@ public void testMissingRequiredFieldWhenSaveAsync() throws Exception {
     ParseInstallation installation = ParseInstallation.getCurrentInstallation();
     assertNotNull(installation);
     installation.put("key", "value");
-    installation.saveAsync(sessionToken, toAwait);
+    ParseTaskUtils.wait(installation.saveAsync(sessionToken, toAwait));
     verify(controller).getAsync();
     verify(objController, times(2)).saveAsync(
         any(ParseObject.State.class),
@@ -187,7 +187,7 @@ public void testObjectNotFoundWhenSaveAsync() throws Exception {
     assertNotNull(installation);
     installation.setState(state);
     installation.put("key", "value");
-    installation.saveAsync(sessionToken, toAwait);
+    ParseTaskUtils.wait(installation.saveAsync(sessionToken, toAwait));
 
     verify(controller).getAsync();
     verify(objController, times(2)).saveAsync(

Parse/src/test/java/com/parse/ParseTestUtils.java

@@ -8,12 +8,15 @@
  */
 package com.parse;
 
+import android.content.Context;
+
 import com.parse.http.ParseHttpRequest;
 import com.parse.http.ParseHttpResponse;
 
 import org.json.JSONObject;
 
 import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.IOException;
 
 import bolts.Task;
@@ -41,15 +44,51 @@ public static void setTestParseUser() {
 
   public static ParseHttpClient mockParseHttpClientWithResponse(
       JSONObject content, int statusCode, String reasonPhrase) throws IOException {
+    ParseHttpClient client = mock(ParseHttpClient.class);
+    updateMockParseHttpClientWithResponse(client, content, statusCode, reasonPhrase);
+    return client;
+  }
+
+  static void updateMockParseHttpClientWithResponse(
+          ParseHttpClient client, JSONObject content, int statusCode, String reasonPhrase) throws IOException {
     byte[] contentBytes = content.toString().getBytes();
     ParseHttpResponse response = new ParseHttpResponse.Builder()
-        .setContent(new ByteArrayInputStream(contentBytes))
-        .setStatusCode(statusCode)
-        .setTotalSize(contentBytes.length)
-        .setContentType("application/json")
-        .build();
-    ParseHttpClient client = mock(ParseHttpClient.class);
+            .setContent(new ByteArrayInputStream(contentBytes))
+            .setStatusCode(statusCode)
+            .setTotalSize(contentBytes.length)
+            .setContentType("application/json")
+            .build();
     when(client.execute(any(ParseHttpRequest.class))).thenReturn(response);
-    return client;
+  }
+
+  static ParsePlugins mockParsePlugins(Parse.Configuration configuration) {
+    ParsePlugins parsePlugins = mock(ParsePlugins.class);
+    when(parsePlugins.applicationId()).thenReturn(configuration.applicationId);
+    when(parsePlugins.clientKey()).thenReturn(configuration.clientKey);
+    when(parsePlugins.configuration()).thenReturn(configuration);
+    Context applicationContext = configuration.context.getApplicationContext();
+    when(parsePlugins.applicationContext()).thenReturn(applicationContext);
+    File parseDir = createFileDir(applicationContext.getDir("Parse", Context.MODE_PRIVATE));
+    when(parsePlugins.installationId())
+            .thenReturn(
+                    new InstallationId(new File(parseDir, "installationId")));
+    when(parsePlugins.getParseDir())
+            .thenReturn(parseDir);
+    when(parsePlugins.getCacheDir())
+            .thenReturn(createFileDir(
+                    new File(applicationContext.getCacheDir(), "com.parse")));
+    when(parsePlugins.getFilesDir())
+            .thenReturn(createFileDir(
+                    new File(applicationContext.getFilesDir(), "com.parse")));
+    return parsePlugins;
+  }
+
+  private static File createFileDir(File file) {
+    if (!file.exists()) {
+      if (!file.mkdirs()) {
+        return file;
+      }
+    }
+    return file;
   }
 }

Parse/src/test/java/com/parse/ParseUserTest.java

@@ -8,6 +8,7 @@
  */
 package com.parse;
 
+import android.Manifest;
 import android.os.Parcel;
 
 import org.json.JSONObject;
@@ -20,14 +21,20 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Matchers;
 import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.Shadows;
 import org.robolectric.annotation.Config;
 
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 
+import bolts.Capture;
+import bolts.Continuation;
 import bolts.Task;
 
 import static org.junit.Assert.assertEquals;
@@ -68,9 +75,19 @@ public void setUp() throws Exception {
   @After
   public void tearDown() throws Exception {
     super.tearDown();
-    ParseObject.unregisterSubclass(ParseUser.class);
-    ParseObject.unregisterSubclass(ParseSession.class);
-    Parse.disableLocalDatastore();
+    if (ParsePlugins.get() != null) {
+      ParseCurrentInstallationController installationController =
+              ParseCorePlugins.getInstance().getCurrentInstallationController();
+      if (installationController != null) {
+        installationController.clearFromDisk();
+      }
+      ParseCurrentUserController userController =
+              ParseCorePlugins.getInstance().getCurrentUserController();
+      if (userController != null) {
+        userController.clearFromDisk();
+      }
+    }
+    Parse.destroy();
   }
 
   @Test
@@ -1541,4 +1558,111 @@ private static void setLazy(ParseUser user) {
     anonymousAuthData.put("anonymousToken", "anonymousTest");
     user.putAuthData(ParseAnonymousUtils.AUTH_TYPE, anonymousAuthData);
   }
+
+  //region testSaveEventuallyWhenServerError
+
+  @Test
+  public void testSaveEventuallyWhenServerError() throws Exception {
+    Shadows.shadowOf(RuntimeEnvironment.application)
+            .grantPermissions(Manifest.permission.ACCESS_NETWORK_STATE);
+    Parse.Configuration configuration =
+            new Parse.Configuration.Builder(RuntimeEnvironment.application)
+            .applicationId(BuildConfig.APPLICATION_ID)
+            .server("https://api.parse.com/1")
+            .enableLocalDataStore()
+            .build();
+    ParsePlugins plugins = ParseTestUtils.mockParsePlugins(configuration);
+    JSONObject mockResponse = new JSONObject();
+    mockResponse.put("objectId", "objectId");
+    mockResponse.put("email", "email@parse.com");
+    mockResponse.put("username", "username");
+    mockResponse.put("sessionToken", "r:sessionToken");
+    mockResponse.put("createdAt", ParseDateFormat.getInstance().format(new Date(1000)));
+    mockResponse.put("updatedAt", ParseDateFormat.getInstance().format(new Date(2000)));
+    ParseHttpClient restClient = ParseTestUtils.mockParseHttpClientWithResponse(
+            mockResponse,200, "OK");
+    when(plugins.restClient())
+            .thenReturn(restClient);
+    Parse.initialize(configuration, plugins);
+
+    ParseUser user = ParseUser.logIn("username", "password");
+    assertFalse(user.isDirty());
+
+    user.put("field", "data");
+    assertTrue(user.isDirty());
+
+    mockResponse = new JSONObject();
+    mockResponse.put("updatedAt", ParseDateFormat.getInstance().format(new Date(3000)));
+    ParseTestUtils.updateMockParseHttpClientWithResponse(
+            restClient, mockResponse, 200, "OK");
+
+    final CountDownLatch saveCountDown1 = new CountDownLatch(1);
+    final Capture<Exception> exceptionCapture = new Capture<>();
+    user.saveInBackground().continueWith(new Continuation<Void, Void>() {
+      @Override
+      public Void then(Task<Void> task) throws Exception {
+        exceptionCapture.set(task.getError());
+        saveCountDown1.countDown();
+        return null;
+      }
+    });
+    assertTrue(saveCountDown1.await(5, TimeUnit.SECONDS));
+    assertNull(exceptionCapture.get());
+    assertFalse(user.isDirty());
+
+    user.put("field", "other data");
+    assertTrue(user.isDirty());
+
+    mockResponse = new JSONObject();
+    mockResponse.put("error", "Save is not allowed");
+    mockResponse.put("code", 141);
+    ParseTestUtils.updateMockParseHttpClientWithResponse(
+            restClient, mockResponse, 400, "Bad Request");
+
+    final CountDownLatch saveEventuallyCountDown = new CountDownLatch(1);
+    user.saveEventually().continueWith(new Continuation<Void, Void>() {
+      @Override
+      public Void then(Task<Void> task) throws Exception {
+        exceptionCapture.set(task.getError());
+        saveEventuallyCountDown.countDown();
+        return null;
+      }
+    });
+    assertTrue(saveEventuallyCountDown.await(5, TimeUnit.SECONDS));
+    assertTrue(exceptionCapture.get() instanceof ParseException);
+    assertEquals(ParseException.SCRIPT_ERROR, ((ParseException)exceptionCapture.get()).getCode());
+    assertEquals("Save is not allowed", exceptionCapture.get().getMessage());
+    assertTrue(user.isDirty());
+
+    // Simulate reboot
+    Parse.destroy();
+    Parse.initialize(configuration, plugins);
+
+    user = ParseUser.getCurrentUser();
+    assertTrue(user.isDirty());
+
+    assertEquals("other data", user.get("field"));
+    user.put("field", "another data");
+
+    mockResponse = new JSONObject();
+    mockResponse.put("updatedAt", ParseDateFormat.getInstance().format(new Date(4000)));
+    ParseTestUtils.updateMockParseHttpClientWithResponse(
+            restClient, mockResponse, 200, "OK");
+
+    final CountDownLatch saveCountDown2 = new CountDownLatch(1);
+    user.saveInBackground().continueWith(new Continuation<Void, Void>() {
+      @Override
+      public Void then(Task<Void> task) throws Exception {
+        exceptionCapture.set(task.getError());
+        saveCountDown2.countDown();
+        return null;
+      }
+    });
+
+    assertTrue(saveCountDown2.await(5, TimeUnit.SECONDS));
+    assertNull(exceptionCapture.get());
+    assertFalse(user.isDirty());
+  }
+
+  //endregion
 }