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

Fix various Signal/DB/XML import issues #629

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

matty0ung
Copy link
Contributor

@matty0ung matty0ung commented Dec 12, 2021

See #628

A couple of changes here, fixing things from new DB layer and from XML import code.

  • Firstly, there was a signals bug where if we have more than one instance of HopTableModel (or MashStepTableModel or similar) then each table model was getting and acting on signals for all table models of the same type - in particular when a new Hop/MashStep/etc is created. I think this is because I changed the source of the signals when I created ObjectStore. Anyway, fix is to add filtering so that when a message is received about a new Hop/MashStep/etc being created, we first check whether it belongs to the Recipe/Mash we're watching.
  • Secondly when we were creating new MashSteps, there was an inconsistency about where they get added to the Mash and where they get stored in the DB. I've changed callers to use Mash::addMashStep (which already had pretty much the right logic).
  • Still on MashSteps, they were being put in reverse order when imported from XML. (This was because, if you put a bunch of things in a QMultiHash with the same key, then iterating over them gives you most-recently-added first. Changing to a QVector gives an iteration order that goes from first-added to last-added.)
  • I also found an d'oh bug in the XML code where I was being an idiot with std::shared_ptr. Result was a crash if you import the same XML file twice. Now that we have the new ObjectStore stuff, we don't have to use raw pointers so much in the XML code so it's easier to be consistent. (Essentially the issue is that, if you have a shared_ptr to something and you want to make an additional shared_ptr to it, you must copy the new shared_ptr from the existing one -- so that they'll share reference counts etc. If you make the new one from the raw pointer then you'll have two separate completely unrelated shared_ptrs each thinking they own the same object and it will get deleted before you expect resulting in a crash.
  • I had a one-line fix for BT 2.3.1: BeerXML output does not contain estimated IBU value #452 (inclusion of IBU in BeerXML export) coded in Brewken which I've brought across because I was editing that file anyway.
  • Finally, there was a bug when deleting a Recipe that you would get a foreign key violation in the DB. This is down to trying to delete an about-to-be-unused unnamed Mash before the Recipe being deleted instead of afterwards. New function Recipe::hardDeleteOrphanedEntities() now ensures things get deleted in the correct order.

As an aside it feels like there is a bunch of common code that could be pulled out of the FooTableModel classes, but that's for another time. (I've started doing a little bit of this in the separate branch I'm working on for BeerJSON, but only the very low-hanging fruit so far!)

@matty0ung matty0ung changed the title Fix various XML import issues Fix various Signal/DB/XML import issues Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants