IndexedDB: add functionality for cleaning up MediaStore

[mgoldenberg]

Background

This pull request is part of a 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). This particular pull request adds IndexedDB-backed implementations for cleaning expired and over-sized media in IndexedDB via MediaStore.

Changes

IndexedDB implementations of top-level MediaStore functions

The overarching change is the removal of the nested MemoryStore and the addition of IndexedDB-backed implementations for the following functions.

• MediaStoreInner::clean_inner
• MediaStoreInner::last_media_cleanup_time_inner
• MediaStoreInner::set_ignore_media_retention_policy

Additionally, the following types, trait implementations, and functions were added to support the functions above.

Types

• MediaCleanupTime - a basic wrapper around UnixTime for representing the last time the MediaStore was cleaned. This also includes indexed versions of this type optimized for IndexedDB storage.

Trait Implementations

• Add<Duration> and Sub<Duration> for UnixTime
  • Helps calculate expiration times, etc. for Media

Functions

• Transaction::get_keys
• Transaction::fold_keys_while
• IndexeddbMediaStoreTransaction::get_media_cleanup_time
• IndexeddbMediaStoreTransaction::put_media_cleanup_time
• IndexeddbMediaStoreTransaction::get_media_keys_by_content_size
• IndexeddbMediaStoreTransaction::fold_media_keys_by_retention_metadata_while
• IndexeddbMediaStoreTransaction::get_cache_size
• IndexeddbMediaStoreTransaction::put_media
• IndexeddbMediaStoreTransaction::delete_media_by_content_size
• IndexeddbMediaStoreTransaction::delete_media_by_content_size_greater_than
• IndexeddbMediaStoreTransaction::delete_media_by_last_access
• IndexeddbMediaStoreTransaction::delete_media_by_last_access_earlier_than
• IndexeddbMediaStoreTransaction::delete_media_by_retention_metadata
• IndexeddbMediaStoreTransaction::delete_media_by_retention_metadata_to

Bug Fixes

1. 3a379e061a5d2266ae2be3d5e897d089d0e83df7 - Integration tests for MediaStore that assure media size constraints are properly implemented expect that content will maintain its size after encoding. For this reason, these tests are recommended to be run against an unencrypted MediaStore only (see documentation). Previously, unencrypted media contents were Base64 encoded, which increased the size of the contents, causing the tests to fail. Now, an unencrypted MediaStore does not modify the contents for serialization, but rather stores it as it is provided.
2. 377271814a0284183be2c4cdb49c7ef31f189760 - MediaStoreInner::set_media_retention_policy_inner previously failed to commit the transaction used to set the MediaRetentionPolicy and so the policy was not properly recorded. The transaction is now being commited.
3. 870e1e26718c75e9bb71206fd4085a98557afd14 - IndexeddbMediaStoreTransaction::put_media_if_policy_compliant previously failed to put Media items in IndexedDB when the given Media was set to ignore the MediaRetentionPolicy. The condition for adding Media through this function was modified to correct the error.

Caveats

IndexedDB does not seem to allow partial updates to existing objects. So, if one would like to update a single field on an object in IndexedDB, they must do the following.

1. Deserialize the object from the database.
2. Make the desired modifications to the deserialized object.
3. Re-serialize the modified object.
4. Overwrite the old object with the modified object.

This can, of course, be very inefficient when an object store's schema contains large fields.
And, unfortunately, this issue affects the existing schema for the Media object store.

The issue is that the Media object store stores metadata and content alongside each other in a single object. So, for instance, to set the last_access field of an object in IndexedDB, one has to deserialize and re-serialized the entire content of the Media, which might be very large.

The following functions are affected by this issue.

• MediaStore::replace_media_key
• MediaStore::get_media_content
  • Due to the use of IndexeddbMediaStoreTransaction::access_media_by_id which sets the last_access field of a Media object
• MediaStore::get_media_content_for_uri
  • Due to the use of IndexeddbMediaStoreTransaction::access_media_by_id which sets the last_access field of a Media object
• MediaStore::set_ignore_media_retention_policy

Proposed Solution

I am currently working on a solution to this problem which separates the metadata and content of a Media object into two separate object stores. As a result, this would require that retrieving media contents would require two separate queries to IndexedDB, but I think this is preferable to the excessive (de)serialization described above.

Future Work

• Separate metadata and contents in Media object store
• Refactor feature flags
  • The current feature flags are a bit convoluted and could be simplified and made more modular
• Expose EventCacheStore and MediaStore outside of the matrix-sdk-indexeddb

---

• 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.42%. Comparing base (9b48501) to head (abe244e).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5749   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files         360      360           
  Lines       99796    99796           
  Branches    99796    99796           
=======================================
+ Hits        88244    88246    +2     
+ Misses       7411     7410    -1     
+ Partials     4141     4140    -1     

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

[codspeed-hq]

CodSpeed Performance Report

Merging #5749 will not alter performance

_{Comparing mgoldenberg:indexeddb-media-store-clean (abe244e) with main (9b48501)}

Summary

✅ 50 untouched

[mgoldenberg]

(Test failure is due to an intermittent timing issue in how media_store_integration_tests_time::test_leased_locks is implemented).

[bnjbvr]

Thanks, looks good! Mostly small nits here and there, so will merge after they've been addressed.

Great work there 👍

[bnjbvr]

lgtm, thanks. Do you want to squash the fixup commits, or should I squash the entire PR into a single commit? I'm fine both ways.

[mgoldenberg]

Do you want to squash the fixup commits, or should I squash the entire PR into a single commit?

Ivan has opted for squashing the fixup commits for the other pull requests, so let's stick with that! Let me know if you want me to rebase!