test(base): Improve test_media_content.

This patch improves `test_media_content` to ensure that, in case of
multiple medias, only the expected ones are removed. Previously, the
test wasn't testing _other_ medias that should be kept in case of
removals.

This patch continues to improve `test_media_content` to ensure that the
content of the media are the expected ones.

Finally, this patch updates the `MemoryStore` implementation to make
tests happy.

crates/matrix-sdk-base/src/store/integration_tests.rs

@@ -200,11 +200,8 @@ impl StateStoreIntegrationTests for DynStateStore {
 
     async fn test_media_content(&self) {
         let uri = mxc_uri!("mxc://localhost/media");
-        let content: Vec<u8> = "somebinarydata".into();
-
         let request_file =
             MediaRequest { source: MediaSource::Plain(uri.to_owned()), format: MediaFormat::File };
-
         let request_thumbnail = MediaRequest {
             source: MediaSource::Plain(uri.to_owned()),
             format: MediaFormat::Thumbnail(MediaThumbnailSize {
@@ -214,6 +211,17 @@ impl StateStoreIntegrationTests for DynStateStore {
             }),
         };
 
+        let other_uri = mxc_uri!("mxc://localhost/media-other");
+        let request_other_file = MediaRequest {
+            source: MediaSource::Plain(other_uri.to_owned()),
+            format: MediaFormat::File,
+        };
+
+        let content: Vec<u8> = "hello".into();
+        let thumbnail_content: Vec<u8> = "world".into();
+        let other_content: Vec<u8> = "foo".into();
+
+        // Media isn't present in the cache.
         assert!(
             self.get_media_content(&request_file).await.unwrap().is_none(),
             "unexpected media found"
@@ -223,35 +231,63 @@ impl StateStoreIntegrationTests for DynStateStore {
             "media not found"
         );
 
+        // Let's add the media.
         self.add_media_content(&request_file, content.clone()).await.expect("adding media failed");
-        assert!(
-            self.get_media_content(&request_file).await.unwrap().is_some(),
+
+        // Media is present in the cache.
+        assert_eq!(
+            self.get_media_content(&request_file).await.unwrap().as_ref(),
+            Some(&content),
             "media not found though added"
         );
 
+        // Let's remove the media.
         self.remove_media_content(&request_file).await.expect("removing media failed");
+
+        // Media isn't present in the cache.
         assert!(
             self.get_media_content(&request_file).await.unwrap().is_none(),
             "media still there after removing"
         );
 
+        // Let's add the media again.
         self.add_media_content(&request_file, content.clone())
             .await
             .expect("adding media again failed");
-        assert!(
-            self.get_media_content(&request_file).await.unwrap().is_some(),
+
+        assert_eq!(
+            self.get_media_content(&request_file).await.unwrap().as_ref(),
+            Some(&content),
             "media not found after adding again"
         );
 
-        self.add_media_content(&request_thumbnail, content.clone())
+        // Let's add the thumbnail media.
+        self.add_media_content(&request_thumbnail, thumbnail_content.clone())
             .await
             .expect("adding thumbnail failed");
-        assert!(
-            self.get_media_content(&request_thumbnail).await.unwrap().is_some(),
+
+        // Media's thumbnail is present.
+        assert_eq!(
+            self.get_media_content(&request_thumbnail).await.unwrap().as_ref(),
+            Some(&thumbnail_content),
             "thumbnail not found"
         );
 
+        // Let's add another media with a different URI.
+        self.add_media_content(&request_other_file, other_content.clone())
+            .await
+            .expect("adding other media failed");
+
+        // Other file is present.
+        assert_eq!(
+            self.get_media_content(&request_other_file).await.unwrap().as_ref(),
+            Some(&other_content),
+            "other file not found"
+        );
+
+        // Let's remove media based on URI.
         self.remove_media_content_for_uri(uri).await.expect("removing all media for uri failed");
+
         assert!(
             self.get_media_content(&request_file).await.unwrap().is_none(),
             "media wasn't removed"
@@ -260,6 +296,10 @@ impl StateStoreIntegrationTests for DynStateStore {
             self.get_media_content(&request_thumbnail).await.unwrap().is_none(),
             "thumbnail wasn't removed"
         );
+        assert!(
+            self.get_media_content(&request_other_file).await.unwrap().is_some(),
+            "other media was removed"
+        );
     }
 
     async fn test_topic_redaction(&self) -> Result<()> {

crates/matrix-sdk-base/src/store/memory_store.rs

@@ -78,7 +78,7 @@ pub struct MemoryStore {
             HashMap<(String, Option<String>), HashMap<OwnedEventId, HashMap<OwnedUserId, Receipt>>>,
         >,
     >,
-    media: StdRwLock<RingBuffer<(OwnedMxcUri, Vec<u8>)>>,
+    media: StdRwLock<RingBuffer<(OwnedMxcUri, String /* unique key */, Vec<u8>)>>,
     custom: StdRwLock<HashMap<Vec<u8>, Vec<u8>>>,
 }
 
@@ -703,16 +703,19 @@ impl StateStore for MemoryStore {
     }
 
     async fn add_media_content(&self, request: &MediaRequest, data: Vec<u8>) -> Result<()> {
-        self.media.write().unwrap().push((request.uri().to_owned(), data));
+        // Avoid duplication. Let's try to remove it first.
+        self.remove_media_content(request).await?;
+        // Now, let's add it.
+        self.media.write().unwrap().push((request.uri().to_owned(), request.unique_key(), data));
 
         Ok(())
     }
 
     async fn get_media_content(&self, request: &MediaRequest) -> Result<Option<Vec<u8>>> {
         let media = self.media.read().unwrap();
-        let expected_key = request.uri().to_owned();
+        let expected_key = request.unique_key();
 
-        for (media_key, media_content) in media.iter() {
+        for (_media_uri, media_key, media_content) in media.iter() {
             if media_key == &expected_key {
                 return Ok(Some(media_content.to_owned()));
             }
@@ -723,8 +726,11 @@ impl StateStore for MemoryStore {
 
     async fn remove_media_content(&self, request: &MediaRequest) -> Result<()> {
         let mut media = self.media.write().unwrap();
-        let expected_key = request.uri().to_owned();
-        let Some(index) = media.iter().position(|(media_key, _)| media_key == &expected_key) else {
+        let expected_key = request.unique_key();
+        let Some(index) = media
+            .iter()
+            .position(|(_media_uri, media_key, _media_content)| media_key == &expected_key)
+        else {
             return Ok(());
         };
 
@@ -739,8 +745,8 @@ impl StateStore for MemoryStore {
         let positions = media
             .iter()
             .enumerate()
-            .filter_map(|(position, (media_key, _))| {
-                (media_key == &expected_key).then_some(position)
+            .filter_map(|(position, (media_uri, _media_key, _media_content))| {
+                (media_uri == &expected_key).then_some(position)
             })
             .collect::<Vec<_>>();