From 253658ece59f7677554f9a8d544336afc6904616 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 22:31:09 +0100 Subject: [PATCH 1/8] Fix: FileID::from() should also succeed if we pass a ID as string --- src/storage/file_id.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/file_id.rs b/src/storage/file_id.rs index f05eaa4e..c582aa8e 100644 --- a/src/storage/file_id.rs +++ b/src/storage/file_id.rs @@ -127,7 +127,7 @@ impl From for FileID { impl<'a> From<&'a String> for FileID { fn from(string: &'a String) -> FileID { - + // we assume that it is an path let regex = Regex::new(r"([:alnum:]*)-([:upper:]*)-([A-Za-z0-9-_]*)\.(.*)").unwrap(); let s = string.split("/").last().unwrap_or(""); @@ -160,9 +160,10 @@ impl<'a> From<&'a String> for FileID { Some(FileID::new(idtype, String::from(hash))) }).unwrap_or({ debug!("Did not match"); + debug!("It is no path, actually. So we assume it is an ID already"); FileID { id_type: FileIDType::NONE, - id: None, + id: Some(string.clone()), } }) } From dac4911ea225b7eb3da0e9071585071b03024a88 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 22:49:00 +0100 Subject: [PATCH 2/8] (partly) Rewrite StorageBackend::get_file_by_id(), so we can get a file with a partially available ID --- src/storage/backend.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 9021517b..74edc34b 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -189,16 +189,35 @@ impl StorageBackend { pub fn get_file_by_id<'a, HP>(&self, m: &'a Module, id: &FileID, p: &Parser) -> Option> where HP: FileHeaderParser { + use std::ops::Index; + debug!("Searching for file with id '{}'", id); - if let Ok(mut fs) = FSFile::open(self.build_filepath_with_id(m, id.clone())) { - let mut s = String::new(); - fs.read_to_string(&mut s); - debug!("Success opening file with id '{}'", id); - debug!("Parsing to internal structure now"); - p.read(s).and_then(|(h, d)| Ok(File::from_parser_result(m, id.clone(), h, d))).ok() + + if id.get_type() == FileIDType::NONE { + // We don't know the hash type, so we glob() around a bit. + + let globstr = self.prefix_of_files_for_module(m) + "*" + ".imag"; + glob(&globstr[..]).map(|globlist| { + let mut vec = globlist.filter_map(Result::ok) + .map(|pbuf| FileID::from(&pbuf)) + .filter_map(|id| self.get_file_by_id(m, &id, p)) + .collect::>(); + vec.reverse(); + vec.pop() + }).map_err(|e| e).unwrap() } else { - debug!("No file with id '{}'", id); - None + // The (hash)type is already in the FileID object, so we can just + // build a path from the information we already have + if let Ok(mut fs) = FSFile::open(self.build_filepath_with_id(m, id.clone())) { + let mut s = String::new(); + fs.read_to_string(&mut s); + debug!("Success opening file with id '{}'", id); + debug!("Parsing to internal structure now"); + p.read(s).and_then(|(h, d)| Ok(File::from_parser_result(m, id.clone(), h, d))).ok() + } else { + debug!("No file with id '{}'", id); + None + } } } From 6473b743a3cfe0b339745fddb013c85338550e49 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 22:53:31 +0100 Subject: [PATCH 3/8] Outsource globlist -> Vec transformation code snippet --- src/storage/backend.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 74edc34b..bf209461 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -59,10 +59,7 @@ impl StorageBackend { glob(&globstr[..]) .and_then(|globlist| { debug!("Iterating over globlist"); - Ok(globlist.filter_map(Result::ok) - .map(|pbuf| FileID::from(&pbuf)) - .collect::>() - .into_iter()) + Ok(globlist_to_file_id_vec(globlist).into_iter()) }) .map_err(|e| { debug!("glob() returned error: {:?}", e); @@ -198,10 +195,9 @@ impl StorageBackend { let globstr = self.prefix_of_files_for_module(m) + "*" + ".imag"; glob(&globstr[..]).map(|globlist| { - let mut vec = globlist.filter_map(Result::ok) - .map(|pbuf| FileID::from(&pbuf)) - .filter_map(|id| self.get_file_by_id(m, &id, p)) - .collect::>(); + let mut vec = globlist_to_file_id_vec(globlist).into_iter() + .filter_map(|id| self.get_file_by_id(m, &id, p)) + .collect::>(); vec.reverse(); vec.pop() }).map_err(|e| e).unwrap() @@ -326,3 +322,9 @@ fn write_with_parser<'a, HP>(f: &File, p: &Parser) -> Result Vec { + globlist.filter_map(Result::ok) + .map(|pbuf| FileID::from(&pbuf)) + .collect::>() +} From fd3d2ec8dc8a502ca7f3bd7a8aef424636a56ad7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 22:57:28 +0100 Subject: [PATCH 4/8] Fixup get_file_by_id() --- src/storage/backend.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index bf209461..508dac9e 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -192,6 +192,7 @@ impl StorageBackend { if id.get_type() == FileIDType::NONE { // We don't know the hash type, so we glob() around a bit. + debug!("Having FileIDType::NONE, so we glob() for the raw ID"); let globstr = self.prefix_of_files_for_module(m) + "*" + ".imag"; glob(&globstr[..]).map(|globlist| { @@ -200,10 +201,14 @@ impl StorageBackend { .collect::>(); vec.reverse(); vec.pop() - }).map_err(|e| e).unwrap() + }).unwrap_or({ + debug!("No glob matches, actually. We can't do anything at this point"); + None + }) } else { // The (hash)type is already in the FileID object, so we can just // build a path from the information we already have + debug!("We know FileIDType, so we build the path directly now"); if let Ok(mut fs) = FSFile::open(self.build_filepath_with_id(m, id.clone())) { let mut s = String::new(); fs.read_to_string(&mut s); From 87d5852986caa2f2d3db258e53f6e45e1dd9b274 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:01:37 +0100 Subject: [PATCH 5/8] Ensure we dont crash if there are no files found --- src/module/bm/commands.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index 1256e87f..84efcf21 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -60,18 +60,22 @@ pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { debug!("Remove by id: {}", id); let parser = Parser::new(JsonHeaderParser::new(None)); - let file = env.bk.get_file_by_id(module, &id.into(), &parser).unwrap(); - debug!("Remove file : {:?}", file); + env.bk.get_file_by_id(module, &id.into(), &parser).map(|file| { + debug!("Remove file : {:?}", file); - if let Err(e) = env.bk.remove_file(module, file, checked) { - debug!("Remove failed"); - let mut err = ModuleError::new("Removing file failed"); - err.caused_by = Some(Box::new(e)); - Err(err) - } else { - debug!("Remove worked"); + if let Err(e) = env.bk.remove_file(module, file, checked) { + debug!("Remove failed"); + let mut err = ModuleError::new("Removing file failed"); + err.caused_by = Some(Box::new(e)); + Err(err) + } else { + info!("Remove worked"); + Ok(()) + } + }).unwrap_or({ + info!("No files found"); Ok(()) - } + }) } else { debug!("Remove more than one file"); From 261fbdd038bacec1a23d407350b8777f9df1405e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:11:46 +0100 Subject: [PATCH 6/8] Add id getter for FileID --- src/storage/file_id.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/storage/file_id.rs b/src/storage/file_id.rs index c582aa8e..da44de36 100644 --- a/src/storage/file_id.rs +++ b/src/storage/file_id.rs @@ -73,6 +73,10 @@ impl FileID { self.id_type.clone() } + pub fn get_id(&self) -> Option { + self.id.clone() + } + } impl Debug for FileID { From 848fb0b4b42d0bd3f1393c3bb3d4f1f0a59ed21e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:11:59 +0100 Subject: [PATCH 7/8] Ensure we build the globstring _with_ the ID in it --- src/storage/backend.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/storage/backend.rs b/src/storage/backend.rs index 508dac9e..719932ee 100644 --- a/src/storage/backend.rs +++ b/src/storage/backend.rs @@ -194,7 +194,9 @@ impl StorageBackend { // We don't know the hash type, so we glob() around a bit. debug!("Having FileIDType::NONE, so we glob() for the raw ID"); - let globstr = self.prefix_of_files_for_module(m) + "*" + ".imag"; + let id_str = id.get_id().unwrap_or(String::from("INVALID")); + let globstr = self.prefix_of_files_for_module(m) + "*" + &id_str[..] + ".imag"; + debug!("Globbing with globstr = '{}'", globstr); glob(&globstr[..]).map(|globlist| { let mut vec = globlist_to_file_id_vec(globlist).into_iter() .filter_map(|id| self.get_file_by_id(m, &id, p)) From cd1bb88f20fd32d0fafc098487b83439123a7124 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 4 Dec 2015 23:19:18 +0100 Subject: [PATCH 8/8] Rework remove_command() --- src/module/bm/commands.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/module/bm/commands.rs b/src/module/bm/commands.rs index 84efcf21..59f507f3 100644 --- a/src/module/bm/commands.rs +++ b/src/module/bm/commands.rs @@ -60,22 +60,24 @@ pub fn remove_command(module: &Module, env: CommandEnv) -> CommandResult { debug!("Remove by id: {}", id); let parser = Parser::new(JsonHeaderParser::new(None)); - env.bk.get_file_by_id(module, &id.into(), &parser).map(|file| { - debug!("Remove file : {:?}", file); + let file = env.bk + .get_file_by_id(module, &id.into(), &parser) + .unwrap_or({ + info!("No files found"); + return Ok(()) + }); - if let Err(e) = env.bk.remove_file(module, file, checked) { - debug!("Remove failed"); - let mut err = ModuleError::new("Removing file failed"); - err.caused_by = Some(Box::new(e)); - Err(err) - } else { - info!("Remove worked"); - Ok(()) - } - }).unwrap_or({ - info!("No files found"); + debug!("Remove file: {:?}", file); + + if let Err(e) = env.bk.remove_file(module, file, checked) { + debug!("Remove failed"); + let mut err = ModuleError::new("Removing file failed"); + err.caused_by = Some(Box::new(e)); + Err(err) + } else { + info!("Remove worked"); Ok(()) - }) + } } else { debug!("Remove more than one file");