Fixed: ParseUser.getCurrentUser() returns a non authenticated user when local datastore is used

[lukas1994]

Repro project: https://www.dropbox.com/s/m6vl215mg3nj8uo/Parse-Starter-Project-1.9.4.zip
1- change keys
2- click SIGN UP
-> make sure a user & a post are created
3- click GETCURRENTUSER
-> make sure user and session are shown
4- click ISAUTHENTICATED()
-> make sure it returns true
5- now click QUERY DATA WITH INCLUDE("USER")
6- repeat step 3 and 4 and notice the issue

[grantland]

I'm not convinced that this is where the actual problem lies, since we shouldn't be overwriting sessionToken when merging from LDS when the JSON isn't complete. Could you add some unit tests that reproduces the problem so I could see more into why this problem occurs?

[lukas1994]

If the server returns the query that contains a reference to the currentUser the following happens:
The ParseDecoder calls ParseObject.fromJSON() with isComplete == true.
Then ParseObject.createWithoutData() gets called.
If the local datastore is enabled it gets currentUser from the store (by object id).
Finally, we call currentUser.mergeFromServer() with isComplete == true which overwrites currentUser with the data from the server (which doesn't contain the sessionToken, ...).

There are different ways to fix this. We need to add a special case somewhere: ParseUser is the best place I guess. Another fix would be to set isComplete == false appropriately.

What do you think?

[lukas1994]

One interesting thing to note is that the server normally returns the sessionToken if you query for users but if you query for posts that contain a pointer to a user then it doesn't return the sessionToken.

[grantland]

If that's the case, it seems like this current modification will have some unintended side affects such as stale data such as keys that were removed on the server persisting after a fetch.

We'd probably want to have a more precise fix that is conditional upon the ParseUser instancing being current and only persisting the sessionToken locally if there was no sessionToken in the new state.

Additionally, this should be verified with actual code that hits the server, but the PR should contain a unit test that prevents this from happening in the future.

[grantland]

For future PRs, we use /* package */ to designate package protected methods.

[grantland]

This is looking much better! Just an optimization and a few minor nits.

[grantland]

This logic seems a tad messy, could we simplify it to the following?

if (/* ... */) {
  newState = // ...
}
super.setState(newState);

[grantland]

Sorry, I realized that there are some additional use cases that we should cover in the tests.

[grantland]

Looks awesome, thanks!

Also same comments RE commits, etc. as I made on your other diff.

[facebook-github-bot]

@lukas1994 updated the pull request.