Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!(media): flatten and rename MediaRequest and fields #4223

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions bindings/matrix-sdk-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use std::{
use anyhow::{anyhow, Context as _};
use matrix_sdk::{
media::{
MediaFileHandle as SdkMediaFileHandle, MediaFormat, MediaRequest, MediaThumbnailSettings,
MediaFileHandle as SdkMediaFileHandle, MediaFormat, MediaRequestParameters,
MediaThumbnailSettings,
},
oidc::{
registrations::{ClientId, OidcRegistrations},
Expand Down Expand Up @@ -442,7 +443,7 @@ impl Client {
.inner
.media()
.get_media_file(
&MediaRequest { source, format: MediaFormat::File },
&MediaRequestParameters { source, format: MediaFormat::File },
filename,
&mime_type,
use_cache,
Expand Down Expand Up @@ -721,7 +722,7 @@ impl Client {
Ok(self
.inner
.media()
.get_media_content(&MediaRequest { source, format: MediaFormat::File }, true)
.get_media_content(&MediaRequestParameters { source, format: MediaFormat::File }, true)
.await?)
}

Expand All @@ -738,7 +739,7 @@ impl Client {
.inner
.media()
.get_media_content(
&MediaRequest {
&MediaRequestParameters {
source,
format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
Method::Scale,
Expand Down
20 changes: 12 additions & 8 deletions crates/matrix-sdk-base/src/event_cache_store/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use ruma::{
};

use super::DynEventCacheStore;
use crate::media::{MediaFormat, MediaRequest, MediaThumbnailSettings};
use crate::media::{MediaFormat, MediaRequestParameters, MediaThumbnailSettings};

/// `EventCacheStore` integration tests.
///
Expand All @@ -41,9 +41,11 @@ pub trait EventCacheStoreIntegrationTests {
impl EventCacheStoreIntegrationTests for DynEventCacheStore {
async fn test_media_content(&self) {
let uri = mxc_uri!("mxc://localhost/media");
let request_file =
MediaRequest { source: MediaSource::Plain(uri.to_owned()), format: MediaFormat::File };
let request_thumbnail = MediaRequest {
let request_file = MediaRequestParameters {
source: MediaSource::Plain(uri.to_owned()),
format: MediaFormat::File,
};
let request_thumbnail = MediaRequestParameters {
source: MediaSource::Plain(uri.to_owned()),
format: MediaFormat::Thumbnail(MediaThumbnailSettings::new(
Method::Crop,
Expand All @@ -53,7 +55,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {
};

let other_uri = mxc_uri!("mxc://localhost/media-other");
let request_other_file = MediaRequest {
let request_other_file = MediaRequestParameters {
source: MediaSource::Plain(other_uri.to_owned()),
format: MediaFormat::File,
};
Expand Down Expand Up @@ -145,8 +147,10 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {

async fn test_replace_media_key(&self) {
let uri = mxc_uri!("mxc://sendqueue.local/tr4n-s4ct-10n1-d");
let req =
MediaRequest { source: MediaSource::Plain(uri.to_owned()), format: MediaFormat::File };
let req = MediaRequestParameters {
source: MediaSource::Plain(uri.to_owned()),
format: MediaFormat::File,
};

let content = "hello".as_bytes().to_owned();

Expand All @@ -161,7 +165,7 @@ impl EventCacheStoreIntegrationTests for DynEventCacheStore {

// Replacing a media request works.
let new_uri = mxc_uri!("mxc://matrix.org/tr4n-s4ct-10n1-d");
let new_req = MediaRequest {
let new_req = MediaRequestParameters {
source: MediaSource::Plain(new_uri.to_owned()),
format: MediaFormat::File,
};
Expand Down
16 changes: 10 additions & 6 deletions crates/matrix-sdk-base/src/event_cache_store/memory_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use matrix_sdk_common::ring_buffer::RingBuffer;
use ruma::{MxcUri, OwnedMxcUri};

use super::{EventCacheStore, EventCacheStoreError, Result};
use crate::media::{MediaRequest, UniqueKey as _};
use crate::media::{MediaRequestParameters, UniqueKey as _};

/// In-memory, non-persistent implementation of the `EventCacheStore`.
///
Expand Down Expand Up @@ -51,7 +51,11 @@ impl MemoryStore {
impl EventCacheStore for MemoryStore {
type Error = EventCacheStoreError;

async fn add_media_content(&self, request: &MediaRequest, data: Vec<u8>) -> Result<()> {
async fn add_media_content(
&self,
request: &MediaRequestParameters,
data: Vec<u8>,
) -> Result<()> {
// Avoid duplication. Let's try to remove it first.
self.remove_media_content(request).await?;
// Now, let's add it.
Expand All @@ -62,8 +66,8 @@ impl EventCacheStore for MemoryStore {

async fn replace_media_key(
&self,
from: &MediaRequest,
to: &MediaRequest,
from: &MediaRequestParameters,
to: &MediaRequestParameters,
) -> Result<(), Self::Error> {
let expected_key = from.unique_key();

Expand All @@ -76,7 +80,7 @@ impl EventCacheStore for MemoryStore {
Ok(())
}

async fn get_media_content(&self, request: &MediaRequest) -> Result<Option<Vec<u8>>> {
async fn get_media_content(&self, request: &MediaRequestParameters) -> Result<Option<Vec<u8>>> {
let expected_key = request.unique_key();

let media = self.media.read().unwrap();
Expand All @@ -85,7 +89,7 @@ impl EventCacheStore for MemoryStore {
}))
}

async fn remove_media_content(&self, request: &MediaRequest) -> Result<()> {
async fn remove_media_content(&self, request: &MediaRequestParameters) -> Result<()> {
let expected_key = request.unique_key();

let mut media = self.media.write().unwrap();
Expand Down
28 changes: 17 additions & 11 deletions crates/matrix-sdk-base/src/event_cache_store/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use matrix_sdk_common::AsyncTraitDeps;
use ruma::MxcUri;

use super::EventCacheStoreError;
use crate::media::MediaRequest;
use crate::media::MediaRequestParameters;

/// An abstract trait that can be used to implement different store backends
/// for the event cache of the SDK.
Expand All @@ -38,7 +38,7 @@ pub trait EventCacheStore: AsyncTraitDeps {
/// * `content` - The content of the file.
async fn add_media_content(
&self,
request: &MediaRequest,
request: &MediaRequestParameters,
content: Vec<u8>,
) -> Result<(), Self::Error>;

Expand All @@ -63,8 +63,8 @@ pub trait EventCacheStore: AsyncTraitDeps {
/// * `to` - The new `MediaRequest` of the file.
async fn replace_media_key(
&self,
from: &MediaRequest,
to: &MediaRequest,
from: &MediaRequestParameters,
to: &MediaRequestParameters,
) -> Result<(), Self::Error>;

/// Get a media file's content out of the media store.
Expand All @@ -74,15 +74,18 @@ pub trait EventCacheStore: AsyncTraitDeps {
/// * `request` - The `MediaRequest` of the file.
async fn get_media_content(
&self,
request: &MediaRequest,
request: &MediaRequestParameters,
) -> Result<Option<Vec<u8>>, Self::Error>;

/// Remove a media file's content from the media store.
///
/// # Arguments
///
/// * `request` - The `MediaRequest` of the file.
async fn remove_media_content(&self, request: &MediaRequest) -> Result<(), Self::Error>;
async fn remove_media_content(
&self,
request: &MediaRequestParameters,
) -> Result<(), Self::Error>;

/// Remove all the media files' content associated to an `MxcUri` from the
/// media store.
Expand Down Expand Up @@ -110,28 +113,31 @@ impl<T: EventCacheStore> EventCacheStore for EraseEventCacheStoreError<T> {

async fn add_media_content(
&self,
request: &MediaRequest,
request: &MediaRequestParameters,
content: Vec<u8>,
) -> Result<(), Self::Error> {
self.0.add_media_content(request, content).await.map_err(Into::into)
}

async fn replace_media_key(
&self,
from: &MediaRequest,
to: &MediaRequest,
from: &MediaRequestParameters,
to: &MediaRequestParameters,
) -> Result<(), Self::Error> {
self.0.replace_media_key(from, to).await.map_err(Into::into)
}

async fn get_media_content(
&self,
request: &MediaRequest,
request: &MediaRequestParameters,
) -> Result<Option<Vec<u8>>, Self::Error> {
self.0.get_media_content(request).await.map_err(Into::into)
}

async fn remove_media_content(&self, request: &MediaRequest) -> Result<(), Self::Error> {
async fn remove_media_content(
&self,
request: &MediaRequestParameters,
) -> Result<(), Self::Error> {
self.0.remove_media_content(request).await.map_err(Into::into)
}

Expand Down
35 changes: 12 additions & 23 deletions crates/matrix-sdk-base/src/media.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ impl UniqueKey for MediaFormat {
}
}

/// The requested size of a media thumbnail.
/// The desired settings of a media thumbnail.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct MediaThumbnailSize {
pub struct MediaThumbnailSettings {
/// The desired resizing method.
pub method: Method,

Expand All @@ -57,19 +57,6 @@ pub struct MediaThumbnailSize {
/// The desired height of the thumbnail. The actual thumbnail may not match
/// the size specified.
pub height: UInt,
}

impl UniqueKey for MediaThumbnailSize {
fn unique_key(&self) -> String {
format!("{}{UNIQUE_SEPARATOR}{}x{}", self.method, self.width, self.height)
}
}

/// The desired settings of a media thumbnail.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct MediaThumbnailSettings {
/// The desired size of the thumbnail.
pub size: MediaThumbnailSize,

/// If we want to request an animated thumbnail from the homeserver.
///
Expand All @@ -84,13 +71,13 @@ impl MediaThumbnailSettings {
/// Constructs a new `MediaThumbnailSettings` with the given method, width
/// and height.
pub fn new(method: Method, width: UInt, height: UInt) -> Self {
Self { size: MediaThumbnailSize { method, width, height }, animated: false }
Self { method, width, height, animated: false }
}
}

impl UniqueKey for MediaThumbnailSettings {
fn unique_key(&self) -> String {
let mut key = self.size.unique_key();
let mut key = format!("{}{UNIQUE_SEPARATOR}{}x{}", self.method, self.width, self.height);

if self.animated {
key.push_str(UNIQUE_SEPARATOR);
Expand All @@ -110,17 +97,19 @@ impl UniqueKey for MediaSource {
}
}

/// A request for media data.
/// Parameters for a request for retrieve media data.
///
/// This is used as a key in the media cache too.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct MediaRequest {
pub struct MediaRequestParameters {
/// The source of the media file.
pub source: MediaSource,

/// The requested format of the media data.
pub format: MediaFormat,
}

impl MediaRequest {
impl MediaRequestParameters {
/// Get the [`MxcUri`] from `Self`.
pub fn uri(&self) -> &MxcUri {
match &self.source {
Expand All @@ -130,7 +119,7 @@ impl MediaRequest {
}
}

impl UniqueKey for MediaRequest {
impl UniqueKey for MediaRequestParameters {
fn unique_key(&self) -> String {
format!("{}{UNIQUE_SEPARATOR}{}", self.source.unique_key(), self.format.unique_key())
}
Expand Down Expand Up @@ -226,14 +215,14 @@ mod tests {
fn test_media_request_url() {
let mxc_uri = mxc_uri!("mxc://homeserver/media");

let plain = MediaRequest {
let plain = MediaRequestParameters {
source: MediaSource::Plain(mxc_uri.to_owned()),
format: MediaFormat::File,
};

assert_eq!(plain.uri(), mxc_uri);

let file = MediaRequest {
let file = MediaRequestParameters {
source: MediaSource::Encrypted(Box::new(
serde_json::from_value(json!({
"url": mxc_uri,
Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk-base/src/store/send_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use ruma::{
};
use serde::{Deserialize, Serialize};

use crate::media::MediaRequest;
use crate::media::MediaRequestParameters;

/// A thin wrapper to serialize a `AnyMessageLikeEventContent`.
#[derive(Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -95,7 +95,7 @@ pub enum QueuedRequestKind {

/// The cache key used to retrieve the media's bytes in the event cache
/// store.
cache_key: MediaRequest,
cache_key: MediaRequestParameters,

/// An optional media source for a thumbnail already uploaded.
thumbnail_source: Option<MediaSource>,
Expand Down Expand Up @@ -216,7 +216,7 @@ pub enum DependentQueuedRequestKind {

/// Media request necessary to retrieve the file itself (not the
/// thumbnail).
cache_key: MediaRequest,
cache_key: MediaRequestParameters,

/// To which media transaction id does this upload relate to?
related_to: OwnedTransactionId,
Expand Down
Loading