feat(lib): carry audio attributes in browse and folder listings
A browse listed each file as a name and a size, so a searcher who saw a bitrate in a search reply lost it the moment they opened the sharer's files. Listings (codes 5 and 37) now carry each file's attributes the way Nicotine+ sends them, as a SharedFileEntry with name, size and attributes in place of the (name, size) pair; the CLI's browse record gains bitrate and duration, and the daemon protocol moves to version 2 for the new listing shape.
This commit is contained in:
@@ -30,12 +30,24 @@ use std::time::{Duration, Instant};
|
||||
|
||||
use soulseek_rs::message::Message;
|
||||
use soulseek_rs::message::peer::{
|
||||
FileEntry, SharedDirectory, build_file_search_response,
|
||||
FileEntry, SharedDirectory, SharedFileEntry, build_file_search_response,
|
||||
build_shared_file_list,
|
||||
};
|
||||
use soulseek_rs::message::server::MessageFactory;
|
||||
use soulseek_rs::{Client, ClientSettings, ClientVersion, PeerAddress};
|
||||
|
||||
/// A mock's `(name, size)` files in listing form; mocks advertise no attributes.
|
||||
fn entries(files: &[(String, u64)]) -> Vec<SharedFileEntry> {
|
||||
files
|
||||
.iter()
|
||||
.map(|(name, size)| SharedFileEntry {
|
||||
name: name.clone(),
|
||||
size: *size,
|
||||
attributes: Vec::new(),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Files matching this tag are what every mock seeder answers searches for.
|
||||
const TAG: &str = "zqxstress";
|
||||
|
||||
@@ -536,7 +548,7 @@ impl SeederState {
|
||||
fn send_listing(&self, p: &mut TcpStream, shared: bool) -> bool {
|
||||
let dir = SharedDirectory {
|
||||
name: format!("stress\\{}", self.name),
|
||||
files: self.files.clone(),
|
||||
files: entries(&self.files),
|
||||
};
|
||||
let list = build_shared_file_list(std::slice::from_ref(&dir));
|
||||
self.reply(p, shared, &list.get_buffer())
|
||||
|
||||
@@ -100,11 +100,10 @@ fn tick_reaps_a_peer_idle_past_the_deadline() {
|
||||
/// ticks proves the actor drains the socket rather than sipping 1 KiB a tick.
|
||||
fn big_listing() -> Vec<u8> {
|
||||
let files = (0..400u64)
|
||||
.map(|i| {
|
||||
(
|
||||
format!("track-{i:03}-{}.flac", "x".repeat(i as usize % 17)),
|
||||
i,
|
||||
)
|
||||
.map(|i| crate::message::peer::SharedFileEntry {
|
||||
name: format!("track-{i:03}-{}.flac", "x".repeat(i as usize % 17)),
|
||||
size: i,
|
||||
attributes: Vec::new(),
|
||||
})
|
||||
.collect();
|
||||
let dir = crate::message::peer::SharedDirectory {
|
||||
|
||||
@@ -763,7 +763,7 @@ impl Client {
|
||||
&requester_key,
|
||||
|ctx| {
|
||||
crate::message::peer::build_shared_file_list(
|
||||
&shared_directories(ctx),
|
||||
&ctx.shares.directories(),
|
||||
)
|
||||
},
|
||||
);
|
||||
@@ -792,7 +792,9 @@ impl Client {
|
||||
&client_context,
|
||||
&requester_key,
|
||||
|ctx| {
|
||||
let dirs: Vec<_> = shared_directories(ctx)
|
||||
let dirs: Vec<_> = ctx
|
||||
.shares
|
||||
.directories()
|
||||
.into_iter()
|
||||
.filter(|dir| dir.name == folder)
|
||||
.collect();
|
||||
@@ -896,17 +898,3 @@ impl Client {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Our shares in the wire form a browse or folder reply carries.
|
||||
fn shared_directories(
|
||||
ctx: &ClientContext,
|
||||
) -> Vec<crate::message::peer::SharedDirectory> {
|
||||
ctx.shares
|
||||
.directories()
|
||||
.into_iter()
|
||||
.map(|(name, files)| crate::message::peer::SharedDirectory {
|
||||
name,
|
||||
files,
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -30,7 +30,7 @@ pub mod prelude {
|
||||
pub use actor::server_actor::{PeerAddress, UserMessage};
|
||||
pub use client::{Client, ClientSettings};
|
||||
pub use error::{Result, SoulseekRs};
|
||||
pub use message::peer::SharedDirectory;
|
||||
pub use message::peer::{SharedDirectory, SharedFileEntry};
|
||||
pub use types::{
|
||||
ClientVersion, DownloadStatus, File, RoomEvent, RoomInfo, Search,
|
||||
SearchResult, SessionLoss, Transfer, UploadInfo, UploadStatus, UserInfo,
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
//! folder" in other clients asks for.
|
||||
|
||||
use crate::message::peer::SharedDirectory;
|
||||
#[cfg(test)]
|
||||
use crate::message::peer::SharedFileEntry;
|
||||
use crate::message::peer::shared_file_list::{
|
||||
decompress_body, read_directories, write_directories,
|
||||
};
|
||||
@@ -56,7 +58,18 @@ pub fn parse_folder_contents(
|
||||
fn a_folder_listing_round_trips() {
|
||||
let dirs = vec![SharedDirectory {
|
||||
name: "music\\album".to_string(),
|
||||
files: vec![("one.flac".to_string(), 40), ("two.flac".to_string(), 50)],
|
||||
files: vec![
|
||||
SharedFileEntry {
|
||||
name: "one.flac".to_string(),
|
||||
size: 40,
|
||||
attributes: vec![(0, 992)],
|
||||
},
|
||||
SharedFileEntry {
|
||||
name: "two.flac".to_string(),
|
||||
size: 50,
|
||||
attributes: Vec::new(),
|
||||
},
|
||||
],
|
||||
}];
|
||||
let built = build_folder_contents(7, "music\\album", &dirs);
|
||||
let mut message = Message::new_with_data(built.get_data());
|
||||
|
||||
@@ -25,8 +25,8 @@ pub use place_in_queue_request::PlaceInQueueRequest;
|
||||
pub use place_in_queue_response::PlaceInQueueResponse;
|
||||
pub use queue_upload::QueueUploadHandler;
|
||||
pub use shared_file_list::{
|
||||
SharedDirectory, SharedFileListResponseHandler, build_shared_file_list,
|
||||
parse_shared_file_list,
|
||||
SharedDirectory, SharedFileEntry, SharedFileListResponseHandler,
|
||||
build_shared_file_list, parse_shared_file_list,
|
||||
};
|
||||
pub use transfer_request::TransferRequest;
|
||||
pub use transfer_response::TransferResponse;
|
||||
|
||||
@@ -7,11 +7,32 @@ use crate::peer::PeerMessage;
|
||||
use crate::utils::zlib::{deflate, inflate};
|
||||
use std::sync::mpsc::Sender;
|
||||
|
||||
/// One shared directory and the files directly in it (basename + size).
|
||||
/// One file in a shared directory: its basename, size and the `(code, value)`
|
||||
/// audio attributes the owner advertises for it.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SharedFileEntry {
|
||||
pub name: String,
|
||||
pub size: u64,
|
||||
pub attributes: Vec<(u32, u32)>,
|
||||
}
|
||||
|
||||
impl SharedFileEntry {
|
||||
/// The value of attribute `code` (0 bitrate, 1 duration, 2 VBR, 4 sample
|
||||
/// rate, 5 bit depth), if the owner advertised it.
|
||||
#[must_use]
|
||||
pub fn attribute(&self, code: u32) -> Option<u32> {
|
||||
self.attributes
|
||||
.iter()
|
||||
.find(|&&(c, _)| c == code)
|
||||
.map(|&(_, v)| v)
|
||||
}
|
||||
}
|
||||
|
||||
/// One shared directory and the files directly in it.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SharedDirectory {
|
||||
pub name: String,
|
||||
pub files: Vec<(String, u64)>,
|
||||
pub files: Vec<SharedFileEntry>,
|
||||
}
|
||||
|
||||
/// Receives a peer's `SharedFileListResponse` (peer code 5) when browsing them.
|
||||
@@ -58,13 +79,16 @@ pub fn write_directories(payload: &mut Message, dirs: &[SharedDirectory]) {
|
||||
payload
|
||||
.write_string(&dir.name)
|
||||
.write_int32(dir.files.len() as u32);
|
||||
for (name, size) in &dir.files {
|
||||
for file in &dir.files {
|
||||
payload
|
||||
.write_int8(1)
|
||||
.write_string(name)
|
||||
.write_int64(*size)
|
||||
.write_string(&file.name)
|
||||
.write_int64(file.size)
|
||||
.write_string("") // extension
|
||||
.write_int32(0); // attribute count
|
||||
.write_int32(file.attributes.len() as u32);
|
||||
for &(code, value) in &file.attributes {
|
||||
payload.write_int32(code).write_int32(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -87,20 +111,24 @@ pub fn read_directories(body: &mut Message) -> Vec<SharedDirectory> {
|
||||
break;
|
||||
}
|
||||
body.read_int8(); // code
|
||||
let filename = body.read_string();
|
||||
let file_size = body.read_int64();
|
||||
let name = body.read_string();
|
||||
let size = body.read_int64();
|
||||
body.read_string(); // extension
|
||||
let attr_count = body.read_int32();
|
||||
let mut attributes = Vec::new();
|
||||
for _ in 0..attr_count {
|
||||
// Each attribute is two int32s (8 bytes); read_int32 does not
|
||||
// advance past the end, so bound the loop explicitly.
|
||||
if body.get_pointer() + 8 > body.get_size() {
|
||||
break;
|
||||
}
|
||||
body.read_int32();
|
||||
body.read_int32();
|
||||
attributes.push((body.read_int32(), body.read_int32()));
|
||||
}
|
||||
files.push((filename, file_size));
|
||||
files.push(SharedFileEntry {
|
||||
name,
|
||||
size,
|
||||
attributes,
|
||||
});
|
||||
}
|
||||
dirs.push(SharedDirectory { name, files });
|
||||
}
|
||||
@@ -127,19 +155,56 @@ fn hostile_dir_count_does_not_hang() {
|
||||
assert!(parse_shared_file_list(&mut message).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_listing_carries_each_files_attributes() {
|
||||
let dirs = vec![SharedDirectory {
|
||||
name: "music\\album".to_string(),
|
||||
files: vec![
|
||||
SharedFileEntry {
|
||||
name: "one.mp3".to_string(),
|
||||
size: 40,
|
||||
attributes: vec![(0, 320), (1, 187)],
|
||||
},
|
||||
SharedFileEntry {
|
||||
name: "two.flac".to_string(),
|
||||
size: 50,
|
||||
attributes: Vec::new(),
|
||||
},
|
||||
],
|
||||
}];
|
||||
let mut message = crate::message::framed(|m| {
|
||||
m.write_raw_bytes(
|
||||
build_shared_file_list(&dirs).get_data()[4..].to_vec(),
|
||||
);
|
||||
});
|
||||
assert_eq!(parse_shared_file_list(&mut message), dirs);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shared_file_list_roundtrips() {
|
||||
let dirs = vec![
|
||||
SharedDirectory {
|
||||
name: "music\\album".to_string(),
|
||||
files: vec![
|
||||
("song one.flac".to_string(), 123),
|
||||
("song two.flac".to_string(), 456),
|
||||
SharedFileEntry {
|
||||
name: "song one.flac".to_string(),
|
||||
size: 123,
|
||||
attributes: vec![(1, 300), (4, 44_100), (5, 16)],
|
||||
},
|
||||
SharedFileEntry {
|
||||
name: "song two.flac".to_string(),
|
||||
size: 456,
|
||||
attributes: Vec::new(),
|
||||
},
|
||||
],
|
||||
},
|
||||
SharedDirectory {
|
||||
name: "music".to_string(),
|
||||
files: vec![("top.mp3".to_string(), 789)],
|
||||
files: vec![SharedFileEntry {
|
||||
name: "top.mp3".to_string(),
|
||||
size: 789,
|
||||
attributes: Vec::new(),
|
||||
}],
|
||||
},
|
||||
];
|
||||
let message = build_shared_file_list(&dirs);
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
mod audio;
|
||||
|
||||
use crate::message::peer::{SharedDirectory, SharedFileEntry};
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::io;
|
||||
use std::path::{Path, PathBuf};
|
||||
@@ -129,12 +130,13 @@ impl Shares {
|
||||
}
|
||||
|
||||
/// Files grouped by their virtual directory (everything before the final
|
||||
/// backslash), as `(directory, [(basename, size)])`. Used to build a
|
||||
/// SharedFileListResponse.
|
||||
/// backslash), in the form a browse or folder listing carries.
|
||||
#[must_use]
|
||||
pub fn directories(&self) -> Vec<(String, Vec<(String, u64)>)> {
|
||||
let mut by_dir: std::collections::BTreeMap<String, Vec<(String, u64)>> =
|
||||
std::collections::BTreeMap::new();
|
||||
pub fn directories(&self) -> Vec<SharedDirectory> {
|
||||
let mut by_dir: std::collections::BTreeMap<
|
||||
String,
|
||||
Vec<SharedFileEntry>,
|
||||
> = std::collections::BTreeMap::new();
|
||||
for file in &self.files {
|
||||
let (dir, base) = file
|
||||
.virtual_path
|
||||
@@ -143,9 +145,16 @@ impl Shares {
|
||||
by_dir
|
||||
.entry(dir.to_string())
|
||||
.or_default()
|
||||
.push((base.to_string(), file.size));
|
||||
.push(SharedFileEntry {
|
||||
name: base.to_string(),
|
||||
size: file.size,
|
||||
attributes: file.attributes.clone(),
|
||||
});
|
||||
}
|
||||
by_dir.into_iter().collect()
|
||||
by_dir
|
||||
.into_iter()
|
||||
.map(|(name, files)| SharedDirectory { name, files })
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
|
||||
@@ -1838,7 +1838,7 @@ fn a_runtime_share_update_is_visible_to_browsers() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| d.files.iter().any(|(name, _)| name == "late.mp3")),
|
||||
.any(|d| d.files.iter().any(|f| f.name == "late.mp3")),
|
||||
"the listing should include the file shared at runtime"
|
||||
);
|
||||
|
||||
@@ -1888,7 +1888,7 @@ fn browse_a_peers_shared_files() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| { d.files.iter().any(|(name, _)| name == "track.flac") }),
|
||||
.any(|d| { d.files.iter().any(|f| f.name == "track.flac") }),
|
||||
"the listing should include the shared file"
|
||||
);
|
||||
|
||||
@@ -1949,7 +1949,7 @@ fn browse_a_firewalled_peer_via_broker() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| d.files.iter().any(|(name, _)| name == "hidden.flac")),
|
||||
.any(|d| d.files.iter().any(|f| f.name == "hidden.flac")),
|
||||
"the brokered listing should include the shared file"
|
||||
);
|
||||
|
||||
@@ -2019,7 +2019,7 @@ fn a_third_party_client_browses_our_shares_directly() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| d.files.iter().any(|(name, _)| name == "track.flac")),
|
||||
.any(|d| d.files.iter().any(|f| f.name == "track.flac")),
|
||||
"the listing should include the shared file, got {directories:?}"
|
||||
);
|
||||
|
||||
@@ -2127,7 +2127,7 @@ fn a_third_party_client_browses_our_shares_via_the_server_broker() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| d.files.iter().any(|(name, _)| name == "brokered.flac")),
|
||||
.any(|d| d.files.iter().any(|f| f.name == "brokered.flac")),
|
||||
"the brokered listing should include the shared file, got {directories:?}"
|
||||
);
|
||||
|
||||
@@ -2180,7 +2180,7 @@ fn a_stalled_peer_connection_does_not_wedge_the_listener() {
|
||||
assert!(
|
||||
directories
|
||||
.iter()
|
||||
.any(|d| d.files.iter().any(|(name, _)| name == "still.flac")),
|
||||
.any(|d| d.files.iter().any(|f| f.name == "still.flac")),
|
||||
"the listing should include the shared file, got {directories:?}"
|
||||
);
|
||||
|
||||
@@ -3255,7 +3255,7 @@ fn a_third_party_client_fetches_one_folder_of_our_shares() {
|
||||
assert_eq!(echoed, folder);
|
||||
let mut names: Vec<(String, Vec<String>)> = directories
|
||||
.into_iter()
|
||||
.map(|d| (d.name, d.files.into_iter().map(|(name, _)| name).collect()))
|
||||
.map(|d| (d.name, d.files.into_iter().map(|f| f.name).collect()))
|
||||
.collect();
|
||||
for (_, files) in &mut names {
|
||||
files.sort();
|
||||
@@ -3572,3 +3572,48 @@ fn a_shared_mp3_advertises_its_bitrate_and_duration() {
|
||||
|
||||
let _ = std::fs::remove_dir_all(share_dir);
|
||||
}
|
||||
|
||||
// A browse shows what a search shows: the bitrate and duration a searcher
|
||||
// filters on are in the listing too, as Nicotine+ sends them.
|
||||
#[test]
|
||||
fn a_browse_listing_carries_the_files_attributes() {
|
||||
let server = server_or_skip!();
|
||||
|
||||
let share_dir = unique_download_dir();
|
||||
std::fs::write(share_dir.join("listed.mp3"), cbr_mp3(1000)).unwrap();
|
||||
let sharer_port = free_port().expect("sharer port");
|
||||
let mut sharer = Client::with_settings(ClientSettings {
|
||||
shared_directories: vec![share_dir.display().to_string()],
|
||||
..server.listening_settings("e2e_lattr_sharer", "pw", sharer_port)
|
||||
});
|
||||
sharer.connect().expect("sharer connect");
|
||||
assert!(sharer.login().expect("sharer login"));
|
||||
let server_addr = format!("{}:{}", server.host, server.port);
|
||||
let _qt = login_raw(&server_addr, "e2e_lattr_browser", "pw")
|
||||
.expect("third-party client logs in");
|
||||
|
||||
let directories = third_party_browse(
|
||||
&format!("127.0.0.1:{sharer_port}"),
|
||||
"e2e_lattr_browser",
|
||||
Duration::ZERO,
|
||||
Duration::from_secs(15),
|
||||
)
|
||||
.expect("a listing");
|
||||
let listed = directories
|
||||
.iter()
|
||||
.flat_map(|d| d.files.iter())
|
||||
.find(|f| f.name == "listed.mp3")
|
||||
.expect("the mp3 is listed");
|
||||
assert!(
|
||||
listed.attributes.contains(&(0, 128)),
|
||||
"{:?}",
|
||||
listed.attributes
|
||||
);
|
||||
assert!(
|
||||
listed.attributes.contains(&(1, 26)),
|
||||
"{:?}",
|
||||
listed.attributes
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_dir_all(share_dir);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user