IndexedDB: use implementations of EventCacheStore and MediaStore in client

[mgoldenberg]

Background

This is the final pull request in the series of pull requests to add a full IndexedDB implementation of the EventCacheStore and MediaStore (see #4617, #4996, #5090, #5138, #5226, #5274, #5343, #5384, #5406, #5414, #5497, #5506, #5540, #5574, #5603, #5676, #5682, #5749, #5795, #5933). This particular pull request adds the IndexedDB implementations of EventCacheStore and MediaStore into the matrix_sdk::Client when it is configured to use IndexedDB as its data store.

Changes

Expose EventCacheStore and MediaStore functions outside matrix-sdk-indexeddb

This involved re-exporting some of the types internal to the crate - e.g., IndexeddbEventCacheStore and IndexeddbMediaStore.

Additionally, a composite type was added to hold all stores in a single structure - i.e., IndexeddbStores. This structure is also equipped with a set of functions for opening all the stores with a single function call.

Refactor feature flags

There were some issues with enabling individual feature flags - e.g., enabling event-cache-store or media-store without e2e-encryption failed to compile. These issues surfaced once trying to use them via matrix-sdk.

The flags have been refactored so that each of the different stores may be used independent of and in combination with the others.

Use implementations of EventCacheStore and MediaStore in matrix-sdk::Client

matrix_sdk::client::builder::build_indexeddb_store_config has been updated so that it initializes the StoreConfig with IndexedDB implementations of all of the stores. And, of course, it includes the crypto store when e2e-encryption is enabled.

---

• Public API changes documented in changelogs (optional)

Signed-off-by: Michael Goldenberg [email protected]

[codspeed-hq]

CodSpeed Performance Report

Merging #5946 will not alter performance

_{Comparing mgoldenberg:expose-event-cache-and-media-store (d754333) with main (f2ba338)}

Summary

✅ 50 untouched

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.50%. Comparing base (acf3a7a) to head (d754333).
⚠️ Report is 27 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5946      +/-   ##
==========================================
- Coverage   88.50%   88.50%   -0.01%     
==========================================
  Files         362      362              
  Lines      103351   103351              
  Branches   103351   103351              
==========================================
- Hits        91474    91470       -4     
- Misses       7531     7534       +3     
- Partials     4346     4347       +1     

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

[Hywan]

Thanks for the patches!

I wonder why we would need a struct holding all the stores. We don't have that for SQLite nor in-memory stores. Why is it important here?

[Hywan]

What safe means here?

[mgoldenberg]

safe is part of the name of an encoder and serializer - i.e., trait SafeEncode.

I agree that this isn't a good name for a feature, but it helps a lot to have more granular feature flags when adding [#cfg(feature = ...)] statements in the code. I thought about prefixing it with _ or internal- to show that it really isn't for an end user, but for internal organization.

[mgoldenberg]

Open to other directions with this!

[Hywan]

Nop, those are a sublist of the previous item.

[mgoldenberg]

Oh... these must be formatting changes... would you like me to try to revert them? I didn't realize those were in there!

[Hywan]

Nop.

[mgoldenberg]

Same as above!

[mgoldenberg]

I wonder why we would need a struct holding all the stores. We don't have that for SQLite nor in-memory stores. Why is it important here?

I was mostly just trying to stick to the pattern that was already established in matrix-sdk-indexeddb::open_stores_with_name. I don't know that it is super important, but my guess is that the original author was trying to help ensure that all the names of all the databases would have the same prefix.

I am happy to re-think this and, instead, expose functions that enable users of the crate to construct each store individually. However, there's one consideration to note about going that route.

The original author initialized a single StoreCipher through the IndexeddbStateStore and passed this around to subsequent stores.

pub async fn open_stores_with_name(
    name: &str,
    passphrase: Option<&str>,
) -> Result<(IndexeddbStateStore, IndexeddbCryptoStore), OpenStoreError> {
    // -- snip --

    let crypto_store =
        IndexeddbCryptoStore::open_with_store_cipher(name, state_store.store_cipher.clone())
            .await?;

    // -- snip --
}

I'm not sure why this was done, but it requires access to IndexeddbStateStore::store_cipher, which is currently pub(crate).

In order to allow users of the crate to construct each store individually, we would have to do one of two things.

1. Make IndexeddbStateStore::store_cipher accessible in some way to users of the crate
2. Add functionality to each store so that it can initialize its own StoreCipher from a passphrase.
   • I believe matrix-sdk-sqlite does something like this

Let me know what you think, happy to go in any direction! And also happy to explain more if this isn't making sense.