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
Added validate email function in mysql.go
Refactored AddUser() function in mysql.go to use database transactions.
User tests added and updated
Testing
[X] ➕ Added units to existing tests
[ ] 🐣 Started a new group of tests
[ ] 🙅 New tests aren't needed
[ ] 🙋 Need help to add tests
Dependencies
N/A
Documentation Changes
[ ] 📜 README.md
[ ] 💬 Added necessary comments to existing code
[ ] 🙅 no documentation needed
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.
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.