Skip to content
Open
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
Binary file added crates/matrix-sdk/changelog.d/6712.added.md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatting here is not right. I think you want backticks where you currently have backslashes.

Binary file not shown.
14 changes: 14 additions & 0 deletions crates/matrix-sdk/src/client/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::{
collections::BTreeSet,
fmt,
sync::{Arc, RwLock as StdRwLock},
time::Duration,
};

#[cfg(feature = "sqlite")]
Expand Down Expand Up @@ -138,6 +139,7 @@ pub struct ClientBuilder {
search_index_store_kind: SearchIndexStoreKind,
dm_room_definition: DmRoomDefinition,
media_fetcher: Arc<dyn MediaFetcher>,
discovery_cache_timeout: Duration,
}

impl ClientBuilder {
Expand Down Expand Up @@ -176,6 +178,7 @@ impl ClientBuilder {
search_index_store_kind: SearchIndexStoreKind::InMemory,
dm_room_definition: DmRoomDefinition::MatrixSpec,
media_fetcher: Arc::new(DefaultMediaFetcher),
discovery_cache_timeout: Duration::from_secs(60 * 60 * 24),
}
}

Expand Down Expand Up @@ -671,13 +674,24 @@ impl ClientBuilder {
search_index,
thread_subscriptions_catchup,
self.media_fetcher.clone(),
self.discovery_cache_timeout,
)
.await;

debug!("Done building the Client");

Ok(Client { inner })
}

/// Set the duration after which cached discovery data (e.g. supported
/// versions, well-known info) is considered stale and needs to be
/// refreshed.
///
/// By default, this is 24 hours
pub fn discovery_cache_timeout(mut self, stale_timeout: Duration) -> Self {
self.discovery_cache_timeout = stale_timeout;
self
}
Comment on lines +690 to +694
}

/// Creates a server name from a user supplied string. The string is first
Expand Down
4 changes: 3 additions & 1 deletion crates/matrix-sdk/src/client/caches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::sync::Arc;
use std::{sync::Arc, time::Duration};

use matrix_sdk_base::store::WellKnownResponse;
use matrix_sdk_common::{locks::Mutex, ttl::TtlValue};
Expand Down Expand Up @@ -43,6 +43,8 @@ pub(crate) struct ClientCaches {
pub(crate) server_metadata: Cache<AuthorizationServerMetadata, ()>,
/// Homeserver capabilities.
pub(crate) homeserver_capabilities: Cache<Capabilities, Arc<HttpError>>,
/// The duration after which cached discovery data is considered stale.
pub(crate) discovery_cache_timeout: Duration,
}

/// A cached value that can either be set or not set, used to avoid confusion
Expand Down
51 changes: 49 additions & 2 deletions crates/matrix-sdk/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ use crate::{
Account, AuthApi, AuthSession, Error, HttpError, Media, Pusher, RefreshTokenError, Result,
Room, SessionTokens, TransmissionProgress,
authentication::{
AuthCtx, AuthData, ReloadSessionCallback, SaveSessionCallback, matrix::MatrixAuth,
AuthCtx, AuthData, ReloadSessionCallback, SaveSessionCallback,
matrix::MatrixAuth,
oauth::OAuth,
},
client::{
Expand Down Expand Up @@ -448,12 +449,14 @@ impl ClientInner {
#[cfg(feature = "experimental-search")] search_index_handler: SearchIndex,
thread_subscription_catchup: OnceCell<Arc<ThreadSubscriptionCatchup>>,
media_fetcher: Arc<dyn MediaFetcher>,
discovery_cache_timeout: Duration,
) -> Arc<Self> {
let caches = ClientCaches {
supported_versions: Cache::with_value(supported_versions),
well_known: Cache::with_value(well_known),
server_metadata: Cache::new(),
homeserver_capabilities: Cache::new(),
discovery_cache_timeout,
};

let client = Self {
Expand Down Expand Up @@ -3308,6 +3311,7 @@ impl Client {
self.inner.search_index.clone(),
self.inner.thread_subscription_catchup.clone(),
(*self.inner.media_fetcher.read().await).clone(),
self.inner.caches.discovery_cache_timeout,
)
.await,
};
Expand Down Expand Up @@ -3621,6 +3625,24 @@ impl Client {
pub async fn get_media_fetcher(&self) -> Arc<dyn MediaFetcher> {
self.inner.media_fetcher.read().await.clone()
}

/// Force a refresh of all the cached discovery data (supported versions,
/// well-known info, homeserver capabilities, and OAuth 2.0 server
/// metadata), ignoring whether the existing cache is still fresh.
///
/// If refreshing the well-known info fails, or if the OAuth 2.0 server
/// metadata request fails for any reason, it is silently ignored.
/// refresh failures are returned as an error, and any remaining
/// refreshes that haven't run yet are skipped.
Comment on lines +3633 to +3636
pub async fn rediscover(&self) -> Result<(), Error> {
self.refresh_supported_versions_cache(false).await?;
self.refresh_well_known_cache().await;
self.homeserver_capabilities().refresh().await?;
match self.oauth().server_metadata().await {
Ok(_) | Err(_) => {}
}
Comment on lines +3641 to +3643
Ok(())
}
}

/// Contains the disk size of the different stores, if known. It won't be
Expand Down Expand Up @@ -3716,7 +3738,11 @@ pub(crate) mod tests {
RoomId, ServerName, UserId,
api::{
FeatureFlag, MatrixVersion,
client::{room::create_room::v3::Request as CreateRoomRequest, rtc::RtcTransport},
client::{
discovery::get_capabilities::v3::Capabilities,
room::create_room::v3::Request as CreateRoomRequest,
rtc::RtcTransport,
},
},
assign,
events::{
Expand Down Expand Up @@ -3940,6 +3966,27 @@ pub(crate) mod tests {
client.server_versions().await.unwrap();
}

#[async_test]
async fn test_rediscover() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a lot more tests. This test does not check that rediscover actually hits the endpoints, or that it uses what is being returned from them. You don't have any tests that check that the timeout is being used, and as far as I can see it's not being used, so as it stands this PR doesn't make any sense at all.

Please check out https://github.com/matrix-org/matrix-rust-sdk/blob/main/CONTRIBUTING.md#ai-policy . I am not convinced that a human has read and understood this code, which would be a violation of our contribution policy.

let server = MatrixMockServer::new().await;

// Mock the two required endpoints
server.mock_versions().ok().mock_once().named("versions").mount().await;
server
.mock_get_homeserver_capabilities()
.ok_with_capabilities(Capabilities::default())
.mock_once()
.named("capabilities")
.mount()
.await;

// Build the client
let client = server.client_builder().build().await;

// Call rediscover and assert it succeeded
client.rediscover().await.unwrap();
}

#[async_test]
async fn test_discovery_broken_server() {
let server = MatrixMockServer::new().await;
Expand Down
Loading