Skip to content

Commit

Permalink
chathistory: Fix target of incoming DMs
Browse files Browse the repository at this point in the history
With the recent changes to the HistoryService interface, the target of incoming DMs
was rewritten to be the sender instead of the recipient.
  • Loading branch information
progval authored and spb committed Nov 23, 2024
1 parent aee6309 commit 0576ef4
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 8 deletions.
2 changes: 1 addition & 1 deletion sable_history/src/pg_history_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ fn make_historical_event(
source: format!("{}!{}@{}", source_nick, source_ident, source_vhost),
source_account,
message_type: message_type.into(),
target: channel.name.clone(), // assume it's the same
target: Some(channel.name.clone()), // assume it's the same
text,
}
}
32 changes: 30 additions & 2 deletions sable_ircd/src/command/handlers/chathistory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ async fn list_targets<'a>(
}

fn send_history_entries(
_server: &ClientServer,
server: &ClientServer,
conn: impl MessageSink,
target: &str,
entries: impl IntoIterator<Item = HistoricalEvent>,
Expand All @@ -221,10 +221,38 @@ fn send_history_entries(
timestamp,
source,
source_account,
target: _, // assume it's the same as the one we got as parameter
target,
message_type,
text,
} => {
let target = match target {
None => {
// DM sent by the user requesting history
let Some(user_id) = conn.user_id() else {
return Err(CommandError::Fail {
command: "CHATHISTORY",
code: "MESSAGE_ERROR",
context: "".to_owned(),
description: "Could not format chathistory for non-user".to_owned(),
});
};
server
.network()
.user(user_id)
.map_err(|e| CommandError::Fail {
command: "CHATHISTORY",
code: "MESSAGE_ERROR",
context: "".to_owned(),
description: e.to_string(),
})?
.nick()
.to_string()
}
Some(target) => {
// Not a DM, or not sent by the user requesting history
target
}
};
let msg = message::Message::new(&source, &target, message_type, &text)
.with_tag(server_time::server_time_tag(timestamp))
.with_tag(OutboundMessageTag::new(
Expand Down
33 changes: 30 additions & 3 deletions sable_network/src/history/local_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::HashMap;
use tracing::instrument;

use crate::network::state::HistoricMessageTargetId;
use crate::network::wrapper::MessageTarget;
use crate::prelude::*;

/// Helper to extract the target name for chathistory purposes from a given event.
Expand Down Expand Up @@ -113,13 +114,17 @@ impl<'a, NetworkPolicy: policy::PolicyService> LocalHistoryService<'a, NetworkPo
.into_iter()
.rev()
.chain(forward_entries)
.flat_map(move |entry| Self::translate_log_entry(entry, &net)))
.flat_map(move |entry| Self::translate_log_entry(entry, &net, source)))
} else {
Err(HistoryError::InvalidTarget(target))
}
}

fn translate_log_entry(entry: HistoryLogEntry, net: &Network) -> Option<HistoricalEvent> {
fn translate_log_entry(
entry: HistoryLogEntry,
net: &Network,
history_request_source: UserId,
) -> Option<HistoricalEvent> {
match entry.details {
NetworkStateChange::NewMessage(update::NewMessage {
message,
Expand All @@ -129,14 +134,36 @@ impl<'a, NetworkPolicy: policy::PolicyService> LocalHistoryService<'a, NetworkPo
let message = net.message(message).ok()?;
let source = message.source().ok()?;
let target = message.target().ok()?;
tracing::error!(
"requested by {:?}, source: {:?}, target: {}",
history_request_source,
source,
target
);
let target = if let MessageTarget::User(target_user) = &target {
tracing::error!("target: {:?}", target_user.id());
if target_user.id() == history_request_source {
tracing::error!("equal");
// This is a DM, and the message was sent by the user this history item will be sent to,
// so the the target needs to be rewritten
None
} else {
tracing::error!("not equal");
// This is a DM, and the message was sent to the user this history item will be sent to
Some(target.to_string())
}
} else {
// Not a DM
Some(target.to_string())
};

Some(HistoricalEvent::Message {
id: message.id(),
timestamp: entry.timestamp(), // update's timestamp, may differ from the message's timestamp
message_type: message.message_type(),
source: source.nuh(),
source_account: source.account_name().map(|n| n.to_string()),
target: target.to_string(),
target,
text: message.text().to_string(),
})
}
Expand Down
3 changes: 2 additions & 1 deletion sable_network/src/history/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ pub enum HistoricalEvent {
timestamp: i64,
source: String,
source_account: Option<String>,
target: String,
/// If `None`, it should be replaced by the recipient's current nick
target: Option<String>,
message_type: MessageType,
text: String,
},
Expand Down
2 changes: 1 addition & 1 deletion sable_network/src/network/wrapper/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl MessageTarget<'_> {
impl std::fmt::Display for MessageTarget<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::User(u) => u.nuh().fmt(f),
Self::User(u) => u.nick().fmt(f),
Self::Channel(c) => c.name().fmt(f),
}
}
Expand Down
1 change: 1 addition & 0 deletions sable_network/src/network/wrapper/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::*;
use crate::prelude::*;

/// A wrapper around a [`state::User`]
#[derive(Debug)]
pub struct User<'a> {
network: &'a Network,
data: &'a state::User,
Expand Down

0 comments on commit 0576ef4

Please sign in to comment.