Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: failed signup attempt with anonymous ParseUser leaves it in inconsistent state #1136

Conversation

shlusiak
Copy link
Contributor

Issue Description

The key "authData" contains a Map of authentication related data. Because unfortunately estimatedData is a shallow copy of state, the object pointed to by state["authData"] and estimatedData["authData"] is the same object. Modifying the data in this map directly prevents us from being able to call revert() to revert the data back to the server state.

This is impotant e.g. when trying to revert the user after converting it from anonymous to registered fails, where anonymity has been stripped and there is no way to get it back.

Related issue: #401

Approach

This would just create another (shallow) copy of "authData", which in this case would be fine, because we are only modifying the first layer. By replacing the value of the map with a new map we ensure that the values in state and estimatedData are different and that revert() works.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 27, 2021

Thanks for opening this pull request!

  • ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

Copy link
Contributor

@azlekov azlekov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shlusiak great job again, LGTM!

@azlekov
Copy link
Contributor

azlekov commented Nov 9, 2021

@shlusiak can you check please the failing test

com.parse.ParseUserTest > testSaveAsyncWithLazyAndCurrentUser FAILED
1579
    java.lang.AssertionError at ParseUserTest.java:962

@shlusiak
Copy link
Contributor Author

@L3K0V I think I fixed the failing test. Let's see if it passes now.

@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #1136 (4bc9067) into master (1844b3e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1136      +/-   ##
============================================
- Coverage     65.31%   65.30%   -0.02%     
+ Complexity     2219     2218       -1     
============================================
  Files           122      122              
  Lines          9961     9961              
  Branches       1338     1338              
============================================
- Hits           6506     6505       -1     
  Misses         2943     2943              
- Partials        512      513       +1     
Impacted Files Coverage Δ
parse/src/main/java/com/parse/ParseUser.java 82.42% <100.00%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 301c29b...4bc9067. Read the comment docs.

@azlekov
Copy link
Contributor

azlekov commented Nov 10, 2021

@mtrezza LGTM

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good!

@mtrezza mtrezza changed the title fix: make sure to not clobber state["authData"] with values if saving of user fails fix: failed signup attempt with anonymous ParseUser leaves it in inconsistent state Nov 10, 2021
@mtrezza mtrezza merged commit ac6d9e0 into parse-community:master Nov 10, 2021
parseplatformorg pushed a commit that referenced this pull request Nov 10, 2021
## [2.0.5](2.0.4...2.0.5) (2021-11-10)

### Bug Fixes

* failed signup attempt with anonymous ParseUser leaves it in inconsistent state ([#1136](#1136)) ([ac6d9e0](ac6d9e0))
@parseplatformorg
Copy link

🎉 This change has been released in version 2.0.5

@parseplatformorg parseplatformorg added the state:released Released as stable version label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants