Skip to content

Commit

Permalink
Merge pull request #1182 from PSeitz/remove_directory_generic
Browse files Browse the repository at this point in the history
use Box<dyn Directory> as parameter to open/create an Index
  • Loading branch information
PSeitz authored Oct 25, 2021
2 parents 737ecc7 + 99cd25b commit e2fbbc0
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 33 deletions.
44 changes: 26 additions & 18 deletions src/core/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ impl IndexBuilder {
/// If a previous index was in this directory, it returns an `IndexAlreadyExists` error.
#[cfg(feature = "mmap")]
pub fn create_in_dir<P: AsRef<Path>>(self, directory_path: P) -> crate::Result<Index> {
let mmap_directory = MmapDirectory::open(directory_path)?;
if Index::exists(&mmap_directory)? {
let mmap_directory: Box<dyn Directory> = Box::new(MmapDirectory::open(directory_path)?);
if Index::exists(&*mmap_directory)? {
return Err(TantivyError::IndexAlreadyExists);
}
self.create(mmap_directory)
Expand All @@ -139,7 +139,7 @@ impl IndexBuilder {
/// For other unit tests, prefer the `RAMDirectory`, see: `create_in_ram`.
#[cfg(feature = "mmap")]
pub fn create_from_tempdir(self) -> crate::Result<Index> {
let mmap_directory = MmapDirectory::create_from_tempdir()?;
let mmap_directory: Box<dyn Directory> = Box::new(MmapDirectory::create_from_tempdir()?);
self.create(mmap_directory)
}
fn get_expect_schema(&self) -> crate::Result<Schema> {
Expand All @@ -149,8 +149,9 @@ impl IndexBuilder {
.ok_or(TantivyError::IndexBuilderMissingArgument("schema"))
}
/// Opens or creates a new index in the provided directory
pub fn open_or_create<Dir: Directory>(self, dir: Dir) -> crate::Result<Index> {
if !Index::exists(&dir)? {
pub fn open_or_create<T: Into<Box<dyn Directory>>>(self, dir: T) -> crate::Result<Index> {
let dir = dir.into();
if !Index::exists(&*dir)? {
return self.create(dir);
}
let index = Index::open(dir)?;
Expand All @@ -165,7 +166,8 @@ impl IndexBuilder {
/// Creates a new index given an implementation of the trait `Directory`.
///
/// If a directory previously existed, it will be erased.
fn create<Dir: Directory>(self, dir: Dir) -> crate::Result<Index> {
fn create<T: Into<Box<dyn Directory>>>(self, dir: T) -> crate::Result<Index> {
let dir = dir.into();
let directory = ManagedDirectory::wrap(dir)?;
save_new_metas(
self.get_expect_schema()?,
Expand Down Expand Up @@ -198,7 +200,7 @@ impl Index {
/// Examines the directory to see if it contains an index.
///
/// Effectively, it only checks for the presence of the `meta.json` file.
pub fn exists<Dir: Directory>(dir: &Dir) -> Result<bool, OpenReadError> {
pub fn exists(dir: &dyn Directory) -> Result<bool, OpenReadError> {
dir.exists(&META_FILEPATH)
}

Expand Down Expand Up @@ -250,7 +252,11 @@ impl Index {
}

/// 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<T: Into<Box<dyn Directory>>>(
dir: T,
schema: Schema,
) -> crate::Result<Index> {
let dir = dir.into();
IndexBuilder::new().schema(schema).open_or_create(dir)
}

Expand All @@ -270,11 +276,12 @@ impl Index {
/// Creates a new index given an implementation of the trait `Directory`.
///
/// If a directory previously existed, it will be erased.
pub fn create<Dir: Directory>(
dir: Dir,
pub fn create<T: Into<Box<dyn Directory>>>(
dir: T,
schema: Schema,
settings: IndexSettings,
) -> crate::Result<Index> {
let dir: Box<dyn Directory> = dir.into();
let mut builder = IndexBuilder::new().schema(schema);
builder = builder.settings(settings);
builder.create(dir)
Expand Down Expand Up @@ -365,7 +372,8 @@ impl Index {
}

/// Open the index using the provided directory
pub fn open<D: Directory>(directory: D) -> crate::Result<Index> {
pub fn open<T: Into<Box<dyn Directory>>>(directory: T) -> crate::Result<Index> {
let directory = directory.into();
let directory = ManagedDirectory::wrap(directory)?;
let inventory = SegmentMetaInventory::default();
let metas = load_metas(&directory, &inventory)?;
Expand Down Expand Up @@ -574,15 +582,15 @@ mod tests {

#[test]
fn test_index_exists() {
let directory = RamDirectory::create();
assert!(!Index::exists(&directory).unwrap());
let directory: Box<dyn Directory> = Box::new(RamDirectory::create());
assert!(!Index::exists(directory.as_ref()).unwrap());
assert!(Index::create(
directory.clone(),
throw_away_schema(),
IndexSettings::default()
)
.is_ok());
assert!(Index::exists(&directory).unwrap());
assert!(Index::exists(directory.as_ref()).unwrap());
}

#[test]
Expand All @@ -595,27 +603,27 @@ mod tests {

#[test]
fn open_or_create_should_open() {
let directory = RamDirectory::create();
let directory: Box<dyn Directory> = Box::new(RamDirectory::create());
assert!(Index::create(
directory.clone(),
throw_away_schema(),
IndexSettings::default()
)
.is_ok());
assert!(Index::exists(&directory).unwrap());
assert!(Index::exists(directory.as_ref()).unwrap());
assert!(Index::open_or_create(directory, throw_away_schema()).is_ok());
}

#[test]
fn create_should_wipeoff_existing() {
let directory = RamDirectory::create();
let directory: Box<dyn Directory> = Box::new(RamDirectory::create());
assert!(Index::create(
directory.clone(),
throw_away_schema(),
IndexSettings::default()
)
.is_ok());
assert!(Index::exists(&directory).unwrap());
assert!(Index::exists(directory.as_ref()).unwrap());
assert!(Index::create(
directory,
Schema::builder().build(),
Expand Down
12 changes: 12 additions & 0 deletions src/directory/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,15 @@ where
Box::new(self.clone())
}
}

impl Clone for Box<dyn Directory> {
fn clone(&self) -> Self {
self.box_clone()
}
}

impl<T: Directory + 'static> From<T> for Box<dyn Directory> {
fn from(t: T) -> Self {
Box::new(t)
}
}
12 changes: 6 additions & 6 deletions src/directory/managed_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn save_managed_paths(

impl ManagedDirectory {
/// Wraps a directory as managed directory.
pub fn wrap<Dir: Directory>(directory: Dir) -> crate::Result<ManagedDirectory> {
pub fn wrap(directory: Box<dyn Directory>) -> crate::Result<ManagedDirectory> {
match directory.atomic_read(&MANAGED_FILEPATH) {
Ok(data) => {
let managed_files_json = String::from_utf8_lossy(&data);
Expand All @@ -76,14 +76,14 @@ impl ManagedDirectory {
)
})?;
Ok(ManagedDirectory {
directory: Box::new(directory),
directory,
meta_informations: Arc::new(RwLock::new(MetaInformation {
managed_paths: managed_files,
})),
})
}
Err(OpenReadError::FileDoesNotExist(_)) => Ok(ManagedDirectory {
directory: Box::new(directory),
directory,
meta_informations: Arc::default(),
}),
io_err @ Err(OpenReadError::IoError { .. }) => Err(io_err.err().unwrap().into()),
Expand Down Expand Up @@ -340,7 +340,7 @@ mod tests_mmap_specific {
let test_path2: &'static Path = Path::new("some_path_for_test_2");
{
let mmap_directory = MmapDirectory::open(&tempdir_path).unwrap();
let mut managed_directory = ManagedDirectory::wrap(mmap_directory).unwrap();
let mut managed_directory = ManagedDirectory::wrap(Box::new(mmap_directory)).unwrap();
let write_file = managed_directory.open_write(test_path1).unwrap();
write_file.terminate().unwrap();
managed_directory
Expand All @@ -355,7 +355,7 @@ mod tests_mmap_specific {
}
{
let mmap_directory = MmapDirectory::open(&tempdir_path).unwrap();
let mut managed_directory = ManagedDirectory::wrap(mmap_directory).unwrap();
let mut managed_directory = ManagedDirectory::wrap(Box::new(mmap_directory)).unwrap();
assert!(managed_directory.exists(test_path1).unwrap());
assert!(!managed_directory.exists(test_path2).unwrap());
let living_files: HashSet<PathBuf> = HashSet::new();
Expand All @@ -374,7 +374,7 @@ mod tests_mmap_specific {
let living_files = HashSet::new();

let mmap_directory = MmapDirectory::open(&tempdir_path).unwrap();
let mut managed_directory = ManagedDirectory::wrap(mmap_directory).unwrap();
let mut managed_directory = ManagedDirectory::wrap(Box::new(mmap_directory)).unwrap();
let mut write = managed_directory.open_write(test_path1).unwrap();
write.write_all(&[0u8, 1u8]).unwrap();
write.terminate().unwrap();
Expand Down
9 changes: 6 additions & 3 deletions src/indexer/demuxer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ fn get_alive_bitsets(
/// The number of output_directories need to match max new segment ordinal from `demux_mapping`.
///
/// The ordinal of `segments` need to match the ordinals provided in `demux_mapping`.
pub fn demux<Dir: Directory>(
pub fn demux(
segments: &[Segment],
demux_mapping: &DemuxMapping,
target_settings: IndexSettings,
output_directories: Vec<Dir>,
output_directories: Vec<Box<dyn Directory>>,
) -> crate::Result<Vec<Index>> {
let mut indices = vec![];
for (target_segment_ord, output_directory) in output_directories.into_iter().enumerate() {
Expand Down Expand Up @@ -253,7 +253,10 @@ mod tests {
&segments,
&demux_mapping,
target_settings,
vec![RamDirectory::default(), RamDirectory::default()],
vec![
Box::new(RamDirectory::default()),
Box::new(RamDirectory::default()),
],
)?;

{
Expand Down
11 changes: 6 additions & 5 deletions src/indexer/segment_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ fn merge(
/// meant to work if you have an IndexWriter running for the origin indices, or
/// the destination Index.
#[doc(hidden)]
pub fn merge_indices<Dir: Directory>(
pub fn merge_indices<T: Into<Box<dyn Directory>>>(
indices: &[Index],
output_directory: Dir,
output_directory: T,
) -> crate::Result<Index> {
if indices.is_empty() {
// If there are no indices to merge, there is no need to do anything.
Expand Down Expand Up @@ -206,11 +206,11 @@ pub fn merge_indices<Dir: Directory>(
/// meant to work if you have an IndexWriter running for the origin indices, or
/// the destination Index.
#[doc(hidden)]
pub fn merge_filtered_segments<Dir: Directory>(
pub fn merge_filtered_segments<T: Into<Box<dyn Directory>>>(
segments: &[Segment],
target_settings: IndexSettings,
filter_doc_ids: Vec<Option<AliveBitSet>>,
output_directory: Dir,
output_directory: T,
) -> crate::Result<Index> {
if segments.is_empty() {
// If there are no indices to merge, there is no need to do anything.
Expand Down Expand Up @@ -690,6 +690,7 @@ mod tests {
use crate::indexer::segment_updater::merge_filtered_segments;
use crate::query::QueryParser;
use crate::schema::*;
use crate::Directory;
use crate::DocAddress;
use crate::Index;
use crate::Segment;
Expand Down Expand Up @@ -882,7 +883,7 @@ mod tests {
}

assert_eq!(indices.len(), 3);
let output_directory = RamDirectory::default();
let output_directory: Box<dyn Directory> = Box::new(RamDirectory::default());
let index = merge_indices(&indices, output_directory)?;
assert_eq!(index.schema(), schema);

Expand Down
2 changes: 1 addition & 1 deletion tests/failpoints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn test_failpoints_managed_directory_gc_if_delete_fails() {

let test_path: &'static Path = Path::new("some_path_for_test");

let ram_directory = RamDirectory::create();
let ram_directory = Box::new(RamDirectory::create());
let mut managed_directory = ManagedDirectory::wrap(ram_directory).unwrap();
managed_directory
.open_write(test_path)
Expand Down

0 comments on commit e2fbbc0

Please sign in to comment.