Database layers

[matty0ung]

I'm coming towards the end (I think!) of reworking the database layer. I've been doing the work in Brewken, and I want to know whether there is appetite (when it's finished) to backport it to Brewtarget (which I'd be happy to do). It is quite a big change, but I think it will make life easier for future DB schema changes (and I think now BeerJSON is released, there might be a fair few of those to come as it introduces a bunch of things not present in BeerXML 1.0).

The basic idea of my reworking is to move from DB-centric to object-centric code. I've pulled a lot of the model/business "logic" (eg how do we add something to a Recipe or change the order of MashSteps) up into the model classes (rather than having special functions on Database class to handle it). I've also separated out the "how do I connect to a database / do backups / etc" (which stays in database/Database.cpp) from "how do I map classes and their fields to database tables and their fields" (mostly in database/ObjectStoreTyped.cpp and database/ObjectStore.cpp). I think it makes the code smaller and easier to extend. (I'll get to test this soon when I port @mikfire's recipe versioning to Brewken.)

Anyway, wanted to get folks' thoughts...

[mikfire]

Beyond "it works", I am not really all that invested in how the database code works now. rocketman did an impressive job in the initial transition from beerXML to rdbms. I've made pretty much a hash of it since.

After pulling the XML code out of database.cpp, I had thought the split you mention would be the next most logical. The meta of starting, opening, etc. shouldn't be intermixed with "write this field to the database".

I am, in other words, very interested. I just hope I can understand what you've done. If what you did in the XML code is any indication, it will take me a lot of work to figure out how it works.

A few things to suggest. I know the DatabaseSchema may not be the best way to do it but your solution has to address the base problem. Simply expressed, we need some fairly generic way to handle the same object field having different names in the database. id is a protected name in MySQL, for example, so we cannot easily support it until we solve this problem.

Backporting the versions will be a good test, as it requires adding a new column. Try adding a new table and see how easy that is or isn't. If I had to make a choice, I would rather it be easier to add a column than a table. I would also like to keep the number of edits required to add a new column to a bare minimum. Again, TableSchema isn't the greatest but it really did make it easier.

Any changes to the database layer should make sure that upgrading still works (pull a database from 2.3.1 and see what happens). I would also be happy if it didn't break the postgresql support, or being able to move between them. Since I am likely the only one who cares about that, I will do the tests and and PR to fix it.

[matty0ung]

Thanks. Some of the ideas are similar to the XML code, but I think overall it's simpler, because we don't need recursion.

Each type of object has an ObjectStore (or actually a templated subclass thereof - eg ObjectStoreTyped, ObjectStoreTyped) that stores the in-memory cache of all such objects and reads/writes them from/to the database. Internally, the stores hold each object inside std::shared_ptr, but things are mostly cast to more useful shared or raw pointers by wrapper functions. The code in ObjectStore is entirely generic and the class specific mappings between objects and DB tables are just data - even for relational mappings.

To make this work, I changed the object model a bit so that links between objects are mostly stored as IDs. Eg Recipe has a list of Hop IDs, a list of Fermentable IDs, etc. This saves us worrying about what order things are pulled out of the database. And any time you want to turn an ID into an object, you just call something such as Hop * hop = ObjectStoreWrapper<Hop>::getByIdRaw(hopId);.

One net result of this is that we need less interaction with the database. Eg, to find all Hops that need to be shown in the inventory printout, you just pass a lambda to ObjectStoreWrapper::findAllMatching()<Hop> that filters on Hops with no parent and an inventory amount > 0. Another result is that more of the code in Recipe.cpp can be templated - so we don't have to write lots of similar functions. In 15 lines of code, we implement every version of Recipe::uses(), which allows you to ask a Recipe whether it uses (aka "is the parent Recipe for") a particular Hop, Fermentable, Instruction, Equipment, Style, etc.

Another result is that we have fewer different signals. Eg, signalObjectInserted replaces newBrewNoteSignal, newEquipmentSignal, etc without ambiguity because you know what's been inserted by the sender (ObjectStoreTyped<BrewNote>::getInstance(), ObjectStoreTyped<Equipment>::getInstance(), etc).

As far as I can tell, everything is working. I still have a small amount of stuff to implement:

• Table creation, which should be straightforward (even with different DB typename mappings to support SQLite and PostegreSQL) UPDATE: Done as of 2021-06-27
• Database upgrades UPDATE: Done as of 2021-06-30
• Database copies UPDATE: Done as of 2021-07-13
• Dealing with the BT "default data" tables, though perhaps we'll find a different way of supplying default data when we implement BeerJSON UPDATE: Done as of 2021-07-24
• Get recipe versioning working in the new system UPDATE: Done as of 2021-07-11

I'll have a think about the restrictions on database fieldnames. My instinct would be to just change the schema so it doesn't conflict with reserved words of any DB, rather than try and add another layer of abstraction in the code, but I guess it depends how many different DBs we want to support and what all their constraints are.

[matty0ung]

OK, FYI, I finally have this all working in Brewken - including porting the ancestor stuff and finishing off the BeerXML export. Am now working on a patch to merge back to Brewtarget.