HMC/handmade-revolt-backend
6
0
Fork 0
forked from jmug/stoatchat
Code Pull Requests Actions Packages Releases 2 Activity

fix: don't remove timeouts when a member leaves a server (#409)

Browse Source
This commit is contained in:
Tom
2025-09-06 18:41:23 -07:00
committed by GitHub
parent 3cb7da95e3
commit e635bc23ec
12 changed files with 778 additions and 449 deletions
Download Patch File Download Diff File Expand all files Collapse all files

960
Cargo.lock generated
View File

File diff suppressed because it is too large Load Diff

84
crates/core/database/src/models/server_members/model.rs
View File

@@ -30,6 +30,9 @@ auto_derived_partial!(
/// Timestamp this member is timed out until
#[serde(skip_serializing_if = "Option::is_none")]
pub timeout: Option<Timestamp>,
// This value only exists in the database, not the models.
// If it is not-None, the database layer should return None to member fetching queries.
// pub pending_deletion_at: Option<Timestamp>
},
"PartialMember"
);
@@ -50,6 +53,7 @@ auto_derived!(
Avatar,
Roles,
Timeout,
JoinedAt,
}
/// Member removal intention
@@ -90,7 +94,7 @@ impl Member {
return Err(create_error!(AlreadyInServer));
}
let member = Member {
let mut member = Member {
id: MemberCompositeKey {
server: server.id.to_string(),
user: user.id.to_string(),
@@ -98,7 +102,9 @@ impl Member {
..Default::default()
};
db.insert_member(&member).await?;
if let Some(updated) = db.insert_or_merge_member(&member).await? {
member = updated;
}
let should_fetch = channels.is_none();
let mut channels = channels.unwrap_or_default();
@@ -187,6 +193,7 @@ impl Member {
pub fn remove_field(&mut self, field: &FieldsMember) {
match field {
FieldsMember::JoinedAt => (),
FieldsMember::Avatar => self.avatar = None,
FieldsMember::Nickname => self.nickname = None,
FieldsMember::Roles => self.roles.clear(),
@@ -225,7 +232,7 @@ impl Member {
intention: RemovalIntention,
silent: bool,
) -> Result<()> {
db.delete_member(&self.id).await?;
db.soft_delete_member(&self.id).await?;
EventV1::ServerMemberLeave {
id: self.id.server.to_string(),
@@ -261,3 +268,74 @@ impl Member {
Ok(())
}
}
#[cfg(test)]
mod tests {
use iso8601_timestamp::{Duration, Timestamp};
use revolt_models::v0::DataCreateServer;
use crate::{Member, PartialMember, RemovalIntention, Server, User};
#[async_std::test]
async fn muted_member_rejoin() {
database_test!(|db| async move {
match db {
crate::Database::Reference(_) => return,
crate::Database::MongoDb(_) => (),
}
let owner = User::create(&db, "Server Owner".to_string(), None, None)
.await
.unwrap();
let kickable_user = User::create(&db, "Member".to_string(), None, None)
.await
.unwrap();
let server = Server::create(
&db,
DataCreateServer {
name: "Server".to_string(),
description: None,
nsfw: None,
},
&owner,
false,
)
.await
.unwrap()
.0;
Member::create(&db, &server, &owner, None).await.unwrap();
let mut kickable_member = Member::create(&db, &server, &kickable_user, None)
.await
.unwrap()
.0;
kickable_member
.update(
&db,
PartialMember {
timeout: Some(Timestamp::now_utc() + Duration::minutes(5)),
..Default::default()
},
vec![],
)
.await
.unwrap();
assert!(kickable_member.in_timeout());
kickable_member
.remove(&db, &server, RemovalIntention::Kick, false)
.await
.unwrap();
let kickable_member = Member::create(&db, &server, &kickable_user, None)
.await
.unwrap()
.0;
assert!(kickable_member.in_timeout())
});
}
}

14
crates/core/database/src/models/server_members/ops.rs
View File

@@ -73,7 +73,7 @@ impl ChunkedServerMembersGenerator {
#[async_trait]
pub trait AbstractServerMembers: Sync + Send {
/// Insert a new server member into the database
async fn insert_member(&self, member: &Member) -> Result<()>;
async fn insert_or_merge_member(&self, member: &Member) -> Result<Option<Member>>;
/// Fetch a server member by their id
async fn fetch_member(&self, server_id: &str, user_id: &str) -> Result<Member>;
@@ -119,6 +119,14 @@ pub trait AbstractServerMembers: Sync + Send {
remove: Vec<FieldsMember>,
) -> Result<()>;
/// Delete a server member by their id
async fn delete_member(&self, id: &MemberCompositeKey) -> Result<()>;
/// Marks a user as no longer a member of a server, while retaining the database value.
/// This is used to keep information such as timeouts in place, but will remove information such as join date and applied roles.
async fn soft_delete_member(&self, id: &MemberCompositeKey) -> Result<()>;
/// Forcibly delete a server member by their id.
/// This will cancel any pending timeouts or other longer term actions, and they will not be reapplied on rejoin.
async fn force_delete_member(&self, id: &MemberCompositeKey) -> Result<()>;
/// Fetch all members who have been marked for deletion.
async fn remove_dangling_members(&self) -> Result<()>;
}

107
crates/core/database/src/models/server_members/ops/mongodb.rs
View File

@@ -1,4 +1,6 @@
use bson::Document;
use futures::StreamExt;
use iso8601_timestamp::Timestamp;
use mongodb::options::ReadConcern;
use revolt_result::Result;
@@ -11,9 +13,42 @@ static COL: &str = "server_members";
#[async_trait]
impl AbstractServerMembers for MongoDb {
/// Insert a new server member into the database
async fn insert_member(&self, member: &Member) -> Result<()> {
query!(self, insert_one, COL, &member).map(|_| ())
/// Insert a new server member (or use the existing member if one is found)
async fn insert_or_merge_member(&self, member: &Member) -> Result<Option<Member>> {
let existing: Result<Option<Document>> = query!(
self,
find_one,
COL,
doc! {
"_id.server": &member.id.server,
"_id.user": &member.id.user,
"pending_deletion_at": {"$exists": true}
}
);
// Update the existing record if it exist, otherwise make a new record
if existing.is_ok_and(|x| x.is_some()) {
self.col::<Member>(COL)
.find_one_and_update(
doc! {
"_id.server": &member.id.server,
"_id.user": &member.id.user,
},
doc! {
"$set": {
"joined_at": member.joined_at.duration_since(Timestamp::UNIX_EPOCH).whole_seconds(),
},
"$unset": {
"pending_deletion_at": ""
}
},
)
.return_document(mongodb::options::ReturnDocument::After)
.await
.map_err(|_| create_database_error!("update_one", COL))
} else {
query!(self, insert_one, COL, &member).map(|_| ())?;
Ok(None)
}
}
/// Fetch a server member by their id
@@ -24,7 +59,8 @@ impl AbstractServerMembers for MongoDb {
COL,
doc! {
"_id.server": server_id,
"_id.user": user_id
"_id.user": user_id,
"pending_deletion_at": {"$exists": false}
}
)?
.ok_or_else(|| create_error!(NotFound))
@@ -35,7 +71,8 @@ impl AbstractServerMembers for MongoDb {
Ok(self
.col::<Member>(COL)
.find(doc! {
"_id.server": server_id
"_id.server": server_id,
"pending_deletion_at": {"$exists": false}
})
.await
.map_err(|_| create_database_error!("find", COL))?
@@ -143,7 +180,8 @@ impl AbstractServerMembers for MongoDb {
Ok(self
.col::<Member>(COL)
.find(doc! {
"_id.user": user_id
"_id.user": user_id,
"pending_deletion_at": {"$exists": false}
})
.await
.map_err(|_| create_database_error!("find", COL))?
@@ -164,6 +202,7 @@ impl AbstractServerMembers for MongoDb {
.col::<Member>(COL)
.find(doc! {
"_id.server": server_id,
"pending_deletion_at": {"$exists": false},
"_id.user": {
"$in": ids
}
@@ -185,7 +224,8 @@ impl AbstractServerMembers for MongoDb {
async fn fetch_member_count(&self, server_id: &str) -> Result<usize> {
self.col::<Member>(COL)
.count_documents(doc! {
"_id.server": server_id
"_id.server": server_id,
"pending_deletion_at": {"$exists": false}
})
.await
.map(|c| c as usize)
@@ -196,7 +236,8 @@ impl AbstractServerMembers for MongoDb {
async fn fetch_server_count(&self, user_id: &str) -> Result<usize> {
self.col::<Member>(COL)
.count_documents(doc! {
"_id.user": user_id
"_id.user": user_id,
"pending_deletion_at": {"$exists": false}
})
.await
.map(|c| c as usize)
@@ -225,8 +266,42 @@ impl AbstractServerMembers for MongoDb {
.map(|_| ())
}
/// Marks a member for deletion.
/// This will remove the record if the user has no pending actions (eg. timeout),
/// otherwise will slate the record for deletion by revolt_crond once the actions expire.
async fn soft_delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
let member = self.fetch_member(&id.server, &id.user).await;
if let Ok(member) = member {
if member.in_timeout() {
self.col::<Document>(COL)
.update_many(
doc! {
"_id.server": &id.server,
"_id.user": &id.user,
},
doc! {
"$set": {"pending_deletion_at": format!("{}", member.timeout.unwrap().format())},
"$unset": {
"joined_at": "",
"avatar": "",
"nickname": "",
"roles": ""
}
},
)
.await
.map(|_| ())
.map_err(|_| create_database_error!("update_many", COL))
} else {
self.force_delete_member(id).await
}
} else {
Err(create_database_error!("fetch_member", COL))
}
}
/// Delete a server member by their id
async fn delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
async fn force_delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
query!(
self,
delete_one,
@@ -238,11 +313,25 @@ impl AbstractServerMembers for MongoDb {
)
.map(|_| ())
}
async fn remove_dangling_members(&self) -> Result<()> {
let now = Timestamp::now_utc();
let date = bson::to_bson(&now).expect("Failed to serialize timestamp");
self.col::<Document>(COL)
.delete_many(doc! {
"pending_deletion_at": {"$lt": date}
})
.await
.map(|_| ())
.map_err(|_| create_database_error!("count_documents", COL))
}
}
impl IntoDocumentPath for FieldsMember {
fn as_path(&self) -> Option<&'static str> {
Some(match self {
FieldsMember::JoinedAt => "joined_at",
FieldsMember::Avatar => "avatar",
FieldsMember::Nickname => "nickname",
FieldsMember::Roles => "roles",

28
crates/core/database/src/models/server_members/ops/reference.rs
View File

@@ -8,13 +8,13 @@ use super::{AbstractServerMembers, ChunkedServerMembersGenerator};
#[async_trait]
impl AbstractServerMembers for ReferenceDb {
/// Insert a new server member into the database
async fn insert_member(&self, member: &Member) -> Result<()> {
async fn insert_or_merge_member(&self, member: &Member) -> Result<Option<Member>> {
let mut server_members = self.server_members.lock().await;
if server_members.contains_key(&member.id) {
Err(create_database_error!("insert", "member"))
} else {
server_members.insert(member.id.clone(), member.clone());
Ok(())
Ok(None)
}
}
@@ -169,8 +169,26 @@ impl AbstractServerMembers for ReferenceDb {
}
}
/// Soft delete a member
async fn soft_delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
let mut server_members = self.server_members.lock().await;
let member = server_members.get_mut(id);
if let Some(member) = member {
if member.in_timeout() {
panic!("Soft deletion is not implemented.")
} else if server_members.remove(id).is_some() {
Ok(())
} else {
Err(create_error!(NotFound))
}
} else {
Err(create_error!(NotFound))
}
}
/// Delete a server member by their id
async fn delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
async fn force_delete_member(&self, id: &MemberCompositeKey) -> Result<()> {
let mut server_members = self.server_members.lock().await;
if server_members.remove(id).is_some() {
Ok(())
@@ -178,4 +196,8 @@ impl AbstractServerMembers for ReferenceDb {
Err(create_error!(NotFound))
}
}
async fn remove_dangling_members(&self) -> Result<()> {
todo!()
}
}

2
crates/core/database/src/util/bridge/v0.rs
View File

@@ -707,6 +707,7 @@ impl From<crate::FieldsMember> for FieldsMember {
crate::FieldsMember::Nickname => FieldsMember::Nickname,
crate::FieldsMember::Roles => FieldsMember::Roles,
crate::FieldsMember::Timeout => FieldsMember::Timeout,
crate::FieldsMember::JoinedAt => FieldsMember::JoinedAt,
}
}
}
@@ -718,6 +719,7 @@ impl From<FieldsMember> for crate::FieldsMember {
FieldsMember::Nickname => crate::FieldsMember::Nickname,
FieldsMember::Roles => crate::FieldsMember::Roles,
FieldsMember::Timeout => crate::FieldsMember::Timeout,
FieldsMember::JoinedAt => crate::FieldsMember::JoinedAt,
}
}
}

4
crates/core/database/src/util/test_fixtures.rs
View File

@@ -39,7 +39,9 @@ pub async fn load_fixture(db: &Database, input: &str) -> HashMap<String, String>
LoadedFixture::User(user) => db.insert_user(&user).await.unwrap(),
LoadedFixture::Channel(channel) => db.insert_channel(&channel).await.unwrap(),
LoadedFixture::Server(server) => db.insert_server(&server).await.unwrap(),
LoadedFixture::ServerMember(member) => db.insert_member(&member).await.unwrap(),
LoadedFixture::ServerMember(member) => {
db.insert_or_merge_member(&member).await.unwrap();
}
}
}

1
crates/core/models/src/v0/server_members.rs
View File

@@ -77,6 +77,7 @@ auto_derived!(
Avatar,
Roles,
Timeout,
JoinedAt,
}
/// Member removal intention

5
crates/daemons/crond/src/main.rs
View File

@@ -1,7 +1,7 @@
use revolt_config::configure;
use revolt_database::DatabaseInfo;
use revolt_result::Result;
use tasks::{file_deletion, prune_dangling_files};
use tasks::{file_deletion, prune_dangling_files, prune_members};
use tokio::try_join;
pub mod tasks;
@@ -13,7 +13,8 @@ async fn main() -> Result<()> {
let db = DatabaseInfo::Auto.connect().await.expect("database");
try_join!(
file_deletion::task(db.clone()),
prune_dangling_files::task(db)
prune_dangling_files::task(db.clone()),
prune_members::task(db.clone())
)
.map(|_| ())
}

1
crates/daemons/crond/src/tasks/mod.rs
View File

@@ -1,2 +1,3 @@
pub mod file_deletion;
pub mod prune_dangling_files;
pub mod prune_members;

18
crates/daemons/crond/src/tasks/prune_members.rs Normal file
View File

@@ -0,0 +1,18 @@
use std::time::Duration;
use log::warn;
use revolt_database::Database;
use revolt_result::Result;
use tokio::time::sleep;
pub async fn task(db: Database) -> Result<()> {
loop {
let success = db.remove_dangling_members().await;
if let Err(s) = success {
revolt_config::capture_error(&s);
warn!("Failed to prune dangling members: {:?}", &s);
}
sleep(Duration::from_secs(90)).await;
}
}

3
crates/delta/src/util/ratelimiter.rs
View File

@@ -109,6 +109,9 @@ fn resolve_bucket<'r>(request: &'r rocket::Request<'_>) -> (&'r str, Option<&'r
};
if let Some(segment) = segment {
#[allow(clippy::redundant_locals)]
let resource = resource;
let method = request.method();
match (segment, resource, method) {
("users", target, Method::Patch) => ("user_edit", target),