From 465533d32b32368880f444243001d71b0249c46b Mon Sep 17 00:00:00 2001 From: strawberry Date: Mon, 4 Mar 2024 19:51:00 -0500 Subject: [PATCH] attempt keeping track/cache remote profiles locally again also fixes logic error where we always say we couldnt find the profile Signed-off-by: strawberry --- src/api/client_server/profile.rs | 98 +++++++++++++--------------- src/service/rooms/state_cache/mod.rs | 32 +++++---- 2 files changed, 62 insertions(+), 68 deletions(-) diff --git a/src/api/client_server/profile.rs b/src/api/client_server/profile.rs index fc4eff66..3c5157f2 100644 --- a/src/api/client_server/profile.rs +++ b/src/api/client_server/profile.rs @@ -1,4 +1,5 @@ -use crate::{service::pdu::PduBuilder, services, Error, Result, Ruma}; +use std::sync::Arc; + use ruma::{ api::{ client::{ @@ -7,13 +8,14 @@ use ruma::{ get_avatar_url, get_display_name, get_profile, set_avatar_url, set_display_name, }, }, - federation::{self, query::get_profile_information::v1::ProfileField}, + federation, }, events::{room::member::RoomMemberEventContent, StateEventType, TimelineEventType}, presence::PresenceState, }; use serde_json::value::to_raw_value; -use std::sync::Arc; + +use crate::{service::pdu::PduBuilder, services, Error, Result, Ruma}; /// # `PUT /_matrix/client/r0/profile/{userId}/displayname` /// @@ -113,39 +115,35 @@ pub async fn set_displayname_route( pub async fn get_displayname_route( body: Ruma, ) -> Result { - if (services().users.exists(&body.user_id)?) - && (body.user_id.server_name() != services().globals.server_name()) - { + if body.user_id.server_name() != services().globals.server_name() { let response = services() .sending .send_federation_request( body.user_id.server_name(), federation::query::get_profile_information::v1::Request { user_id: body.user_id.clone(), - field: Some(ProfileField::DisplayName), + field: None, // we want the full user's profile to update locally too }, ) .await?; - /* - TODO: ignore errors properly? - // Create and update our local copy of the user - // these are `let _` because it's fine if we can't find these for the user. - // also these requests are sent on room join so dead servers will make room joins annoying again - let _ = services().users.create(&body.user_id, None); - let _ = services() + // Create and update our local copy of the user for only the fields we request for + if !services().users.exists(&body.user_id)? { + services().users.create(&body.user_id, None)?; + } + + services() .users .set_displayname(&body.user_id, response.displayname.clone()) - .await; - let _ = services() + .await?; + services() .users - .set_avatar_url(&body.user_id, response.avatar_url) - .await; - let _ = services() + .set_avatar_url(&body.user_id, response.avatar_url.clone()) + .await?; + services() .users - .set_blurhash(&body.user_id, response.blurhash) - .await; - */ + .set_blurhash(&body.user_id, response.blurhash.clone()) + .await?; return Ok(get_display_name::v3::Response { displayname: response.displayname, @@ -260,39 +258,35 @@ pub async fn set_avatar_url_route( pub async fn get_avatar_url_route( body: Ruma, ) -> Result { - if (services().users.exists(&body.user_id)?) - && (body.user_id.server_name() != services().globals.server_name()) - { + if body.user_id.server_name() != services().globals.server_name() { let response = services() .sending .send_federation_request( body.user_id.server_name(), federation::query::get_profile_information::v1::Request { user_id: body.user_id.clone(), - field: Some(ProfileField::AvatarUrl), + field: None, // we want the full user's profile to update locally as well }, ) .await?; - /* - TODO: ignore errors properly? // Create and update our local copy of the user - // these are `let _` because it's fine if we can't find these for the user. - // also these requests are sent on room join so dead servers will make room joins annoying again - let _ = services().users.create(&body.user_id, None); - let _ = services() + if !services().users.exists(&body.user_id)? { + services().users.create(&body.user_id, None)?; + } + + services() .users - .set_displayname(&body.user_id, response.displayname) - .await; - let _ = services() + .set_displayname(&body.user_id, response.displayname.clone()) + .await?; + services() .users .set_avatar_url(&body.user_id, response.avatar_url.clone()) - .await; - let _ = services() + .await?; + services() .users .set_blurhash(&body.user_id, response.blurhash.clone()) - .await; - */ + .await?; return Ok(get_avatar_url::v3::Response { avatar_url: response.avatar_url, @@ -315,9 +309,7 @@ pub async fn get_avatar_url_route( pub async fn get_profile_route( body: Ruma, ) -> Result { - if (services().users.exists(&body.user_id)?) - && (body.user_id.server_name() != services().globals.server_name()) - { + if body.user_id.server_name() != services().globals.server_name() { let response = services() .sending .send_federation_request( @@ -329,25 +321,23 @@ pub async fn get_profile_route( ) .await?; - /* - TODO: ignore errors properly? // Create and update our local copy of the user - // these are `let _` because it's fine if we can't find these for the user. - // also these requests are sent on room join so dead servers will make room joins annoying again - let _ = services().users.create(&body.user_id, None); - let _ = services() + if !services().users.exists(&body.user_id)? { + services().users.create(&body.user_id, None)?; + } + + services() .users .set_displayname(&body.user_id, response.displayname.clone()) - .await; - let _ = services() + .await?; + services() .users .set_avatar_url(&body.user_id, response.avatar_url.clone()) - .await; - let _ = services() + .await?; + services() .users .set_blurhash(&body.user_id, response.blurhash.clone()) - .await; - */ + .await?; return Ok(get_profile::v3::Response { displayname: response.displayname, diff --git a/src/service/rooms/state_cache/mod.rs b/src/service/rooms/state_cache/mod.rs index 6b792906..8e12f817 100644 --- a/src/service/rooms/state_cache/mod.rs +++ b/src/service/rooms/state_cache/mod.rs @@ -1,8 +1,6 @@ -mod data; use std::{collections::HashSet, sync::Arc}; -pub use data::Data; - +use ruma::api::federation; use ruma::{ api::appservice::Registration, events::{ @@ -20,8 +18,12 @@ use ruma::{ }; use tracing::warn; +pub use data::Data; + use crate::{services, Error, Result}; +mod data; + pub struct Service { pub db: &'static dyn Data, } @@ -39,12 +41,14 @@ impl Service { update_joined_count: bool, ) -> Result<()> { let membership = membership_event.membership; + // Keep track what remote users exist by adding them as "deactivated" users if user_id.server_name() != services().globals.server_name() { - services().users.create(user_id, None)?; - /* + if !services().users.exists(user_id)? { + services().users.create(user_id, None)?; + } + // Try to update our local copy of the user if ours does not match - // TODO: ignore errors properly? if ((services().users.displayname(user_id)? != membership_event.displayname) || (services().users.avatar_url(user_id)? != membership_event.avatar_url) || (services().users.blurhash(user_id)? != membership_event.blurhash)) @@ -56,24 +60,24 @@ impl Service { user_id.server_name(), federation::query::get_profile_information::v1::Request { user_id: user_id.into(), - field: Some(ProfileField::AvatarUrl), + field: None, // we want the full user's profile to update locally too }, ) .await?; - let _ = services() + + services() .users .set_displayname(user_id, response.displayname.clone()) - .await; - let _ = services() + .await?; + services() .users .set_avatar_url(user_id, response.avatar_url) - .await; - let _ = services() + .await?; + services() .users .set_blurhash(user_id, response.blurhash) - .await; + .await?; }; - */ } match &membership {