Modified Directory::exists API to return Result<bool, OpenReadError>

This commit is contained in:
Adrien Guillo
2020-11-09 17:57:43 -08:00
parent 726d32eac5
commit 7fd6054145
7 changed files with 43 additions and 35 deletions

View File

@@ -5,6 +5,7 @@ use crate::core::SegmentId;
use crate::core::SegmentMeta; use crate::core::SegmentMeta;
use crate::core::SegmentMetaInventory; use crate::core::SegmentMetaInventory;
use crate::core::META_FILEPATH; use crate::core::META_FILEPATH;
use crate::directory::error::OpenReadError;
use crate::directory::ManagedDirectory; use crate::directory::ManagedDirectory;
#[cfg(feature = "mmap")] #[cfg(feature = "mmap")]
use crate::directory::MmapDirectory; use crate::directory::MmapDirectory;
@@ -59,7 +60,7 @@ impl Index {
/// Examines the directory to see if it contains an index. /// Examines the directory to see if it contains an index.
/// ///
/// Effectively, it only checks for the presence of the `meta.json` file. /// Effectively, it only checks for the presence of the `meta.json` file.
pub fn exists<Dir: Directory>(dir: &Dir) -> bool { pub fn exists<Dir: Directory>(dir: &Dir) -> Result<bool, OpenReadError> {
dir.exists(&META_FILEPATH) dir.exists(&META_FILEPATH)
} }
@@ -106,7 +107,7 @@ impl Index {
schema: Schema, schema: Schema,
) -> crate::Result<Index> { ) -> crate::Result<Index> {
let mmap_directory = MmapDirectory::open(directory_path)?; let mmap_directory = MmapDirectory::open(directory_path)?;
if Index::exists(&mmap_directory) { if Index::exists(&mmap_directory)? {
return Err(TantivyError::IndexAlreadyExists); return Err(TantivyError::IndexAlreadyExists);
} }
Index::create(mmap_directory, schema) Index::create(mmap_directory, schema)
@@ -114,7 +115,7 @@ impl Index {
/// Opens or creates a new index in the provided directory /// Opens or creates a new index in the provided directory
pub fn open_or_create<Dir: Directory>(dir: Dir, schema: Schema) -> crate::Result<Index> { pub fn open_or_create<Dir: Directory>(dir: Dir, schema: Schema) -> crate::Result<Index> {
if !Index::exists(&dir) { if !Index::exists(&dir)? {
return Index::create(dir, schema); return Index::create(dir, schema);
} }
let index = Index::open(dir)?; let index = Index::open(dir)?;
@@ -423,24 +424,24 @@ mod tests {
#[test] #[test]
fn test_index_exists() { fn test_index_exists() {
let directory = RAMDirectory::create(); let directory = RAMDirectory::create();
assert!(!Index::exists(&directory)); assert!(!Index::exists(&directory).unwrap());
assert!(Index::create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::create(directory.clone(), throw_away_schema()).is_ok());
assert!(Index::exists(&directory)); assert!(Index::exists(&directory).unwrap());
} }
#[test] #[test]
fn open_or_create_should_create() { fn open_or_create_should_create() {
let directory = RAMDirectory::create(); let directory = RAMDirectory::create();
assert!(!Index::exists(&directory)); assert!(!Index::exists(&directory).unwrap());
assert!(Index::open_or_create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::open_or_create(directory.clone(), throw_away_schema()).is_ok());
assert!(Index::exists(&directory)); assert!(Index::exists(&directory).unwrap());
} }
#[test] #[test]
fn open_or_create_should_open() { fn open_or_create_should_open() {
let directory = RAMDirectory::create(); let directory = RAMDirectory::create();
assert!(Index::create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::create(directory.clone(), throw_away_schema()).is_ok());
assert!(Index::exists(&directory)); assert!(Index::exists(&directory).unwrap());
assert!(Index::open_or_create(directory, throw_away_schema()).is_ok()); assert!(Index::open_or_create(directory, throw_away_schema()).is_ok());
} }
@@ -448,7 +449,7 @@ mod tests {
fn create_should_wipeoff_existing() { fn create_should_wipeoff_existing() {
let directory = RAMDirectory::create(); let directory = RAMDirectory::create();
assert!(Index::create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::create(directory.clone(), throw_away_schema()).is_ok());
assert!(Index::exists(&directory)); assert!(Index::exists(&directory).unwrap());
assert!(Index::create(directory.clone(), Schema::builder().build()).is_ok()); assert!(Index::create(directory.clone(), Schema::builder().build()).is_ok());
} }
@@ -456,7 +457,7 @@ mod tests {
fn open_or_create_exists_but_schema_does_not_match() { fn open_or_create_exists_but_schema_does_not_match() {
let directory = RAMDirectory::create(); let directory = RAMDirectory::create();
assert!(Index::create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::create(directory.clone(), throw_away_schema()).is_ok());
assert!(Index::exists(&directory)); assert!(Index::exists(&directory).unwrap());
assert!(Index::open_or_create(directory.clone(), throw_away_schema()).is_ok()); assert!(Index::open_or_create(directory.clone(), throw_away_schema()).is_ok());
let err = Index::open_or_create(directory, Schema::builder().build()); let err = Index::open_or_create(directory, Schema::builder().build());
assert_eq!( assert_eq!(

View File

@@ -131,7 +131,7 @@ pub trait Directory: DirectoryClone + fmt::Debug + Send + Sync + 'static {
fn delete(&self, path: &Path) -> Result<(), DeleteError>; fn delete(&self, path: &Path) -> Result<(), DeleteError>;
/// Returns true iff the file exists /// Returns true iff the file exists
fn exists(&self, path: &Path) -> bool; fn exists(&self, path: &Path) -> Result<bool, OpenReadError>;
/// Opens a writer for the *virtual file* associated with /// Opens a writer for the *virtual file* associated with
/// a Path. /// a Path.

View File

@@ -307,7 +307,7 @@ impl Directory for ManagedDirectory {
self.directory.delete(path) self.directory.delete(path)
} }
fn exists(&self, path: &Path) -> bool { fn exists(&self, path: &Path) -> Result<bool, OpenReadError> {
self.directory.exists(path) self.directory.exists(path)
} }
@@ -355,22 +355,22 @@ mod tests_mmap_specific {
managed_directory managed_directory
.atomic_write(test_path2, &[0u8, 1u8]) .atomic_write(test_path2, &[0u8, 1u8])
.unwrap(); .unwrap();
assert!(managed_directory.exists(test_path1)); assert!(managed_directory.exists(test_path1).unwrap());
assert!(managed_directory.exists(test_path2)); assert!(managed_directory.exists(test_path2).unwrap());
let living_files: HashSet<PathBuf> = [test_path1.to_owned()].iter().cloned().collect(); let living_files: HashSet<PathBuf> = [test_path1.to_owned()].iter().cloned().collect();
assert!(managed_directory.garbage_collect(|| living_files).is_ok()); assert!(managed_directory.garbage_collect(|| living_files).is_ok());
assert!(managed_directory.exists(test_path1)); assert!(managed_directory.exists(test_path1).unwrap());
assert!(!managed_directory.exists(test_path2)); assert!(!managed_directory.exists(test_path2).unwrap());
} }
{ {
let mmap_directory = MmapDirectory::open(&tempdir_path).unwrap(); let mmap_directory = MmapDirectory::open(&tempdir_path).unwrap();
let mut managed_directory = ManagedDirectory::wrap(mmap_directory).unwrap(); let mut managed_directory = ManagedDirectory::wrap(mmap_directory).unwrap();
assert!(managed_directory.exists(test_path1)); assert!(managed_directory.exists(test_path1).unwrap());
assert!(!managed_directory.exists(test_path2)); assert!(!managed_directory.exists(test_path2).unwrap());
let living_files: HashSet<PathBuf> = HashSet::new(); let living_files: HashSet<PathBuf> = HashSet::new();
assert!(managed_directory.garbage_collect(|| living_files).is_ok()); assert!(managed_directory.garbage_collect(|| living_files).is_ok());
assert!(!managed_directory.exists(test_path1)); assert!(!managed_directory.exists(test_path1).unwrap());
assert!(!managed_directory.exists(test_path2)); assert!(!managed_directory.exists(test_path2).unwrap());
} }
} }
@@ -387,7 +387,7 @@ mod tests_mmap_specific {
let mut write = managed_directory.open_write(test_path1).unwrap(); let mut write = managed_directory.open_write(test_path1).unwrap();
write.write_all(&[0u8, 1u8]).unwrap(); write.write_all(&[0u8, 1u8]).unwrap();
write.terminate().unwrap(); write.terminate().unwrap();
assert!(managed_directory.exists(test_path1)); assert!(managed_directory.exists(test_path1).unwrap());
let _mmap_read = managed_directory.open_read(test_path1).unwrap(); let _mmap_read = managed_directory.open_read(test_path1).unwrap();
assert!(managed_directory assert!(managed_directory
@@ -395,15 +395,15 @@ mod tests_mmap_specific {
.is_ok()); .is_ok());
if cfg!(target_os = "windows") { if cfg!(target_os = "windows") {
// On Windows, gc should try and fail the file as it is mmapped. // On Windows, gc should try and fail the file as it is mmapped.
assert!(managed_directory.exists(test_path1)); assert!(managed_directory.exists(test_path1).unwrap());
// unmap should happen here. // unmap should happen here.
drop(_mmap_read); drop(_mmap_read);
// The file should still be in the list of managed file and // The file should still be in the list of managed file and
// eventually be deleted once mmap is released. // eventually be deleted once mmap is released.
assert!(managed_directory.garbage_collect(|| living_files).is_ok()); assert!(managed_directory.garbage_collect(|| living_files).is_ok());
assert!(!managed_directory.exists(test_path1)); assert!(!managed_directory.exists(test_path1).unwrap());
} else { } else {
assert!(!managed_directory.exists(test_path1)); assert!(!managed_directory.exists(test_path1).unwrap());
} }
} }

View File

@@ -370,9 +370,9 @@ impl Directory for MmapDirectory {
} }
} }
fn exists(&self, path: &Path) -> bool { fn exists(&self, path: &Path) -> Result<bool, OpenReadError> {
let full_path = self.resolve_path(path); let full_path = self.resolve_path(path);
full_path.exists() Ok(full_path.exists())
} }
fn open_write(&self, path: &Path) -> Result<WritePtr, OpenWriteError> { fn open_write(&self, path: &Path) -> Result<WritePtr, OpenWriteError> {

View File

@@ -177,8 +177,15 @@ impl Directory for RAMDirectory {
self.fs.write().unwrap().delete(path) self.fs.write().unwrap().delete(path)
} }
fn exists(&self, path: &Path) -> bool { fn exists(&self, path: &Path) -> Result<bool, OpenReadError> {
self.fs.read().unwrap().exists(path) Ok(self
.fs
.read()
.map_err(|e| OpenReadError::IOError {
io_error: io::Error::new(io::ErrorKind::Other, e.to_string()),
filepath: path.to_path_buf(),
})?
.exists(path))
} }
fn open_write(&self, path: &Path) -> Result<WritePtr, OpenWriteError> { fn open_write(&self, path: &Path) -> Result<WritePtr, OpenWriteError> {

View File

@@ -130,7 +130,7 @@ fn ram_directory_panics_if_flush_forgotten() {
fn test_simple(directory: &dyn Directory) -> crate::Result<()> { fn test_simple(directory: &dyn Directory) -> crate::Result<()> {
let test_path: &'static Path = Path::new("some_path_for_test"); let test_path: &'static Path = Path::new("some_path_for_test");
let mut write_file = directory.open_write(test_path)?; let mut write_file = directory.open_write(test_path)?;
assert!(directory.exists(test_path)); assert!(directory.exists(test_path).unwrap());
write_file.write_all(&[4])?; write_file.write_all(&[4])?;
write_file.write_all(&[3])?; write_file.write_all(&[3])?;
write_file.write_all(&[7, 3, 5])?; write_file.write_all(&[7, 3, 5])?;
@@ -139,14 +139,14 @@ fn test_simple(directory: &dyn Directory) -> crate::Result<()> {
assert_eq!(read_file.as_slice(), &[4u8, 3u8, 7u8, 3u8, 5u8]); assert_eq!(read_file.as_slice(), &[4u8, 3u8, 7u8, 3u8, 5u8]);
mem::drop(read_file); mem::drop(read_file);
assert!(directory.delete(test_path).is_ok()); assert!(directory.delete(test_path).is_ok());
assert!(!directory.exists(test_path)); assert!(!directory.exists(test_path).unwrap());
Ok(()) Ok(())
} }
fn test_rewrite_forbidden(directory: &dyn Directory) -> crate::Result<()> { fn test_rewrite_forbidden(directory: &dyn Directory) -> crate::Result<()> {
let test_path: &'static Path = Path::new("some_path_for_test"); let test_path: &'static Path = Path::new("some_path_for_test");
directory.open_write(test_path)?; directory.open_write(test_path)?;
assert!(directory.exists(test_path)); assert!(directory.exists(test_path).unwrap());
assert!(directory.open_write(test_path).is_err()); assert!(directory.open_write(test_path).is_err());
assert!(directory.delete(test_path).is_ok()); assert!(directory.delete(test_path).is_ok());
Ok(()) Ok(())
@@ -157,7 +157,7 @@ fn test_write_create_the_file(directory: &dyn Directory) {
{ {
assert!(directory.open_read(test_path).is_err()); assert!(directory.open_read(test_path).is_err());
let _w = directory.open_write(test_path).unwrap(); let _w = directory.open_write(test_path).unwrap();
assert!(directory.exists(test_path)); assert!(directory.exists(test_path).unwrap());
assert!(directory.open_read(test_path).is_ok()); assert!(directory.open_read(test_path).is_ok());
assert!(directory.delete(test_path).is_ok()); assert!(directory.delete(test_path).is_ok());
} }

View File

@@ -18,7 +18,7 @@ fn test_failpoints_managed_directory_gc_if_delete_fails() {
.unwrap() .unwrap()
.terminate() .terminate()
.unwrap(); .unwrap();
assert!(managed_directory.exists(test_path)); assert!(managed_directory.exists(test_path).unwrap());
// triggering gc and setting the delete operation to fail. // triggering gc and setting the delete operation to fail.
// //
// We are checking that the gc operation is not removing the // We are checking that the gc operation is not removing the
@@ -29,12 +29,12 @@ fn test_failpoints_managed_directory_gc_if_delete_fails() {
// lock file. // lock file.
fail::cfg("RAMDirectory::delete", "1*off->1*return").unwrap(); fail::cfg("RAMDirectory::delete", "1*off->1*return").unwrap();
assert!(managed_directory.garbage_collect(Default::default).is_ok()); assert!(managed_directory.garbage_collect(Default::default).is_ok());
assert!(managed_directory.exists(test_path)); assert!(managed_directory.exists(test_path).unwrap());
// running the gc a second time should remove the file. // running the gc a second time should remove the file.
assert!(managed_directory.garbage_collect(Default::default).is_ok()); assert!(managed_directory.garbage_collect(Default::default).is_ok());
assert!( assert!(
!managed_directory.exists(test_path), !managed_directory.exists(test_path).unwrap(),
"The file should have been deleted" "The file should have been deleted"
); );
} }