SQL Client Access: Shared database handling between client and server

[ZenBre4ker]

To be able to access the sql database freely we are making a shared interface, which handles instatiating a database, getting and converting values as well as changing them.

TODO:

• [x] Implement serverside registering database and create them
• [x] Implement functionality to synchronize data between client and server
• [x] Implement requesting values
• [x] Handle different players requesting values
• [x] Implement conversion of values (LOCALLY for now!!!)
• [x] Implement changing values
• [x] Implement OnChange Callbacks
• [x] Document Functions
• [x] Add GetValue-Function on Server
• [x] Add Drop SQL table as Reset function
• [x] Add Local Per Item defaults
• [x] Implement it as a test case in one instance (maybe playermodels)
• [x] Test it!!!

Discussion:

1. In a next step I need to make sure, that values are properly converted. That could be my chance to implement this directly in the ORM-module or handle this locally for now and change it later. Depending what @Histalek thinks if he wants to tackle it himself or if I should just implement it in the ORM, as I guess @saibotk wont let me do another local conversion again 😅
2. Also I wonder, SQL is quite sensitive with strings as commands in combination with user given keys, if I should prohibit read-access to everyone and only allow superadmins to access the library, even if I will try to make it as safe as possible without direct access to it, but do we still want to? I am unexperienced in this matter so I can only grasp what danger this could be.

[ZenBre4ker]

Okay, Ive drafted nearly the complete database library. It is still untested and not implemented anywhere. But in general that is the structure I would want to use 😄

[TimGoll]

About your first point:

In a next step I need to make sure, that values are properly converted. That could be my chance to implement this directly in the ORM-module or handle this locally for now and change it later. Depending what @Histalek thinks if he wants to tackle it himself or if I should just implement it in the ORM, as I guess @saibotk wont let me do another local conversion again

In my opinion the datatype conversion should happen in the ORM system. Or do we always want to use your new system instead of using ORM directly?

Regarding your second point:

Also I wonder, SQL is quite sensitive with strings as commands in combination with user given keys, if I should prohibit read-access to everyone and only allow superadmins to access the library, even if I will try to make it as safe as possible without direct access to it, but do we still want to? I am unexperienced in this matter so I can only grasp what danger this could be.

If you allow users to create their own read SQL commands then this would be really bad. But since you create them on the server side and therefore only provide an interface to read it without users being able to write their own SQL, I don't see a problem here.

[saibotk]

I think we should just schedule a little meeting, since i think we are developing mutliple syncing systems in TTT2 now (ttt2net) which all try to do roughly the same or could actually be combined.

[ZenBre4ker]

Implementing conversion in orm would be my favorite. And I want to use the database system for everything that needs a sync between client and server as we still use manual systems for that especially in the shopeditor.

I found the serverConVars really comfortable to use and wanted to extend it to the sql databases. Also I like it to sync only when absolutely necessary. With our f1 menu its mostly about visualizing information. The shop also needs information about the items, but only to visualize as well.

So yeah probably the syncing aspect could be combined with ttt2net later on 😄

[ZenBre4ker]

As this is quite huge, take your time to discuss this and give me feedback in your review. ❤️ Some things I want to point out inbefore:

• The F1 Menu playermodel code got bigger, but that is only because it is in the "front-end" programmers side. One goal of this module is to later move this code directly into the element creation, so that you have similar to the serverConvars the possibility to just say, database = name, item = itemName, key = keyName and the rest is handled for you
• I only implemented it for the playermodels, which is a not so important module and therefore can be tested with a bigger audience, without disturbing live-servers too much, should be more easily to hotfix if we dont find all bugs -I am not sure if I like the official callback structure I mimicked of convars with (cvarName, oldValue, newValue) but i wanted to stick to a known structure
• For now I kept the conversion inside this module, wont be too hard to move it into orm or something like that later on, but as we have to send everything over the network anyways, having a converter on hand should be nice.
• Another goal is to centralize all our sql-activities over this module to be able to easily sync between client and server and set values over the F1-Menu, for example the item-values like AutoSpawnable or any value you want to change, that is normally saved in the weapons table on loading

In general I tried to make the module itself as modular as useful and if we want to move the syncing or conversion to another module that should be easily doable later, first I want to get the kinks out though and see if this is working as intended.

So future TODOs are:

• [ ] Use Module for all SQL-Access (e.g. items)
• [ ] Implement Database-Access into all elements like slider, checkboxes and dropdowns similar to serverConVars
• [ ] Maybe fuse ttt2net, cvars and database sync into one module
• [ ] Make better net.Write stuff similar to the streamTable method, that doesnt care about size

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ZenBre4ker]

Bump. Waiting for reviews.

[ZenBre4ker]

I just wanted to nicely remember, if we merge this, I'd start to implement proper database handling for sliders, checkboxes and so on. Could also need this for my own addon, so I can easily use sql database without having to write a lot of code 😄

Also this feature doesnt go deep into any structures, so errors would only affect the playermodels and should be easily debugged as I couldnt find anything anymore in my playtests.

[saibotk]

I remember, that we wanted to talk about this first, since there are some points in the general idea of this PR that i disagree with being useful. From my current overview, i do not really see a good point in implementing another synchronization method, especially when handling rather complex processes as managing databases. There are many pain points like migrations etc that a developer should decide on. Other than that, we already can easily sync data to players and database handling is also pretty easy. That is why i think we need to talk about this rather than going back and forth :)

[ZenBre4ker]

Well I thought I made my points clear, that this is not about syncing methods (ttt2net) or database handling (orm/migrations) in general, which we already have, its a simple tool to easily have acces from the client to the server database for the F1 Menu combining all features into accesible functions.

And as there is currently not much happening I would love to keep going in implementing more useful stuff for F1 Menu making access for database features easy. As those functions are modular and the internal structure can be changed, if somebody really wants to improve it, they can implement other syncing methods or add migrations later on. Right now, I only need it for client access to server database. And thought a generalized approach can be more useful later on ^^

[ZenBre4ker]

Ah yeah and I also need some form of callback functions similar to convars, so that database changes can be instantly shown on the client. And as there is no way beside hooking to sql function I though combining them in one centralized place helps. This is mainly meant for internal stuff in ttt2.

[ZenBre4ker]

After discussion:

• [x] Check Security Concerns (SQL Read-, Write-Access)
• [x] Make it possible to limit acces to database (e.g. admin-only)

[ZenBre4ker]

For the accessLevel restriction you can give read access to any, admins or only server in case you want to use the library on the server too, but dont need client access. Whereas write access is hardcoded to be limited to at minimum admin or higher, such that careless programmmers cant give full access to anything.

What security concerns, I used the ORM-Model and use the functions: orm.Make, :Find, :All, :New, :Save, :Delete and the access-operator [ ] So as long as access per itemname is protected via orm, my wrapper should be secure as well. You cant access any other sql table beside the ones registered by the server, which should limit abuse.

[TimGoll]

@saibotk just a ping for visibility

[ZenBre4ker]

Okay, this should be it @saibotk. Now registeredPlayers cant be explloited, you only get the information of database you are registered to.

[ZenBre4ker]

Okay, it's tested and works. Thanks for your input ❤️ gonna merge it now.