Skip to content

Commit fc249c0

Browse files
grantlandgrantland
committed
Fix signUp and linkWith with an anonymous user
Fixes integration test failures caused by #52 We strip anonymity in `signUp` and `linkWith` before calling into `saveAsync` where there's a fork in logic for `isLazy()`. Since we now determine `isLazy()` based off `objectId != null && ParseAnonymousUtils.isLinked(this)`, `isLazy()` returns the incorrect value for these logic paths. The solution is to get the value of `isLazy()` before stripping anonymity and passing it through to `saveAsync()`. * Callers of resolveLazinessAsync never use it's return value, might as well make it Void * Make signUp more robust by reverting current user's password on failure if that's how it originally was * Make signUp more robust by reverting the current user's password on success
1 parent 4f93715 commit fc249c0

File tree

2 files changed

+73
-85
lines changed

2 files changed

+73
-85
lines changed

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

Lines changed: 44 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -475,26 +475,28 @@ private void restoreAnonymity(Map<String, String> anonymousData) {
475475

476476
@Override
477477
/* package */ Task<Void> saveAsync(String sessionToken, Task<Void> toAwait) {
478-
synchronized (mutex) {
479-
Task<Void> task;
480-
if (isLazy()) {
481-
task = resolveLazinessAsync(toAwait).makeVoid();
482-
} else {
483-
task = super.saveAsync(sessionToken, toAwait);
484-
}
478+
return saveAsync(sessionToken, isLazy(), toAwait);
479+
}
485480

486-
return task.onSuccessTask(new Continuation<Void, Task<Void>>() {
487-
@Override
488-
public Task<Void> then(Task<Void> task) throws Exception {
489-
// If the user is the currently logged in user, we persist all data to disk
490-
if (isCurrentUser()) {
491-
cleanUpAuthData();
492-
return saveCurrentUserAsync(ParseUser.this);
493-
}
494-
return Task.forResult(null);
495-
}
496-
});
481+
/* package for tests */ Task<Void> saveAsync(String sessionToken, boolean isLazy, Task<Void> toAwait) {
482+
Task<Void> task;
483+
if (isLazy) {
484+
task = resolveLazinessAsync(toAwait);
485+
} else {
486+
task = super.saveAsync(sessionToken, toAwait);
497487
}
488+
489+
return task.onSuccessTask(new Continuation<Void, Task<Void>>() {
490+
@Override
491+
public Task<Void> then(Task<Void> task) throws Exception {
492+
// If the user is the currently logged in user, we persist all data to disk
493+
if (isCurrentUser()) {
494+
cleanUpAuthData();
495+
return saveCurrentUserAsync(ParseUser.this);
496+
}
497+
return Task.forResult(null);
498+
}
499+
});
498500
}
499501

500502
@Override
@@ -622,6 +624,7 @@ public Task<Void> then(Task<Void> task) throws Exception {
622624
checkForChangesToMutableContainers();
623625
user.checkForChangesToMutableContainers();
624626

627+
boolean isLazy = user.isLazy();
625628
final String oldUsername = user.getUsername();
626629
final String oldPassword = user.getPassword();
627630
final Map<String, String> anonymousData = user.getAuthData(ParseAnonymousUtils.AUTH_TYPE);
@@ -631,21 +634,26 @@ public Task<Void> then(Task<Void> task) throws Exception {
631634
user.setPassword(getPassword());
632635
revert();
633636

634-
return user.saveAsync(sessionToken, toAwait).continueWithTask(new Continuation<Void, Task<Void>>() {
637+
return user.saveAsync(sessionToken, isLazy, toAwait).continueWithTask(new Continuation<Void, Task<Void>>() {
635638
@Override
636639
public Task<Void> then(Task<Void> task) throws Exception {
637640
if (task.isCancelled() || task.isFaulted()) { // Error
638641
synchronized (user.mutex) {
639642
if (oldUsername != null) {
640643
user.setUsername(oldUsername);
644+
} else {
645+
user.revert(KEY_USERNAME);
641646
}
642647
if (oldPassword != null) {
643648
user.setPassword(oldPassword);
649+
} else {
650+
user.revert(KEY_PASSWORD);
644651
}
645652
user.restoreAnonymity(anonymousData);
646653
}
647654
return task;
648655
} else { // Success
656+
user.revert(KEY_PASSWORD);
649657
revert(KEY_PASSWORD);
650658
}
651659

@@ -1165,7 +1173,7 @@ public Task<Void> then(Task<Void> task) throws Exception {
11651173
user.stripAnonymity();
11661174
user.putAuthData(authType, authData);
11671175

1168-
return user.resolveLazinessAsync(task).makeVoid();
1176+
return user.resolveLazinessAsync(task);
11691177
}
11701178
}
11711179
}).continueWithTask(new Continuation<Void, Task<ParseUser>>() {
@@ -1224,20 +1232,14 @@ private Task<Void> linkWithAsync(
12241232
final Map<String, String> authData,
12251233
final Task<Void> toAwait,
12261234
final String sessionToken) {
1227-
final Map<String, String> oldAnonymousData = getAuthData(ParseAnonymousUtils.AUTH_TYPE);
12281235
synchronized (mutex) {
1236+
final boolean isLazy = isLazy();
1237+
final Map<String, String> oldAnonymousData = getAuthData(ParseAnonymousUtils.AUTH_TYPE);
1238+
12291239
stripAnonymity();
12301240
putAuthData(authType, authData);
12311241

1232-
// TODO(grantland): Should we really be getting the current user's sessionToken?
1233-
// What if we're not the current user?
1234-
// TODO(mengyan): Delete getCurrentSessionTokenAsync or delete the inject sessionToken
1235-
return ParseUser.getCurrentSessionTokenAsync().onSuccessTask(new Continuation<String, Task<Void>>() {
1236-
@Override
1237-
public Task<Void> then(Task<String> task) throws Exception {
1238-
return saveAsync(sessionToken, toAwait);
1239-
}
1240-
}).continueWithTask(new Continuation<Void, Task<Void>>() {
1242+
return saveAsync(sessionToken, isLazy, toAwait).continueWithTask(new Continuation<Void, Task<Void>>() {
12411243
@Override
12421244
public Task<Void> then(Task<Void> task) throws Exception {
12431245
synchronized (mutex) {
@@ -1285,32 +1287,22 @@ private void logOutWith(String authType) {
12851287
* @return A {@code Task} that will resolve to the current user. If this is a SignUp it'll be this
12861288
* {@code ParseUser} instance, otherwise it'll be a new {@code ParseUser} instance.
12871289
*/
1288-
/* package for tests */ Task<ParseUser> resolveLazinessAsync(Task<Void> toAwait) {
1290+
/* package for tests */ Task<Void> resolveLazinessAsync(Task<Void> toAwait) {
12891291
synchronized (mutex) {
1290-
if (!isLazy()) {
1291-
return Task.forResult(null);
1292-
}
12931292
if (getAuthData().size() == 0) { // TODO(grantland): Could we just check isDirty(KEY_AUTH_DATA)?
12941293
// If there are no linked services, treat this as a SignUp.
1295-
return signUpAsync(toAwait).onSuccess(new Continuation<Void, ParseUser>() {
1296-
@Override
1297-
public ParseUser then(Task<Void> task) throws Exception {
1298-
synchronized (mutex) {
1299-
return ParseUser.this;
1300-
}
1301-
}
1302-
});
1294+
return signUpAsync(toAwait);
13031295
}
13041296

13051297
final ParseOperationSet operations = startSave();
13061298

13071299
// Otherwise, treat this as a SignUpOrLogIn
1308-
return toAwait.onSuccessTask(new Continuation<Void, Task<ParseUser>>() {
1300+
return toAwait.onSuccessTask(new Continuation<Void, Task<Void>>() {
13091301
@Override
1310-
public Task<ParseUser> then(Task<Void> task) throws Exception {
1311-
return getUserController().logInAsync(getState(), operations).onSuccessTask(new Continuation<ParseUser.State, Task<ParseUser>>() {
1302+
public Task<Void> then(Task<Void> task) throws Exception {
1303+
return getUserController().logInAsync(getState(), operations).onSuccessTask(new Continuation<ParseUser.State, Task<Void>>() {
13121304
@Override
1313-
public Task<ParseUser> then(Task<ParseUser.State> task) throws Exception {
1305+
public Task<Void> then(Task<ParseUser.State> task) throws Exception {
13141306
final ParseUser.State result = task.getResult();
13151307

13161308
Task<ParseUser.State> resultTask;
@@ -1319,29 +1311,25 @@ public Task<ParseUser> then(Task<ParseUser.State> task) throws Exception {
13191311
if (Parse.isLocalDatastoreEnabled() && !result.isNew()) {
13201312
resultTask = Task.forResult(result);
13211313
} else {
1322-
resultTask = handleSaveResultAsync(result, operations).onSuccess(new Continuation<Void, ParseUser.State>() {
1314+
resultTask = handleSaveResultAsync(result,
1315+
operations).onSuccess(new Continuation<Void, ParseUser.State>() {
13231316
@Override
13241317
public ParseUser.State then(Task<Void> task) throws Exception {
13251318
return result;
13261319
}
13271320
});
13281321
}
1329-
return resultTask.onSuccessTask(new Continuation<ParseUser.State, Task<ParseUser>>() {
1322+
return resultTask.onSuccessTask(new Continuation<ParseUser.State, Task<Void>>() {
13301323
@Override
1331-
public Task<ParseUser> then(Task<ParseUser.State> task) throws Exception {
1324+
public Task<Void> then(Task<ParseUser.State> task) throws Exception {
13321325
ParseUser.State result = task.getResult();
13331326
if (!result.isNew()) {
13341327
// If the result is not a new user, treat this as a fresh logIn with complete
13351328
// serverData, and switch the current user to the new user.
13361329
final ParseUser newUser = ParseObject.from(result);
1337-
return saveCurrentUserAsync(newUser).onSuccess(new Continuation<Void, ParseUser>() {
1338-
@Override
1339-
public ParseUser then(Task<Void> task) throws Exception {
1340-
return newUser;
1341-
}
1342-
});
1330+
return saveCurrentUserAsync(newUser);
13431331
}
1344-
return Task.forResult(ParseUser.this);
1332+
return task.makeVoid();
13451333
}
13461334
});
13471335
}

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

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.junit.Test;
1616
import org.junit.rules.ExpectedException;
1717
import org.junit.runner.RunWith;
18+
import org.mockito.ArgumentCaptor;
1819
import org.mockito.Matchers;
1920
import org.robolectric.RobolectricGradleTestRunner;
2021
import org.robolectric.annotation.Config;
@@ -217,10 +218,11 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUser() throws Exception {
217218
ParseUser currentUser = mock(ParseUser.class);
218219
when(currentUser.getUsername()).thenReturn("oldUserName");
219220
when(currentUser.getPassword()).thenReturn("oldPassword");
221+
when(currentUser.isLazy()).thenReturn(false);
220222
when(currentUser.isLinked(ParseAnonymousUtils.AUTH_TYPE)).thenReturn(true);
221223
when(currentUser.getSessionToken()).thenReturn("oldSessionToken");
222224
when(currentUser.getAuthData()).thenReturn(new HashMap<String, Map<String, String>>());
223-
when(currentUser.saveAsync(anyString(), Matchers.<Task<Void>>any()))
225+
when(currentUser.saveAsync(anyString(), eq(false), Matchers.<Task<Void>>any()))
224226
.thenReturn(Task.<Void>forResult(null));
225227
ParseUser.State state = new ParseUser.State.Builder()
226228
.put("oldKey", "oldValue")
@@ -248,7 +250,8 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUser() throws Exception {
248250
verify(currentUser, times(1)).setUsername("userName");
249251
verify(currentUser, times(1)).setPassword("password");
250252
// Make sure we save currentUser
251-
verify(currentUser, times(1)).saveAsync(eq("oldSessionToken"), Matchers.<Task<Void>>any());
253+
verify(currentUser, times(1))
254+
.saveAsync(eq("oldSessionToken"), eq(false), Matchers.<Task<Void>>any());
252255
// Make sure we merge currentUser with user after save
253256
assertEquals("oldValue", user.get("oldKey"));
254257
// Make sure set currentUser
@@ -262,14 +265,15 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUserSaveFailure() throws Exce
262265
Map<String, String> oldAnonymousAuthData = new HashMap<>();
263266
oldAnonymousAuthData.put("oldKey", "oldToken");
264267
currentUser.putAuthData(ParseAnonymousUtils.AUTH_TYPE, oldAnonymousAuthData);
265-
ParseUser partialMockCurrentUser = spy(currentUser);
268+
ParseUser partialMockCurrentUser = spy(currentUser); // Spy since we need mutex
266269
when(partialMockCurrentUser.getUsername()).thenReturn("oldUserName");
267270
when(partialMockCurrentUser.getPassword()).thenReturn("oldPassword");
268271
when(partialMockCurrentUser.getSessionToken()).thenReturn("oldSessionToken");
272+
when(partialMockCurrentUser.isLazy()).thenReturn(false);
269273
ParseException saveException = new ParseException(ParseException.OTHER_CAUSE, "");
270274
doReturn(Task.<Void>forError(saveException))
271275
.when(partialMockCurrentUser)
272-
.saveAsync(anyString(), Matchers.<Task<Void>>any());
276+
.saveAsync(anyString(), eq(false), Matchers.<Task<Void>>any());
273277
ParseCurrentUserController currentUserController = mock(ParseCurrentUserController.class);
274278
when(currentUserController.getAsync(anyBoolean()))
275279
.thenReturn(Task.forResult(partialMockCurrentUser));
@@ -294,7 +298,7 @@ public void testSignUpAsyncWithMergeInDiskAnonymousUserSaveFailure() throws Exce
294298
verify(partialMockCurrentUser, times(1)).copyChangesFrom(eq(user));
295299
// Make sure we save currentUser
296300
verify(partialMockCurrentUser, times(1))
297-
.saveAsync(eq("oldSessionToken"), Matchers.<Task<Void>>any());
301+
.saveAsync(eq("oldSessionToken"), eq(false), Matchers.<Task<Void>>any());
298302
// Make sure we restore old username and password after save fails
299303
verify(partialMockCurrentUser, times(1)).setUsername("oldUserName");
300304
verify(partialMockCurrentUser, times(1)).setPassword("oldPassword");
@@ -574,7 +578,7 @@ public void testLinkWithAsyncWithSaveAsyncSuccess() throws Exception {
574578
ParseUser partialMockUser = spy(user);
575579
doReturn(Task.<Void>forResult(null))
576580
.when(partialMockUser)
577-
.saveAsync(anyString(), Matchers.<Task<Void>>any());
581+
.saveAsync(anyString(), eq(false), Matchers.<Task<Void>>any());
578582
String authType = "facebook";
579583
Map<String, String> authData = new HashMap<>();
580584
authData.put("token", "test");
@@ -586,7 +590,8 @@ public void testLinkWithAsyncWithSaveAsyncSuccess() throws Exception {
586590
// Make sure new authData is added
587591
assertSame(authData, partialMockUser.getAuthData().get("facebook"));
588592
// Make sure we save the user
589-
verify(partialMockUser, times(1)).saveAsync(eq("sessionTokenAgain"), Matchers.<Task<Void>>any());
593+
verify(partialMockUser, times(1))
594+
.saveAsync(eq("sessionTokenAgain"), eq(false), Matchers.<Task<Void>>any());
590595
// Make sure synchronizeAuthData() is called
591596
verify(provider, times(1)).restoreAuthentication(authData);
592597
}
@@ -609,7 +614,7 @@ public void testLinkWithAsyncWithSaveAsyncFailure() throws Exception {
609614
Exception saveException = new Exception();
610615
doReturn(Task.<Void>forError(saveException))
611616
.when(partialMockUser)
612-
.saveAsync(anyString(), Matchers.<Task<Void>>any());
617+
.saveAsync(anyString(), eq(false), Matchers.<Task<Void>>any());
613618
String facebookAuthType = "facebook";
614619
Map<String, String> facebookAuthData = new HashMap<>();
615620
facebookAuthData.put("facebookToken", "facebookTest");
@@ -621,7 +626,8 @@ public void testLinkWithAsyncWithSaveAsyncFailure() throws Exception {
621626
// Make sure new authData is added
622627
assertSame(facebookAuthData, partialMockUser.getAuthData().get("facebook"));
623628
// Make sure we save the user
624-
verify(partialMockUser, times(1)).saveAsync(eq("sessionTokenAgain"), Matchers.<Task<Void>>any());
629+
verify(partialMockUser, times(1))
630+
.saveAsync(eq("sessionTokenAgain"), eq(false), Matchers.<Task<Void>>any());
625631
// Make sure old authData is restored
626632
assertSame(anonymousAuthData, partialMockUser.getAuthData().get(ParseAnonymousUtils.AUTH_TYPE));
627633
// Verify exception
@@ -632,13 +638,6 @@ public void testLinkWithAsyncWithSaveAsyncFailure() throws Exception {
632638

633639
//region testResolveLazinessAsync
634640

635-
@Test
636-
public void testResolveLazinessAsyncWithNotLazyUser() throws Exception {
637-
ParseUser user = new ParseUser();
638-
639-
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
640-
}
641-
642641
@Test
643642
public void testResolveLazinessAsyncWithAuthDataAndNotNewUser() throws Exception {
644643
ParseUser user = new ParseUser();
@@ -660,14 +659,16 @@ public void testResolveLazinessAsyncWithAuthDataAndNotNewUser() throws Exception
660659
.thenReturn(Task.<Void>forResult(null));
661660
ParseCorePlugins.getInstance().registerCurrentUserController(currentUserController);
662661

663-
ParseUser userAfterResolveLaziness =
664-
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
662+
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
663+
ArgumentCaptor<ParseUser> userAfterResolveLazinessCaptor =
664+
ArgumentCaptor.forClass(ParseUser.class);
665665

666666
// Make sure we logIn the lazy user
667667
verify(userController, times(1)).logInAsync(
668668
any(ParseUser.State.class), any(ParseOperationSet.class));
669669
// Make sure we save currentUser
670-
verify(currentUserController, times(1)).setAsync(any(ParseUser.class));
670+
verify(currentUserController, times(1)).setAsync(userAfterResolveLazinessCaptor.capture());
671+
ParseUser userAfterResolveLaziness = userAfterResolveLazinessCaptor.getValue();
671672
// Make sure user's data is correct
672673
assertEquals("newSessionToken", userAfterResolveLaziness.getSessionToken());
673674
assertEquals("newValue", userAfterResolveLaziness.get("newKey"));
@@ -697,21 +698,18 @@ public void testResolveLazinessAsyncWithAuthDataAndNewUser() throws Exception {
697698
ParseCurrentUserController currentUserController = mock(ParseCurrentUserController.class);
698699
ParseCorePlugins.getInstance().registerCurrentUserController(currentUserController);
699700

700-
ParseUser userAfterResolveLaziness =
701-
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
701+
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
702702

703703
// Make sure we logIn the lazy user
704704
verify(userController, times(1)).logInAsync(
705705
any(ParseUser.State.class), any(ParseOperationSet.class));
706706
// Make sure we do not save currentUser
707707
verify(currentUserController, never()).setAsync(any(ParseUser.class));
708708
// Make sure userAfterResolveLaziness's data is correct
709-
assertEquals("newSessionToken", userAfterResolveLaziness.getSessionToken());
710-
assertEquals("newValue", userAfterResolveLaziness.get("newKey"));
709+
assertEquals("newSessionToken", user.getSessionToken());
710+
assertEquals("newValue", user.get("newKey"));
711711
// Make sure userAfterResolveLaziness is not lazy
712-
assertFalse(userAfterResolveLaziness.isLazy());
713-
// Make sure we do not create new user
714-
assertSame(user, userAfterResolveLaziness);
712+
assertFalse(user.isLazy());
715713
}
716714

717715
@Test
@@ -739,8 +737,9 @@ public void testResolveLazinessAsyncWithAuthDataAndNotNewUserAndLDSEnabled() thr
739737
// Enable LDS
740738
Parse.enableLocalDatastore(null);
741739

742-
ParseUser userAfterResolveLaziness =
743-
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
740+
ParseTaskUtils.wait(user.resolveLazinessAsync(Task.<Void>forResult(null)));
741+
ArgumentCaptor<ParseUser> userAfterResolveLazinessCaptor =
742+
ArgumentCaptor.forClass(ParseUser.class);
744743

745744
// Make sure we logIn the lazy user
746745
verify(userController, times(1)).logInAsync(
@@ -749,7 +748,8 @@ public void testResolveLazinessAsyncWithAuthDataAndNotNewUserAndLDSEnabled() thr
749748
// field should be cleaned
750749
assertEquals("password", user.getPassword());
751750
// Make sure we do not save currentUser
752-
verify(currentUserController, times(1)).setAsync(any(ParseUser.class));
751+
verify(currentUserController, times(1)).setAsync(userAfterResolveLazinessCaptor.capture());
752+
ParseUser userAfterResolveLaziness = userAfterResolveLazinessCaptor.getValue();
753753
// Make sure userAfterResolveLaziness's data is correct
754754
assertEquals("newSessionToken", userAfterResolveLaziness.getSessionToken());
755755
assertEquals("newValue", userAfterResolveLaziness.get("newKey"));

0 commit comments

Comments
 (0)