IndexedDB: remove assumption that all EventCacheStore keys contain a RoomId

[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). This particular pull request focuses on removing the assumption that all EventCacheStore keys contain a RoomId. This refactoring is necessary in order to support the queries related to leases and media, which make no reference to rooms - e.g., here, here, here, etc.

Changes

No References to RoomId in Serialization Traits

The overarching change is that each of the traits in event_cache_store::serializer::traits no longer makes reference to a RoomId, which ultimately lays the foundation for keys which do not contain a RoomId. For the existing keys - all of which contain a RoomId - their components are now fully represented in IndexedKey::KeyComponents.

So, for instance, IndexedChunkIdKey previously had the following implementation.

impl IndexedKey<Chunk> for IndexedChunkIdKey {
    type KeyComponents = ChunkIdentifier;

    fn encode(
        room_id: &RoomId,
        chunk_id: &ChunkIdentifier,
        serializer: &IndexeddbSerializer,
    ) -> Self { /* -- snip -- */ }
}

But now, has an updated implementation.

impl IndexedKey<Chunk> for IndexedChunkIdKey {
    type KeyComponents<'a> = (&'a RoomId, ChunkIdentifier);


    fn encode(
        (room_id, chunk_id): Self::KeyComponents<'_>,
        serializer: &IndexeddbSerializer,
    ) -> Self { /* -- snip -- */ }
}

Additional Traits for Representing Prefix Keys

By encoding the RoomId as an element of IndexedKey::KeyComponents, the trait IndexedKeyComponentBounds, which expresses the bounds of the key components, now includes the lower and upper bounds of the RoomId as well. This becomes insufficient as the existing queries are always minimally searching within the bounds of a single room.

Consequently, I have added a trait for constructing key components bounds that allows one to hold leading components constant, while providing bounds on the remaining components. So, IndexedKeyComponentBounds is supplemented by IndexedPrefixKeyComponentBounds shown below.

pub trait IndexedKeyComponentBounds<T: Indexed>: IndexedKeyBounds<T> {
    fn lower_key_components() -> Self::KeyComponents<'static>;
    fn upper_key_components() -> Self::KeyComponents<'static>;
}

pub trait IndexedPrefixKeyComponentBounds<'a, T: Indexed, P: 'a>: IndexedKey<T> {
    fn lower_key_components_with_prefix(prefix: P) -> Self::KeyComponents<'a>;
    fn upper_key_components_with_prefix(prefix: P) -> Self::KeyComponents<'a>;
}

Let's again consider IndexedChunkIdKey. Previously, this key had an implementation of IndexedKeyComponentBounds, shown below.

impl IndexedKeyComponentBounds<Chunk> for IndexedChunkIdKey {
    fn lower_key_components() -> Self::KeyComponents {
        ChunkIdentifier::new(0)
    }

    fn upper_key_components() -> Self::KeyComponents {
        ChunkIdentifier::new(js_sys::Number::MAX_SAFE_INTEGER as u64)
    }
}

When RoomId becomes a key component, the functions above would provide us with the lower and upper bounds of keys for all chunks in all rooms.

Alternatively, a relevant implementation of IndexedPrefixKeyComponentBounds can provide us with the lower and upper bounds of keys for all chunks in a single room.

impl<'a> IndexedPrefixKeyComponentBounds<'a, Chunk, &'a RoomId> for IndexedChunkIdKey {
    fn lower_key_components_with_prefix(room_id: &'a RoomId) -> Self::KeyComponents<'a> {
        (room_id, ChunkIdentifier::new(0))
    }

    fn upper_key_components_with_prefix(room_id: &'a RoomId) -> Self::KeyComponents<'a> {
        (room_id, ChunkIdentifier::new(js_sys::Number::MAX_SAFE_INTEGER as u64))
    }
}

Propogate Changes

The remaining changes are simply propogations of the changes outlined above. These include adding and removing references to RoomId where necessary, modifying type constraints on certain generalized functions, etc.

Future Work

• Proper handling of LinkedChunkId (see here).
• Add implementation of EventCacheStore::try_take_leased_lock functions without relying on MemoryStore
• Add implementation of EventCacheStoreMedia

---

• Public API changes documented in changelogs (optional)

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.93%. Comparing base (787861e) to head (3054131).
⚠️ Report is 38 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5497   +/-   ##
=======================================
  Coverage   88.93%   88.93%           
=======================================
  Files         335      335           
  Lines       93274    93274           
  Branches    93274    93274           
=======================================
+ Hits        82949    82957    +8     
+ Misses       6462     6454    -8     
  Partials     3863     3863           

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

[codspeed-hq]

CodSpeed Performance Report

Merging #5497 will not alter performance

_{Comparing mgoldenberg:indexeddb-event-cache-store-room-id-key-component (3054131) with main (872713c)}

Summary

✅ 31 untouched benchmarks

[andybalaam]

This all sounds very sensible, and the code looks like it does what is described, but I definitely need Ivan to weigh in on the direction, since he has already been involved in this.

Thanks for the small commits! Very helpful.

[Hywan]

What can I say? PR description is clear. Commits are clear. Code is great.

Bravo 👏 !

[Hywan]

Lovely.