feat(event cache): return relations in their local ordering

[bnjbvr]

This makes it so that the relations returned by RoomEventCache::event_with_relations are now sorted by their position in the linked chunk, if available (events that can't be located in a linked chunk will be at the front of the array, as they might be "bundled" relations they should be handled in priority).

This will make it possible, for a threaded timeline, to retrieve all related edits in a thread, and to correctly order them (if we've seen them in the main linked chunk first).

(In the meantime, we could even use it to include the edit to a latest thread event as the latest event 👀 Will look into that as a separate PR.)

On top of #5225. Part of #4869 / #5122.

[codecov]

Codecov Report

Attention: Patch coverage is 93.26425% with 13 lines in your changes missing coverage. Please review.

Project coverage is 88.74%. Comparing base (0d08093) to head (ade063f).
Report is 11 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-sqlite/src/event_cache_store.rs	66.66%	0 Missing and 6 partials ⚠️
crates/matrix-sdk/src/event_cache/room/mod.rs	77.77%	4 Missing ⚠️
...s/matrix-sdk-common/src/linked_chunk/relational.rs	98.07%	0 Missing and 2 partials ⚠️
...rix-sdk-base/src/event_cache/store/memory_store.rs	93.75%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5258      +/-   ##
==========================================
- Coverage   88.75%   88.74%   -0.01%     
==========================================
  Files         331      331              
  Lines       89504    89548      +44     
  Branches    89504    89548      +44     
==========================================
+ Hits        79435    79469      +34     
- Misses       6260     6268       +8     
- Partials     3809     3811       +2     

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

[poljar]

Looks good.

[poljar]

It's quite strange to have OwnedLinkedChunkId and LinkedChunkId but also use &OwnedLinkedChunkId.

Why don't we have borrow() implemented to convert from one to the other?

[bnjbvr]

I don't think I can do this, because the Borrow trait resembles this:

pub trait Borrow<Borrowed: ?Sized> {
    fn borrow(&self) -> &Borrowed;
}

So if I implemented it for OwnedLinkedChunk, then I'd need to return &LinkedChunk<'_>. Unfortunately, it's impossible to construct a temporary LinkedChunk and return a reference to it (since it's dropped at the end of the function's scope). I've added a fake OwnedLinkedChunkId::as_ref() method, but it can't be used for a HashMap<OwnedLinkedChunkId, T> lookup (as the lookups really want &impl Borrow<HashMapKey>). Hence this sub-optimal API.

It's something I would appreciate help with, because I've tried many things when implementing the pair LinkedChunkId / OwnedLinkedChunkId, and couldn't find something that's nice to implement for hashmap lookups.

[poljar]

Yes, for maps you'll need a Borrow. I think the docs on Borrow explain why:

/// In particular `Eq`, `Ord` and `Hash` must be equivalent for
/// borrowed and owned values: `x.borrow() == y.borrow()` should give the
/// same result as `x == y`.
///
/// If generic code merely needs to work for all types that can
/// provide a reference to related type `T`, it is often better to use
/// [`AsRef<T>`] as more types can safely implement it.

So if I implemented it for OwnedLinkedChunk, then I'd need to return &LinkedChunk<'_>. Unfortunately, it's impossible to construct a temporary LinkedChunk and return a reference to it (since it's dropped at the end of the function's scope).

Sadly for borrow you usually need a bit of unsafe, you can transmute from one type to the other, this is probably a minimal example:

use std::collections::HashMap;
use std::borrow::Borrow;

#[derive(PartialEq, Eq, Hash)]
struct Foo(str);

impl From<&str> for &Foo {
    fn from(s: &str) -> Self {
        unsafe { std::mem::transmute(s) }
    }
}

#[derive(PartialEq, Eq, Hash)]
struct OwnedFoo(String);

impl Borrow<Foo> for OwnedFoo {
    fn borrow(&self) -> &Foo {
        self.0.as_str().into()
    }
}

fn main() {
    let owned_foo = OwnedFoo("foo".to_owned());
    let mut map = HashMap::new();
    
    map.insert(owned_foo, 1);
    
    let foo = <&Foo>::from("foo");
    let get = map.get(foo).expect("We should get foo");
    
    println!("Hello {get}")
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=870437c03819f5771cf1904d0e68b7f7

You can also take a look at what the Ruma types do using cargo expand -p ruma-common:

For example OwnedRoomId is defined as:

pub struct OwnedRoomId {
    inner: Box<RoomId>,
}

Which makes a Borrow and AsRef implementation are pretty easy. Though they do require unsafe somewhere:

impl RoomId {
    pub(super) const fn from_borrowed(s: &str) -> &Self {
        unsafe { std::mem::transmute(s) }
    }
}