refactor: Rename Paths to their more specific exported names

pull/24376/head
Carol (Nichols || Goulding) 2021-08-18 11:16:56 -04:00
parent 10c9acf5c4
commit 997f61c5f6
4 changed files with 65 additions and 59 deletions

View File

@ -30,8 +30,8 @@ use tokio_stream::wrappers::ReceiverStream;
mod paths;
pub use paths::{
parquet_file::{Path as ParquetFilePath, PathParseError as ParquetFilePathParseError},
transaction_file::Path as TransactionFilePath,
parquet_file::{ParquetFilePath, ParquetFilePathParseError},
transaction_file::TransactionFilePath,
};
use paths::{DataPath, RootPath, TransactionsPath};

View File

@ -7,10 +7,10 @@ use object_store::{
};
pub mod parquet_file;
use parquet_file::Path as ParquetFilePath;
use parquet_file::ParquetFilePath;
pub mod transaction_file;
use transaction_file::Path as TransactionFilePath;
use transaction_file::TransactionFilePath;
/// The path all database root paths should be in. Used for listing all databases and building
/// database `RootPath`s in the same way. Not its own type because it's only needed ephemerally.

View File

@ -10,14 +10,14 @@ use uuid::Uuid;
/// Location of a Parquet file within a database's object store.
/// The exact format is an implementation detail and is subject to change.
#[derive(Debug, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct Path {
pub struct ParquetFilePath {
table_name: Arc<str>,
partition_key: Arc<str>,
chunk_id: u32,
uuid: Uuid,
}
impl Path {
impl ParquetFilePath {
/// Create a location for this chunk's parquet file. Calling this twice on the same `ChunkAddr`
/// will return different `parquet_file::Path`s.
pub fn new(chunk_addr: &ChunkAddr) -> Self {
@ -42,7 +42,7 @@ impl Path {
/// Create from serialized protobuf strings.
pub fn from_relative_dirs_and_file_name(
dirs_and_file_name: &DirsAndFileName,
) -> Result<Self, PathParseError> {
) -> Result<Self, ParquetFilePathParseError> {
let mut directories = dirs_and_file_name.directories.iter();
let table_name = directories
.next()
@ -86,7 +86,9 @@ impl Path {
}
// Deliberately pub(crate); this transformation should only happen within this crate
pub(crate) fn from_absolute(absolute_path: ObjStoPath) -> Result<Self, PathParseError> {
pub(crate) fn from_absolute(
absolute_path: ObjStoPath,
) -> Result<Self, ParquetFilePathParseError> {
let absolute_path: DirsAndFileName = absolute_path.into();
let mut absolute_dirs = absolute_path.directories.into_iter().fuse();
@ -106,7 +108,7 @@ impl Path {
}
}
impl From<&Self> for Path {
impl From<&Self> for ParquetFilePath {
fn from(borrowed: &Self) -> Self {
borrowed.clone()
}
@ -114,7 +116,7 @@ impl From<&Self> for Path {
#[derive(Snafu, Debug, PartialEq)]
#[allow(missing_docs)]
pub enum PathParseError {
pub enum ParquetFilePathParseError {
#[snafu(display("Could not find required table name"))]
MissingTableName,
@ -174,22 +176,22 @@ mod tests {
chunk_id: 13,
};
let p1 = Path::new(&chunk_addr);
let p2 = Path::new(&chunk_addr);
let p1 = ParquetFilePath::new(&chunk_addr);
let p2 = ParquetFilePath::new(&chunk_addr);
assert_ne!(p1, p2);
}
#[test]
fn test_parquet_file_path_deserialization() {
// Error cases
use PathParseError::*;
use ParquetFilePathParseError::*;
let mut df = DirsAndFileName::default();
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(matches!(result, Err(MissingTableName)), "got {:?}", result);
df.push_dir("foo");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(MissingPartitionKey)),
"got {:?}",
@ -197,7 +199,7 @@ mod tests {
);
df.push_dir("bar");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(MissingChunkId { .. })),
"got {:?}",
@ -206,7 +208,7 @@ mod tests {
let mut extra = df.clone();
extra.push_dir("nope");
let result = Path::from_relative_dirs_and_file_name(&extra);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&extra);
assert!(
matches!(result, Err(UnexpectedDirectory)),
"got {:?}",
@ -214,7 +216,7 @@ mod tests {
);
df.set_file_name("bleh");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidChunkId { .. })),
"got {:?}",
@ -222,7 +224,7 @@ mod tests {
);
df.set_file_name("3");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(MissingUuid { .. })),
"got {:?}",
@ -230,7 +232,7 @@ mod tests {
);
df.set_file_name("3.nope");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidUuid { .. })),
"got {:?}",
@ -239,11 +241,11 @@ mod tests {
let uuid = Uuid::new_v4();
df.set_file_name(&format!("3.{}", uuid));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(matches!(result, Err(MissingExtension)), "got {:?}", result);
df.set_file_name(&format!("3.{}.exe", uuid));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidExtension { .. })),
"got {:?}",
@ -251,7 +253,7 @@ mod tests {
);
df.set_file_name(&format!("3.{}.parquet.v6", uuid));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(UnexpectedExtension)),
"got {:?}",
@ -260,10 +262,10 @@ mod tests {
// Success case
df.set_file_name(&format!("3.{}.parquet", uuid));
let result = Path::from_relative_dirs_and_file_name(&df).unwrap();
let result = ParquetFilePath::from_relative_dirs_and_file_name(&df).unwrap();
assert_eq!(
result,
Path {
ParquetFilePath {
table_name: "foo".into(),
partition_key: "bar".into(),
chunk_id: 3,
@ -279,14 +281,14 @@ mod tests {
let object_store = make_object_store();
// Error cases
use PathParseError::*;
use ParquetFilePathParseError::*;
let mut path = object_store.new_path();
// incorrect directories are fine, we're assuming that list(data_path) scoped to the
// right directories so we don't check again on the way out
path.push_all_dirs(&["foo", "bar", "baz", "}*", "aoeu"]);
path.set_file_name("rules.pb");
let result = Path::from_absolute(path);
let result = ParquetFilePath::from_absolute(path);
assert!(
matches!(result, Err(InvalidChunkId { .. })),
"got: {:?}",
@ -296,7 +298,7 @@ mod tests {
let mut path = object_store.new_path();
path.push_all_dirs(&["foo", "bar", "baz", "}*", "aoeu"]);
// missing file name
let result = Path::from_absolute(path);
let result = ParquetFilePath::from_absolute(path);
assert!(matches!(result, Err(MissingChunkId)), "got: {:?}", result);
// Success case
@ -304,10 +306,10 @@ mod tests {
let mut path = object_store.new_path();
path.push_all_dirs(&["foo", "bar", "baz", "}*", "aoeu"]);
path.set_file_name(&format!("10.{}.parquet", uuid));
let result = Path::from_absolute(path);
let result = ParquetFilePath::from_absolute(path);
assert_eq!(
result.unwrap(),
Path {
ParquetFilePath {
table_name: "}*".into(),
partition_key: "aoeu".into(),
chunk_id: 10,
@ -319,7 +321,7 @@ mod tests {
#[test]
fn parquet_file_relative_dirs_and_file_path() {
let uuid = Uuid::new_v4();
let pfp = Path {
let pfp = ParquetFilePath {
table_name: "}*".into(),
partition_key: "aoeu".into(),
chunk_id: 10,
@ -330,7 +332,8 @@ mod tests {
dirs_and_file_name.to_string(),
format!("%7D%2A/aoeu/10.{}.parquet", uuid)
);
let round_trip = Path::from_relative_dirs_and_file_name(&dirs_and_file_name).unwrap();
let round_trip =
ParquetFilePath::from_relative_dirs_and_file_name(&dirs_and_file_name).unwrap();
assert_eq!(pfp, round_trip);
}
@ -343,7 +346,7 @@ mod tests {
IoxObjectStore::new(Arc::clone(&object_store), server_id, &database_name);
let uuid = Uuid::new_v4();
let pfp = Path {
let pfp = ParquetFilePath {
table_name: "}*".into(),
partition_key: "aoeu".into(),
chunk_id: 10,

View File

@ -15,7 +15,7 @@ const CHECKPOINT_FILE_SUFFIX: &str = "ckpt";
/// Location of a catalog transaction file within a database's object store.
/// The exact format is an implementation detail and is subject to change.
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct Path {
pub struct TransactionFilePath {
/// Transaction revision
pub revision_counter: u64,
/// Transaction identifier
@ -23,7 +23,7 @@ pub struct Path {
suffix: TransactionFileSuffix,
}
impl Path {
impl TransactionFilePath {
/// Create a new file path to store transaction info.
pub fn new_transaction(revision_counter: u64, uuid: Uuid) -> Self {
Self {
@ -62,7 +62,7 @@ impl Path {
/// Create from serialized protobuf strings.
pub fn from_relative_dirs_and_file_name(
dirs_and_file_name: &DirsAndFileName,
) -> Result<Self, PathParseError> {
) -> Result<Self, TransactionFilePathParseError> {
let mut directories = dirs_and_file_name.directories.iter();
let revision_counter = directories
@ -103,7 +103,9 @@ impl Path {
}
// Deliberately pub(crate); this transformation should only happen within this crate
pub(crate) fn from_absolute(absolute_path: ObjStoPath) -> Result<Self, PathParseError> {
pub(crate) fn from_absolute(
absolute_path: ObjStoPath,
) -> Result<Self, TransactionFilePathParseError> {
let absolute_path: DirsAndFileName = absolute_path.into();
let mut absolute_dirs = absolute_path.directories.into_iter().fuse();
@ -125,7 +127,7 @@ impl Path {
#[derive(Snafu, Debug, PartialEq)]
#[allow(missing_docs)]
pub enum PathParseError {
pub enum TransactionFilePathParseError {
#[snafu(display("Could not find required revision counter"))]
MissingRevisionCounter,
@ -213,20 +215,20 @@ mod tests {
fn is_checkpoint_works() {
let uuid = Uuid::new_v4();
let transaction = Path::new_transaction(0, uuid);
let transaction = TransactionFilePath::new_transaction(0, uuid);
assert!(!transaction.is_checkpoint());
let checkpoint = Path::new_checkpoint(0, uuid);
let checkpoint = TransactionFilePath::new_checkpoint(0, uuid);
assert!(checkpoint.is_checkpoint());
}
#[test]
fn test_transaction_file_path_deserialization() {
// Error cases
use PathParseError::*;
use TransactionFilePathParseError::*;
let mut df = DirsAndFileName::default();
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(MissingRevisionCounter)),
"got {:?}",
@ -234,7 +236,7 @@ mod tests {
);
df.push_dir("foo");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidRevisionCounter { .. })),
"got {:?}",
@ -244,7 +246,7 @@ mod tests {
let mut df = DirsAndFileName::default();
df.push_dir("00000000000000000123");
df.push_dir("foo");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(UnexpectedDirectory)),
"got {:?}",
@ -253,11 +255,11 @@ mod tests {
let mut df = DirsAndFileName::default();
df.push_dir("00000000000000000123");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(matches!(result, Err(MissingFileName)), "got {:?}", result);
df.set_file_name("foo");
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidUuid { .. })),
"got {:?}",
@ -267,11 +269,11 @@ mod tests {
let uuid = Uuid::new_v4();
df.set_file_name(&format!("{}", uuid));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(matches!(result, Err(MissingSuffix)), "got {:?}", result);
df.set_file_name(&format!("{}.exe", uuid));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(InvalidSuffix { .. })),
"got {:?}",
@ -279,7 +281,7 @@ mod tests {
);
df.set_file_name(&format!("{}.{}.foo", uuid, TRANSACTION_FILE_SUFFIX));
let result = Path::from_relative_dirs_and_file_name(&df);
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df);
assert!(
matches!(result, Err(UnexpectedExtension)),
"got {:?}",
@ -288,10 +290,10 @@ mod tests {
// Success case
df.set_file_name(&format!("{}.{}", uuid, TRANSACTION_FILE_SUFFIX));
let result = Path::from_relative_dirs_and_file_name(&df).unwrap();
let result = TransactionFilePath::from_relative_dirs_and_file_name(&df).unwrap();
assert_eq!(
result,
Path {
TransactionFilePath {
revision_counter: 123,
uuid,
suffix: TransactionFileSuffix::Transaction,
@ -306,14 +308,14 @@ mod tests {
let object_store = make_object_store();
// Error cases
use PathParseError::*;
use TransactionFilePathParseError::*;
let mut path = object_store.new_path();
// incorrect directories are fine, we're assuming that list(transactions_path) scoped to the
// right directories so we don't check again on the way out
path.push_all_dirs(&["foo", "bar", "baz", "}*", "aoeu"]);
path.set_file_name("rules.pb");
let result = Path::from_absolute(path);
let result = TransactionFilePath::from_absolute(path);
assert!(
matches!(result, Err(InvalidRevisionCounter { .. })),
"got: {:?}",
@ -323,7 +325,7 @@ mod tests {
let mut path = object_store.new_path();
path.push_all_dirs(&["foo", "bar", "baz", "00000000000000000123"]);
// missing file name
let result = Path::from_absolute(path);
let result = TransactionFilePath::from_absolute(path);
assert!(matches!(result, Err(MissingFileName)), "got: {:?}", result);
// Success case
@ -331,10 +333,10 @@ mod tests {
let mut path = object_store.new_path();
path.push_all_dirs(&["foo", "bar", "baz", "00000000000000000123"]);
path.set_file_name(&format!("{}.{}", uuid, CHECKPOINT_FILE_SUFFIX));
let result = Path::from_absolute(path);
let result = TransactionFilePath::from_absolute(path);
assert_eq!(
result.unwrap(),
Path {
TransactionFilePath {
revision_counter: 123,
uuid,
suffix: TransactionFileSuffix::Checkpoint,
@ -345,7 +347,7 @@ mod tests {
#[test]
fn transaction_file_relative_dirs_and_file_path() {
let uuid = Uuid::new_v4();
let tfp = Path {
let tfp = TransactionFilePath {
revision_counter: 555,
uuid,
suffix: TransactionFileSuffix::Transaction,
@ -355,7 +357,8 @@ mod tests {
dirs_and_file_name.to_string(),
format!("00000000000000000555/{}.{}", uuid, TRANSACTION_FILE_SUFFIX)
);
let round_trip = Path::from_relative_dirs_and_file_name(&dirs_and_file_name).unwrap();
let round_trip =
TransactionFilePath::from_relative_dirs_and_file_name(&dirs_and_file_name).unwrap();
assert_eq!(tfp, round_trip);
}
@ -368,7 +371,7 @@ mod tests {
IoxObjectStore::new(Arc::clone(&object_store), server_id, &database_name);
let uuid = Uuid::new_v4();
let tfp = Path {
let tfp = TransactionFilePath {
revision_counter: 555,
uuid,
suffix: TransactionFileSuffix::Checkpoint,