12 decrease auto counter in database when user fails to sign up#45
Open
Alishah634 wants to merge 9 commits intomasterfrom
Open
12 decrease auto counter in database when user fails to sign up#45Alishah634 wants to merge 9 commits intomasterfrom
Alishah634 wants to merge 9 commits intomasterfrom
Conversation
… WIP. Not a working commit
…abase, hence solving the auto increment ID issue
… db and incrementing the id counter.
… db and incrementing the id counter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
If this PR addresses an existing issue, link to it here._
This resolves issue #12.
The user ID is meant to be incremented when a user is correctly added to the database. The problem occurs when duplicate unique user information is added and the id is incremented despite the failure. This can lead to incorrect id assignment, and can unnecessarily use IDs because while the duplicate user is not added to the database that user was still assigned a user ID.
Usage
The usage does not change.
Changes
The email of the user is validated before starting a database transaction, returning the failure if any. This method prevents the id from being incremented as the transaction has not started.
Non-trivial Files
mysql.goAddUser()function inmysql.goto use database transactions.Testing
Dependencies
N/A
Documentation Changes
Issues and Bugs
During development:
If any kind of transaction is started the user id is incremented despite rolling the transaction back.
Additionally, not in the scope of this PR, identified re-using the open IDs is not functioning as expected and needs to be rectified.
Possible Solutions
At the moment we make a table for the newly made open IDs and store them in there, which can take up a lot of extra memory.
Instead:
If we track the largest valid ID, then if an ID anywhere else is no longer attached to user(i.e. a user deletes their account) then the largest ID user takes that deleted users ID, and the second largest user ID becomes the largest Id.
This way we avoid using a whole table for IDs, replacing it with one variable. This additionally solves re-using the open ids which do not currently work as intended.
Visualization of the idea:
So if the IDs are [0,1,2,3,4,5,6,7,8,9]
Lets track the largest valid ID i.e 9.
Now if the user with the ID 3 deletes its acc, then user 9's id becomes 3, and 8 becomes the largest Id,
[0,1,2,3,4,5,6,7,8,9] -> [0,1,2,3,4,5,6,7,8]; user 3 was deleted, 9 took over user 3 id, user 8 is now the largest ID.
Additional Notes
With the new commit the code coverage has dropped, as specific errors that do no need to be tested are not covered.
Note, this is a few commits behind master and will need to be merge/rebased correctly after approval.
Important note/request:
One of the most important parts of this commit is that test cases need to be setup correctly for TestAddUser(), this function needs to be scrutinized in depth by reviewers. If the test cases are set up incorrectly, specifically the ids for each user then it will be a bug that affects future progress.