Folders and files are now deleted more comprehensively

crates/librqbit/src/storage/examples/inmemory.rs

@@ -25,7 +25,7 @@ pub struct InMemoryExampleStorageFactory {}
 impl StorageFactory for InMemoryExampleStorageFactory {
     type Storage = InMemoryExampleStorage;
 
-    fn init_storage(
+    fn create(
         &self,
         info: &crate::torrent_state::ManagedTorrentInfo,
     ) -> anyhow::Result<InMemoryExampleStorage> {
@@ -110,4 +110,12 @@ impl TorrentStorage for InMemoryExampleStorage {
             file_infos: self.file_infos.clone(),
         }))
     }
+
+    fn init(&mut self, _meta: &crate::ManagedTorrentInfo) -> anyhow::Result<()> {
+        Ok(())
+    }
+
+    fn remove_directory_if_empty(&self, _path: &Path) -> anyhow::Result<()> {
+        Ok(())
+    }
 }

crates/librqbit/src/storage/examples/mmap.rs

@@ -18,7 +18,7 @@ pub struct MmapStorage {
 impl StorageFactory for MmapStorageFactory {
     type Storage = MmapStorage;
 
-    fn init_storage(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+    fn create(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
         Ok(MmapStorage {
             mmap: RwLock::new(
                 MmapOptions::new()
@@ -62,4 +62,12 @@ impl TorrentStorage for MmapStorage {
     fn take(&self) -> anyhow::Result<Box<dyn TorrentStorage>> {
         anyhow::bail!("not implemented")
     }
+
+    fn init(&mut self, _meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        Ok(())
+    }
+
+    fn remove_directory_if_empty(&self, _path: &std::path::Path) -> anyhow::Result<()> {
+        Ok(())
+    }
 }

crates/librqbit/src/storage/filesystem/fs.rs

@@ -4,6 +4,7 @@ use std::{
 };
 
 use anyhow::Context;
+use tracing::warn;
 
 use crate::{storage::StorageFactoryExt, torrent_state::ManagedTorrentInfo};
 
@@ -17,45 +18,10 @@ pub struct FilesystemStorageFactory {}
 impl StorageFactory for FilesystemStorageFactory {
     type Storage = FilesystemStorage;
 
-    fn init_storage(&self, meta: &ManagedTorrentInfo) -> anyhow::Result<FilesystemStorage> {
-        let mut files = Vec::<OpenedFile>::new();
-        let output_folder = &meta.options.output_folder;
-        for file_details in meta.info.iter_file_details(&meta.lengths)? {
-            let mut full_path = output_folder.clone();
-            let relative_path = file_details
-                .filename
-                .to_pathbuf()
-                .context("error converting file to path")?;
-            full_path.push(relative_path);
-
-            std::fs::create_dir_all(full_path.parent().context("bug: no parent")?)?;
-            let file = if meta.options.allow_overwrite {
-                OpenOptions::new()
-                    .create(true)
-                    .truncate(false)
-                    .read(true)
-                    .write(true)
-                    .open(&full_path)
-                    .with_context(|| format!("error opening {full_path:?} in read/write mode"))?
-            } else {
-                // create_new does not seem to work with read(true), so calling this twice.
-                OpenOptions::new()
-                    .create_new(true)
-                    .write(true)
-                    .open(&full_path)
-                    .with_context(|| {
-                        format!(
-                            "error creating a new file (because allow_overwrite = false) {:?}",
-                            &full_path
-                        )
-                    })?;
-                OpenOptions::new().read(true).write(true).open(&full_path)?
-            };
-            files.push(OpenedFile::new(file));
-        }
+    fn create(&self, meta: &ManagedTorrentInfo) -> anyhow::Result<FilesystemStorage> {
         Ok(FilesystemStorage {
-            output_folder: output_folder.clone(),
-            opened_files: files,
+            output_folder: meta.options.output_folder.clone(),
+            opened_files: Default::default(),
         })
     }
 
@@ -142,4 +108,57 @@ impl TorrentStorage for FilesystemStorage {
             output_folder: self.output_folder.clone(),
         }))
     }
+
+    fn remove_directory_if_empty(&self, path: &Path) -> anyhow::Result<()> {
+        let path = self.output_folder.join(path);
+        if !path.is_dir() {
+            anyhow::bail!("cannot remove dir: {path:?} is not a directory")
+        }
+        if std::fs::read_dir(&path)?.count() == 0 {
+            std::fs::remove_dir(&path).with_context(|| format!("error removing {path:?}"))
+        } else {
+            warn!("did not remove {path:?} as it was not empty");
+            Ok(())
+        }
+    }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        let mut files = Vec::<OpenedFile>::new();
+        for file_details in meta.info.iter_file_details(&meta.lengths)? {
+            let mut full_path = self.output_folder.clone();
+            let relative_path = file_details
+                .filename
+                .to_pathbuf()
+                .context("error converting file to path")?;
+            full_path.push(relative_path);
+
+            std::fs::create_dir_all(full_path.parent().context("bug: no parent")?)?;
+            let file = if meta.options.allow_overwrite {
+                OpenOptions::new()
+                    .create(true)
+                    .truncate(false)
+                    .read(true)
+                    .write(true)
+                    .open(&full_path)
+                    .with_context(|| format!("error opening {full_path:?} in read/write mode"))?
+            } else {
+                // create_new does not seem to work with read(true), so calling this twice.
+                OpenOptions::new()
+                    .create_new(true)
+                    .write(true)
+                    .open(&full_path)
+                    .with_context(|| {
+                        format!(
+                            "error creating a new file (because allow_overwrite = false) {:?}",
+                            &full_path
+                        )
+                    })?;
+                OpenOptions::new().read(true).write(true).open(&full_path)?
+            };
+            files.push(OpenedFile::new(file));
+        }
+
+        self.opened_files = files;
+        Ok(())
+    }
 }

crates/librqbit/src/storage/filesystem/mmap.rs

@@ -22,19 +22,11 @@ fn dummy_mmap() -> anyhow::Result<MmapMut> {
 impl StorageFactory for MmapFilesystemStorageFactory {
     type Storage = MmapFilesystemStorage;
 
-    fn init_storage(&self, meta: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
-        let fs_storage = FilesystemStorageFactory::default().init_storage(meta)?;
-        let mut mmaps = Vec::new();
-        for (idx, file) in fs_storage.opened_files.iter().enumerate() {
-            let fg = file.file.write();
-            fg.set_len(meta.file_infos[idx].len)
-                .context("mmap storage: error setting length")?;
-            let mmap = unsafe { MmapOptions::new().map_mut(&*fg) }.context("error mapping file")?;
-            mmaps.push(RwLock::new(mmap));
-        }
+    fn create(&self, meta: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+        let fs_storage = FilesystemStorageFactory::default().create(meta)?;
 
         Ok(MmapFilesystemStorage {
-            opened_mmaps: mmaps,
+            opened_mmaps: Vec::new(),
             fs: fs_storage,
         })
     }
@@ -82,6 +74,10 @@ impl TorrentStorage for MmapFilesystemStorage {
         self.fs.remove_file(file_id, filename)
     }
 
+    fn remove_directory_if_empty(&self, path: &Path) -> anyhow::Result<()> {
+        self.fs.remove_directory_if_empty(path)
+    }
+
     fn ensure_file_length(&self, file_id: usize, len: u64) -> anyhow::Result<()> {
         self.fs.ensure_file_length(file_id, len)
     }
@@ -100,4 +96,19 @@ impl TorrentStorage for MmapFilesystemStorage {
             fs: self.fs.take_fs()?,
         }))
     }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        self.fs.init(meta)?;
+        let mut mmaps = Vec::new();
+        for (idx, file) in self.fs.opened_files.iter().enumerate() {
+            let fg = file.file.write();
+            fg.set_len(meta.file_infos[idx].len)
+                .context("mmap storage: error setting length")?;
+            let mmap = unsafe { MmapOptions::new().map_mut(&*fg) }.context("error mapping file")?;
+            mmaps.push(RwLock::new(mmap));
+        }
+
+        self.opened_mmaps = mmaps;
+        Ok(())
+    }
 }

crates/librqbit/src/storage/middleware/slow.rs

@@ -14,7 +14,10 @@ use std::{
 
 use parking_lot::Mutex;
 
-use crate::storage::{StorageFactory, StorageFactoryExt, TorrentStorage};
+use crate::{
+    storage::{StorageFactory, StorageFactoryExt, TorrentStorage},
+    ManagedTorrentInfo,
+};
 
 #[derive(Clone)]
 pub struct SlowStorageFactory<U> {
@@ -32,9 +35,9 @@ impl<U: StorageFactory> SlowStorageFactory<U> {
 impl<U: StorageFactory + Clone> StorageFactory for SlowStorageFactory<U> {
     type Storage = SlowStorage<U::Storage>;
 
-    fn init_storage(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+    fn create(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
         Ok(SlowStorage {
-            underlying: self.underlying_factory.init_storage(info)?,
+            underlying: self.underlying_factory.create(info)?,
             pwrite_all_bufread: Mutex::new(Box::new(
                 BufReader::new(
                     File::open(
@@ -108,4 +111,12 @@ impl<U: TorrentStorage> TorrentStorage for SlowStorage<U> {
     fn take(&self) -> anyhow::Result<Box<dyn TorrentStorage>> {
         anyhow::bail!("not implemented")
     }
+
+    fn remove_directory_if_empty(&self, path: &std::path::Path) -> anyhow::Result<()> {
+        self.underlying.remove_directory_if_empty(path)
+    }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        self.underlying.init(meta)
+    }
 }

crates/librqbit/src/storage/middleware/timing.rs

@@ -2,7 +2,10 @@
 A storage middleware that logs the time underlying storage operations took.
 */
 
-use crate::storage::{StorageFactory, StorageFactoryExt, TorrentStorage};
+use crate::{
+    storage::{StorageFactory, StorageFactoryExt, TorrentStorage},
+    ManagedTorrentInfo,
+};
 
 #[derive(Clone)]
 pub struct TimingStorageFactory<U> {
@@ -22,10 +25,10 @@ impl<U> TimingStorageFactory<U> {
 impl<U: StorageFactory + Clone> StorageFactory for TimingStorageFactory<U> {
     type Storage = TimingStorage<U::Storage>;
 
-    fn init_storage(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+    fn create(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
         Ok(TimingStorage {
             name: self.name.clone(),
-            underlying: self.underlying_factory.init_storage(info)?,
+            underlying: self.underlying_factory.create(info)?,
         })
     }
 
@@ -96,4 +99,12 @@ impl<U: TorrentStorage> TorrentStorage for TimingStorage<U> {
             name: self.name.clone(),
         }))
     }
+
+    fn remove_directory_if_empty(&self, path: &std::path::Path) -> anyhow::Result<()> {
+        self.underlying.remove_directory_if_empty(path)
+    }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        self.underlying.init(meta)
+    }
 }

crates/librqbit/src/storage/middleware/write_through_cache.rs

@@ -14,7 +14,7 @@ use parking_lot::RwLock;
 
 use crate::{
     storage::{StorageFactory, StorageFactoryExt, TorrentStorage},
-    FileInfos,
+    FileInfos, ManagedTorrentInfo,
 };
 
 #[derive(Clone, Copy)]
@@ -35,7 +35,7 @@ impl<U> WriteThroughCacheStorageFactory<U> {
 impl<U: StorageFactory + Clone> StorageFactory for WriteThroughCacheStorageFactory<U> {
     type Storage = WriteThroughCacheStorage<U::Storage>;
 
-    fn init_storage(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+    fn create(&self, info: &crate::ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
         let pieces = self
             .max_cache_bytes
             .div_ceil(info.lengths.default_piece_length() as u64)
@@ -44,7 +44,7 @@ impl<U: StorageFactory + Clone> StorageFactory for WriteThroughCacheStorageFacto
         let lru = RwLock::new(LruCache::new(pieces));
         Ok(WriteThroughCacheStorage {
             lru,
-            underlying: self.underlying.init_storage(info)?,
+            underlying: self.underlying.create(info)?,
             lengths: info.lengths,
             file_infos: info.file_infos.clone(),
         })
@@ -116,4 +116,12 @@ impl<U: TorrentStorage> TorrentStorage for WriteThroughCacheStorage<U> {
             file_infos: self.file_infos.clone(),
         }))
     }
+
+    fn remove_directory_if_empty(&self, path: &std::path::Path) -> anyhow::Result<()> {
+        self.underlying.remove_directory_if_empty(path)
+    }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        self.underlying.init(meta)
+    }
 }

crates/librqbit/src/storage/mod.rs

@@ -16,7 +16,13 @@ use crate::torrent_state::ManagedTorrentInfo;
 pub trait StorageFactory: Send + Sync + Any {
     type Storage: TorrentStorage;
 
-    fn init_storage(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage>;
+    fn create(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage>;
+    fn create_and_init(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+        let mut storage = self.create(info)?;
+        storage.init(info)?;
+        Ok(storage)
+    }
+
     fn is_type_id(&self, type_id: TypeId) -> bool {
         Self::type_id(self) == type_id
     }
@@ -38,8 +44,8 @@ impl<SF: StorageFactory> StorageFactoryExt for SF {
         impl<SF: StorageFactory> StorageFactory for Wrapper<SF> {
             type Storage = Box<dyn TorrentStorage>;
 
-            fn init_storage(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
-                let s = self.sf.init_storage(info)?;
+            fn create(&self, info: &ManagedTorrentInfo) -> anyhow::Result<Self::Storage> {
+                let s = self.sf.create(info)?;
                 Ok(Box::new(s))
             }
 
@@ -59,8 +65,8 @@ impl<SF: StorageFactory> StorageFactoryExt for SF {
 impl<U: StorageFactory + ?Sized> StorageFactory for Box<U> {
     type Storage = U::Storage;
 
-    fn init_storage(&self, info: &ManagedTorrentInfo) -> anyhow::Result<U::Storage> {
-        (**self).init_storage(info)
+    fn create(&self, info: &ManagedTorrentInfo) -> anyhow::Result<U::Storage> {
+        (**self).create(info)
     }
 
     fn clone_box(&self) -> BoxStorageFactory {
@@ -69,6 +75,9 @@ impl<U: StorageFactory + ?Sized> StorageFactory for Box<U> {
 }
 
 pub trait TorrentStorage: Send + Sync {
+    // Create/open files etc.
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()>;
+
     /// Given a file_id (which you can get more info from in init_storage() through torrent info)
     /// read buf.len() bytes into buf at offset.
     fn pread_exact(&self, file_id: usize, offset: u64, buf: &mut [u8]) -> anyhow::Result<()>;
@@ -80,6 +89,8 @@ pub trait TorrentStorage: Send + Sync {
     /// Remove a file from the storage. If not supported, or it doesn't matter, just return Ok(())
     fn remove_file(&self, file_id: usize, filename: &Path) -> anyhow::Result<()>;
 
+    fn remove_directory_if_empty(&self, path: &Path) -> anyhow::Result<()>;
+
     /// E.g. for filesystem backend ensure that the file has a certain length, and grow/shrink as needed.
     fn ensure_file_length(&self, file_id: usize, length: u64) -> anyhow::Result<()>;
 
@@ -108,4 +119,12 @@ impl<U: TorrentStorage + ?Sized> TorrentStorage for Box<U> {
     fn take(&self) -> anyhow::Result<Box<dyn TorrentStorage>> {
         (**self).take()
     }
+
+    fn remove_directory_if_empty(&self, path: &Path) -> anyhow::Result<()> {
+        (**self).remove_directory_if_empty(path)
+    }
+
+    fn init(&mut self, meta: &ManagedTorrentInfo) -> anyhow::Result<()> {
+        (**self).init(meta)
+    }
 }