Merge pull request #117 from ParsePlatform/grantland.authentication_async

Convert `restoreAuthentication` to asynchronous

Author: grantland

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

@@ -38,8 +38,8 @@ public Task<Void> deauthenticateAsync() {
   }
 
   @Override
-  public boolean restoreAuthentication(Map<String, String> authData) {
-    return true;
+  public Task<Boolean> restoreAuthenticationAsync(Map<String, String> authData) {
+    return Task.forResult(true);
   }
 
   @Override

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

@@ -55,18 +55,24 @@ public Task<Void> then(Task<Void> task) throws Exception {
             if (oldCurrentUser != null && oldCurrentUser != user) {
               // We don't need to revoke the token since we're not explicitly calling logOut
               // We don't need to remove persisted files since we're overwriting them
-              return oldCurrentUser.logOutAsync(false);
+              return oldCurrentUser.logOutAsync(false).continueWith(new Continuation<Void, Void>() {
+                @Override
+                public Void then(Task<Void> task) throws Exception {
+                  return null; // ignore errors
+                }
+              });
             }
             return task;
           }
-        }).continueWithTask(new Continuation<Void, Task<Void>>() {
+        }).onSuccessTask(new Continuation<Void, Task<Void>>() {
+          @Override
+          public Task<Void> then(Task<Void> task) throws Exception {
+            user.setIsCurrentUser(true);
+            return user.synchronizeAllAuthDataAsync();
+          }
+        }).onSuccessTask(new Continuation<Void, Task<Void>>() {
           @Override
           public Task<Void> then(Task<Void> task) throws Exception {
-            synchronized (user.mutex) {
-              user.setIsCurrentUser(true);
-              user.synchronizeAllAuthData();
-            }
-
             return store.setAsync(user).continueWith(new Continuation<Void, Void>() {
               @Override
               public Void then(Task<Void> task) throws Exception {

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

@@ -14,7 +14,7 @@
 import bolts.Continuation;
 import bolts.Task;
 
-/*** package */ class ParseAuthenticationManager {
+/** package */ class ParseAuthenticationManager {
 
   private final Object lock = new Object();
   private final Map<String, ParseAuthenticationProvider> authenticationProviders = new HashMap<>();
@@ -44,24 +44,27 @@ public void register(ParseAuthenticationProvider provider) {
     }
 
     // Synchronize the current user with the auth provider.
-    controller.getAsync(false).onSuccess(new Continuation<ParseUser, Void>() {
+    controller.getAsync(false).onSuccessTask(new Continuation<ParseUser, Task<Void>>() {
       @Override
-      public Void then(Task<ParseUser> task) throws Exception {
+      public Task<Void> then(Task<ParseUser> task) throws Exception {
         ParseUser user = task.getResult();
         if (user != null) {
-          user.synchronizeAuthData(authType);
+          return user.synchronizeAuthDataAsync(authType);
         }
         return null;
       }
     });
   }
 
-  public boolean restoreAuthentication(String authType, Map<String, String> authData) {
+  public Task<Boolean> restoreAuthenticationAsync(String authType, Map<String, String> authData) {
     ParseAuthenticationProvider provider;
     synchronized (lock) {
       provider = authenticationProviders.get(authType);
     }
-    return provider == null || provider.restoreAuthentication(authData);
+    if (provider == null) {
+      return Task.forResult(true);
+    }
+    return provider.restoreAuthenticationAsync(authData);
   }
 
   public Task<Void> deauthenticateAsync(String authType) {

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

@@ -24,16 +24,16 @@
   String getAuthType();
 
   /**
-   * Begins the authentication process and invokes onSuccess() or onError() on
-   * the callback upon completion. This call should not block.
+   * Authenticates with the service.
    *
-   * @return A task that will be resolved upon the completion of authentication.
+   * @return A {@code Task} that will be resolved when authentication is complete.
    */
   Task<Map<String, String>> authenticateAsync();
 
   /**
-   * Deauthenticates (logs out) the user associated with this provider. This
-   * call may block.
+   * Deauthenticates (logs out) the user associated with this provider. This call may block.
+   *
+   * @return A {@link Task} that resolves when deauthentication is complete.
    */
   Task<Void> deauthenticateAsync();
 
@@ -44,9 +44,10 @@
    * @param authData
    *          the auth data for the provider. This value may be null when
    *          unlinking an account.
-   * @return true iff the authData was successfully synchronized. A false return
-   *         value indicates that the user should no longer be associated
-   *         because of bad auth data.
+   *
+   * @return A {@link Task} that resolves to {@code true} iff the {@code authData} was successfully
+   *         synchronized or {@code false} if user should no longer be associated because of bad
+   *         {@code authData}.
    */
-  boolean restoreAuthentication(Map<String, String> authData);
+  Task<Boolean> restoreAuthenticationAsync(Map<String, String> authData);
 }

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

@@ -247,28 +247,32 @@ public void remove(String key) {
     return super.toRest(state, cleanOperationSetQueue, objectEncoder);
   }
 
-  /* package for tests */ void cleanUpAuthData() {
+  /* package for tests */ Task<Void> cleanUpAuthDataAsync() {
     ParseAuthenticationManager controller = getAuthenticationManager();
+    Map<String, Map<String, String>> authData;
     synchronized (mutex) {
-      Map<String, Map<String, String>> authData = getState().authData();
+      authData = getState().authData();
       if (authData.size() == 0) {
-        return; // Nothing to see or do here...
+        return Task.forResult(null); // Nothing to see or do here...
       }
+    }
 
-      Iterator<Map.Entry<String, Map<String, String>>> i = authData.entrySet().iterator();
-      while (i.hasNext()) {
-        Map.Entry<String, Map<String, String>> entry = i.next();
-        if (entry.getValue() == null) {
-          i.remove();
-          controller.restoreAuthentication(entry.getKey(), null);
-        }
+    List<Task<Void>> tasks = new ArrayList<>();
+    Iterator<Map.Entry<String, Map<String, String>>> i = authData.entrySet().iterator();
+    while (i.hasNext()) {
+      Map.Entry<String, Map<String, String>> entry = i.next();
+      if (entry.getValue() == null) {
+        i.remove();
+        tasks.add(controller.restoreAuthenticationAsync(entry.getKey(), null).makeVoid());
       }
-
-      State newState = getState().newBuilder()
-          .authData(authData)
-          .build();
-      setState(newState);
     }
+
+    State newState = getState().newBuilder()
+        .authData(authData)
+        .build();
+    setState(newState);
+
+    return Task.whenAll(tasks);
   }
 
   @Override
@@ -488,17 +492,22 @@ private void restoreAnonymity(Map<String, String> anonymousData) {
       task = super.saveAsync(sessionToken, toAwait);
     }
 
-    return task.onSuccessTask(new Continuation<Void, Task<Void>>() {
-      @Override
-      public Task<Void> then(Task<Void> task) throws Exception {
-        // If the user is the currently logged in user, we persist all data to disk
-        if (isCurrentUser()) {
-          cleanUpAuthData();
+    if (isCurrentUser()) {
+      // If the user is the currently logged in user, we persist all data to disk
+      return task.onSuccessTask(new Continuation<Void, Task<Void>>() {
+        @Override
+        public Task<Void> then(Task<Void> task) throws Exception {
+            return cleanUpAuthDataAsync();
+        }
+      }).onSuccessTask(new Continuation<Void, Task<Void>>() {
+        @Override
+        public Task<Void> then(Task<Void> task) throws Exception {
           return saveCurrentUserAsync(ParseUser.this);
         }
-        return Task.forResult(null);
-      }
-    });
+      });
+    }
+
+    return task;
   }
 
   @Override
@@ -531,29 +540,34 @@ public ParseUser fetch() throws ParseException {
   @Override
   /* package */ <T extends ParseObject> Task<T> fetchAsync(
       String sessionToken, Task<Void> toAwait) {
-    synchronized (mutex) {
-      //TODO (grantland): It doesn't seem like we should do this.. Why don't we error like we do
-      // when fetching an unsaved ParseObject?
-      if (isLazy()) {
-        return Task.forResult((T) this);
-      }
+    //TODO (grantland): It doesn't seem like we should do this.. Why don't we error like we do
+    // when fetching an unsaved ParseObject?
+    if (isLazy()) {
+      return Task.forResult((T) this);
+    }
 
-      return super.<T> fetchAsync(sessionToken, toAwait).onSuccessTask(new Continuation<T, Task<T>>() {
+    Task<T> task = super.fetchAsync(sessionToken, toAwait);
+
+    if (isCurrentUser()) {
+      return task.onSuccessTask(new Continuation<T, Task<Void>>() {
         @Override
-        public Task<T> then(final Task<T> fetchAsyncTask) throws Exception {
-          if (isCurrentUser()) {
-            cleanUpAuthData();
-            return saveCurrentUserAsync(ParseUser.this).continueWithTask(new Continuation<Void, Task<T>>() {
-              @Override
-              public Task<T> then(Task<Void> task) throws Exception {
-                return fetchAsyncTask;
-              }
-            });
-          }
-          return fetchAsyncTask;
+        public Task<Void> then(final Task<T> fetchAsyncTask) throws Exception {
+          return cleanUpAuthDataAsync();
+        }
+      }).onSuccessTask(new Continuation<Void, Task<Void>>() {
+        @Override
+        public Task<Void> then(Task<Void> task) throws Exception {
+          return saveCurrentUserAsync(ParseUser.this);
+        }
+      }).onSuccess(new Continuation<Void, T>() {
+        @Override
+        public T then(Task<Void> task) throws Exception {
+          return (T) ParseUser.this;
         }
       });
     }
+
+    return task;
   }
 
   /**
@@ -676,7 +690,8 @@ public Task<Void> then(Task<Void> task) throws Exception {
             @Override
             public Task<Void> then(final Task<ParseUser.State> signUpTask) throws Exception {
               ParseUser.State result = signUpTask.getResult();
-              return handleSaveResultAsync(result, operations).continueWithTask(new Continuation<Void, Task<Void>>() {
+              return handleSaveResultAsync(result,
+                  operations).continueWithTask(new Continuation<Void, Task<Void>>() {
                 @Override
                 public Task<Void> then(Task<Void> task) throws Exception {
                   if (!signUpTask.isCancelled() && !signUpTask.isFaulted()) {
@@ -1068,29 +1083,48 @@ public ParseUser fetchIfNeeded() throws ParseException {
     return authData.containsKey(authType) && authData.get(authType) != null;
   }
 
-  /* package */ void synchronizeAuthData(String authType) {
+  /**
+   * Ensures that all auth providers have auth data (e.g. access tokens, etc.) that matches this
+   * user.
+   */
+  /* package */ Task<Void> synchronizeAllAuthDataAsync() {
+    Map<String, Map<String, String>> authData;
     synchronized (mutex) {
       if (!isCurrentUser()) {
-        return;
-      }
-      boolean success = getAuthenticationManager()
-          .restoreAuthentication(authType, getAuthData(authType));
-      if (!success) {
-        unlinkFromAsync(authType);
+        return Task.forResult(null);
       }
+      authData = getAuthData();
     }
+    List<Task<Void>> tasks = new ArrayList<>(authData.size());
+    for (String authType : authData.keySet()) {
+      tasks.add(synchronizeAuthDataAsync(authType));
+    }
+    return Task.whenAll(tasks);
   }
 
-  /**
-   * Ensures that all auth providers have auth data (e.g. access tokens, etc.) that matches this
-   * user.
-   */
-  /* package */ void synchronizeAllAuthData() {
+  /* package */ Task<Void> synchronizeAuthDataAsync(String authType) {
+    Map<String, String> authData;
     synchronized (mutex) {
-      for (Map.Entry<String, Map<String, String>> entry : getAuthData().entrySet()) {
-        synchronizeAuthData(entry.getKey());
+      if (!isCurrentUser()) {
+        return Task.forResult(null);
       }
+      authData = getAuthData(authType);
     }
+    return synchronizeAuthDataAsync(getAuthenticationManager(), authType, authData);
+  }
+
+  private Task<Void> synchronizeAuthDataAsync(
+      ParseAuthenticationManager manager, final String authType, Map<String, String> authData) {
+    return manager.restoreAuthenticationAsync(authType, authData).onSuccessTask(new Continuation<Boolean, Task<Void>>() {
+      @Override
+      public Task<Void> then(Task<Boolean> task) throws Exception {
+        boolean success = task.getResult();
+        if (!success) {
+          return unlinkFromAsync(authType);
+        }
+        return task.makeVoid();
+      }
+    });
   }
 
   /* package */ Task<Void> unlinkFromAsync(final String authType) {
@@ -1236,8 +1270,7 @@ public Task<Void> then(Task<Void> task) throws Exception {
               restoreAnonymity(oldAnonymousData);
               return task;
             }
-            synchronizeAuthData(authType);
-            return task;
+            return synchronizeAuthDataAsync(authType);
           }
         }
       });

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

@@ -12,12 +12,14 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.mockito.Matchers;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import bolts.Task;
 
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -70,20 +72,22 @@ public void testRegister() {
 
     manager.register(provider);
     verify(controller).getAsync(false);
-    verify(user).synchronizeAuthData("test_provider");
+    verify(user).synchronizeAuthDataAsync("test_provider");
   }
 
   //endregion
 
   @Test
-  public void testRestoreAuthentication() {
+  public void testRestoreAuthentication() throws ParseException {
     when(controller.getAsync(false)).thenReturn(Task.<ParseUser>forResult(null));
+    when(provider.restoreAuthenticationAsync(Matchers.<Map<String, String>>any()))
+        .thenReturn(Task.forResult(true));
     manager.register(provider);
 
     Map<String, String> authData = new HashMap<>();
-    manager.restoreAuthentication("test_provider", authData);
+    assertTrue(ParseTaskUtils.wait(manager.restoreAuthenticationAsync("test_provider", authData)));
 
-    verify(provider).restoreAuthentication(authData);
+    verify(provider).restoreAuthenticationAsync(authData);
   }
 
   @Test