diff --git a/Parse/src/main/java/com/parse/AnonymousAuthenticationProvider.java b/Parse/src/main/java/com/parse/AnonymousAuthenticationProvider.java index c15b48eef..492a6b2dd 100644 --- a/Parse/src/main/java/com/parse/AnonymousAuthenticationProvider.java +++ b/Parse/src/main/java/com/parse/AnonymousAuthenticationProvider.java @@ -38,8 +38,8 @@ public Task deauthenticateAsync() { } @Override - public boolean restoreAuthentication(Map authData) { - return true; + public Task restoreAuthenticationAsync(Map authData) { + return Task.forResult(true); } @Override diff --git a/Parse/src/main/java/com/parse/CachedCurrentUserController.java b/Parse/src/main/java/com/parse/CachedCurrentUserController.java index 331e359fd..3d07893ec 100644 --- a/Parse/src/main/java/com/parse/CachedCurrentUserController.java +++ b/Parse/src/main/java/com/parse/CachedCurrentUserController.java @@ -55,18 +55,24 @@ public Task then(Task 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() { + @Override + public Void then(Task task) throws Exception { + return null; // ignore errors + } + }); } return task; } - }).continueWithTask(new Continuation>() { + }).onSuccessTask(new Continuation>() { + @Override + public Task then(Task task) throws Exception { + user.setIsCurrentUser(true); + return user.synchronizeAllAuthDataAsync(); + } + }).onSuccessTask(new Continuation>() { @Override public Task then(Task task) throws Exception { - synchronized (user.mutex) { - user.setIsCurrentUser(true); - user.synchronizeAllAuthData(); - } - return store.setAsync(user).continueWith(new Continuation() { @Override public Void then(Task task) throws Exception { diff --git a/Parse/src/main/java/com/parse/ParseAuthenticationManager.java b/Parse/src/main/java/com/parse/ParseAuthenticationManager.java index 45e602e70..0f1625e7e 100644 --- a/Parse/src/main/java/com/parse/ParseAuthenticationManager.java +++ b/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 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() { + controller.getAsync(false).onSuccessTask(new Continuation>() { @Override - public Void then(Task task) throws Exception { + public Task then(Task 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 authData) { + public Task restoreAuthenticationAsync(String authType, Map 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 deauthenticateAsync(String authType) { diff --git a/Parse/src/main/java/com/parse/ParseAuthenticationProvider.java b/Parse/src/main/java/com/parse/ParseAuthenticationProvider.java index 2845ca1f0..1be1cde56 100644 --- a/Parse/src/main/java/com/parse/ParseAuthenticationProvider.java +++ b/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> 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 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 authData); + Task restoreAuthenticationAsync(Map authData); } diff --git a/Parse/src/main/java/com/parse/ParseUser.java b/Parse/src/main/java/com/parse/ParseUser.java index d80c3373e..651a1c3fb 100644 --- a/Parse/src/main/java/com/parse/ParseUser.java +++ b/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 cleanUpAuthDataAsync() { ParseAuthenticationManager controller = getAuthenticationManager(); + Map> authData; synchronized (mutex) { - Map> 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>> i = authData.entrySet().iterator(); - while (i.hasNext()) { - Map.Entry> entry = i.next(); - if (entry.getValue() == null) { - i.remove(); - controller.restoreAuthentication(entry.getKey(), null); - } + List> tasks = new ArrayList<>(); + Iterator>> i = authData.entrySet().iterator(); + while (i.hasNext()) { + Map.Entry> 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 anonymousData) { task = super.saveAsync(sessionToken, toAwait); } - return task.onSuccessTask(new Continuation>() { - @Override - public Task then(Task 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>() { + @Override + public Task then(Task task) throws Exception { + return cleanUpAuthDataAsync(); + } + }).onSuccessTask(new Continuation>() { + @Override + public Task then(Task 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 */ Task fetchAsync( String sessionToken, Task 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. fetchAsync(sessionToken, toAwait).onSuccessTask(new Continuation>() { + Task task = super.fetchAsync(sessionToken, toAwait); + + if (isCurrentUser()) { + return task.onSuccessTask(new Continuation>() { @Override - public Task then(final Task fetchAsyncTask) throws Exception { - if (isCurrentUser()) { - cleanUpAuthData(); - return saveCurrentUserAsync(ParseUser.this).continueWithTask(new Continuation>() { - @Override - public Task then(Task task) throws Exception { - return fetchAsyncTask; - } - }); - } - return fetchAsyncTask; + public Task then(final Task fetchAsyncTask) throws Exception { + return cleanUpAuthDataAsync(); + } + }).onSuccessTask(new Continuation>() { + @Override + public Task then(Task task) throws Exception { + return saveCurrentUserAsync(ParseUser.this); + } + }).onSuccess(new Continuation() { + @Override + public T then(Task task) throws Exception { + return (T) ParseUser.this; } }); } + + return task; } /** @@ -676,7 +690,8 @@ public Task then(Task task) throws Exception { @Override public Task then(final Task signUpTask) throws Exception { ParseUser.State result = signUpTask.getResult(); - return handleSaveResultAsync(result, operations).continueWithTask(new Continuation>() { + return handleSaveResultAsync(result, + operations).continueWithTask(new Continuation>() { @Override public Task then(Task 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 synchronizeAllAuthDataAsync() { + Map> authData; synchronized (mutex) { if (!isCurrentUser()) { - return; - } - boolean success = getAuthenticationManager() - .restoreAuthentication(authType, getAuthData(authType)); - if (!success) { - unlinkFromAsync(authType); + return Task.forResult(null); } + authData = getAuthData(); } + List> 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 synchronizeAuthDataAsync(String authType) { + Map authData; synchronized (mutex) { - for (Map.Entry> entry : getAuthData().entrySet()) { - synchronizeAuthData(entry.getKey()); + if (!isCurrentUser()) { + return Task.forResult(null); } + authData = getAuthData(authType); } + return synchronizeAuthDataAsync(getAuthenticationManager(), authType, authData); + } + + private Task synchronizeAuthDataAsync( + ParseAuthenticationManager manager, final String authType, Map authData) { + return manager.restoreAuthenticationAsync(authType, authData).onSuccessTask(new Continuation>() { + @Override + public Task then(Task task) throws Exception { + boolean success = task.getResult(); + if (!success) { + return unlinkFromAsync(authType); + } + return task.makeVoid(); + } + }); } /* package */ Task unlinkFromAsync(final String authType) { @@ -1236,8 +1270,7 @@ public Task then(Task task) throws Exception { restoreAnonymity(oldAnonymousData); return task; } - synchronizeAuthData(authType); - return task; + return synchronizeAuthDataAsync(authType); } } }); diff --git a/Parse/src/test/java/com/parse/ParseAuthenticationManagerTest.java b/Parse/src/test/java/com/parse/ParseAuthenticationManagerTest.java index cb08014d9..eda9b1a96 100644 --- a/Parse/src/test/java/com/parse/ParseAuthenticationManagerTest.java +++ b/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.forResult(null)); + when(provider.restoreAuthenticationAsync(Matchers.>any())) + .thenReturn(Task.forResult(true)); manager.register(provider); Map 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 diff --git a/Parse/src/test/java/com/parse/ParseUserTest.java b/Parse/src/test/java/com/parse/ParseUserTest.java index 762a1da06..e115b3b40 100644 --- a/Parse/src/test/java/com/parse/ParseUserTest.java +++ b/Parse/src/test/java/com/parse/ParseUserTest.java @@ -567,7 +567,8 @@ public void testLinkWithAsyncWithSaveAsyncSuccess() throws Exception { // Register a mock authenticationProvider ParseAuthenticationProvider provider = mock(ParseAuthenticationProvider.class); when(provider.getAuthType()).thenReturn("facebook"); - when(provider.restoreAuthentication(Matchers.>any())).thenReturn(true); + when(provider.restoreAuthenticationAsync(Matchers.>any())) + .thenReturn(Task.forResult(true)); ParseUser.registerAuthenticationProvider(provider); ParseUser user = new ParseUser(); @@ -596,7 +597,7 @@ public void testLinkWithAsyncWithSaveAsyncSuccess() throws Exception { verify(partialMockUser, times(1)) .saveAsync(eq("sessionTokenAgain"), eq(false), Matchers.>any()); // Make sure synchronizeAuthData() is called - verify(provider, times(1)).restoreAuthentication(authData); + verify(provider, times(1)).restoreAuthenticationAsync(authData); } @Test @@ -1287,7 +1288,8 @@ public void testSynchronizeAuthData() throws Exception { // Register a mock authenticationProvider ParseAuthenticationProvider provider = mock(ParseAuthenticationProvider.class); when(provider.getAuthType()).thenReturn("facebook"); - when(provider.restoreAuthentication(Matchers.>any())).thenReturn(true); + when(provider.restoreAuthenticationAsync(Matchers.>any())) + .thenReturn(Task.forResult(true)); ParseUser.registerAuthenticationProvider(provider); // Set user initial state @@ -1300,10 +1302,10 @@ public void testSynchronizeAuthData() throws Exception { ParseUser user = ParseObject.from(userState); user.setIsCurrentUser(true); - user.synchronizeAuthData(authType); + ParseTaskUtils.wait(user.synchronizeAuthDataAsync(authType)); // Make sure we restore authentication - verify(provider, times(1)).restoreAuthentication(authData); + verify(provider, times(1)).restoreAuthenticationAsync(authData); } @Test @@ -1316,7 +1318,8 @@ public void testSynchronizeAllAuthData() throws Exception { // Register a mock authenticationProvider ParseAuthenticationProvider provider = mock(ParseAuthenticationProvider.class); when(provider.getAuthType()).thenReturn("facebook"); - when(provider.restoreAuthentication(Matchers.>any())).thenReturn(true); + when(provider.restoreAuthenticationAsync(Matchers.>any())) + .thenReturn(Task.forResult(true)); ParseUser.registerAuthenticationProvider(provider); // Set user initial state @@ -1329,10 +1332,10 @@ public void testSynchronizeAllAuthData() throws Exception { ParseUser user = ParseObject.from(userState); user.setIsCurrentUser(true); - user.synchronizeAllAuthData(); + ParseTaskUtils.wait(user.synchronizeAllAuthDataAsync()); // Make sure we restore authentication - verify(provider, times(1)).restoreAuthentication(facebookAuthData); + verify(provider, times(1)).restoreAuthenticationAsync(facebookAuthData); } //endregion