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

WIP: Refactor server database #6263

Draft
wants to merge 259 commits into
base: master
Choose a base branch
from

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented Nov 19, 2023

This PR completely rewrites the database handling on the server side. In so doing it cleans up and unifies the mess of the current implementation and adds proper unit testing support - in particular for data migration paths, which should provide an overall more reliable experience for users as the tests more or less guarantee that the data migration will work as intended. Additionally, since unit tests are run against all supported backends (SQLite, MySQL, PostgreSQL), all should work equally fine (at least from a functionality point of view).

As a bonus, the dependence on Qt for the database handling is completely removed. Instead, the new framework is built on SOCI.

Fixes #4980 (data can now be exported to and imported from JSON)
Fixes #4717 (all database backends are now unit-tested for dealing with unicode characters)
Fixes #3795 (all operations on the database are now encapsulated in transactions)
Fixes #3666
Fixes #3588 (we are using a completely new backend and new code - hopefully this fixes the issue)
Fixes #3254 (again: transactions should prevent database corruption from happening - unless there is a bug in the code)
Fixes #2960 (backups can now be done by explicitly exporting data to JSON - it's up to the user though. No automatic backup implemented)
Fixes #6283
Fixes #6383
Fixes #3796 (we don't hardcode timeouts, so this was likely a Qt thing - new backend, new hope :D )
Fixes #6332

Checks

@Krzmbrzl Krzmbrzl added server feature-request This issue or PR deals with a new feature labels Nov 19, 2023
@Krzmbrzl Krzmbrzl changed the title WIP: Refactor database WIP: Refactor server database Nov 19, 2023
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 5 times, most recently from a26dd78 to 28acdc8 Compare January 1, 2024 13:39
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 2 times, most recently from 2a149e9 to 825b6b1 Compare March 7, 2024 20:00
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 3 times, most recently from a0b5044 to 0cff06a Compare April 6, 2024 15:09
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 2 times, most recently from 3584ad3 to 62ac5fc Compare May 20, 2024 06:54
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 9 times, most recently from 8c00f1f to 811cfa6 Compare June 6, 2024 17:37
@Krzmbrzl Krzmbrzl force-pushed the refac-database branch 2 times, most recently from de96b46 to 5c928be Compare June 23, 2024 14:43
Each supported backend can be excluded from the build separately
The dumped JSON format may not be 100% DB backend independent but it
should be somewhat close to it.

Implements mumble-voip#4980
Implements mumble-voip#2960 (must be done manually though)
Mumble treats usernames case-insensitively. However, in the server,
there is a cache that maps usernames to IDs of registered users. We do
lazy loading so when a given name is not yet in the cache, we check the
database for a match and if that returns something, we insert the result
into the cache. However, the DB does case-insensitive lookups whereas
the cache used to be case-sensitive. Therefore, different casings of the
same username could end up in the cache (all pointing to the same user
ID). If that user got unregistered, only one of the possible ways of
writing their name gets removed from the cache, leaving the other
entries (which are now stale) untouched.

This can cause problems when another user now wants to register with a
name that corresponds to one of those stale cache entries. The server
will think that the given username is already taken, ensuring leading to
a rejection of the registration request.

A server restart purges the stale entries from the cache.

This commit ensures that the cache is now also operating in a
case-insensitive manner such that we shouldn't ever create any duplicate
entries in there in the first place.

Fixes mumble-voip#6383
The expected behavior is to return an empty string as the name. However,
the current code will run into a database error that will shut the
server down.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment