IndexedDB: handle LinkedChunkId properly in EventCacheStore implementation

[mgoldenberg]

Background

This pull request is part of a series of pull requests to add a full IndexedDB implementation of the EventCacheStore (see #4617, #4996, #5090, #5138, #5226, #5274, #5343, #5384, #5406, #5414, #5497, #5506). This particular pull request adds proper handling of the LinkedChunkId argument provided in a number of EventCacheStore functions by making modifications to key formats in a number of object stores.

Prior to #5182, many functions of EventCacheStore were provided a RoomId argument. In order to accommodate the possibility of thread-based operations, LinkedChunkId replaced many of the RoomId arguments. Then, in #5445, the Thread variant was added to LinkedChunkId and would require that implementations of EventCacheStore properly handle thread-based operations.

This issue was surfaced by @bnjbvr here.

Changes

LinkedChunkId and OwnedLinkedChunkId

For ease of use, I have made OwnedLinkedChunkId::as_ref accessible outside of [#cfg(test)] contexts, as well as adding From implementations between LinkedChunkId and OwnedLinkedChunkId. This also made it possible to consolidate the Display implementation for these two types.

Additionally, I have derived Serialize and Deserialize on OwnedLinkedChunkId in order to allow this type to be easily stored in a database, e.g., IndexedDB.

Note that these changes were discussed with @bnjbvr, though he may want to give final confirmation that this is aligned with his intention for the types.

Keys and Indices

All existing keys replace RoomId with LinkedChunkId, with one exception: IndexedEventRelationKey still uses a RoomId in its key because EventCacheStore::find_event_relations is provided with a RoomId and not a LinkedChunkId.

Due to other event-related functions also continuing to operate on RoomIds rather than LinkedChunkIds, a new index (and associated key) was added to the event object store.

• Index
  • Unique: true
  • Name: events_room
  • Key:
    • Path: room
    • Format: (RoomId, EventId)

This index (and key) are necessary to support the following operations.

• EventCacheStore::find_event - finds an event in a particular room.
• EventCacheStore::save_event - saves an event to a particular room.

Tests

Up until this pull request, matrix_sdk_base::event_cache_store_integration_tests was recreated in matrix_sdk_indexeddb::event_cache_store::integration_tests and selectively chose tests based on what was properly implemented. This has now been removed and this EventCacheStore implementation is now being tested against the integration tests in matrix_sdk_base.

Future Work

• Add implementation of EventCacheStoreMedia

---

• Public API changes documented in changelogs (optional)

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

[codecov]

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.59%. Comparing base (bb0d480) to head (03d02d7).
⚠️ Report is 16 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-common/src/linked_chunk/mod.rs	57.14%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5540      +/-   ##
==========================================
- Coverage   88.59%   88.59%   -0.01%     
==========================================
  Files         340      340              
  Lines       93850    93859       +9     
  Branches    93850    93859       +9     
==========================================
+ Hits        83147    83150       +3     
- Misses       6571     6578       +7     
+ Partials     4132     4131       -1     

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

[codspeed-hq]

CodSpeed Performance Report

Merging #5540 will not alter performance

_{Comparing mgoldenberg:indexeddb-event-cache-store-linked-chunk-id (03d02d7) with main (bb0d480)}

Summary

✅ 31 untouched benchmarks

[Hywan]

Excellent work, as always. Thanks for the small commits and all the small clean up.
I've left few feedback. Eager to merge it!

[Hywan]

Events aren't uniquely indexed by their ID?

[mgoldenberg]

You are right, they are uniquely indexed by their ID!

That being said, the IndexedDB API's function for getting a single value - i.e., IDBObjectStore.get - from an object store will return the "first record matching the given key". Consequently, I have no way of knowing whether the returned record from calling this function was the one and only record or the first of a set of records.

So, throughout the codebase, I opt for getting a set of records and then verifying that there is only one. That way, if for some reason I forgot to make a particular index unique, requests for a single value that should be unique but actually match multiple records do not go unnoticed.

Does that make sense?

[Hywan]

It does make sense, yes. Thanks for the explanation.

[bnjbvr]

Thanks, only read the description, and this all looks great!

[Hywan]

Let's rebase the fixup commits, we are good!