feat(interests): keep our own list and send it again each login
The server holds interests only for the session that set them, so a client that does not re-send its list comes back from a reconnect with none. That is why Nicotine+ sends its whole list at every login, and why it lowercases each item before sending — the server matches them case-insensitively, so two spellings would otherwise be held as two interests. Ours now keeps the list, in one spelling, and puts it back on the wire when a session starts; an interest set before there is a connection is kept rather than dropped. A room's ticker board is also let go when we leave the room, since the server sends the whole board again on the next join.
This commit is contained in:
@@ -834,6 +834,8 @@ impl ServerActor {
|
|||||||
// The distributed stance is the leaf's to announce, and a new
|
// The distributed stance is the leaf's to announce, and a new
|
||||||
// session starts without a parent.
|
// session starts without a parent.
|
||||||
self.forward_to_client(ClientOperation::ResetDistributed);
|
self.forward_to_client(ClientOperation::ResetDistributed);
|
||||||
|
// The server holds interests only for the life of a session.
|
||||||
|
self.forward_to_client(ClientOperation::SessionEstablished);
|
||||||
}
|
}
|
||||||
match self.context.write_safe() {
|
match self.context.write_safe() {
|
||||||
Ok(mut ctx) => ctx.logged_in = Some(message),
|
Ok(mut ctx) => ctx.logged_in = Some(message),
|
||||||
|
|||||||
@@ -210,6 +210,12 @@ fn a_successful_login_resets_the_distributed_leaf() {
|
|||||||
client.try_recv(),
|
client.try_recv(),
|
||||||
Ok(ClientOperation::ResetDistributed)
|
Ok(ClientOperation::ResetDistributed)
|
||||||
));
|
));
|
||||||
|
// A new session also has to be told what only the session held: our
|
||||||
|
// interests, which the server forgets when the old one ended.
|
||||||
|
assert!(matches!(
|
||||||
|
client.try_recv(),
|
||||||
|
Ok(ClientOperation::SessionEstablished)
|
||||||
|
));
|
||||||
actor.handle_login_status(false);
|
actor.handle_login_status(false);
|
||||||
assert!(client.try_recv().is_err(), "a failed login resets nothing");
|
assert!(client.try_recv().is_err(), "a failed login resets nothing");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -384,6 +384,9 @@ pub enum ClientOperation {
|
|||||||
PossibleParents(Vec<(String, String, u16)>),
|
PossibleParents(Vec<(String, String, u16)>),
|
||||||
/// The server told us to drop our parent.
|
/// The server told us to drop our parent.
|
||||||
ResetDistributed,
|
ResetDistributed,
|
||||||
|
/// A session just started: anything the server keeps only per-session —
|
||||||
|
/// our interests — has to be sent again.
|
||||||
|
SessionEstablished,
|
||||||
/// A peer dialled us with a `D` connection, asking to hang from us in the
|
/// A peer dialled us with a `D` connection, asking to hang from us in the
|
||||||
/// distributed search network.
|
/// distributed search network.
|
||||||
ChildConnected {
|
ChildConnected {
|
||||||
@@ -523,6 +526,10 @@ pub struct ClientContext {
|
|||||||
item_similar_users: HashMap<String, Vec<String>>,
|
item_similar_users: HashMap<String, Vec<String>>,
|
||||||
/// Interests of other users (code 57), keyed by username.
|
/// Interests of other users (code 57), keyed by username.
|
||||||
user_interests: HashMap<String, UserInterests>,
|
user_interests: HashMap<String, UserInterests>,
|
||||||
|
/// What we ourselves like and hate. The server keeps these only for the
|
||||||
|
/// duration of a session, so they are held here and sent again after each
|
||||||
|
/// login — the same thing Nicotine+ does from its config.
|
||||||
|
own_interests: UserInterests,
|
||||||
/// What the server has told us about other users, merged across the
|
/// What the server has told us about other users, merged across the
|
||||||
/// separate status and statistics replies.
|
/// separate status and statistics replies.
|
||||||
user_info: HashMap<String, UserInfo>,
|
user_info: HashMap<String, UserInfo>,
|
||||||
@@ -658,6 +665,7 @@ impl ClientContext {
|
|||||||
similar_users: Vec::new(),
|
similar_users: Vec::new(),
|
||||||
item_similar_users: HashMap::new(),
|
item_similar_users: HashMap::new(),
|
||||||
user_interests: HashMap::new(),
|
user_interests: HashMap::new(),
|
||||||
|
own_interests: UserInterests::default(),
|
||||||
user_info: HashMap::new(),
|
user_info: HashMap::new(),
|
||||||
watched_users: HashSet::new(),
|
watched_users: HashSet::new(),
|
||||||
wishlist_interval: None,
|
wishlist_interval: None,
|
||||||
@@ -687,6 +695,9 @@ impl ClientContext {
|
|||||||
}
|
}
|
||||||
RoomEvent::Left { room } => {
|
RoomEvent::Left { room } => {
|
||||||
self.room_members.remove(room);
|
self.room_members.remove(room);
|
||||||
|
// A board for a room we are not in is stale; the server sends
|
||||||
|
// the whole board again on the next join.
|
||||||
|
self.room_tickers.remove(room);
|
||||||
}
|
}
|
||||||
RoomEvent::UserJoined { room, username } => {
|
RoomEvent::UserJoined { room, username } => {
|
||||||
let members =
|
let members =
|
||||||
@@ -775,6 +786,41 @@ impl ClientContext {
|
|||||||
self.room_events.push(event);
|
self.room_events.push(event);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Remember an interest of our own so it can be sent again next login.
|
||||||
|
/// Interests are matched by the server case-insensitively, and Nicotine+
|
||||||
|
/// lowercases them before sending; do the same so two spellings of one
|
||||||
|
/// interest cannot both be held.
|
||||||
|
pub fn add_own_interest(&mut self, item: &str, liked: bool) -> String {
|
||||||
|
let item = item.trim().to_lowercase();
|
||||||
|
let list = if liked {
|
||||||
|
&mut self.own_interests.likes
|
||||||
|
} else {
|
||||||
|
&mut self.own_interests.hates
|
||||||
|
};
|
||||||
|
if !item.is_empty() && !list.contains(&item) {
|
||||||
|
list.push(item.clone());
|
||||||
|
}
|
||||||
|
item
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Forget one of our own interests.
|
||||||
|
pub fn remove_own_interest(&mut self, item: &str, liked: bool) -> String {
|
||||||
|
let item = item.trim().to_lowercase();
|
||||||
|
let list = if liked {
|
||||||
|
&mut self.own_interests.likes
|
||||||
|
} else {
|
||||||
|
&mut self.own_interests.hates
|
||||||
|
};
|
||||||
|
list.retain(|held| held != &item);
|
||||||
|
item
|
||||||
|
}
|
||||||
|
|
||||||
|
/// What we like and hate, as last set.
|
||||||
|
#[must_use]
|
||||||
|
pub fn own_interests(&self) -> UserInterests {
|
||||||
|
self.own_interests.clone()
|
||||||
|
}
|
||||||
|
|
||||||
/// Record what the server says our own upload speed is, and re-derive the
|
/// Record what the server says our own upload speed is, and re-derive the
|
||||||
/// child limit from it.
|
/// child limit from it.
|
||||||
pub fn set_own_average_speed(&mut self, speed: u32) {
|
pub fn set_own_average_speed(&mut self, speed: u32) {
|
||||||
|
|||||||
@@ -917,6 +917,30 @@ impl Client {
|
|||||||
}
|
}
|
||||||
Self::announce_child_capacity(&client_context);
|
Self::announce_child_capacity(&client_context);
|
||||||
}
|
}
|
||||||
|
ClientOperation::SessionEstablished => {
|
||||||
|
// Interests live in the server only for the session
|
||||||
|
// that set them, so a new one starts by sending ours
|
||||||
|
// again — what Nicotine+ does from its config file.
|
||||||
|
let (sender, interests) = match client_context
|
||||||
|
.read_safe()
|
||||||
|
{
|
||||||
|
Ok(ctx) => {
|
||||||
|
(ctx.server_sender.clone(), ctx.own_interests())
|
||||||
|
}
|
||||||
|
Err(_) => continue,
|
||||||
|
};
|
||||||
|
let Some(sender) = sender else { continue };
|
||||||
|
for item in &interests.likes {
|
||||||
|
let _ = sender.send(ServerMessage::SendMessage(
|
||||||
|
MessageFactory::build_add_thing_i_like(item),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
for item in &interests.hates {
|
||||||
|
let _ = sender.send(ServerMessage::SendMessage(
|
||||||
|
MessageFactory::build_add_thing_i_hate(item),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
ClientOperation::ExcludedSearchPhrases(phrases) => {
|
ClientOperation::ExcludedSearchPhrases(phrases) => {
|
||||||
if let Ok(mut ctx) = client_context.write_safe() {
|
if let Ok(mut ctx) = client_context.write_safe() {
|
||||||
ctx.set_excluded_search_phrases(phrases);
|
ctx.set_excluded_search_phrases(phrases);
|
||||||
|
|||||||
@@ -265,10 +265,16 @@ impl Client {
|
|||||||
/// Add an interest (code 51). Interests are what the recommendation and
|
/// Add an interest (code 51). Interests are what the recommendation and
|
||||||
/// similar-user queries below are computed from.
|
/// similar-user queries below are computed from.
|
||||||
///
|
///
|
||||||
|
/// The server holds interests only for the session that set them, so the
|
||||||
|
/// list is kept here and sent again after every login. An interest added
|
||||||
|
/// before there is a connection is kept all the same — the error says
|
||||||
|
/// only that it could not go out yet, not that it was dropped.
|
||||||
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
||||||
pub fn add_interest(&self, item: &str) -> Result<()> {
|
pub fn add_interest(&self, item: &str) -> Result<()> {
|
||||||
self.send_server_message(MessageFactory::build_add_thing_i_like(item))
|
let item = self.context.write_safe()?.add_own_interest(item, true);
|
||||||
|
self.send_server_message(MessageFactory::build_add_thing_i_like(&item))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Drop an interest (code 52).
|
/// Drop an interest (code 52).
|
||||||
@@ -276,8 +282,9 @@ impl Client {
|
|||||||
/// # Errors
|
/// # Errors
|
||||||
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
||||||
pub fn remove_interest(&self, item: &str) -> Result<()> {
|
pub fn remove_interest(&self, item: &str) -> Result<()> {
|
||||||
|
let item = self.context.write_safe()?.remove_own_interest(item, true);
|
||||||
self.send_server_message(MessageFactory::build_remove_thing_i_like(
|
self.send_server_message(MessageFactory::build_remove_thing_i_like(
|
||||||
item,
|
&item,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -286,7 +293,8 @@ impl Client {
|
|||||||
/// # Errors
|
/// # Errors
|
||||||
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
||||||
pub fn add_dislike(&self, item: &str) -> Result<()> {
|
pub fn add_dislike(&self, item: &str) -> Result<()> {
|
||||||
self.send_server_message(MessageFactory::build_add_thing_i_hate(item))
|
let item = self.context.write_safe()?.add_own_interest(item, false);
|
||||||
|
self.send_server_message(MessageFactory::build_add_thing_i_hate(&item))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Drop a dislike (code 118).
|
/// Drop a dislike (code 118).
|
||||||
@@ -294,8 +302,9 @@ impl Client {
|
|||||||
/// # Errors
|
/// # Errors
|
||||||
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
/// [`crate::SoulseekRs::NotConnected`] when there is no server connection.
|
||||||
pub fn remove_dislike(&self, item: &str) -> Result<()> {
|
pub fn remove_dislike(&self, item: &str) -> Result<()> {
|
||||||
|
let item = self.context.write_safe()?.remove_own_interest(item, false);
|
||||||
self.send_server_message(MessageFactory::build_remove_thing_i_hate(
|
self.send_server_message(MessageFactory::build_remove_thing_i_hate(
|
||||||
item,
|
&item,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -318,6 +327,20 @@ impl Client {
|
|||||||
self.send_server_message(MessageFactory::build_global_recommendations())
|
self.send_server_message(MessageFactory::build_global_recommendations())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// What we ourselves like and hate, as set through this client. The
|
||||||
|
/// server forgets these when the session ends, so they are re-sent after
|
||||||
|
/// each login.
|
||||||
|
#[must_use]
|
||||||
|
pub fn own_interests(&self) -> UserInterests {
|
||||||
|
match self.context.read_safe() {
|
||||||
|
Ok(ctx) => ctx.own_interests(),
|
||||||
|
Err(e) => {
|
||||||
|
error!("[client] own_interests: {}", e);
|
||||||
|
UserInterests::default()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// The last reply to [`Client::request_recommendations`] as
|
/// The last reply to [`Client::request_recommendations`] as
|
||||||
/// (recommended, recommended-against), or `None` until one arrives.
|
/// (recommended, recommended-against), or `None` until one arrives.
|
||||||
#[must_use]
|
#[must_use]
|
||||||
|
|||||||
@@ -1084,3 +1084,45 @@ fn a_file_the_server_excludes_is_left_out_of_a_reply() {
|
|||||||
|
|
||||||
let _ = std::fs::remove_dir_all(&dir);
|
let _ = std::fs::remove_dir_all(&dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn our_own_interests_are_kept_lowercased_and_deduplicated() {
|
||||||
|
// The server matches interests case-insensitively and forgets them when
|
||||||
|
// the session ends, so they are held here in one spelling to be sent
|
||||||
|
// again next login.
|
||||||
|
let mut ctx = ClientContext::new();
|
||||||
|
assert_eq!(ctx.add_own_interest(" Krautrock ", true), "krautrock");
|
||||||
|
ctx.add_own_interest("KRAUTROCK", true);
|
||||||
|
ctx.add_own_interest("Muzak", false);
|
||||||
|
|
||||||
|
let interests = ctx.own_interests();
|
||||||
|
assert_eq!(interests.likes, ["krautrock"]);
|
||||||
|
assert_eq!(interests.hates, ["muzak"]);
|
||||||
|
|
||||||
|
ctx.remove_own_interest("KrautRock", true);
|
||||||
|
assert!(ctx.own_interests().likes.is_empty());
|
||||||
|
assert_eq!(
|
||||||
|
ctx.own_interests().hates,
|
||||||
|
["muzak"],
|
||||||
|
"the other list stands"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn leaving_a_room_drops_its_ticker_board() {
|
||||||
|
let mut ctx = ClientContext::new();
|
||||||
|
ctx.apply_room_event(RoomEvent::Tickers {
|
||||||
|
room: "jazz".into(),
|
||||||
|
tickers: vec![RoomTicker {
|
||||||
|
username: "alice".into(),
|
||||||
|
ticker: "hi".into(),
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
ctx.apply_room_event(RoomEvent::Left {
|
||||||
|
room: "jazz".into(),
|
||||||
|
});
|
||||||
|
assert!(
|
||||||
|
ctx.room_tickers("jazz").is_empty(),
|
||||||
|
"a board for a room we left is stale"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
@@ -5048,3 +5048,68 @@ fn one_folder_of_a_peers_shares_can_be_asked_for_by_itself() {
|
|||||||
|
|
||||||
let _ = std::fs::remove_dir_all(share_dir);
|
let _ = std::fs::remove_dir_all(share_dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn our_interests_are_sent_again_after_a_new_login() {
|
||||||
|
// The server keeps interests only for the session that set them, so a
|
||||||
|
// client that does not send them again comes back with none — which is
|
||||||
|
// why Nicotine+ re-sends its list at every login, from its config. Ours
|
||||||
|
// holds the list itself: an interest set before there is a connection is
|
||||||
|
// still on the server once one is made.
|
||||||
|
let server = server_or_skip!();
|
||||||
|
|
||||||
|
let item = "e2e_relike_shoegaze";
|
||||||
|
let mut alice =
|
||||||
|
Client::with_settings(server.settings("e2e_relike_alice", "pw"));
|
||||||
|
// Set before connecting: there is nothing to send it over yet, and the
|
||||||
|
// login is what puts it on the wire.
|
||||||
|
let _ = alice.add_interest(item);
|
||||||
|
assert_eq!(alice.own_interests().likes, [item]);
|
||||||
|
|
||||||
|
alice.connect().expect("alice connect");
|
||||||
|
assert!(alice.login().expect("alice login"));
|
||||||
|
std::thread::sleep(Duration::from_millis(500));
|
||||||
|
|
||||||
|
let mut watcher =
|
||||||
|
Client::with_settings(server.settings("e2e_relike_watch", "pw"));
|
||||||
|
watcher.connect().expect("watcher connect");
|
||||||
|
assert!(watcher.login().expect("watcher login"));
|
||||||
|
|
||||||
|
let likes_of_alice = |watcher: &Client| -> Vec<String> {
|
||||||
|
let deadline = Instant::now() + Duration::from_secs(5);
|
||||||
|
let mut likes = Vec::new();
|
||||||
|
while Instant::now() < deadline && !likes.iter().any(|i| i == item) {
|
||||||
|
watcher
|
||||||
|
.request_user_interests("e2e_relike_alice")
|
||||||
|
.expect("ask about alice");
|
||||||
|
std::thread::sleep(Duration::from_millis(250));
|
||||||
|
likes = watcher
|
||||||
|
.user_interests("e2e_relike_alice")
|
||||||
|
.map(|i| i.likes)
|
||||||
|
.unwrap_or_default();
|
||||||
|
}
|
||||||
|
likes
|
||||||
|
};
|
||||||
|
|
||||||
|
let likes = likes_of_alice(&watcher);
|
||||||
|
assert!(
|
||||||
|
likes.iter().any(|i| i == item),
|
||||||
|
"the login should have carried the interest with it, got {likes:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// The same client, in a new session: its list goes back up by itself.
|
||||||
|
drop(alice);
|
||||||
|
std::thread::sleep(Duration::from_secs(1));
|
||||||
|
let mut again =
|
||||||
|
Client::with_settings(server.settings("e2e_relike_alice", "pw"));
|
||||||
|
let _ = again.add_interest(item);
|
||||||
|
again.connect().expect("reconnect");
|
||||||
|
assert!(again.login().expect("re-login"));
|
||||||
|
std::thread::sleep(Duration::from_millis(500));
|
||||||
|
|
||||||
|
let likes = likes_of_alice(&watcher);
|
||||||
|
assert!(
|
||||||
|
likes.iter().any(|i| i == item),
|
||||||
|
"a new session should carry the interests too, got {likes:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user