Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tiledb-rs error styling part 1: Context::capi_call returns its own error type #191

Merged
merged 3 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions tiledb/api/src/array/attribute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,12 @@ impl Attribute {
return Ok(None);
}

TDBString {
raw: RawTDBString::Owned(c_str),
}
.to_string()
.map(Some)
Ok(Some(
TDBString {
raw: RawTDBString::Owned(c_str),
}
.to_string()?,
))
}
}

Expand Down
4 changes: 2 additions & 2 deletions tiledb/api/src/array/enumeration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ impl Enumeration {
ffi::tiledb_enumeration_get_name(ctx, c_enmr, &mut c_str)
})?;

TDBString {
Ok(TDBString {
raw: RawTDBString::Owned(c_str),
}
.to_string()
.to_string()?)
}

pub fn datatype(&self) -> TileDBResult<Datatype> {
Expand Down
2 changes: 1 addition & 1 deletion tiledb/api/src/array/fragment_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl FragmentInfoInternal {
})?;

let str = TDBString::from_raw(RawTDBString::Owned(c_str));
str.to_string()
Ok(str.to_string()?)
}

pub fn fragment_uri(&self, frag_idx: u32) -> TileDBResult<String> {
Expand Down
47 changes: 28 additions & 19 deletions tiledb/api/src/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use anyhow::anyhow;

use crate::array::enumeration::RawEnumeration;
use crate::array::schema::RawSchema;
use crate::context::{CApiInterface, Context, ContextBound};
use crate::context::{
CApiInterface, CApiResult, Context, ContextBound, ObjectType,
};
use crate::datatype::PhysicalType;
use crate::error::{DatatypeError, Error};
use crate::key::LookupKey;
Expand Down Expand Up @@ -86,7 +88,9 @@ impl FromStr for Encryption {
)
};
if c_ret == ffi::TILEDB_OK {
Self::try_from(c_encryption)
// SAFETY: libtiledb returned TILEDB_OK, hence c_ret must be valid
let encryption_type = Self::try_from(c_encryption).unwrap();
Ok(encryption_type)
} else {
Err(Error::InvalidArgument(anyhow!(format!(
"Invalid encryption type: {}",
Expand All @@ -110,20 +114,19 @@ impl From<Encryption> for ffi::tiledb_encryption_type_t {
}

impl TryFrom<ffi::tiledb_encryption_type_t> for Encryption {
type Error = crate::error::Error;
type Error = ffi::tiledb_encryption_type_t;

fn try_from(value: ffi::tiledb_encryption_type_t) -> TileDBResult<Self> {
fn try_from(
value: ffi::tiledb_encryption_type_t,
) -> Result<Self, Self::Error> {
match value {
ffi::tiledb_encryption_type_t_TILEDB_NO_ENCRYPTION => {
Ok(Self::Unencrypted)
}
ffi::tiledb_encryption_type_t_TILEDB_AES_256_GCM => {
Ok(Self::Aes256Gcm)
}
_ => Err(Self::Error::LibTileDB(format!(
"Invalid encryption type: {}",
value
))),
_ => Err(value),
}
}
}
Expand Down Expand Up @@ -187,14 +190,13 @@ impl Array {
Ok(())
}

pub fn exists<S>(context: &Context, uri: S) -> TileDBResult<bool>
pub fn exists<S>(context: &Context, uri: S) -> CApiResult<bool>
where
S: AsRef<str>,
{
Ok(matches!(
context.object_type(uri)?,
Some(crate::context::ObjectType::Array)
))
context
.object_type(uri)
.map(|object_type| matches!(object_type, Some(ObjectType::Array)))
}

/// Returns the manner in which the array located at `uri` is encrypted.
Expand All @@ -213,7 +215,9 @@ impl Array {
)
})?;

Encryption::try_from(c_encryption_type)
// SAFETY: libtiledb returned TILEDB_OK hence c_encryption_type is valid
let encryption_type = Encryption::try_from(c_encryption_type).unwrap();
Ok(encryption_type)
}

/// Opens the array located at `uri` for queries of type `mode` using default configurations.
Expand Down Expand Up @@ -416,7 +420,8 @@ impl Array {
c_array_uri.as_ptr(),
unwrap_config_to_ptr(config),
)
})
})?;
Ok(())
}

/// Upgrades an array to the latest format version.
Expand All @@ -435,7 +440,8 @@ impl Array {
c_array_uri.as_ptr(),
unwrap_config_to_ptr(config),
)
})
})?;
Ok(())
}

/// Depending on the consolidation mode in the config, consolidates either the fragment files,
Expand All @@ -455,7 +461,8 @@ impl Array {
c_array_uri.as_ptr(),
unwrap_config_to_ptr(config),
)
})
})?;
Ok(())
}

/// Consolidates the given fragment URIs into a single fragment.
Expand Down Expand Up @@ -488,7 +495,8 @@ impl Array {
fragment_names_ptr.len() as u64,
unwrap_config_to_ptr(config),
)
})
})?;
Ok(())
}

/// Removes the array located at [array_uri].
Expand All @@ -503,7 +511,8 @@ impl Array {

context.capi_call(|ctx| unsafe {
ffi::tiledb_array_delete(ctx, c_array_uri.as_ptr())
})
})?;
Ok(())
}

// Implements `dimension_nonempty_domain` for dimensions with CellValNum::Fixed
Expand Down
9 changes: 5 additions & 4 deletions tiledb/api/src/array/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,8 @@ impl Builder {
let c_schema = *self.schema.raw;
self.capi_call(|ctx| unsafe {
ffi::tiledb_array_schema_check(ctx, c_schema)
})
.map(|_| self.schema)
})?;
Ok(self.schema)
}
}

Expand Down Expand Up @@ -736,6 +736,7 @@ mod tests {
use crate::array::{
AttributeBuilder, DimensionBuilder, DimensionConstraints, DomainBuilder,
};
use crate::context::CApiError;
use crate::filter::{
CompressionData, CompressionType, FilterData, FilterListBuilder,
};
Expand Down Expand Up @@ -1553,7 +1554,7 @@ mod tests {
datatype,
);
assert_eq!(allowed, r.is_ok(), "try_construct => {:?}", r.err());
if let Err(Error::LibTileDB(s)) = r {
if let Err(Error::LibTileDB(CApiError::Error(s))) = r {
assert!(
s.contains("not a valid Dimension Datatype")
|| s.contains("do not support dimension datatype"),
Expand Down Expand Up @@ -1581,7 +1582,7 @@ mod tests {
datatype,
);
assert_eq!(allowed, r.is_ok(), "try_construct => {:?}", r.err());
if let Err(Error::LibTileDB(s)) = r {
if let Err(Error::LibTileDB(CApiError::Error(s))) = r {
assert!(
s.contains("not a valid Dimension Datatype")
|| s.contains("do not support dimension datatype"),
Expand Down
38 changes: 17 additions & 21 deletions tiledb/api/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::ops::Deref;

use crate::error::{Error, RawError};
use crate::Result as TileDBResult;
use crate::context::{CApiError, CApiResult, RawError};

pub(crate) enum RawConfig {
Owned(*mut ffi::tiledb_config_t),
Expand Down Expand Up @@ -59,7 +58,7 @@ impl Config {
*self.raw
}

pub fn new() -> TileDBResult<Config> {
pub fn new() -> CApiResult<Config> {
let mut c_cfg: *mut ffi::tiledb_config_t = out_ptr!();
let mut c_err: *mut ffi::tiledb_error_t = std::ptr::null_mut();
let res = unsafe { ffi::tiledb_config_alloc(&mut c_cfg, &mut c_err) };
Expand All @@ -68,15 +67,15 @@ impl Config {
raw: RawConfig::Owned(c_cfg),
})
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}

pub(crate) fn from_raw(raw: RawConfig) -> Self {
Self { raw }
}

pub fn set<B>(&mut self, key: &str, val: B) -> TileDBResult<()>
pub fn set<B>(&mut self, key: &str, val: B) -> CApiResult<()>
where
B: AsRef<[u8]>,
{
Expand All @@ -96,11 +95,11 @@ impl Config {
if res == ffi::TILEDB_OK {
Ok(())
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}

pub fn with<B>(self, key: &str, val: B) -> TileDBResult<Self>
pub fn with<B>(self, key: &str, val: B) -> CApiResult<Self>
where
B: AsRef<[u8]>,
{
Expand All @@ -109,7 +108,7 @@ impl Config {
Ok(s)
}

pub fn get(&self, key: &str) -> TileDBResult<Option<String>> {
pub fn get(&self, key: &str) -> CApiResult<Option<String>> {
let c_key =
std::ffi::CString::new(key).expect("Error creating CString");
let mut val = std::ptr::null::<std::os::raw::c_char>();
Expand All @@ -128,24 +127,21 @@ impl Config {
} else if res == ffi::TILEDB_OK {
Ok(None)
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}

pub fn set_common_option(
&mut self,
opt: &CommonOption,
) -> TileDBResult<()> {
pub fn set_common_option(&mut self, opt: &CommonOption) -> CApiResult<()> {
opt.apply(self)
}

pub fn with_common_option(self, opt: &CommonOption) -> TileDBResult<Self> {
pub fn with_common_option(self, opt: &CommonOption) -> CApiResult<Self> {
let mut s = self;
s.set_common_option(opt)?;
Ok(s)
}

pub fn unset(&mut self, key: &str) -> TileDBResult<()> {
pub fn unset(&mut self, key: &str) -> CApiResult<()> {
let c_key =
std::ffi::CString::new(key).expect("Error creating CString");
let mut c_err: *mut ffi::tiledb_error_t = std::ptr::null_mut();
Expand All @@ -159,11 +155,11 @@ impl Config {
if res == ffi::TILEDB_OK {
Ok(())
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}

pub fn load(&mut self, path: &str) -> TileDBResult<()> {
pub fn load(&mut self, path: &str) -> CApiResult<()> {
let c_path =
std::ffi::CString::new(path).expect("Error creating CString");
let mut c_err: *mut ffi::tiledb_error_t = std::ptr::null_mut();
Expand All @@ -177,11 +173,11 @@ impl Config {
if res == ffi::TILEDB_OK {
Ok(())
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}

pub fn save(&mut self, path: &str) -> TileDBResult<()> {
pub fn save(&mut self, path: &str) -> CApiResult<()> {
let c_path =
std::ffi::CString::new(path).expect("Error creating CString");
let mut c_err: *mut ffi::tiledb_error_t = std::ptr::null_mut();
Expand All @@ -195,7 +191,7 @@ impl Config {
if res == ffi::TILEDB_OK {
Ok(())
} else {
Err(Error::from(RawError::Owned(c_err)))
Err(CApiError::from(RawError::Owned(c_err)))
}
}
}
Expand Down Expand Up @@ -312,7 +308,7 @@ pub enum CommonOption {
}

impl CommonOption {
fn apply(&self, config: &mut Config) -> TileDBResult<()> {
fn apply(&self, config: &mut Config) -> CApiResult<()> {
match self {
Self::Aes256GcmEncryptionKey(ref key) => {
config.set("sm.encryption_type", "AES_256_GCM")?;
Expand Down
Loading
Loading