IndexedDB: Add implementation of MediaRetentionPolicy-related functions in EventCacheStoreMedia

[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, #5540). This particular pull request adds a MemoryStore-backed implementation of EventCacheStoreMedia, as well as, IndexedDB-backed implementations for MediaRetentionPolicy-related functions.

Changes

The overarching change adds a MemoryStore-backed implementation of EventCacheStoreMedia for IndexeddbEventCacheStore. Furthermore, the implementation of EventCacheStore now delegates to the implementation of EventCacheStoreMedia via a MediaService nested in IndexeddbEventCacheStore.

Additionally, there are now implementations and tests for the following EventCacheStoreMedia functions.

• EventCacheStoreMedia::media_retention_policy_inner
  • Supported by IndexeddbEventCacheStoreTransaction::get_media_retention_policy
• EventCacheStoreMedia::set_media_retention_policy_inner

And finally, to support these queries, the CORE object store now has a primary key with key path id.

Caveats

EventCacheStoreMedia requires that the implementing type be Cloneable. Consequently, some rather minimal changes were required in order to support Clone.

There is, however, one notable change for supporting Clone, which is that IndexeddbEventCacheStore.inner went from being an IdbDatabase to an Rc<IdbDatabase>.

While this may seem like a problem in a concurrent environment, it is ultimately not an issue, as IndexedDB ensures that only a single transaction is running at a time. Any interfering transaction will be canceled by IndexedDB and return an error.

The downside of this architecture, however, is that it would require re-trying in a loop if multiple instances of IndexeddbEventCacheStore were contending for access.

An alternative would be to use a tokio::sync::Mutex in conjunction with an Arc, which would replace the loop with an await. The consequences of this are that the implementation of IndexeddbEventCacheStoreTransaction becomes rather tricky, as it seems to require a self-referential struct.

I'm not entirely sure why the EventCacheStoreMedia requires Clone and how it will be used, so I opted for using an Rc as it was far more straightforward and still a sound approach.

Future Work

• Add implementation of remaining EventCacheStoreMedia functions

---

• 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.63%. Comparing base (7adaf7b) to head (ba701a7).
⚠️ Report is 46 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5574      +/-   ##
==========================================
- Coverage   88.63%   88.63%   -0.01%     
==========================================
  Files         340      340              
  Lines       95102    95102              
  Branches    95102    95102              
==========================================
- Hits        84298    84296       -2     
- Misses       6616     6619       +3     
+ Partials     4188     4187       -1     

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

[codspeed-hq]

CodSpeed Performance Report

Merging #5574 will not alter performance

_{Comparing mgoldenberg:indexeddb-event-cache-store-media-retention-policy (ba701a7) with main (85d5258)}

Summary

✅ 37 untouched benchmarks

[Hywan]

The MediaService trait implements Clone because of this:

matrix-rust-sdk/crates/matrix-sdk-base/src/event_cache/store/media/media_service.rs

Lines 319 to 328 in f4ce435

	let this = self.clone();
	let store = store.clone();
	let handle = spawn(async move {
	if let Err(error) = this.clean_up_media_cache_inner(&store, current_time).await {
	error!("Failed to run automatic media cache cleanup: {error}");
	}
	});
	*join_handle = Some(handle);

The cleanup phase is done in a separate task. The task handle is stored here:

matrix-rust-sdk/crates/matrix-sdk-base/src/event_cache/store/media/media_service.rs

Lines 53 to 57 in f4ce435

	/// The [`JoinHandle`] for an automatic media cleanup task.
	///
	/// Used to ensure that only one automatic cleanup is running at a time, and
	/// to stop the cleanup when the [`MediaServiceInner`] is dropped.
	automatic_media_cleanup_join_handle: Mutex<Option<JoinHandle<()>>>,

If it is really annoying for you, we may want to revisit this.

[Hywan]

It looks good to me, thanks!