indexeddb: Tidy the migrations code

[andybalaam]

I found it quite hard to understand the Indexed DB schema migration code, so I did my best to tidy it.

Definitely review this commit by commit. Each commit is supposed to be quite trivial, but together they add up to code that I am able to understand much better.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ae97772) 83.73% compared to head (78e3350) 83.73%.
Report is 18 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3095   +/-   ##
=======================================
  Coverage   83.73%   83.73%           
=======================================
  Files         224      224           
  Lines       23536    23536           
=======================================
  Hits        19708    19708           
  Misses       3828     3828           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Hywan]

I admire the effort to clean this tech debt. Thank you very much. The new code structure and new API is so much easier. Thanks!

[richvdh]

looks great to me, thank you for doing this!

One thing is that there is now an awful lot of opening and closing the DB going on, even in the simple case that there is no upgrade to be done, but maybe that's a quick enough operation, and there is no point optimising it prematurely.

[richvdh]

it might be worth documenting that this function will create the db (at version 1) if it does not already exist.

[andybalaam]

One thing is that there is now an awful lot of opening and closing the DB going on, even in the simple case that there is no upgrade to be done, but maybe that's a quick enough operation, and there is no point optimising it prematurely.

If there's no upgrade don't we only open it twice (to find the version, then to actually open it)?

If the DB is empty, we open and close a lot, but I haven't seen any performance problem from that so hopefully it's fine.

I'm not sure there is a good solution if we did find it is a problem in general, because need multiple separate schema migrations to happen, and the data migrations need to happen in between each one. We could special-case the empty DB case, but we'd only want to do that if we were convinced it caused a problem.

[richvdh]

One thing is that there is now an awful lot of opening and closing the DB going on, even in the simple case that there is no upgrade to be done, but maybe that's a quick enough operation, and there is no point optimising it prematurely.

If there's no upgrade don't we only open it twice (to find the version, then to actually open it)?

If the DB is empty, we open and close a lot, but I haven't seen any performance problem from that so hopefully it's fine.

True. It's not as bad as I thought it was going to be.

I'm not sure there is a good solution if we did find it is a problem in general, because need multiple separate schema migrations to happen, and the data migrations need to happen in between each one. We could special-case the empty DB case, but we'd only want to do that if we were convinced it caused a problem.

I think we could approximately halve the number of opens by re-using db handles. But in any case, you're right that its an unnecessary optimisation.

[andybalaam]

I think we could approximately halve the number of opens by re-using db handles. But in any case, you're right that its an unnecessary optimisation.

Ah right, by doing the data migration using the same DB handle as the schema upgrade we just did.

But yeah, probably no need.