IndexedDB: prepare matrix-sdk-indexeddb crate for updating indexed_db_futures to latest version

[mgoldenberg]

Background

This pull request makes the preliminary changes necessary to update indexed_db_futures in the matrix-sdk-indexeddb crate. The reason we'd like to update this dependency is because the version currently used does not fully support the Chrome browser (see #5420).

The latest version of indexed_db_futures has significant changes. Many of these changes can be integrated without issue. There is, however, a single change which requires substantive modification to the matrix-sdk-indexeddb crate. Namely, one cannot access the active transaction in the callback to update the database (for details, see Alorel/rust-indexed-db#66).

This is only an issue in the crypto_store module, where migrations are provided access to the active transaction (see here). Furthermore, there is only a single migration (v11_to_v12) in the crypto_store module which makes use of the active transaction (see here).

For clarity, the reason v11_to_v12 is problematic in the latest versions of indexed_db_futures is because it is simply adding an index to an object store which was created in a different migration and this requires access to the active transaction. All the other migrations create object stores and indices in the same migration, which does not suffer from the same issue.

Changes

Due to the issues above, I have taken the following steps to prepare matrix-sdk-indexeddb for an update to indexed_db_futures.

1. Modified the schema upgrade logic so that none of the individual migrations expect access to the active transaction
2. Removed the logic from the v11_to_v12 migration, which could no longer be accomplished without the active transaction
3. Added a new set of migrations, v13_to_v15, which replicate the logic from v11_to_v12 in a way that is compatible with later versions of indexed_db_futures.

Alternatives

My guess is that this problem is an oversight in the interface developed in the indexed_db_futures crate and not a deeper issue - i.e., it seems it would be possible to expose the active transaction in later versions, as is the case in the current version used in matrix-sdk-indexeddb.

Alternatives to the changes in this pull request include the following.

1. Wait for the maintainer of indexed_db_futures to make the necessary modifications in a future version
2. Vendor a version with the necessary modifications specifically for matrix-sdk-indexeddb

Neither of these seemed particularly expedient or attractive, so I opted for making changes to the migrations.

Future Work

• Update indexed_db_futures to the latest version - i.e., 0.6.4

---

• Public API changes documented in changelogs (optional)

Signed-off-by: Michael Goldenberg m@mgoldenberg.net

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.89%. Comparing base (8091094) to head (0d5048d).
⚠️ Report is 628 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5467      +/-   ##
==========================================
+ Coverage   88.88%   88.89%   +0.01%     
==========================================
  Files         333      333              
  Lines       92082    92082              
  Branches    92082    92082              
==========================================
+ Hits        81850    81860      +10     
+ Misses       6403     6394       -9     
+ Partials     3829     3828       -1     

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

[codspeed-hq]

CodSpeed Performance Report

Merging #5467 will not alter performance

_{Comparing mgoldenberg:prepare-to-update-indexed-db-futures (0d5048d) with main (92192c5)}

Summary

✅ 6 untouched benchmarks

[andybalaam]

Thank you for this well-thought-through and well-explained contribution!

It's been ages since I looked at this code, so I might be misunderstanding, but here is what I think is happening:

• we are unable to pass the IdbTransaction argument to the callback in do_schema_upgrade, which means that we can't get hold of an existing object store inside a schema_add function to add an index to it.
• Therefore, we need to create a new object store and migrate across if we want to add an index to it.

Is that right?

And then you've added new version numbers that represent the creation, migration and deletion involved in creating a new object store.

If I've understood it right, then here is my response: I agree that it is inconvenient to wait for dependencies to add (reinstate!) features so I would normally be strongly in favour of working around something like this, BUT this migration will probably take a very long time for some users, and as far as I know Element Web has zero user feedback while it is going on, making for a bad user experience.

This Element Web problem is tracked here: element-hq/element-web#26892

I don't think we can add a migration like this without first seeing this issue addressed in Element Web.

Further, this change will cause a long delay for lots of users even if they are already on the latest schema version, so they already have the right index! So, it would be highly desirable to get this fixed upstream.

What do you think?

[mgoldenberg]

@andybalaam: thanks for your response! And yes, I believe you have understood correctly!

Now my turn to make sure I am understanding you: the problem is that the web-based Element client uses the crypto_store module of matrix-sdk-indexeddb - presumably as a WASM module? And that the user interface is essentially unresponsive during migrations. Finally, the migration I have added will likely take a long time for many users - depending on how much data they have in inbound_group_sessions3 - and, furthermore, provide no added benefit as the schema hasn't really changed.

That's a rather tricky problem... I hadn't thought about how long the migration might take.

So, the concern with waiting for an upstream patch to indexed_db_futures seems like it will be rather open-ended. Unfortunately, the project appears to be maintained by a single person and their current status on GitHub is: "I'm probs not having a look at that issue you opened anytime soon. Hands full.". This, of course, isn't your problem to fix. It just isn't instilling a lot of confidence that we will see a change any time soon 😅

I will confer with the people at @filament-dm, as this work is being done in service of their client. Perhaps we will have to keep this in their fork until we see some movement elsewhere.

Out of curiosity, do you have any concrete sense of when element-hq/element-web#26892 might be resolved? And is this something I can help with?

Also, tagging @zzorba here, in case he wants to weigh in or has other ideas about how to move forward.

[andybalaam]

@mgoldenberg yes, I think we are understanding each other very well.

I'd really like to avoid a slow migration for every user when it is not needed. If rust-indexed-db is not being updated, it might make sense to fork it, but I guess the first step would be to open a PR against the project and see whether it gets attention.

Help with fixing element-hq/element-web#26892 would be great - it might need some input from Element Design team to decide how it should look, but we could probably get that, or merge a good-enough solution in the meantime.

BUT the more I think about this the less I like adding a delay for all users simply because of a dependency issue.

I guess part of the question is how urgent this is. Maybe we are prepared to wait a while to see whether rust-indexed-db accepts a contribution. If the only downsides are that we can't test on Chrome and Filament needs to keep this in their fork for a while, maybe we can accept those for a while?

[mgoldenberg]

Closing this in favor of #5722.