Skip to content

Commit

Permalink
FileHandle: Change from boxed to Arc. (#1415)
Browse files Browse the repository at this point in the history
* FileHandle: Change from boxed to Arc.

Changing from a Box<dyn FileHandle> to an Arc<dyn FileHandle> would
allow for a user of tantivy to manage file handles outside of tantivy
and be able to manage their life cycle.

* Fix: Rust linter
  • Loading branch information
Pier-Olivier Thibault authored Jul 21, 2022
1 parent 23fe73a commit 775e936
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/directory/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub trait Directory: DirectoryClone + fmt::Debug + Send + Sync + 'static {
///
/// Users of `Directory` should typically call `Directory::open_read(...)`,
/// while `Directory` implementor should implement `get_file_handle()`.
fn get_file_handle(&self, path: &Path) -> Result<Box<dyn FileHandle>, OpenReadError>;
fn get_file_handle(&self, path: &Path) -> Result<Arc<dyn FileHandle>, OpenReadError>;

/// Once a virtual file is open, its data may not
/// change.
Expand Down
17 changes: 9 additions & 8 deletions src/directory/file_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<B> From<B> for FileSlice
where B: StableDeref + Deref<Target = [u8]> + 'static + Send + Sync
{
fn from(bytes: B) -> FileSlice {
FileSlice::new(Box::new(OwnedBytes::new(bytes)))
FileSlice::new(Arc::new(OwnedBytes::new(bytes)))
}
}

Expand All @@ -75,17 +75,17 @@ impl fmt::Debug for FileSlice {

impl FileSlice {
/// Wraps a FileHandle.
pub fn new(file_handle: Box<dyn FileHandle>) -> Self {
pub fn new(file_handle: Arc<dyn FileHandle>) -> Self {
let num_bytes = file_handle.len();
FileSlice::new_with_num_bytes(file_handle, num_bytes)
}

/// Wraps a FileHandle.
#[doc(hidden)]
#[must_use]
pub fn new_with_num_bytes(file_handle: Box<dyn FileHandle>, num_bytes: usize) -> Self {
pub fn new_with_num_bytes(file_handle: Arc<dyn FileHandle>, num_bytes: usize) -> Self {
FileSlice {
data: Arc::from(file_handle),
data: file_handle,
range: 0..num_bytes,
}
}
Expand Down Expand Up @@ -235,14 +235,15 @@ impl FileHandle for OwnedBytes {
#[cfg(test)]
mod tests {
use std::io;
use std::sync::Arc;

use common::HasLen;

use super::{FileHandle, FileSlice};

#[test]
fn test_file_slice() -> io::Result<()> {
let file_slice = FileSlice::new(Box::new(b"abcdef".as_ref()));
let file_slice = FileSlice::new(Arc::new(b"abcdef".as_ref()));
assert_eq!(file_slice.len(), 6);
assert_eq!(file_slice.slice_from(2).read_bytes()?.as_slice(), b"cdef");
assert_eq!(file_slice.slice_to(2).read_bytes()?.as_slice(), b"ab");
Expand Down Expand Up @@ -286,7 +287,7 @@ mod tests {

#[test]
fn test_slice_simple_read() -> io::Result<()> {
let slice = FileSlice::new(Box::new(&b"abcdef"[..]));
let slice = FileSlice::new(Arc::new(&b"abcdef"[..]));
assert_eq!(slice.len(), 6);
assert_eq!(slice.read_bytes()?.as_ref(), b"abcdef");
assert_eq!(slice.slice(1..4).read_bytes()?.as_ref(), b"bcd");
Expand All @@ -295,15 +296,15 @@ mod tests {

#[test]
fn test_slice_read_slice() -> io::Result<()> {
let slice_deref = FileSlice::new(Box::new(&b"abcdef"[..]));
let slice_deref = FileSlice::new(Arc::new(&b"abcdef"[..]));
assert_eq!(slice_deref.read_bytes_slice(1..4)?.as_ref(), b"bcd");
Ok(())
}

#[test]
#[should_panic(expected = "end of requested range exceeds the fileslice length (10 > 6)")]
fn test_slice_read_slice_invalid_range_exceeds() {
let slice_deref = FileSlice::new(Box::new(&b"abcdef"[..]));
let slice_deref = FileSlice::new(Arc::new(&b"abcdef"[..]));
assert_eq!(
slice_deref.read_bytes_slice(0..10).unwrap().as_ref(),
b"bcd"
Expand Down
9 changes: 5 additions & 4 deletions src/directory/footer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl<W: TerminatingWrite> TerminatingWrite for FooterProxy<W> {
mod tests {

use std::io;
use std::sync::Arc;

use common::BinarySerializable;

Expand All @@ -168,7 +169,7 @@ mod tests {
let footer = Footer::new(123);
footer.append_footer(&mut buf).unwrap();
let owned_bytes = OwnedBytes::new(buf);
let fileslice = FileSlice::new(Box::new(owned_bytes));
let fileslice = FileSlice::new(Arc::new(owned_bytes));
let (footer_deser, _body) = Footer::extract_footer(fileslice).unwrap();
assert_eq!(footer_deser.crc(), footer.crc());
}
Expand All @@ -181,7 +182,7 @@ mod tests {

let owned_bytes = OwnedBytes::new(buf);

let fileslice = FileSlice::new(Box::new(owned_bytes));
let fileslice = FileSlice::new(Arc::new(owned_bytes));
let err = Footer::extract_footer(fileslice).unwrap_err();
assert_eq!(
err.to_string(),
Expand All @@ -198,7 +199,7 @@ mod tests {

let owned_bytes = OwnedBytes::new(buf);

let fileslice = FileSlice::new(Box::new(owned_bytes));
let fileslice = FileSlice::new(Arc::new(owned_bytes));
let err = Footer::extract_footer(fileslice).unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof);
assert_eq!(
Expand All @@ -217,7 +218,7 @@ mod tests {

let owned_bytes = OwnedBytes::new(buf);

let fileslice = FileSlice::new(Box::new(owned_bytes));
let fileslice = FileSlice::new(Arc::new(owned_bytes));
let err = Footer::extract_footer(fileslice).unwrap_err();
assert_eq!(err.kind(), io::ErrorKind::InvalidData);
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions src/directory/managed_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ impl ManagedDirectory {
}

impl Directory for ManagedDirectory {
fn get_file_handle(&self, path: &Path) -> Result<Box<dyn FileHandle>, OpenReadError> {
fn get_file_handle(&self, path: &Path) -> Result<Arc<dyn FileHandle>, OpenReadError> {
let file_slice = self.open_read(path)?;
Ok(Box::new(file_slice))
Ok(Arc::new(file_slice))
}

fn open_read(&self, path: &Path) -> result::Result<FileSlice, OpenReadError> {
Expand Down
4 changes: 2 additions & 2 deletions src/directory/mmap_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ pub(crate) fn atomic_write(path: &Path, content: &[u8]) -> io::Result<()> {
}

impl Directory for MmapDirectory {
fn get_file_handle(&self, path: &Path) -> result::Result<Box<dyn FileHandle>, OpenReadError> {
fn get_file_handle(&self, path: &Path) -> result::Result<Arc<dyn FileHandle>, OpenReadError> {
debug!("Open Read {:?}", path);
let full_path = self.resolve_path(path);

Expand All @@ -331,7 +331,7 @@ impl Directory for MmapDirectory {
})
.unwrap_or_else(OwnedBytes::empty);

Ok(Box::new(owned_bytes))
Ok(Arc::new(owned_bytes))
}

/// Any entry associated to the path in the mmap will be
Expand Down
4 changes: 2 additions & 2 deletions src/directory/ram_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ impl RamDirectory {
}

impl Directory for RamDirectory {
fn get_file_handle(&self, path: &Path) -> Result<Box<dyn FileHandle>, OpenReadError> {
fn get_file_handle(&self, path: &Path) -> Result<Arc<dyn FileHandle>, OpenReadError> {
let file_slice = self.open_read(path)?;
Ok(Box::new(file_slice))
Ok(Arc::new(file_slice))
}

fn open_read(&self, path: &Path) -> result::Result<FileSlice, OpenReadError> {
Expand Down
3 changes: 2 additions & 1 deletion src/termdict/sstable_termdict/termdict.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::io;
use std::sync::Arc;

use common::BinarySerializable;
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -138,7 +139,7 @@ impl TermDictionary {
}

pub fn from_bytes(owned_bytes: OwnedBytes) -> crate::Result<TermDictionary> {
TermDictionary::open(FileSlice::new(Box::new(owned_bytes)))
TermDictionary::open(FileSlice::new(Arc::new(owned_bytes)))
}

/// Creates an empty term dictionary which contains no terms.
Expand Down

0 comments on commit 775e936

Please sign in to comment.