diff --git a/crates/librqbit/src/storage/middleware/slow.rs b/crates/librqbit/src/storage/middleware/slow.rs index e8c8571bd..2a269ba95 100644 --- a/crates/librqbit/src/storage/middleware/slow.rs +++ b/crates/librqbit/src/storage/middleware/slow.rs @@ -12,6 +12,7 @@ use std::{ time::Duration, }; +use librqbit_core::lengths::ValidPieceIndex; use parking_lot::Mutex; use crate::{ @@ -128,4 +129,24 @@ impl TorrentStorage for SlowStorage { ) -> anyhow::Result<()> { self.underlying.init(shared, metadata) } + + fn on_piece_completed(&self, piece_index: ValidPieceIndex) -> anyhow::Result<()> { + self.underlying.on_piece_completed(piece_index) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::storage::test_util::{Probe, assert_forwards_defaults}; + + #[test] + fn test_the_defaulted_methods_reach_the_underlying_storage() { + let storage = SlowStorage { + underlying: Probe::default(), + pwrite_all_bufread: Mutex::new(Box::new(std::iter::empty())), + pread_exact_bufread: Mutex::new(Box::new(std::iter::empty())), + }; + assert_forwards_defaults(&storage, &storage.underlying); + } } diff --git a/crates/librqbit/src/storage/middleware/timing.rs b/crates/librqbit/src/storage/middleware/timing.rs index e71e490b9..2dca7deaa 100644 --- a/crates/librqbit/src/storage/middleware/timing.rs +++ b/crates/librqbit/src/storage/middleware/timing.rs @@ -2,6 +2,8 @@ A storage middleware that logs the time underlying storage operations took. */ +use librqbit_core::lengths::ValidPieceIndex; + use crate::{ ManagedTorrentShared, storage::{StorageFactory, StorageFactoryExt, TorrentStorage}, @@ -116,4 +118,23 @@ impl TorrentStorage for TimingStorage { ) -> anyhow::Result<()> { self.underlying.init(shared, metadata) } + + fn on_piece_completed(&self, piece_index: ValidPieceIndex) -> anyhow::Result<()> { + self.underlying.on_piece_completed(piece_index) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::storage::test_util::{Probe, assert_forwards_defaults}; + + #[test] + fn test_the_defaulted_methods_reach_the_underlying_storage() { + let storage = TimingStorage { + name: "test".to_owned(), + underlying: Probe::default(), + }; + assert_forwards_defaults(&storage, &storage.underlying); + } } diff --git a/crates/librqbit/src/storage/middleware/write_through_cache.rs b/crates/librqbit/src/storage/middleware/write_through_cache.rs index 1dd0e4f7f..0ec10a56b 100644 --- a/crates/librqbit/src/storage/middleware/write_through_cache.rs +++ b/crates/librqbit/src/storage/middleware/write_through_cache.rs @@ -137,4 +137,33 @@ impl TorrentStorage for WriteThroughCacheStorage { ) -> anyhow::Result<()> { self.underlying.init(shared, metadata) } + + fn on_piece_completed(&self, piece_index: ValidPieceIndex) -> anyhow::Result<()> { + self.underlying.on_piece_completed(piece_index) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::storage::test_util::{ + NUM_PIECES, PIECE_LEN, Probe, assert_forwards_defaults, lengths, + }; + + #[test] + fn test_the_defaulted_methods_reach_the_underlying_storage() { + let storage = WriteThroughCacheStorage { + lru: RwLock::new(LruCache::new(NonZeroUsize::new(1).unwrap())), + lengths: lengths(), + file_infos: vec![crate::file_info::FileInfo { + relative_filename: "test.dat".into(), + offset_in_torrent: 0, + len: PIECE_LEN as u64 * NUM_PIECES as u64, + piece_range: 0..NUM_PIECES, + attrs: Default::default(), + }], + underlying: Probe::default(), + }; + assert_forwards_defaults(&storage, &storage.underlying); + } } diff --git a/crates/librqbit/src/storage/mod.rs b/crates/librqbit/src/storage/mod.rs index 0ef696f53..53030fb93 100644 --- a/crates/librqbit/src/storage/mod.rs +++ b/crates/librqbit/src/storage/mod.rs @@ -30,6 +30,9 @@ pub mod examples; #[cfg(feature = "storage_middleware")] pub mod middleware; +#[cfg(test)] +pub(crate) mod test_util; + use std::{ any::{Any, TypeId}, io::IoSlice, @@ -222,7 +225,9 @@ mod tests { use std::any::TypeId; use super::{ - BoxStorageFactory, StorageFactory, StorageFactoryExt, filesystem::FilesystemStorageFactory, + BoxStorageFactory, StorageFactory, StorageFactoryExt, + filesystem::FilesystemStorageFactory, + test_util::{Probe, assert_forwards_defaults}, }; use crate::torrent_state::{ManagedTorrentShared, TorrentMetadata}; @@ -279,4 +284,12 @@ mod tests { .is_type_id(TypeId::of::>()) ); } + + // What a torrent holds is a Box, so a method this impl doesn't + // forward is one no storage in rqbit ever gets asked. + #[test] + fn test_the_defaulted_methods_survive_boxing() { + let boxed: Box = Box::default(); + assert_forwards_defaults(&boxed, &boxed); + } } diff --git a/crates/librqbit/src/storage/test_util.rs b/crates/librqbit/src/storage/test_util.rs new file mode 100644 index 000000000..159aa4f21 --- /dev/null +++ b/crates/librqbit/src/storage/test_util.rs @@ -0,0 +1,84 @@ +// A storage that records what reached it, to check the wrappers around one against. +// +// A middleware is transparent: anything it has no opinion about has to reach what it +// wraps. The methods that make that easy to get wrong are the ones [`TorrentStorage`] +// gives a default, because forgetting one still compiles and then answers something +// plausible of its own instead of asking the storage underneath. + +use std::path::Path; + +use librqbit_core::{ + constants::CHUNK_SIZE, + lengths::{Lengths, ValidPieceIndex}, +}; +use parking_lot::Mutex; + +use crate::{ManagedTorrentShared, TorrentMetadata, storage::TorrentStorage}; + +pub(crate) const PIECE_LEN: u32 = CHUNK_SIZE * 2; +pub(crate) const NUM_PIECES: u32 = 2; + +pub(crate) fn lengths() -> Lengths { + Lengths::new(PIECE_LEN as u64 * NUM_PIECES as u64, PIECE_LEN).unwrap() +} + +pub(crate) fn piece(id: u32) -> ValidPieceIndex { + lengths().validate_piece_index(id).unwrap() +} + +#[derive(Default)] +pub(crate) struct Probe { + /// The pieces on_piece_completed() was called for, in order. + pub completed: Mutex>, +} + +impl TorrentStorage for Probe { + fn init( + &mut self, + _shared: &ManagedTorrentShared, + _metadata: &TorrentMetadata, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn pread_exact(&self, _file_id: usize, _offset: u64, _buf: &mut [u8]) -> anyhow::Result<()> { + Ok(()) + } + + fn pwrite_all(&self, _file_id: usize, _offset: u64, _buf: &[u8]) -> anyhow::Result<()> { + Ok(()) + } + + fn remove_file(&self, _file_id: usize, _filename: &Path) -> anyhow::Result<()> { + Ok(()) + } + + fn remove_directory_if_empty(&self, _path: &Path) -> anyhow::Result<()> { + Ok(()) + } + + fn ensure_file_length(&self, _file_id: usize, _length: u64) -> anyhow::Result<()> { + Ok(()) + } + + fn take(&self) -> anyhow::Result> { + anyhow::bail!("not used") + } + + fn on_piece_completed(&self, piece_index: ValidPieceIndex) -> anyhow::Result<()> { + self.completed.lock().push(piece_index.get()); + Ok(()) + } +} + +// Everything a wrapper has to pass through to the storage it wraps and can't decide on +// its own. Called from each middleware's own tests, where its private fields are. +pub(crate) fn assert_forwards_defaults(storage: &S, probe: &Probe) { + storage.on_piece_completed(piece(1)).unwrap(); + assert_eq!( + *probe.completed.lock(), + vec![1], + "on_piece_completed() didn't reach the storage: a store that makes a piece \ + visible there never gets told" + ); +}