simplify database backup interface related

Signed-off-by: Jason Volk <jason@zemos.net>

src/admin/server/commands.rs

@@ -1,6 +1,11 @@
 use std::{fmt::Write, path::PathBuf, sync::Arc};
 
-use conduwuit::{Err, Result, info, utils::time, warn};
+use conduwuit::{
+	Err, Result, info,
+	utils::{stream::IterStream, time},
+	warn,
+};
+use futures::TryStreamExt;
 
 use crate::admin_command;
 
@@ -81,33 +86,31 @@ pub(super) async fn clear_caches(&self) -> Result {
 
 #[admin_command]
 pub(super) async fn list_backups(&self) -> Result {
-	let result = self.services.db.db.backup_list()?;
-
-	if result.is_empty() {
-		return Err!("No backups found.");
-	}
-
-	self.write_str(&result).await
+	self.services
+		.db
+		.db
+		.backup_list()?
+		.try_stream()
+		.try_for_each(|result| write!(self, "{result}"))
+		.await
 }
 
 #[admin_command]
 pub(super) async fn backup_database(&self) -> Result {
 	let db = Arc::clone(&self.services.db);
-	let mut result = self
+	let result = self
 		.services
 		.server
 		.runtime()
 		.spawn_blocking(move || match db.db.backup() {
-			| Ok(()) => String::new(),
-			| Err(e) => e.to_string(),
+			| Ok(()) => "Done".to_owned(),
+			| Err(e) => format!("Failed: {e}"),
 		})
 		.await?;
 
-	if result.is_empty() {
-		result = self.services.db.db.backup_list()?;
-	}
-
-	self.write_str(&result).await
+	let count = self.services.db.db.backup_count()?;
+	self.write_str(&format!("{result}. Currently have {count} backups."))
+		.await
 }
 
 #[admin_command]

src/database/engine/backup.rs

@@ -1,24 +1,16 @@
-use std::fmt::Write;
+use std::{ffi::OsString, path::PathBuf};
 
-use conduwuit::{Result, error, implement, info, utils::time::rfc2822_from_seconds, warn};
+use conduwuit::{Err, Result, error, implement, info, utils::time::rfc2822_from_seconds, warn};
 use rocksdb::backup::{BackupEngine, BackupEngineOptions};
 
 use super::Engine;
-use crate::{or_else, util::map_err};
+use crate::util::map_err;
 
 #[implement(Engine)]
 #[tracing::instrument(skip(self))]
 pub fn backup(&self) -> Result {
-	let server = &self.ctx.server;
-	let config = &server.config;
-	let path = config.database_backup_path.as_ref();
-	if path.is_none() || path.is_some_and(|path| path.as_os_str().is_empty()) {
-		return Ok(());
-	}
-
-	let options =
-		BackupEngineOptions::new(path.expect("valid database backup path")).map_err(map_err)?;
-	let mut engine = BackupEngine::open(&options, &*self.ctx.env.lock()?).map_err(map_err)?;
+	let mut engine = self.backup_engine()?;
+	let config = &self.ctx.server.config;
 	if config.database_backups_to_keep > 0 {
 		let flush = !self.is_read_only();
 		engine
@@ -40,34 +32,62 @@ pub fn backup(&self) -> Result {
 		}
 	}
 
+	if config.database_backups_to_keep == 0 {
+		warn!("Configuration item `database_backups_to_keep` is set to 0.");
+	}
+
 	Ok(())
 }
 
 #[implement(Engine)]
-pub fn backup_list(&self) -> Result<String> {
-	let server = &self.ctx.server;
-	let config = &server.config;
-	let path = config.database_backup_path.as_ref();
-	if path.is_none() || path.is_some_and(|path| path.as_os_str().is_empty()) {
-		return Ok("Configure database_backup_path to enable backups, or the path specified is \
-		           not valid"
-			.to_owned());
+pub fn backup_list(&self) -> Result<impl Iterator<Item = String> + Send> {
+	let info = self.backup_engine()?.get_backup_info();
+
+	if info.is_empty() {
+		return Err!("No backups found.");
 	}
 
-	let mut res = String::new();
-	let options =
-		BackupEngineOptions::new(path.expect("valid database backup path")).or_else(or_else)?;
-	let engine = BackupEngine::open(&options, &*self.ctx.env.lock()?).or_else(or_else)?;
-	for info in engine.get_backup_info() {
-		writeln!(
-			res,
+	let list = info.into_iter().map(|info| {
+		format!(
 			"#{} {}: {} bytes, {} files",
 			info.backup_id,
 			rfc2822_from_seconds(info.timestamp),
 			info.size,
 			info.num_files,
-		)?;
+		)
+	});
+
+	Ok(list)
+}
+
+#[implement(Engine)]
+pub fn backup_count(&self) -> Result<usize> {
+	let info = self.backup_engine()?.get_backup_info();
+
+	Ok(info.len())
+}
+
+#[implement(Engine)]
+fn backup_engine(&self) -> Result<BackupEngine> {
+	let path = self.backup_path()?;
+	let options = BackupEngineOptions::new(path).map_err(map_err)?;
+	BackupEngine::open(&options, &*self.ctx.env.lock()?).map_err(map_err)
+}
+
+#[implement(Engine)]
+fn backup_path(&self) -> Result<OsString> {
+	let path = self
+		.ctx
+		.server
+		.config
+		.database_backup_path
+		.clone()
+		.map(PathBuf::into_os_string)
+		.unwrap_or_default();
+
+	if path.is_empty() {
+		return Err!(Config("database_backup_path", "Configure path to enable backups"));
 	}
 
-	Ok(res)
+	Ok(path)
 }

src/service/globals/data.rs

@@ -72,10 +72,4 @@ impl Data {
 	pub fn bump_database_version(&self, new_version: u64) {
 		self.global.raw_put(b"version", new_version);
 	}
-
-	#[inline]
-	pub fn backup(&self) -> Result { self.db.db.backup() }
-
-	#[inline]
-	pub fn backup_list(&self) -> Result<String> { self.db.db.backup_list() }
 }